diff --git a/caravel/models.py b/caravel/models.py index 36d58255d1..3ee979b687 100644 --- a/caravel/models.py +++ b/caravel/models.py @@ -1196,19 +1196,19 @@ class DruidDatasource(Model, AuditMixinNullable, Queryable): m.metric_name: m.json_obj for m in self.metrics if m.metric_name in all_metrics - } + } rejected_metrics = [ m.metric_name for m in self.metrics if m.is_restricted and m.metric_name in aggregations.keys() and not sm.has_access('metric_access', m.perm) - ] + ] if rejected_metrics: raise MetricPermException( "Access to the metrics denied: " + ', '.join(rejected_metrics) - ) + ) granularity = granularity or "all" if granularity != "all": diff --git a/caravel/utils.py b/caravel/utils.py index 871972c6d0..97615aab9c 100644 --- a/caravel/utils.py +++ b/caravel/utils.py @@ -34,6 +34,15 @@ class MetricPermException(Exception): pass +def can_access(security_manager, permission_name, view_name): + """Protecting from has_access failing from missing perms/view""" + try: + return security_manager.has_access(permission_name, view_name) + except: + pass + return False + + def flasher(msg, severity=None): """Flask's flash if available, logging call if not""" try: @@ -232,18 +241,24 @@ def init(caravel): def init_metrics_perm(caravel, metrics=None): + """Create permissions for restricted metrics + + :param metrics: a list of metrics to be processed, if not specified, + all metrics are processed + :type metrics: models.SqlMetric or models.DruidMetric + """ db = caravel.db models = caravel.models sm = caravel.appbuilder.sm - if metrics is None: + if not metrics: metrics = [] for model in [models.SqlMetric, models.DruidMetric]: metrics += list(db.session.query(model).all()) - metric_perms = filter(None, [metric.perm for metric in metrics]) - for metric_perm in metric_perms: - merge_perm(sm, 'metric_access', metric_perm) + for metric in metrics: + if metric.is_restricted and metric.perm: + merge_perm(sm, 'metric_access', metric.perm) def datetime_f(dttm): diff --git a/caravel/views.py b/caravel/views.py index a83bcd1974..3004dd563e 100644 --- a/caravel/views.py +++ b/caravel/views.py @@ -36,6 +36,12 @@ from caravel import appbuilder, db, models, viz, utils, app, sm, ascii_art config = app.config log_this = models.Log.log_this +can_access = utils.can_access + + +class BaseCaravelView(BaseView): + def can_access(self, permission_name, view_name): + return utils.can_access(appbuilder.sm, permission_name, view_name) def get_error_msg(): @@ -244,8 +250,7 @@ appbuilder.add_view_no_menu(DruidColumnInlineView) class SqlMetricInlineView(CompactCRUDMixin, CaravelModelView): # noqa datamodel = SQLAInterface(models.SqlMetric) - list_columns = ['metric_name', 'verbose_name', 'metric_type', - 'is_restricted'] + list_columns = ['metric_name', 'verbose_name', 'metric_type'] edit_columns = [ 'metric_name', 'description', 'verbose_name', 'metric_type', 'expression', 'table', 'is_restricted'] @@ -269,16 +274,18 @@ class SqlMetricInlineView(CompactCRUDMixin, CaravelModelView): # noqa 'table': _("Table"), } - def post_add(self, new_item): - utils.init_metrics_perm(caravel, [new_item]) + def post_add(self, metric): + utils.init_metrics_perm(caravel, [metric]) + + def post_update(self, metric): + utils.init_metrics_perm(caravel, [metric]) appbuilder.add_view_no_menu(SqlMetricInlineView) class DruidMetricInlineView(CompactCRUDMixin, CaravelModelView): # noqa datamodel = SQLAInterface(models.DruidMetric) - list_columns = ['metric_name', 'verbose_name', 'metric_type', - 'is_restricted'] + list_columns = ['metric_name', 'verbose_name', 'metric_type'] edit_columns = [ 'metric_name', 'description', 'verbose_name', 'metric_type', 'json', 'datasource', 'is_restricted'] @@ -307,8 +314,11 @@ class DruidMetricInlineView(CompactCRUDMixin, CaravelModelView): # noqa 'datasource': _("Druid Datasource"), } - def post_add(self, new_item): - utils.init_metrics_perm(caravel, [new_item]) + def post_add(self, metric): + utils.init_metrics_perm(caravel, [metric]) + + def post_update(self, metric): + utils.init_metrics_perm(caravel, [metric]) appbuilder.add_view_no_menu(DruidMetricInlineView) @@ -685,7 +695,7 @@ def ping(): return "OK" -class R(BaseView): +class R(BaseCaravelView): """used for short urls""" @@ -718,16 +728,7 @@ class R(BaseView): appbuilder.add_view_no_menu(R) -def caravel_has_access(permission_name, view_name): - """Protecting from has_access failing from missing perms/view""" - try: - return appbuilder.sm.has_access(permission_name, view_name) - except: - pass - return False - - -class Caravel(BaseView): +class Caravel(BaseCaravelView): """The base views for Caravel!""" @@ -749,9 +750,9 @@ class Caravel(BaseView): datasource = datasource[0] if datasource else None slice_id = request.args.get("slice_id") slc = None - slice_add_perm = caravel_has_access('can_add', 'SliceModelView') - slice_edit_perm = caravel_has_access('can_edit', 'SliceModelView') - slice_download_perm = caravel_has_access('can_download', 'SliceModelView') + slice_add_perm = self.can_access('can_add', 'SliceModelView') + slice_edit_perm = self.can_access('can_edit', 'SliceModelView') + slice_download_perm = self.can_access('can_download', 'SliceModelView') if slice_id: slc = ( @@ -763,9 +764,9 @@ class Caravel(BaseView): flash(__("The datasource seems to have been deleted"), "alert") return redirect(error_redirect) - all_datasource_access = caravel_has_access( + all_datasource_access = self.can_access( 'all_datasource_access', 'all_datasource_access') - datasource_access = caravel_has_access( + datasource_access = self.can_access( 'datasource_access', datasource.perm) if not (all_datasource_access or datasource_access): flash(__("You don't seem to have access to this datasource"), "danger") @@ -1029,15 +1030,15 @@ class Caravel(BaseView): return self.render_template( "caravel/dashboard.html", dashboard=dash, templates=templates, - dash_save_perm=appbuilder.sm.has_access('can_save_dash', 'Caravel'), - dash_edit_perm=appbuilder.sm.has_access('can_edit', 'DashboardModelView')) + dash_save_perm=self.can_access('can_save_dash', 'Caravel'), + dash_edit_perm=self.can_access('can_edit', 'DashboardModelView')) @has_access @expose("/sql//") @log_this def sql(self, database_id): if ( - not self.appbuilder.sm.has_access( + not self.can_access( 'all_datasource_access', 'all_datasource_access')): flash( "This view requires the `all_datasource_access` " @@ -1102,7 +1103,7 @@ class Caravel(BaseView): mydb = session.query(models.Database).filter_by(id=database_id).first() if ( - not self.appbuilder.sm.has_access( + not self.can_access( 'all_datasource_access', 'all_datasource_access')): raise utils.CaravelSecurityException(_( "This view requires the `all_datasource_access` permission"))