From 7c4c77d70e0ac93a8fb631379b18d2d15056da8d Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Thu, 26 Nov 2015 20:17:11 +0100 Subject: [PATCH] KEYCLOAK-2147 --- .../managers/BruteForceProtector.java | 35 ++----------------- .../admin/AttackDetectionResource.java | 4 +-- .../resources/admin/UsersResource.java | 2 +- .../testsuite/forms/BruteForceTest.java | 28 ++++++++++++--- 4 files changed, 29 insertions(+), 40 deletions(-) diff --git a/services/src/main/java/org/keycloak/services/managers/BruteForceProtector.java b/services/src/main/java/org/keycloak/services/managers/BruteForceProtector.java index 8107462524..191f7fa986 100755 --- a/services/src/main/java/org/keycloak/services/managers/BruteForceProtector.java +++ b/services/src/main/java/org/keycloak/services/managers/BruteForceProtector.java @@ -53,12 +53,6 @@ public class BruteForceProtector implements Runnable { } } - protected class SuccessfulLogin extends LoginEvent { - public SuccessfulLogin(String realmId, String userId, String ip) { - super(realmId, userId, ip); - } - } - protected class ShutdownEvent extends LoginEvent { public ShutdownEvent() { super(null, null, null); @@ -83,7 +77,7 @@ public class BruteForceProtector implements Runnable { logFailure(event); UsernameLoginFailureModel user = getUserModel(session, event); if (user == null) { - user = session.sessions().addUserLoginFailure(realm, event.username); + user = session.sessions().addUserLoginFailure(realm, event.username.toLowerCase()); } user.setLastIPFailure(event.ip); long currentTime = System.currentTimeMillis(); @@ -122,7 +116,7 @@ public class BruteForceProtector implements Runnable { protected UsernameLoginFailureModel getUserModel(KeycloakSession session, LoginEvent event) { RealmModel realm = getRealmModel(session, event); if (realm == null) return null; - UsernameLoginFailureModel user = session.sessions().getUserLoginFailure(realm, event.username); + UsernameLoginFailureModel user = session.sessions().getUserLoginFailure(realm, event.username.toLowerCase()); if (user == null) return null; return user; } @@ -147,7 +141,6 @@ public class BruteForceProtector implements Runnable { } } - public void run() { final ArrayList events = new ArrayList(TRANSACTION_SIZE + 1); try { @@ -196,10 +189,6 @@ public class BruteForceProtector implements Runnable { } } - protected void logSuccess(LoginEvent event) { - logger.warn("login success for user " + event.username + " from ip " + event.ip); - } - protected void logFailure(LoginEvent event) { logger.warn("login failure for user " + event.username + " from ip " + event.ip); failures++; @@ -215,15 +204,6 @@ public class BruteForceProtector implements Runnable { } } - public void successfulLogin(RealmModel realm, String username, ClientConnection clientConnection) { - logger.info("successful login user: " + username + " from ip " + clientConnection.getRemoteAddr()); - } - - public void invalidUser(RealmModel realm, String username, ClientConnection clientConnection) { - logger.warn("invalid user: " + username + " from ip " + clientConnection.getRemoteAddr()); - // todo more? - } - public void failedLogin(RealmModel realm, String username, ClientConnection clientConnection) { try { FailedLogin event = new FailedLogin(realm.getId(), username, clientConnection.getRemoteAddr()); @@ -238,7 +218,7 @@ public class BruteForceProtector implements Runnable { } public boolean isTemporarilyDisabled(KeycloakSession session, RealmModel realm, String username) { - UsernameLoginFailureModel failure = session.sessions().getUserLoginFailure(realm, username); + UsernameLoginFailureModel failure = session.sessions().getUserLoginFailure(realm, username.toLowerCase()); if (failure == null) { return false; } @@ -251,13 +231,4 @@ public class BruteForceProtector implements Runnable { return false; } - public long getFailures() { - return failures; - } - - public long getLastFailure() { - return lastFailure; - } - - } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/AttackDetectionResource.java b/services/src/main/java/org/keycloak/services/resources/admin/AttackDetectionResource.java index 3cdf9880a3..bc77c336c5 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/AttackDetectionResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/AttackDetectionResource.java @@ -75,7 +75,7 @@ public class AttackDetectionResource { data.put("lastIPFailure", "n/a"); if (!realm.isBruteForceProtected()) return data; - UsernameLoginFailureModel model = session.sessions().getUserLoginFailure(realm, username); + UsernameLoginFailureModel model = session.sessions().getUserLoginFailure(realm, username.toLowerCase()); if (model == null) return data; if (protector.isTemporarilyDisabled(session, realm, username)) { data.put("disabled", true); @@ -97,7 +97,7 @@ public class AttackDetectionResource { @DELETE public void clearBruteForceForUser(@PathParam("username") String username) { auth.requireManage(); - UsernameLoginFailureModel model = session.sessions().getUserLoginFailure(realm, username); + UsernameLoginFailureModel model = session.sessions().getUserLoginFailure(realm, username.toLowerCase()); if (model != null) { session.sessions().removeUserLoginFailure(realm, username); adminEvent.operation(OperationType.DELETE).success(); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java index 901de1f7b2..73031ed3b5 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java @@ -144,7 +144,7 @@ public class UsersResource { } if (rep.isEnabled() != null && rep.isEnabled()) { - UsernameLoginFailureModel failureModel = session.sessions().getUserLoginFailure(realm, rep.getUsername()); + UsernameLoginFailureModel failureModel = session.sessions().getUserLoginFailure(realm, rep.getUsername().toLowerCase()); if (failureModel != null) { failureModel.clearFailures(); } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java index 8f0778cfe4..4e654aa189 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java @@ -302,6 +302,15 @@ public class BruteForceTest { loginSuccess(); } + @Test + public void testBrowserInvalidPasswordDifferentCase() throws Exception { + loginSuccess("test-user@localhost"); + loginInvalidPassword("test-User@localhost"); + loginInvalidPassword("Test-user@localhost"); + expectTemporarilyDisabled(); + clearAllUserFailures(); + } + @Test public void testBrowserMissingPassword() throws Exception { loginSuccess(); @@ -333,8 +342,12 @@ public class BruteForceTest { } public void expectTemporarilyDisabled() throws Exception { + expectTemporarilyDisabled("test-user@localhost"); + } + + public void expectTemporarilyDisabled(String username) throws Exception { loginPage.open(); - loginPage.login("test-user@localhost", "password"); + loginPage.login(username, "password"); loginPage.assertCurrent(); String src = driver.getPageSource(); @@ -345,9 +358,11 @@ public class BruteForceTest { .assertEvent(); } - - public void loginSuccess() throws Exception { + loginSuccess("test-user@localhost"); + } + + public void loginSuccess(String username) throws Exception { loginPage.open(); loginPage.login("test-user@localhost", "password"); @@ -391,10 +406,13 @@ public class BruteForceTest { events.clear(); } - public void loginInvalidPassword() throws Exception { + loginInvalidPassword("test-user@localhost"); + } + + public void loginInvalidPassword(String username) throws Exception { loginPage.open(); - loginPage.login("test-user@localhost", "invalid"); + loginPage.login(username, "invalid"); loginPage.assertCurrent();