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 952078b62ee7222b67857845b46eb48466567ed0 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 | 2 +- superset/db_engine_specs/gsheets.py | 24 +++++++-------- superset/models/core.py | 11 +++++-- superset/static/assets/.gitkeep | 0 7 files changed, 46 insertions(+), 52 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..b501754ea5 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/constants.ts +++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/constants.ts @@ -52,9 +52,9 @@ export const FormFieldOrder = [ 'http_path_field', 'database_name', 'project_id', + 'catalog', 'credentials_info', 'service_account_info', - 'catalog', 'query', 'encryption', 'account', diff --git a/superset/db_engine_specs/gsheets.py b/superset/db_engine_specs/gsheets.py index 65afd9aee9..20c2a40a00 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 = 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): @@ -221,18 +233,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
