Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Closed
Labels
Description
If an account gets locked (say to a user retrying too many times), the auth will still try the credentials and return the "Bad credentials" message.
Instead once the account is locked, it should never try the authentication credentials again.
The reason for this is that it allows an attacker to guess the password of a locked account by brute force based on the response error message.
The code is in UserChecker:
/** * {@inheritdoc} */publicfunctioncheckPreAuth(UserInterface$user) {if (!$userinstanceof AdvancedUserInterface) {return; }if (!$user->isCredentialsNonExpired()) {$ex =newCredentialsExpiredException('User credentials have expired.');$ex->setUser($user);throw$ex; } }/** * {@inheritdoc} */publicfunction checkPostAuth(UserInterface$user) {if (!$userinstanceof AdvancedUserInterface) {return; }if (!$user->isAccountNonLocked()) {$ex =newLockedException('User account is locked.');$ex->setUser($user);throw$ex; }
Which I suggest to change to be this:
/** * {@inheritdoc} */publicfunctioncheckPreAuth(UserInterface$user) {if (!$userinstanceof AdvancedUserInterface) {return; }if (!$user->isCredentialsNonExpired()) {$ex =newCredentialsExpiredException('User credentials have expired.');$ex->setUser($user);throw$ex; }if (!$user->isAccountNonLocked()) {$ex =newLockedException('User account is locked.');$ex->setUser($user);throw$ex; } }/** * {@inheritdoc} */publicfunction checkPostAuth(UserInterface$user) {if (!$userinstanceof AdvancedUserInterface) {return; }