This is an automated email from the ASF dual-hosted git repository.
maximebeauchemin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new abc2d46fed refactor: remove obsolete Flask flash messaging system
(#35237)
abc2d46fed is described below
commit abc2d46fedf9ac0978d075e80941d3745b417470
Author: Maxime Beauchemin <[email protected]>
AuthorDate: Thu Sep 25 00:05:16 2025 -0700
refactor: remove obsolete Flask flash messaging system (#35237)
Co-authored-by: Claude <[email protected]>
---
.../spec/fixtures/mockDashboardInfo.js | 1 -
.../FlashProvider/FlashProvider.test.tsx | 65 ----------------------
.../src/components/FlashProvider/index.tsx | 51 -----------------
.../src/components/FlashProvider/types.ts | 20 -------
superset-frontend/src/components/index.ts | 1 -
superset-frontend/src/constants.ts | 1 -
superset-frontend/src/dashboard/actions/hydrate.js | 1 -
.../src/embedded/EmbeddedContextProviders.tsx | 38 ++++++-------
superset-frontend/src/explore/types.ts | 1 -
superset-frontend/src/pages/Login/Login.test.tsx | 6 +-
superset-frontend/src/pages/Login/index.tsx | 28 ++++++++--
.../src/theme/tests/ThemeController.test.ts | 2 -
superset-frontend/src/types/bootstrapTypes.ts | 2 -
.../src/views/RootContextProviders.tsx | 42 +++++++-------
superset/views/auth.py | 7 +--
superset/views/base.py | 12 +---
superset/views/core.py | 61 +++-----------------
superset/views/utils.py | 8 +--
.../dashboards/security/security_rbac_tests.py | 12 ++--
19 files changed, 85 insertions(+), 274 deletions(-)
diff --git a/superset-frontend/spec/fixtures/mockDashboardInfo.js
b/superset-frontend/spec/fixtures/mockDashboardInfo.js
index a046a554d0..31328e18ab 100644
--- a/superset-frontend/spec/fixtures/mockDashboardInfo.js
+++ b/superset-frontend/spec/fixtures/mockDashboardInfo.js
@@ -49,7 +49,6 @@ export default {
dash_edit_perm: true,
dash_save_perm: true,
common: {
- flash_messages: [],
conf: { SUPERSET_WEBSERVER_TIMEOUT: 60 },
},
filterBarOrientation: FilterBarOrientation.Vertical,
diff --git
a/superset-frontend/src/components/FlashProvider/FlashProvider.test.tsx
b/superset-frontend/src/components/FlashProvider/FlashProvider.test.tsx
deleted file mode 100644
index 6b93e4009d..0000000000
--- a/superset-frontend/src/components/FlashProvider/FlashProvider.test.tsx
+++ /dev/null
@@ -1,65 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-import { render, screen } from 'spec/helpers/testing-library';
-import { Provider } from 'react-redux';
-import { store } from 'src/views/store';
-import type { FlashMessage } from './types';
-import { FlashProvider } from '.';
-
-test('Rerendering correctly with default props', () => {
- const messages: FlashMessage[] = [];
- render(
- <FlashProvider messages={messages}>
- <div data-test="my-component">My Component</div>
- </FlashProvider>,
- { store },
- );
- expect(screen.getByTestId('my-component')).toBeInTheDocument();
-});
-
-test('messages should only be inserted in the State when the component is
mounted', () => {
- const messages: FlashMessage[] = [
- ['info', 'teste message 01'],
- ['info', 'teste message 02'],
- ];
- expect(store.getState().messageToasts).toEqual([]);
- const { rerender } = render(
- <Provider store={store}>
- <FlashProvider messages={messages}>
- <div data-teste="my-component">My Component</div>
- </FlashProvider>
- </Provider>,
- );
- const fistRender = store.getState().messageToasts;
- expect(fistRender).toHaveLength(2);
- expect(fistRender[1].text).toBe(messages[0][1]);
- expect(fistRender[0].text).toBe(messages[1][1]);
-
- rerender(
- <Provider store={store}>
- <FlashProvider messages={[...messages, ['info', 'teste message 03']]}>
- <div data-teste="my-component">My Component</div>
- </FlashProvider>
- </Provider>,
- );
-
- const secondRender = store.getState().messageToasts;
- expect(secondRender).toEqual(fistRender);
-});
diff --git a/superset-frontend/src/components/FlashProvider/index.tsx
b/superset-frontend/src/components/FlashProvider/index.tsx
deleted file mode 100644
index d6c07d4652..0000000000
--- a/superset-frontend/src/components/FlashProvider/index.tsx
+++ /dev/null
@@ -1,51 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-import { useToasts } from 'src/components/MessageToasts/withToasts';
-import { useComponentDidMount } from '@superset-ui/core';
-import type { FlashMessage } from './types';
-
-interface Props {
- children: JSX.Element;
- messages: FlashMessage[];
-}
-
-const flashObj = {
- info: 'addInfoToast',
- alert: 'addDangerToast',
- danger: 'addDangerToast',
- warning: 'addWarningToast',
- success: 'addSuccessToast',
-};
-
-export function FlashProvider({ children, messages }: Props) {
- const toasts = useToasts();
- useComponentDidMount(() => {
- messages.forEach(message => {
- const [type, text] = message;
- const flash = flashObj[type];
- const toast = toasts[flash as keyof typeof toasts];
- if (toast) {
- toast(text);
- }
- });
- });
- return children;
-}
-
-export type { FlashMessage };
diff --git a/superset-frontend/src/components/FlashProvider/types.ts
b/superset-frontend/src/components/FlashProvider/types.ts
deleted file mode 100644
index 8716eb985b..0000000000
--- a/superset-frontend/src/components/FlashProvider/types.ts
+++ /dev/null
@@ -1,20 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-type FlashMessageType = 'info' | 'alert' | 'danger' | 'warning' | 'success';
-export type FlashMessage = [FlashMessageType, string];
diff --git a/superset-frontend/src/components/index.ts
b/superset-frontend/src/components/index.ts
index 09d50bb315..558f080261 100644
--- a/superset-frontend/src/components/index.ts
+++ b/superset-frontend/src/components/index.ts
@@ -38,7 +38,6 @@ export * from './ErrorMessage';
export { ImportModal, type ImportModelsModalProps } from './ImportModal';
export { ErrorBoundary, type ErrorBoundaryProps } from './ErrorBoundary';
export * from './GenericLink';
-export { FlashProvider, type FlashMessage } from './FlashProvider';
export { GridTable, type TableProps } from './GridTable';
export * from './Tag';
export * from './TagsList';
diff --git a/superset-frontend/src/constants.ts
b/superset-frontend/src/constants.ts
index 4ea7c276d9..faf3012906 100644
--- a/superset-frontend/src/constants.ts
+++ b/superset-frontend/src/constants.ts
@@ -122,7 +122,6 @@ export const RESERVED_DASHBOARD_URL_PARAMS: string[] = [
export const DEFAULT_COMMON_BOOTSTRAP_DATA: CommonBootstrapData = {
application_root: '/',
static_assets_prefix: '',
- flash_messages: [],
conf: {},
locale: 'en',
feature_flags: {},
diff --git a/superset-frontend/src/dashboard/actions/hydrate.js
b/superset-frontend/src/dashboard/actions/hydrate.js
index 99a74ddbd1..fe41d8aded 100644
--- a/superset-frontend/src/dashboard/actions/hydrate.js
+++ b/superset-frontend/src/dashboard/actions/hydrate.js
@@ -274,7 +274,6 @@ export const hydrateDashboard =
superset_can_csv: findPermission('can_csv', 'Superset', roles),
common: {
// legacy, please use state.common instead
- flash_messages: common?.flash_messages,
conf: common?.conf,
},
filterBarOrientation:
diff --git a/superset-frontend/src/embedded/EmbeddedContextProviders.tsx
b/superset-frontend/src/embedded/EmbeddedContextProviders.tsx
index 207a1d320d..34d1070e86 100644
--- a/superset-frontend/src/embedded/EmbeddedContextProviders.tsx
+++ b/superset-frontend/src/embedded/EmbeddedContextProviders.tsx
@@ -22,13 +22,12 @@ import { Provider as ReduxProvider } from 'react-redux';
import { QueryParamProvider } from 'use-query-params';
import { DndProvider } from 'react-dnd';
import { HTML5Backend } from 'react-dnd-html5-backend';
-import { FlashProvider, DynamicPluginProvider } from 'src/components';
+import { DynamicPluginProvider } from 'src/components';
import { EmbeddedUiConfigProvider } from 'src/components/UiConfigContext';
import { SupersetThemeProvider } from 'src/theme/ThemeProvider';
import { ThemeController } from 'src/theme/ThemeController';
import type { ThemeStorage } from '@superset-ui/core';
import { store } from 'src/views/store';
-import getBootstrapData from 'src/utils/getBootstrapData';
/**
* In-memory implementation of ThemeStorage interface for embedded contexts.
@@ -56,7 +55,6 @@ const themeController = new ThemeController({
export const getThemeController = (): ThemeController => themeController;
-const { common } = getBootstrapData();
const extensionsRegistry = getExtensionsRegistry();
export const EmbeddedContextProviders: React.FC = ({ children }) => {
@@ -68,24 +66,22 @@ export const EmbeddedContextProviders: React.FC = ({
children }) => {
<SupersetThemeProvider themeController={themeController}>
<ReduxProvider store={store}>
<DndProvider backend={HTML5Backend}>
- <FlashProvider messages={common.flash_messages}>
- <EmbeddedUiConfigProvider>
- <DynamicPluginProvider>
- <QueryParamProvider
- ReactRouterRoute={Route}
- stringifyOptions={{ encode: false }}
- >
- {RootContextProviderExtension ? (
- <RootContextProviderExtension>
- {children}
- </RootContextProviderExtension>
- ) : (
- children
- )}
- </QueryParamProvider>
- </DynamicPluginProvider>
- </EmbeddedUiConfigProvider>
- </FlashProvider>
+ <EmbeddedUiConfigProvider>
+ <DynamicPluginProvider>
+ <QueryParamProvider
+ ReactRouterRoute={Route}
+ stringifyOptions={{ encode: false }}
+ >
+ {RootContextProviderExtension ? (
+ <RootContextProviderExtension>
+ {children}
+ </RootContextProviderExtension>
+ ) : (
+ children
+ )}
+ </QueryParamProvider>
+ </DynamicPluginProvider>
+ </EmbeddedUiConfigProvider>
</DndProvider>
</ReduxProvider>
</SupersetThemeProvider>
diff --git a/superset-frontend/src/explore/types.ts
b/superset-frontend/src/explore/types.ts
index f5a04a3f1d..fb2ce66596 100644
--- a/superset-frontend/src/explore/types.ts
+++ b/superset-frontend/src/explore/types.ts
@@ -98,7 +98,6 @@ export interface ExploreResponsePayload {
export interface ExplorePageState {
user: UserWithPermissionsAndRoles;
common: {
- flash_messages: string[];
conf: JsonObject;
locale: string;
};
diff --git a/superset-frontend/src/pages/Login/Login.test.tsx
b/superset-frontend/src/pages/Login/Login.test.tsx
index efe51be80c..6673dfdd99 100644
--- a/superset-frontend/src/pages/Login/Login.test.tsx
+++ b/superset-frontend/src/pages/Login/Login.test.tsx
@@ -33,7 +33,7 @@ jest.mock('src/utils/getBootstrapData', () => ({
}));
test('should render login form elements', () => {
- render(<Login />);
+ render(<Login />, { useRedux: true });
expect(screen.getByTestId('login-form')).toBeInTheDocument();
expect(screen.getByTestId('username-input')).toBeInTheDocument();
expect(screen.getByTestId('password-input')).toBeInTheDocument();
@@ -42,13 +42,13 @@ test('should render login form elements', () => {
});
test('should render username and password labels', () => {
- render(<Login />);
+ render(<Login />, { useRedux: true });
expect(screen.getByText('Username:')).toBeInTheDocument();
expect(screen.getByText('Password:')).toBeInTheDocument();
});
test('should render form instruction text', () => {
- render(<Login />);
+ render(<Login />, { useRedux: true });
expect(
screen.getByText('Enter your login and password below:'),
).toBeInTheDocument();
diff --git a/superset-frontend/src/pages/Login/index.tsx
b/superset-frontend/src/pages/Login/index.tsx
index 8c56a4c8c9..9bb10dfd1d 100644
--- a/superset-frontend/src/pages/Login/index.tsx
+++ b/superset-frontend/src/pages/Login/index.tsx
@@ -27,8 +27,10 @@ import {
Typography,
Icons,
} from '@superset-ui/core/components';
-import { useState } from 'react';
+import { useState, useEffect } from 'react';
import { capitalize } from 'lodash/fp';
+import { addDangerToast } from 'src/components/MessageToasts/actions';
+import { useDispatch } from 'react-redux';
import getBootstrapData from 'src/utils/getBootstrapData';
type OAuthProvider = {
@@ -77,6 +79,7 @@ const StyledLabel = styled(Typography.Text)`
export default function Login() {
const [form] = Form.useForm<LoginForm>();
const [loading, setLoading] = useState(false);
+ const dispatch = useDispatch();
const bootstrapData = getBootstrapData();
@@ -85,11 +88,28 @@ export default function Login() {
const authRegistration: boolean =
bootstrapData.common.conf.AUTH_USER_REGISTRATION;
+ // TODO: This is a temporary solution for showing login errors after form
submission.
+ // Should be replaced with proper SPA-style authentication (JSON API with
error responses)
+ // when Flask-AppBuilder is updated or we implement a custom login endpoint.
+ useEffect(() => {
+ const loginAttempted = sessionStorage.getItem('login_attempted');
+
+ if (loginAttempted === 'true') {
+ sessionStorage.removeItem('login_attempted');
+ dispatch(addDangerToast(t('Invalid username or password')));
+ // Clear password field for security
+ form.setFieldsValue({ password: '' });
+ }
+ }, [dispatch, form]);
+
const onFinish = (values: LoginForm) => {
setLoading(true);
- SupersetClient.postForm('/login/', values, '').finally(() => {
- setLoading(false);
- });
+
+ // Mark that we're attempting login (for error detection after redirect)
+ sessionStorage.setItem('login_attempted', 'true');
+
+ // Use standard form submission for Flask-AppBuilder compatibility
+ SupersetClient.postForm('/login/', values, '');
};
const getAuthIconElement = (
diff --git a/superset-frontend/src/theme/tests/ThemeController.test.ts
b/superset-frontend/src/theme/tests/ThemeController.test.ts
index e0ed88aa52..c7276132f3 100644
--- a/superset-frontend/src/theme/tests/ThemeController.test.ts
+++ b/superset-frontend/src/theme/tests/ThemeController.test.ts
@@ -83,7 +83,6 @@ const createMockBootstrapData = (
common: {
application_root: '/',
static_assets_prefix: '/static/assets/',
- flash_messages: [],
conf: {},
locale: 'en',
feature_flags: {},
@@ -391,7 +390,6 @@ describe('ThemeController', () => {
common: {
application_root: '/',
static_assets_prefix: '/static/assets/',
- flash_messages: [],
conf: {},
locale: 'en',
feature_flags: {},
diff --git a/superset-frontend/src/types/bootstrapTypes.ts
b/superset-frontend/src/types/bootstrapTypes.ts
index abe32d6583..1d8357a56e 100644
--- a/superset-frontend/src/types/bootstrapTypes.ts
+++ b/superset-frontend/src/types/bootstrapTypes.ts
@@ -20,7 +20,6 @@ import { FormatLocaleDefinition } from 'd3-format';
import { TimeLocaleDefinition } from 'd3-time-format';
import { isPlainObject } from 'lodash';
import { Languages } from 'src/features/home/LanguagePicker';
-import type { FlashMessage } from 'src/components';
import type {
AnyThemeConfig,
ColorSchemeConfig,
@@ -154,7 +153,6 @@ export interface BootstrapThemeDataConfig {
export interface CommonBootstrapData {
application_root: string;
static_assets_prefix: string;
- flash_messages: FlashMessage[];
conf: JsonObject;
locale: Locale;
feature_flags: FeatureFlagMap;
diff --git a/superset-frontend/src/views/RootContextProviders.tsx
b/superset-frontend/src/views/RootContextProviders.tsx
index fbff76b64c..c10f04ae81 100644
--- a/superset-frontend/src/views/RootContextProviders.tsx
+++ b/superset-frontend/src/views/RootContextProviders.tsx
@@ -23,8 +23,7 @@ import { Provider as ReduxProvider } from 'react-redux';
import { QueryParamProvider } from 'use-query-params';
import { DndProvider } from 'react-dnd';
import { HTML5Backend } from 'react-dnd-html5-backend';
-import getBootstrapData from 'src/utils/getBootstrapData';
-import { FlashProvider, DynamicPluginProvider } from 'src/components';
+import { DynamicPluginProvider } from 'src/components';
import { EmbeddedUiConfigProvider } from 'src/components/UiConfigContext';
import { SupersetThemeProvider } from 'src/theme/ThemeProvider';
import { ThemeController } from 'src/theme/ThemeController';
@@ -32,7 +31,6 @@ import { ExtensionsProvider } from
'src/extensions/ExtensionsContext';
import { store } from './store';
import '../preamble';
-const { common } = getBootstrapData();
const themeController = new ThemeController();
const extensionsRegistry = getExtensionsRegistry();
@@ -45,26 +43,24 @@ export const RootContextProviders: React.FC = ({ children
}) => {
<SupersetThemeProvider themeController={themeController}>
<ReduxProvider store={store}>
<DndProvider backend={HTML5Backend}>
- <FlashProvider messages={common.flash_messages}>
- <EmbeddedUiConfigProvider>
- <DynamicPluginProvider>
- <QueryParamProvider
- ReactRouterRoute={Route}
- stringifyOptions={{ encode: false }}
- >
- <ExtensionsProvider>
- {RootContextProviderExtension ? (
- <RootContextProviderExtension>
- {children}
- </RootContextProviderExtension>
- ) : (
- children
- )}
- </ExtensionsProvider>
- </QueryParamProvider>
- </DynamicPluginProvider>
- </EmbeddedUiConfigProvider>
- </FlashProvider>
+ <EmbeddedUiConfigProvider>
+ <DynamicPluginProvider>
+ <QueryParamProvider
+ ReactRouterRoute={Route}
+ stringifyOptions={{ encode: false }}
+ >
+ <ExtensionsProvider>
+ {RootContextProviderExtension ? (
+ <RootContextProviderExtension>
+ {children}
+ </RootContextProviderExtension>
+ ) : (
+ children
+ )}
+ </ExtensionsProvider>
+ </QueryParamProvider>
+ </DynamicPluginProvider>
+ </EmbeddedUiConfigProvider>
</DndProvider>
</ReduxProvider>
</SupersetThemeProvider>
diff --git a/superset/views/auth.py b/superset/views/auth.py
index c24e0ae362..123333a639 100644
--- a/superset/views/auth.py
+++ b/superset/views/auth.py
@@ -18,9 +18,8 @@
import logging
from typing import Optional
-from flask import flash, g, redirect
+from flask import g, redirect
from flask_appbuilder import expose
-from flask_appbuilder._compat import as_unicode
from flask_appbuilder.const import LOGMSG_ERR_SEC_NO_REGISTER_HASH
from flask_appbuilder.security.decorators import no_cache
from flask_appbuilder.security.views import AuthView, WerkzeugResponse
@@ -66,7 +65,7 @@ class SupersetRegisterUserView(BaseSupersetView):
reg = self.appbuilder.sm.find_register_user(activation_hash)
if not reg:
logger.error(LOGMSG_ERR_SEC_NO_REGISTER_HASH, activation_hash)
- flash(as_unicode(self.false_error_message), "danger")
+ logger.error("Registration activation failed: %s",
self.false_error_message)
return redirect(self.appbuilder.get_url_for_index)
if not self.appbuilder.sm.add_user(
username=reg.username,
@@ -78,7 +77,7 @@ class SupersetRegisterUserView(BaseSupersetView):
),
hashed_password=reg.password,
):
- flash(as_unicode(self.error_message), "danger")
+ logger.error("User registration failed: %s", self.error_message)
return redirect(self.appbuilder.get_url_for_index)
else:
self.appbuilder.sm.del_register_user(reg)
diff --git a/superset/views/base.py b/superset/views/base.py
index b07d63f5cb..b0ad2c2e64 100644
--- a/superset/views/base.py
+++ b/superset/views/base.py
@@ -27,9 +27,7 @@ from babel import Locale
from flask import (
abort,
current_app as app,
- flash,
g,
- get_flashed_messages,
redirect,
Response,
session,
@@ -499,10 +497,7 @@ def cached_common_bootstrap_data( # pylint:
disable=unused-argument
def common_bootstrap_payload() -> dict[str, Any]:
- return {
- **cached_common_bootstrap_data(utils.get_user_id(), get_locale()),
- "flash_messages": get_flashed_messages(with_categories=True),
- }
+ return cached_common_bootstrap_data(utils.get_user_id(), get_locale())
def get_spa_payload(extra_data: dict[str, Any] | None = None) -> dict[str,
Any]:
@@ -597,7 +592,7 @@ class DeleteMixin: # pylint: disable=too-few-public-methods
try:
self.pre_delete(item)
except Exception as ex: # pylint: disable=broad-except
- flash(str(ex), "danger")
+ logger.error("Pre-delete error: %s", str(ex))
else:
view_menu = security_manager.find_view_menu(item.get_perm())
pvs = (
@@ -617,7 +612,6 @@ class DeleteMixin: # pylint: disable=too-few-public-methods
db.session.commit() # pylint:
disable=consider-using-transaction
- flash(*self.datamodel.message)
self.update_redirect()
@action(
@@ -630,7 +624,7 @@ class DeleteMixin: # pylint: disable=too-few-public-methods
try:
self.pre_delete(item)
except Exception as ex: # pylint: disable=broad-except
- flash(str(ex), "danger")
+ logger.error("Pre-delete error: %s", str(ex))
else:
self._delete(item.id)
self.update_redirect()
diff --git a/superset/views/core.py b/superset/views/core.py
index d52a01f356..140c0775aa 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -28,7 +28,6 @@ from urllib import parse
from flask import (
abort,
current_app as app,
- flash,
g,
redirect,
request,
@@ -109,7 +108,6 @@ from superset.views.utils import (
get_form_data,
get_viz,
loads_request_json,
- redirect_with_flash,
sanitize_datasource_data,
)
from superset.viz import BaseViz
@@ -415,7 +413,6 @@ class Superset(BaseSupersetView):
initial_form_data = {}
- form_data_key = request.args.get("form_data_key")
if key is not None:
command = GetExplorePermalinkCommand(key)
try:
@@ -430,9 +427,10 @@ class Superset(BaseSupersetView):
_("Error: permalink state not found"), status=404
)
except (ChartNotFoundError, ExplorePermalinkGetFailedError) as ex:
- flash(__("Error: %(msg)s", msg=ex.message), "danger")
- return redirect(url_for("SliceModelView.list"))
- elif form_data_key:
+ return json_error_response(
+ __("Error: %(msg)s", msg=ex.message), status=404
+ )
+ elif form_data_key := request.args.get("form_data_key"):
parameters = CommandParameters(key=form_data_key)
value = GetFormDataCommand(parameters).run()
initial_form_data = json.loads(value) if value else {}
@@ -442,18 +440,8 @@ class Superset(BaseSupersetView):
dataset_id = request.args.get("dataset_id")
if slice_id:
initial_form_data["slice_id"] = slice_id
- if form_data_key:
- flash(
- _("Form data not found in cache, reverting to chart
metadata.")
- )
elif dataset_id:
initial_form_data["datasource"] = f"{dataset_id}__table"
- if form_data_key:
- flash(
- _(
- "Form data not found in cache, reverting to
dataset metadata." # noqa: E501
- )
- )
form_data, slc = get_form_data(
use_slice_data=True, initial_form_data=initial_form_data
@@ -626,13 +614,9 @@ class Superset(BaseSupersetView):
if action == "saveas" and slice_add_perm:
ChartDAO.create(slc)
db.session.commit() # pylint: disable=consider-using-transaction
- msg = _("Chart [{}] has been saved").format(slc.slice_name)
- flash(msg, "success")
elif action == "overwrite" and slice_overwrite_perm:
ChartDAO.update(slc)
db.session.commit() # pylint: disable=consider-using-transaction
- msg = _("Chart [{}] has been overwritten").format(slc.slice_name)
- flash(msg, "success")
# Adding slice to a dashboard if requested
dash: Dashboard | None = None
@@ -654,13 +638,6 @@ class Superset(BaseSupersetView):
_("You don't have the rights to alter this dashboard"),
status=403,
)
-
- flash(
- _("Chart [{}] was added to dashboard [{}]").format(
- slc.slice_name, dash.dashboard_title
- ),
- "success",
- )
elif new_dashboard_name:
# Creating and adding to a new dashboard
# check create dashboard permissions
@@ -675,12 +652,6 @@ class Superset(BaseSupersetView):
dashboard_title=request.args.get("new_dashboard_name"),
owners=[g.user] if g.user else [],
)
- flash(
- _(
- "Dashboard [{}] just got created and chart [{}] was added
to it"
- ).format(dash.dashboard_title, slc.slice_name),
- "success",
- )
if dash and slc not in dash.slices:
dash.slices.append(slc)
@@ -798,19 +769,9 @@ class Superset(BaseSupersetView):
try:
dashboard.raise_for_access()
- except SupersetSecurityException as ex:
- # anonymous users should get the login screen, others should go to
dashboard list # noqa: E501
- if g.user is None or g.user.is_anonymous:
- redirect_url =
f"{appbuilder.get_url_for_login}?next={request.url}"
- warn_msg = "Users must be logged in to view this dashboard."
- else:
- redirect_url = url_for("DashboardModelView.list")
- warn_msg = utils.error_msg_from_exception(ex)
- return redirect_with_flash(
- url=redirect_url,
- message=warn_msg,
- category="danger",
- )
+ except SupersetSecurityException:
+ # Return 404 to avoid revealing dashboard existence
+ return Response(status=404)
add_extra_log_payload(
dashboard_id=dashboard.id,
dashboard_version="v2",
@@ -841,12 +802,8 @@ class Superset(BaseSupersetView):
) -> FlaskResponse:
try:
value = GetDashboardPermalinkCommand(key).run()
- except DashboardPermalinkGetFailedError as ex:
- flash(__("Error: %(msg)s", msg=ex.message), "danger")
- return redirect(url_for("DashboardModelView.list"))
- except DashboardAccessDeniedError as ex:
- flash(__("Error: %(msg)s", msg=ex.message), "danger")
- return redirect(url_for("DashboardModelView.list"))
+ except (DashboardPermalinkGetFailedError, DashboardAccessDeniedError)
as ex:
+ return json_error_response(__("Error: %(msg)s", msg=ex.message),
status=404)
if not value:
return json_error_response(_("permalink state not found"),
status=404)
diff --git a/superset/views/utils.py b/superset/views/utils.py
index a431c307fb..cbe5ef0dcf 100644
--- a/superset/views/utils.py
+++ b/superset/views/utils.py
@@ -22,12 +22,11 @@ from typing import Any, Callable, DefaultDict, Optional,
Union
import msgpack
import pyarrow as pa
-from flask import current_app as app, flash, g, has_request_context, redirect,
request
+from flask import current_app as app, g, has_request_context, request
from flask_appbuilder.security.sqla import models as ab_models
from flask_appbuilder.security.sqla.models import User
from flask_babel import _
from sqlalchemy.exc import NoResultFound
-from werkzeug.wrappers.response import Response
from superset import dataframe, db, result_set, viz
from superset.common.db_query_status import QueryStatus
@@ -551,8 +550,3 @@ def get_cta_schema_name(
if not func:
return None
return func(database, user, schema, sql)
-
-
-def redirect_with_flash(url: str, message: str, category: str) -> Response:
- flash(message=message, category=category)
- return redirect(url)
diff --git a/tests/integration_tests/dashboards/security/security_rbac_tests.py
b/tests/integration_tests/dashboards/security/security_rbac_tests.py
index 4754854047..c6ea811636 100644
--- a/tests/integration_tests/dashboards/security/security_rbac_tests.py
+++ b/tests/integration_tests/dashboards/security/security_rbac_tests.py
@@ -108,7 +108,7 @@ class
TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity):
# act
response = self.get_dashboard_view_response(dashboard_to_access)
- assert response.status_code == 302
+ assert response.status_code == 404
request_payload = get_query_context("birth_names")
rv = self.post_assert_metric(CHART_DATA_URI, request_payload, "data")
@@ -129,7 +129,7 @@ class
TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity):
response = self.get_dashboard_view_response(dashboard_to_access)
# assert
- assert response.status_code == 302
+ assert response.status_code == 404
# post
revoke_access_to_dashboard(dashboard_to_access, new_role) # noqa: F405
@@ -147,9 +147,9 @@ class
TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity):
dashboard = create_dashboard_to_db(published=True, slices=[slice])
self.login(GAMMA_USERNAME)
- # assert redirect on regular rbac access denied
+ # assert 404 on regular rbac access denied (prevents information
leakage)
response = self.get_dashboard_view_response(dashboard)
- assert response.status_code == 302
+ assert response.status_code == 404
request_payload = get_query_context("birth_names")
rv = self.post_assert_metric(CHART_DATA_URI, request_payload, "data")
@@ -221,7 +221,7 @@ class
TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity):
response = self.get_dashboard_view_response(dashboard_to_access)
# assert
- assert response.status_code == 302
+ assert response.status_code == 404
@pytest.mark.usefixtures("public_role_like_gamma")
def
test_get_dashboard_view__public_user_with_dashboard_permission_can_not_access_draft(
# noqa: E501
@@ -234,7 +234,7 @@ class
TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity):
response = self.get_dashboard_view_response(dashboard_to_access)
# assert
- assert response.status_code == 302
+ assert response.status_code == 404
# post
revoke_access_to_dashboard(dashboard_to_access, "Public") # noqa: F405
