Sharing embeds with safe parameters (#3495)

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

* 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 #3394

* support querystring params

* extract coercing of numbers to function, along with a friendlier
implementation

* wire embeds to textless endpoint

* allow users with view_only permissions to execute queries on the
textless endpoint, as it only allows safe queries to run

* enqueue jobs for ApiUsers

* add parameters component for embeds

* include existing parameters in embed code

* fetch correct values for json requests

* remove previous embed parameter code

* 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

* bring back ALLOW_PARAMETERS_IN_EMBEDS (with link on deprecation coming up)

* 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 on setting deprecation

* rephrase deprecation message

* 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

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

* split has_access between normal users and ApiKey users

* remove residues from bad rebase

* allow access to safe queries via api keys

* rename `object` to `obj`

* support both objects and group dicts in `has_access` and `require_access`

* simplify permission tests once `has_access` accepts groups

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

* rename `object` to `obj`

* support both objects and group dicts in `has_access` and `require_access`

* simplify permission tests once `has_access` accepts groups

* fix bad rebase

* send embed parameters through POST data

* no need to log `is_api_key`

* move query fetching by api_key to within the Query model

* fetch user by adding a get_by_id function on the User model

* pass parameters as POST data (fixes test failure introduced by switching
from query string parameters to POST data)

* test the right thing - queries with safe parameters should be embeddable

* introduce cy.clickThrough

* add another Cypress test to make sure unsafe queries cannot be embedded

* serialize Parameters into query string

* set is_api_key as the last parameter to (hopefully) avoid
backward-dependency problems

* Update redash/models/parameterized_query.py

Co-Authored-By: rauchy <omer@rauchy.net>

* attempt to fix empty percy snapshots

* snap percies after DOM is fully loaded

client/app/components/EditParameterSettingsDialog.jsx

@@ -131,7 +131,7 @@ function EditParameterSettingsDialog(props) {
       footer={[(
         <Button key="cancel" onClick={props.dialog.dismiss}>Cancel</Button>
       ), (
-        <Button key="submit" htmlType="submit" disabled={!isFulfilled()} type="primary" form="paramForm">
+        <Button key="submit" htmlType="submit" disabled={!isFulfilled()} type="primary" form="paramForm" data-test="SaveParameterSettings">
           {isNew ? 'Add Parameter' : 'OK'}
         </Button>
       )]}
@@ -153,9 +153,9 @@ function EditParameterSettingsDialog(props) {
           />
         </Form.Item>
         <Form.Item label="Type" {...formItemProps}>
-          <Select value={param.type} onChange={type => setParam({ ...param, type })}>
-            <Option value="text">Text</Option>
-            <Option value="number">Number</Option>
+          <Select value={param.type} onChange={type => setParam({ ...param, type })} data-test="ParameterTypeSelect">
+            <Option value="text" data-test="TextParameterTypeOption">Text</Option>
+            <Option value="number" data-test="NumberParameterTypeOption">Number</Option>
             <Option value="enum">Dropdown List</Option>
             <Option value="query">Query Based Dropdown List</Option>
             <Option disabled key="dv1">

client/app/components/ParameterValueInput.jsx

@@ -148,6 +148,7 @@ export class ParameterValueInput extends React.Component {
       <Input
         className={'form-control ' + className}
         defaultValue={value || ''}
+        data-test="TextParamInput"
         onChange={event => onSelect(event.target.value)}
       />
     );
@@ -179,7 +180,6 @@ export default function init(ngModule) {
         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/QueryEditor.jsx

@@ -289,7 +289,13 @@ class QueryEditor extends React.Component {
               </select>
               {this.props.canEdit ? (
                 <Tooltip placement="top" title={modKey + ' + S'}>
-                  <button type="button" className="btn btn-default m-l-5" onClick={this.props.saveQuery} title="Save">
+                  <button
+                    type="button"
+                    className="btn btn-default m-l-5"
+                    onClick={this.props.saveQuery}
+                    data-test="SaveButton"
+                    title="Save"
+                  >
                     <span className="fa fa-floppy-o" />
                     <span className="hidden-xs m-l-5">Save</span>
                     {this.props.isDirty ? '*' : null}

client/app/components/app-view/template.html

@@ -1,5 +1,5 @@
 <app-header ng-if="$ctrl.layout.showHeader"></app-header>
-<div ng-if="$ctrl.handler.error" class="fixed-container">
+<div ng-if="$ctrl.handler.error" class="fixed-container" data-test="ErrorMessage">
   <div class="container">
     <div class="col-md-8 col-md-push-2">
       <div class="error-state bg-white tiled">

client/app/components/parameters.html

@@ -5,7 +5,7 @@
 >
   <div class="form-group m-r-10" ng-repeat="param in parameters" data-test="ParameterName{{ param.name }}">
     <label class="parameter-label">{{param.title}}</label>
-    <button class="btn btn-default btn-xs" ng-if="editable" ng-click="showParameterSettings(param, $index)">
+    <button class="btn btn-default btn-xs" ng-if="editable" ng-click="showParameterSettings(param, $index)" data-test="ParameterSettings-{{ param.name }}">
       <i class="zmdi zmdi-settings"></i>
     </button>
     <parameter-value-input param="param"></parameter-value-input>

client/app/components/queries/embed-code-dialog.html

@@ -5,7 +5,7 @@ <h4 class="modal-title">Embed Code</h4>
 <div class="modal-body">
   <h5>IFrame Embed</h5>
   <div>
-    <code>&lt;iframe src="{{ $ctrl.embedUrl }}" width="720" height="391"&gt;&lt;/iframe&gt;</code>
+    <code data-test="EmbedIframe">&lt;iframe src="{{ $ctrl.embedUrl }}" width="720" height="391"&gt;&lt;/iframe&gt;</code>
   </div>
   <span class="text-muted">(height should be adjusted)</span>
   <div ng-if="$ctrl.snapshotUrl">

client/app/components/queries/embed-code-dialog.js

@@ -9,7 +9,7 @@ const EmbedCodeDialog = {
 
     this.embedUrl = `${clientConfig.basePath}embed/query/${this.query.id}/visualization/${
       this.visualization.id
-    }?api_key=${this.query.api_key}`;
+    }?api_key=${this.query.api_key}&${this.query.getParameters().toUrlParams()}`;
     if (window.snapshotUrlBuilder) {
       this.snapshotUrl = window.snapshotUrlBuilder(this.query, this.visualization);
     }

client/app/components/queries/visualization-embed.html

@@ -1,4 +1,4 @@
-<div class="tile m-l-10 m-r-10 embed__vis">
+<div class="tile m-l-10 m-r-10 embed__vis" data-test="VisualizationEmbed">
   <div class="embed-heading p-t-10 p-b-10 p-r-15 p-l-15">
     <h3>
       <img ng-src="{{$ctrl.logoUrl}}" style="height: 24px;"/>
@@ -12,6 +12,10 @@ <h3>
   </div>
 
   <div class="col-md-12 query__vis">
+    <div class="p-t-15 p-b-5" ng-if="$ctrl.query.getParametersDefs().length > 0">
+      <parameters parameters="$ctrl.query.getParametersDefs()"></parameters>
+    </div>
+
     <visualization-renderer visualization="$ctrl.visualization" query-result="$ctrl.queryResult" class="t-body">
     </visualization-renderer>
   </div>

client/app/components/queries/visualization-embed.js

@@ -22,6 +22,9 @@ const VisualizationEmbed = {
   },
 };
 
+const queryStringAsObject = () => location.search.slice(1).split('&').map(p => p.split('='))
+  .reduce((obj, [key, value]) => ({ ...obj, [key.replace(/^p_/, '')]: value }), {});
+
 export default function init(ngModule) {
   ngModule.component('visualizationEmbed', VisualizationEmbed);
 
@@ -37,7 +40,8 @@ export default function init(ngModule) {
     return session($http, $route, Auth).then(() => {
       const queryId = $route.current.params.queryId;
       const query = $http.get(`api/queries/${queryId}`).then(response => response.data);
-      const queryResult = $http.get(`api/queries/${queryId}/results.json${location.search}`).then(response => response.data);
+      const { api_key: apiKey, ...parameters } = queryStringAsObject();
+      const queryResult = $http.post(`api/queries/${queryId}/results?api_key=${apiKey}`, { parameters }).then(response => response.data);
       return $q.all([query, queryResult]);
     });
   }

client/app/pages/home/home.html

@@ -1,5 +1,8 @@
 <div class="container">
-  <div ng-if="!$ctrl.isEmailVerified" class="alert alert-warning">
+  <div ng-if="$ctrl.messages.includes('using-deprecated-embed-feature')" class="alert alert-warning">
+      You have enabled <code>ALLOW_PARAMETERS_IN_EMBEDS</code>. This setting is now deprecated and should be turned off. Parameters in embeds are supported by default. <a href="https://discuss.redash.io/t/support-for-parameters-in-embedded-visualizations/3337" target="_blank">Read more</a>.
+  </div>
+  <div ng-if="$ctrl.messages.includes('email-not-verified')" class="alert alert-warning">
       We have sent an email with a confirmation link to your email address. Please follow the link to verify your email address. <a ng-click="$ctrl.verifyEmail()">Resend email</a>.
   </div>
   <empty-state

client/app/pages/home/index.js

@@ -1,13 +1,13 @@
 import template from './home.html';
 import notification from '@/services/notification';
 
-function HomeCtrl(Events, Dashboard, Query, $http, currentUser) {
+function HomeCtrl(Events, Dashboard, Query, $http, messages) {
   Events.record('view', 'page', 'personal_homepage');
 
   this.noDashboards = false;
   this.noQueries = false;
 
-  this.isEmailVerified = currentUser.is_email_verified;
+  this.messages = messages;
 
   Dashboard.favorites().$promise.then((data) => {
     this.favoriteDashboards = data.results;

client/app/pages/queries/query.html

@@ -269,7 +269,7 @@ <h3>
                 </a>
               </li>
               <li>
-                <a ng-click="showEmbedDialog(query, selectedTab)" ng-if="!query.isNew()">
+                <a ng-click="showEmbedDialog(query, selectedTab)" ng-if="!query.isNew()" data-test="ShowEmbedDialogButton">
                   <span class="zmdi zmdi-code"></span> Embed elsewhere
                 </a>
               </li>

client/app/services/auth.js

@@ -20,6 +20,7 @@ export const currentUser = {
 };
 
 export const clientConfig = {};
+export const messages = [];
 
 const logger = debug('redash:auth');
 const session = { loaded: false };
@@ -29,6 +30,7 @@ function updateSession(sessionData) {
   extend(session, sessionData, { loaded: true });
   extend(currentUser, session.user);
   extend(clientConfig, session.client_config);
+  extend(messages, session.messages);
 }
 
 function AuthService($window, $location, $q, $http) {
@@ -61,7 +63,7 @@ function AuthService($window, $location, $q, $http) {
     loadConfig() {
       logger('Loading config');
       return $http.get('/api/config').then((response) => {
-        updateSession({ client_config: response.data.client_config, user: { permissions: [] } });
+        updateSession({ client_config: response.data.client_config, user: { permissions: [] }, messages: [] });
         return response.data;
       });
     },
@@ -111,6 +113,7 @@ export default function init(ngModule) {
   ngModule.factory('Auth', AuthService);
   ngModule.value('currentUser', currentUser);
   ngModule.value('clientConfig', clientConfig);
+  ngModule.value('messages', messages);
   ngModule.factory('apiKeyHttpInterceptor', apiKeyHttpInterceptor);
 
   ngModule.config(($httpProvider) => {

client/app/services/query.js

@@ -294,6 +294,14 @@ class Parameters {
     const params = this.get();
     return zipObject(map(params, i => i.name), map(params, i => i.getValue()));
   }
+
+  toUrlParams() {
+    const params = Object.assign(...this.get().map(p => p.toUrlParams()));
+    return Object
+      .keys(params)
+      .map(k => `${encodeURIComponent(k)}=${encodeURIComponent(params[k])}`)
+      .join('&');
+  }
 }
 
 function QueryResultErrorFactory($q) {

client/cypress/integration/embed/share_embed_spec.js

@@ -0,0 +1,54 @@
+describe('Embedded Queries', () => {
+  beforeEach(() => {
+    cy.login();
+    cy.visit('/queries/new');
+  });
+
+  it('are shared with safe parameters', () => {
+    cy.getByTestId('QueryEditor')
+      .get('.ace_text-input')
+      .type('SELECT * FROM organizations WHERE id=\'{{}{{}id}}\'{esc}', { force: true });
+
+    cy.getByTestId('TextParamInput').type('1');
+    cy.clickThrough(`
+      ParameterSettings-id
+      ParameterTypeSelect
+      NumberParameterTypeOption
+      SaveParameterSettings
+      ExecuteButton
+      SaveButton
+    `);
+    cy.getByTestId('ShowEmbedDialogButton').click({ force: true });
+    cy.getByTestId('EmbedIframe').invoke('text').then((iframe) => {
+      const embedUrl = iframe.match(/"(.*?)"/)[1];
+      cy.logout();
+      cy.visit(embedUrl);
+      cy.getByTestId('VisualizationEmbed', { timeout: 10000 }).should('exist');
+      cy.percySnapshot('Successfully Embedded Parameterized Query');
+    });
+  });
+
+  it('cannot be shared with unsafe parameters', () => {
+    cy.getByTestId('QueryEditor')
+      .get('.ace_text-input')
+      .type('SELECT * FROM organizations WHERE name=\'{{}{{}name}}\'{esc}', { force: true });
+
+    cy.getByTestId('TextParamInput').type('Redash');
+    cy.clickThrough(`
+      ParameterSettings-name
+      ParameterTypeSelect
+      TextParameterTypeOption
+      SaveParameterSettings
+      ExecuteButton
+      SaveButton
+    `);
+    cy.getByTestId('ShowEmbedDialogButton').click({ force: true });
+    cy.getByTestId('EmbedIframe').invoke('text').then((iframe) => {
+      const embedUrl = iframe.match(/"(.*?)"/)[1];
+      cy.logout();
+      cy.visit(embedUrl, { failOnStatusCode: false }); // prevent 403 from failing test
+      cy.getByTestId('ErrorMessage', { timeout: 10000 }).should('exist');
+      cy.percySnapshot('Unsuccessfully Embedded Parameterized Query');
+    });
+  });
+});

client/cypress/support/commands.js

@@ -12,3 +12,11 @@ Cypress.Commands.add('login', (email = 'admin@redash.io', password = 'password')
 
 Cypress.Commands.add('logout', () => cy.request('/logout'));
 Cypress.Commands.add('getByTestId', element => cy.get('[data-test="' + element + '"]'));
+Cypress.Commands.add('clickThrough', (elements) => {
+  elements
+    .trim()
+    .split(/\s/)
+    .filter(Boolean)
+    .forEach(element => cy.getByTestId(element).click());
+  return undefined;
+});

redash/handlers/authentication.py

@@ -243,6 +243,18 @@ def client_config():
     return client_config
 
 
+def messages():
+    messages = []
+
+    if not current_user.is_email_verified:
+        messages.append('email-not-verified')
+
+    if settings.ALLOW_PARAMETERS_IN_EMBEDS:
+        messages.append('using-deprecated-embed-feature')
+
+    return messages
+
+
 @routes.route('/api/config', methods=['GET'])
 def config(org_slug=None):
     return json_response({
@@ -266,12 +278,12 @@ def session(org_slug=None):
             'name': current_user.name,
             'email': current_user.email,
             'groups': current_user.group_ids,
-            'permissions': current_user.permissions,
-            'is_email_verified': current_user.is_email_verified
+            'permissions': current_user.permissions
         }
 
     return json_response({
         'user': user,
+        'messages': messages(),
         'org_slug': current_org.slug,
         'client_config': client_config()
     })

redash/handlers/queries.py

@@ -171,6 +171,7 @@ 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']
@@ -179,7 +180,8 @@ def require_access_to_dropdown_queries(user, query_def):
         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.')
+            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)