Repository navigation
Conversation
rmartinc
requested changes
Jun 15, 2026
rmartinc
left a comment
Contributor
There was a problem hiding this comment.
Thanks a lot @niels-van-nerum! But I prefer to just reset the counter if any of the allowed credentials are used. This would cover other scenarios that can appear later (for example if someone says that using webauthn credential should not reset the brute force counter).
rmartinc
requested changes
Jun 16, 2026
rmartinc
left a comment
Contributor
There was a problem hiding this comment.
Thanks @niels-van-nerum! We are almost there. 😄
Just a few comments. With this I also detected that the ResetPassword should pass the credential to perform the reset, which I think it is OK because this means we are strict now.
diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/resetcred/ResetPassword.java b/services/src/main/java/org/keycloak/authentication/authenticators/resetcred/ResetPassword.java
index 5ad402390d..6c943b709f 100755
--- a/services/src/main/java/org/keycloak/authentication/authenticators/resetcred/ResetPassword.java
+++ b/services/src/main/java/org/keycloak/authentication/authenticators/resetcred/ResetPassword.java
@@ -37,7 +37,8 @@ public class ResetPassword extends AbstractSetRequiredActionAuthenticator {
configuredFor(context))) {
context.getAuthenticationSession().addRequiredAction(UserModel.RequiredAction.UPDATE_PASSWORD);
}
- context.success();
+ // send password category to force brute-force reset for temporary lockout
+ context.success(PasswordCredentialModel.TYPE);
}I tested with this commit rmartinc@2673597 and I think it is OK.
With these changes everything is OK to me.
Contributor
Author
|
Should be good now! I'll squash when you give the OK. |
rmartinc
approved these changes
Jun 16, 2026
rmartinc
left a comment
Contributor
There was a problem hiding this comment.
Thanks @niels-van-nerum! LGTM! Squash it if you want and I'll start the tests.
This commit fixes an issue where a SSO re-auth was calling a reset on the brute-force counter, and clearing lockout, allowing a malicious actor to continue brute-forcing as long as the user was logged in and active. Brute-force reset is now only called on a successful login that wasn't a SSO re-auth. It also resets the brute-force counter based on credential categories: only categories defined in the BruteForceProtector under ALLOWED_AUTHENTICATION_CATEGORIES are processed, allowing flexibility on what counts as a valid login attempt. Null-handling in BruteForceProtector was tightened for success and loosened for failure. Closes keycloak#49960 Signed-off-by: Niels Van Nerum <vannerumniels@icloud.com>
niels-van-nerum
force-pushed
the
fix-sso-reauth-resetting-brute-force-detection
branch
from
June 16, 2026 12:36
f997416 to
ee66646
Compare
Contributor
Author
|
@rmartinc I am done squashing, thanks for the awesome guidance! |
Contributor
|
Thanks @niels-van-nerum! I'll provide the backport to 26.6. |
rmartinc
pushed a commit
to rmartinc/keycloak
that referenced
this pull request
Jun 16, 2026
…cloak#49996) This commit fixes an issue where a SSO re-auth was calling a reset on the brute-force counter, and clearing lockout, allowing a malicious actor to continue brute-forcing as long as the user was logged in and active. Brute-force reset is now only called on a successful login that wasn't a SSO re-auth. It also resets the brute-force counter based on credential categories: only categories defined in the BruteForceProtector under ALLOWED_AUTHENTICATION_CATEGORIES are processed, allowing flexibility on what counts as a valid login attempt. Null-handling in BruteForceProtector was tightened for success and loosened for failure. Closes keycloak#49960 Signed-off-by: Niels Van Nerum <vannerumniels@icloud.com> (cherry picked from commit ed270d9) test
ahus1
pushed a commit
that referenced
this pull request
Jun 29, 2026
) (#50047) This commit fixes an issue where a SSO re-auth was calling a reset on the brute-force counter, and clearing lockout, allowing a malicious actor to continue brute-forcing as long as the user was logged in and active. Brute-force reset is now only called on a successful login that wasn't a SSO re-auth. It also resets the brute-force counter based on credential categories: only categories defined in the BruteForceProtector under ALLOWED_AUTHENTICATION_CATEGORIES are processed, allowing flexibility on what counts as a valid login attempt. Null-handling in BruteForceProtector was tightened for success and loosened for failure. Closes #49960 Signed-off-by: Niels Van Nerum <vannerumniels@icloud.com> (cherry picked from commit ed270d9) test Co-authored-by: Niels Van Nerum <vannerumniels@gmail.com>
sashyo
pushed a commit
to tide-foundation/keycloak-IGA
that referenced
this pull request
Jul 20, 2026
…cloak#49996) This commit fixes an issue where a SSO re-auth was calling a reset on the brute-force counter, and clearing lockout, allowing a malicious actor to continue brute-forcing as long as the user was logged in and active. Brute-force reset is now only called on a successful login that wasn't a SSO re-auth. It also resets the brute-force counter based on credential categories: only categories defined in the BruteForceProtector under ALLOWED_AUTHENTICATION_CATEGORIES are processed, allowing flexibility on what counts as a valid login attempt. Null-handling in BruteForceProtector was tightened for success and loosened for failure. Closes keycloak#49960 Signed-off-by: Niels Van Nerum <vannerumniels@icloud.com>
1 of 2 tasks
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.
Cookie-based SSO re-authentication (e.g. a page reload in an already-authenticated
browser) was reusing the browser success path in
AuthenticationManager.redirectAfterSuccessfulFlow, which callslogSuccessandclears the user's accumulated brute-force login failures.
This let an attacker continue password-guessing against a locked account as long as
a legitimate user with an active session triggered an SSO re-auth, the lockout
counter and "Temporarily locked" state were reset without any credential being
verified.
The fix gates the
logSuccesscall on!isSSOAuthentication, so the brute-forcecounter is only reset by genuine credential-based authentication. SSO re-auth is now
neutral with respect to lockout state, while existing sessions remain active.
The OAuth
refresh_tokengrant was investigated and is unaffected, it never reacheslogSuccess, which is covered by a regression test.Added two tests to
BruteForceTest:testCookieSsoReauthenticationDoesNotResetLockout-> reproduces the bug (failed before the fix)testAuthenticatedSessionRefreshDoesNotResetLockout-> confirms the token-refresh path is unaffectedClaude Code was used for understanding and discovering the codebase, and as a rubber-duck.
Closes #49960