From 7cb39c2dfcbb3ceb25cadc0ef0dd2bbd6869cc50 Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Fri, 15 Dec 2017 12:16:24 -0500 Subject: [PATCH 1/7] KEYCLOAK-5420 --- .../AbstractUsernameFormAuthenticator.java | 6 ++++-- .../admin/FineGrainAdminUnitTest.java | 2 +- .../testsuite/forms/BruteForceTest.java | 20 +++++++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) 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 72c10f5e22..b944149b8f 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 @@ -125,7 +125,8 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth context.getEvent().user(user); context.getEvent().error(Errors.USER_DISABLED); Response challengeResponse = disabledUser(context); - context.failureChallenge(AuthenticationFlowError.USER_DISABLED, challengeResponse); + // this is not a failure so don't call failureChallenge. + context.forceChallenge(challengeResponse); return false; } if (context.getRealm().isBruteForceProtected()) { @@ -133,7 +134,8 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth context.getEvent().user(user); context.getEvent().error(Errors.USER_TEMPORARILY_DISABLED); Response challengeResponse = temporarilyDisabledUser(context); - context.failureChallenge(AuthenticationFlowError.USER_TEMPORARILY_DISABLED, challengeResponse); + // this is not a failure so don't call failureChallenge. + context.forceChallenge(challengeResponse); return false; } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminUnitTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminUnitTest.java index 1df661285f..15f3b41591 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminUnitTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminUnitTest.java @@ -389,7 +389,7 @@ public class FineGrainAdminUnitTest extends AbstractKeycloakTest { return true; } - //@Test + @Test public void testDemo() throws Exception { testingClient.server().run(FineGrainAdminUnitTest::setupDemo); Thread.sleep(1000000000); 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 a8fe41b0fe..a10ffb6fec 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 @@ -45,6 +45,7 @@ import org.keycloak.testsuite.util.RealmRepUtil; import org.keycloak.testsuite.util.UserBuilder; import java.net.MalformedURLException; +import java.util.Collections; import static org.junit.Assert.assertEquals; @@ -67,6 +68,10 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { testRealm.setBruteForceProtected(true); testRealm.setFailureFactor(2); + testRealm.setMaxDeltaTimeSeconds(200); + testRealm.setMaxFailureWaitSeconds(1000); + testRealm.setWaitIncrementSeconds(50); + testRealm.setQuickLoginCheckMilliSeconds(0L); userId = user.getId(); @@ -286,6 +291,21 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { loginSuccess(); } + @Test + public void testWait() throws Exception { + loginSuccess(); + loginInvalidPassword(); + loginInvalidPassword(); + expectTemporarilyDisabled(); + // KEYCLOAK-5420 + // Test to make sure that temporarily disabled doesn't increment failure count + testingClient.testing().setTimeOffset(Collections.singletonMap("offset", String.valueOf(52))); + loginSuccess(); + clearUserFailures(); + clearAllUserFailures(); + loginSuccess(); + } + @Test public void testBrowserInvalidPasswordDifferentCase() throws Exception { loginSuccess("test-user@localhost"); From 1eec2747efa4fe929e3d1576568f593cda3e9b8b Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Fri, 15 Dec 2017 15:31:14 -0500 Subject: [PATCH 2/7] fix stupidity --- .../org/keycloak/testsuite/admin/FineGrainAdminUnitTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminUnitTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminUnitTest.java index 15f3b41591..1df661285f 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminUnitTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminUnitTest.java @@ -389,7 +389,7 @@ public class FineGrainAdminUnitTest extends AbstractKeycloakTest { return true; } - @Test + //@Test public void testDemo() throws Exception { testingClient.server().run(FineGrainAdminUnitTest::setupDemo); Thread.sleep(1000000000); From 80be4c9dbc402243aee2f2c4922d2bc2a050f954 Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Sat, 16 Dec 2017 07:12:32 -0500 Subject: [PATCH 3/7] fix more --- .../browser/AbstractUsernameFormAuthenticator.java | 2 ++ .../test/java/org/keycloak/testsuite/forms/BruteForceTest.java | 1 + 2 files changed, 3 insertions(+) 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 b944149b8f..6b6da564f9 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 @@ -126,6 +126,7 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth context.getEvent().error(Errors.USER_DISABLED); Response challengeResponse = disabledUser(context); // this is not a failure so don't call failureChallenge. + //context.failureChallenge(AuthenticationFlowError.USER_DISABLED, challengeResponse); context.forceChallenge(challengeResponse); return false; } @@ -135,6 +136,7 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth 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; } 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 a10ffb6fec..a6c95fe11d 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 @@ -300,6 +300,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { // KEYCLOAK-5420 // Test to make sure that temporarily disabled doesn't increment failure count testingClient.testing().setTimeOffset(Collections.singletonMap("offset", String.valueOf(52))); + // should be unlocked now loginSuccess(); clearUserFailures(); clearAllUserFailures(); From 76cccc3f2bc3cec9fa81344c263bd914ffb06e5c Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Sat, 16 Dec 2017 08:29:58 -0500 Subject: [PATCH 4/7] fix more --- .../test/java/org/keycloak/testsuite/forms/BruteForceTest.java | 1 + 1 file changed, 1 insertion(+) 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 a6c95fe11d..54a975b401 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 @@ -305,6 +305,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { clearUserFailures(); clearAllUserFailures(); loginSuccess(); + testingClient.testing().setTimeOffset(Collections.singletonMap("offset", String.valueOf(0))); } @Test From 003f27e9bd6b51999b022a0c24a693b1ddfd3018 Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Sat, 16 Dec 2017 08:31:33 -0500 Subject: [PATCH 5/7] fix more --- .../test/java/org/keycloak/testsuite/forms/BruteForceTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 54a975b401..4202c95977 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 @@ -71,7 +71,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { testRealm.setMaxDeltaTimeSeconds(200); testRealm.setMaxFailureWaitSeconds(1000); testRealm.setWaitIncrementSeconds(50); - testRealm.setQuickLoginCheckMilliSeconds(0L); + //testRealm.setQuickLoginCheckMilliSeconds(0L); userId = user.getId(); From a27097e9ef0544bdc03e5751825f6a3fe75216bc Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Sat, 16 Dec 2017 11:28:15 -0500 Subject: [PATCH 6/7] reset defaults --- .../testsuite/forms/BruteForceTest.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) 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 4202c95977..020e1e1fc6 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 @@ -81,11 +81,39 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { @Before public void config() { + try { + clearUserFailures(); + clearAllUserFailures(); + RealmRepresentation realm = adminClient.realm("test").toRepresentation(); + realm.setFailureFactor(2); + realm.setMaxDeltaTimeSeconds(200); + realm.setMaxFailureWaitSeconds(1000); + realm.setWaitIncrementSeconds(50); + adminClient.realm("test").update(realm); + } catch (Exception e) { + throw new RuntimeException(e); + } + events.clear(); } @After public void slowItDown() throws Exception { + try { + clearUserFailures(); + clearAllUserFailures(); + RealmRepresentation realm = adminClient.realm("test").toRepresentation(); + realm.setMaxFailureWaitSeconds(900); + realm.setMinimumQuickLoginWaitSeconds(60); + realm.setWaitIncrementSeconds(60); + realm.setQuickLoginCheckMilliSeconds(1000L); + realm.setMaxDeltaTimeSeconds(60 * 60 * 12); // 12 hours + realm.setFailureFactor(30); + adminClient.realm("test").update(realm); + } catch (Exception e) { + throw new RuntimeException(e); + } + events.clear(); Thread.sleep(100); } From b5ae7e836da5eb65fa89c3728531e12249a97c4f Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Sat, 16 Dec 2017 13:18:41 -0500 Subject: [PATCH 7/7] smaller times --- .../keycloak/testsuite/forms/BruteForceTest.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) 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 020e1e1fc6..7b65a0c528 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 @@ -68,9 +68,9 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { testRealm.setBruteForceProtected(true); testRealm.setFailureFactor(2); - testRealm.setMaxDeltaTimeSeconds(200); - testRealm.setMaxFailureWaitSeconds(1000); - testRealm.setWaitIncrementSeconds(50); + testRealm.setMaxDeltaTimeSeconds(20); + testRealm.setMaxFailureWaitSeconds(100); + testRealm.setWaitIncrementSeconds(5); //testRealm.setQuickLoginCheckMilliSeconds(0L); userId = user.getId(); @@ -86,9 +86,9 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { clearAllUserFailures(); RealmRepresentation realm = adminClient.realm("test").toRepresentation(); realm.setFailureFactor(2); - realm.setMaxDeltaTimeSeconds(200); - realm.setMaxFailureWaitSeconds(1000); - realm.setWaitIncrementSeconds(50); + realm.setMaxDeltaTimeSeconds(20); + realm.setMaxFailureWaitSeconds(100); + realm.setWaitIncrementSeconds(5); adminClient.realm("test").update(realm); } catch (Exception e) { throw new RuntimeException(e); @@ -110,6 +110,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { realm.setMaxDeltaTimeSeconds(60 * 60 * 12); // 12 hours realm.setFailureFactor(30); adminClient.realm("test").update(realm); + testingClient.testing().setTimeOffset(Collections.singletonMap("offset", String.valueOf(0))); } catch (Exception e) { throw new RuntimeException(e); } @@ -327,7 +328,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { expectTemporarilyDisabled(); // KEYCLOAK-5420 // Test to make sure that temporarily disabled doesn't increment failure count - testingClient.testing().setTimeOffset(Collections.singletonMap("offset", String.valueOf(52))); + testingClient.testing().setTimeOffset(Collections.singletonMap("offset", String.valueOf(6))); // should be unlocked now loginSuccess(); clearUserFailures();