mirror of https://github.com/apache/superset.git
chore: deprecate restricted metrics (#8197)
* chore: deprecate restricted metrics An early community contribution added the concept of restricted metrics. The idea was to allow for some metrics to be restricted, and if a metric was tagged as such, a user would need to be given access to that metric more explicitely, through a special perm we would maintain for that metric. Now since the new concept of "Adhoc Metrics", the popover that lets a user pick a column and an aggregate function or to write their own SQL expression inline, this restriction is completely bypassed. Adhoc metrics was developed without the restricted metrics in mind. Anyhow, in the near future, we'll be rethinking the ideas behind data-access permissions, and things like column-level or row-level security will be redesigned from scratch. By deprecating this feature, we're removing a confusing and mostly broken feature, and making it easy to move forward * Use context manager to drop columns * disable jest's maxWorkers
This commit is contained in:
parent
7546ea3191
commit
9d4b955cc7
|
@ -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
|
||||
|
|
|
@ -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.
|
||||
|
|
|
@ -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",
|
||||
|
|
|
@ -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)
|
||||
|
||||
|
|
|
@ -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 {}
|
||||
|
|
|
@ -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)
|
||||
|
||||
|
|
|
@ -241,7 +241,6 @@ class SqlMetric(Model, BaseMetric):
|
|||
"table_id",
|
||||
"expression",
|
||||
"description",
|
||||
"is_restricted",
|
||||
"d3format",
|
||||
"warning_text",
|
||||
)
|
||||
|
|
|
@ -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)
|
||||
|
||||
|
|
|
@ -36,10 +36,6 @@ class SupersetSecurityException(SupersetException):
|
|||
self.link = link
|
||||
|
||||
|
||||
class MetricPermException(SupersetException):
|
||||
pass
|
||||
|
||||
|
||||
class NoDataException(SupersetException):
|
||||
status = 400
|
||||
|
||||
|
|
|
@ -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))
|
|
@ -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.
|
||||
|
|
Loading…
Reference in New Issue