Nest query ACL to dropdowns (getredash#3544)

* change API to /api/queries/:id/dropdowns/:dropdown_id

* extract  property

* split to 2 different dropdown endpoints and implement the second

* make access control optional for dropdowns (assuming it is verified at a
different level)

* add test cases for /api/queries/:id/dropdowns/:id

* use new /dropdowns endpoint in frontend

* require access to dropdown queries when creating or updating parent
queries

* rename Query resource dropdown endpoints

* check access to dropdown query associations in one fugly query

* move ParameterizedQuery to models folder

* add dropdown association tests to query creation

* move group by query ids query into models.Query

* use bound parameters for groups query

* format groups query

* use new associatedDropdowns endpoint in dashboards

* pass down parameter and let it return dropdown options. Go Levko!

* change API to /api/queries/:id/dropdowns/:dropdown_id

* split to 2 different dropdown endpoints and implement the second

* use new /dropdowns endpoint in frontend

* pass down parameter and let it return dropdown options. Go Levko!

* fix bad rebase

* add comment to clarify the purpose of checking the queryId

client/app/components/ParameterValueInput.jsx

@@ -18,6 +18,7 @@ export class ParameterValueInput extends React.Component {
     value: PropTypes.any, // eslint-disable-line react/forbid-prop-types
     enumOptions: PropTypes.string,
     queryId: PropTypes.number,
+    parameter: PropTypes.any, // eslint-disable-line react/forbid-prop-types
     onSelect: PropTypes.func,
     className: PropTypes.string,
   };
@@ -27,6 +28,7 @@ export class ParameterValueInput extends React.Component {
     value: null,
     enumOptions: '',
     queryId: null,
+    parameter: null,
     onSelect: () => {},
     className: '',
   };
@@ -117,10 +119,11 @@ export class ParameterValueInput extends React.Component {
   }
 
   renderQueryBasedInput() {
-    const { value, onSelect, queryId } = this.props;
+    const { value, onSelect, queryId, parameter } = this.props;
     return (
       <QueryBasedParameterInput
         className={this.props.className}
+        parameter={parameter}
         value={value}
         queryId={queryId}
         onSelect={onSelect}
@@ -173,8 +176,10 @@ export default function init(ngModule) {
       <parameter-value-input-impl
         type="$ctrl.param.type"
         value="$ctrl.param.normalizedValue"
+        parameter="$ctrl.param"
         enum-options="$ctrl.param.enumOptions"
         query-id="$ctrl.param.queryId"
+        parent-query-id="$ctrl.param.parentQueryId"
         on-select="$ctrl.setValue"
       ></parameter-value-input-impl>
     `,

client/app/components/QueryBasedParameterInput.jsx

@@ -3,12 +3,12 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import { react2angular } from 'react2angular';
 import Select from 'antd/lib/select';
-import { Query } from '@/services/query';
 
 const { Option } = Select;
 
 export class QueryBasedParameterInput extends React.Component {
   static propTypes = {
+    parameter: PropTypes.any, // eslint-disable-line react/forbid-prop-types
     value: PropTypes.any, // eslint-disable-line react/forbid-prop-types
     queryId: PropTypes.number,
     onSelect: PropTypes.func,
@@ -17,6 +17,7 @@ export class QueryBasedParameterInput extends React.Component {
 
   static defaultProps = {
     value: null,
+    parameter: null,
     queryId: null,
     onSelect: () => {},
     className: '',
@@ -41,19 +42,20 @@ export class QueryBasedParameterInput extends React.Component {
     }
   }
 
-  _loadOptions(queryId) {
+  async _loadOptions(queryId) {
     if (queryId && (queryId !== this.state.queryId)) {
       this.setState({ loading: true });
-      Query.dropdownOptions({ id: queryId }, (options) => {
-        if (this.props.queryId === queryId) {
-          this.setState({ options, loading: false });
+      const options = await this.props.parameter.loadDropdownValues();
 
-          const found = find(options, option => option.value === this.props.value) !== undefined;
-          if (!found && isFunction(this.props.onSelect)) {
-            this.props.onSelect(options[0].value);
-          }
+      // stale queryId check
+      if (this.props.queryId === queryId) {
+        this.setState({ options, loading: false });
+
+        const found = find(options, option => option.value === this.props.value) !== undefined;
+        if (!found && isFunction(this.props.onSelect)) {
+          this.props.onSelect(options[0].value);
         }
-      });
+      }
     }
   }
 

client/app/services/query.js

@@ -50,14 +50,15 @@ function isDateRangeParameter(paramType) {
 }
 
 export class Parameter {
-  constructor(parameter) {
+  constructor(parameter, parentQueryId) {
     this.title = parameter.title;
     this.name = parameter.name;
     this.type = parameter.type;
     this.useCurrentDateTime = parameter.useCurrentDateTime;
     this.global = parameter.global; // backward compatibility in Widget service
     this.enumOptions = parameter.enumOptions;
     this.queryId = parameter.queryId;
+    this.parentQueryId = parentQueryId;
 
     // Used for meta-parameters (i.e. dashboard-level params)
     this.locals = [];
@@ -75,7 +76,7 @@ export class Parameter {
   }
 
   clone() {
-    return new Parameter(this);
+    return new Parameter(this, this.parentQueryId);
   }
 
   get isEmpty() {
@@ -200,6 +201,14 @@ export class Parameter {
     }
     return `{{ ${this.name} }}`;
   }
+
+  loadDropdownValues() {
+    if (this.parentQueryId) {
+      return Query.associatedDropdown({ queryId: this.parentQueryId, dropdownQueryId: this.queryId }).$promise;
+    }
+
+    return Query.asDropdown({ id: this.queryId }).$promise;
+  }
 }
 
 class Parameters {
@@ -250,7 +259,8 @@ class Parameters {
     });
 
     const parameterExists = p => includes(parameterNames, p.name);
-    this.query.options.parameters = this.query.options.parameters.filter(parameterExists).map(p => new Parameter(p));
+    const parameters = this.query.options.parameters;
+    this.query.options.parameters = parameters.filter(parameterExists).map(p => new Parameter(p, this.query.id));
   }
 
   initFromQueryString(query) {
@@ -371,11 +381,16 @@ function QueryResource(
         isArray: false,
         url: 'api/queries/:id/results.json',
       },
-      dropdownOptions: {
+      asDropdown: {
         method: 'get',
         isArray: true,
         url: 'api/queries/:id/dropdown',
       },
+      associatedDropdown: {
+        method: 'get',
+        isArray: true,
+        url: 'api/queries/:queryId/dropdowns/:dropdownQueryId',
+      },
       favorites: {
         method: 'get',
         isArray: false,

redash/handlers/api.py

@@ -38,6 +38,7 @@
                                      QueryTagsResource)
 from redash.handlers.query_results import (JobResource,
                                            QueryResultDropdownResource,
+                                           QueryDropdownsResource,
                                            QueryResultListResource,
                                            QueryResultResource)
 from redash.handlers.query_snippets import (QuerySnippetListResource,
@@ -120,6 +121,7 @@ def json_representation(data, code, headers=None):
 
 api.add_org_resource(QueryResultListResource, '/api/query_results', endpoint='query_results')
 api.add_org_resource(QueryResultDropdownResource, '/api/queries/<query_id>/dropdown', endpoint='query_result_dropdown')
+api.add_org_resource(QueryDropdownsResource, '/api/queries/<query_id>/dropdowns/<dropdown_query_id>', endpoint='query_result_dropdowns')
 api.add_org_resource(QueryResultResource,
                      '/api/query_results/<query_result_id>.<filetype>',
                      '/api/query_results/<query_result_id>',

redash/handlers/queries.py

@@ -16,7 +16,7 @@
                                 require_permission, view_only)
 from redash.utils import collect_parameters_from_request
 from redash.serializers import QuerySerializer
-from redash.utils.parameterized_query import ParameterizedQuery
+from redash.models.parameterized_query import ParameterizedQuery
 
 
 # Ordering map for relationships
@@ -171,6 +171,17 @@ def get(self):
 
         return response
 
+def require_access_to_dropdown_queries(user, query_def):
+    parameters = query_def.get('options', {}).get('parameters', [])
+    dropdown_query_ids = [str(p['queryId']) for p in parameters if p['type'] == 'query']
+
+    if dropdown_query_ids:
+        groups = models.Query.all_groups_for_query_ids(dropdown_query_ids)
+
+        if len(groups) < len(dropdown_query_ids):
+            abort(400, message='You are trying to associate a dropdown query that does not have a matching group. Please verify the dropdown query id you are trying to associate with this query.')
+
+        require_access(dict(groups), user, view_only)
 
 class QueryListResource(BaseQueryListResource):
     @require_permission('create_query')
@@ -210,6 +221,7 @@ def post(self):
         query_def = request.get_json(force=True)
         data_source = models.DataSource.get_by_id_and_org(query_def.pop('data_source_id'), self.current_org)
         require_access(data_source.groups, self.current_user, not_view_only)
+        require_access_to_dropdown_queries(self.current_user, query_def)
 
         for field in ['id', 'created_at', 'api_key', 'visualizations', 'latest_query_data', 'last_modified_by']:
             query_def.pop(field, None)
@@ -310,6 +322,7 @@ def post(self, query_id):
         query_def = request.get_json(force=True)
 
         require_object_modify_permission(query, self.current_user)
+        require_access_to_dropdown_queries(self.current_user, query_def)
 
         for field in ['id', 'created_at', 'api_key', 'visualizations', 'latest_query_data', 'user', 'last_modified_by', 'org']:
             query_def.pop(field, None)

redash/handlers/query_results.py

@@ -11,7 +11,7 @@
 from redash.tasks import QueryTask
 from redash.tasks.queries import enqueue_query
 from redash.utils import (collect_parameters_from_request, gen_query_hash, json_dumps, utcnow, to_filename)
-from redash.utils.parameterized_query import ParameterizedQuery, InvalidParameterError, dropdown_values
+from redash.models.parameterized_query import ParameterizedQuery, InvalidParameterError, dropdown_values
 
 
 def error_response(message):
@@ -151,11 +151,20 @@ def post(self):
 
 ONE_YEAR = 60 * 60 * 24 * 365.25
 
-
 class QueryResultDropdownResource(BaseResource):
     def get(self, query_id):
         return dropdown_values(query_id)
 
+class QueryDropdownsResource(BaseResource):
+    def get(self, query_id, dropdown_query_id):
+        query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org)
+
+        related_queries_ids = [p['queryId'] for p in query.parameters if p['type'] == 'query']
+        if int(dropdown_query_id) not in related_queries_ids:
+            abort(403)
+
+        return dropdown_values(dropdown_query_id, should_require_access=False)
+
 
 class QueryResultResource(BaseResource):
     @staticmethod

redash/models/__init__.py

@@ -28,7 +28,7 @@
                                  get_query_runner)
 from redash.utils import generate_token, json_dumps, json_loads
 from redash.utils.configuration import ConfigurationContainer
-from redash.utils.parameterized_query import ParameterizedQuery
+from redash.models.parameterized_query import ParameterizedQuery
 
 from .base import db, gfk_type, Column, GFKBase, SearchBaseQuery
 from .changes import ChangeTrackingMixin, Change  # noqa
@@ -627,6 +627,15 @@ def recent(cls, group_ids, user_id=None, limit=20):
     def get_by_id(cls, _id):
         return cls.query.filter(cls.id == _id).one()
 
+    @classmethod
+    def all_groups_for_query_ids(cls, query_ids):
+        query = """SELECT group_id, view_only
+                   FROM queries
+                   JOIN data_source_groups ON queries.data_source_id = data_source_groups.data_source_id
+                   WHERE queries.id in :ids"""
+
+        return db.session.execute(query, {'ids': tuple(query_ids)}).fetchall()
+
     def fork(self, user):
         forked_list = ['org', 'data_source', 'latest_query_data', 'description',
                        'query_text', 'query_hash', 'options']
@@ -669,9 +678,13 @@ def lowercase_name(cls):
         "The SQLAlchemy expression for the property above."
         return func.lower(cls.name)
 
+    @property
+    def parameters(self):
+        return self.options.get("parameters", [])
+
     @property
     def parameterized(self):
-        return ParameterizedQuery(self.query_text, self.options.get("parameters", []))
+        return ParameterizedQuery(self.query_text, self.parameters)
 
 
 @listens_for(Query.query_text, 'set')

redash/utils/parameterized_query.py → redash/models/parameterized_query.py

@@ -16,19 +16,22 @@ def _pluck_name_and_value(default_column, row):
     return {"name": row[name_column], "value": unicode(row[value_column])}
 
 
-def _load_result(query_id):
+def _load_result(query_id, should_require_access):
     from redash.authentication.org_resolving import current_org
     from redash import models
 
     query = models.Query.get_by_id_and_org(query_id, current_org)
-    require_access(query.data_source.groups, current_user, view_only)
+
+    if should_require_access:
+        require_access(query.data_source.groups, current_user, view_only)
+
     query_result = models.QueryResult.get_by_id_and_org(query.latest_query_data_id, current_org)
 
     return json_loads(query_result.data)
 
 
-def dropdown_values(query_id):
-    data = _load_result(query_id)
+def dropdown_values(query_id, should_require_access=True):
+    data = _load_result(query_id, should_require_access)
     first_column = data["columns"][0]["name"]
     pluck = partial(_pluck_name_and_value, first_column)
     return map(pluck, data["rows"])

redash/serializers.py

@@ -10,7 +10,7 @@
 from redash import models
 from redash.permissions import has_access, view_only
 from redash.utils import json_loads
-from redash.utils.parameterized_query import ParameterizedQuery
+from redash.models.parameterized_query import ParameterizedQuery
 
 
 def public_widget(widget):