diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts index 453e58d9437..af7125a91af 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts @@ -90,6 +90,7 @@ export interface Dataset { database?: Record; normalize_columns?: boolean; always_filter_main_dttm?: boolean; + extra?: object | string; } export interface ControlPanelState { 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 46d2eb4ba8a..91938f2ea55 100644 --- a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx +++ b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx @@ -479,3 +479,30 @@ test('should show missing dataset state', () => { ), ).toBeVisible(); }); + +test('should show forbidden dataset state', () => { + // @ts-ignore + delete window.location; + // @ts-ignore + window.location = { search: '?slice_id=152' }; + const error = { + error_type: 'TABLE_SECURITY_ACCESS_ERROR', + statusText: 'FORBIDDEN', + message: 'You do not have access to the following tables: blocked_table', + extra: { + datasource: 152, + datasource_name: 'forbidden dataset', + }, + }; + const props = createProps({ + datasource: { + ...fallbackExploreInitialData.dataset, + extra: { + error, + }, + }, + }); + render(, { useRedux: true, useRouter: true }); + expect(screen.getByText(error.message)).toBeInTheDocument(); + expect(screen.getByText(error.statusText)).toBeVisible(); +}); diff --git a/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx b/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx index 574484e0dc2..cbd4705d65d 100644 --- a/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx +++ b/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx @@ -50,6 +50,7 @@ import { userHasPermission, isUserAdmin, } from 'src/dashboard/util/permissionUtils'; +import { ErrorMessageWithStackTrace } from 'src/components/ErrorMessage/ErrorMessageWithStackTrace'; import ViewQueryModalFooter from 'src/explore/components/controls/ViewQueryModalFooter'; import ViewQuery from 'src/explore/components/controls/ViewQuery'; import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal'; @@ -281,7 +282,17 @@ class DatasourceControl extends PureComponent { showSaveDatasetModal, } = this.state; const { datasource, onChange, theme } = this.props; - const isMissingDatasource = !datasource?.id; + let extra; + if (datasource?.extra) { + if (typeof datasource.extra === 'string') { + try { + extra = JSON.parse(datasource.extra); + } catch {} // eslint-disable-line no-empty + } else { + extra = datasource.extra; // eslint-disable-line prefer-destructuring + } + } + const isMissingDatasource = !datasource?.id || Boolean(extra?.error); let isMissingParams = false; if (isMissingDatasource) { const datasourceId = getUrlParam(URL_PARAMS.datasourceId); @@ -386,20 +397,10 @@ class DatasourceControl extends PureComponent { const { health_check_message: healthCheckMessage } = datasource; - let extra; - if (datasource?.extra) { - if (typeof datasource.extra === 'string') { - try { - extra = JSON.parse(datasource.extra); - } catch {} // eslint-disable-line no-empty - } else { - extra = datasource.extra; // eslint-disable-line prefer-destructuring - } - } - - const titleText = isMissingDatasource - ? t('Missing dataset') - : getDatasourceTitle(datasource); + const titleText = + isMissingDatasource && !datasource.name + ? t('Missing dataset') + : getDatasourceTitle(datasource); const tooltip = titleText; @@ -452,31 +453,44 @@ class DatasourceControl extends PureComponent { )} {isMissingDatasource && !isMissingParams && (
- -

- {t( - 'The dataset linked to this chart may have been deleted.', - )} -

-

- -

- - } - /> + {extra?.error ? ( +
+ +
+ ) : ( + +

+ {t( + 'The dataset linked to this chart may have been deleted.', + )} +

+

+ +

+ + } + /> + )}
)} {showEditDatasourceModal && ( diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx index 4e7df9a3640..de5fff8eab6 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx @@ -90,7 +90,10 @@ const DndFilterSelect = (props: DndFilterSelectProps) => { let extra = {}; if (datasource?.extra) { try { - extra = JSON.parse(datasource.extra); + extra = + typeof datasource.extra === 'string' + ? JSON.parse(datasource.extra) + : datasource.extra; } catch {} // eslint-disable-line no-empty } return extra; diff --git a/superset-frontend/src/explore/fixtures.tsx b/superset-frontend/src/explore/fixtures.tsx index 1e4afe9b613..815790885da 100644 --- a/superset-frontend/src/explore/fixtures.tsx +++ b/superset-frontend/src/explore/fixtures.tsx @@ -159,8 +159,8 @@ export const fallbackExploreInitialData: ExplorePageInitialData = { verbose_map: {}, main_dttm_col: '', owners: [], - datasource_name: 'missing_datasource', - name: 'missing_datasource', + datasource_name: '', + name: '', description: null, }, slice: null, diff --git a/superset-frontend/src/explore/types.ts b/superset-frontend/src/explore/types.ts index 708ffa9dfa5..f045c8e2951 100644 --- a/superset-frontend/src/explore/types.ts +++ b/superset-frontend/src/explore/types.ts @@ -69,7 +69,7 @@ export type Datasource = Dataset & { catalog?: string | null; schema?: string; is_sqllab_view?: boolean; - extra?: string; + extra?: string | object; }; export interface ExplorePageInitialData { diff --git a/superset-frontend/src/pages/Chart/Chart.test.tsx b/superset-frontend/src/pages/Chart/Chart.test.tsx index 6b0e9f820a3..918199d5156 100644 --- a/superset-frontend/src/pages/Chart/Chart.test.tsx +++ b/superset-frontend/src/pages/Chart/Chart.test.tsx @@ -31,6 +31,7 @@ import getFormDataWithExtraFilters from 'src/dashboard/util/charts/getFormDataWi import { URL_PARAMS } from 'src/constants'; import { JsonObject, VizType } from '@superset-ui/core'; import { useUnsavedChangesPrompt } from 'src/hooks/useUnsavedChangesPrompt'; +import { getParsedExploreURLParams } from 'src/explore/exploreUtils/getParsedExploreURLParams'; import ChartPage from '.'; jest.mock('src/hooks/useUnsavedChangesPrompt', () => ({ @@ -49,6 +50,9 @@ jest.mock( ), ); jest.mock('src/dashboard/util/charts/getFormDataWithExtraFilters'); +jest.mock('src/explore/exploreUtils/getParsedExploreURLParams', () => ({ + getParsedExploreURLParams: jest.fn(), +})); describe('ChartPage', () => { beforeEach(() => { @@ -89,6 +93,91 @@ describe('ChartPage', () => { ); }); + test('displays the dataset name and error when it is prohibited', async () => { + const chartApiRoute = `glob:*/api/v1/chart/*`; + const exploreApiRoute = 'glob:*/api/v1/explore/*'; + const expectedDatasourceName = 'failed datasource name'; + (getParsedExploreURLParams as jest.Mock).mockReturnValue( + new Map([['datasource_id', 1]]), + ); + fetchMock.get(exploreApiRoute, () => { + class Extra { + datasource = 123; + + datasource_name = expectedDatasourceName; + } + class SupersetSecurityError { + message = 'You do not have a permission to the table'; + + extra = new Extra(); + } + throw new SupersetSecurityError(); + }); + fetchMock.get(chartApiRoute, 200); + const { getByTestId } = render(, { + useRouter: true, + useRedux: true, + useDnd: true, + }); + await waitFor( + () => + expect(getByTestId('mock-explore-chart-panel')).toHaveTextContent( + JSON.stringify({ datasource_name: expectedDatasourceName }).slice( + 1, + -1, + ), + ), + { + timeout: 5000, + }, + ); + expect(fetchMock.calls(chartApiRoute).length).toEqual(0); + expect(fetchMock.calls(exploreApiRoute).length).toBeGreaterThanOrEqual(1); + }); + + test('fetches the chart api when explore metadata is prohibited and access from the chart link', async () => { + const expectedChartId = 7; + const expectedChartName = 'Unauthorized dataset owned chart name'; + (getParsedExploreURLParams as jest.Mock).mockReturnValue( + new Map([['slice_id', expectedChartId]]), + ); + const chartApiRoute = `glob:*/api/v1/chart/${expectedChartId}`; + const exploreApiRoute = 'glob:*/api/v1/explore/*'; + + fetchMock.get(exploreApiRoute, () => { + class Extra { + datasource = 123; + } + class SupersetSecurityError { + message = 'You do not have a permission to the table'; + + extra = new Extra(); + } + throw new SupersetSecurityError(); + }); + fetchMock.get(chartApiRoute, { + result: { + id: expectedChartId, + slice_name: expectedChartName, + url: 'chartid', + }, + }); + const { getByTestId, getByText } = render(, { + useRouter: true, + useRedux: true, + useDnd: true, + }); + await waitFor(() => expect(fetchMock.calls(chartApiRoute).length).toBe(1), { + timeout: 5000, + }); + expect(fetchMock.calls(exploreApiRoute).length).toBeGreaterThanOrEqual(1); + expect(getByTestId('mock-explore-chart-panel')).toBeInTheDocument(); + expect(getByTestId('mock-explore-chart-panel')).toHaveTextContent( + JSON.stringify({ datasource: 123 }).slice(1, -1), + ); + expect(getByText(expectedChartName)).toBeInTheDocument(); + }); + describe('with dashboardContextFormData', () => { const dashboardPageId = 'mockPageId'; diff --git a/superset-frontend/src/pages/Chart/index.tsx b/superset-frontend/src/pages/Chart/index.tsx index 61edb9bfe56..4de03f7be73 100644 --- a/superset-frontend/src/pages/Chart/index.tsx +++ b/superset-frontend/src/pages/Chart/index.tsx @@ -41,6 +41,7 @@ import { ExploreResponsePayload, SaveActionType } from 'src/explore/types'; import { fallbackExploreInitialData } from 'src/explore/fixtures'; import { getItem, LocalStorageKeys } from 'src/utils/localStorageHelpers'; import { getFormDataWithDashboardContext } from 'src/explore/controlUtils/getFormDataWithDashboardContext'; +import type Chart from 'src/types/Chart'; const isValidResult = (rv: JsonObject): boolean => rv?.result?.form_data && rv?.result?.dataset; @@ -49,41 +50,31 @@ const hasDatasetId = (rv: JsonObject): boolean => isDefined(rv?.result?.dataset?.id); const fetchExploreData = async (exploreUrlParams: URLSearchParams) => { - try { - const rv = await makeApi<{}, ExploreResponsePayload>({ - method: 'GET', - endpoint: 'api/v1/explore/', - })(exploreUrlParams); - if (isValidResult(rv)) { - if (hasDatasetId(rv)) { - return rv; - } - // Since there's no dataset id but the API responded with a valid payload, - // we assume the dataset was deleted, so we preserve some values from previous - // state so if the user decide to swap the datasource, the chart config remains - fallbackExploreInitialData.form_data = { - ...rv.result.form_data, - ...fallbackExploreInitialData.form_data, - }; - if (rv.result?.slice) { - fallbackExploreInitialData.slice = rv.result.slice; - } + const rv = await makeApi<{}, ExploreResponsePayload>({ + method: 'GET', + endpoint: 'api/v1/explore/', + })(exploreUrlParams); + if (isValidResult(rv)) { + if (hasDatasetId(rv)) { + return rv; } - let message = t('Failed to load chart data'); - const responseError = rv?.result?.message; - if (responseError) { - message = `${message}:\n${responseError}`; + // Since there's no dataset id but the API responded with a valid payload, + // we assume the dataset was deleted, so we preserve some values from previous + // state so if the user decide to swap the datasource, the chart config remains + fallbackExploreInitialData.form_data = { + ...rv.result.form_data, + ...fallbackExploreInitialData.form_data, + }; + if (rv.result?.slice) { + fallbackExploreInitialData.slice = rv.result.slice; } - throw new Error(message); - } catch (err) { - // todo: encapsulate the error handler - const clientError = await getClientErrorObject(err); - throw new Error( - clientError.message || - clientError.error || - t('Failed to load chart data.'), - ); } + let message = t('Failed to load chart data'); + const responseError = rv?.result?.message; + if (responseError) { + message = `${message}:\n${responseError}`; + } + throw new Error(message); }; const getDashboardPageContext = (pageId?: string | null) => { @@ -166,9 +157,62 @@ export default function ExplorePage() { }), ); }) - .catch(err => { + .catch(err => Promise.all([getClientErrorObject(err), err])) + .then(resolved => { + const [clientError, err] = resolved || []; + if (!err) { + return Promise.resolve(); + } + const errorMesage = + clientError?.message || + clientError?.error || + t('Failed to load chart data.'); + dispatch(addDangerToast(errorMesage)); + + if (err.extra?.datasource) { + const exploreData = { + ...fallbackExploreInitialData, + dataset: { + ...fallbackExploreInitialData.dataset, + id: err.extra?.datasource, + name: err.extra?.datasource_name, + extra: { + error: err, + }, + }, + }; + const chartId = exploreUrlParams.get('slice_id'); + return ( + chartId + ? makeApi({ + method: 'GET', + endpoint: `api/v1/chart/${chartId}`, + })() + : Promise.reject() + ) + .then( + ({ result: { id, url, owners, form_data: _, ...data } }) => { + const slice = { + ...data, + datasource: err.extra?.datasource_name, + slice_id: id, + slice_url: url, + owners: owners?.map(({ id }) => id), + }; + dispatch( + hydrateExplore({ + ...exploreData, + slice, + }), + ); + }, + ) + .catch(() => { + dispatch(hydrateExplore(exploreData)); + }); + } dispatch(hydrateExplore(fallbackExploreInitialData)); - dispatch(addDangerToast(err.message)); + return Promise.resolve(); }) .finally(() => { setIsLoaded(true); diff --git a/superset/explore/api.py b/superset/explore/api.py index 7e7813beca6..e16b083feb8 100644 --- a/superset/explore/api.py +++ b/superset/explore/api.py @@ -27,6 +27,7 @@ from superset.commands.temporary_cache.exceptions import ( TemporaryCacheResourceNotFoundError, ) from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP +from superset.exceptions import SupersetSecurityException from superset.explore.exceptions import DatasetAccessDeniedError, WrongEndpointError from superset.explore.permalink.exceptions import ExplorePermalinkGetFailedError from superset.explore.schemas import ExploreContextSchema @@ -118,6 +119,11 @@ class ExploreRestApi(BaseSupersetApi): return self.response(200, result=result) except ValueError as ex: return self.response(400, message=str(ex)) + except SupersetSecurityException as ex: + return self.response( + 403, + **ex.to_dict(), + ) except DatasetAccessDeniedError as ex: return self.response( 403, diff --git a/superset/security/manager.py b/superset/security/manager.py index 0db217b30dd..884152b195b 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -670,6 +670,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods extra={ "link": self.get_datasource_access_link(datasource), "datasource": datasource.id, + "datasource_name": datasource.name, }, )