This is an automated email from the ASF dual-hosted git repository. beto pushed a commit to branch gsheets-improvement in repository https://gitbox.apache.org/repos/asf/superset.git
commit fdf753cf5b783aa3102e2dfaab31d1bdc44cbe7c Author: Beto Dealmeida <[email protected]> AuthorDate: Wed Jan 29 14:08:00 2025 -0500 WIP --- .../DatabaseConnectionForm/EncryptedField.tsx | 35 +++----------------- .../DatabaseConnectionForm/OAuth2ClientField.tsx | 17 +++++++--- .../DatabaseConnectionForm/TableCatalog.tsx | 9 ++++- .../DatabaseConnectionForm/constants.ts | 6 ++-- superset/commands/database/validate.py | 2 +- superset/databases/api.py | 10 ++++-- superset/databases/schemas.py | 4 ++- superset/db_engine_specs/gsheets.py | 38 +++++++++++++++------- superset/models/core.py | 11 +++++-- superset/static/assets/.gitkeep | 0 10 files changed, 73 insertions(+), 59 deletions(-) diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx index fd11cd3271..12c3ece190 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx @@ -52,10 +52,7 @@ export const EncryptedField = ({ const [fileToUpload, setFileToUpload] = useState<string | null | undefined>( null, ); - const [isPublic, setIsPublic] = useState<boolean>(true); - const showCredentialsInfo = - db?.engine === 'gsheets' ? !isEditMode && !isPublic : !isEditMode; - const isEncrypted = isEditMode && db?.masked_encrypted_extra !== '{}'; + const showCredentialsInfo = !isEditMode; const encryptedField = db?.engine && encryptedCredentialsMap[db.engine]; const paramValue = db?.parameters?.[encryptedField]; const encryptedValue = @@ -64,33 +61,9 @@ export const EncryptedField = ({ : paramValue; return ( <CredentialInfoForm> - {db?.engine === 'gsheets' && ( - <div className="catalog-type-select"> - <FormLabel - css={(theme: SupersetTheme) => labelMarginBottom(theme)} - required - > - {t('Type of Google Sheets allowed')} - </FormLabel> - <AntdSelect - style={{ width: '100%' }} - defaultValue={isEncrypted ? 'false' : 'true'} - onChange={(value: string) => - setIsPublic(castStringToBoolean(value)) - } - > - <AntdSelect.Option value="true" key={1}> - {t('Publicly shared sheets only')} - </AntdSelect.Option> - <AntdSelect.Option value="false" key={2}> - {t('Public and privately shared sheets')} - </AntdSelect.Option> - </AntdSelect> - </div> - )} {showCredentialsInfo && ( <> - <FormLabel required> + <FormLabel> {t('How do you want to enter service account credentials?')} </FormLabel> <AntdSelect @@ -112,7 +85,7 @@ export const EncryptedField = ({ isEditMode || editNewDb ? ( <div className="input-container"> - <FormLabel required>{t('Service Account')}</FormLabel> + <FormLabel>{t('Service Account')}</FormLabel> <textarea className="input-form" name={encryptedField} @@ -133,7 +106,7 @@ export const EncryptedField = ({ css={(theme: SupersetTheme) => infoTooltip(theme)} > <div css={{ display: 'flex', alignItems: 'center' }}> - <FormLabel required>{t('Upload Credentials')}</FormLabel> + <FormLabel>{t('Upload Credentials')}</FormLabel> <InfoTooltip tooltip={t( 'Use the JSON file you automatically downloaded when creating your service account.', diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.tsx index fac3c33318..d8ffefd06d 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.tsx @@ -40,16 +40,25 @@ interface OAuth2ClientInfo { scope: string; } -export const OAuth2ClientField = ({ changeMethods, db }: FieldPropTypes) => { +export const OAuth2ClientField = ({ + changeMethods, + db, + default_value: defaultValue, +}: FieldPropTypes) => { const encryptedExtra = JSON.parse(db?.masked_encrypted_extra || '{}'); const [oauth2ClientInfo, setOauth2ClientInfo] = useState<OAuth2ClientInfo>({ id: encryptedExtra.oauth2_client_info?.id || '', secret: encryptedExtra.oauth2_client_info?.secret || '', authorization_request_uri: - encryptedExtra.oauth2_client_info?.authorization_request_uri || '', + encryptedExtra.oauth2_client_info?.authorization_request_uri || + defaultValue?.authorization_request_uri || + '', token_request_uri: - encryptedExtra.oauth2_client_info?.token_request_uri || '', - scope: encryptedExtra.oauth2_client_info?.scope || '', + encryptedExtra.oauth2_client_info?.token_request_uri || + defaultValue?.token_request_uri || + '', + scope: + encryptedExtra.oauth2_client_info?.scope || defaultValue?.scope || '', }); const handleChange = (key: any) => (e: any) => { diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/TableCatalog.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/TableCatalog.tsx index 4fe71d0906..91b6a63c0b 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/TableCatalog.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/TableCatalog.tsx @@ -40,7 +40,7 @@ export const TableCatalog = ({ <div> {tableCatalog?.map((sheet: CatalogObject, idx: number) => ( <> - <FormLabel className="catalog-label" required> + <FormLabel className="catalog-label"> {t('Google Sheet Name and URL')} </FormLabel> <div className="catalog-name"> @@ -105,6 +105,13 @@ export const TableCatalog = ({ + {t('Add sheet')} </StyledFooterButton> </div> + <div className="helper"> + <div> + {t( + 'In order to connect to non-public sheets you need to either provide a service account or configure an OAuth2 client.', + )} + </div> + </div> </StyledCatalogTable> ); }; diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/constants.ts b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/constants.ts index a0bf7a5dd7..4512d1f707 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/constants.ts +++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/constants.ts @@ -52,16 +52,16 @@ export const FormFieldOrder = [ 'http_path_field', 'database_name', 'project_id', + 'catalog', 'credentials_info', 'service_account_info', - 'catalog', 'query', 'encryption', 'account', 'warehouse', 'role', 'ssh', - 'oauth2_client', + 'oauth2_client_info', ]; const extensionsRegistry = getExtensionsRegistry(); @@ -79,7 +79,7 @@ export const FORM_FIELD_MAP = { default_schema: defaultSchemaField, username: usernameField, password: passwordField, - oauth2_client: OAuth2ClientField, + oauth2_client_info: OAuth2ClientField, access_token: accessTokenField, database_name: displayField, query: queryField, diff --git a/superset/commands/database/validate.py b/superset/commands/database/validate.py index e1584d59c0..eda5d75bed 100644 --- a/superset/commands/database/validate.py +++ b/superset/commands/database/validate.py @@ -96,7 +96,7 @@ class ValidateDatabaseParametersCommand(BaseCommand): server_cert=self._properties.get("server_cert", ""), extra=self._properties.get("extra", "{}"), impersonate_user=self._properties.get("impersonate_user", False), - encrypted_extra=serialized_encrypted_extra, + encrypted_extra=json.dumps(encrypted_extra), ) database.set_sqlalchemy_uri(sqlalchemy_uri) database.db_engine_spec.mutate_db_for_connection_test(database) diff --git a/superset/databases/api.py b/superset/databases/api.py index 5c97ac688b..ba0aee0e15 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -535,12 +535,18 @@ class DatabaseRestApi(BaseSupersetModelRestApi): $ref: '#/components/responses/500' """ try: + print(3) item = self.edit_model_schema.load(request.json) # This validates custom Schema with custom validations except ValidationError as error: + print(request.json) + print(error) + print(4) return self.response_400(message=error.messages) try: + print(1) changed_model = UpdateDatabaseCommand(pk, item).run() + print(2) # Return censored version for sqlalchemy URI item["sqlalchemy_uri"] = changed_model.sqlalchemy_uri if changed_model.parameters: @@ -1891,9 +1897,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi): @protect() @statsd_metrics @event_logger.log_this_with_context( - action=lambda self, - *args, - **kwargs: f"{self.__class__.__name__}.columnar_upload", + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.columnar_upload", log_to_statsd=False, ) @requires_form_data diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py index 0c821a6f34..0f16c992ab 100644 --- a/superset/databases/schemas.py +++ b/superset/databases/schemas.py @@ -709,7 +709,9 @@ class TableMetadataResponseSchema(Schema): TableMetadataPrimaryKeyResponseSchema, metadata={"description": "Primary keys metadata"}, ) - selectStar = fields.String(metadata={"description": "SQL select star"}) # noqa: N815 + selectStar = fields.String( + metadata={"description": "SQL select star"} + ) # noqa: N815 class TableExtraMetadataResponseSchema(Schema): diff --git a/superset/db_engine_specs/gsheets.py b/superset/db_engine_specs/gsheets.py index 65afd9aee9..6fd5baaaa8 100644 --- a/superset/db_engine_specs/gsheets.py +++ b/superset/db_engine_specs/gsheets.py @@ -67,6 +67,18 @@ class GSheetsParametersSchema(Schema): "field_name": "service_account_info", }, ) + oauth2_client_info = EncryptedString( + required=False, + metadata={ + "description": "OAuth2 client information", + "default": { + "scope": " ".join(SCOPES), + "authorization_request_uri": "https://accounts.google.com/o/oauth2/v2/auth";, + "token_request_uri": "https://oauth2.googleapis.com/token";, + }, + }, + allow_none=True, + ) class GSheetsParametersType(TypedDict): @@ -165,8 +177,22 @@ class GSheetsEngineSpec(ShillelaghEngineSpec): _: GSheetsParametersType, encrypted_extra: None | (dict[str, Any]) = None, ) -> str: + if encrypted_extra and "oauth2_client_info" in encrypted_extra: + del encrypted_extra["oauth2_client_info"] + return "gsheets://" + @staticmethod + def update_params_from_encrypted_extra( + database: Database, + params: dict[str, Any], + ) -> None: + """ + Remove `oauth2_client_info` from `encrypted_extra`. + """ + if "oauth2_client_info" in params.get("encrypted_extra", {}): + del params["encrypted_extra"]["oauth2_client_info"] + @classmethod def get_parameters_from_uri( cls, @@ -221,18 +247,6 @@ class GSheetsEngineSpec(ShillelaghEngineSpec): if isinstance(encrypted_credentials, str): encrypted_credentials = json.loads(encrypted_credentials) - if not table_catalog: - # Allowing users to submit empty catalogs - errors.append( - SupersetError( - message="Sheet name is required", - error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR, - level=ErrorLevel.WARNING, - extra={"catalog": {"idx": 0, "name": True}}, - ), - ) - return errors - # We need a subject in case domain wide delegation is set, otherwise the # check will fail. This means that the admin will be able to add sheets # that only they have access, even if later users are not able to access diff --git a/superset/models/core.py b/superset/models/core.py index 6f32383ab8..ddfe32c280 100755 --- a/superset/models/core.py +++ b/superset/models/core.py @@ -38,6 +38,7 @@ import sqlalchemy as sqla import sshtunnel from flask import g, request from flask_appbuilder import Model +from marshmallow.exceptions import ValidationError from sqlalchemy import ( Boolean, Column, @@ -1133,9 +1134,13 @@ class Database(Model, AuditMixinNullable, ImportExportMixin): # pylint: disable admins to create custom OAuth2 clients from the Superset UI, and assign them to specific databases. """ - encrypted_extra = json.loads(self.encrypted_extra or "{}") - oauth2_client_info = encrypted_extra.get("oauth2_client_info", {}) - return bool(oauth2_client_info) or self.db_engine_spec.is_oauth2_enabled() + try: + client_config = self.db_engine_spec.get_oauth2_config() + except ValidationError: + logger.warning("Invalid OAuth2 client configuration for database %s", self) + client_config = None + + return client_config or self.db_engine_spec.is_oauth2_enabled() def get_oauth2_config(self) -> OAuth2ClientConfig | None: """ diff --git a/superset/static/assets/.gitkeep b/superset/static/assets/.gitkeep deleted file mode 100644 index e69de29bb2..0000000000
