Repository navigation
Added event for temporary lockout for brute force protector - #26630
Conversation
Passing run #10949 ↗︎
Details:
Review all test suite changes for PR #26630 ↗︎ |
|||||||||||||||
|
One observation regarding the default implementation It converts "success" events to |
There was a problem hiding this comment.
Thank you for this change, I like the direction. See below for some comments that I'd like to see changed.
32bfe63 to
052b5dc
Compare
|
@tsaarni - I've seen that you have merged the main branch into this PR. Within the Keycloak project, we usually prefer a rebase, as that allows for simpler squashing of commits later. |
052b5dc to
50afa72
Compare
Sure! I've removed the merge commit, rebased and force pushed - also to re-trigger CI since there seemed to be some problems with Quarkus 3.7.0.CR1 that I think were unrelated. |
50afa72 to
52fafc5
Compare
This change adds event for brute force protector when user account is temporarily disabled. It also lowers the priority of free-text log for failed login attempts. Signed-off-by: Tero Saarni <tero.saarni@est.tech>
Signed-off-by: Tero Saarni <tero.saarni@est.tech>
Signed-off-by: Tero Saarni <tero.saarni@est.tech>
52fafc5 to
dbb68f0
Compare
|
I've submitted a PR with documentation changes to this PR for @tsaarni to review and merge: Nordix#235 |
Signed-off-by: Tero Saarni <tero.saarni@est.tech>
Thanks @ahus1! DCO check seems unhappy, maybe your commit should not have had my sign-off. Do you want it fixed? |
|
I've set the DCO to pass, since @ahus1 is a member of the Keycloak team this should not be an issue. |
There was a problem hiding this comment.
Approving based on previous reviews of @pedroigor and the docs review by @tsaarni.
…#26630) This change adds event for brute force protector when user account is temporarily disabled. It also lowers the priority of free-text log for failed login attempts. Signed-off-by: Tero Saarni <tero.saarni@est.tech> Signed-off-by: Alexander Schwartz <aschwart@redhat.com> Co-authored-by: Alexander Schwartz <aschwart@redhat.com> Signed-off-by: Krzysztof Walczewski <kwalczewski@gmail.com>
This PR adds new event when user account is temporarily disabled by brute force protector. Before this change, and event was sent for only permanent lockout. Having event also for temporary lockout can improve visibility of ongoing brute force attack.
The PR also removes the free-text log for failed login attempts. This avoids having following printout in logs:
Reasoning for this change is that unconditionally printing user id and IP address in free-text logs for every invalid login may be seen as unnecessary logging of personally identifiable information. This can be also better served by event now that both permanent and temporary lockout are covered. In lockout event the presence of this information is justified and due to the structured format of events, is easier to process appropriately.
Fixes #26626