diff --git a/UPDATING.md b/UPDATING.md index 654eb914b3..946df61c3e 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -23,6 +23,8 @@ assists people when migrating to a new version. ## Next Version +* We're deprecating the concept of "restricted metric", this feature + was not fully working anyhow. * [8117](https://github.com/apache/incubator-superset/pull/8117): If you are using `ENABLE_PROXY_FIX = True`, review the newly-introducted variable, `PROXY_FIX_CONFIG`, which changes the proxy behavior in accordance with @@ -34,7 +36,7 @@ using `ENABLE_PROXY_FIX = True`, review the newly-introducted variable, backend serialization. To disable set `RESULTS_BACKEND_USE_MSGPACK = False` in your configuration. -* [7848](https://github.com/apache/incubator-superset/pull/7848): If you are +* [7848](https://github.com/apache/incubator-superset/pull/7848): If you are running redis with celery, celery bump to 4.3.0 requires redis-py upgrade to 3.2.0 or later. diff --git a/docs/security.rst b/docs/security.rst index 67c3596877..0e796e3c88 100644 --- a/docs/security.rst +++ b/docs/security.rst @@ -20,7 +20,7 @@ Security Security in Superset is handled by Flask AppBuilder (FAB). FAB is a "Simple and rapid application development framework, built on top of Flask.". FAB provides authentication, user management, permissions and roles. -Please read its `Security documentation +Please read its `Security documentation `_. Provided Roles @@ -153,26 +153,3 @@ a set of data sources that power dashboards only made available to executives. When looking at its dashboard list, this user will only see the list of dashboards it has access to, based on the roles and permissions that were attributed. - - -Restricting the access to some metrics -"""""""""""""""""""""""""""""""""""""" - -Sometimes some metrics are relatively sensitive (e.g. revenue). -We may want to restrict those metrics to only a few roles. -For example, assumed there is a metric ``[cluster1].[datasource1].[revenue]`` -and only Admin users are allowed to see it. Here’s how to restrict the access. - -1. Edit the datasource (``Menu -> Source -> Druid datasources -> edit the - record "datasource1"``) and go to the tab ``List Druid Metric``. Check - the checkbox ``Is Restricted`` in the row of the metric ``revenue``. - -2. Edit the role (``Menu -> Security -> List Roles -> edit the record - “Admin”``), in the permissions field, type-and-search the permission - ``metric access on [cluster1].[datasource1].[revenue] (id: 1)``, then - click the Save button on the bottom of the page. - -Any users without the permission will see the error message -*Access to the metrics denied: revenue (Status: 500)* in the slices. -It also happens when the user wants to access a post-aggregation metric that -is dependent on revenue. diff --git a/superset/assets/package.json b/superset/assets/package.json index 3c936f36b8..2407c2651f 100644 --- a/superset/assets/package.json +++ b/superset/assets/package.json @@ -10,7 +10,7 @@ "scripts": { "tdd": "jest --watch", "test": "jest", - "cover": "jest --maxWorkers=8 --coverage", + "cover": "jest --coverage", "dev": "webpack --mode=development --colors --progress --debug --watch", "dev-server": "webpack-dev-server --mode=development --progress", "prod": "node --max_old_space_size=4096 webpack --mode=production --colors --progress", diff --git a/superset/connectors/base/models.py b/superset/connectors/base/models.py index da7fec5d6a..d82f4d6aa0 100644 --- a/superset/connectors/base/models.py +++ b/superset/connectors/base/models.py @@ -417,7 +417,6 @@ class BaseMetric(AuditMixinNullable, ImportMixin): verbose_name = Column(String(1024)) metric_type = Column(String(32)) description = Column(Text) - is_restricted = Column(Boolean, default=False, nullable=True) d3format = Column(String(128)) warning_text = Column(Text) diff --git a/superset/connectors/druid/models.py b/superset/connectors/druid/models.py index 4eb38dc9ac..f204a90c9b 100644 --- a/superset/connectors/druid/models.py +++ b/superset/connectors/druid/models.py @@ -66,7 +66,7 @@ from sqlalchemy_utils import EncryptedType from superset import conf, db, security_manager from superset.connectors.base.models import BaseColumn, BaseDatasource, BaseMetric -from superset.exceptions import MetricPermException, SupersetException +from superset.exceptions import SupersetException from superset.models.helpers import AuditMixinNullable, ImportMixin, QueryResult from superset.utils import core as utils, import_datasource @@ -389,7 +389,6 @@ class DruidMetric(Model, BaseMetric): "datasource_id", "json", "description", - "is_restricted", "d3format", "warning_text", ) @@ -1044,19 +1043,6 @@ class DruidDatasource(Model, BaseDatasource): } return aggregations - def check_restricted_metrics(self, aggregations): - rejected_metrics = [ - m.metric_name - for m in self.metrics - if m.is_restricted - and m.metric_name in aggregations.keys() - and not security_manager.has_access("metric_access", m.perm) - ] - if rejected_metrics: - raise MetricPermException( - "Access to the metrics denied: " + ", ".join(rejected_metrics) - ) - def get_dimensions(self, groupby, columns_dict): dimensions = [] groupby = [gb for gb in groupby if gb in columns_dict] @@ -1164,8 +1150,6 @@ class DruidDatasource(Model, BaseDatasource): metrics, metrics_dict ) - self.check_restricted_metrics(aggregations) - # the dimensions list with dimensionSpecs expanded dimensions = self.get_dimensions(groupby, columns_dict) extras = extras or {} diff --git a/superset/connectors/druid/views.py b/superset/connectors/druid/views.py index 8de7e95199..775e73f216 100644 --- a/superset/connectors/druid/views.py +++ b/superset/connectors/druid/views.py @@ -151,7 +151,6 @@ class DruidMetricInlineView(CompactCRUDMixin, SupersetModelView): # noqa "json", "datasource", "d3format", - "is_restricted", "warning_text", ] add_columns = edit_columns @@ -163,13 +162,7 @@ class DruidMetricInlineView(CompactCRUDMixin, SupersetModelView): # noqa "[Druid Post Aggregation]" "(http://druid.io/docs/latest/querying/post-aggregations.html)", True, - ), - "is_restricted": _( - "Whether access to this metric is restricted " - "to certain roles. Only roles with the permission " - "'metric access on XXX (the name of this metric)' " - "are allowed to access this metric" - ), + ) } label_columns = { "metric_name": _("Metric"), @@ -179,7 +172,6 @@ class DruidMetricInlineView(CompactCRUDMixin, SupersetModelView): # noqa "json": _("JSON"), "datasource": _("Druid Datasource"), "warning_text": _("Warning Message"), - "is_restricted": _("Is Restricted"), } add_form_extra_fields = { @@ -193,18 +185,6 @@ class DruidMetricInlineView(CompactCRUDMixin, SupersetModelView): # noqa edit_form_extra_fields = add_form_extra_fields - def post_add(self, metric): - if metric.is_restricted: - security_manager.add_permission_view_menu( - "metric_access", metric.get_perm() - ) - - def post_update(self, metric): - if metric.is_restricted: - security_manager.add_permission_view_menu( - "metric_access", metric.get_perm() - ) - appbuilder.add_view_no_menu(DruidMetricInlineView) diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index fbb909b5a5..41a7957947 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -241,7 +241,6 @@ class SqlMetric(Model, BaseMetric): "table_id", "expression", "description", - "is_restricted", "d3format", "warning_text", ) diff --git a/superset/connectors/sqla/views.py b/superset/connectors/sqla/views.py index 0600dfe0c5..3b242ec8ed 100644 --- a/superset/connectors/sqla/views.py +++ b/superset/connectors/sqla/views.py @@ -154,7 +154,6 @@ class SqlMetricInlineView(CompactCRUDMixin, SupersetModelView): # noqa "expression", "table", "d3format", - "is_restricted", "warning_text", ] description_columns = { @@ -163,12 +162,6 @@ class SqlMetricInlineView(CompactCRUDMixin, SupersetModelView): # noqa "underlying backend. Example: `count(DISTINCT userid)`", True, ), - "is_restricted": _( - "Whether access to this metric is restricted " - "to certain roles. Only roles with the permission " - "'metric access on XXX (the name of this metric)' " - "are allowed to access this metric" - ), "d3format": utils.markdown( "d3 formatting string as defined [here]" "(https://github.com/d3/d3-format/blob/master/README.md#format). " @@ -188,7 +181,6 @@ class SqlMetricInlineView(CompactCRUDMixin, SupersetModelView): # noqa "expression": _("SQL Expression"), "table": _("Table"), "d3format": _("D3 Format"), - "is_restricted": _("Is Restricted"), "warning_text": _("Warning Message"), } @@ -203,18 +195,6 @@ class SqlMetricInlineView(CompactCRUDMixin, SupersetModelView): # noqa edit_form_extra_fields = add_form_extra_fields - def post_add(self, metric): - if metric.is_restricted: - security_manager.add_permission_view_menu( - "metric_access", metric.get_perm() - ) - - def post_update(self, metric): - if metric.is_restricted: - security_manager.add_permission_view_menu( - "metric_access", metric.get_perm() - ) - appbuilder.add_view_no_menu(SqlMetricInlineView) diff --git a/superset/exceptions.py b/superset/exceptions.py index ade6ef81c6..53ce9bb94e 100644 --- a/superset/exceptions.py +++ b/superset/exceptions.py @@ -36,10 +36,6 @@ class SupersetSecurityException(SupersetException): self.link = link -class MetricPermException(SupersetException): - pass - - class NoDataException(SupersetException): status = 400 diff --git a/superset/migrations/versions/11c737c17cc6_deprecate_restricted_metrics.py b/superset/migrations/versions/11c737c17cc6_deprecate_restricted_metrics.py new file mode 100644 index 0000000000..9ea8218114 --- /dev/null +++ b/superset/migrations/versions/11c737c17cc6_deprecate_restricted_metrics.py @@ -0,0 +1,44 @@ +# 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. +"""deprecate_restricted_metrics + +Revision ID: 11c737c17cc6 +Revises: def97f26fdfb +Create Date: 2019-09-08 21:50:58.200229 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = "11c737c17cc6" +down_revision = "def97f26fdfb" + + +def upgrade(): + with op.batch_alter_table("metrics") as batch_op: + batch_op.drop_column("is_restricted") + with op.batch_alter_table("sql_metrics") as batch_op: + batch_op.drop_column("is_restricted") + + +def downgrade(): + op.add_column( + "sql_metrics", sa.Column("is_restricted", sa.BOOLEAN(), nullable=True) + ) + op.add_column("metrics", sa.Column("is_restricted", sa.BOOLEAN(), nullable=True)) diff --git a/superset/security.py b/superset/security.py index 918d7d1907..8c95f81471 100644 --- a/superset/security.py +++ b/superset/security.py @@ -535,10 +535,6 @@ class SupersetSecurityManager(SecurityManager): for datasource_class in ConnectorRegistry.sources.values(): metrics += list(db.session.query(datasource_class.metric_class).all()) - for metric in metrics: - if metric.is_restricted: - merge_pv("metric_access", metric.perm) - def clean_perms(self) -> None: """ Clean up the FAB faulty permissions.