Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from
Oct 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just confirmed that this doesn't show in show_columns

]
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)}")