Skip to content

Redirect authenticated users away from the password restore form - #1557

Merged
LukeTowers merged 2 commits into
developfrom
fix/restore-form-authenticated
Sep 30, 2026
Merged

LukeTowers merged 2 commits into
developfrom
fix/restore-form-authenticated

Conversation

@LukeTowers

@LukeTowers LukeTowers commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

The backend's password restore form is for signed-out users: it takes a login, sends that account a reset link, and reports the same thing either way. It has no notion of an acting user, while the code it calls does — Backend\Models\User::getResetPasswordCode() applies an authorization check when a user is signed in — so the form's behaviour could depend on whether a session was present, which it should not. Both restore() and restore_onSubmit() now redirect a signed-in user to the backend instead.

The guard is in both methods because Backend\Classes\Controller::run() dispatches AJAX handlers before the page action, so restore_onSubmit is reachable as the onSubmit handler on the restore action without restore() running at all.

Behaviour changes

  • A signed-in backend user who opens backend/auth/restore, or posts to it, is redirected to the backend rather than served the form. Signing out first still reaches it, which is the flow the form is for, and a user who wants to change their own password uses My Settings.

Tests

Backend\Tests\Controllers\AuthRestoreOracleTest. testAuthenticatedRestoreDoesNotRevealAccountExistence and testAuthenticatedRestoreAjaxHandlerDoesNotRevealAccountExistence fail without the change. testGuestRestoreResponseIsIdenticalForKnownAndUnknownLogins, testGuestRestoreStillIssuesACodeForAKnownAccount and testResetEndpointIsIdenticalForKnownAndUnknownUserIds are controls and pass either way.

modules/backend is green at 301 tests / 718 assertions; phpcs is clean on both files.

Summary by CodeRabbit

  • Bug Fixes
    • Signed-in users are redirected to the backend when accessing password recovery.
    • Password recovery responses are consistent whether the provided login or user ID belongs to an account, reducing the risk of revealing account existence.

The backend's password restore form is for signed-out users: it takes a login, sends that account a reset link, and reports the same thing either way. It has no notion of an acting user, while the code it calls does — `Backend\Models\User::getResetPasswordCode()` applies an authorization check when a user is signed in — so the form's behaviour could depend on whether a session was present, which it should not. Both `restore()` and `restore_onSubmit()` now redirect a signed-in user to the backend instead.

The guard is in both methods because `Backend\Classes\Controller::run()` dispatches AJAX handlers before the page action, so `restore_onSubmit` is reachable as the `onSubmit` handler on the `restore` action without `restore()` running at all.

Behaviour changes:

- A signed-in backend user who opens `backend/auth/restore`, or posts to it, is redirected to the backend rather than served the form. Signing out first still reaches it, which is the flow the form is for, and a user who wants to change their own password uses My Settings.

Tests: `Backend\Tests\Controllers\AuthRestoreOracleTest` — `testAuthenticatedRestoreDoesNotRevealAccountExistence` and `testAuthenticatedRestoreAjaxHandlerDoesNotRevealAccountExistence` fail without the change; `testGuestRestoreResponseIsIdenticalForKnownAndUnknownLogins`, `testGuestRestoreStillIssuesACodeForAKnownAccount` and `testResetEndpointIsIdenticalForKnownAndUnknownUserIds` are controls that pass either way. `modules/backend` is green at 301 tests / 718 assertions and `phpcs` is clean.
@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 9d126fca-f27a-4359-b642-9892a13283f3

📥 Commits

Reviewing files that changed from the base of the PR and between 4ab9e5a and 0e3f531.

📒 Files selected for processing (2)
  • modules/backend/controllers/Auth.php
  • modules/backend/tests/controllers/AuthRestoreOracleTest.php
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread modules/backend/controllers/Auth.php Outdated
@LukeTowers LukeTowers added this to the v1.2.15 milestone Sep 30, 2026
@LukeTowers
LukeTowers merged commit eec0c42 into develop Sep 30, 2026
15 of 16 checks passed
@LukeTowers
LukeTowers deleted the fix/restore-form-authenticated branch September 30, 2026 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant