feat: Add Private Google Sheets to dynamic form (#16628)

* first pass private gsheets

* made encrypted extra into string, refactored onParametersChanged

* private sheets working, credential_info errors

* all but test connection working

* first pass private gsheets

* made encrypted extra into string, refactored onParametersChanged

* private sheets working, credential_info errors

* all but test connection working

* Regenerate package-lock.json

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx

@@ -32,14 +32,25 @@ import {
   infoTooltip,
   StyledFooterButton,
   StyledCatalogTable,
+  labelMarginBotton,
 } from './styles';
 import { CatalogObject, DatabaseForm, DatabaseObject } from '../types';
 
+// These are the columns that are going to be added to encrypted extra, they differ in name based
+// on the engine, however we want to use the same component for each of them. Make sure to add the
+// the engine specific name here.
+export const encryptedCredentialsMap = {
+  gsheets: 'service_account_info',
+  bigquery: 'credentials_info',
+};
+
 enum CredentialInfoOptions {
   jsonUpload,
   copyPaste,
 }
 
+const castStringToBoolean = (optionValue: string) => optionValue === 'true';
+
 export const FormFieldOrder = [
   'host',
   'port',
@@ -48,6 +59,7 @@ export const FormFieldOrder = [
   'password',
   'database_name',
   'credentials_info',
+  'service_account_info',
   'catalog',
   'query',
   'encryption',
@@ -56,7 +68,7 @@ export const FormFieldOrder = [
 interface FieldPropTypes {
   required: boolean;
   hasTooltip?: boolean;
-  tooltipText?: (valuse: any) => string;
+  tooltipText?: (value: any) => string;
   onParametersChange: (value: any) => string;
   onParametersUploadFileChange: (value: any) => string;
   changeMethods: { onParametersChange: (value: any) => string } & {
@@ -88,12 +100,46 @@ const CredentialsInfo = ({
   const [fileToUpload, setFileToUpload] = useState<string | null | undefined>(
     null,
   );
+  const [isPublic, setIsPublic] = useState<boolean>(true);
+  const showCredentialsInfo =
+    db?.engine === 'gsheets' ? !isEditMode && !isPublic : !isEditMode;
+  // a database that has an optional encrypted field has an encrypted_extra that is an empty object, this checks for that.
+  const isEncrypted = isEditMode && db?.encrypted_extra !== '{}';
+  const encryptedField = db?.engine && encryptedCredentialsMap[db.engine];
+  const encryptedValue =
+    typeof db?.parameters?.[encryptedField] === 'object'
+      ? JSON.stringify(db?.parameters?.[encryptedField])
+      : db?.parameters?.[encryptedField];
   return (
     <CredentialInfoForm>
-      {!isEditMode && (
+      {db?.engine === 'gsheets' && (
+        <div className="catalog-type-select">
+          <FormLabel
+            css={(theme: SupersetTheme) => labelMarginBotton(theme)}
+            required
+          >
+            {t('Type of Google Sheets allowed')}
+          </FormLabel>
+          <Select
+            style={{ width: '100%' }}
+            defaultValue={isEncrypted ? 'false' : 'true'}
+            onChange={(value: string) =>
+              setIsPublic(castStringToBoolean(value))
+            }
+          >
+            <Select.Option value="true" key={1}>
+              {t('Publicly shared sheets only')}
+            </Select.Option>
+            <Select.Option value="false" key={2}>
+              {t('Public and privately shared sheets')}
+            </Select.Option>
+          </Select>
+        </div>
+      )}
+      {showCredentialsInfo && (
         <>
           <FormLabel required>
-            {t('How do you want to enter service account credentials?')}
+            {t('How∂ do you want to enter service account credentials?')}
           </FormLabel>
           <Select
             defaultValue={uploadOption}
@@ -117,8 +163,8 @@ const CredentialsInfo = ({
           <FormLabel required>{t('Service Account')}</FormLabel>
           <textarea
             className="input-form"
-            name="credentials_info"
+            name={encryptedField}
-            value={db?.parameters?.credentials_info}
+            value={encryptedValue}
             onChange={changeMethods.onParametersChange}
             placeholder="Paste content of service credentials JSON file here"
           />
@@ -127,6 +173,7 @@ const CredentialsInfo = ({
           </span>
         </div>
       ) : (
+        showCredentialsInfo && (
           <div
             className="input-container"
             css={(theme: SupersetTheme) => infoTooltip(theme)}
@@ -135,7 +182,7 @@ const CredentialsInfo = ({
               <FormLabel required>{t('Upload Credentials')}</FormLabel>
               <InfoTooltip
                 tooltip={t(
-                'Use the JSON file you automatically downloaded when creating your service account in Google BigQuery.',
+                  'Use the JSON file you automatically downloaded when creating your service account.',
                 )}
                 viewBox="0 0 24 24"
               />
@@ -144,7 +191,9 @@ const CredentialsInfo = ({
             {!fileToUpload && (
               <Button
                 className="input-upload-btn"
-              onClick={() => document?.getElementById('selectedFile')?.click()}
+                onClick={() =>
+                  document?.getElementById('selectedFile')?.click()
+                }
               >
                 {t('Choose File')}
               </Button>
@@ -157,7 +206,7 @@ const CredentialsInfo = ({
                     setFileToUpload(null);
                     changeMethods.onParametersChange({
                       target: {
-                      name: 'credentials_info',
+                        name: encryptedField,
                         value: '',
                       },
                     });
@@ -179,7 +228,7 @@ const CredentialsInfo = ({
                 changeMethods.onParametersChange({
                   target: {
                     type: null,
-                  name: 'credentials_info',
+                    name: encryptedField,
                     value: await file?.text(),
                     checked: false,
                   },
@@ -190,6 +239,7 @@ const CredentialsInfo = ({
               }}
             />
           </div>
+        )
       )}
     </CredentialInfoForm>
   );
@@ -204,16 +254,9 @@ const TableCatalog = ({
 }: FieldPropTypes) => {
   const tableCatalog = db?.catalog || [];
   const catalogError = validationErrors || {};
+
   return (
     <StyledCatalogTable>
-      <div className="catalog-type-select">
-        <FormLabel required>{t('Type of Google Sheets Allowed')}</FormLabel>
-        <Select style={{ width: '100%' }} defaultValue="true" disabled>
-          <Select.Option value="true" key={1}>
-            {t('Publicly shared sheets only')}
-          </Select.Option>
-        </Select>
-      </div>
       <h4 className="gsheet-title">
         {t('Connect Google Sheets as tables to this database')}
       </h4>
@@ -472,6 +515,7 @@ const FORM_FIELD_MAP = {
   query: queryField,
   encryption: forceSSLField,
   credentials_info: CredentialsInfo,
+  service_account_info: CredentialsInfo,
   catalog: TableCatalog,
 };
 

superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx

@@ -180,6 +180,7 @@ type DBReducerActionType =
         database_name?: string;
         engine?: string;
         configuration_method: CONFIGURATION_METHOD;
+        paramProperties?: Record<string, any>;
       };
     }
   | {
@@ -358,47 +359,27 @@ function dbReducer(
         .join('&');
 
       if (
-        action.payload.backend === 'bigquery' &&
+        action.payload.encrypted_extra &&
         action.payload.configuration_method ===
           CONFIGURATION_METHOD.DYNAMIC_FORM
       ) {
+        const engineParamsCatalog = Object.keys(
+          extra_json?.engine_params?.catalog || {},
+        ).map(e => ({
+          name: e,
+          value: extra_json?.engine_params?.catalog[e],
+        }));
         return {
           ...action.payload,
-          engine: action.payload.backend,
+          engine: action.payload.backend || trimmedState.engine,
           configuration_method: action.payload.configuration_method,
           extra_json: deserializeExtraJSON,
-          parameters: {
+          catalog: engineParamsCatalog,
-            credentials_info: JSON.stringify(
+          parameters: action.payload.parameters,
-              action.payload?.parameters?.credentials_info || '',
-            ),
-            query,
-          },
           query_input,
         };
       }
 
-      if (
-        action.payload.backend === 'gsheets' &&
-        action.payload.configuration_method ===
-          CONFIGURATION_METHOD.DYNAMIC_FORM &&
-        extra_json?.engine_params?.catalog !== undefined
-      ) {
-        // pull catalog from engine params
-        const engineParamsCatalog = extra_json?.engine_params?.catalog;
-
-        return {
-          ...action.payload,
-          engine: action.payload.backend,
-          configuration_method: action.payload.configuration_method,
-          extra_json: deserializeExtraJSON,
-          catalog: Object.keys(engineParamsCatalog).map(e => ({
-            name: e,
-            value: engineParamsCatalog[e],
-          })),
-          query_input,
-        } as DatabaseObject;
-      }
-
       return {
         ...action.payload,
         encrypted_extra: action.payload.encrypted_extra || '',
@@ -513,7 +494,6 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
       encrypted_extra: db?.encrypted_extra || '',
       server_cert: db?.server_cert || undefined,
     };
-
     testDatabaseConnection(connection, addDangerToast, addSuccessToast);
   };
 
@@ -525,11 +505,9 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
     setEditNewDb(false);
     onHide();
   };
-
   const onSave = async () => {
     // eslint-disable-next-line @typescript-eslint/no-unused-vars
     const { id, ...update } = db || {};
-
     // Clone DB object
     const dbToUpdate = JSON.parse(JSON.stringify(update));
 
@@ -539,32 +517,41 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
       if (validationErrors && !isEmpty(validationErrors)) {
         return;
       }
+      const parameters_schema = isEditMode
+        ? dbToUpdate.parameters_schema.properties
+        : dbModel?.parameters.properties;
+      const additionalEncryptedExtra = JSON.parse(
+        dbToUpdate.encrypted_extra || '{}',
+      );
+      const paramConfigArray = Object.keys(parameters_schema || {});
 
-      const engine = dbToUpdate.backend || dbToUpdate.engine;
+      paramConfigArray.forEach(paramConfig => {
-      if (engine === 'bigquery' && dbToUpdate.parameters?.credentials_info) {
+        /*
-        // wrap encrypted_extra in credentials_info only for BigQuery
+         * Parameters that are annotated with the `x-encrypted-extra` properties should be moved to
+         * `encrypted_extra`, so that they are stored encrypted in the backend when the database is
+         * created or edited.
+         */
         if (
-          dbToUpdate.parameters?.credentials_info &&
+          parameters_schema[paramConfig]['x-encrypted-extra'] &&
-          typeof dbToUpdate.parameters?.credentials_info === 'object' &&
+          dbToUpdate.parameters?.[paramConfig]
-          dbToUpdate.parameters?.credentials_info.constructor === Object
         ) {
-          // Don't cast if object
+          if (typeof dbToUpdate.parameters?.[paramConfig] === 'object') {
-          dbToUpdate.encrypted_extra = JSON.stringify({
+            // add new encrypted extra to encrypted_extra object
-            credentials_info: dbToUpdate.parameters?.credentials_info,
+            additionalEncryptedExtra[paramConfig] =
-          });
+              dbToUpdate.parameters?.[paramConfig];
-
+            // The backend expects `encrypted_extra` as a string for historical reasons.
-          // Convert credentials info string before updating
+            dbToUpdate.parameters[paramConfig] = JSON.stringify(
-          dbToUpdate.parameters.credentials_info = JSON.stringify(
+              dbToUpdate.parameters[paramConfig],
-            dbToUpdate.parameters.credentials_info,
             );
           } else {
-          dbToUpdate.encrypted_extra = JSON.stringify({
+            additionalEncryptedExtra[paramConfig] = JSON.parse(
-            credentials_info: JSON.parse(
+              dbToUpdate.parameters?.[paramConfig] || '{}',
-              dbToUpdate.parameters?.credentials_info,
+            );
-            ),
+          }
+        }
       });
-        }
+      // cast the new encrypted extra object into a string
-      }
+      dbToUpdate.encrypted_extra = JSON.stringify(additionalEncryptedExtra);
     }
 
     if (dbToUpdate?.parameters?.catalog) {
@@ -660,10 +647,11 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
         type: ActionType.dbSelected,
         payload: {
           database_name,
+          engine,
           configuration_method: isDynamic
             ? CONFIGURATION_METHOD.DYNAMIC_FORM
             : CONFIGURATION_METHOD.SQLALCHEMY_URI,
-          engine,
+          paramProperties: parameters?.properties,
         },
       });
     }

superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts

@@ -31,6 +31,10 @@ export const no_margin_bottom = css`
   margin-bottom: 0;
 `;
 
+export const labelMarginBotton = (theme: SupersetTheme) => css`
+  margin-bottom: ${theme.gridUnit * 2}px;
+`;
+
 export const marginBottom = (theme: SupersetTheme) => css`
   margin-bottom: ${theme.gridUnit * 4}px;
 `;
@@ -442,6 +446,10 @@ export const EditHeaderSubtitle = styled.div`
 `;
 
 export const CredentialInfoForm = styled.div`
+  .catalog-type-select {
+    margin: 0 0 20px;
+  }
+
   .label-select {
     text-transform: uppercase;
     color: ${({ theme }) => theme.colors.grayscale.dark1};
@@ -542,13 +550,13 @@ export const StyledCatalogTable = styled.div`
   margin-bottom: 16px;
 
   .catalog-type-select {
-    margin: 0 0 40px;
+    margin: 0 0 20px;
   }
 
   .gsheet-title {
     font-size: ${({ theme }) => theme.typography.sizes.l * 1.1}px;
     font-weight: bold;
-    margin: ${({ theme }) => theme.gridUnit * 6}px 0 16px;
+    margin: ${({ theme }) => theme.gridUnit * 10}px 0 16px;
   }
 
   .catalog-label {

superset-frontend/src/views/CRUD/data/database/types.ts

@@ -45,11 +45,14 @@ export type DatabaseObject = {
     password?: string;
     encryption?: boolean;
     credentials_info?: string;
+    service_account_info?: string;
     query?: Record<string, string>;
     catalog?: Record<string, string>;
+    properties?: Record<string, any>;
   };
   configuration_method: CONFIGURATION_METHOD;
   engine?: string;
+  paramProperties?: Record<string, any>;
 
   // Performance
   cache_timeout?: string;
@@ -68,6 +71,7 @@ export type DatabaseObject = {
   server_cert?: string;
   allow_csv_upload?: boolean;
   impersonate_user?: boolean;
+  parameters_schema?: Record<string, any>;
 
   // Extra
   extra_json?: {
@@ -131,6 +135,11 @@ export type DatabaseForm = {
         nullable: boolean;
         type: string;
       };
+      service_account_info: {
+        description: string;
+        nullable: boolean;
+        type: string;
+      };
     };
     required: string[];
     type: string;

superset/databases/api.py

@@ -116,6 +116,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
         "encrypted_extra",
         "extra",
         "parameters",
+        "parameters_schema",
         "server_cert",
         "sqlalchemy_uri",
     ]

superset/databases/schemas.py

@@ -602,7 +602,17 @@ class ImportV1DatabaseSchema(Schema):
             raise ValidationError("Must provide a password for the database")
 
 
-class EncryptedField(fields.String):
+class EncryptedField:  # pylint: disable=too-few-public-methods
+    """
+    A database field that should be stored in encrypted_extra.
+    """
+
+
+class EncryptedString(EncryptedField, fields.String):
+    pass
+
+
+class EncryptedDict(EncryptedField, fields.Dict):
     pass
 
 

superset/db_engine_specs/bigquery.py

@@ -14,6 +14,7 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
+import json
 import re
 import urllib
 from datetime import datetime
@@ -30,7 +31,7 @@ from sqlalchemy.engine.url import make_url
 from sqlalchemy.sql.expression import ColumnClause
 from typing_extensions import TypedDict
 
-from superset.databases.schemas import encrypted_field_properties, EncryptedField
+from superset.databases.schemas import encrypted_field_properties, EncryptedString
 from superset.db_engine_specs.base import BaseEngineSpec
 from superset.db_engine_specs.exceptions import SupersetDBAPIDisconnectionError
 from superset.errors import SupersetError, SupersetErrorType
@@ -69,7 +70,7 @@ ma_plugin = MarshmallowPlugin()
 
 
 class BigQueryParametersSchema(Schema):
-    credentials_info = EncryptedField(
+    credentials_info = EncryptedString(
         required=False, description="Contents of BigQuery JSON credentials.",
     )
     query = fields.Dict(required=False)
@@ -367,11 +368,14 @@ class BigQueryEngineSpec(BaseEngineSpec):
         query = parameters.get("query", {})
         query_params = urllib.parse.urlencode(query)
 
+        if encrypted_extra:
+            credentials_info = encrypted_extra.get("credentials_info")
+            if isinstance(credentials_info, str):
+                credentials_info = json.loads(credentials_info)
+            project_id = credentials_info.get("project_id")
         if not encrypted_extra:
             raise ValidationError("Missing service credentials")
 
-        project_id = encrypted_extra.get("credentials_info", {}).get("project_id")
-
         if project_id:
             return f"{cls.default_driver}://{project_id}/?{query_params}"
 

superset/db_engine_specs/gsheets.py

@@ -30,7 +30,7 @@ from sqlalchemy.engine.url import URL
 from typing_extensions import TypedDict
 
 from superset import security_manager
-from superset.databases.schemas import encrypted_field_properties
+from superset.databases.schemas import encrypted_field_properties, EncryptedString
 from superset.db_engine_specs.sqlite import SqliteEngineSpec
 from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
 
@@ -45,10 +45,15 @@ ma_plugin = MarshmallowPlugin()
 
 class GSheetsParametersSchema(Schema):
     catalog = fields.Dict()
+    service_account_info = EncryptedString(
+        required=False,
+        description="Contents of GSheets JSON credentials.",
+        field_name="service_account_info",
+    )
 
 
 class GSheetsParametersType(TypedDict):
-    credentials_info: Dict[str, Any]
+    service_account_info: str
     catalog: Dict[str, str]
 
 
@@ -113,7 +118,9 @@ class GSheetsEngineSpec(SqliteEngineSpec):
 
     @classmethod
     def get_parameters_from_uri(
-        cls, encrypted_extra: Optional[Dict[str, str]] = None,
+        cls,
+        uri: str,  # pylint: disable=unused-argument
+        encrypted_extra: Optional[Dict[str, str]] = None,
     ) -> Any:
         # Building parameters from encrypted_extra and uri
         if encrypted_extra:
@@ -146,7 +153,10 @@ class GSheetsEngineSpec(SqliteEngineSpec):
         cls, parameters: GSheetsParametersType,
     ) -> List[SupersetError]:
         errors: List[SupersetError] = []
-        credentials_info = parameters.get("credentials_info")
+        encrypted_credentials = json.loads(
+            parameters.get("service_account_info") or "{}"
+        )
+
         table_catalog = parameters.get("catalog", {})
 
         if not table_catalog:
@@ -160,7 +170,7 @@ class GSheetsEngineSpec(SqliteEngineSpec):
         subject = g.user.email if g.user else None
 
         engine = create_engine(
-            "gsheets://", service_account_info=credentials_info, subject=subject,
+            "gsheets://", service_account_info=encrypted_credentials, subject=subject,
         )
         conn = engine.connect()
         idx = 0

superset/models/core.py

@@ -223,6 +223,7 @@ class Database(
             "allows_virtual_table_explore": self.allows_virtual_table_explore,
             "explore_database_id": self.explore_database_id,
             "parameters": self.parameters,
+            "parameters_schema": self.parameters_schema,
         }
 
     @property
@@ -249,6 +250,14 @@ class Database(
 
         return parameters
 
+    @property
+    def parameters_schema(self) -> Dict[str, Any]:
+        try:
+            parameters_schema = self.db_engine_spec.parameters_json_schema()  # type: ignore # pylint: disable=line-too-long
+        except Exception:  # pylint: disable=broad-except
+            parameters_schema = {}
+        return parameters_schema
+
     @property
     def metadata_cache_timeout(self) -> Dict[str, Any]:
         return self.get_extra().get("metadata_cache_timeout", {})

tests/integration_tests/databases/api_tests.py

@@ -1576,7 +1576,14 @@ class TestDatabaseApi(SupersetTestCase):
                     "engine": "gsheets",
                     "name": "Google Sheets",
                     "parameters": {
-                        "properties": {"catalog": {"type": "object"},},
+                        "properties": {
+                            "catalog": {"type": "object"},
+                            "service_account_info": {
+                                "description": "Contents of GSheets JSON credentials.",
+                                "type": "string",
+                                "x-encrypted-extra": True,
+                            },
+                        },
                         "type": "object",
                     },
                     "preferred": False,

tests/unit_tests/db_engine_specs/test_gsheets.py

@@ -35,7 +35,7 @@ def test_validate_parameters_simple(
     )
 
     parameters: GSheetsParametersType = {
-        "credentials_info": {},
+        "service_account_info": "",
         "catalog": {},
     }
     errors = GSheetsEngineSpec.validate_parameters(parameters)
@@ -63,7 +63,7 @@ def test_validate_parameters_catalog(
     ]
 
     parameters: GSheetsParametersType = {
-        "credentials_info": {},
+        "service_account_info": "",
         "catalog": {
             "private_sheet": "https://docs.google.com/spreadsheets/d/1/edit",
             "public_sheet": "https://docs.google.com/spreadsheets/d/1/edit#gid=1",
@@ -137,7 +137,7 @@ def test_validate_parameters_catalog_and_credentials(
     ]
 
     parameters: GSheetsParametersType = {
-        "credentials_info": {},
+        "service_account_info": "",
         "catalog": {
             "private_sheet": "https://docs.google.com/spreadsheets/d/1/edit",
             "public_sheet": "https://docs.google.com/spreadsheets/d/1/edit#gid=1",