fix(embedded): CSV download for chart (apache#20261)

* move postForm to superset client * lint * fix lint * fix type * update tests * add tests * add test for form submit * add test for request form * lint * fix test * fix tests * more tests * more tests * test * lint * more test for postForm * lint * Update superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts Co-authored-by: David Aaron Suddjian <[email protected]> * update tests * remove useless test * make test cover happy * make test cover happy * make test cover happy * make codecov happy * make codecov happy Co-authored-by: David Aaron Suddjian <[email protected]>
benny-ew · Jun 18, 2022 · ab9f72f · ab9f72f
1 parent f53018c
commit ab9f72f
Show file tree

Hide file tree

Showing 12 changed files with 190 additions and 40 deletions.
diff --git a/superset-frontend/packages/superset-ui-core/src/connection/SupersetClient.ts b/superset-frontend/packages/superset-ui-core/src/connection/SupersetClient.ts
@@ -45,6 +45,7 @@ const SupersetClient: SupersetClientInterface = {
   init: force => getInstance().init(force),
   isAuthenticated: () => getInstance().isAuthenticated(),
   post: request => getInstance().post(request),
+  postForm: (...args) => getInstance().postForm(...args),
   put: request => getInstance().put(request),
   reAuthenticate: () => getInstance().reAuthenticate(),
   request: request => getInstance().request(request),

diff --git a/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts b/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts
@@ -119,6 +119,36 @@ export default class SupersetClientClass {
     return this.getCSRFToken();
   }
 
+  async postForm(url: string, payload: Record<string, any>, target = '_blank') {
+    if (url) {
+      await this.ensureAuth();
+      const hiddenForm = document.createElement('form');
+      hiddenForm.action = url;
+      hiddenForm.method = 'POST';
+      hiddenForm.target = target;
+      const payloadWithToken: Record<string, any> = {
+        ...payload,
+        csrf_token: this.csrfToken!,
+      };
+
+      if (this.guestToken) {
+        payloadWithToken.guest_token = this.guestToken;
+      }
+
+      Object.entries(payloadWithToken).forEach(([key, value]) => {
+        const data = document.createElement('input');
+        data.type = 'hidden';
+        data.name = key;
+        data.value = value;
+        hiddenForm.appendChild(data);
+      });
+
+      document.body.appendChild(hiddenForm);
+      hiddenForm.submit();
+      document.body.removeChild(hiddenForm);
+    }
+  }
+
   async reAuthenticate() {
     return this.init(true);
   }

diff --git a/superset-frontend/packages/superset-ui-core/src/connection/types.ts b/superset-frontend/packages/superset-ui-core/src/connection/types.ts
@@ -146,6 +146,7 @@ export interface SupersetClientInterface
     | 'delete'
     | 'get'
     | 'post'
+    | 'postForm'
     | 'put'
     | 'request'
     | 'init'

diff --git a/superset-frontend/packages/superset-ui-core/test/connection/SupersetClient.test.ts b/superset-frontend/packages/superset-ui-core/test/connection/SupersetClient.test.ts
@@ -30,21 +30,23 @@ describe('SupersetClient', () => {
 
   afterEach(SupersetClient.reset);
 
-  it('exposes reset, configure, init, get, post, isAuthenticated, and reAuthenticate methods', () => {
+  it('exposes reset, configure, init, get, post, postForm, isAuthenticated, and reAuthenticate methods', () => {
     expect(typeof SupersetClient.configure).toBe('function');
     expect(typeof SupersetClient.init).toBe('function');
     expect(typeof SupersetClient.get).toBe('function');
     expect(typeof SupersetClient.post).toBe('function');
+    expect(typeof SupersetClient.postForm).toBe('function');
     expect(typeof SupersetClient.isAuthenticated).toBe('function');
     expect(typeof SupersetClient.reAuthenticate).toBe('function');
     expect(typeof SupersetClient.request).toBe('function');
     expect(typeof SupersetClient.reset).toBe('function');
   });
 
-  it('throws if you call init, get, post, isAuthenticated, or reAuthenticate before configure', () => {
+  it('throws if you call init, get, post, postForm, isAuthenticated, or reAuthenticate before configure', () => {
     expect(SupersetClient.init).toThrow();
     expect(SupersetClient.get).toThrow();
     expect(SupersetClient.post).toThrow();
+    expect(SupersetClient.postForm).toThrow();
     expect(SupersetClient.isAuthenticated).toThrow();
     expect(SupersetClient.reAuthenticate).toThrow();
     expect(SupersetClient.request).toThrow();

diff --git a/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts b/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts
@@ -605,4 +605,107 @@ describe('SupersetClientClass', () => {
       }
     });
   });
+
+  describe('.postForm()', () => {
+    const protocol = 'https:';
+    const host = 'host';
+    const mockPostFormEndpoint = '/post_form/url';
+    const mockPostFormUrl = `${protocol}//${host}${mockPostFormEndpoint}`;
+    const guestToken = 'test-guest-token';
+    const postFormPayload = { number: 123, array: [1, 2, 3] };
+
+    let authSpy: jest.SpyInstance;
+    let client: SupersetClientClass;
+    let appendChild: any;
+    let removeChild: any;
+    let submit: any;
+    let createElement: any;
+
+    beforeEach(async () => {
+      client = new SupersetClientClass({ protocol, host });
+      authSpy = jest.spyOn(SupersetClientClass.prototype, 'ensureAuth');
+      await client.init();
+      appendChild = jest.fn();
+      removeChild = jest.fn();
+      submit = jest.fn();
+      createElement = jest.fn(() => ({
+        appendChild: jest.fn(),
+        submit,
+      }));
+
+      document.createElement = createElement as any;
+      document.body.appendChild = appendChild;
+      document.body.removeChild = removeChild;
+    });
+
+    afterEach(() => {
+      jest.restoreAllMocks();
+    });
+
+    it('makes postForm request', async () => {
+      await client.postForm(mockPostFormUrl, {});
+
+      const hiddenForm = createElement.mock.results[0].value;
+      const csrfTokenInput = createElement.mock.results[1].value;
+
+      expect(createElement.mock.calls).toHaveLength(2);
+
+      expect(hiddenForm.action).toBe(mockPostFormUrl);
+      expect(hiddenForm.method).toBe('POST');
+      expect(hiddenForm.target).toBe('_blank');
+
+      expect(csrfTokenInput.type).toBe('hidden');
+      expect(csrfTokenInput.name).toBe('csrf_token');
+      expect(csrfTokenInput.value).toBe(1234);
+
+      expect(appendChild.mock.calls).toHaveLength(1);
+      expect(removeChild.mock.calls).toHaveLength(1);
+      expect(authSpy).toHaveBeenCalledTimes(1);
+    });
+
+    it('makes postForm request with guest token', async () => {
+      client = new SupersetClientClass({ protocol, host, guestToken });
+      await client.init();
+
+      await client.postForm(mockPostFormUrl, {});
+
+      const guestTokenInput = createElement.mock.results[2].value;
+
+      expect(createElement.mock.calls).toHaveLength(3);
+
+      expect(guestTokenInput.type).toBe('hidden');
+      expect(guestTokenInput.name).toBe('guest_token');
+      expect(guestTokenInput.value).toBe(guestToken);
+
+      expect(appendChild.mock.calls).toHaveLength(1);
+      expect(removeChild.mock.calls).toHaveLength(1);
+      expect(authSpy).toHaveBeenCalledTimes(1);
+    });
+
+    it('makes postForm request with payload', async () => {
+      await client.postForm(mockPostFormUrl, { form_data: postFormPayload });
+
+      const postFormPayloadInput = createElement.mock.results[1].value;
+
+      expect(createElement.mock.calls).toHaveLength(3);
+
+      expect(postFormPayloadInput.type).toBe('hidden');
+      expect(postFormPayloadInput.name).toBe('form_data');
+      expect(postFormPayloadInput.value).toBe(postFormPayload);
+
+      expect(appendChild.mock.calls).toHaveLength(1);
+      expect(removeChild.mock.calls).toHaveLength(1);
+      expect(submit.mock.calls).toHaveLength(1);
+      expect(authSpy).toHaveBeenCalledTimes(1);
+    });
+
+    it('should do nothing when url is empty string', async () => {
+      const result = await client.postForm('', {});
+      expect(result).toBeUndefined();
+      expect(createElement.mock.calls).toHaveLength(0);
+      expect(appendChild.mock.calls).toHaveLength(0);
+      expect(removeChild.mock.calls).toHaveLength(0);
+      expect(authSpy).toHaveBeenCalledTimes(0);
+    });
+  });
 });
diff --git a/superset-frontend/src/components/Chart/chartAction.js b/superset-frontend/src/components/Chart/chartAction.js
@@ -27,7 +27,6 @@ import {
   getExploreUrl,
   getLegacyEndpointType,
   buildV1ChartDataPayload,
-  postForm,
   shouldUseLegacyApi,
   getChartDataUri,
 } from 'src/explore/exploreUtils';
@@ -40,6 +39,7 @@ import { addDangerToast } from 'src/components/MessageToasts/actions';
 import { logEvent } from 'src/logger/actions';
 import { Logger, LOG_ACTIONS_LOAD_CHART } from 'src/logger/LogUtils';
 import { getClientErrorObject } from 'src/utils/getClientErrorObject';
+import { safeStringify } from 'src/utils/safeStringify';
 import { allowCrossDomain as domainShardingEnabled } from 'src/utils/hostNamesConfig';
 import { updateDataMask } from 'src/dataMask/actions';
 import { waitForAsyncData } from 'src/middleware/asyncEvent';
@@ -563,7 +563,9 @@ export function redirectSQLLab(formData) {
           datasourceKey: formData.datasource,
           sql: json.result[0].query,
         };
-        postForm(redirectUrl, payload);
+        SupersetClient.postForm(redirectUrl, {
+          form_data: safeStringify(payload),
+        });
       })
       .catch(() =>
         dispatch(addDangerToast(t('An error occurred while loading the SQL'))),

diff --git a/...set-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx b/...set-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
@@ -21,7 +21,6 @@ import React from 'react';
 import { render, screen, act } from 'spec/helpers/testing-library';
 import userEvent from '@testing-library/user-event';
 import { SupersetClient, DatasourceType } from '@superset-ui/core';
-import * as Utils from 'src/explore/exploreUtils';
 import DatasourceControl from '.';
 
 const SupersetClientGet = jest.spyOn(SupersetClient, 'get');
@@ -142,7 +141,7 @@ test('Click on Edit dataset', async () => {
 
 test('Click on View in SQL Lab', async () => {
   const props = createProps();
-  const postFormSpy = jest.spyOn(Utils, 'postForm');
+  const postFormSpy = jest.spyOn(SupersetClient, 'postForm');
   postFormSpy.mockImplementation(jest.fn());
 
   render(<DatasourceControl {...props} />, {

diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx b/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx
@@ -19,7 +19,13 @@
  */
 import React from 'react';
 import PropTypes from 'prop-types';
-import { t, styled, withTheme, DatasourceType } from '@superset-ui/core';
+import {
+  DatasourceType,
+  SupersetClient,
+  styled,
+  t,
+  withTheme,
+} from '@superset-ui/core';
 import { getUrlParam } from 'src/utils/urlUtils';
 
 import { AntdDropdown } from 'src/components';
@@ -30,13 +36,13 @@ import {
   ChangeDatasourceModal,
   DatasourceModal,
 } from 'src/components/Datasource';
-import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal';
-import { postForm } from 'src/explore/exploreUtils';
 import Button from 'src/components/Button';
 import ErrorAlert from 'src/components/ErrorMessage/ErrorAlert';
 import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip';
 import { URL_PARAMS } from 'src/constants';
 import { isUserAdmin } from 'src/dashboard/util/findPermission';
+import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal';
+import { safeStringify } from 'src/utils/safeStringify';
 
 const propTypes = {
   actions: PropTypes.object.isRequired,
@@ -193,7 +199,9 @@ class DatasourceControl extends React.PureComponent {
             datasourceKey: `${datasource.id}__${datasource.type}`,
             sql: datasource.sql,
           };
-          postForm('/superset/sqllab/', payload);
+          SupersetClient.postForm('/superset/sqllab/', {
+            form_data: safeStringify(payload),
+          });
         }
         break;
 

diff --git a/superset-frontend/src/explore/exploreUtils/exploreUtils.test.jsx b/superset-frontend/src/explore/exploreUtils/exploreUtils.test.jsx
@@ -21,13 +21,14 @@ import sinon from 'sinon';
 import URI from 'urijs';
 import {
   buildV1ChartDataPayload,
+  exploreChart,
   getExploreUrl,
-  shouldUseLegacyApi,
   getSimpleSQLExpression,
+  shouldUseLegacyApi,
 } from 'src/explore/exploreUtils';
 import { DashboardStandaloneMode } from 'src/dashboard/util/constants';
 import * as hostNamesConfig from 'src/utils/hostNamesConfig';
-import { getChartMetadataRegistry } from '@superset-ui/core';
+import { getChartMetadataRegistry, SupersetClient } from '@superset-ui/core';
 
 describe('exploreUtils', () => {
   const { location } = window;
@@ -275,4 +276,16 @@ describe('exploreUtils', () => {
       );
     });
   });
+
+  describe('.exploreChart()', () => {
+    it('postForm', () => {
+      const postFormSpy = jest.spyOn(SupersetClient, 'postForm');
+      postFormSpy.mockImplementation(jest.fn());
+
+      exploreChart({
+        formData: { ...formData, viz_type: 'my_custom_viz' },
+      });
+      expect(postFormSpy).toBeCalledTimes(1);
+    });
+  });
 });
diff --git a/superset-frontend/src/explore/exploreUtils/index.js b/superset-frontend/src/explore/exploreUtils/index.js
@@ -25,6 +25,7 @@ import {
   ensureIsArray,
   getChartBuildQueryRegistry,
   getChartMetadataRegistry,
+  SupersetClient,
 } from '@superset-ui/core';
 import { availableDomains } from 'src/utils/hostNamesConfig';
 import { safeStringify } from 'src/utils/safeStringify';
@@ -234,31 +235,6 @@ export const buildV1ChartDataPayload = ({
 export const getLegacyEndpointType = ({ resultType, resultFormat }) =>
   resultFormat === 'csv' ? resultFormat : resultType;
 
-export function postForm(url, payload, target = '_blank') {
-  if (!url) {
-    return;
-  }
-
-  const hiddenForm = document.createElement('form');
-  hiddenForm.action = url;
-  hiddenForm.method = 'POST';
-  hiddenForm.target = target;
-  const token = document.createElement('input');
-  token.type = 'hidden';
-  token.name = 'csrf_token';
-  token.value = (document.getElementById('csrf_token') || {}).value;
-  hiddenForm.appendChild(token);
-  const data = document.createElement('input');
-  data.type = 'hidden';
-  data.name = 'form_data';
-  data.value = safeStringify(payload);
-  hiddenForm.appendChild(data);
-
-  document.body.appendChild(hiddenForm);
-  hiddenForm.submit();
-  document.body.removeChild(hiddenForm);
-}
-
 export const exportChart = ({
   formData,
   resultFormat = 'json',
@@ -286,7 +262,8 @@ export const exportChart = ({
       ownState,
     });
   }
-  postForm(url, payload);
+
+  SupersetClient.postForm(url, { form_data: safeStringify(payload) });
 };
 
 export const exploreChart = formData => {
@@ -295,7 +272,7 @@ export const exploreChart = formData => {
     endpointType: 'base',
     allowDomainSharding: false,
   });
-  postForm(url, formData);
+  SupersetClient.postForm(url, { form_data: safeStringify(formData) });
 };
 
 export const useDebouncedEffect = (effect, delay, deps) => {

diff --git a/superset/security/manager.py b/superset/security/manager.py
@@ -1383,7 +1383,9 @@ def get_guest_user_from_request(self, req: Request) -> Optional[GuestUser]:
 
         :return: A guest user object
         """
-        raw_token = req.headers.get(current_app.config["GUEST_TOKEN_HEADER_NAME"])
+        raw_token = req.headers.get(
+            current_app.config["GUEST_TOKEN_HEADER_NAME"]
+        ) or req.form.get("guest_token")
         if raw_token is None:
             return None