From f7f1fad12df1a334e2f2aa2f41db7c1fb575115e Mon Sep 17 00:00:00 2001 From: geido Date: Tue, 30 Nov 2021 13:06:05 +0000 Subject: [PATCH 01/17] Redirect on 401 --- .../callApi/callApiAndParseWithTimeout.ts | 14 ++++++++++++-- .../src/connection/callApi/parseResponse.ts | 3 +-- .../superset-ui-core/src/connection/types.ts | 1 + 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts b/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts index 0195b0c60b9f..12fa7b7045cf 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts @@ -20,13 +20,19 @@ import callApi from './callApi'; import rejectAfterTimeout from './rejectAfterTimeout'; import parseResponse from './parseResponse'; -import { CallApi, ClientTimeout, ParseMethod } from '../types'; +import { CallApi, ClientTimeout, ParseMethod, Url } from '../types'; + +function redirectUnauthorized(redirectUrl: Url) { + // the next param will be picked by flask to redirect the user after the login + window.location.href = `${redirectUrl}?next=${window.location.href}`; +} export default async function callApiAndParseWithTimeout< T extends ParseMethod = 'json', >({ timeout, parseMethod, + unauthorizedRedirectUrl = '/login', ...rest }: { timeout?: ClientTimeout; parseMethod?: T } & CallApi) { const apiPromise = callApi(rest); @@ -34,6 +40,10 @@ export default async function callApiAndParseWithTimeout< typeof timeout === 'number' && timeout > 0 ? Promise.race([apiPromise, rejectAfterTimeout(timeout)]) : apiPromise; + const response = await racedPromise; - return parseResponse(racedPromise, parseMethod); + if (response && response.status === 401) { + return redirectUnauthorized(unauthorizedRedirectUrl); + } + return parseResponse(response, parseMethod); } diff --git a/superset-frontend/packages/superset-ui-core/src/connection/callApi/parseResponse.ts b/superset-frontend/packages/superset-ui-core/src/connection/callApi/parseResponse.ts index a0b9f149113a..b7b986dcfe86 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/callApi/parseResponse.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/callApi/parseResponse.ts @@ -20,7 +20,7 @@ import { ParseMethod, TextResponse, JsonResponse } from '../types'; export default async function parseResponse( - apiPromise: Promise, + response: Response, parseMethod?: T, ) { type ReturnType = T extends 'raw' | null @@ -30,7 +30,6 @@ export default async function parseResponse( : T extends 'text' ? TextResponse : never; - const response = await apiPromise; // reject failed HTTP requests with the raw response if (!response.ok) { return Promise.reject(response); 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 3f02f1c61d0c..0e23d46c03ca 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/types.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/types.ts @@ -95,6 +95,7 @@ export interface CallApi extends RequestBase { url: Url; cache?: Cache; redirect?: Redirect; + unauthorizedRedirectUrl?: Url; } export interface RequestWithEndpoint extends RequestBase { From 1f85890d2c9dfac92135e6ce66d96679335c4457 Mon Sep 17 00:00:00 2001 From: geido Date: Thu, 2 Dec 2021 15:38:57 +0000 Subject: [PATCH 02/17] Bump FAB --- requirements/base.txt | 2 +- setup.py | 2 +- superset/config.py | 2 ++ superset/views/base.py | 15 ++++++++++----- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/requirements/base.txt b/requirements/base.txt index 3f55070249a8..5052b3e91288 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -77,7 +77,7 @@ flask==1.1.4 # flask-openid # flask-sqlalchemy # flask-wtf -flask-appbuilder==3.4.0 +flask-appbuilder==3.4.1rc1 # via apache-superset flask-babel==1.0.0 # via flask-appbuilder diff --git a/setup.py b/setup.py index 4f63c284f780..1295cbd3d400 100644 --- a/setup.py +++ b/setup.py @@ -75,7 +75,7 @@ def get_git_sha() -> str: "cryptography>=3.3.2", "deprecation>=2.1.0, <2.2.0", "flask>=1.1.0, <2.0.0", - "flask-appbuilder>=3.4.0, <4.0.0", + "flask-appbuilder>=3.4.1rc1, <4.0.0", "flask-caching>=1.10.0", "flask-compress", "flask-talisman", diff --git a/superset/config.py b/superset/config.py index 2d7e7dbb4696..44cd7967e355 100644 --- a/superset/config.py +++ b/superset/config.py @@ -286,6 +286,8 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: # { 'name': 'Yahoo', 'url': 'https://open.login.yahoo.com/' }, # { 'name': 'Flickr', 'url': 'https://www.flickr.com/' }, +AUTH_STRICT_RESPONSE_CODES = True + # --------------------------------------------------- # Roles config # --------------------------------------------------- diff --git a/superset/views/base.py b/superset/views/base.py index 72dc8053d461..beba4807ca60 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -258,9 +258,11 @@ def validate_sqlatable(table: models.SqlaTable) -> None: def create_table_permissions(table: models.SqlaTable) -> None: - security_manager.add_permission_view_menu("datasource_access", table.get_perm()) + security_manager.add_permission_view_menu( + "datasource_access", table.get_perm()) if table.schema: - security_manager.add_permission_view_menu("schema_access", table.schema_perm) + security_manager.add_permission_view_menu( + "schema_access", table.schema_perm) def get_user_roles() -> List[Role]: @@ -373,7 +375,8 @@ def common_bootstrap_payload() -> Dict[str, Any]: "theme_overrides": conf["THEME_OVERRIDES"], "menu_data": menu_data(), } - bootstrap_data.update(conf["COMMON_BOOTSTRAP_OVERRIDES_FUNC"](bootstrap_data)) + bootstrap_data.update( + conf["COMMON_BOOTSTRAP_OVERRIDES_FUNC"](bootstrap_data)) return bootstrap_data @@ -545,7 +548,8 @@ def yaml_export( data = [t.export_to_dict() for t in items] return Response( - yaml.safe_dump({self.yaml_dict_key: data} if self.yaml_dict_key else data), + yaml.safe_dump({self.yaml_dict_key: data} + if self.yaml_dict_key else data), headers=generate_download_headers("yaml"), mimetype="application/text", ) @@ -612,7 +616,8 @@ class DatasourceFilter(BaseFilter): # pylint: disable=too-few-public-methods def apply(self, query: Query, value: Any) -> Query: if security_manager.can_access_all_datasources(): return query - datasource_perms = security_manager.user_view_menu_names("datasource_access") + datasource_perms = security_manager.user_view_menu_names( + "datasource_access") schema_perms = security_manager.user_view_menu_names("schema_access") return query.filter( or_( From 77ba2616ac907506b3f54cf07bac55858bbdf527 Mon Sep 17 00:00:00 2001 From: geido Date: Thu, 2 Dec 2021 15:44:08 +0000 Subject: [PATCH 03/17] Format --- superset/views/base.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/superset/views/base.py b/superset/views/base.py index beba4807ca60..72dc8053d461 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -258,11 +258,9 @@ def validate_sqlatable(table: models.SqlaTable) -> None: def create_table_permissions(table: models.SqlaTable) -> None: - security_manager.add_permission_view_menu( - "datasource_access", table.get_perm()) + security_manager.add_permission_view_menu("datasource_access", table.get_perm()) if table.schema: - security_manager.add_permission_view_menu( - "schema_access", table.schema_perm) + security_manager.add_permission_view_menu("schema_access", table.schema_perm) def get_user_roles() -> List[Role]: @@ -375,8 +373,7 @@ def common_bootstrap_payload() -> Dict[str, Any]: "theme_overrides": conf["THEME_OVERRIDES"], "menu_data": menu_data(), } - bootstrap_data.update( - conf["COMMON_BOOTSTRAP_OVERRIDES_FUNC"](bootstrap_data)) + bootstrap_data.update(conf["COMMON_BOOTSTRAP_OVERRIDES_FUNC"](bootstrap_data)) return bootstrap_data @@ -548,8 +545,7 @@ def yaml_export( data = [t.export_to_dict() for t in items] return Response( - yaml.safe_dump({self.yaml_dict_key: data} - if self.yaml_dict_key else data), + yaml.safe_dump({self.yaml_dict_key: data} if self.yaml_dict_key else data), headers=generate_download_headers("yaml"), mimetype="application/text", ) @@ -616,8 +612,7 @@ class DatasourceFilter(BaseFilter): # pylint: disable=too-few-public-methods def apply(self, query: Query, value: Any) -> Query: if security_manager.can_access_all_datasources(): return query - datasource_perms = security_manager.user_view_menu_names( - "datasource_access") + datasource_perms = security_manager.user_view_menu_names("datasource_access") schema_perms = security_manager.user_view_menu_names("schema_access") return query.filter( or_( From b33ace1b08467a2654471b263f20d57a3fd7640d Mon Sep 17 00:00:00 2001 From: geido Date: Fri, 3 Dec 2021 15:11:07 +0000 Subject: [PATCH 04/17] Update Cypress save test --- .../cypress/integration/dashboard/save.test.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js b/superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js index 2555730c38bd..14e0ce31462d 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js @@ -71,28 +71,26 @@ describe('Dashboard save action', () => { cy.get('[aria-label="edit-alt"]').click({ timeout: 5000 }); cy.get('[data-test="dashboard-delete-component-button"]') .last() - .trigger('moustenter') + .trigger('mouseenter') .click(); - cy.get('[data-test="grid-container"]') - .find('.box_plot') - .should('not.exist'); + cy.get('[data-test="grid-container"]').find('.treemap').should('not.exist'); - cy.intercept('POST', '/superset/save_dash/**/').as('saveRequest'); + cy.intercept('PUT', '/api/v1/dashboard/**').as('putDashboardRequest'); cy.get('[data-test="dashboard-header"]') .find('[data-test="header-save-button"]') .contains('Save') .click(); // go back to view mode - cy.wait('@saveRequest'); + cy.wait('@putDashboardRequest'); cy.get('[data-test="dashboard-header"]') .find('[aria-label="edit-alt"]') .click(); - // deleted boxplot should still not exist + // deleted treemap should still not exist cy.get('[data-test="grid-container"]') - .find('.box_plot', { timeout: 20000 }) + .find('.treemap', { timeout: 20000 }) .should('not.exist'); }); From 7a484b3077a1cbe0cd748e304f298823436c1d9c Mon Sep 17 00:00:00 2001 From: geido Date: Fri, 3 Dec 2021 15:13:37 +0000 Subject: [PATCH 05/17] Revert Cypress change --- .../cypress/integration/dashboard/save.test.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js b/superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js index 14e0ce31462d..8064f81fa14d 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js @@ -74,7 +74,9 @@ describe('Dashboard save action', () => { .trigger('mouseenter') .click(); - cy.get('[data-test="grid-container"]').find('.treemap').should('not.exist'); + cy.get('[data-test="grid-container"]') + .find('.box_plot') + .should('not.exist'); cy.intercept('PUT', '/api/v1/dashboard/**').as('putDashboardRequest'); cy.get('[data-test="dashboard-header"]') @@ -88,9 +90,9 @@ describe('Dashboard save action', () => { .find('[aria-label="edit-alt"]') .click(); - // deleted treemap should still not exist + // deleted boxplot should still not exist cy.get('[data-test="grid-container"]') - .find('.treemap', { timeout: 20000 }) + .find('.box_plot', { timeout: 20000 }) .should('not.exist'); }); From b9c6d4ac8351b071213948c4efef036b98fe6357 Mon Sep 17 00:00:00 2001 From: geido Date: Mon, 6 Dec 2021 14:21:38 +0000 Subject: [PATCH 06/17] Bump FAB 3.4.1rc2 --- requirements/base.txt | 2 +- setup.py | 5 +++-- .../cypress-base/cypress/integration/dashboard/save.test.js | 6 +++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/requirements/base.txt b/requirements/base.txt index 5052b3e91288..239d53adc43c 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -77,7 +77,7 @@ flask==1.1.4 # flask-openid # flask-sqlalchemy # flask-wtf -flask-appbuilder==3.4.1rc1 +flask-appbuilder==3.4.1rc2 # via apache-superset flask-babel==1.0.0 # via flask-appbuilder diff --git a/setup.py b/setup.py index 1295cbd3d400..5788db80f729 100644 --- a/setup.py +++ b/setup.py @@ -75,7 +75,7 @@ def get_git_sha() -> str: "cryptography>=3.3.2", "deprecation>=2.1.0, <2.2.0", "flask>=1.1.0, <2.0.0", - "flask-appbuilder>=3.4.1rc1, <4.0.0", + "flask-appbuilder>=3.4.1rc2, <4.0.0", "flask-caching>=1.10.0", "flask-compress", "flask-talisman", @@ -109,7 +109,8 @@ def get_git_sha() -> str: "sqlalchemy-utils>=0.37.8, <0.38", "sqlparse==0.3.0", # PINNED! see https://github.com/andialbrecht/sqlparse/issues/562 "tabulate==0.8.9", - "typing-extensions>=3.10, <4", # needed to support Literal (3.8) and TypeGuard (3.10) + # needed to support Literal (3.8) and TypeGuard (3.10) + "typing-extensions>=3.10, <4", "wtforms-json", ], extras_require={ diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js b/superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js index 8064f81fa14d..2555730c38bd 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js @@ -71,21 +71,21 @@ describe('Dashboard save action', () => { cy.get('[aria-label="edit-alt"]').click({ timeout: 5000 }); cy.get('[data-test="dashboard-delete-component-button"]') .last() - .trigger('mouseenter') + .trigger('moustenter') .click(); cy.get('[data-test="grid-container"]') .find('.box_plot') .should('not.exist'); - cy.intercept('PUT', '/api/v1/dashboard/**').as('putDashboardRequest'); + cy.intercept('POST', '/superset/save_dash/**/').as('saveRequest'); cy.get('[data-test="dashboard-header"]') .find('[data-test="header-save-button"]') .contains('Save') .click(); // go back to view mode - cy.wait('@putDashboardRequest'); + cy.wait('@saveRequest'); cy.get('[data-test="dashboard-header"]') .find('[aria-label="edit-alt"]') .click(); From ed06c8230406144160836dfb502d10696d18da4f Mon Sep 17 00:00:00 2001 From: geido Date: Mon, 6 Dec 2021 14:49:54 +0000 Subject: [PATCH 07/17] Update test --- tests/integration_tests/log_api_tests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration_tests/log_api_tests.py b/tests/integration_tests/log_api_tests.py index 5885301e9e76..e5f78c754146 100644 --- a/tests/integration_tests/log_api_tests.py +++ b/tests/integration_tests/log_api_tests.py @@ -102,10 +102,10 @@ def test_get_list_not_allowed(self): self.login(username="gamma") uri = "api/v1/log/" rv = self.client.get(uri) - self.assertEqual(rv.status_code, 401) + self.assertEqual(rv.status_code, 403) self.login(username="alpha") rv = self.client.get(uri) - self.assertEqual(rv.status_code, 401) + self.assertEqual(rv.status_code, 403) def test_get_item(self): """ From b40daaf102da270a54f6d3c1c892b0083fb1c0b8 Mon Sep 17 00:00:00 2001 From: geido Date: Mon, 6 Dec 2021 15:06:55 +0000 Subject: [PATCH 08/17] Update return statement --- .../src/connection/callApi/callApiAndParseWithTimeout.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts b/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts index 12fa7b7045cf..8be4c6cb0b24 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts @@ -43,7 +43,7 @@ export default async function callApiAndParseWithTimeout< const response = await racedPromise; if (response && response.status === 401) { - return redirectUnauthorized(unauthorizedRedirectUrl); + redirectUnauthorized(unauthorizedRedirectUrl); } return parseResponse(response, parseMethod); } From 8425d466e6f8e2e22a51c146003a809c1ec8bf21 Mon Sep 17 00:00:00 2001 From: geido Date: Mon, 6 Dec 2021 15:46:10 +0000 Subject: [PATCH 09/17] Update api test --- tests/integration_tests/databases/api_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration_tests/databases/api_tests.py b/tests/integration_tests/databases/api_tests.py index 9fc6fb99dadc..a0e8bc3fdc11 100644 --- a/tests/integration_tests/databases/api_tests.py +++ b/tests/integration_tests/databases/api_tests.py @@ -1141,7 +1141,7 @@ def test_export_database_not_allowed(self): argument = [database.id] uri = f"api/v1/database/export/?q={prison.dumps(argument)}" rv = self.client.get(uri) - assert rv.status_code == 401 + assert rv.status_code == 403 def test_export_database_non_existing(self): """ From c78ee502a0140aa197fd4eb0cf00e7de6383aa2b Mon Sep 17 00:00:00 2001 From: geido Date: Mon, 6 Dec 2021 16:14:58 +0000 Subject: [PATCH 10/17] Update datasets api test --- tests/integration_tests/datasets/api_tests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py index b219b588a95d..6e324683840b 100644 --- a/tests/integration_tests/datasets/api_tests.py +++ b/tests/integration_tests/datasets/api_tests.py @@ -432,7 +432,7 @@ def test_create_dataset_item_gamma(self): } uri = "api/v1/dataset/" rv = self.client.post(uri, json=table_data) - assert rv.status_code == 401 + assert rv.status_code == 403 def test_create_dataset_item_owner(self): """ @@ -1094,7 +1094,7 @@ def test_delete_dataset_item_not_authorized(self): self.login(username="gamma") uri = f"api/v1/dataset/{dataset.id}" rv = self.client.delete(uri) - assert rv.status_code == 401 + assert rv.status_code == 403 db.session.delete(dataset) db.session.commit() @@ -1313,7 +1313,7 @@ def test_bulk_delete_dataset_item_not_authorized(self): self.login(username="gamma") uri = f"api/v1/dataset/?q={prison.dumps(dataset_ids)}" rv = self.client.delete(uri) - assert rv.status_code == 401 + assert rv.status_code == 403 @pytest.mark.usefixtures("create_datasets") def test_bulk_delete_dataset_item_incorrect(self): From def451d6bc9e4ad39ca6b6e39c7705729b5276b5 Mon Sep 17 00:00:00 2001 From: Geido <60598000+geido@users.noreply.github.com> Date: Mon, 6 Dec 2021 18:33:04 +0200 Subject: [PATCH 11/17] Update datasets api 401s to 403s --- tests/integration_tests/datasets/api_tests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py index 6e324683840b..09984a3dbac8 100644 --- a/tests/integration_tests/datasets/api_tests.py +++ b/tests/integration_tests/datasets/api_tests.py @@ -986,7 +986,7 @@ def test_update_dataset_item_gamma(self): table_data = {"description": "changed_description"} uri = f"api/v1/dataset/{dataset.id}" rv = self.client.put(uri, json=table_data) - assert rv.status_code == 401 + assert rv.status_code == 403 db.session.delete(dataset) db.session.commit() @@ -1438,7 +1438,7 @@ def test_export_dataset_gamma(self): self.login(username="gamma") rv = self.client.get(uri) - assert rv.status_code == 401 + assert rv.status_code == 403 perm1 = security_manager.find_permission_view_menu("can_export", "Dataset") @@ -1516,7 +1516,7 @@ def test_export_dataset_bundle_gamma(self): self.login(username="gamma") rv = self.client.get(uri) # gamma users by default do not have access to this dataset - assert rv.status_code == 401 + assert rv.status_code == 403 @unittest.skip("Number of related objects depend on DB") @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") From 2ce755371a0f1a85f68c1bd8a09d3f28aaf5f74c Mon Sep 17 00:00:00 2001 From: geido Date: Mon, 6 Dec 2021 18:50:45 +0000 Subject: [PATCH 12/17] Add typeguard --- .../connection/callApi/callApiAndParseWithTimeout.ts | 4 +++- .../src/connection/callApi/parseResponse.ts | 10 +++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts b/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts index 8be4c6cb0b24..e7df8e3341fa 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts @@ -41,9 +41,11 @@ export default async function callApiAndParseWithTimeout< ? Promise.race([apiPromise, rejectAfterTimeout(timeout)]) : apiPromise; const response = await racedPromise; + const parsedResponse = parseResponse(response, parseMethod); if (response && response.status === 401) { redirectUnauthorized(unauthorizedRedirectUrl); } - return parseResponse(response, parseMethod); + + return parsedResponse; } diff --git a/superset-frontend/packages/superset-ui-core/src/connection/callApi/parseResponse.ts b/superset-frontend/packages/superset-ui-core/src/connection/callApi/parseResponse.ts index b7b986dcfe86..30a2d47e322b 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/callApi/parseResponse.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/callApi/parseResponse.ts @@ -19,8 +19,14 @@ import { ParseMethod, TextResponse, JsonResponse } from '../types'; +function isPromise( + apiRequest: Response | Promise, +): apiRequest is Promise { + return (apiRequest as Promise).finally !== undefined; +} + export default async function parseResponse( - response: Response, + apiRequest: Response | Promise, parseMethod?: T, ) { type ReturnType = T extends 'raw' | null @@ -30,6 +36,8 @@ export default async function parseResponse( : T extends 'text' ? TextResponse : never; + + const response = isPromise(apiRequest) ? await apiRequest : apiRequest; // reject failed HTTP requests with the raw response if (!response.ok) { return Promise.reject(response); From 1814d1b86afe49022f7d3885a072405373ce567f Mon Sep 17 00:00:00 2001 From: geido Date: Mon, 6 Dec 2021 19:33:46 +0000 Subject: [PATCH 13/17] Use Promise.resolve --- .../src/connection/callApi/parseResponse.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/connection/callApi/parseResponse.ts b/superset-frontend/packages/superset-ui-core/src/connection/callApi/parseResponse.ts index 30a2d47e322b..6f850aa0ce7b 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/callApi/parseResponse.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/callApi/parseResponse.ts @@ -19,12 +19,6 @@ import { ParseMethod, TextResponse, JsonResponse } from '../types'; -function isPromise( - apiRequest: Response | Promise, -): apiRequest is Promise { - return (apiRequest as Promise).finally !== undefined; -} - export default async function parseResponse( apiRequest: Response | Promise, parseMethod?: T, @@ -37,7 +31,7 @@ export default async function parseResponse( ? TextResponse : never; - const response = isPromise(apiRequest) ? await apiRequest : apiRequest; + const response = await Promise.resolve(apiRequest); // reject failed HTTP requests with the raw response if (!response.ok) { return Promise.reject(response); From bcec1428b27e507081768141c32e6ee3115825f1 Mon Sep 17 00:00:00 2001 From: geido Date: Mon, 6 Dec 2021 20:55:12 +0000 Subject: [PATCH 14/17] Update callApiAndParseWithhTimeout test --- .../connection/callApi/callApiAndParseWithTimeout.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApiAndParseWithTimeout.test.ts b/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApiAndParseWithTimeout.test.ts index 29e871286998..722180fc4603 100644 --- a/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApiAndParseWithTimeout.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApiAndParseWithTimeout.test.ts @@ -54,9 +54,13 @@ describe('callApiAndParseWithTimeout()', () => { }); describe('parseResponse', () => { - it('calls parseResponse()', () => { + it('calls parseResponse()', async () => { const parseSpy = jest.spyOn(parseResponse, 'default'); - callApiAndParseWithTimeout({ url: mockGetUrl, method: 'GET' }); + + await callApiAndParseWithTimeout({ + url: mockGetUrl, + method: 'GET', + }); expect(parseSpy).toHaveBeenCalledTimes(1); parseSpy.mockClear(); From dda5524cac5bbae28823e02ad4867ace51c49d5e Mon Sep 17 00:00:00 2001 From: geido Date: Tue, 7 Dec 2021 11:37:57 +0000 Subject: [PATCH 15/17] Disable parseResponse test --- .../test/connection/callApi/callApiAndParseWithTimeout.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApiAndParseWithTimeout.test.ts b/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApiAndParseWithTimeout.test.ts index 722180fc4603..4f80080546c6 100644 --- a/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApiAndParseWithTimeout.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApiAndParseWithTimeout.test.ts @@ -53,7 +53,7 @@ describe('callApiAndParseWithTimeout()', () => { }); }); - describe('parseResponse', () => { + describe.skip('parseResponse', () => { it('calls parseResponse()', async () => { const parseSpy = jest.spyOn(parseResponse, 'default'); From ed51780be06f6565c4f471c4b6558532da3f467e Mon Sep 17 00:00:00 2001 From: geido Date: Tue, 7 Dec 2021 12:17:44 +0000 Subject: [PATCH 16/17] Try catch --- .../callApi/callApiAndParseWithTimeout.ts | 24 ++++++++++--------- .../callApiAndParseWithTimeout.test.ts | 2 +- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts b/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts index e7df8e3341fa..efe6eaa9d258 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts @@ -35,17 +35,19 @@ export default async function callApiAndParseWithTimeout< unauthorizedRedirectUrl = '/login', ...rest }: { timeout?: ClientTimeout; parseMethod?: T } & CallApi) { - const apiPromise = callApi(rest); - const racedPromise = - typeof timeout === 'number' && timeout > 0 - ? Promise.race([apiPromise, rejectAfterTimeout(timeout)]) - : apiPromise; - const response = await racedPromise; - const parsedResponse = parseResponse(response, parseMethod); + try { + const apiPromise = callApi(rest); + const racedPromise = + typeof timeout === 'number' && timeout > 0 + ? Promise.race([apiPromise, rejectAfterTimeout(timeout)]) + : apiPromise; + const response = await racedPromise; - if (response && response.status === 401) { - redirectUnauthorized(unauthorizedRedirectUrl); + if (response && response.status === 401) { + redirectUnauthorized(unauthorizedRedirectUrl); + } + return parseResponse(response, parseMethod); + } catch (e) { + throw new Error(e); } - - return parsedResponse; } diff --git a/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApiAndParseWithTimeout.test.ts b/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApiAndParseWithTimeout.test.ts index 4f80080546c6..722180fc4603 100644 --- a/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApiAndParseWithTimeout.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApiAndParseWithTimeout.test.ts @@ -53,7 +53,7 @@ describe('callApiAndParseWithTimeout()', () => { }); }); - describe.skip('parseResponse', () => { + describe('parseResponse', () => { it('calls parseResponse()', async () => { const parseSpy = jest.spyOn(parseResponse, 'default'); From 717937733e69bb10c34be3b32f5ec0feb3347a83 Mon Sep 17 00:00:00 2001 From: geido Date: Tue, 7 Dec 2021 17:51:23 +0000 Subject: [PATCH 17/17] Handle npm 8 issues --- .../src/connection/SupersetClientClass.ts | 12 ++++++ .../callApi/callApiAndParseWithTimeout.ts | 28 ++++--------- .../src/connection/callApi/parseResponse.ts | 5 +-- .../superset-ui-core/src/connection/types.ts | 1 - .../PropertiesModal/PropertiesModal.test.tsx | 2 +- .../FiltersConfigForm/ColumnSelect.test.tsx | 6 +-- .../ExploreChartHeader.test.tsx | 3 -- .../components/ExploreChartHeader/index.jsx | 40 +++++++++++-------- .../PropertiesModal/PropertiesModal.test.tsx | 29 ++++++-------- 9 files changed, 61 insertions(+), 65 deletions(-) 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 a70c164ce6f8..4959fb1bf2d5 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts @@ -33,6 +33,13 @@ import { } from './types'; import { DEFAULT_FETCH_RETRY_OPTIONS, DEFAULT_BASE_URL } from './constants'; +function redirectUnauthorized() { + // the next param will be picked by flask to redirect the user after the login + setTimeout(() => { + window.location.href = `/login?next=${window.location.href}`; + }); +} + export default class SupersetClientClass { credentials: Credentials; @@ -151,6 +158,11 @@ export default class SupersetClientClass { headers: { ...this.headers, ...headers }, timeout: timeout ?? this.timeout, fetchRetryOptions: fetchRetryOptions ?? this.fetchRetryOptions, + }).catch(res => { + if (res && res.status === 401) { + redirectUnauthorized(); + } + return Promise.reject(res); }); } diff --git a/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts b/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts index efe6eaa9d258..0195b0c60b9f 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApiAndParseWithTimeout.ts @@ -20,34 +20,20 @@ import callApi from './callApi'; import rejectAfterTimeout from './rejectAfterTimeout'; import parseResponse from './parseResponse'; -import { CallApi, ClientTimeout, ParseMethod, Url } from '../types'; - -function redirectUnauthorized(redirectUrl: Url) { - // the next param will be picked by flask to redirect the user after the login - window.location.href = `${redirectUrl}?next=${window.location.href}`; -} +import { CallApi, ClientTimeout, ParseMethod } from '../types'; export default async function callApiAndParseWithTimeout< T extends ParseMethod = 'json', >({ timeout, parseMethod, - unauthorizedRedirectUrl = '/login', ...rest }: { timeout?: ClientTimeout; parseMethod?: T } & CallApi) { - try { - const apiPromise = callApi(rest); - const racedPromise = - typeof timeout === 'number' && timeout > 0 - ? Promise.race([apiPromise, rejectAfterTimeout(timeout)]) - : apiPromise; - const response = await racedPromise; + const apiPromise = callApi(rest); + const racedPromise = + typeof timeout === 'number' && timeout > 0 + ? Promise.race([apiPromise, rejectAfterTimeout(timeout)]) + : apiPromise; - if (response && response.status === 401) { - redirectUnauthorized(unauthorizedRedirectUrl); - } - return parseResponse(response, parseMethod); - } catch (e) { - throw new Error(e); - } + return parseResponse(racedPromise, parseMethod); } diff --git a/superset-frontend/packages/superset-ui-core/src/connection/callApi/parseResponse.ts b/superset-frontend/packages/superset-ui-core/src/connection/callApi/parseResponse.ts index 6f850aa0ce7b..a0b9f149113a 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/callApi/parseResponse.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/callApi/parseResponse.ts @@ -20,7 +20,7 @@ import { ParseMethod, TextResponse, JsonResponse } from '../types'; export default async function parseResponse( - apiRequest: Response | Promise, + apiPromise: Promise, parseMethod?: T, ) { type ReturnType = T extends 'raw' | null @@ -30,8 +30,7 @@ export default async function parseResponse( : T extends 'text' ? TextResponse : never; - - const response = await Promise.resolve(apiRequest); + const response = await apiPromise; // reject failed HTTP requests with the raw response if (!response.ok) { return Promise.reject(response); 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 0e23d46c03ca..3f02f1c61d0c 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/types.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/types.ts @@ -95,7 +95,6 @@ export interface CallApi extends RequestBase { url: Url; cache?: Cache; redirect?: Redirect; - unauthorizedRedirectUrl?: Url; } export interface RequestWithEndpoint extends RequestBase { diff --git a/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx b/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx index ddfe2f22cdf2..3a05398b1bb1 100644 --- a/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx +++ b/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx @@ -85,7 +85,7 @@ fetchMock.get( }, ); -fetchMock.get('http://localhost/api/v1/dashboard/26', { +fetchMock.get('glob:*/api/v1/dashboard/26', { body: { result: { certified_by: 'John Doe', diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ColumnSelect.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ColumnSelect.test.tsx index efb6b9d8e651..424b16835dfb 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ColumnSelect.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ColumnSelect.test.tsx @@ -24,7 +24,7 @@ import { Column, JsonObject } from '@superset-ui/core'; import userEvent from '@testing-library/user-event'; import { ColumnSelect } from './ColumnSelect'; -fetchMock.get('http://localhost/api/v1/dataset/123', { +fetchMock.get('glob:*/api/v1/dataset/123', { body: { result: { columns: [ @@ -35,7 +35,7 @@ fetchMock.get('http://localhost/api/v1/dataset/123', { }, }, }); -fetchMock.get('http://localhost/api/v1/dataset/456', { +fetchMock.get('glob:*/api/v1/dataset/456', { body: { result: { columns: [ @@ -47,7 +47,7 @@ fetchMock.get('http://localhost/api/v1/dataset/456', { }, }); -fetchMock.get('http://localhost/api/v1/dataset/789', { status: 404 }); +fetchMock.get('glob:*/api/v1/dataset/789', { status: 404 }); const createProps = (extraProps: JsonObject = {}) => ({ filterId: 'filterId', diff --git a/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx b/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx index 7c2ac69b1e7c..2033d3368e17 100644 --- a/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx +++ b/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx @@ -21,11 +21,8 @@ import React from 'react'; import { Slice } from 'src/types/Chart'; import { render, screen } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; -import fetchMock from 'fetch-mock'; import ExploreHeader from '.'; -fetchMock.get('http://localhost/api/v1/chart/318', {}); - const createProps = () => ({ chart: { latestQueryFormData: { diff --git a/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx b/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx index 7a53faa125a4..587bfe215038 100644 --- a/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx +++ b/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx @@ -143,26 +143,32 @@ export class ExploreChartHeader extends React.PureComponent { async fetchChartDashboardData() { const { dashboardId, slice } = this.props; - const response = await SupersetClient.get({ + await SupersetClient.get({ endpoint: `/api/v1/chart/${slice.slice_id}`, - }); - const chart = response.json.result; - const dashboards = chart.dashboards || []; - const dashboard = - dashboardId && - dashboards.length && - dashboards.find(d => d.id === dashboardId); + }) + .then(res => { + const response = res?.json?.result; + if (response && response.dashboards && response.dashboards.length) { + const { dashboards } = response; + const dashboard = + dashboardId && + dashboards.length && + dashboards.find(d => d.id === dashboardId); - if (dashboard && dashboard.json_metadata) { - // setting the chart to use the dashboard custom label colors if any - const labelColors = - JSON.parse(dashboard.json_metadata).label_colors || {}; - const categoricalNamespace = CategoricalColorNamespace.getNamespace(); + if (dashboard && dashboard.json_metadata) { + // setting the chart to use the dashboard custom label colors if any + const labelColors = + JSON.parse(dashboard.json_metadata).label_colors || {}; + const categoricalNamespace = + CategoricalColorNamespace.getNamespace(); - Object.keys(labelColors).forEach(label => { - categoricalNamespace.setColor(label, labelColors[label]); - }); - } + Object.keys(labelColors).forEach(label => { + categoricalNamespace.setColor(label, labelColors[label]); + }); + } + } + }) + .catch(() => {}); } getSliceName() { diff --git a/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx b/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx index d7376801d5b7..8dba7fbb40c1 100644 --- a/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx +++ b/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx @@ -70,7 +70,7 @@ const createProps = () => ({ onSave: jest.fn(), }); -fetchMock.get('http://localhost/api/v1/chart/318', { +fetchMock.get('glob:*/api/v1/chart/318', { body: { description_columns: {}, id: 318, @@ -128,23 +128,20 @@ fetchMock.get('http://localhost/api/v1/chart/318', { }, }); -fetchMock.get( - 'http://localhost/api/v1/chart/related/owners?q=(filter:%27%27)', - { - body: { - count: 1, - result: [ - { - text: 'Superset Admin', - value: 1, - }, - ], - }, - sendAsJson: true, +fetchMock.get('glob:*/api/v1/chart/related/owners?q=(filter:%27%27)', { + body: { + count: 1, + result: [ + { + text: 'Superset Admin', + value: 1, + }, + ], }, -); + sendAsJson: true, +}); -fetchMock.put('http://localhost/api/v1/chart/318', { +fetchMock.put('glob:*/api/v1/chart/318', { body: { id: 318, result: {