Skip to content

Commit

Permalink
Add access control over metrics (#584)
Browse files Browse the repository at this point in the history
* Add the new field "is_restricted" to SqlMetric and DruidMetric

* Add the access control on metrics

* Add the more descriptions on is_restricted

* Update docs/security.rst

* Update docs/security.rst
  • Loading branch information
x4base authored and mistercrunch committed Jun 10, 2016
1 parent 55baab4 commit 4c6026f
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 12 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
"""Add new field 'is_restricted' to SqlMetric and DruidMetric
Revision ID: d8bc074f7aad
Revises: 1226819ee0e3
Create Date: 2016-06-07 12:33:25.756640
"""

# revision identifiers, used by Alembic.
revision = 'd8bc074f7aad'
down_revision = '1226819ee0e3'

from alembic import op
import sqlalchemy as sa
from caravel import db
from caravel import models


def upgrade():
with op.batch_alter_table('metrics', schema=None) as batch_op:
batch_op.add_column(
sa.Column('is_restricted', sa.Boolean(), nullable=True))

with op.batch_alter_table('sql_metrics', schema=None) as batch_op:
batch_op.add_column(
sa.Column('is_restricted', sa.Boolean(), nullable=True))

bind = op.get_bind()
session = db.Session(bind=bind)

session.query(models.DruidMetric).update({
'is_restricted': False
})
session.query(models.SqlMetric).update({
'is_restricted': False
})

session.commit()
session.close()


def downgrade():
with op.batch_alter_table('sql_metrics', schema=None) as batch_op:
batch_op.drop_column('is_restricted')

with op.batch_alter_table('metrics', schema=None) as batch_op:
batch_op.drop_column('is_restricted')
40 changes: 37 additions & 3 deletions caravel/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,10 @@
from sqlalchemy.sql import table, literal_column, text, column
from sqlalchemy_utils import EncryptedType

from caravel import app, db, get_session, utils
import caravel
from caravel import app, db, get_session, utils, sm
from caravel.viz import viz_types
from caravel.utils import flasher
from caravel.utils import flasher, MetricPermException

config = app.config

Expand Down Expand Up @@ -858,12 +859,20 @@ class SqlMetric(Model, AuditMixinNullable):
'SqlaTable', backref='metrics', foreign_keys=[table_id])
expression = Column(Text)
description = Column(Text)
is_restricted = Column(Boolean, default=False, nullable=True)

@property
def sqla_col(self):
name = self.metric_name
return literal_column(self.expression).label(name)

@property
def perm(self):
return (
"{parent_name}.[{obj.metric_name}](id:{obj.id})"
).format(obj=self,
parent_name=self.table.full_name)


class TableColumn(Model, AuditMixinNullable):

Expand Down Expand Up @@ -1135,11 +1144,25 @@ def recursive_get_fields(_conf):
conf.get('fn', "/"),
conf.get('fields', []),
conf.get('name', ''))

aggregations = {
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":
granularity = utils.parse_human_timedelta(
Expand Down Expand Up @@ -1329,6 +1352,7 @@ class DruidMetric(Model, AuditMixinNullable):
enable_typechecks=False)
json = Column(Text)
description = Column(Text)
is_restricted = Column(Boolean, default=False, nullable=True)

@property
def json_obj(self):
Expand All @@ -1338,6 +1362,12 @@ def json_obj(self):
obj = {}
return obj

@property
def perm(self):
return (
"{parent_name}.[{obj.metric_name}](id:{obj.id})"
).format(obj=self,
parent_name=self.datasource.full_name)

class DruidColumn(Model, AuditMixinNullable):

Expand Down Expand Up @@ -1428,6 +1458,7 @@ def generate_metrics(self):
'fieldNames': [self.column_name]})
))
session = get_session()
new_metrics = []
for metric in metrics:
m = (
session.query(M)
Expand All @@ -1438,9 +1469,12 @@ def generate_metrics(self):
)
metric.datasource_name = self.datasource_name
if not m:
new_metrics.append(metric)
session.add(metric)
session.flush()

utils.init_metrics_perm(caravel, new_metrics)


class FavStar(Model):
__tablename__ = 'favstar'
Expand Down
21 changes: 21 additions & 0 deletions caravel/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ class CaravelSecurityException(CaravelException):
pass


class MetricPermException(Exception):
pass


def flasher(msg, severity=None):
"""Flask's flash if available, logging call if not"""
try:
Expand Down Expand Up @@ -211,6 +215,23 @@ def init(caravel):
for table_perm in table_perms:
merge_perm(sm, 'datasource_access', table_perm)

init_metrics_perm(caravel)


def init_metrics_perm(caravel, metrics=None):
db = caravel.db
models = caravel.models
sm = caravel.appbuilder.sm

if metrics is None:
metrics = []
for model in [models.SqlMetric, models.DruidMetric]:
metrics += list(db.session.query(model).all())

metric_perms = [metric.perm for metric in metrics]
for metric_perm in metric_perms:
merge_perm(sm, 'metric_access', metric_perm)


def datetime_f(dttm):
"""Formats datetime to take less room when it is recent"""
Expand Down
37 changes: 28 additions & 9 deletions caravel/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import json
import logging
import re
import sys
import time
import traceback
from datetime import datetime
Expand All @@ -29,6 +30,7 @@
from werkzeug.routing import BaseConverter
from wtforms.validators import ValidationError

import caravel
from caravel import appbuilder, db, models, viz, utils, app, sm, ascii_art

config = app.config
Expand Down Expand Up @@ -209,14 +211,19 @@ def post_update(self, col):

class SqlMetricInlineView(CompactCRUDMixin, CaravelModelView): # noqa
datamodel = SQLAInterface(models.SqlMetric)
list_columns = ['metric_name', 'verbose_name', 'metric_type']
list_columns = ['metric_name', 'verbose_name', 'metric_type',
'is_restricted']
edit_columns = [
'metric_name', 'description', 'verbose_name', 'metric_type',
'expression', 'table']
'expression', 'table', 'is_restricted']
description_columns = {
'expression': utils.markdown(
"a valid SQL expression as supported by the underlying backend. "
"Example: `count(DISTINCT userid)`", True),
'is_restricted': _("Whether the 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"),
}
add_columns = edit_columns
page_size = 500
Expand All @@ -228,15 +235,20 @@ class SqlMetricInlineView(CompactCRUDMixin, CaravelModelView): # noqa
'expression': _("SQL Expression"),
'table': _("Table"),
}

def post_add(self, new_item):
utils.init_metrics_perm(caravel, [new_item])

appbuilder.add_view_no_menu(SqlMetricInlineView)


class DruidMetricInlineView(CompactCRUDMixin, CaravelModelView): # noqa
datamodel = SQLAInterface(models.DruidMetric)
list_columns = ['metric_name', 'verbose_name', 'metric_type']
list_columns = ['metric_name', 'verbose_name', 'metric_type',
'is_restricted']
edit_columns = [
'metric_name', 'description', 'verbose_name', 'metric_type', 'json',
'datasource']
'datasource', 'is_restricted']
add_columns = edit_columns
page_size = 500
validators_columns = {
Expand All @@ -248,6 +260,10 @@ class DruidMetricInlineView(CompactCRUDMixin, CaravelModelView): # noqa
"[Druid Post Aggregation]"
"(http://druid.io/docs/latest/querying/post-aggregations.html)",
True),
'is_restricted': _("Whether the 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"),
Expand All @@ -257,6 +273,11 @@ class DruidMetricInlineView(CompactCRUDMixin, CaravelModelView): # noqa
'json': _("JSON"),
'datasource': _("Druid Datasource"),
}

def post_add(self, new_item):
utils.init_metrics_perm(caravel, [new_item])


appbuilder.add_view_no_menu(DruidMetricInlineView)


Expand Down Expand Up @@ -819,11 +840,9 @@ def overwrite_slice(self, slc):
@expose("/checkbox/<model_view>/<id_>/<attr>/<value>", methods=['GET'])
def checkbox(self, model_view, id_, attr, value):
"""endpoint for checking/unchecking any boolean in a sqla model"""
model = None
if model_view == 'TableColumnInlineView':
model = models.TableColumn
elif model_view == 'DruidColumnInlineView':
model = models.DruidColumn
views = sys.modules[__name__]
model_view_cls = getattr(views, model_view)
model = model_view_cls.datamodel.obj

obj = db.session.query(model).filter_by(id=id_).first()
if obj:
Expand Down
22 changes: 22 additions & 0 deletions docs/security.rst
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,25 @@ you to create your own roles, and union them to existing ones.

The best way to go is probably to give user ``Gamma`` plus another role
that would add specific permissions needed by this type of users.


Restricting the access to the 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.

0 comments on commit 4c6026f

Please sign in to comment.