Merge pull request #2122 from abstractj/KEYCLOAK-2151

Brute force detector active for non-existing accounts

services/src/main/java/org/keycloak/services/managers/DefaultBruteForceProtector.java

@@ -21,6 +21,7 @@ import org.keycloak.common.ClientConnection;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.KeycloakSessionFactory;
 import org.keycloak.models.RealmModel;
+import org.keycloak.models.UserModel;
 import org.keycloak.models.UsernameLoginFailureModel;
 import org.keycloak.services.ServicesLogger;
 
@@ -91,44 +92,49 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector
         logger.debug("failure");
         RealmModel realm = getRealmModel(session, event);
         logFailure(event);
-        UsernameLoginFailureModel user = getUserModel(session, event);
+        UserModel user = session.users().getUserByUsername(event.username.toString(), realm);
-        if (user == null) {
+        UsernameLoginFailureModel userLoginFailure = getUserModel(session, event);
-            user = session.sessions().addUserLoginFailure(realm, event.username.toLowerCase());
+        if (user != null) {
-        }
+            if (userLoginFailure == null) {
-        user.setLastIPFailure(event.ip);
+                userLoginFailure = session.sessions().addUserLoginFailure(realm, event.username.toLowerCase());
-        long currentTime = System.currentTimeMillis();
-        long last = user.getLastFailure();
-        long deltaTime = 0;
-        if (last > 0) {
-            deltaTime = currentTime - last;
-        }
-        user.setLastFailure(currentTime);
-        if (deltaTime > 0) {
-            // if last failure was more than MAX_DELTA clear failures
-            if (deltaTime > (long)realm.getMaxDeltaTimeSeconds() *1000L) {
-                user.clearFailures();
             }
-        }
+            userLoginFailure.setLastIPFailure(event.ip);
-        user.incrementFailures();
+            long currentTime = System.currentTimeMillis();
-        logger.debugv("new num failures: {0}" , user.getNumFailures());
+            long last = userLoginFailure.getLastFailure();
+            long deltaTime = 0;
+            if (last > 0) {
+                deltaTime = currentTime - last;
+            }
+            userLoginFailure.setLastFailure(currentTime);
+            if (deltaTime > 0) {
+                // if last failure was more than MAX_DELTA clear failures
+                if (deltaTime > (long) realm.getMaxDeltaTimeSeconds() * 1000L) {
+                    userLoginFailure.clearFailures();
+                }
+            }
+            userLoginFailure.incrementFailures();
+            logger.debugv("new num failures: {0}", userLoginFailure.getNumFailures());
 
-        int waitSeconds = realm.getWaitIncrementSeconds() * (user.getNumFailures() / realm.getFailureFactor());
+            int waitSeconds = realm.getWaitIncrementSeconds() * (userLoginFailure.getNumFailures() / realm.getFailureFactor());
-        logger.debugv("waitSeconds: {0}", waitSeconds);
+            logger.debugv("waitSeconds: {0}", waitSeconds);
-        logger.debugv("deltaTime: {0}", deltaTime);
+            logger.debugv("deltaTime: {0}", deltaTime);
-        if (waitSeconds == 0) {
+
-            if (last > 0 && deltaTime < realm.getQuickLoginCheckMilliSeconds()) {
+            if (waitSeconds == 0) {
-                logger.debugv("quick login, set min wait seconds");
+                if (last > 0 && deltaTime < realm.getQuickLoginCheckMilliSeconds()) {
-                waitSeconds = realm.getMinimumQuickLoginWaitSeconds();
+                    logger.debugv("quick login, set min wait seconds");
+                    waitSeconds = realm.getMinimumQuickLoginWaitSeconds();
+                }
+            }
+            if (waitSeconds > 0) {
+                waitSeconds = Math.min(realm.getMaxFailureWaitSeconds(), waitSeconds);
+                int notBefore = (int) (currentTime / 1000) + waitSeconds;
+                logger.debugv("set notBefore: {0}", notBefore);
+                userLoginFailure.setFailedLoginNotBefore(notBefore);
             }
-        }
-        if (waitSeconds > 0) {
-            waitSeconds = Math.min(realm.getMaxFailureWaitSeconds(), waitSeconds);
-            int notBefore = (int) (currentTime / 1000) + waitSeconds;
-            logger.debugv("set notBefore: {0}", notBefore);
-            user.setFailedLoginNotBefore(notBefore);
         }
     }
 
+
     protected UsernameLoginFailureModel getUserModel(KeycloakSession session, LoginEvent event) {
         RealmModel realm = getRealmModel(session, event);
         if (realm == null) return null;

testsuite/integration/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java

@@ -41,6 +41,7 @@ import org.keycloak.testsuite.pages.AppPage;
 import org.keycloak.testsuite.pages.AppPage.RequestType;
 import org.keycloak.testsuite.pages.LoginPage;
 import org.keycloak.testsuite.pages.LoginTotpPage;
+import org.keycloak.testsuite.pages.RegisterPage;
 import org.keycloak.testsuite.rule.GreenMailRule;
 import org.keycloak.testsuite.rule.KeycloakRule;
 import org.keycloak.testsuite.rule.KeycloakRule.KeycloakSetup;
@@ -101,13 +102,15 @@ public class BruteForceTest {
     @WebResource
     protected LoginPage loginPage;
 
+    @WebResource
+    private RegisterPage registerPage;
+
     @WebResource
     protected LoginTotpPage loginTotpPage;
 
     @WebResource
     protected OAuthClient oauth;
 
-
     private TimeBasedOTP totp = new TimeBasedOTP();
 
     private int lifespan;
@@ -340,6 +343,17 @@ public class BruteForceTest {
         loginSuccess();
     }
 
+    @Test
+    public void testNonExistingAccounts() throws Exception {
+
+        loginInvalidPassword("non-existent-user");
+        loginInvalidPassword("non-existent-user");
+        loginInvalidPassword("non-existent-user");
+
+        registerUser("non-existent-user");
+
+    }
+
     public void expectTemporarilyDisabled() throws Exception {
         expectTemporarilyDisabled("test-user@localhost");
     }
@@ -430,4 +444,16 @@ public class BruteForceTest {
         events.clear();
     }
 
+    public void registerUser(String username){
+        loginPage.open();
+        loginPage.clickRegister();
+        registerPage.assertCurrent();
+
+        registerPage.register("user", "name",  username + "@localhost", username, "password", "password");
+
+        Assert.assertNull(registerPage.getInstruction());
+
+        events.clear();
+    }
+
 }

testsuite/integration/src/test/java/org/keycloak/testsuite/pages/RegisterPage.java

@@ -57,6 +57,10 @@ public class RegisterPage extends AbstractPage {
     @FindBy(className = "alert-error")
     private WebElement loginErrorMessage;
 
+    @FindBy(className = "instruction")
+    private WebElement loginInstructionMessage;
+
+
     public void register(String firstName, String lastName, String email, String username, String password, String passwordConfirm) {
         firstNameInput.clear();
         if (firstName != null) {
@@ -131,6 +135,15 @@ public class RegisterPage extends AbstractPage {
         return loginErrorMessage != null ? loginErrorMessage.getText() : null;
     }
 
+    public String getInstruction() {
+        try {
+            return loginInstructionMessage != null ? loginInstructionMessage.getText() : null;
+        } catch (NoSuchElementException e){
+            // OK
+        }
+        return null;
+    }
+
     public String getFirstName() {
         return firstNameInput.getAttribute("value");
     }
@@ -164,4 +177,4 @@ public class RegisterPage extends AbstractPage {
         throw new UnsupportedOperationException();
     }
 
-}
+}