Add workflow event providers for user lockout events by brute force protection - #51466
Conversation
There was a problem hiding this comment.
Pull request overview
Adds workflow triggers for temporary and permanent brute-force user lockouts, addressing issue #50452.
Changes:
- Adds and registers two lockout workflow event providers.
- Adds integration coverage for both lockout modes.
- Extends
RealmBuilderwith brute-force configuration methods.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tests/base/.../UserDisabledByLockoutWorkflowTest.java |
Tests both lockout triggers. |
test-framework/.../RealmBuilder.java |
Adds brute-force builder methods. |
model/jpa/.../WorkflowEventProviderFactory |
Registers both providers. |
model/jpa/.../UserDisabledByTemporaryLockoutWorkflowEventProvider.java |
Matches temporary-lockout events. |
model/jpa/.../UserDisabledByTemporaryLockoutWorkflowEventFactory.java |
Defines the temporary trigger. |
model/jpa/.../UserDisabledByPermanentLockoutWorkflowEventProvider.java |
Matches permanent-lockout events. |
model/jpa/.../UserDisabledByPermanentLockoutWorkflowEventFactory.java |
Defines the permanent trigger. |
| org.keycloak.models.workflow.events.UserDisabledByTemporaryLockoutWorkflowEventFactory | ||
| org.keycloak.models.workflow.events.UserDisabledByPermanentLockoutWorkflowEventFactory |
98fe1d3 to
b93aa12
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (1)
docs/documentation/server_admin/topics/workflows/listening-workflow-events.adoc:58
- The permanent-lockout description contradicts itself by saying the user is also disabled “temporarily.” Remove that word so administrators can distinguish this trigger from the temporary-lockout event above.
| `user-disabled-by-permanent-lockout` | User was permanently disabled temporarily by brute force protection. | None
|
@robson90 - thanks for pinging me. While this looks good overall, it might need some more justification in the parent issue to add this. I already added a comment, but maybe you can make this stronger. There is one nitbit that GitHub copilot found in the docs: The "User was permanently disabled temporarily by brute force protection." contains an extra "temporarily" that should be removed. Maybe someone from the core-iam team like @sguilhen can have a look. |
b93aa12 to
cfc7639
Compare
Unreported flaky test detectedIf the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR. org.keycloak.testsuite.webauthn.registration.WebAuthnOtherSettingsTest#apiSecurityErrorMessage |
|
IMO this is a worthy addition. We do have a mechanism to send emails based on certain events, but Workflows can enable other strategies well beyond that. For example, one could set a required action in response to such an event in addition to sending the email. Or notify the admins in addition to the user. The scenario depends on the requirements of each deployment, but in my mind it does make sense to support these events. |
Signed-off-by: Robin Meese <39960884+robson90@users.noreply.github.com>
cfc7639 to
72d362b
Compare
|
Rebased to re-run the failed flaky build. Might have been caused by #47482 |
Closes: #50452
Ive consolidated the tests into one, as they are almost the same. To that I added the missing
RealmBuilderfunctions.CC: @ahus1