From 148cec4690d9c01499d924980518bd1e05d533ea Mon Sep 17 00:00:00 2001 From: Will Barrett Date: Mon, 28 Oct 2019 08:49:40 -0700 Subject: [PATCH] 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 | 2 +- ...cca2f5d568c8_add_encrypted_extra_to_dbs.py | 38 +++++++++++++++++++ superset/models/core.py | 15 +++++++- .../superset/models/database/add.html | 1 + .../superset/models/database/edit.html | 1 + .../superset/models/database/macros.html | 6 +++ superset/views/database/__init__.py | 19 +++++++++- 7 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 superset/migrations/versions/cca2f5d568c8_add_encrypted_extra_to_dbs.py diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4dbfcd54cc..049503bbbf 100644 --- a/CONTRIBUTING.md +++ b/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 diff --git a/superset/migrations/versions/cca2f5d568c8_add_encrypted_extra_to_dbs.py b/superset/migrations/versions/cca2f5d568c8_add_encrypted_extra_to_dbs.py new file mode 100644 index 0000000000..5f99fb910d --- /dev/null +++ b/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") diff --git a/superset/models/core.py b/superset/models/core.py index b919c1e7fe..c9b63da449 100755 --- a/superset/models/core.py +++ b/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", {})) diff --git a/superset/templates/superset/models/database/add.html b/superset/templates/superset/models/database/add.html index d1067b9349..98d511c9cd 100644 --- a/superset/templates/superset/models/database/add.html +++ b/superset/templates/superset/models/database/add.html @@ -23,4 +23,5 @@ {{ super() }} {{ macros.testconn() }} {{ macros.expand_extra_textarea() }} + {{ macros.expand_encrypted_extra_textarea() }} {% endblock %} diff --git a/superset/templates/superset/models/database/edit.html b/superset/templates/superset/models/database/edit.html index 339df38f29..42a340a817 100644 --- a/superset/templates/superset/models/database/edit.html +++ b/superset/templates/superset/models/database/edit.html @@ -23,4 +23,5 @@ {{ super() }} {{ macros.testconn() }} {{ macros.expand_extra_textarea() }} + {{ macros.expand_encrypted_extra_textarea() }} {% endblock %} diff --git a/superset/templates/superset/models/database/macros.html b/superset/templates/superset/models/database/macros.html index 2087a3702a..dbe36dcc92 100644 --- a/superset/templates/superset/models/database/macros.html +++ b/superset/templates/superset/models/database/macros.html @@ -81,3 +81,9 @@ $('#extra').attr('rows', '5'); {% endmacro %} + +{% macro expand_encrypted_extra_textarea() %} + +{% endmacro %} diff --git a/superset/views/database/__init__.py b/superset/views/database/__init__.py index 0fe5734038..b9b207a703 100644 --- a/superset/views/database/__init__.py +++ b/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.
" + "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.
" @@ -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)}")