You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by be...@apache.org on 2025/01/29 22:08:30 UTC
(superset) branch gsheets-improvement updated (9603767f7a -> 00d302d710)
This is an automated email from the ASF dual-hosted git repository.
beto pushed a change to branch gsheets-improvement
in repository https://gitbox.apache.org/repos/asf/superset.git
discard 9603767f7a feat: improve GSheets OAuth2
new 00d302d710 feat: improve GSheets OAuth2
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (9603767f7a)
\
N -- N -- N refs/heads/gsheets-improvement (00d302d710)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
superset/db_engine_specs/base.py | 3 ---
1 file changed, 3 deletions(-)
(superset) 01/01: feat: improve GSheets OAuth2
Posted by be...@apache.org.
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 00d302d7101f54d2983970269f7e55a012d55703
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Wed Jan 29 14:08:00 2025 -0500
feat: improve GSheets OAuth2
---
.../DatabaseConnectionForm/EncryptedField.tsx | 35 +++-----------------
.../DatabaseConnectionForm/OAuth2ClientField.tsx | 24 ++++++++++----
.../DatabaseConnectionForm/TableCatalog.tsx | 9 ++++-
.../DatabaseConnectionForm/constants.ts | 6 ++--
superset-frontend/src/features/databases/types.ts | 2 +-
superset/commands/database/validate.py | 2 +-
superset/db_engine_specs/base.py | 10 +++---
superset/db_engine_specs/gsheets.py | 38 +++++++++++++++-------
superset/models/core.py | 19 +++++++----
superset/sqllab/api.py | 3 ++
superset/sqllab/sql_json_executer.py | 1 -
11 files changed, 81 insertions(+), 68 deletions(-)
diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx
index 147b983c45..7f4f11917d 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 as keyof typeof encryptedCredentialsMap];
@@ -67,33 +64,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
@@ -115,7 +88,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}
@@ -140,7 +113,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..627ad51622 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.tsx
@@ -22,7 +22,7 @@ import { useState } from 'react';
import Collapse from 'src/components/Collapse';
import { Input } from 'src/components/Input';
import { FormItem } from 'src/components/Form';
-import { FieldPropTypes } from '../../types';
+import { CustomParametersChangeType, FieldPropTypes } from '../../types';
const LABELS = {
CLIENT_ID: 'Client ID',
@@ -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) => {
@@ -60,13 +69,14 @@ export const OAuth2ClientField = ({ changeMethods, db }: FieldPropTypes) => {
setOauth2ClientInfo(updatedInfo);
- const event = {
+ const event: CustomParametersChangeType = {
target: {
+ type: 'object',
name: 'oauth2_client_info',
value: updatedInfo,
},
};
- changeMethods.onEncryptedExtraInputChange(event);
+ changeMethods.onParametersChange(event);
};
return (
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-frontend/src/features/databases/types.ts b/superset-frontend/src/features/databases/types.ts
index 209100941b..a41499b980 100644
--- a/superset-frontend/src/features/databases/types.ts
+++ b/superset-frontend/src/features/databases/types.ts
@@ -265,7 +265,7 @@ export interface ExtraJson {
}
export type CustomTextType = {
- value?: string | boolean | number;
+ value?: string | boolean | number | object;
type?: string | null;
name?: string;
checked?: boolean;
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/db_engine_specs/base.py b/superset/db_engine_specs/base.py
index f239ef2019..8b46139e82 100644
--- a/superset/db_engine_specs/base.py
+++ b/superset/db_engine_specs/base.py
@@ -138,7 +138,9 @@ builtin_time_grains: dict[str | None, str] = {
}
-class TimestampExpression(ColumnClause): # pylint: disable=abstract-method, too-many-ancestors
+class TimestampExpression(
+ ColumnClause
+): # pylint: disable=abstract-method, too-many-ancestors
def __init__(self, expr: str, col: ColumnClause, **kwargs: Any) -> None:
"""Sqlalchemy class that can be used to render native column elements respecting
engine-specific quoting rules as part of a string-based expression.
@@ -397,9 +399,9 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
max_column_name_length: int | None = None
try_remove_schema_from_table_name = True # pylint: disable=invalid-name
run_multiple_statements_as_one = False
- custom_errors: dict[
- Pattern[str], tuple[str, SupersetErrorType, dict[str, Any]]
- ] = {}
+ custom_errors: dict[Pattern[str], tuple[str, SupersetErrorType, dict[str, Any]]] = (
+ {}
+ )
# List of JSON path to fields in `encrypted_extra` that should be masked when the
# database is edited. By default everything is masked.
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..6f62bd593f 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,
@@ -122,7 +123,9 @@ class ConfigurationMethod(StrEnum):
DYNAMIC_FORM = "dynamic_form"
-class Database(Model, AuditMixinNullable, ImportExportMixin): # pylint: disable=too-many-public-methods
+class Database(
+ Model, AuditMixinNullable, ImportExportMixin
+): # pylint: disable=too-many-public-methods
"""An ORM object that stores Database related information"""
__tablename__ = "dbs"
@@ -397,9 +400,7 @@ class Database(Model, AuditMixinNullable, ImportExportMixin): # pylint: disable
return (
username
if (username := get_username())
- else object_url.username
- if self.impersonate_user
- else None
+ else object_url.username if self.impersonate_user else None
)
@contextmanager
@@ -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.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/sqllab/api.py b/superset/sqllab/api.py
index 2403a36583..0184da5215 100644
--- a/superset/sqllab/api.py
+++ b/superset/sqllab/api.py
@@ -415,6 +415,9 @@ class SqlLabRestApi(BaseSupersetApi):
)
# return the execution result without special encoding
return json_success(command_result["payload"], response_status)
+ except Exception as ex:
+ print(ex)
+ raise
except SqlLabException as ex:
payload = {"errors": [ex.to_dict()]}
diff --git a/superset/sqllab/sql_json_executer.py b/superset/sqllab/sql_json_executer.py
index e0d723c7b8..d65dbc071d 100644
--- a/superset/sqllab/sql_json_executer.py
+++ b/superset/sqllab/sql_json_executer.py
@@ -111,7 +111,6 @@ class SynchronousSqlJsonExecutor(SqlJsonExecutorBase):
[SupersetError(**params) for params in data["errors"]] # type: ignore
)
# old string-only error message
- print(data)
raise SupersetGenericDBErrorException(data["error"]) # type: ignore
return SqlJsonExecutionStatus.HAS_RESULTS