refactor: remove obsolete Flask flash messaging system by mistercrunch · Pull Request #35237 · apache/superset

mistercrunch · 2025-09-23T04:05:42Z

Summary

The Flask flash message system is a legacy feature from when Superset was a server-rendered application. In the current SPA architecture, these messages are never displayed to users because:

Flash messages only render on page load via FlashProvider's componentDidMount
Modern UI interactions use async API calls that don't trigger page reloads
The main consumer (/explore/ POST endpoint) is already marked @deprecated

All user-facing notifications are already handled by the frontend toast system, including chart save operations (see saveModalActions.ts) which dispatches success toasts for:

Chart saved/overwritten
Chart added to dashboard
New dashboard created with chart

Changes

Backend

Removed all flash() calls from views (14 occurrences in 4 files)
Converted error flashes to JSON responses or logging
Removed redirect_with_flash utility function
Fixed open redirect vulnerability in dashboard access denial

Frontend

Deleted FlashProvider component and tests
Removed flash_messages from CommonBootstrapData type
Cleaned up context providers and test fixtures
Removed unused getBootstrapData imports

Security Fixes

Fixed open redirect vulnerability by using url_for() instead of request.url
Dashboard access denial now uses safe URL construction

Code Cleanup

Removed unnecessary pass statements and comments
Converted permalink errors to JSON responses for consistency
Verified no tests depend on flash functionality

BREAKING CHANGE

Removes flask.flash() messaging infrastructure. However, no actual functionality is lost as the frontend already handles all notifications through its own toast system.

TESTING INSTRUCTIONS

Save a chart from Explore - verify toast notifications appear
Add chart to dashboard - verify success message
Try accessing a dashboard without permissions - verify proper redirect
Run frontend tests: npm test
Run backend tests: pytest

ADDITIONAL INFORMATION

The application now relies entirely on client-side toast notifications for user feedback, which properly support async operations and provide a consistent UX.

korbit-ai

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Category	Issue	Status
	Redundant string conversion in logging ▹ view	🧠 Not in scope

Files scanned

File Path	Reviewed
superset-frontend/src/components/index.ts	✅
superset-frontend/src/explore/types.ts	✅
superset-frontend/src/constants.ts	✅
superset/views/auth.py	✅
superset-frontend/src/views/RootContextProviders.tsx	✅
superset-frontend/src/types/bootstrapTypes.ts	✅
superset-frontend/src/embedded/EmbeddedContextProviders.tsx	✅
superset-frontend/src/dashboard/actions/hydrate.js	✅
superset/views/utils.py	✅
superset/views/base.py	✅
superset/views/core.py	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

@deprecated

BREAKING CHANGE: Removes flask.flash() messaging infrastructure The Flask flash message system is a legacy feature from when Superset was a server-rendered application. In the current SPA architecture, these messages are never displayed to users because: - Flash messages only render on page load via FlashProvider's componentDidMount - Modern UI interactions use async API calls that don't trigger page reloads - The main consumer (/explore/ POST endpoint) is already marked @deprecated All user-facing notifications are already handled by the frontend toast system, including chart save operations (see saveModalActions.ts) which dispatches success toasts for: - Chart saved/overwritten - Chart added to dashboard - New dashboard created with chart Changes: - Backend: Removed all flash() calls from views (14 occurrences in 4 files) - Backend: Converted error flashes to JSON responses or logging - Backend: Removed redirect_with_flash utility function - Backend: Fixed open redirect vulnerability in dashboard access denial - Frontend: Deleted FlashProvider component and tests - Frontend: Removed flash_messages from CommonBootstrapData type - Frontend: Cleaned up context providers and test fixtures - Frontend: Removed unused getBootstrapData imports Security fixes: - Fixed open redirect vulnerability by using url_for() instead of request.url - Dashboard access denial now uses safe URL construction Code cleanup: - Removed unnecessary pass statements and comments - Converted permalink errors to JSON responses for consistency - Verified no tests depend on flash functionality The application now relies entirely on client-side toast notifications for user feedback, which properly support async operations and provide a consistent UX. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

rusackas · 2025-09-23T17:26:46Z

          superset_can_csv: findPermission('can_csv', 'Superset', roles),
          common: {
            // legacy, please use state.common instead
-            flash_messages: common?.flash_messages,


Question from review session: Where are server-side messages being processed/summoned now that they're not provided as part of the bootstrap data?

Answer is case by case, but short answer is "as they should" from async calls. Checked that most had been re-implemented the right way since they were used. Also confirmed that old mechanics are to surface "on the next page load", which has very few pre-determined triggers, stuff like logging in maybe.

rusackas · 2025-09-23T17:29:49Z

        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)


@mistercrunch this is one of the errors that used to be (rightly) surfaced to users, but now might silent in the UI. These (and other?) server-side messages might no longer appear on the frontend.

Relayed: We just need to make sure we have a frontend counterpart for all server side messages that were deleted.

Let me confirm that logging in is a SPA operation as it should be as opposed to a backend-round-trip.

Login screen is the only place where this seems to actually work (round-trip + timely flash). Let me refactor this nonsense!

Mitigated with a toast, but had to keep the round trip (same as now) at least for now

After removing Flask's flash message system, login errors weren't being displayed. This adds a temporary solution using sessionStorage and toast notifications to show 'Invalid username or password' when login fails. Implementation: - Set a flag in sessionStorage before form submission - On page reload, check if flag exists (indicates failed login) - Show error toast and clear password field for security - Added TODO comment noting this should be replaced with proper SPA auth This addresses reviewer concerns about missing login error messages while keeping the solution simple since it will eventually be replaced with a JSON API approach.

mistercrunch · 2025-09-24T06:57:37Z

                self.pre_delete(item)
            except Exception as ex:  # pylint: disable=broad-except
-                flash(str(ex), "danger")
+                logger.error("Pre-delete error: %s", str(ex))


this probably never should have gone through to user

mistercrunch · 2025-09-24T06:58:19Z

                        _("Error: permalink state not found"), status=404
                    )
            except (ChartNotFoundError, ExplorePermalinkGetFailedError) as ex:
-                flash(__("Error: %(msg)s", msg=ex.message), "danger")


function flagged as @deprecated

mistercrunch · 2025-09-24T06:58:43Z

                initial_form_data["slice_id"] = slice_id
-                if form_data_key:
-                    flash(
-                        _("Form data not found in cache, reverting to chart metadata.")


part of a function flagged as @deprecated

mistercrunch · 2025-09-24T06:58:47Z

-                if form_data_key:
-                    flash(
-                        _(
-                            "Form data not found in cache, reverting to dataset metadata."  # noqa: E501


part of a function flagged as @deprecated

mistercrunch · 2025-09-24T07:04:11Z

                )
-
-            flash(
-                _("Chart [{}] was added to dashboard [{}]").format(


tested that chart adding to dashboard is handled frontend now and flashed properly without these obsolete flash messages

mistercrunch · 2025-09-24T07:04:17Z

            )
-            flash(
-                _(
-                    "Dashboard [{}] just got created and chart [{}] was added to it"


tested that chart adding to dashboard is handled frontend now and flashed properly without these obsolete flash messages

mistercrunch · 2025-09-24T07:22:48Z

    return func(database, user, schema, sql)
-
-
-def redirect_with_flash(url: str, message: str, category: str) -> Response:


not used anymore

mistercrunch · 2025-09-24T07:34:59Z

-            )
+        except SupersetSecurityException:
+            # Return 404 to avoid revealing dashboard existence
+            abort(404)


404 is most accurate when you don't have access

Yep... it's even in the official spec for 403 (I got curious):

An origin server that wishes to "hide" the current existence of a forbidden target resource MAY instead respond with a status code of 404 (Not Found).

rusackas

LGTM!

…rmation leakage - Dashboard access denial now returns 404 instead of redirecting - Prevents attackers from enumerating which dashboards exist - Updated tests to expect 404 instead of 302 for access denial - 18 of 20 RBAC tests now pass (2 have teardown issues) Co-Authored-By: Claude <noreply@anthropic.com>

Using abort(404) inside an exception handler causes issues: - Raises werkzeug.exceptions.NotFound which may not be caught properly - Can leave database transactions in inconsistent state - Causes test teardown failures Solution: Return make_response('Not Found', 404) which returns a proper Flask Response object with 404 status code. All 20 RBAC security tests now pass. Co-Authored-By: Claude <noreply@anthropic.com>

Cleaner solution for returning 404 in the dashboard access denial case: - Response(status=404) returns an empty response with proper status - Simpler than make_response('Not Found', 404) - Lets browser/client handle 404 appropriately - All tests still pass Co-Authored-By: Claude <noreply@anthropic.com>

Co-authored-by: Claude <noreply@anthropic.com>

pull-request-size Bot added the size/L label Sep 23, 2025

github-actions Bot added the api Related to the REST API label Sep 23, 2025

dosubot Bot added change:backend Requires changing the backend change:frontend Requires changing the frontend labels Sep 23, 2025

korbit-ai Bot reviewed Sep 23, 2025

View reviewed changes

Comment thread superset/views/base.py

github-advanced-security AI found potential problems Sep 23, 2025

View reviewed changes

Comment thread superset/views/core.py Fixed

mistercrunch force-pushed the test-env branch from 02df3ec to 3499c7f Compare September 23, 2025 04:25

github-actions Bot added the preset-io label Sep 23, 2025

rusackas reviewed Sep 23, 2025

View reviewed changes

rusackas requested review from michael-s-molina and msyavuz September 23, 2025 17:31

mistercrunch added 2 commits September 23, 2025 18:40

rm PR_BODY

3060ee0

mistercrunch commented Sep 24, 2025

View reviewed changes

404

5df2bb3

mistercrunch commented Sep 24, 2025

View reviewed changes

rusackas approved these changes Sep 24, 2025

View reviewed changes

mistercrunch and others added 3 commits September 24, 2025 17:54

mistercrunch merged commit abc2d46 into master Sep 25, 2025
73 of 74 checks passed

mistercrunch deleted the test-env branch September 25, 2025 07:05

dosubot Bot mentioned this pull request Sep 30, 2025

Redirect flow for not-logged-in users attempting to view a dashboard has broken #35344

Closed

3 tasks

sfirke mentioned this pull request Sep 30, 2025

fix(auth): redirect anonymous attempts to view dashboard with next #35345

Merged

1 task

qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026

refactor: remove obsolete Flask flash messaging system (apache#35237)

cf01010

Co-authored-by: Claude <noreply@anthropic.com>

		return func(database, user, schema, sql)


		def redirect_with_flash(url: str, message: str, category: str) -> Response:

Conversation

mistercrunch commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Backend

Frontend

Security Fixes

Code Cleanup

BREAKING CHANGE

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

korbit-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mistercrunch Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mistercrunch Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rusackas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mistercrunch commented Sep 23, 2025 •

edited

Loading

korbit-ai Bot left a comment •

edited

Loading

mistercrunch Sep 24, 2025 •

edited

Loading

mistercrunch Sep 24, 2025 •

edited

Loading