Skip to content

Commit

Permalink
KEYCLOAK-3091 Change brute force to use userId
Browse files Browse the repository at this point in the history
  • Loading branch information
@stianst
stianst committed Jun 13, 2016
1 parent 46b17e6 commit e538394
Show file tree
Hide file tree
Showing 19 changed files with 134 additions and 138 deletions.
10 changes: 5 additions & 5 deletions ...dmin-client/src/main/java/org/keycloak/admin/client/resource/AttackDetectionResource.java
View file
Expand Up @@ -33,16 +33,16 @@
public interface AttackDetectionResource {

@GET
@Path("brute-force/usernames/{username}")
@Path("brute-force/users/{userId}")
@NoCache
@Produces(MediaType.APPLICATION_JSON)
Map<String, Object> bruteForceUserStatus(@PathParam("username") String username);
Map<String, Object> 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();

Expand Down
20 changes: 10 additions & 10 deletions .../src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java
View file
Expand Up @@ -30,7 +30,7 @@
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;
Expand Down Expand Up @@ -377,24 +377,24 @@ protected void removeUserSessions(RealmModel realm, boolean offline) {
}

@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
Expand Down Expand Up @@ -538,8 +538,8 @@ ClientInitialAccessAdapter wrap(RealmModel realm, ClientInitialAccessEntity enti
}


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<ClientSessionModel> wrapClientSessions(RealmModel realm, Collection<ClientSessionEntity> entities, boolean offline) {
Expand Down
10 changes: 5 additions & 5 deletions ...finispan/UsernameLoginFailureAdapter.java → ...s/infinispan/UserLoginFailureAdapter.java
View file
Expand Up @@ -18,30 +18,30 @@
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 <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
*/
public class UsernameLoginFailureAdapter implements UsernameLoginFailureModel {
public class UserLoginFailureAdapter implements UserLoginFailureModel {

private InfinispanUserSessionProvider provider;
private Cache<LoginFailureKey, LoginFailureEntity> cache;
private LoginFailureKey key;
private LoginFailureEntity entity;

public UsernameLoginFailureAdapter(InfinispanUserSessionProvider provider, Cache<LoginFailureKey, LoginFailureEntity> cache, LoginFailureKey key, LoginFailureEntity entity) {
public UserLoginFailureAdapter(InfinispanUserSessionProvider provider, Cache<LoginFailureKey, LoginFailureEntity> cache, LoginFailureKey key, LoginFailureEntity entity) {
this.provider = provider;
this.cache = cache;
this.key = key;
this.entity = entity;
}

@Override
public String getUsername() {
return entity.getUsername();
public String getUserId() {
return entity.getUserId();
}

@Override
Expand Down
10 changes: 5 additions & 5 deletions ...an/src/main/java/org/keycloak/models/sessions/infinispan/entities/LoginFailureEntity.java
View file
Expand Up @@ -24,19 +24,19 @@
*/
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() {
Expand Down
10 changes: 5 additions & 5 deletions ...ispan/src/main/java/org/keycloak/models/sessions/infinispan/entities/LoginFailureKey.java
View file
Expand Up @@ -25,11 +25,11 @@
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
Expand All @@ -40,15 +40,15 @@ public boolean equals(Object o) {
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;
}

@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;
}

Expand Down
4 changes: 2 additions & 2 deletions ...oak/models/UsernameLoginFailureModel.java → ...eycloak/models/UserLoginFailureModel.java
View file
Expand Up @@ -21,9 +21,9 @@
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
* @version $Revision: 1 $
*/
public interface UsernameLoginFailureModel
public interface UserLoginFailureModel
{
String getUsername();
String getUserId();
int getFailedLoginNotBefore();
void setFailedLoginNotBefore(int notBefore);
int getNumFailures();
Expand Down
6 changes: 3 additions & 3 deletions server-spi/src/main/java/org/keycloak/models/UserSessionProvider.java
View file
Expand Up @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions server-spi/src/main/java/org/keycloak/services/managers/BruteForceProtector.java
View file
Expand Up @@ -20,14 +20,15 @@
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;

/**
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
* @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);
}
9 changes: 6 additions & 3 deletions services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java
View file
Expand Up @@ -37,6 +37,7 @@
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;
Expand Down Expand Up @@ -543,8 +544,10 @@ public void logFailure() {
if (username == null) {

} else {
getBruteForceProtector().failedLogin(realm, username, connection);

UserModel user = KeycloakModelUtils.findUserByNameOrEmail(session, realm, username);
if (user != null) {
getBruteForceProtector().failedLogin(realm, user, connection);
}
}
}
}
Expand Down Expand Up @@ -851,7 +854,7 @@ public void validateUser(UserModel authenticatedUser) {
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);
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java
View file
Expand Up @@ -100,7 +100,7 @@ public boolean enabledUser(AuthenticationFlowContext context, UserModel user) {
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);
Expand Down
2 changes: 1 addition & 1 deletion ...rc/main/java/org/keycloak/authentication/authenticators/directgrant/ValidateUsername.java
View file
Expand Up @@ -84,7 +84,7 @@ public void authenticate(AuthenticationFlowContext context) {
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");
Expand Down
48 changes: 24 additions & 24 deletions services/src/main/java/org/keycloak/services/managers/DefaultBruteForceProtector.java
View file
Expand Up @@ -22,7 +22,7 @@
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;
Expand Down Expand Up @@ -55,18 +55,18 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector

protected abstract class LoginEvent implements Comparable<LoginEvent> {
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);
}
}

Expand All @@ -79,8 +79,8 @@ public ShutdownEvent() {
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);
}
}

Expand All @@ -92,11 +92,11 @@ public void failure(KeycloakSession session, LoginEvent event) {
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();
Expand Down Expand Up @@ -135,10 +135,10 @@ public void failure(KeycloakSession session, LoginEvent event) {
}


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;
}
Expand Down Expand Up @@ -212,7 +212,7 @@ public void run() {
}

protected void logFailure(LoginEvent event) {
logger.loginFailure(event.username, event.ip);
logger.loginFailure(event.userId, event.ip);
failures++;
long delta = 0;
if (lastFailure > 0) {
Expand All @@ -227,9 +227,9 @@ protected void logFailure(LoginEvent event) {
}

@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
Expand All @@ -241,17 +241,17 @@ public void failedLogin(RealmModel realm, String username, ClientConnection clie
}

@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;
}

Expand Down

0 comments on commit e538394

Please sign in to comment.