fix: verify the password once per panel sign-in - #20347
Draft
shaheenfawzy wants to merge 1 commit into
Draft
Conversation
shaheenfawzy
force-pushed
the
fix/login-duplicate-password-verification
branch
from
August 7, 2026 18:14
24382d6 to
e5f58a0
Compare
`authenticate()` retrieved the user and verified the password inside the first `Timebox`, then handed the same credentials to `attemptWhen()`, which retrieved and verified them again. Every successful sign-in cost two bcrypt comparisons and two `SELECT`s, and fired `Attempting` twice. The first `Timebox` already establishes everything `attemptWhen()` re-checks, including the panel access check, so the user is now logged in directly. `Validated` is dispatched where the guard would have dispatched it, and the password is still rehashed when `hashing.rehash_on_login` allows it. At a bcrypt cost of 12 this takes roughly 200ms off every sign-in.
shaheenfawzy
force-pushed
the
fix/login-duplicate-password-verification
branch
from
August 7, 2026 18:41
e5f58a0 to
cdaa304
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Login::authenticate()verifies the submitted password twice on every successful sign-in.The first
Timeboxretrieves the user and callsvalidateCredentials(), so by the time it returns the credentials are known to be good and the panel access check has already run:Those same credentials are then handed to the guard:
attemptWhen()runsretrieveByCredentials()andvalidateCredentials()from scratch, so every sign-in pays two bcrypt comparisons and twoSELECTs, firesAttemptingtwice, and runs the panel access check twice. At a production bcrypt cost of 12 the redundant hash alone is around 200ms.Since nothing is left for the guard to establish, the user is logged in directly with
$authGuard->login(). The behaviour the guard would have contributed is kept:SessionGuard::hasValidCredentials()dispatchesValidated, and is protected, soValidatedis now dispatched fromauthenticate()at the point the guard would have dispatched it.SessionGuard::$rehashOnLoginis protected, so the rehash readsconfig('hashing.rehash_on_login', true)directly, which is the same valueAuthManagerpasses to the guard when it constructs it.One deliberate difference:
SessionGuard::$lastAttemptedis protected with no setter, so it is no longer populated by a panel login. Nothing in Filament reads it.Reproduction: https://github.com/shaheenfawzy/filament-login-double-hash (
composer setup && php artisan test --filter=LoginPerformanceTest)The same method also pads every sign-in with the multi-factor timebox; that is a separate concern and is fixed in #20348.
Visual changes
None.
Functional changes
composer cscommand.tests/src/Panels/Auth/LoginTest.phpgains three tests:AttemptingandValidatedeach fire exactly once on a successful sign-in, a password stored at a lower bcrypt cost is rehashed, and it is left alone whenhashing.rehash_on_loginis disabled. The first fails on5.xwithout this change, seeingAttemptingtwice. The two rehash tests pass either way and are regression guards, sinceattemptWhen()rehashed too.