fix(auth): remove the legacy FAB password reset views and move password resets into the SPA

[rusackas]

SUMMARY

Fixes #37700. Supersedes #37773 by @jayvenn21, whose add_view_no_menu registration intercept, launcher hiding and tests this builds on (credited as co-author on the commit).

The Flask-AppBuilder server-rendered password reset pages (/resetpassword/form, /resetmypassword/form) survived the SPA migration and were still reachable in 6.0. #37773 gated them behind a new ENABLE_LEGACY_FAB_PASSWORD_VIEWS flag. Looking at what people actually asked for, it was the SPA inputs, not a switch to keep the old pages around, so this drops the flag idea and removes the views outright, and makes sure the SPA covers every job they had:

• Admin resets live in the Users list edit modal, which gains optional "New password" / "Confirm new password" fields wired to the existing PUT /api/v1/security/users/<id>. That endpoint is can_put on User (admin) and never required a current password, which is the right rule for an admin resetting someone else's account. The self-service PUT /api/v1/me/ requires current_password whenever the account already has a stored password; an account without one (external auth, first-time setup) sets its first password without it.
• Self-service stays on the profile page's "Reset my password" modal, which now sends current_password when filled in (the field is optional in the modal, the API decides whether it is needed). It didn't send it before, so since fix(security): harden account password-change and session-invalidation handling #42934 made the field mandatory the SPA modal was actually broken against its own API.
• Forced password change (ENABLE_FORCE_PASSWORD_CHANGE) used to redirect to ResetMyPasswordView. It now lands on /user_info/, with the profile page and only the endpoints it calls (GET/PUT /api/v1/me/ and the CSRF token) exempt from the hook so the page is usable; the rest of those APIs (guest_token included) stays behind the gate, so a flagged user can still only reach that page and log out. A successful self-service change through /api/v1/me/ now clears password_must_change (previously only the FAB view did), in the same unit of work as the new hash.

Backend: the two views are never registered (FAB adds them unconditionally for AUTH_DB, and a blueprint can't be removed once added, so register_views() intercepts add_view_no_menu for the duration of FAB's registration), the "Reset Password" / "Reset my password" launchers on the FAB user pages are hidden and answer 404 instead of BuildError, and sync_role_definitions stops assigning the stale ResetPasswordView / ResetMyPasswordView / launcher permissions that upgraded installs still carry in their metadata DB. ENABLE_LEGACY_FAB_PASSWORD_VIEWS does not exist.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before: /resetpassword/form?pk=<id> and /resetmypassword/form render the FAB forms; the Users list edit modal has no password fields; the profile "Reset my password" modal fails with "This field is required to change the password."

After: both routes 404. The Users list edit modal shows "New password" and "Confirm new password" under Groups (blank keeps the current password, mismatch is rejected client-side). The profile "Reset my password" modal shows "Current password" (optional, needed only when the account already has one), "New password" and "Confirm Password".

TESTING INSTRUCTIONS

1. As Admin, open Settings > List Users, edit a user, set a new password in the two new fields and save; log in as that user with the new password. Save the same modal with the fields blank and confirm the password is unchanged.
2. As any user, open the profile page (/user_info/), "Reset my password", enter the current and a new password; confirm the new one works and the old one doesn't.
3. Set ENABLE_FORCE_PASSWORD_CHANGE = True, flag a user (set_password_must_change(user_id)), log in as them: every page redirects to /user_info/, the reset modal there works, and after changing the password the redirect stops. Logging out is possible throughout.
4. /resetpassword/form?pk=1 and /resetmypassword/form return 404; superset init leaves no role with ResetPasswordView, ResetMyPasswordView, resetpasswords or resetmypassword permissions.

Locally: security + users unit tests (418 passed), the touched integration tests in core_tests.py (2 passed) and the 5 touched jest suites (27 passed). users/api_tests.py and session_invalidation_tests.py have 8 failures here that reproduce identically on master (this machine's test config makes Public role like Gamma and has no Redis), so CI is the arbiter for those.

ADDITIONAL INFORMATION

• Has associated issue: Reset My Password and Reset Password views are still accessible in Superset 6.0.0 after the major frontend porting. #37700
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

Breaking change: the two legacy routes and their permissions are gone (UPDATING.md entry included). Deployments that deep-link to them should point at /user_info/ or the Users list.

🤖 Generated with Claude Code

[bito-code-review]

Code Review Agent Run #6bb91d

Actionable Suggestions - 0

Review Details

• Files reviewed - 16 · Commit Range: 1e218ad..1e218ad

  • docs/admin_docs/configuration/configuring-superset.mdx
  • superset-frontend/src/features/userInfo/UserInfoModal.test.tsx
  • superset-frontend/src/features/userInfo/UserInfoModal.tsx
  • superset-frontend/src/features/users/UserListModal.test.tsx
  • superset-frontend/src/features/users/UserListModal.tsx
  • superset-frontend/src/features/users/utils.ts
  • superset/config.py
  • superset/initialization/__init__.py
  • superset/security/manager.py
  • superset/security/password_change.py
  • superset/views/users/api.py
  • superset/views/users/schemas.py
  • tests/integration_tests/core_tests.py
  • tests/unit_tests/security/manager_test.py
  • tests/unit_tests/security/test_password_change.py
  • tests/unit_tests/views/test_current_user_api.py
• Files skipped - 3

  • RESOURCES/STANDARD_ROLES.md - Reason: Filter setting
  • UPDATING.md - Reason: Filter setting
  • docs/static/resources/openapi.json - Reason: Filter setting
• 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

[netlify]

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	b517717
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6abdcb12a7cf8d00083ddb04
😎 Deploy Preview	https://deploy-preview-44626--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.

[bito-code-review]

The flagged issue is correct. Exempting all SecurityRestApi endpoints from forced password changes can allow users with a pending password change to access restricted functionality, such as generating a guest token, if they have the necessary permissions.

To resolve this, you should narrow the exemption to only the specific endpoints required for the password change flow (e.g., PUT /api/v1/me/) rather than the entire SecurityRestApi blueprint. This ensures that other sensitive endpoints remain protected by the forced password change requirement.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.70%. Comparing base (d80fcfc) to head (b517717).
⚠️ Report is 12 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #44626      +/-   ##
==========================================
+ Coverage   81.65%   81.70%   +0.05%     
==========================================
  Files        2977     2977              
  Lines      181262   181508     +246     
  Branches    41876    41936      +60     
==========================================
+ Hits       148008   148303     +295     
+ Misses      30526    30487      -39     
+ Partials     2728     2718      -10     

Flag	Coverage Δ
hive	36.67% <75.60%> (-0.02%)	⬇️
javascript	77.09% <100.00%> (+0.02%)	⬆️
mysql	55.81% <85.36%> (-0.05%)	⬇️
postgres	55.81% <85.36%> (?)
presto	38.57% <75.60%> (-0.02%)	⬇️
python	85.90% <100.00%> (+0.06%)	⬆️
sqlite	55.54% <85.36%> (-0.05%)	⬇️
unit	78.81% <100.00%> (+0.13%)	⬆️

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 #5acea8

Actionable Suggestions - 0

Additional Suggestions - 2

• tests/unit_tests/views/test_current_user_api.py - 1

  • duplicate rejection coverage · Line 97-106
    The loop at lines 97-106 re-tests the same missing/wrong `current_password` rejection scenarios already covered by `test_update_me_rejects_password_change_without_correct_current_password` (lines 122-134). Only the exact-message assertion adds value. Consolidate so a behavior change doesn't require editing both tests.
• superset-frontend/src/features/userInfo/UserInfoModal.test.tsx - 1

  • Test setup duplication · Line 134-162
    This new test duplicates ~15 lines of setup/assertion from the sibling test at lines 104-132 (render, fill password/confirm, waitFor enabled, click, waitFor onSave, parse payload, assert success toast). If a field or the payload shape changes, both tests must be updated in lockstep. Consider a shared helper parameterized by current_password value and expected payload.

Review Details

• Files reviewed - 7 · Commit Range: 1e218ad..779f810

  • superset-frontend/src/features/userInfo/UserInfoModal.test.tsx
  • superset-frontend/src/features/userInfo/UserInfoModal.tsx
  • superset/security/password_change.py
  • superset/views/users/schemas.py
  • tests/unit_tests/security/test_password_change.py
  • tests/unit_tests/views/test_current_user_api.py
  • superset/translations/messages.pot
• Files skipped - 1

  • docs/static/resources/openapi.json - Reason: Filter setting
• Tools

  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful
  • 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

[EnxDev]

Went through this one and the logic holds up. The forced-change exemptions are tight, the flag clears in the same commit as the new hash, and the admin reset path hashes and invalidates sessions the way it should.

Two small notes inline: the Confirm Password asterisk in the create form, and an integration assertion that passes without the launcher patch.

[EnxDev]

Follow-up to my earlier pass on 779f810.

Both notes are handled in 517affe. required={!isEditMode} brings the asterisk back on Confirm Password in the create form without touching the validator. The integration loop that passed on the permission redirect is gone, and test_disable_legacy_password_reset_launchers really does check the hidden buttons and the NotFound, so the docstring pointing there holds up.

Nothing new from me, LGTM once CI finishes.

[bito-code-review]

Code Review Agent Run #37e415

Actionable Suggestions - 0

Additional Suggestions - 1

• superset-frontend/src/features/users/UserListModal.tsx - 1

  • Redundant required rule · Line 230-230
    `required={!isEditMode}` adds an antd required rule on top of the existing validator, which already rejects an empty `confirmPassword` in add mode (line 240). In add mode the field now surfaces two error messages (the required rule's default plus 'Please confirm your password'). The prop only meaningfully adds the label asterisk; consider removing it or the redundant `!value` reject to avoid the duplicate error.

Review Details

• Files reviewed - 2 · Commit Range: 779f810..517affe

  • superset-frontend/src/features/users/UserListModal.tsx
  • tests/integration_tests/core_tests.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

---

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]

Code Review Agent Run #a4f814

Actionable Suggestions - 0

Additional Suggestions - 11

• tests/unit_tests/security/test_password_change.py - 1

  • Duplicated enforcement test setup · Line 172-205
    The setup here (user construction, the `_user_callback`/`_get_user_attribute`/`_get_user_invalidated_at` patches, session seeding) near-duplicates `test_enforcement_exempts_health_blueprint`'s block. Extracting a shared flagged-user fixture would keep future enforcement-setup changes single-sourced and prevent the two copies drifting.
• tests/unit_tests/views/test_current_user_api.py - 4

  • Unannotated mock variable · Line 238-238
    `mock_commit` is an unannotated mock variable, which BITO adaptive rule 12787 disallows; the repo already annotates mocks in ~168 test files. Add an explicit `MagicMock` annotation, matching the fix for `mock_clear` at line 205.
  • Unannotated mock variable · Line 205-205
    `mock_clear` is an unannotated mock variable, which BITO adaptive rule 12787 disallows; the repo already annotates mocks in ~168 test files (e.g. `variable: MagicMock`). Add an explicit `MagicMock` annotation for consistency and static-checker visibility.
  • Inline import violates rule · Line 231-231
    The inline `from superset.models.user_attributes import UserAttribute` violates the module-level-import rule: no circular dependency exists (tests/unit_tests/dao/user_test.py imports it at module level), and none is documented as BITO rule 12745 requires. Hoist it to the top-level imports with the other `superset.*` imports.
  • Broad patch on shared session · Line 238-238
    `patch("superset.views.users.api.db.session.commit")` patches `commit` on the process-wide `db` session singleton (same object as `superset.extensions.db`), not a module-local seam: any code running inside this `with` block would have its commits silently swallowed by the mock. Today only `_run_update_me` runs inside, so it is harmless, but the assertion silently widens if the block grows. Consider a narrower seam.
• tests/integration_tests/core_tests.py - 2

  • Missing return type hint · Line 135-135
    New test method lacks an explicit return type annotation. BITO.md rule 7819 and dev-standard.mdc require `-> None` on all new functions/methods, including tests. Add `-> None` to `test_legacy_fab_password_views_are_gone`.
  • Duplicated magic strings · Line 141-158
    The test re-encodes `ResetPasswordView`/`ResetMyPasswordView`/`resetpasswords`/`resetmypassword`/`UserDBModelView` and the `/resetpassword/form` routes as literals, duplicating `LEGACY_PASSWORD_VIEWS` and `LEGACY_PASSWORD_LAUNCHERS` in `superset/security/manager.py`. If those constants change, this test silently diverges. Reuse the constants.
• superset-frontend/src/features/users/UserListModal.test.tsx - 3

  • Dead mock default export · Line 33-33
    The mocked module's default export is an HOC (`withToasts(BaseComponent)` in `withToasts.tsx`), but `UserListModal` imports only the named `useToasts` hook, so the `default: (Component: T) => Component` entry is never exercised. This dead mock surface drifts silently if the component switches back to the HOC. Drop the `default` entry and keep only `useToasts`.
  • Unguarded body cast in helper · Line 72-72
    `options?.body as string` asserts a body exists; if a recorded call ever lacks one, `JSON.parse(undefined)` throws a raw TypeError instead of a clear assertion failure. Today `SupersetClient.put` with `jsonPayload` always serializes a body, so this is latent rather than active. Consider a guarded parse or an explicit expect on the body before parsing so failures stay diagnosable.
  • Brittle no-call assertion · Line 144-144
    `fetchMock.callHistory.calls(userEndpoint)` matches any request to `/api/v1/security/users/7`, not just the form's PUT. The assertion would also pass if a different verb or an unrelated fetch hit the endpoint, weakening the 'no save happened' guarantee the test name promises. Filter the recorded calls by method (or scope the mock route to PUT) so the negative assertion targets the actual submit path through `updateUser`.
• superset-frontend/src/features/users/UserListModal.tsx - 1

  • Misleading mismatch message · Line 245-250
    In `confirmPassword`'s validator, when the password field is empty but the confirmation is filled (edit mode, or after clearing a typed password), evaluation falls through to the 'Passwords do not match!' reject at lines 248-250, since `password === value` is false for `undefined`/`''` vs a string. Blocking is correct, but the message points the user at the wrong field; reject with a distinct message when `!password`.

Review Details

• Files reviewed - 17 · Commit Range: e402336..41e24c3

  • docs/admin_docs/configuration/configuring-superset.mdx
  • superset-frontend/src/features/userInfo/UserInfoModal.test.tsx
  • superset-frontend/src/features/userInfo/UserInfoModal.tsx
  • superset-frontend/src/features/users/UserListModal.test.tsx
  • superset-frontend/src/features/users/UserListModal.tsx
  • superset-frontend/src/features/users/utils.ts
  • superset/config.py
  • superset/initialization/__init__.py
  • superset/security/manager.py
  • superset/security/password_change.py
  • superset/translations/messages.pot
  • superset/views/users/api.py
  • superset/views/users/schemas.py
  • tests/integration_tests/core_tests.py
  • tests/unit_tests/security/manager_test.py
  • tests/unit_tests/security/test_password_change.py
  • tests/unit_tests/views/test_current_user_api.py
• Files skipped - 3

  • RESOURCES/STANDARD_ROLES.md - Reason: Filter setting
  • UPDATING.md - Reason: Filter setting
  • docs/static/resources/openapi.json - Reason: Filter setting
• 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

[EnxDev]

Follow-up to my earlier pass on 517affe.

Range-diffed the rebase: the three code commits are unchanged, and the only real difference is the messages.pot commit, which now carries all eight new modal msgids and matches every t() string in the two modals. Master's drift over the base doesn't touch any of these code paths, and CI is green.

One optional wording nit inline on UPDATING.md. Still LGTM.

[sadpandajoe]

A flagged user is unconditionally redirected to UserInfoView.list, which itself requires can_read on user — but the old redirect target, ResetMyPasswordView, was reachable by every authenticated user regardless of role (it was in ACCESSIBLE_PERMS). A custom role that lacks can_read on user (for example one that was only ever granted the legacy reset permissions) now gets redirected here, denied by FAB's own access check, and falls back only to logout — with no way left to reach a password-change flow and clear password_must_change. Is that intentional, or should the profile page (or some reachable change-password path) stay available to any authenticated user the way self-service reset used to be?

[sadpandajoe]

This filter only feeds set_role calls for Admin, Alpha, Gamma, sql_lab, and optionally Public — sync_role_definitions never rebuilds a custom (operator-defined) role. A custom role that already held ResetPasswordView/ResetMyPasswordView/UserDBModelView resetpasswords/resetmypassword permissions before upgrading keeps them after superset init, even though UPDATING.md and the new integration test claim/verify only that Admin/Alpha/Gamma no longer hold them. Should custom roles be swept for these stale permissions too, or is the UPDATING.md wording meant to be narrower than "no role"?

[bito-code-review]

Code Review Agent Run #fcd016

Actionable Suggestions - 0

Review Details

• Files reviewed - 2 · Commit Range: 41e24c3..6780f4b

  • superset-frontend/src/features/users/UserListModal.test.tsx
  • superset-frontend/src/features/users/UserListModal.tsx
• Files skipped - 1

  • UPDATING.md - Reason: Filter setting
• 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]

Dang, I just merged this and noticed there were open questions. Block if you wanna block, I guess ;)

I'll dig in and open up a follow-up PR if warranted.

[bito-code-review]

Bito Automatic Review Skipped – PR Already Merged

Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.
No action is needed if you didn't intend to review it. To get a review, you can type /review in a comment and save it