[security] allow for requesting access when denied on a dashboard view (

#1192)

* Request access on dashboard view

* Fixing the unit tests

* Refactored much in the tests

caravel/migrations/versions/ab3d66c4246e_add_cache_timeout_to_druid_cluster.py

@@ -0,0 +1,23 @@
+"""add_cache_timeout_to_druid_cluster
+
+Revision ID: ab3d66c4246e
+Revises: eca4694defa7
+Create Date: 2016-09-30 18:01:30.579760
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'ab3d66c4246e'
+down_revision = 'eca4694defa7'
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    op.add_column(
+        'clusters', sa.Column('cache_timeout', sa.Integer(), nullable=True))
+
+
+def downgrade():
+    op.drop_column('clusters', 'cache_timeout')

caravel/migrations/versions/ef8843b41dac_.py

@@ -0,0 +1,22 @@
+"""empty message
+
+Revision ID: ef8843b41dac
+Revises: ('3b626e2a6783', 'ab3d66c4246e')
+Create Date: 2016-10-02 10:35:38.825231
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'ef8843b41dac'
+down_revision = ('3b626e2a6783', 'ab3d66c4246e')
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    pass
+
+
+def downgrade():
+    pass

caravel/models.py

@@ -337,6 +337,10 @@ def table_names(self):
     def url(self):
         return "/caravel/dashboard/{}/".format(self.slug or self.id)
 
+    @property
+    def datasources(self):
+        return {slc.datasource for slc in self.slices}
+
     @property
     def metadata_dejson(self):
         if self.json_metadata:
@@ -1180,6 +1184,7 @@ class DruidCluster(Model, AuditMixinNullable):
     broker_port = Column(Integer)
     broker_endpoint = Column(String(255), default='druid/v2')
     metadata_last_refreshed = Column(DateTime)
+    cache_timeout = Column(Integer)
 
     def __repr__(self):
         return self.cluster_name
@@ -1245,6 +1250,10 @@ def metrics_combo(self):
             [(m.metric_name, m.verbose_name) for m in self.metrics],
             key=lambda x: x[1])
 
+    @property
+    def database(self):
+        return self.cluster
+
     @property
     def num_cols(self):
         return [c.column_name for c in self.columns if c.isnum]

caravel/templates/caravel/request_access.html

@@ -1,24 +1,20 @@
 {% extends "caravel/basic.html" %}
 {% block title %}{{ _("No Access!") }}{% endblock %}
 {% block body %}
+  <div class="container">
     {% include "caravel/flash_wrapper.html" %}
-    <div class="container">
-        <h4>
-            {{ _("You do not have permissions to access the datasource %(name)s.",
-                 name=datasource_name)
-            }}
-        </h4>
-        <div id="buttons">
-            <button  onclick="window.location.href = '{{ request_access_url }}';"
-                     id="request"
-            >
-                {{ _("Request Permissions") }}
-            </button>
-            <button onclick="window.location.href = '{{ slicemodelview_link }}';"
-                    id="cancel"
-            >
-                {{ _("Cancel") }}
-            </button>
-        </div>
+    <h4>
+      {{ _("You do not have permissions to access the datasource(s): %(name)s.",
+      name=datasource_names)
+      }}
+    </h4>
+    <div>
+      <button  onclick="window.location += '&action=go';">
+        {{ _("Request Permissions") }}
+      </button>
+      <button onclick="window.location.href = '/slicemodelview/list/';">
+        {{ _("Cancel") }}
+      </button>
     </div>
-{% endblock %}
+  </div>
+{% endblock %}

caravel/views.py

@@ -57,12 +57,8 @@ def database_access(self, database):
                 self.can_access("database_access", database.perm))
 
     def datasource_access(self, datasource):
-        if hasattr(datasource, "cluster"):
-            return (self.database_access(datasource.cluster) or
-                    self.can_access("datasource_access", datasource.perm))
-        else:
-            return (self.database_access(datasource.database) or
-                    self.can_access("datasource_access", datasource.perm))
+        return (self.database_access(datasource.database) or
+                self.can_access("datasource_access", datasource.perm))
 
 
 class ListWidgetWithCheckboxes(ListWidget):
@@ -202,6 +198,7 @@ def apply(self, query, func):  # noqa
 
 
 class FilterDashboard(CaravelFilter):
+    """List dashboards for which users have access to at least one slice"""
     def apply(self, query, func):  # noqa
         if any([r.name in ('Admin', 'Alpha') for r in get_user_roles()]):
             return query
@@ -662,7 +659,7 @@ class DruidClusterModelView(CaravelModelView, DeleteMixin):  # noqa
     add_columns = [
         'cluster_name',
         'coordinator_host', 'coordinator_port', 'coordinator_endpoint',
-        'broker_host', 'broker_port', 'broker_endpoint',
+        'broker_host', 'broker_port', 'broker_endpoint', 'cache_timeout',
     ]
     edit_columns = add_columns
     list_columns = ['cluster_name', 'metadata_last_refreshed']
@@ -998,53 +995,43 @@ class Caravel(BaseCaravelView):
     """The base views for Caravel!"""
     @log_this
     @has_access
-    @expose("/request_access_form/<datasource_type>/<datasource_id>/"
-            "<datasource_name>")
-    def request_access_form(
-            self, datasource_type, datasource_id, datasource_name):
-        request_access_url = (
-            '/caravel/request_access?datasource_type={}&datasource_id={}&'
-            'datasource_name=datasource_name'.format(
-                datasource_type, datasource_id, datasource_name)
-        )
-        return self.render_template(
-            'caravel/request_access.html',
-            request_access_url=request_access_url,
-            datasource_name=datasource_name,
-            slicemodelview_link='/slicemodelview/list/')
-
-    @log_this
-    @has_access
-    @expose("/request_access")
+    @expose("/request_access/")
     def request_access(self):
+        datasources = set()
+        dashboard_id = request.args.get('dashboard_id')
+        if dashboard_id:
+            dash = (
+                db.session.query(models.Dashboard)
+                .filter_by(id=int(dashboard_id))
+                .one()
+            )
+            datasources |= dash.datasources
         datasource_id = request.args.get('datasource_id')
         datasource_type = request.args.get('datasource_type')
-        datasource_name = request.args.get('datasource_name')
-        session = db.session
+        if datasource_id:
+            ds_class = SourceRegistry.sources.get(datasource_type)
+            datasource = (
+                db.session.query(ds_class)
+                .filter_by(id=int(datasource_id))
+                .one()
+            )
+            datasources.add(datasource)
+        if request.args.get('action') == 'go':
+            for datasource in datasources:
+                access_request = DAR(
+                    datasource_id=datasource.id,
+                    datasource_type=datasource.type)
+                db.session.add(access_request)
+                db.session.commit()
+            flash(__("Access was requested"), "info")
+            return redirect('/')
 
-        duplicates = (
-            session.query(DAR)
-            .filter(
-                DAR.datasource_id == datasource_id,
-                DAR.datasource_type == datasource_type,
-                DAR.created_by_fk == g.user.id)
-            .all()
+        return self.render_template(
+            'caravel/request_access.html',
+            datasources=datasources,
+            datasource_names=", ".join([o.name for o in datasources]),
         )
 
-        if duplicates:
-            flash(__(
-                "You have already requested access to the datasource %(name)s",
-                name=datasource_name), "warning")
-            return redirect('/slicemodelview/list/')
-
-        access_request = DAR(datasource_id=datasource_id,
-                             datasource_type=datasource_type)
-        db.session.add(access_request)
-        db.session.commit()
-        flash(__("Access to the datasource %(name)s was requested",
-                 name=datasource_name), "info")
-        return redirect('/slicemodelview/list/')
-
     @log_this
     @has_access
     @expose("/approve")
@@ -1132,8 +1119,11 @@ def explore(self, datasource_type, datasource_id, slice_id=None):
         if not self.datasource_access(datasource):
             flash(
                 __(get_datasource_access_error_msg(datasource.name)), "danger")
-            return redirect('caravel/request_access_form/{}/{}/{}'.format(
-                datasource_type, datasource_id, datasource.name))
+            return redirect(
+                'caravel/request_access/?'
+                'datasource_type={datasource_type}&'
+                'datasource_id={datasource_id}&'
+                ''.format(**locals()))
 
         request_args_multi_dict = request.args  # MultiDict
 
@@ -1524,15 +1514,26 @@ def dashboard(self, dashboard_id):
             qry = qry.filter_by(slug=dashboard_id)
 
         templates = session.query(models.CssTemplate).all()
-        dash = qry.first()
+        dash = qry.one()
+        datasources = {slc.datasource for slc in dash.slices}
+        for datasource in datasources:
+            if not self.datasource_access(datasource):
+                flash(
+                    __(get_datasource_access_error_msg(datasource.name)),
+                    "danger")
+                return redirect(
+                    'caravel/request_access/?'
+                    'dashboard_id={dash.id}&'
+                    ''.format(**locals()))
 
         # Hack to log the dashboard_id properly, even when getting a slug
         @log_this
         def dashboard(**kwargs):  # noqa
             pass
         dashboard(dashboard_id=dash.id)
         dash_edit_perm = check_ownership(dash, raise_if_false=False)
-        dash_save_perm = dash_edit_perm and self.can_access('can_save_dash', 'Caravel')
+        dash_save_perm = \
+            dash_edit_perm and self.can_access('can_save_dash', 'Caravel')
         return self.render_template(
             "caravel/dashboard.html", dashboard=dash,
             user_id=g.user.get_id(),

run_specific_test.sh

@@ -0,0 +1,8 @@
+#!/usr/bin/env bash
+echo $DB
+rm -f .coverage
+export CARAVEL_CONFIG=tests.caravel_test_config
+set -e
+caravel/bin/caravel version -v
+export SOLO_TEST=1
+nosetests tests.core_tests:CoreTests.test_public_user_dashboard_access