keycloak-uncached

KEYCLOAK-6011

12/19/2017 6:02:39 PM

Details

diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java
index 6b6da56..3448826 100755
--- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java
+++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java
@@ -22,7 +22,6 @@ import org.keycloak.authentication.AbstractFormAuthenticator;
 import org.keycloak.authentication.AuthenticationFlowContext;
 import org.keycloak.authentication.AuthenticationFlowError;
 import org.keycloak.credential.CredentialInput;
-import org.keycloak.credential.CredentialModel;
 import org.keycloak.credential.hash.PasswordHashProvider;
 import org.keycloak.events.Details;
 import org.keycloak.events.Errors;
@@ -130,17 +129,7 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
             context.forceChallenge(challengeResponse);
             return false;
         }
-        if (context.getRealm().isBruteForceProtected()) {
-            if (context.getProtector().isTemporarilyDisabled(context.getSession(), context.getRealm(), user)) {
-                context.getEvent().user(user);
-                context.getEvent().error(Errors.USER_TEMPORARILY_DISABLED);
-                Response challengeResponse = temporarilyDisabledUser(context);
-                // this is not a failure so don't call failureChallenge.
-                //context.failureChallenge(AuthenticationFlowError.USER_TEMPORARILY_DISABLED, challengeResponse);
-                context.forceChallenge(challengeResponse);
-                return false;
-            }
-        }
+        if (isTemporarilyDisabledByBruteForce(context, user)) return false;
         return true;
     }
 
@@ -203,6 +192,9 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
         List<CredentialInput> credentials = new LinkedList<>();
         String password = inputData.getFirst(CredentialRepresentation.PASSWORD);
         credentials.add(UserCredentialModel.password(password));
+
+        if (isTemporarilyDisabledByBruteForce(context, user)) return false;
+
         if (password != null && !password.isEmpty() && context.getSession().userCredentialManager().isValid(context.getRealm(), user, credentials)) {
             return true;
         } else {
@@ -215,4 +207,19 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
         }
     }
 
+    private boolean isTemporarilyDisabledByBruteForce(AuthenticationFlowContext context, UserModel user) {
+        if (context.getRealm().isBruteForceProtected()) {
+            if (context.getProtector().isTemporarilyDisabled(context.getSession(), context.getRealm(), user)) {
+                context.getEvent().user(user);
+                context.getEvent().error(Errors.USER_TEMPORARILY_DISABLED);
+                Response challengeResponse = temporarilyDisabledUser(context);
+                // this is not a failure so don't call failureChallenge.
+                //context.failureChallenge(AuthenticationFlowError.USER_TEMPORARILY_DISABLED, challengeResponse);
+                context.forceChallenge(challengeResponse);
+                return true;
+            }
+        }
+        return false;
+    }
+
 }
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 531b556..e65fdc5 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
@@ -63,7 +63,7 @@ public class AttackDetectionResourceTest extends AbstractAdminTest {
         oauthClient.doLogin("test-user2", "invalid");
         oauthClient.doLogin("nosuchuser", "invalid");
 
-        assertBruteForce(detection.bruteForceUserStatus(findUser("test-user@localhost").getId()), 3, true, true);
+        assertBruteForce(detection.bruteForceUserStatus(findUser("test-user@localhost").getId()), 2, true, true);
         assertBruteForce(detection.bruteForceUserStatus(findUser("test-user2").getId()), 2, true, true);
         assertBruteForce(detection.bruteForceUserStatus("nosuchuser"), 0, false, false);
 
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
index 7b65a0c..d589f2a 100644
--- 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
@@ -311,11 +311,13 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
         loginInvalidPassword();
         loginInvalidPassword();
         expectTemporarilyDisabled();
+        expectTemporarilyDisabled("test-user@localhost", null, "invalid");
         clearUserFailures();
         loginSuccess();
         loginInvalidPassword();
         loginInvalidPassword();
         expectTemporarilyDisabled();
+        expectTemporarilyDisabled("test-user@localhost", null, "invalid");
         clearAllUserFailures();
         loginSuccess();
     }
@@ -326,6 +328,8 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
         loginInvalidPassword();
         loginInvalidPassword();
         expectTemporarilyDisabled();
+        expectTemporarilyDisabled("test-user@localhost", null, "invalid");
+
         // KEYCLOAK-5420
         // Test to make sure that temporarily disabled doesn't increment failure count
         testingClient.testing().setTimeOffset(Collections.singletonMap("offset", String.valueOf(6)));
@@ -343,6 +347,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
         loginInvalidPassword("test-User@localhost");
         loginInvalidPassword("Test-user@localhost");
         expectTemporarilyDisabled();
+        expectTemporarilyDisabled("test-user@localhost", null, "invalid");
         clearAllUserFailures();
     }
 
@@ -363,6 +368,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
         loginMissingPassword();
         loginMissingPassword();
         expectTemporarilyDisabled();
+        expectTemporarilyDisabled("test-user@localhost", null, "invalid");
         clearUserFailures();
         loginSuccess();
     }
@@ -373,6 +379,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
         loginWithTotpFailure();
         loginWithTotpFailure();
         expectTemporarilyDisabled();
+        expectTemporarilyDisabled("test-user@localhost", null, "invalid");
         clearUserFailures();
         loginSuccess();
     }
@@ -383,6 +390,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
         loginWithMissingTotp();
         loginWithMissingTotp();
         expectTemporarilyDisabled();
+        expectTemporarilyDisabled("test-user@localhost", null, "invalid");
         clearUserFailures();
         loginSuccess();
     }
@@ -454,6 +462,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
         loginInvalidPassword();
         loginInvalidPassword();
         expectTemporarilyDisabled();
+        expectTemporarilyDisabled("test-user@localhost", null, "invalid");
 
         loginPage.resetPassword();
         resetPasswordPage.assertCurrent();
@@ -468,12 +477,16 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
     }
 
     public void expectTemporarilyDisabled() throws Exception {
-        expectTemporarilyDisabled("test-user@localhost", null);
+        expectTemporarilyDisabled("test-user@localhost", null, "password");
     }
 
     public void expectTemporarilyDisabled(String username, String userId) throws Exception {
+        expectTemporarilyDisabled(username, userId, "password");
+    }
+
+    public void expectTemporarilyDisabled(String username, String userId, String password) throws Exception {
         loginPage.open();
-        loginPage.login(username, "password");
+        loginPage.login(username, password);
 
         loginPage.assertCurrent();
         String src = driver.getPageSource();