From 4d1292f5c441b7f6192f8790917cba0cea9a86d8 Mon Sep 17 00:00:00 2001
From: Luke Towers <git@luketowers.ca>
Date: Sat, 26 Sep 2026 11:17:41 -0600
Subject: [PATCH 1/2] Redirect authenticated users away from the password
 restore form
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 modules/backend/controllers/Auth.php          |  13 ++
 .../controllers/AuthRestoreOracleTest.php     | 172 ++++++++++++++++++
 2 files changed, 185 insertions(+)
 create mode 100644 modules/backend/tests/controllers/AuthRestoreOracleTest.php

diff --git a/modules/backend/controllers/Auth.php b/modules/backend/controllers/Auth.php
index 549d96f7ac..ec6b126faf 100644
--- a/modules/backend/controllers/Auth.php
+++ b/modules/backend/controllers/Auth.php
@@ -120,6 +120,10 @@ public function signout()
      */
     public function restore()
     {
+        if (BackendAuth::user()) {
+            return Backend::redirect('backend');
+        }
+
         $this->bodyClass = 'restore';
 
         try {
@@ -136,6 +140,15 @@ public function restore()
      */
     public function restore_onSubmit()
     {
+        // Guarded here as well as in restore(), because Controller::run() dispatches an AJAX
+        // handler before the action method, so this is reachable as `onSubmit` on the restore
+        // action without restore() running at all. The form is for signed-out users: it has no
+        // notion of an acting user, while the code it calls does, so its behaviour should not
+        // depend on one being present.
+        if (BackendAuth::user()) {
+            return Backend::redirect('backend');
+        }
+
         // Force Trusted Host verification on password reset link generation
         // regardless of config to protect against host header poisoning
         $trustedHosts = Config::get('app.trustedHosts', false);
diff --git a/modules/backend/tests/controllers/AuthRestoreOracleTest.php b/modules/backend/tests/controllers/AuthRestoreOracleTest.php
new file mode 100644
index 0000000000..020464290e
--- /dev/null
+++ b/modules/backend/tests/controllers/AuthRestoreOracleTest.php
@@ -0,0 +1,172 @@
+<?php
+
+namespace Backend\Tests\Controllers;
+
+use Backend\Controllers\Auth;
+use Backend\Models\User;
+use Illuminate\Http\Request as HttpRequest;
+use Illuminate\Support\Facades\Request;
+use System\Tests\Bootstrap\PluginTestCase;
+use Winter\Storm\Database\Model;
+use Winter\Storm\Support\Facades\Config;
+use Winter\Storm\Support\Facades\Flash;
+use Winter\Storm\Support\Facades\Mail;
+
+/**
+ * The public restore form's response must not depend on whether the submitted login matches
+ * an existing account.
+ *
+ * The form is for signed-out users and has no notion of an acting user, but nothing stopped an
+ * authenticated session reaching it, and the user model it calls does apply an authorization
+ * check when one is present. Its response therefore has to be asserted with a session as well
+ * as without, for both a login that matches an account and one that does not.
+ */
+class AuthRestoreOracleTest extends PluginTestCase
+{
+    protected User $alice;
+
+    public function setUp(): void
+    {
+        parent::setUp();
+
+        Config::set('cms.backendUri', 'backend');
+        Config::set('cms.enableCsrfProtection', false);
+        Config::set('app.url', 'http://localhost');
+        // The known-login branch sends the reset mail inline, and the response it produces is
+        // exactly what is being compared, so nothing must reach a real transport
+        Mail::fake();
+        HttpRequest::setTrustedHosts([]);
+
+        $this->alice = $this->makeUser('alice');
+    }
+
+    protected function makeUser(string $login): User
+    {
+        Model::unguard();
+        $user = User::create([
+            'first_name' => ucfirst($login),
+            'last_name' => 'User',
+            'login' => $login,
+            'email' => "{$login}@test.test",
+            'password' => 'TestPassword1',
+            'password_confirmation' => 'TestPassword1',
+            'is_activated' => true,
+        ]);
+        Model::reguard();
+
+        return $user;
+    }
+
+    protected function submitRestore(string $login): array
+    {
+        Flash::forget();
+
+        Request::swap(HttpRequest::create('http://localhost/', 'POST', [
+            'postback' => 1,
+            'login' => $login,
+        ]));
+
+        try {
+            (new Auth)->restore_onSubmit();
+            $thrown = null;
+        } catch (\Throwable $ex) {
+            $thrown = get_class($ex) . ': ' . $ex->getMessage();
+        }
+
+        return [
+            'exception' => $thrown,
+            'success' => Flash::get('success'),
+            'error' => Flash::get('error'),
+        ];
+    }
+
+    /** Control: as a guest, a known and an unknown login are already indistinguishable. */
+    public function testGuestRestoreResponseIsIdenticalForKnownAndUnknownLogins(): void
+    {
+        $known = $this->submitRestore('alice');
+        $unknown = $this->submitRestore('nobody-here');
+
+        $this->assertNull($known['exception']);
+        $this->assertSame($known, $unknown);
+    }
+
+    /** Invalidation test: the guest flow still issues a code for a real account. */
+    public function testGuestRestoreStillIssuesACodeForAKnownAccount(): void
+    {
+        $this->submitRestore('alice');
+
+        $this->assertNotEmpty(User::find($this->alice->getKey())->reset_password_code);
+    }
+
+    public function testAuthenticatedRestoreDoesNotRevealAccountExistence(): void
+    {
+        $mallory = $this->makeUser('mallory');
+        $this->assertFalse($mallory->hasAccess('backend.manage_users'), 'Precondition: no user management access');
+
+        $this->actingAs($mallory);
+
+        $known = $this->submitRestore('alice');
+        $unknown = $this->submitRestore('nobody-here');
+
+        $this->assertNull($known['exception'], 'A known login must not raise where an unknown one does not');
+        $this->assertSame($known, $unknown);
+    }
+
+    /**
+     * `restore_onSubmit` is also reachable as the `onSubmit` AJAX handler for the restore
+     * action, and Controller::run() dispatches AJAX handlers before the action method - so a
+     * guard placed only in restore() would leave this route open.
+     */
+    public function testAuthenticatedRestoreAjaxHandlerDoesNotRevealAccountExistence(): void
+    {
+        $this->actingAs($this->makeUser('mallory'));
+
+        $responses = [];
+
+        foreach (['alice', 'nobody-here'] as $login) {
+            $request = HttpRequest::create('http://localhost/backend/backend/auth/restore', 'POST', [
+                'login' => $login,
+            ]);
+            $request->headers->set('X-Requested-With', 'XMLHttpRequest');
+            $request->headers->set('X-WINTER-REQUEST-HANDLER', 'onSubmit');
+            Request::swap($request);
+
+            $response = (new Auth)->run('restore', []);
+
+            $responses[$login] = [
+                'status' => $response->getStatusCode(),
+                'content' => (string) $response->getContent(),
+            ];
+        }
+
+        $this->assertSame($responses['alice'], $responses['nobody-here']);
+    }
+
+    /**
+     * The reset URL carries the account's numeric id. Neither the reset page nor a reset
+     * submission answers differently for an id that exists and one that does not, so the id in
+     * the link is not itself an enumeration oracle. Kept as a control, since it is the property
+     * the original report described.
+     */
+    public function testResetEndpointIsIdenticalForKnownAndUnknownUserIds(): void
+    {
+        $this->assertNull(\BackendAuth::findUserById(999999), 'Precondition: id 999999 is absent');
+
+        $render = [];
+
+        foreach ([$this->alice->getKey(), 999999] as $id) {
+            Flash::forget();
+            Request::swap(HttpRequest::create('http://localhost/', 'GET'));
+
+            $controller = new Auth;
+            $controller->reset($id, 'bogus-code');
+
+            $render[$id] = [
+                'flash' => Flash::all(),
+                'code' => $controller->vars['code'],
+            ];
+        }
+
+        $this->assertSame($render[$this->alice->getKey()]['flash'], $render[999999]['flash']);
+    }
+}

From 0e3f5315e0679a1120881c4db7fc477b555ed684 Mon Sep 17 00:00:00 2001
From: Luke Towers <git@luketowers.ca>
Date: Tue, 29 Sep 2026 19:15:00 -0600
Subject: [PATCH 2/2] Apply suggestion from @LukeTowers

---
 modules/backend/controllers/Auth.php | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/modules/backend/controllers/Auth.php b/modules/backend/controllers/Auth.php
index ec6b126faf..16e8a6b9bd 100644
--- a/modules/backend/controllers/Auth.php
+++ b/modules/backend/controllers/Auth.php
@@ -140,11 +140,6 @@ public function restore()
      */
     public function restore_onSubmit()
     {
-        // Guarded here as well as in restore(), because Controller::run() dispatches an AJAX
-        // handler before the action method, so this is reachable as `onSubmit` on the restore
-        // action without restore() running at all. The form is for signed-out users: it has no
-        // notion of an acting user, while the code it calls does, so its behaviour should not
-        // depend on one being present.
         if (BackendAuth::user()) {
             return Backend::redirect('backend');
         }