From 83af01c4c0b0075db5e790d85d03c58040299273 Mon Sep 17 00:00:00 2001 From: Gilvan Filho Date: Tue, 13 Feb 2024 23:22:26 -0300 Subject: [PATCH] Add failedLoginNotBefore to AttackDetectionResource Closes #17574 Signed-off-by: Gilvan Filho --- .../release_notes/topics/24_0_0.adoc | 2 ++ .../admin/AttackDetectionResource.java | 26 ++++++++++++++----- .../admin/AttackDetectionResourceTest.java | 5 +++- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/docs/documentation/release_notes/topics/24_0_0.adoc b/docs/documentation/release_notes/topics/24_0_0.adoc index 3d1f48061c..5077be84cb 100644 --- a/docs/documentation/release_notes/topics/24_0_0.adoc +++ b/docs/documentation/release_notes/topics/24_0_0.adoc @@ -282,6 +282,8 @@ There have been a couple of enhancements to the Brute Protection: 2. In previous versions of Keycloak the Administrator had to choose between whether to disable users temporarily or permanently due to a Brute Force attack on their account. The administrator now has the option to permanently disable a user after a given number of temporary lockouts. +3. The property `failedLoginNotBefore` has been added to the `brute-force/users/{userId}` endpoint + = Authorization Policy In previous versions of Keycloak when the last member of a User, Group or Client policy was deleted then that policy would also be deleted. Unfortunately this could lead to an escalation of privileges if the policy was used in an aggregate policy. To avoid privilege escalation the effect policies are no longer deleted and an administrator will need to update those policies. diff --git a/services/src/main/java/org/keycloak/services/resources/admin/AttackDetectionResource.java b/services/src/main/java/org/keycloak/services/resources/admin/AttackDetectionResource.java index 8ad619f3be..aeb735e76d 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/AttackDetectionResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/AttackDetectionResource.java @@ -98,20 +98,21 @@ public class AttackDetectionResource { data.put("numTemporaryLockouts", 0); data.put("lastFailure", 0); data.put("lastIPFailure", "n/a"); + data.put("failedLoginNotBefore", 0); if (!realm.isBruteForceProtected()) return data; UserLoginFailureModel model = session.loginFailures().getUserLoginFailure(realm, userId); if (model == null) return data; - boolean disabled; - if (user == null) { - disabled = Time.currentTime() < model.getFailedLoginNotBefore(); - } else { - disabled = session.getProvider(BruteForceProtector.class).isTemporarilyDisabled(session, realm, user); - } + boolean disabled = isUserDisabled(model, user); if (disabled) { data.put("disabled", true); + if(session.getProvider(BruteForceProtector.class).isTemporarilyDisabled(session, realm, user)) { + data.put("failedLoginNotBefore", model.getFailedLoginNotBefore()); + } else { + data.put("failedLoginNotBefore", Long.MAX_VALUE); + } } data.put("numFailures", model.getNumFailures()); @@ -121,6 +122,19 @@ public class AttackDetectionResource { return data; } + private boolean isUserDisabled(UserLoginFailureModel model, UserModel user) { + if(user == null) { + return Time.currentTime() < model.getFailedLoginNotBefore(); + } + + return isUserDisabledOrLockedByBruteForce(session, realm, user); + } + + private boolean isUserDisabledOrLockedByBruteForce(KeycloakSession session, RealmModel realm, UserModel user) { + return session.getProvider(BruteForceProtector.class).isPermanentlyLockedOut(session, realm, user) + || session.getProvider(BruteForceProtector.class).isTemporarilyDisabled(session, realm, user); + } + /** * Clear any user login failures for the user * diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/AttackDetectionResourceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/AttackDetectionResourceTest.java index 1b6dea84f8..1a2ae62112 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/AttackDetectionResourceTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/AttackDetectionResourceTest.java @@ -30,6 +30,7 @@ import org.keycloak.testsuite.util.UserBuilder; import java.util.Map; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import static org.keycloak.testsuite.auth.page.AuthRealm.TEST; @@ -83,7 +84,7 @@ public class AttackDetectionResourceTest extends AbstractAdminTest { } private void assertBruteForce(Map status, Integer expectedNumFailures, Integer expectedNumTemporaryLockouts, Boolean expectedFailure, Boolean expectedDisabled) { - assertEquals(5, status.size()); + assertEquals(6, status.size()); assertEquals(expectedNumFailures, status.get("numFailures")); assertEquals(expectedNumTemporaryLockouts, status.get("numTemporaryLockouts")); assertEquals(expectedDisabled, status.get("disabled")); @@ -91,9 +92,11 @@ public class AttackDetectionResourceTest extends AbstractAdminTest { assertEquals("127.0.0.1", status.get("lastIPFailure")); Long lastFailure = (Long) status.get("lastFailure"); assertTrue(lastFailure < (System.currentTimeMillis() + 1) && lastFailure > (System.currentTimeMillis() - 10000)); + assertNotEquals("0", status.get("failedLoginNotBefore").toString()); } else { assertEquals("n/a", status.get("lastIPFailure")); assertEquals("0", status.get("lastFailure").toString()); + assertEquals("0", status.get("failedLoginNotBefore").toString()); } }