Allow Parameters on Public Dashboards (getredash#3659)

* change has_access and require_access signatures to work with the objects that require access, instead of their groups

* use the textless endpoint (/api/queries/:id/results) for pristine
queriest

* Revert "use the textless endpoint (/api/queries/:id/results) for pristine"

This reverts commit cd2cee7.

* go to textless /api/queries/:id/results by default

* change `run_query`'s signature to accept a ParameterizedQuery instead of
constructing it inside

* raise HTTP 400 when receiving invalid parameter values. Fixes getredash#3394

* enqueue jobs for ApiUsers

* rename `id` to `user_id`

* support executing queries using Query api_keys by instantiating an ApiUser that would be able to execute the specific query

* show deprecation messages for ALLOW_PARAMETERS_IN_EMBEDS. Also, move
other message (email not verified) to use the same mechanism

* add link to forum message regarding embed deprecation

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

* split to 2 different dropdown endpoints and implement the second

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

* use new /dropdowns endpoint in frontend

* first e2e test for sharing embeds

* Pleasing the CodeClimate overlords

* All glory to CodeClimate

* remove residues from bad rebase

* add query id and data source id to serialized public dashboards

* add global parameters directive to public dashboards page

* allow access to a query by the api_key of the dashboard which includes
it

* rename `object` to `obj`

* simplify permission tests once `has_access` accepts groups

* support global parameters for public dashboards

* change has_access and require_access signatures to work with the objects that require access, instead of their groups

* rename `object` to `obj`

* simplify permission tests once `has_access` accepts groups

* no need to log `is_api_key`

* send parameters to public dashboard page

* allow access to a query by the api_key of the dashboard which includes it

* disable sharing if dashboard is associated with unsafe queries

* remove cypress test added in the wrong place due to a faulty rebase

* add support for clicking buttons in cy.clickThrough

* Cypress test which verifies that dashboards with safe queries can be shared

* Cypress test which verifies that dashboards with unsafe queries can't be shared

* remove duplicate tests

* use this.enabled and negate when needed

* remove stale comment

* add another Cypress test to verify that unauthenticated users have access to public dashboards with parameters

* obviously, I commit 'only' the first time I use it

* search for query access by query id and not api_key

* no need to fetch latest query data as it is loaded by frontend from the textless endpoint

* test that queries associated with dashboards are accessible when supplying the dashboard api_key

* propagate `isDirty` down to `QueryBasedParameterInput`

* go to /api/:id/dropdown while editing a query, since dropdown queries might still not be associated with the parent. see getredash#3711

* show helpful error message if dropdown values cannot be fetched

* use backticks instead of line concatenation

* remove requirement to have direct access to dropdown query in order validate it. parent query association checks are sufficient

* remove isDirty-based implementation and allow dropdown queries through nested ACL even if they aren't associated yet (given that the user has _direct_ access to the dropdown query)

* fix tests to cover all cases for /api/queries/:id/dropdowns/:id

* fix indentation

* require access to the query, not the data source

* resolve dashboard user by query id

* apply new copy to Cypress tests

* if only something would have prevented me from commiting an 'only' call 🤔

* very important handling of whitespace

* respond to parameter's Apply button

* text widgets are safe for sharing

* remove redundant event

* add a safety check that object has dashboard_api_keys before calling it

* supply a parameter value for text parameters to have it show up

* add parameter values for date and datetime

* use the current year and month to avoid pagination

* use Cypress.moment() instead of preinstalled moment()

* explicitly create parameters

* refresh query data if a  querystring parameter is provided

* avoid sending a data_source_id - it's only relevant to unsaved queries, since a saved query's data_source is available in the backend

* remove empty query text workaround

* provide default value to parameter

* add a few more dashboard sharing specs

* lint

* wait for DynamicTable to appear to reveal that actual results are displaying

* override error message for unsafely shared widgets

client/app/pages/dashboards/ShareDashboardDialog.jsx

@@ -16,7 +16,7 @@ const API_SHARE_URL = 'api/dashboards/{id}/share';
 class ShareDashboardDialog extends React.Component {
   static propTypes = {
     dashboard: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types
-    hasQueryParams: PropTypes.bool.isRequired,
+    hasOnlySafeQueries: PropTypes.bool.isRequired,
     dialog: DialogPropType.isRequired,
   };
 
@@ -35,7 +35,7 @@ class ShareDashboardDialog extends React.Component {
     };
 
     this.apiUrl = replace(API_SHARE_URL, '{id}', dashboard.id);
-    this.disabled = this.props.hasQueryParams && !dashboard.publicAccessEnabled;
+    this.enabled = this.props.hasOnlySafeQueries || dashboard.publicAccessEnabled;
   }
 
   static get headerContent() {
@@ -104,10 +104,10 @@ class ShareDashboardDialog extends React.Component {
         footer={null}
       >
         <Form layout="horizontal">
-          {this.props.hasQueryParams && (
+          {!this.props.hasOnlySafeQueries && (
             <Form.Item>
               <Alert
-                message="Sharing is currently not supported for dashboards containing queries with parameters."
+                message="For your security, sharing is currently not supported for dashboards containing queries with text parameters. Consider changing the text parameters in your query to a different type."
                 type="error"
               />
             </Form.Item>
@@ -117,12 +117,13 @@ class ShareDashboardDialog extends React.Component {
               checked={dashboard.publicAccessEnabled}
               onChange={this.onChange}
               loading={this.state.saving}
-              disabled={this.disabled}
+              disabled={!this.enabled}
+              data-test="PublicAccessEnabled"
             />
           </Form.Item>
           {dashboard.public_url && (
             <Form.Item label="Secret address" {...this.formItemProps}>
-              <InputWithCopy value={dashboard.public_url} />
+              <InputWithCopy value={dashboard.public_url} data-test="SecretAddress" />
             </Form.Item>
           )}
         </Form>

client/app/pages/dashboards/dashboard.html

@@ -67,7 +67,7 @@ <h3>
           <button type="button" class="btn btn-sm hidden-xs" ng-class="{'btn-default': !$ctrl.isFullscreen, 'btn-primary': $ctrl.isFullscreen}" tooltip="Enable/Disable Fullscreen display" ng-click="$ctrl.toggleFullscreen()" ng-if="!$ctrl.dashboard.is_draft && !$ctrl.layoutEditing">
             <span class="zmdi zmdi-fullscreen"></span>
           </button>
-          <button type="button" class="btn btn-sm hidden-xs" ng-class="{'btn-default': !$ctrl.dashboard.publicAccessEnabled, 'btn-primary': $ctrl.dashboard.publicAccessEnabled}" tooltip="Enable/Disable Share URL" ng-click="$ctrl.openShareForm()" ng-if="($ctrl.dashboard.canEdit() || $ctrl.dashboard.publicAccessEnabled) && !$ctrl.dashboard.is_draft && !$ctrl.layoutEditing">
+          <button type="button" class="btn btn-sm hidden-xs" ng-class="{'btn-default': !$ctrl.dashboard.publicAccessEnabled, 'btn-primary': $ctrl.dashboard.publicAccessEnabled}" tooltip="Enable/Disable Share URL" ng-click="$ctrl.openShareForm()" ng-if="($ctrl.dashboard.canEdit() || $ctrl.dashboard.publicAccessEnabled) && !$ctrl.dashboard.is_draft && !$ctrl.layoutEditing" data-test="OpenShareForm">
             <span class="zmdi zmdi-share"></span>
           </button>
       </span>

client/app/pages/dashboards/dashboard.js

@@ -387,15 +387,14 @@ function DashboardCtrl(
   }
 
   this.openShareForm = () => {
-    // check if any of the wigets have query parameters
-    const hasQueryParams = _.some(
+    const hasOnlySafeQueries = _.every(
       this.dashboard.widgets,
-      w => !_.isEmpty(w.getQuery() && w.getQuery().getParametersDefs()),
+      w => (w.getQuery() ? w.getQuery().is_safe : true),
     );
 
     ShareDashboardDialog.showModal({
       dashboard: this.dashboard,
-      hasQueryParams,
+      hasOnlySafeQueries,
     });
   };
 }

client/app/pages/dashboards/public-dashboard-page.html

@@ -1,6 +1,10 @@
-<div class="container p-t-10 p-b-20">
+<div class="container p-t-10 p-b-20" ng-if="$ctrl.dashboard">
   <page-header title="$ctrl.dashboard.name"></page-header>
 
+  <div class="m-b-10 p-15 bg-white tiled" ng-if="$ctrl.globalParameters.length > 0">
+    <parameters parameters="$ctrl.globalParameters" on-values-change="$ctrl.refreshDashboard"></parameters>
+  </div>
+
   <div class="m-b-5">
     <filters filters="$ctrl.filters" on-change="$ctrl.filtersOnChange"></filters>
   </div>

client/app/pages/dashboards/public-dashboard-page.js

@@ -25,41 +25,47 @@ const PublicDashboardPage = {
 
     this.logoUrl = logoUrl;
     this.public = true;
-    this.dashboard.widgets = Dashboard.prepareDashboardWidgets(this.dashboard.widgets);
+    this.globalParameters = [];
+
+    this.extractGlobalParameters = () => {
+      this.globalParameters = this.dashboard.getParametersDefs();
+    };
 
     const refreshRate = Math.max(30, parseFloat($location.search().refresh));
 
-    if (refreshRate) {
-      const refresh = () => {
-        loadDashboard($http, $route).then((data) => {
-          this.dashboard = data;
-          this.dashboard.widgets = Dashboard.prepareDashboardWidgets(this.dashboard.widgets);
-          this.dashboard.widgets.forEach(widget => widget.load());
+    this.refreshDashboard = () => {
+      loadDashboard($http, $route).then((data) => {
+        this.dashboard = new Dashboard(data);
+        this.dashboard.widgets = Dashboard.prepareDashboardWidgets(this.dashboard.widgets);
+        this.dashboard.widgets.forEach((widget) => {
+          widget.load(!!refreshRate).catch((error) => {
+            const isSafe = widget.getQuery() ? widget.getQuery().is_safe : true;
+            if (!isSafe) {
+              error.errorMessage = 'This query contains potentially unsafe parameters and cannot be executed on a publicly shared dashboard.';
+            }
+          });
+        });
+        this.filters = []; // TODO: implement (@/services/dashboard.js:collectDashboardFilters)
+        this.filtersOnChange = (allFilters) => {
+          this.filters = allFilters;
+          $scope.$applyAsync();
+        };
 
-          this.filters = []; // TODO: implement (@/services/dashboard.js:collectDashboardFilters)
-          this.filtersOnChange = (allFilters) => {
-            this.filters = allFilters;
-            $scope.$applyAsync();
-          };
+        this.extractGlobalParameters();
+      });
 
-          $timeout(refresh, refreshRate * 1000.0);
-        });
-      };
+      if (refreshRate) {
+        $timeout(this.refreshDashboard, refreshRate * 1000.0);
+      }
+    };
 
-      $timeout(refresh, refreshRate * 1000.0);
-    }
+    this.refreshDashboard();
   },
 };
 
 export default function init(ngModule) {
   ngModule.component('publicDashboardPage', PublicDashboardPage);
 
-  function loadPublicDashboard($http, $route) {
-    'ngInject';
-
-    return loadDashboard($http, $route);
-  }
-
   function session($http, $route, Auth) {
     const token = $route.current.params.token;
     Auth.setApiKey(token);
@@ -68,10 +74,9 @@ export default function init(ngModule) {
 
   ngModule.config(($routeProvider) => {
     $routeProvider.when('/public/dashboards/:token', {
-      template: '<public-dashboard-page dashboard="$resolve.dashboard"></public-dashboard-page>',
+      template: '<public-dashboard-page></public-dashboard-page>',
       reloadOnSearch: false,
       resolve: {
-        dashboard: loadPublicDashboard,
         session,
       },
     });

client/app/services/dashboard.js

@@ -183,7 +183,6 @@ function DashboardService($resource, $http, $location, currentUser) {
 
   resource.prepareDashboardWidgets = prepareDashboardWidgets;
   resource.prepareWidgetsForDashboard = prepareWidgetsForDashboard;
-
   resource.prototype.getParametersDefs = function getParametersDefs() {
     const globalParams = {};
     const queryParams = $location.search();
@@ -192,7 +191,7 @@ function DashboardService($resource, $http, $location, currentUser) {
         const mappings = widget.getParameterMappings();
         widget
           .getQuery()
-          .getParametersDefs()
+          .getParametersDefs(false)
           .forEach((param) => {
             const mapping = mappings[param.name];
             if (mapping.type === Widget.MappingType.DashboardLevel) {

client/app/services/query.js

@@ -219,25 +219,32 @@ class Parameters {
   }
 
   parseQuery() {
+    const fallback = () => map(this.query.options.parameters, i => i.name);
+
     let parameters = [];
-    try {
-      const parts = Mustache.parse(this.query.query);
-      parameters = uniq(collectParams(parts));
-    } catch (e) {
-      logger('Failed parsing parameters: ', e);
-      // Return current parameters so we don't reset the list
-      parameters = map(this.query.options.parameters, i => i.name);
+    if (this.query.query) {
+      try {
+        const parts = Mustache.parse(this.query.query);
+        parameters = uniq(collectParams(parts));
+      } catch (e) {
+        logger('Failed parsing parameters: ', e);
+        // Return current parameters so we don't reset the list
+        parameters = fallback();
+      }
+    } else {
+      parameters = fallback();
     }
+
     return parameters;
   }
 
-  updateParameters() {
-    if (this.query.query === this.cachedQueryText) {
+  updateParameters(update) {
+    if (this.query.query && this.query.query === this.cachedQueryText) {
       return;
     }
 
     this.cachedQueryText = this.query.query;
-    const parameterNames = this.parseQuery();
+    const parameterNames = update ? this.parseQuery() : map(this.query.options.parameters, p => p.name);
 
     this.query.options.parameters = this.query.options.parameters || [];
 
@@ -269,8 +276,8 @@ class Parameters {
     });
   }
 
-  get() {
-    this.updateParameters();
+  get(update = true) {
+    this.updateParameters(update);
     return this.query.options.parameters;
   }
 
@@ -478,10 +485,6 @@ function QueryResource(
   };
 
   QueryService.prototype.prepareQueryResultExecution = function prepareQueryResultExecution(execute, maxAge) {
-    if (!this.query) {
-      return new QueryResultError("Can't execute empty query.");
-    }
-
     const parameters = this.getParameters();
     const missingParams = parameters.getMissing();
 
@@ -517,10 +520,8 @@ function QueryResource(
       if (!this.queryResult) {
         this.queryResult = QueryResult.getById(this.id, this.latest_query_data_id);
       }
-    } else if (this.data_source_id) {
-      this.queryResult = execute();
     } else {
-      return new QueryResultError('Please select data source to run this query.');
+      this.queryResult = execute();
     }
 
     return this.queryResult;
@@ -533,6 +534,10 @@ function QueryResource(
 
   QueryService.prototype.getQueryResultByText = function getQueryResultByText(maxAge, selectedQueryText) {
     const queryText = selectedQueryText || this.query;
+    if (!queryText) {
+      return new QueryResultError("Can't execute empty query.");
+    }
+
     const parameters = this.getParameters().getValues();
     const execute = () => QueryResult.get(this.data_source_id, queryText, parameters, maxAge, this.id);
     return this.prepareQueryResultExecution(execute, maxAge);
@@ -576,8 +581,8 @@ function QueryResource(
     return this.$parameters;
   };
 
-  QueryService.prototype.getParametersDefs = function getParametersDefs() {
-    return this.getParameters().get();
+  QueryService.prototype.getParametersDefs = function getParametersDefs(update = true) {
+    return this.getParameters().get(update);
   };
 
   return QueryService;

client/app/services/widget.js

@@ -212,7 +212,7 @@ function WidgetFactory($http, $location, Query) {
 
       const existingParams = {};
       // textboxes does not have query
-      const params = this.getQuery() ? this.getQuery().getParametersDefs() : [];
+      const params = this.getQuery() ? this.getQuery().getParametersDefs(false) : [];
       each(params, (param) => {
         existingParams[param.name] = true;
         if (!isObject(this.options.parameterMappings[param.name])) {