Skip to content

Commit

Permalink
Add UI-only database configuration method for extended authorization …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
Will Barrett authored and dpgaspar committed Oct 28, 2019
1 parent c1810aa commit 148cec4
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 3 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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")
15 changes: 14 additions & 1 deletion superset/models/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -903,6 +904,8 @@ def get_sqla_engine(self, schema=None, nullpool=True, user_name=None, source=Non
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(
Expand Down Expand Up @@ -1145,11 +1148,21 @@ def get_extra(self):
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", {}))
Expand Down
1 change: 1 addition & 0 deletions superset/templates/superset/models/database/add.html
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@
{{ super() }}
{{ macros.testconn() }}
{{ macros.expand_extra_textarea() }}
{{ macros.expand_encrypted_extra_textarea() }}
{% endblock %}
1 change: 1 addition & 0 deletions superset/templates/superset/models/database/edit.html
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@
{{ super() }}
{{ macros.testconn() }}
{{ macros.expand_extra_textarea() }}
{{ macros.expand_encrypted_extra_textarea() }}
{% endblock %}
6 changes: 6 additions & 0 deletions superset/templates/superset/models/database/macros.html
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,9 @@
$('#extra').attr('rows', '5');
</script>
{% endmacro %}

{% macro expand_encrypted_extra_textarea() %}
<script>
$('#encrypted_extra').attr('rows', '5');
</script>
{% endmacro %}
19 changes: 18 additions & 1 deletion superset/views/database/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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: "
Expand Down Expand Up @@ -94,6 +94,7 @@ class DatabaseMixin:
"impersonate_user",
"allow_multi_schema_metadata_fetch",
"extra",
"encrypted_extra",
]
search_exclude_columns = (
"password",
Expand Down Expand Up @@ -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/>"
Expand Down Expand Up @@ -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"),
Expand All @@ -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
Expand Down Expand Up @@ -253,3 +263,10 @@ def check_extra(self, db):
"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)}")

0 comments on commit 148cec4

Please sign in to comment.