You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2022/06/23 17:52:32 UTC
[superset] 09/27: fix(embedded): CSV download for chart (#20261)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 2.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit a08499d88d9fce1954cbf65db0c370f8e0c579e0
Author: Lily Kuang <li...@preset.io>
AuthorDate: Fri Jun 17 20:01:08 2022 -0700
fix(embedded): CSV download for chart (#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 <18...@users.noreply.github.com>
* 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 <18...@users.noreply.github.com>
(cherry picked from commit ab9f72f1a1359a59e64afd9e820d5823fd53b77b)
---
.../src/connection/SupersetClient.ts | 1 +
.../src/connection/SupersetClientClass.ts | 30 ++++++
.../superset-ui-core/src/connection/types.ts | 1 +
.../test/connection/SupersetClient.test.ts | 6 +-
.../test/connection/SupersetClientClass.test.ts | 103 +++++++++++++++++++++
.../src/components/Chart/chartAction.js | 6 +-
.../DatasourceControl/DatasourceControl.test.tsx | 3 +-
.../controls/DatasourceControl/index.jsx | 16 +++-
.../src/explore/exploreUtils/exploreUtils.test.jsx | 17 +++-
.../src/explore/exploreUtils/index.js | 31 +------
superset/security/manager.py | 4 +-
tests/integration_tests/security_tests.py | 12 +++
12 files changed, 190 insertions(+), 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
index a5ab65d097..530c710809 100644
--- 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
index 7a6dfd97b0..b7281d0259 100644
--- 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
index 0ab382917e..0602595675 100644
--- 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
index 227d59b172..17a07f3c72 100644
--- 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
index ef31e5d35d..4db26b05b4 100644
--- 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
index d52ac79177..b9aeffec4a 100644
--- 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/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
index dd43ed1a04..8a65a1139d 100644
--- a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
+++ b/superset-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
index 15d07fd6dd..615c61c2bd 100644
--- 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
index f52b575e2d..b385099699 100644
--- 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
index 127766e7f3..0e8be9feaf 100644
--- 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
index b231b93b48..6157959aa3 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -1383,7 +1383,9 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
: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
diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py
index e66bf02e82..a70146db68 100644
--- a/tests/integration_tests/security_tests.py
+++ b/tests/integration_tests/security_tests.py
@@ -1064,6 +1064,7 @@ class TestDatasources(SupersetTestCase):
class FakeRequest:
headers: Any = {}
+ form: Any = {}
class TestGuestTokens(SupersetTestCase):
@@ -1111,6 +1112,17 @@ class TestGuestTokens(SupersetTestCase):
self.assertIsNotNone(guest_user)
self.assertEqual("test_guest", guest_user.username)
+ def test_get_guest_user_with_request_form(self):
+ token = self.create_guest_token()
+ fake_request = FakeRequest()
+ fake_request.headers[current_app.config["GUEST_TOKEN_HEADER_NAME"]] = None
+ fake_request.form["guest_token"] = token
+
+ guest_user = security_manager.get_guest_user_from_request(fake_request)
+
+ self.assertIsNotNone(guest_user)
+ self.assertEqual("test_guest", guest_user.username)
+
@patch("superset.security.SupersetSecurityManager._get_current_epoch_time")
def test_get_guest_user_expired_token(self, get_time_mock):
# make a just-expired token