Skip to content

Allow identity providers to reset brute force login failures on login - #53724

Open
JBoogieman wants to merge 1 commit into
keycloak:mainfrom
JBoogieman:issue-53249
Open

JBoogieman wants to merge 1 commit into
keycloak:mainfrom
JBoogieman:issue-53249

Conversation

@JBoogieman

@JBoogieman JBoogieman commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Closes #53249

Since #49996, the brute force failure count is only reset by a successful login that used a password, OTP, or recovery code. That fixed cookie SSO re-authentication resetting the count (#49960), but it also means a successful brokered login (SAML, OIDC, social) never resets it. Users who mostly sign in through an identity provider keep accumulating failures from occasional mistyped local passwords until they are locked out.

Resetting on every brokered login would reopen #49960, because an identity provider can complete a login without the user entering anything (from an existing session at the provider). This PR makes the reset opt-in per identity provider:

  • New identity provider setting resetLoginFailures (admin console: Advanced settings > "Reset brute force failures on login"). Off by default, so existing behavior is unchanged.
  • When enabled, AuthenticationManager.logSuccess() passes an identity-provider category to the brute force protector for logins completed through that provider, and the failure count is reset the same way as after a password login.
  • The check uses the IDENTITY_PROVIDER user session note set in finishBrokerAuthentication(), so later cookie-based SSO re-authentication still does not reset the count, even with the setting on.

I went with a per-provider setting since trust differs between providers, and it needs no schema changes. Happy to move it to a realm-level setting if you'd prefer.

Tests: three new tests in AbstractAdvancedBrokerTest (runs for SAML, OIDC, and OAuth2 brokering):

  • loginWithBrokerDoesNotResetBruteForceFailureCountByDefault: count stays at 1 after a brokered login with the setting off
  • loginWithBrokerResetsBruteForceFailureCountWhenEnabled: count resets to 0 with the setting on
  • loginWithBrokerThenCookieSsoDoesNotResetBruteForceFailureCount: with the setting on, cookie SSO re-authentication on a session created through the identity provider does not reset the count (Brute-force detection: Authenticated session token refresh resets lockout counter #49960)

Ran locally for KcSamlBrokerTest and KcOidcBrokerTest.

Docs: identity provider general configuration table and the brute force detection page.

AI disclosure: I used Claude to help test, and review old PRs. I reviewed, built, and tested all changes.

Copilot AI balanced review requested due to automatic review settings October 7, 2026 23:54
@JBoogieman
JBoogieman requested review from a team as code owners October 7, 2026 23:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Security-sensitive cookie-SSO behavior with the provider setting enabled lacks regression coverage.

1 open finding
What changed in this PR

Adds opt-in brute-force failure resets for successful identity-provider logins.

Changes:

  • Adds per-provider configuration and Admin UI control.
  • Integrates brokered logins with brute-force protection.
  • Adds broker tests and administrator documentation.
File Description
AbstractAdvancedBrokerTest.java Tests default and enabled behavior.
DefaultBruteForceProtector.java Adds the identity-provider category.
AuthenticationManager.java Detects eligible brokered logins.
IdentityProviderModel.java Defines the new configuration property.
AdvancedSettings.tsx Adds the Admin UI switch.
messages_en.properties Adds UI labels and guidance.
brute-force.adoc Documents reset behavior.
configuration.adoc Documents provider configuration.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

JBoogieman added a commit to JBoogieman/JBoogieman that referenced this pull request Oct 8, 2026
Copilot AI balanced review requested due to automatic review settings October 8, 2026 00:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new regression tests should be migrated from the deprecated Arquillian suite to the current Test Framework.

1 open finding
1 resolved since last review

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Closes keycloak#53249

Signed-off-by: Justin Brueggemann <167224505+JBoogieman@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 14, 2026 14:13
@JBoogieman

Copy link
Copy Markdown
Contributor Author

Rebased on main and squashed to one commit. The only conflict was the admin UI's messages_en.properties.

On moving the new tests to the new Test Framework: they add test cases to the existing AbstractAdvancedBrokerTest as part of a bug fix, which testsuite/DEPRECATED.md permits, and the file gets 99 added lines, under the deprecation check's 100-line limit (the Testsuite Deprecation Check passes). The SAML, OIDC and OAuth2 broker tests inherit these cases from that class, so moving them would mean migrating the advanced broker suite itself, which is out of scope for this fix. Happy to migrate if maintainers prefer.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are identified for the opt-in brokered-login failure reset change.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Brute force failure counter is not reset on successful SAML (IdP) login, causing users to be permanently locked out

2 participants