From e538394e600c727608f58f8643ccc0cf71aba9cf Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Mon, 13 Jun 2016 12:46:18 +0200 Subject: [PATCH] KEYCLOAK-3091 Change brute force to use userId --- .../resource/AttackDetectionResource.java | 10 +-- .../InfinispanUserSessionProvider.java | 20 +++--- ...pter.java => UserLoginFailureAdapter.java} | 10 +-- .../entities/LoginFailureEntity.java | 10 +-- .../infinispan/entities/LoginFailureKey.java | 10 +-- ...eModel.java => UserLoginFailureModel.java} | 4 +- .../keycloak/models/UserSessionProvider.java | 6 +- .../managers/BruteForceProtector.java | 5 +- .../AuthenticationProcessor.java | 9 ++- .../AbstractUsernameFormAuthenticator.java | 2 +- .../directgrant/ValidateUsername.java | 2 +- .../managers/DefaultBruteForceProtector.java | 48 +++++++------- .../admin/AttackDetectionResource.java | 27 ++++---- .../resources/admin/UsersResource.java | 12 ++-- .../admin/AttackDetectionResourceTest.java | 18 +++--- .../testsuite/forms/BruteForceTest.java | 63 +++++++++---------- .../model/UserSessionProviderTest.java | 6 +- .../admin/resources/js/controllers/users.js | 4 +- .../theme/base/admin/resources/js/services.js | 6 +- 19 files changed, 134 insertions(+), 138 deletions(-) rename model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/{UsernameLoginFailureAdapter.java => UserLoginFailureAdapter.java} (86%) rename server-spi/src/main/java/org/keycloak/models/{UsernameLoginFailureModel.java => UserLoginFailureModel.java} (94%) diff --git a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/AttackDetectionResource.java b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/AttackDetectionResource.java index 7888cad94d..ea77759dc3 100644 --- a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/AttackDetectionResource.java +++ b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/AttackDetectionResource.java @@ -33,16 +33,16 @@ import java.util.Map; public interface AttackDetectionResource { @GET - @Path("brute-force/usernames/{username}") + @Path("brute-force/users/{userId}") @NoCache @Produces(MediaType.APPLICATION_JSON) - Map bruteForceUserStatus(@PathParam("username") String username); + Map bruteForceUserStatus(@PathParam("userId") String userId); - @Path("brute-force/usernames/{username}") + @Path("brute-force/users/{userId}") @DELETE - void clearBruteForceForUser(@PathParam("username") String username); + void clearBruteForceForUser(@PathParam("userId") String userId); - @Path("brute-force/usernames") + @Path("brute-force/users") @DELETE void clearAllBruteForce(); diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java index 10d6205367..960ea250b0 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java @@ -30,7 +30,7 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.UserSessionModel; import org.keycloak.models.UserSessionProvider; -import org.keycloak.models.UsernameLoginFailureModel; +import org.keycloak.models.UserLoginFailureModel; import org.keycloak.models.session.UserSessionPersisterProvider; import org.keycloak.models.sessions.infinispan.entities.ClientInitialAccessEntity; import org.keycloak.models.sessions.infinispan.entities.ClientSessionEntity; @@ -377,24 +377,24 @@ public class InfinispanUserSessionProvider implements UserSessionProvider { } @Override - public UsernameLoginFailureModel getUserLoginFailure(RealmModel realm, String username) { - LoginFailureKey key = new LoginFailureKey(realm.getId(), username); + public UserLoginFailureModel getUserLoginFailure(RealmModel realm, String userId) { + LoginFailureKey key = new LoginFailureKey(realm.getId(), userId); return wrap(key, loginFailureCache.get(key)); } @Override - public UsernameLoginFailureModel addUserLoginFailure(RealmModel realm, String username) { - LoginFailureKey key = new LoginFailureKey(realm.getId(), username); + public UserLoginFailureModel addUserLoginFailure(RealmModel realm, String userId) { + LoginFailureKey key = new LoginFailureKey(realm.getId(), userId); LoginFailureEntity entity = new LoginFailureEntity(); entity.setRealm(realm.getId()); - entity.setUsername(username); + entity.setUserId(userId); tx.put(loginFailureCache, key, entity); return wrap(key, entity); } @Override - public void removeUserLoginFailure(RealmModel realm, String username) { - tx.remove(loginFailureCache, new LoginFailureKey(realm.getId(), username)); + public void removeUserLoginFailure(RealmModel realm, String userId) { + tx.remove(loginFailureCache, new LoginFailureKey(realm.getId(), userId)); } @Override @@ -538,8 +538,8 @@ public class InfinispanUserSessionProvider implements UserSessionProvider { } - UsernameLoginFailureModel wrap(LoginFailureKey key, LoginFailureEntity entity) { - return entity != null ? new UsernameLoginFailureAdapter(this, loginFailureCache, key, entity) : null; + UserLoginFailureModel wrap(LoginFailureKey key, LoginFailureEntity entity) { + return entity != null ? new UserLoginFailureAdapter(this, loginFailureCache, key, entity) : null; } List wrapClientSessions(RealmModel realm, Collection entities, boolean offline) { diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/UsernameLoginFailureAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/UserLoginFailureAdapter.java similarity index 86% rename from model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/UsernameLoginFailureAdapter.java rename to model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/UserLoginFailureAdapter.java index e2a92ac3da..a41777dc43 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/UsernameLoginFailureAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/UserLoginFailureAdapter.java @@ -18,21 +18,21 @@ package org.keycloak.models.sessions.infinispan; import org.infinispan.Cache; -import org.keycloak.models.UsernameLoginFailureModel; +import org.keycloak.models.UserLoginFailureModel; import org.keycloak.models.sessions.infinispan.entities.LoginFailureEntity; import org.keycloak.models.sessions.infinispan.entities.LoginFailureKey; /** * @author Stian Thorgersen */ -public class UsernameLoginFailureAdapter implements UsernameLoginFailureModel { +public class UserLoginFailureAdapter implements UserLoginFailureModel { private InfinispanUserSessionProvider provider; private Cache cache; private LoginFailureKey key; private LoginFailureEntity entity; - public UsernameLoginFailureAdapter(InfinispanUserSessionProvider provider, Cache cache, LoginFailureKey key, LoginFailureEntity entity) { + public UserLoginFailureAdapter(InfinispanUserSessionProvider provider, Cache cache, LoginFailureKey key, LoginFailureEntity entity) { this.provider = provider; this.cache = cache; this.key = key; @@ -40,8 +40,8 @@ public class UsernameLoginFailureAdapter implements UsernameLoginFailureModel { } @Override - public String getUsername() { - return entity.getUsername(); + public String getUserId() { + return entity.getUserId(); } @Override diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/entities/LoginFailureEntity.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/entities/LoginFailureEntity.java index 65db3a83e3..d25f58b831 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/entities/LoginFailureEntity.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/entities/LoginFailureEntity.java @@ -24,19 +24,19 @@ import java.io.Serializable; */ public class LoginFailureEntity implements Serializable { - private String username; + private String userId; private String realm; private int failedLoginNotBefore; private int numFailures; private long lastFailure; private String lastIPFailure; - public String getUsername() { - return username; + public String getUserId() { + return userId; } - public void setUsername(String username) { - this.username = username; + public void setUserId(String userId) { + this.userId = userId; } public String getRealm() { diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/entities/LoginFailureKey.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/entities/LoginFailureKey.java index 21dc463eb8..c45237963b 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/entities/LoginFailureKey.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/entities/LoginFailureKey.java @@ -25,11 +25,11 @@ import java.io.Serializable; public class LoginFailureKey implements Serializable { private final String realm; - private final String username; + private final String userId; - public LoginFailureKey(String realm, String username) { + public LoginFailureKey(String realm, String userId) { this.realm = realm; - this.username = username; + this.userId = userId; } @Override @@ -40,7 +40,7 @@ public class LoginFailureKey implements Serializable { LoginFailureKey key = (LoginFailureKey) o; if (realm != null ? !realm.equals(key.realm) : key.realm != null) return false; - if (username != null ? !username.equals(key.username) : key.username != null) return false; + if (userId != null ? !userId.equals(key.userId) : key.userId != null) return false; return true; } @@ -48,7 +48,7 @@ public class LoginFailureKey implements Serializable { @Override public int hashCode() { int result = realm != null ? realm.hashCode() : 0; - result = 31 * result + (username != null ? username.hashCode() : 0); + result = 31 * result + (userId != null ? userId.hashCode() : 0); return result; } diff --git a/server-spi/src/main/java/org/keycloak/models/UsernameLoginFailureModel.java b/server-spi/src/main/java/org/keycloak/models/UserLoginFailureModel.java similarity index 94% rename from server-spi/src/main/java/org/keycloak/models/UsernameLoginFailureModel.java rename to server-spi/src/main/java/org/keycloak/models/UserLoginFailureModel.java index 969bbbd343..7957ad98b4 100755 --- a/server-spi/src/main/java/org/keycloak/models/UsernameLoginFailureModel.java +++ b/server-spi/src/main/java/org/keycloak/models/UserLoginFailureModel.java @@ -21,9 +21,9 @@ package org.keycloak.models; * @author Bill Burke * @version $Revision: 1 $ */ -public interface UsernameLoginFailureModel +public interface UserLoginFailureModel { - String getUsername(); + String getUserId(); int getFailedLoginNotBefore(); void setFailedLoginNotBefore(int notBefore); int getNumFailures(); diff --git a/server-spi/src/main/java/org/keycloak/models/UserSessionProvider.java b/server-spi/src/main/java/org/keycloak/models/UserSessionProvider.java index d824584fbe..585558c102 100755 --- a/server-spi/src/main/java/org/keycloak/models/UserSessionProvider.java +++ b/server-spi/src/main/java/org/keycloak/models/UserSessionProvider.java @@ -48,9 +48,9 @@ public interface UserSessionProvider extends Provider { void removeUserSessions(RealmModel realm); void removeClientSession(RealmModel realm, ClientSessionModel clientSession); - UsernameLoginFailureModel getUserLoginFailure(RealmModel realm, String username); - UsernameLoginFailureModel addUserLoginFailure(RealmModel realm, String username); - void removeUserLoginFailure(RealmModel realm, String username); + UserLoginFailureModel getUserLoginFailure(RealmModel realm, String userId); + UserLoginFailureModel addUserLoginFailure(RealmModel realm, String userId); + void removeUserLoginFailure(RealmModel realm, String userId); void removeAllUserLoginFailures(RealmModel realm); void onRealmRemoved(RealmModel realm); diff --git a/server-spi/src/main/java/org/keycloak/services/managers/BruteForceProtector.java b/server-spi/src/main/java/org/keycloak/services/managers/BruteForceProtector.java index 33ba7749e3..e884b02cdf 100755 --- a/server-spi/src/main/java/org/keycloak/services/managers/BruteForceProtector.java +++ b/server-spi/src/main/java/org/keycloak/services/managers/BruteForceProtector.java @@ -20,6 +20,7 @@ package org.keycloak.services.managers; import org.keycloak.common.ClientConnection; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; +import org.keycloak.models.UserModel; import org.keycloak.provider.Provider; /** @@ -27,7 +28,7 @@ import org.keycloak.provider.Provider; * @version $Revision: 1 $ */ public interface BruteForceProtector extends Provider { - void failedLogin(RealmModel realm, String username, ClientConnection clientConnection); + void failedLogin(RealmModel realm, UserModel user, ClientConnection clientConnection); - boolean isTemporarilyDisabled(KeycloakSession session, RealmModel realm, String username); + boolean isTemporarilyDisabled(KeycloakSession session, RealmModel realm, UserModel user); } diff --git a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java index 68c0620fb3..cd921bbb11 100755 --- a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java +++ b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java @@ -37,6 +37,7 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.UserSessionModel; import org.keycloak.models.utils.FormMessage; +import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.protocol.LoginProtocol; import org.keycloak.protocol.LoginProtocol.Error; import org.keycloak.protocol.oidc.TokenManager; @@ -543,8 +544,10 @@ public class AuthenticationProcessor { if (username == null) { } else { - getBruteForceProtector().failedLogin(realm, username, connection); - + UserModel user = KeycloakModelUtils.findUserByNameOrEmail(session, realm, username); + if (user != null) { + getBruteForceProtector().failedLogin(realm, user, connection); + } } } } @@ -851,7 +854,7 @@ public class AuthenticationProcessor { if (authenticatedUser == null) return; if (!authenticatedUser.isEnabled()) throw new AuthenticationFlowException(AuthenticationFlowError.USER_DISABLED); if (realm.isBruteForceProtected()) { - if (getBruteForceProtector().isTemporarilyDisabled(session, realm, authenticatedUser.getUsername())) { + if (getBruteForceProtector().isTemporarilyDisabled(session, realm, authenticatedUser)) { throw new AuthenticationFlowException(AuthenticationFlowError.USER_TEMPORARILY_DISABLED); } } 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 137e37086a..5046534ea2 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 @@ -100,7 +100,7 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth return false; } if (context.getRealm().isBruteForceProtected()) { - if (context.getProtector().isTemporarilyDisabled(context.getSession(), context.getRealm(), user.getUsername())) { + if (context.getProtector().isTemporarilyDisabled(context.getSession(), context.getRealm(), user)) { context.getEvent().user(user); context.getEvent().error(Errors.USER_TEMPORARILY_DISABLED); Response challengeResponse = temporarilyDisabledUser(context); diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/directgrant/ValidateUsername.java b/services/src/main/java/org/keycloak/authentication/authenticators/directgrant/ValidateUsername.java index 26aeaebc13..de48a3c281 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/directgrant/ValidateUsername.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/directgrant/ValidateUsername.java @@ -84,7 +84,7 @@ public class ValidateUsername extends AbstractDirectGrantAuthenticator { return; } if (context.getRealm().isBruteForceProtected()) { - if (context.getProtector().isTemporarilyDisabled(context.getSession(), context.getRealm(), user.getUsername())) { + if (context.getProtector().isTemporarilyDisabled(context.getSession(), context.getRealm(), user)) { context.getEvent().user(user); context.getEvent().error(Errors.USER_TEMPORARILY_DISABLED); Response challengeResponse = errorResponse(Response.Status.BAD_REQUEST.getStatusCode(), "invalid_grant", "Account temporarily disabled"); diff --git a/services/src/main/java/org/keycloak/services/managers/DefaultBruteForceProtector.java b/services/src/main/java/org/keycloak/services/managers/DefaultBruteForceProtector.java index 665dcd31e5..fa3b1d5d6b 100644 --- a/services/src/main/java/org/keycloak/services/managers/DefaultBruteForceProtector.java +++ b/services/src/main/java/org/keycloak/services/managers/DefaultBruteForceProtector.java @@ -22,7 +22,7 @@ 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.models.UserLoginFailureModel; import org.keycloak.services.ServicesLogger; import java.util.ArrayList; @@ -55,18 +55,18 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector protected abstract class LoginEvent implements Comparable { protected final String realmId; - protected final String username; + protected final String userId; protected final String ip; - protected LoginEvent(String realmId, String username, String ip) { + protected LoginEvent(String realmId, String userId, String ip) { this.realmId = realmId; - this.username = username; + this.userId = userId; this.ip = ip; } @Override public int compareTo(LoginEvent o) { - return username.compareTo(o.username); + return userId.compareTo(o.userId); } } @@ -79,8 +79,8 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector protected class FailedLogin extends LoginEvent { protected final CountDownLatch latch = new CountDownLatch(1); - public FailedLogin(String realmId, String username, String ip) { - super(realmId, username, ip); + public FailedLogin(String realmId, String userId, String ip) { + super(realmId, userId, ip); } } @@ -92,11 +92,11 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector logger.debug("failure"); RealmModel realm = getRealmModel(session, event); logFailure(event); - UserModel user = session.users().getUserByUsername(event.username.toString(), realm); - UsernameLoginFailureModel userLoginFailure = getUserModel(session, event); + UserModel user = session.users().getUserById(event.userId, realm); + UserLoginFailureModel userLoginFailure = getUserModel(session, event); if (user != null) { if (userLoginFailure == null) { - userLoginFailure = session.sessions().addUserLoginFailure(realm, event.username.toLowerCase()); + userLoginFailure = session.sessions().addUserLoginFailure(realm, event.userId); } userLoginFailure.setLastIPFailure(event.ip); long currentTime = System.currentTimeMillis(); @@ -135,10 +135,10 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector } - protected UsernameLoginFailureModel getUserModel(KeycloakSession session, LoginEvent event) { + protected UserLoginFailureModel getUserModel(KeycloakSession session, LoginEvent event) { RealmModel realm = getRealmModel(session, event); if (realm == null) return null; - UsernameLoginFailureModel user = session.sessions().getUserLoginFailure(realm, event.username.toLowerCase()); + UserLoginFailureModel user = session.sessions().getUserLoginFailure(realm, event.userId); if (user == null) return null; return user; } @@ -212,7 +212,7 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector } protected void logFailure(LoginEvent event) { - logger.loginFailure(event.username, event.ip); + logger.loginFailure(event.userId, event.ip); failures++; long delta = 0; if (lastFailure > 0) { @@ -227,9 +227,9 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector } @Override - public void failedLogin(RealmModel realm, String username, ClientConnection clientConnection) { + public void failedLogin(RealmModel realm, UserModel user, ClientConnection clientConnection) { try { - FailedLogin event = new FailedLogin(realm.getId(), username, clientConnection.getRemoteAddr()); + FailedLogin event = new FailedLogin(realm.getId(), user.getId(), clientConnection.getRemoteAddr()); queue.offer(event); // wait a minimum of seconds for type to process so that a hacker // cannot flood with failed logins and overwhelm the queue and not have notBefore updated to block next requests @@ -241,17 +241,17 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector } @Override - public boolean isTemporarilyDisabled(KeycloakSession session, RealmModel realm, String username) { - UsernameLoginFailureModel failure = session.sessions().getUserLoginFailure(realm, username.toLowerCase()); - if (failure == null) { - return false; + public boolean isTemporarilyDisabled(KeycloakSession session, RealmModel realm, UserModel user) { + UserLoginFailureModel failure = session.sessions().getUserLoginFailure(realm, user.getId()); + + if (failure != null) { + int currTime = (int) (System.currentTimeMillis() / 1000); + if (currTime < failure.getFailedLoginNotBefore()) { + logger.debugv("Current: {0} notBefore: {1}", currTime, failure.getFailedLoginNotBefore()); + return true; + } } - int currTime = (int)(System.currentTimeMillis()/1000); - if (currTime < failure.getFailedLoginNotBefore()) { - logger.debugv("Current: {0} notBefore: {1}", currTime , failure.getFailedLoginNotBefore()); - return true; - } return false; } 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 6ffa12cbf1..18b6d43eb1 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 @@ -21,7 +21,8 @@ import org.keycloak.common.ClientConnection; import org.keycloak.events.admin.OperationType; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; -import org.keycloak.models.UsernameLoginFailureModel; +import org.keycloak.models.UserModel; +import org.keycloak.models.UserLoginFailureModel; import org.keycloak.services.ServicesLogger; import org.keycloak.services.managers.BruteForceProtector; @@ -72,14 +73,14 @@ public class AttackDetectionResource { /** * Get status of a username in brute force detection * - * @param username + * @param userId * @return */ @GET - @Path("brute-force/usernames/{username}") + @Path("brute-force/users/{userId}") @NoCache @Produces(MediaType.APPLICATION_JSON) - public Map bruteForceUserStatus(@PathParam("username") String username) { + public Map bruteForceUserStatus(@PathParam("userId") String userId) { auth.requireView(); Map data = new HashMap<>(); @@ -89,9 +90,11 @@ public class AttackDetectionResource { data.put("lastIPFailure", "n/a"); if (!realm.isBruteForceProtected()) return data; - UsernameLoginFailureModel model = session.sessions().getUserLoginFailure(realm, username.toLowerCase()); + UserModel user = session.users().getUserById(userId, realm); + + UserLoginFailureModel model = session.sessions().getUserLoginFailure(realm, userId); if (model == null) return data; - if (session.getProvider(BruteForceProtector.class).isTemporarilyDisabled(session, realm, username)) { + if (session.getProvider(BruteForceProtector.class).isTemporarilyDisabled(session, realm, user)) { data.put("disabled", true); } data.put("numFailures", model.getNumFailures()); @@ -105,16 +108,16 @@ public class AttackDetectionResource { * * This can release temporary disabled user * - * @param username + * @param userId */ - @Path("brute-force/usernames/{username}") + @Path("brute-force/users/{userId}") @DELETE - public void clearBruteForceForUser(@PathParam("username") String username) { + public void clearBruteForceForUser(@PathParam("userId") String userId) { auth.requireManage(); - UsernameLoginFailureModel model = session.sessions().getUserLoginFailure(realm, username.toLowerCase()); + UserLoginFailureModel model = session.sessions().getUserLoginFailure(realm, userId); if (model != null) { - session.sessions().removeUserLoginFailure(realm, username); + session.sessions().removeUserLoginFailure(realm, userId); adminEvent.operation(OperationType.DELETE).resourcePath(uriInfo).success(); } } @@ -125,7 +128,7 @@ public class AttackDetectionResource { * This can release temporary disabled users * */ - @Path("brute-force/usernames") + @Path("brute-force/users") @DELETE public void clearAllBruteForce() { auth.requireManage(); 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 7441517756..1b4586b873 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 @@ -81,7 +81,6 @@ import javax.ws.rs.core.UriBuilder; import javax.ws.rs.core.UriInfo; import javax.ws.rs.WebApplicationException; -import java.io.IOException; import java.net.URI; import java.text.MessageFormat; import java.util.ArrayList; @@ -94,15 +93,12 @@ import java.util.Map; import java.util.Properties; import java.util.Set; import java.util.concurrent.TimeUnit; -import org.keycloak.models.UsernameLoginFailureModel; +import org.keycloak.models.UserLoginFailureModel; import org.keycloak.services.managers.BruteForceProtector; import org.keycloak.services.managers.UserSessionManager; import org.keycloak.services.resources.AccountService; import org.keycloak.common.util.Time; import org.keycloak.services.validation.Validation; -import org.keycloak.theme.Theme; -import org.keycloak.theme.Theme.Type; -import org.keycloak.theme.ThemeProvider; /** * Base resource for managing users @@ -166,8 +162,8 @@ public class UsersResource { attrsToRemove = Collections.emptySet(); } - if (rep.isEnabled() != null && rep.isEnabled() && rep.getUsername() != null) { - UsernameLoginFailureModel failureModel = session.sessions().getUserLoginFailure(realm, rep.getUsername().toLowerCase()); + if (rep.isEnabled() != null && rep.isEnabled()) { + UserLoginFailureModel failureModel = session.sessions().getUserLoginFailure(realm, id); if (failureModel != null) { failureModel.clearFailures(); } @@ -300,7 +296,7 @@ public class UsersResource { rep.setFederatedIdentities(reps); } - if (session.getProvider(BruteForceProtector.class).isTemporarilyDisabled(session, realm, rep.getUsername())) { + if (session.getProvider(BruteForceProtector.class).isTemporarilyDisabled(session, realm, user)) { rep.setEnabled(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 d39f43cd68..008919294f 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 @@ -52,7 +52,7 @@ public class AttackDetectionResourceTest extends AbstractAdminTest { public void test() { AttackDetectionResource detection = adminClient.realm("test").attackDetection(); - assertBruteForce(detection.bruteForceUserStatus("test-user@localhost"), 0, false, false); + assertBruteForce(detection.bruteForceUserStatus(findUser("test-user@localhost").getId()), 0, false, false); oauthClient.doLogin("test-user@localhost", "invalid"); oauthClient.doLogin("test-user@localhost", "invalid"); @@ -62,21 +62,21 @@ public class AttackDetectionResourceTest extends AbstractAdminTest { oauthClient.doLogin("test-user2", "invalid"); oauthClient.doLogin("nosuchuser", "invalid"); - assertBruteForce(detection.bruteForceUserStatus("test-user@localhost"), 3, true, true); - assertBruteForce(detection.bruteForceUserStatus("test-user2"), 2, true, true); + assertBruteForce(detection.bruteForceUserStatus(findUser("test-user@localhost").getId()), 3, true, true); + assertBruteForce(detection.bruteForceUserStatus(findUser("test-user2").getId()), 2, true, true); assertBruteForce(detection.bruteForceUserStatus("nosuchuser"), 0, false, false); - detection.clearBruteForceForUser("test-user@localhost"); - assertAdminEvents.assertEvent("test", OperationType.DELETE, AdminEventPaths.attackDetectionClearBruteForceForUserPath("test-user@localhost")); + detection.clearBruteForceForUser(findUser("test-user@localhost").getId()); + assertAdminEvents.assertEvent("test", OperationType.DELETE, AdminEventPaths.attackDetectionClearBruteForceForUserPath(findUser("test-user@localhost").getId())); - assertBruteForce(detection.bruteForceUserStatus("test-user@localhost"), 0, false, false); - assertBruteForce(detection.bruteForceUserStatus("test-user2"), 2, true, true); + assertBruteForce(detection.bruteForceUserStatus(findUser("test-user@localhost").getId()), 0, false, false); + assertBruteForce(detection.bruteForceUserStatus(findUser("test-user2").getId()), 2, true, true); detection.clearAllBruteForce(); assertAdminEvents.assertEvent("test", OperationType.DELETE, AdminEventPaths.attackDetectionClearAllBruteForcePath()); - assertBruteForce(detection.bruteForceUserStatus("test-user@localhost"), 0, false, false); - assertBruteForce(detection.bruteForceUserStatus("test-user2"), 0, false, false); + assertBruteForce(detection.bruteForceUserStatus(findUser("test-user@localhost").getId()), 0, false, false); + assertBruteForce(detection.bruteForceUserStatus(findUser("test-user2").getId()), 0, false, false); } private void assertBruteForce(Map status, Integer expectedNumFailures, Boolean expectedFailure, Boolean expectedDisabled) { 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 a22c359d59..0d6de842d3 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 @@ -26,24 +26,21 @@ import org.keycloak.models.Constants; import org.keycloak.models.utils.TimeBasedOTP; import org.keycloak.representations.idm.CredentialRepresentation; import org.keycloak.testsuite.AssertEvents; +import org.keycloak.testsuite.AssertEvents.ExpectedEvent; 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 javax.ws.rs.client.Client; -import javax.ws.rs.client.ClientBuilder; -import javax.ws.rs.core.HttpHeaders; -import javax.ws.rs.core.Response; import java.net.MalformedURLException; import org.jboss.arquillian.graphene.page.Page; -import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.TestRealmKeycloakTest; import org.keycloak.testsuite.util.GreenMailRule; import org.keycloak.testsuite.util.OAuthClient; +import org.keycloak.testsuite.util.UserBuilder; /** * @author Stian Thorgersen @@ -64,6 +61,8 @@ public class BruteForceTest extends TestRealmKeycloakTest { testRealm.setFailureFactor(2); findClientInRealmRep(testRealm, "test-app").setDirectAccessGrantsEnabled(true); + + testRealm.getUsers().add(UserBuilder.create().username("user2").email("user2@localhost").password("password").build()); } @Before @@ -110,33 +109,11 @@ public class BruteForceTest extends TestRealmKeycloakTest { } protected void clearUserFailures() throws Exception { - String token = getAdminToken(); - Client client = ClientBuilder.newClient(); - Response response = client.target(AppPage.AUTH_SERVER_URL) - .path("admin/realms/test/attack-detection/brute-force/usernames/test-user@localhost") - .request() - .header(HttpHeaders.AUTHORIZATION, "Bearer " + token) - .delete(); - Assert.assertEquals(204, response.getStatus()); - response.close(); - client.close(); - - + adminClient.realm("test").attackDetection().clearBruteForceForUser(findUser("test-user@localhost").getId()); } protected void clearAllUserFailures() throws Exception { - String token = getAdminToken(); - Client client = ClientBuilder.newClient(); - Response response = client.target(AppPage.AUTH_SERVER_URL) - .path("admin/realms/test/attack-detection/brute-force/usernames") - .request() - .header(HttpHeaders.AUTHORIZATION, "Bearer " + token) - .delete(); - Assert.assertEquals(204, response.getStatus()); - response.close(); - client.close(); - - + adminClient.realm("test").attackDetection().clearAllBruteForce(); } @Test @@ -292,6 +269,17 @@ public class BruteForceTest extends TestRealmKeycloakTest { clearAllUserFailures(); } + @Test + public void testEmail() throws Exception { + String userId = adminClient.realm("test").users().search("user2", null, null, null, 0, 1).get(0).getId(); + + loginSuccess("user2@localhost"); + loginInvalidPassword("user2@localhost"); + loginInvalidPassword("user2@localhost"); + expectTemporarilyDisabled("user2@localhost", userId); + clearAllUserFailures(); + } + @Test public void testBrowserMissingPassword() throws Exception { loginSuccess(); @@ -334,20 +322,25 @@ public class BruteForceTest extends TestRealmKeycloakTest { } public void expectTemporarilyDisabled() throws Exception { - expectTemporarilyDisabled("test-user@localhost"); + expectTemporarilyDisabled("test-user@localhost", null); } - public void expectTemporarilyDisabled(String username) throws Exception { + public void expectTemporarilyDisabled(String username, String userId) throws Exception { loginPage.open(); loginPage.login(username, "password"); loginPage.assertCurrent(); String src = driver.getPageSource(); Assert.assertEquals("Invalid username or password.", loginPage.getError()); - events.expectLogin().session((String) null).error(Errors.USER_TEMPORARILY_DISABLED) - .detail(Details.USERNAME, "test-user@localhost") - .removeDetail(Details.CONSENT) - .assertEvent(); + ExpectedEvent event = events.expectLogin() + .session((String) null) + .error(Errors.USER_TEMPORARILY_DISABLED) + .detail(Details.USERNAME, username) + .removeDetail(Details.CONSENT); + if(userId != null) { + event.user(userId); + } + event.assertEvent(); } public void loginSuccess() throws Exception { diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserSessionProviderTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserSessionProviderTest.java index 3da7d232d5..0a5842ce12 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserSessionProviderTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserSessionProviderTest.java @@ -28,7 +28,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.UserSessionModel; -import org.keycloak.models.UsernameLoginFailureModel; +import org.keycloak.models.UserLoginFailureModel; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.services.managers.UserManager; import org.keycloak.testsuite.rule.KeycloakRule; @@ -471,10 +471,10 @@ public class UserSessionProviderTest { @Test public void loginFailures() { - UsernameLoginFailureModel failure1 = session.sessions().addUserLoginFailure(realm, "user1"); + UserLoginFailureModel failure1 = session.sessions().addUserLoginFailure(realm, "user1"); failure1.incrementFailures(); - UsernameLoginFailureModel failure2 = session.sessions().addUserLoginFailure(realm, "user2"); + UserLoginFailureModel failure2 = session.sessions().addUserLoginFailure(realm, "user2"); failure2.incrementFailures(); failure2.incrementFailures(); diff --git a/themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js b/themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js index 512f55a970..8fb01cf3e9 100755 --- a/themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js +++ b/themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js @@ -365,7 +365,7 @@ module.controller('UserDetailCtrl', function($scope, realm, user, BruteForceUser console.log('realm brute force? ' + realm.bruteForceProtected) $scope.temporarilyDisabled = false; var isDisabled = function () { - BruteForceUser.get({realm: realm.realm, username: user.username}, function(data) { + BruteForceUser.get({realm: realm.realm, userId: user.id}, function(data) { console.log('here in isDisabled ' + data.disabled); $scope.temporarilyDisabled = data.disabled; }); @@ -375,7 +375,7 @@ module.controller('UserDetailCtrl', function($scope, realm, user, BruteForceUser isDisabled(); $scope.unlockUser = function() { - BruteForceUser.delete({realm: realm.realm, username: user.username}, function(data) { + BruteForceUser.delete({realm: realm.realm, userId: user.id}, function(data) { isDisabled(); }); } diff --git a/themes/src/main/resources/theme/base/admin/resources/js/services.js b/themes/src/main/resources/theme/base/admin/resources/js/services.js index a19b7384a6..11e9d592c0 100755 --- a/themes/src/main/resources/theme/base/admin/resources/js/services.js +++ b/themes/src/main/resources/theme/base/admin/resources/js/services.js @@ -227,15 +227,15 @@ module.factory('RealmAdminEvents', function($resource) { }); module.factory('BruteForce', function($resource) { - return $resource(authUrl + '/admin/realms/:realm/attack-detection/brute-force/usernames', { + return $resource(authUrl + '/admin/realms/:realm/attack-detection/brute-force/users', { realm : '@realm' }); }); module.factory('BruteForceUser', function($resource) { - return $resource(authUrl + '/admin/realms/:realm/attack-detection/brute-force/usernames/:username', { + return $resource(authUrl + '/admin/realms/:realm/attack-detection/brute-force/users/:userId', { realm : '@realm', - username : '@username' + userId : '@userId' }); });