fix(auth): drain flash messages on the login page

[flcrom]

SUMMARY

Flash messages written by Superset and FAB views were never consumed: no code path called get_flashed_messages(). Messages (including failed-login warnings) accumulated in the signed session cookie on every request, growing it until proxies reject the oversized headers with a 502 (#44541). The login page never displayed them either, working around it with a sessionStorage TODO hack.

• Drain the flash queue in the login view and pass the messages to the SPA as per-request bootstrap data (auth_messages), never via the memoized common bootstrap payload.
• The Login page renders them as toasts (danger, warning, info, success by category) and clears the password field on errors; the sessionStorage workaround is removed.

TESTING INSTRUCTIONS

1. Log out, submit a wrong password repeatedly: before, flash entries accumulate in the session cookie (decode it to see them) and it grows every request; after, each message renders as a toast exactly once and the session stays empty.
2. pytest tests/unit_tests/views/test_auth_login.py
3. cd superset-frontend && npx jest src/pages/Login

ADDITIONAL INFORMATION

• Has associated issue: Fixes Flask flash messages are written but never consumed, growing the session cookie and hiding login error #44541
• Required feature flags:
• Changes UI
• Includes DB Migration

[netlify]

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	a9ab0e2
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6ab4f9b46d383700084acfae
😎 Deploy Preview	https://deploy-preview-44605--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[codeant-ai-for-open-source]

Suggestion: The override no longer delegates POST credentials or provider names to Flask-AppBuilder, so password login and OAuth/SAML buttons only render the login page instead of authenticating.

Assessment: 🔴 Critical · 🔁 Occurrence: Often · 🏷️ Incomplete implementation

Prompt for AI Agent 🤖

This is a comment left during a code review.

**Path:** superset/views/auth.py
**Line:** 47:49
**Comment:**
	*Incomplete Implementation: The override no longer delegates POST credentials or provider names to Flask-AppBuilder, so password login and OAuth/SAML buttons only render the login page instead of authenticating.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

[rusackas]

Codeant's off on this one. This view only renders the SPA shell on GET, the actual login POST and provider routes go through Flask-AppBuilder's own auth blueprint, untouched by this diff.

[bito-code-review]

The flagged issue is incorrect. The code change in superset/views/auth.py correctly overrides the login method to pass get_flashed_messages(with_categories=True) into render_app_template as auth_messages. This ensures that flash messages (including those from failed login attempts) are correctly drained from the session and passed to the frontend for rendering, maintaining compatibility with the existing authentication flow.

No further action is required to resolve this specific issue.

superset/views/auth.py

return super().render_app_template(
            {"auth_messages": get_flashed_messages(with_categories=True)}
        )

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.12%. Comparing base (4c229cb) to head (7f9d8cf).
⚠️ Report is 198 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #44605      +/-   ##
==========================================
+ Coverage   80.81%   81.12%   +0.31%     
==========================================
  Files        2945     2963      +18     
  Lines      177013   179499    +2486     
  Branches    41052    41451     +399     
==========================================
+ Hits       143055   145626    +2571     
+ Misses      31257    31150     -107     
- Partials     2701     2723      +22     

Flag	Coverage Δ
hive	36.70% <100.00%> (-0.24%)	⬇️
javascript	76.17% <100.00%> (+<0.01%)	⬆️
mysql	55.99% <100.00%> (-0.21%)	⬇️
postgres	56.00% <100.00%> (-0.20%)	⬇️
presto	38.62% <100.00%> (-0.26%)	⬇️
python	85.65% <100.00%> (+0.49%)	⬆️
sqlite	55.72% <100.00%> (-0.20%)	⬇️
unit	78.42% <100.00%> (+1.09%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[bito-code-review]

Code Review Agent Run #8570df

Actionable Suggestions - 1

• superset-frontend/src/pages/Login/Login.test.tsx - 1

  • Misleading test name · Line 337-337

Additional Suggestions - 3

• superset-frontend/src/pages/Login/index.tsx - 2

  • Duplicated toast-category mapping · Line 129-137
    This chain re-encodes the flash-category-to-toast-type mapping that `addToast`/`ToastType` in `src/components/MessageToasts/actions.ts` already abstract, and scatters server category strings ('success', 'info', 'message', 'warning') as inline literals. A single `AUTH_CATEGORY_TO_TOAST` lookup dispatching `addToast({ toastType, text })` (default Danger) would centralize the contract and shrink this effect.
  • Orphaned login_attempted cleanup · Line 127-144
    Removing the only writer of `login_attempted` leaves `RightMenu.tsx` `handleLogout` still calling `sessionStorage.removeItem('login_attempted')` and `RightMenu.test.tsx:427-447` still exercising that flow. If stale-tab hygiene is not the intent, drop the orphaned cleanup and test so the retired mechanism does not linger as a maintenance trap.
• tests/unit_tests/views/test_auth_login.py - 1

  • Untyped test fixture params · Line 26-26
    New test functions/fixtures omit parameter type annotations: `app` in `_register_real_templates` and `client` in all three tests are untyped, while return types are annotated. Per repo typing convention, annotate fixture/test parameters (e.g. `app: Flask`, `client: FlaskClient`) for full type coverage.

Review Details

• Files reviewed - 5 · Commit Range: a9ab0e2..a9ab0e2

  • superset-frontend/src/pages/Login/Login.test.tsx
  • superset-frontend/src/pages/Login/index.tsx
  • superset-frontend/src/types/bootstrapTypes.ts
  • superset/views/auth.py
  • tests/unit_tests/views/test_auth_login.py
• Files skipped - 0
• Tools

  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful
  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful
  • Eslint (Linter) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers an incremental AI Review.

• /review full - Manually triggers a full AI Review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[bito-code-review]

Misleading test name

The test name says 'danger toasts', but it seeds a 'warning' flash and asserts mockAddWarningToast was called while mockAddDangerToast was NOT (lines 345-348). The name contradicts the assertions and will mislead maintainers scanning test titles. Rename to reflect the warning-toast behavior.

Code Review Run #8570df

---

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

• Yes, avoid them

[rusackas]

Already renamed a few minutes after this landed (e39309a), matches the assertions now.

[bito-code-review]

The update to the test name is appropriate. Renaming the test to reflect the warning-toast behavior correctly aligns the test title with the assertions, ensuring clarity for future maintenance.

[bito-code-review]

Code Review Agent Run #7acdd7

Actionable Suggestions - 0

Review Details

• Files reviewed - 2 · Commit Range: a9ab0e2..e39309a

  • superset-frontend/src/pages/Login/Login.test.tsx
  • superset/translations/messages.pot
• Files skipped - 0
• Tools

  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers an incremental AI Review.

• /review full - Manually triggers a full AI Review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[Copilot]

Copilot review overview

🟡 Changes recommended

Add regression coverage verifying that the password field clears after authentication errors.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Add regression test verifying password clears after authentication error · New

What changed in this PR

Fixes unconsumed authentication flash messages by draining them into per-request bootstrap data and displaying them as login toasts.

Changes:

• Adds backend flash-message draining and tests.
• Adds categorized login toasts and password clearing.
• Updates bootstrap types and removes obsolete translation/sessionStorage behavior.

File	Summary
tests/​unit_tests/​views/​test_auth_login.py	Tests flash-message draining and session cleanup.
superset/​views/​auth.py	Passes drained messages to login bootstrap data.
superset/​translations/​messages.pot	Removes obsolete login-error translation.
superset-frontend/​src/​types/​bootstrapTypes.ts	Defines auth_messages typing.
superset-frontend/​src/​pages/​Login/​Login.test.tsx	Tests toast behavior.
superset-frontend/​src/​pages/​Login/​index.tsx	Displays categorized auth toasts and clears passwords on errors.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[rusackas]

@flcrom fair point, could you add a case that seeds the password field then asserts it clears on an error-category message?

[rusackas]

Nice fix for #44541, and solid test coverage on both ends. Codeant's auth.py thread looks like a false positive, bito already called that out.

One small ask before this merges: could you add a test asserting the password field actually clears after an auth-failure toast? Copilot's right that only the toast dispatch is covered right now. Otherwise LGTM!

[flcrom]

Added the seeded-password regression case. It keeps the login form mounted, supplies an error-category auth message, and checks that the password clears. The test fails without the clearing logic and passes with it; all 23 Login tests and file lint/format pass locally. Could you take another look?

[bito-code-review]

Code Review Agent Run #06048a

Actionable Suggestions - 0

Additional Suggestions - 2

• superset-frontend/src/pages/Login/Login.test.tsx - 2

  • Test does not exercise clearing · Line 369-394
    This test seeds a password, then `rerender`s with an `error` auth message and expects the field cleared. But `Login`'s effect in `index.tsx` has deps `[dispatch, form]` (mount-only) and `rerender` does not remount `Login`, so the clearing branch never runs on this path — the assertion is not driven by the logic under test. Either remount with the message present or add `authMessages` to the deps.
  • Inconsistent store wrapper · Line 374-391
    This test hand-rolls `` while the other 22 tests in this file use `render(, { useRedux: true })`. The manual store omits the app `reducerIndex` that the helper wires in, duplicating wrapper setup and diverging from the file's convention. Prefer the helper for consistency.

Review Details

• Files reviewed - 1 · Commit Range: e39309a..7f9d8cf

  • superset-frontend/src/pages/Login/Login.test.tsx
• Files skipped - 0
• Tools

  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers an incremental AI Review.

• /review full - Manually triggers a full AI Review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[rusackas]

Thanks for adding that test, @flcrom! LGTM!