From 5af4e61affb554c0f6c29f63a5f4e4ac77644989 Mon Sep 17 00:00:00 2001
From: Beto Dealmeida <roberto@dealmeida.net>
Date: Mon, 3 Mar 2025 12:55:54 -0500
Subject: [PATCH] feat: improve GSheets OAuth2 (#32048)

---
 .../DatabaseConnectionForm/EncryptedField.tsx | 66 ++++---------------
 .../OAuth2ClientField.test.tsx                | 11 +++-
 .../OAuth2ClientField.tsx                     | 41 +++++++++---
 .../DatabaseConnectionForm/TableCatalog.tsx   |  9 ++-
 .../DatabaseConnectionForm/constants.ts       |  6 +-
 .../databases/DatabaseModal/styles.ts         | 27 +++++---
 .../src/features/databases/types.ts           |  2 +-
 superset/commands/database/validate.py        |  2 +-
 superset/db_engine_specs/gsheets.py           | 42 ++++++++----
 superset/models/core.py                       | 11 +++-
 superset/sqllab/sql_json_executer.py          |  1 -
 .../integration_tests/databases/api_tests.py  | 16 +++++
 .../commands/databases/validate_test.py       |  1 +
 tests/unit_tests/databases/api_test.py        | 15 +++++
 .../db_engine_specs/test_gsheets.py           | 33 ++++++++--
 .../migrations/shared/catalogs_test.py        | 29 +++++++-
 16 files changed, 211 insertions(+), 101 deletions(-)

diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx
index ad80699802e..6bddee3c922 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx
@@ -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={() => {
diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.test.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.test.tsx
index c68aa9d24fe..ef5ce14f7a6 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.test.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.test.tsx
@@ -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',
+          },
         },
       }),
     );
diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.tsx
index fac3c33318d..5cfcfd50813 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.tsx
@@ -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"
diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/TableCatalog.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/TableCatalog.tsx
index 4fe71d09062..91b6a63c0bc 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 a0bf7a5dd71..4512d1f707f 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/DatabaseModal/styles.ts b/superset-frontend/src/features/databases/DatabaseModal/styles.ts
index 537c6bf89c4..83d00e1d49c 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/styles.ts
+++ b/superset-frontend/src/features/databases/DatabaseModal/styles.ts
@@ -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`
diff --git a/superset-frontend/src/features/databases/types.ts b/superset-frontend/src/features/databases/types.ts
index 209100941b4..a41499b9804 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 e1584d59c03..eda5d75bed8 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/gsheets.py b/superset/db_engine_specs/gsheets.py
index 65afd9aee93..883a54abb25 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,
@@ -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
diff --git a/superset/models/core.py b/superset/models/core.py
index 96a1953faeb..dae9e8a0ece 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,
@@ -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:
         """
diff --git a/superset/sqllab/sql_json_executer.py b/superset/sqllab/sql_json_executer.py
index e0d723c7b8c..d65dbc071d0 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
diff --git a/tests/integration_tests/databases/api_tests.py b/tests/integration_tests/databases/api_tests.py
index 7db82ee95bf..f7c9d9da7ad 100644
--- a/tests/integration_tests/databases/api_tests.py
+++ b/tests/integration_tests/databases/api_tests.py
@@ -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",
diff --git a/tests/unit_tests/commands/databases/validate_test.py b/tests/unit_tests/commands/databases/validate_test.py
index c5992356c71..96f613315fe 100644
--- a/tests/unit_tests/commands/databases/validate_test.py
+++ b/tests/unit_tests/commands/databases/validate_test.py
@@ -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:
diff --git a/tests/unit_tests/databases/api_test.py b/tests/unit_tests/databases/api_test.py
index f6eae6544d6..32b53c02a3b 100644
--- a/tests/unit_tests/databases/api_test.py
+++ b/tests/unit_tests/databases/api_test.py
@@ -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",
diff --git a/tests/unit_tests/db_engine_specs/test_gsheets.py b/tests/unit_tests/db_engine_specs/test_gsheets.py
index df05c513120..744be28b503 100644
--- a/tests/unit_tests/db_engine_specs/test_gsheets.py
+++ b/tests/unit_tests/db_engine_specs/test_gsheets.py
@@ -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 == [
diff --git a/tests/unit_tests/migrations/shared/catalogs_test.py b/tests/unit_tests/migrations/shared/catalogs_test.py
index db06b75a8de..29a4a6fb282 100644
--- a/tests/unit_tests/migrations/shared/catalogs_test.py
+++ b/tests/unit_tests/migrations/shared/catalogs_test.py
@@ -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",