This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 5.0
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/5.0 by this push:
new 05e70cdfa2 fix(explore): Display missing dataset for denied access
(#34129)
05e70cdfa2 is described below
commit 05e70cdfa21e625f75a885ff2575a27744e7be0b
Author: JUST.in DO IT <[email protected]>
AuthorDate: Wed Jul 16 13:36:03 2025 -0700
fix(explore): Display missing dataset for denied access (#34129)
---
.../superset-ui-chart-controls/src/types.ts | 1 +
.../DatasourceControl/DatasourceControl.test.tsx | 27 +++++
.../controls/DatasourceControl/index.jsx | 86 ++++++++++------
.../DndColumnSelectControl/DndFilterSelect.tsx | 5 +-
superset-frontend/src/explore/fixtures.tsx | 4 +-
superset-frontend/src/explore/types.ts | 2 +-
superset-frontend/src/pages/Chart/Chart.test.tsx | 90 ++++++++++++++++-
superset-frontend/src/pages/Chart/index.tsx | 112 ++++++++++++++-------
superset/explore/api.py | 6 ++
superset/security/manager.py | 1 +
10 files changed, 263 insertions(+), 71 deletions(-)
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 62ac944688..f8058a3da8 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<string, unknown>;
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 e84fb4500e..d4676edb38 100644
---
a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
+++
b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
@@ -474,3 +474,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(<DatasourceControl {...props} />, { 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 4e5063b1e4..3b562a4ec6 100644
---
a/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx
+++
b/superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx
@@ -47,6 +47,7 @@ import {
isUserAdmin,
} from 'src/dashboard/util/permissionUtils';
import ModalTrigger from 'src/components/ModalTrigger';
+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';
@@ -275,7 +276,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);
@@ -380,20 +391,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;
@@ -439,23 +440,44 @@ class DatasourceControl extends PureComponent {
)}
{isMissingDatasource && !isMissingParams && (
<div className="error-alert">
- <ErrorAlert
- level="warning"
- errorType={t('Missing dataset')}
- description={
- <>
- {t('The dataset linked to this chart may have been
deleted.')}
- <Button
- buttonStyle="primary"
- onClick={() =>
- this.handleMenuItemClick({ key: CHANGE_DATASET })
- }
- >
- {t('Swap dataset')}
- </Button>
- </>
- }
- />
+ {extra?.error ? (
+ <div className="error-alert">
+ <ErrorMessageWithStackTrace
+ title={extra.error.statusText || extra.error.message}
+ subtitle={
+ extra.error.statusText ? extra.error.message : undefined
+ }
+ error={extra.error}
+ source="explore"
+ />
+ </div>
+ ) : (
+ <ErrorAlert
+ type="warning"
+ errorType={t('Missing dataset')}
+ descriptionPre={false}
+ descriptionDetailsCollapsed={false}
+ descriptionDetails={
+ <>
+ <p>
+ {t(
+ 'The dataset linked to this chart may have been
deleted.',
+ )}
+ </p>
+ <p>
+ <Button
+ buttonStyle="warning"
+ onClick={() =>
+ this.handleMenuItemClick({ key: CHANGE_DATASET })
+ }
+ >
+ {t('Swap dataset')}
+ </Button>
+ </p>
+ </>
+ }
+ />
+ )}
</div>
)}
{showEditDatasourceModal && (
diff --git
a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx
b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx
index d15289db8c..d52beba64b 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 eb1d0d6d66..015e4470ea 100644
--- a/superset-frontend/src/explore/fixtures.tsx
+++ b/superset-frontend/src/explore/fixtures.tsx
@@ -158,8 +158,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 708ffa9dfa..f045c8e295 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 2fe7599211..d127b86e9b 100644
--- a/superset-frontend/src/pages/Chart/Chart.test.tsx
+++ b/superset-frontend/src/pages/Chart/Chart.test.tsx
@@ -30,7 +30,7 @@ import { LocalStorageKeys } from
'src/utils/localStorageHelpers';
import getFormDataWithExtraFilters from
'src/dashboard/util/charts/getFormDataWithExtraFilters';
import { URL_PARAMS } from 'src/constants';
import { JsonObject, VizType } from '@superset-ui/core';
-
+import { getParsedExploreURLParams } from
'src/explore/exploreUtils/getParsedExploreURLParams';
import ChartPage from '.';
jest.mock('re-resizable', () => ({
@@ -46,6 +46,9 @@ jest.mock(
),
);
jest.mock('src/dashboard/util/charts/getFormDataWithExtraFilters');
+jest.mock('src/explore/exploreUtils/getParsedExploreURLParams', () => ({
+ getParsedExploreURLParams: jest.fn(),
+}));
describe('ChartPage', () => {
afterEach(() => {
@@ -75,6 +78,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(<ChartPage />, {
+ 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(<ChartPage />, {
+ 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 5bbeb5fa86..3ae68d7502 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) => {
@@ -161,9 +152,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<void, { result: Chart }>({
+ 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 7e7813beca..e16b083feb 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 a48aadb5fe..40a44161ab 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -632,6 +632,7 @@ class SupersetSecurityManager( # pylint:
disable=too-many-public-methods
extra={
"link": self.get_datasource_access_link(datasource),
"datasource": datasource.id,
+ "datasource_name": datasource.name,
},
)
