Brute force protection: Lockout permanently uses parameters configured under lockout temporarily

douglaspalmer · ahus1 · commit ecbd856176fa · 2024-08-29T16:30:22.000+02:00
Closes keycloak#30969 Signed-off-by: Douglas Palmer <dpalmer@redhat.com>
diff --git a/services/src/main/java/org/keycloak/services/managers/DefaultBruteForceProtector.java b/services/src/main/java/org/keycloak/services/managers/DefaultBruteForceProtector.java
@@ -79,7 +79,10 @@ protected void failure(KeycloakSession session, RealmModel realm, String userId,
         userLoginFailure.incrementFailures();
         logger.debugv("new num failures: {0}", userLoginFailure.getNumFailures());
 
-        int waitSeconds = realm.getWaitIncrementSeconds() *  (userLoginFailure.getNumFailures() / realm.getFailureFactor());
+        int waitSeconds = 0;
+        if(!(realm.isPermanentLockout() && realm.getMaxTemporaryLockouts() == 0)) {
+            waitSeconds = realm.getWaitIncrementSeconds() *  (userLoginFailure.getNumFailures() / realm.getFailureFactor());
+        }
         logger.debugv("waitSeconds: {0}", waitSeconds);
         logger.debugv("deltaTime: {0}", deltaTime);
 
@@ -110,7 +113,8 @@ protected void failure(KeycloakSession session, RealmModel realm, String userId,
             return;
         }
 
-        if(userLoginFailure.getNumTemporaryLockouts() > realm.getMaxTemporaryLockouts()) {
+        if(userLoginFailure.getNumTemporaryLockouts() > realm.getMaxTemporaryLockouts() ||
+                (realm.getMaxTemporaryLockouts() == 0 && userLoginFailure.getNumFailures() >= realm.getFailureFactor())) {
             UserModel user = session.users().getUserById(realm, userId);
             if (user == null) {
                 return;
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java
@@ -564,6 +564,49 @@ public void testPermanentLockout() {
             loginInvalidPassword("test-user@localhost");
             loginInvalidPassword("test-user@localhost", false);
 
+            // As of now, there are two events: USER_DISABLED_BY_PERMANENT_LOCKOUT and LOGIN_ERROR but Order is not
+            // guarantee though since the brute force detector is running separately "in its own thread" named
+            // "Brute Force Protector".
+            List<EventRepresentation> actualEvents = Arrays.asList(events.poll(), events.poll(), events.poll());
+            assertIsContained(events.expect(EventType.USER_DISABLED_BY_PERMANENT_LOCKOUT).client((String) null).detail(Details.REASON, "brute_force_attack detected"), actualEvents);
+            assertIsContained(events.expect(EventType.LOGIN_ERROR).error(Errors.INVALID_USER_CREDENTIALS), actualEvents);
+
+            // assert
+            expectPermanentlyDisabled();
+
+            UserRepresentation user = adminClient.realm("test").users().search("test-user@localhost", 0, 1).get(0);
+            assertFalse(user.isEnabled());
+            assertUserDisabledReason(BruteForceProtector.DISABLED_BY_PERMANENT_LOCKOUT);
+
+            user.setEnabled(true);
+            updateUser(user);
+            assertUserDisabledReason(null);
+        } finally {
+            realm.setPermanentLockout(false);
+            testRealm().update(realm);
+            UserRepresentation user = adminClient.realm("test").users().search("test-user@localhost", 0, 1).get(0);
+            user.setEnabled(true);
+            updateUser(user);
+        }
+    }
+
+    // https://github.com/keycloak/keycloak/issues/30969
+    @Test
+    public void testPermanentLockoutWithTempLockoutParamsSet()
+    {
+        RealmRepresentation realm = testRealm().toRepresentation();
+        try {
+            // arrange
+            realm.setWaitIncrementSeconds(0);
+            realm.setPermanentLockout(true);
+            realm.setMaxTemporaryLockouts(0);
+            realm.setQuickLoginCheckMilliSeconds(0L);
+            testRealm().update(realm);
+
+            // act
+            loginInvalidPassword("test-user@localhost");
+            loginInvalidPassword("test-user@localhost", false);
+
             // As of now, there are two events: USER_DISABLED_BY_PERMANENT_LOCKOUT and LOGIN_ERROR but Order is not
             // guarantee though since the brute force detector is running separately "in its own thread" named
             // "Brute Force Protector".
