feat: improve GSheets OAuth2 (#32048)

This commit is contained in:
Beto Dealmeida
2025-03-03 12:55:54 -05:00
committed by GitHub
parent 5766c36372
commit 5af4e61aff
16 changed files with 211 additions and 101 deletions

View File

@@ -19,11 +19,10 @@
import { useRef, useState } from 'react';
import { SupersetTheme, t } from '@superset-ui/core';
import { Button, AntdSelect } from 'src/components';
import InfoTooltip from 'src/components/InfoTooltip';
import FormLabel from 'src/components/Form/FormLabel';
import Icons from 'src/components/Icons';
import { DatabaseParameters, FieldPropTypes } from '../../types';
import { infoTooltip, labelMarginBottom, CredentialInfoForm } from '../styles';
import { infoTooltip, CredentialInfoForm } from '../styles';
enum CredentialInfoOptions {
JsonUpload,
@@ -38,8 +37,6 @@ export const encryptedCredentialsMap = {
bigquery: 'credentials_info',
};
const castStringToBoolean = (optionValue: string) => optionValue === 'true';
export const EncryptedField = ({
changeMethods,
isEditMode,
@@ -53,10 +50,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];
@@ -68,33 +62,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
@@ -116,7 +86,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}
@@ -130,9 +100,6 @@ export const EncryptedField = ({
'Paste content of service credentials JSON file here',
)}
/>
<span className="label-paste">
{t('Copy and paste the entire service account .json file here')}
</span>
</div>
) : (
showCredentialsInfo && (
@@ -140,27 +107,18 @@ export const EncryptedField = ({
className="input-container"
css={(theme: SupersetTheme) => infoTooltip(theme)}
>
<div css={{ display: 'flex', alignItems: 'center' }}>
<FormLabel required>{t('Upload Credentials')}</FormLabel>
<InfoTooltip
tooltip={t(
'Use the JSON file you automatically downloaded when creating your service account.',
)}
viewBox="0 0 24 24"
/>
</div>
{!fileToUpload && (
<Button
className="input-upload-btn"
onClick={() => selectedFileInputRef.current?.click()}
>
{t('Choose File')}
<Button onClick={() => selectedFileInputRef.current?.click()}>
<Icons.Link iconSize="m" />
{t('Upload credentials')}
</Button>
)}
{fileToUpload && (
<div className="input-upload-current">
{fileToUpload}
<div className="credentials-uploaded">
<Button block disabled>
<Icons.Link iconSize="m" />
{t('Credentials uploaded')}
</Button>
<Icons.DeleteFilled
iconSize="m"
onClick={() => {

View File

@@ -122,11 +122,18 @@ describe('OAuth2ClientField', () => {
const clientIdInput = getByTestId('client-id');
fireEvent.change(clientIdInput, { target: { value: 'new-id' } });
expect(mockChangeMethods.onEncryptedExtraInputChange).toHaveBeenCalledWith(
expect(mockChangeMethods.onParametersChange).toHaveBeenCalledWith(
expect.objectContaining({
target: {
name: 'oauth2_client_info',
value: expect.objectContaining({ id: 'new-id' }),
type: 'object',
value: {
authorization_request_uri: 'https://auth-uri',
id: 'new-id',
scope: 'test-scope',
secret: 'test-secret',
token_request_uri: 'https://token-uri',
},
},
}),
);

View File

@@ -18,11 +18,12 @@
*/
import { useState } from 'react';
import { css, SupersetTheme } from '@superset-ui/core';
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',
@@ -32,6 +33,16 @@ const LABELS = {
SCOPE: 'Scope',
};
const collapseStyle = (theme: SupersetTheme) => css`
.ant-collapse-header {
padding-bottom: ${theme.gridUnit * 1.5}px !important;
padding-top: ${theme.gridUnit * 1.5}px !important;
}
.anticon.ant-collapse-arrow {
top: 0 !important;
}
`;
interface OAuth2ClientInfo {
id: string;
secret: string;
@@ -40,16 +51,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,18 +80,23 @@ 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 (
<Collapse>
<Collapse.Panel header="OAuth2 client information" key="1">
<Collapse.Panel
header="OAuth2 client information"
key="1"
css={collapseStyle}
>
<FormItem label={LABELS.CLIENT_ID}>
<Input
data-test="client-id"

View File

@@ -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>
);
};

View File

@@ -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,

View File

@@ -433,7 +433,7 @@ export const CredentialInfoForm = styled.div`
}
.input-container {
margin: ${({ theme }) => theme.gridUnit * 7}px 0;
margin: ${({ theme }) => theme.gridUnit * 4}px 0;
display: flex;
flex-direction: column;
}
@@ -452,16 +452,27 @@ export const CredentialInfoForm = styled.div`
}
.input-container {
width: 100%;
button {
width: fit-content;
}
.credentials-uploaded {
display: flex;
align-items: center;
gap: ${({ theme }) => theme.gridUnit * 3}px;
width: fit-content;
}
.credentials-uploaded-btn, .credentials-uploaded-remove {
flex: 0 0 auto;
}
/* hide native file upload input element */
.input-upload {
display: none !important;
}
.input-upload-current {
display: flex;
justify-content: space-between;
}
.input-upload-btn {
width: ${({ theme }) => theme.gridUnit * 32}px
}
}`;
export const SelectDatabaseStyles = styled.div`

View File

@@ -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;

View File

@@ -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)

View File

@@ -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,
@@ -210,9 +236,9 @@ class GSheetsEngineSpec(ShillelaghEngineSpec):
# via parameters for validation
parameters = properties.get("parameters", {})
if parameters and parameters.get("catalog"):
table_catalog = parameters.get("catalog", {})
table_catalog = parameters.get("catalog") or {}
else:
table_catalog = properties.get("catalog", {})
table_catalog = properties.get("catalog") or {}
encrypted_credentials = parameters.get("service_account_info") or "{}"
@@ -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

View File

@@ -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,
@@ -1132,9 +1133,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 is not None or self.db_engine_spec.is_oauth2_enabled()
def get_oauth2_config(self) -> OAuth2ClientConfig | None:
"""

View File

@@ -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

View File

@@ -3447,6 +3447,22 @@ class TestDatabaseApi(SupersetTestCase):
"parameters": {
"properties": {
"catalog": {"type": "object"},
"oauth2_client_info": {
"default": {
"authorization_request_uri": "https://accounts.google.com/o/oauth2/v2/auth",
"scope": (
"https://www.googleapis.com/auth/"
"drive.readonly "
"https://www.googleapis.com/auth/spreadsheets "
"https://spreadsheets.google.com/feeds"
),
"token_request_uri": "https://oauth2.googleapis.com/token",
},
"description": "OAuth2 client information",
"nullable": True,
"type": "string",
"x-encrypted-extra": True,
},
"service_account_info": {
"description": "Contents of GSheets JSON credentials.",
"type": "string",

View File

@@ -71,6 +71,7 @@ def test_command_invalid(mocker: MockerFixture) -> None:
properties = {
"engine": "gsheets",
"driver": "gsheets",
"catalog": {"": "https://example.org/"},
}
command = ValidateDatabaseParametersCommand(properties)
with pytest.raises(InvalidParametersError) as excinfo:

View File

@@ -281,6 +281,21 @@ def test_database_connection(
"parameters_schema": {
"properties": {
"catalog": {"type": "object"},
"oauth2_client_info": {
"default": {
"authorization_request_uri": "https://accounts.google.com/o/oauth2/v2/auth",
"scope": (
"https://www.googleapis.com/auth/drive.readonly "
"https://www.googleapis.com/auth/spreadsheets "
"https://spreadsheets.google.com/feeds"
),
"token_request_uri": "https://oauth2.googleapis.com/token",
},
"description": "OAuth2 client information",
"nullable": True,
"type": "string",
"x-encrypted-extra": True,
},
"service_account_info": {
"description": "Contents of GSheets JSON credentials.",
"type": "string",

View File

@@ -42,16 +42,38 @@ class ProgrammingError(Exception):
"""
def test_validate_parameters_simple() -> None:
def test_validate_parameters_simple(mocker: MockerFixture) -> None:
from superset.db_engine_specs.gsheets import (
GSheetsEngineSpec,
GSheetsPropertiesType,
)
g = mocker.patch("superset.db_engine_specs.gsheets.g")
g.user.email = "admin@example.org"
properties: GSheetsPropertiesType = {
"parameters": {
"service_account_info": "",
"catalog": {},
"catalog": {"test": "https://docs.google.com/spreadsheets/d/1/edit"},
},
"catalog": {},
}
assert GSheetsEngineSpec.validate_parameters(properties)
def test_validate_parameters_no_catalog(mocker: MockerFixture) -> None:
from superset.db_engine_specs.gsheets import (
GSheetsEngineSpec,
GSheetsPropertiesType,
)
g = mocker.patch("superset.db_engine_specs.gsheets.g")
g.user.email = "admin@example.org"
properties: GSheetsPropertiesType = {
"parameters": {
"service_account_info": "",
"catalog": {"": "https://docs.google.com/spreadsheets/d/1/edit"},
},
"catalog": {},
}
@@ -66,18 +88,21 @@ def test_validate_parameters_simple() -> None:
]
def test_validate_parameters_simple_with_in_root_catalog() -> None:
def test_validate_parameters_simple_with_in_root_catalog(mocker: MockerFixture) -> None:
from superset.db_engine_specs.gsheets import (
GSheetsEngineSpec,
GSheetsPropertiesType,
)
g = mocker.patch("superset.db_engine_specs.gsheets.g")
g.user.email = "admin@example.org"
properties: GSheetsPropertiesType = {
"parameters": {
"service_account_info": "",
"catalog": {},
},
"catalog": {},
"catalog": {"": "https://docs.google.com/spreadsheets/d/1/edit"},
}
errors = GSheetsEngineSpec.validate_parameters(properties)
assert errors == [

View File

@@ -15,6 +15,9 @@
# specific language governing permissions and limitations
# under the License.
import json
import pytest
from pytest_mock import MockerFixture
from sqlalchemy.orm.session import Session
@@ -28,6 +31,29 @@ from superset.migrations.shared.security_converge import (
PermissionView,
ViewMenu,
)
from superset.superset_typing import OAuth2ClientConfig
@pytest.fixture
def oauth2_config() -> OAuth2ClientConfig:
"""
Config for GSheets OAuth2.
"""
return {
"id": "XXX.apps.googleusercontent.com",
"secret": "GOCSPX-YYY",
"scope": " ".join(
[
"https://www.googleapis.com/auth/drive.readonly "
"https://www.googleapis.com/auth/spreadsheets "
"https://spreadsheets.google.com/feeds"
]
),
"redirect_uri": "http://localhost:8088/api/v1/oauth2/",
"authorization_request_uri": "https://accounts.google.com/o/oauth2/v2/auth",
"token_request_uri": "https://oauth2.googleapis.com/token",
"request_content_type": "json",
}
def test_upgrade_catalog_perms(mocker: MockerFixture, session: Session) -> None:
@@ -335,6 +361,7 @@ def test_upgrade_catalog_perms_graceful(
def test_upgrade_catalog_perms_oauth_connection(
mocker: MockerFixture,
session: Session,
oauth2_config: OAuth2ClientConfig,
) -> None:
"""
Test the `upgrade_catalog_perms` function when the DB is set up using OAuth.
@@ -362,7 +389,7 @@ def test_upgrade_catalog_perms_oauth_connection(
database = Database(
database_name="my_db",
sqlalchemy_uri="bigquery://my-test-project",
encrypted_extra='{"oauth2_client_info": "fake_mock_oauth_conn"}',
encrypted_extra=json.dumps({"oauth2_client_info": oauth2_config}),
)
dataset = SqlaTable(
table_name="my_table",