Add UI-only database configuration method for extended authorization scenarios (#8448)

* Add encrypted_extra to dbs

* WIP - UI-based BigQuery connection configuration

* Fix 500 bubbling to the surface when adding a database connection

* Add check for valid json

* black formatting:

* isort

* Incorporate PR feedback. Thanks all!

* black

* Typo fix in CONTRIBUTING.md

CONTRIBUTING.md

@@ -109,7 +109,7 @@ For large features or major changes to codebase, please create **Superset Improv
 
 ### Fix Bugs
 
-Look through the GitHub issues. Issues tagged with `#bug` is
+Look through the GitHub issues. Issues tagged with `#bug` are
 open to whoever wants to implement it.
 
 ### Implement Features

superset/migrations/versions/cca2f5d568c8_add_encrypted_extra_to_dbs.py

@@ -0,0 +1,38 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""add encrypted_extra to dbs
+
+Revision ID: cca2f5d568c8
+Revises: b6fa807eac07
+Create Date: 2019-10-09 15:05:06.965042
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "cca2f5d568c8"
+down_revision = "b6fa807eac07"
+
+import sqlalchemy as sa
+from alembic import op
+
+
+def upgrade():
+    op.add_column("dbs", sa.Column("encrypted_extra", sa.Text(), nullable=True))
+
+
+def downgrade():
+    op.drop_column("dbs", "encrypted_extra")

superset/models/core.py

@@ -748,6 +748,7 @@ class Database(Model, AuditMixinNullable, ImportMixin):
     """
         ),
     )
+    encrypted_extra = Column(EncryptedType(Text, config["SECRET_KEY"]), nullable=True)
     perm = Column(String(1000))
     impersonate_user = Column(Boolean, default=False)
     export_fields = [
@@ -903,6 +904,8 @@ class Database(Model, AuditMixinNullable, ImportMixin):
             d["configuration"] = configuration
             params["connect_args"] = d
 
+        params.update(self.get_encrypted_extra())
+
         DB_CONNECTION_MUTATOR = config.get("DB_CONNECTION_MUTATOR")
         if DB_CONNECTION_MUTATOR:
             url, params = DB_CONNECTION_MUTATOR(
@@ -1145,11 +1148,21 @@ class Database(Model, AuditMixinNullable, ImportMixin):
         if self.extra:
             try:
                 extra = json.loads(self.extra)
-            except Exception as e:
+            except json.JSONDecodeError as e:
                 logging.error(e)
                 raise e
         return extra
 
+    def get_encrypted_extra(self):
+        encrypted_extra = {}
+        if self.encrypted_extra:
+            try:
+                encrypted_extra = json.loads(self.encrypted_extra)
+            except json.JSONDecodeError as e:
+                logging.error(e)
+                raise e
+        return encrypted_extra
+
     def get_table(self, table_name, schema=None):
         extra = self.get_extra()
         meta = MetaData(**extra.get("metadata_params", {}))

superset/templates/superset/models/database/add.html

@@ -23,4 +23,5 @@
   {{ super() }}
   {{ macros.testconn() }}
   {{ macros.expand_extra_textarea() }}
+  {{ macros.expand_encrypted_extra_textarea() }}
 {% endblock %}

superset/templates/superset/models/database/edit.html

@@ -23,4 +23,5 @@
   {{ super() }}
   {{ macros.testconn() }}
   {{ macros.expand_extra_textarea() }}
+  {{ macros.expand_encrypted_extra_textarea() }}
 {% endblock %}

superset/templates/superset/models/database/macros.html

@@ -81,3 +81,9 @@
     $('#extra').attr('rows', '5');
   </script>
 {% endmacro %}
+
+{% macro expand_encrypted_extra_textarea() %}
+  <script>
+    $('#encrypted_extra').attr('rows', '5');
+  </script>
+{% endmacro %}

superset/views/database/__init__.py

@@ -39,7 +39,7 @@ def sqlalchemy_uri_validator(
     """
     try:
         make_url(uri.strip())
-    except ArgumentError:
+    except (ArgumentError, AttributeError):
         raise exception(
             _(
                 "Invalid connnection string, a valid string follows: "
@@ -94,6 +94,7 @@ class DatabaseMixin:
         "impersonate_user",
         "allow_multi_schema_metadata_fetch",
         "extra",
+        "encrypted_extra",
     ]
     search_exclude_columns = (
         "password",
@@ -170,6 +171,13 @@ class DatabaseMixin:
             "This should be used with Presto DBs so that the syntax is correct",
             True,
         ),
+        "encrypted_extra": utils.markdown(
+            "JSON string containing additional connection configuration.<br/>"
+            "This is used to provide connection information for systems like "
+            "Hive, Presto, and BigQuery, which do not conform to the username:password "
+            "syntax normally used by SQLAlchemy.",
+            True,
+        ),
         "impersonate_user": _(
             "If Presto, all the queries in SQL Lab are going to be executed as the "
             "currently logged on user who must have permission to run them.<br/>"
@@ -203,6 +211,7 @@ class DatabaseMixin:
         "sqlalchemy_uri": _("SQLAlchemy URI"),
         "cache_timeout": _("Chart Cache Timeout"),
         "extra": _("Extra"),
+        "encrypted_extra": _("Secure Extra"),
         "allow_run_async": _("Asynchronous Query Execution"),
         "impersonate_user": _("Impersonate the logged on user"),
         "allow_csv_upload": _("Allow Csv Upload"),
@@ -213,6 +222,7 @@ class DatabaseMixin:
 
     def _pre_add_update(self, db):
         self.check_extra(db)
+        self.check_encrypted_extra(db)
         db.set_sqlalchemy_uri(db.sqlalchemy_uri)
         security_manager.add_permission_view_menu("database_access", db.perm)
         # adding a new database we always want to force refresh schema list
@@ -253,3 +263,10 @@ class DatabaseMixin:
                     "is not configured correctly. The key "
                     "{} is invalid.".format(key)
                 )
+
+    def check_encrypted_extra(self, db):
+        # this will check whether json.loads(secure_extra) can succeed
+        try:
+            extra = db.get_encrypted_extra()
+        except Exception as e:
+            raise Exception(f"Secure Extra field cannot be decoded as JSON. {str(e)}")