keycloak-uncached
Changes
services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java 31(+19 -12)
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();