KEYCLOAK-17835 Account Permanent Lockout and login error messages

This commit is contained in:
Václav Muzikář 2021-04-27 17:44:08 +02:00 committed by Marek Posolda
parent 7d4255b2a1
commit 315b9e3c29
11 changed files with 120 additions and 48 deletions

View file

@ -28,9 +28,21 @@ import org.keycloak.provider.Provider;
* @version $Revision: 1 $
*/
public interface BruteForceProtector extends Provider {
String DISABLED_BY_PERMANENT_LOCKOUT = "permanentLockout";
void failedLogin(RealmModel realm, UserModel user, ClientConnection clientConnection);
void successfulLogin(RealmModel realm, UserModel user, ClientConnection clientConnection);
boolean isTemporarilyDisabled(KeycloakSession session, RealmModel realm, UserModel user);
boolean isPermanentlyLockedOut(KeycloakSession session, RealmModel realm, UserModel user);
/**
* Clears any remaining traces of the permanent lockout. Does not enable the user as such!
* @param session
* @param realm
* @param user
*/
void cleanUpPermanentLockout(KeycloakSession session, RealmModel realm, UserModel user);
}

View file

@ -46,6 +46,7 @@ public interface UserModel extends RoleMapperModel {
String GROUPS = "keycloak.session.realm.users.query.groups";
String SEARCH = "keycloak.session.realm.users.query.search";
String EXACT = "keycloak.session.realm.users.query.exact";
String DISABLED_REASON = "disabledReason";
Comparator<UserModel> COMPARE_BY_USERNAME = Comparator.comparing(UserModel::getUsername, String.CASE_INSENSITIVE_ORDER);

View file

@ -34,8 +34,8 @@ import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.representations.idm.CredentialRepresentation;
import org.keycloak.services.ServicesLogger;
import org.keycloak.services.managers.AuthenticationManager;
import org.keycloak.services.managers.BruteForceProtector;
import org.keycloak.services.messages.Messages;
import org.keycloak.services.validation.Validation;
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.Response;
@ -80,11 +80,11 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
return form.createLoginUsernamePassword();
}
protected String tempDisabledError() {
protected String disabledByBruteForceError() {
return Messages.INVALID_USER;
}
protected String tempDisabledFieldError(){
protected String disabledByBruteForceFieldError(){
return FIELD_USERNAME;
}
@ -129,6 +129,7 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
}
public boolean enabledUser(AuthenticationFlowContext context, UserModel user) {
if (isDisabledByBruteForce(context, user)) return false;
if (!user.isEnabled()) {
context.getEvent().user(user);
context.getEvent().error(Errors.USER_DISABLED);
@ -136,7 +137,6 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
context.forceChallenge(challengeResponse);
return false;
}
if (isTemporarilyDisabledByBruteForce(context, user)) return false;
return true;
}
@ -213,7 +213,7 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
return badPasswordHandler(context, user, clearUser,true);
}
if (isTemporarilyDisabledByBruteForce(context, user)) return false;
if (isDisabledByBruteForce(context, user)) return false;
if (password != null && !password.isEmpty() && context.getSession().userCredentialManager().isValid(context.getRealm(), user, UserCredentialModel.password(password))) {
return true;
@ -239,12 +239,15 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
return false;
}
protected boolean isTemporarilyDisabledByBruteForce(AuthenticationFlowContext context, UserModel user) {
protected boolean isDisabledByBruteForce(AuthenticationFlowContext context, UserModel user) {
if (context.getRealm().isBruteForceProtected()) {
if (context.getProtector().isTemporarilyDisabled(context.getSession(), context.getRealm(), user)) {
BruteForceProtector protector = context.getProtector();
boolean isPermanentlyLockedOut = protector.isPermanentlyLockedOut(context.getSession(), context.getRealm(), user);
if (isPermanentlyLockedOut || protector.isTemporarilyDisabled(context.getSession(), context.getRealm(), user)) {
context.getEvent().user(user);
context.getEvent().error(Errors.USER_TEMPORARILY_DISABLED);
Response challengeResponse = challenge(context, tempDisabledError(), tempDisabledFieldError());
context.getEvent().error(isPermanentlyLockedOut ? Errors.USER_DISABLED : Errors.USER_TEMPORARILY_DISABLED);
Response challengeResponse = challenge(context, disabledByBruteForceError(), disabledByBruteForceFieldError());
context.forceChallenge(challengeResponse);
return true;
}

View file

@ -88,7 +88,7 @@ public class OTPFormAuthenticator extends AbstractUsernameFormAuthenticator impl
UserModel userModel = context.getUser();
if (!enabledUser(context, userModel)) {
// error in context is set in enabledUser/isTemporarilyDisabledByBruteForce
// error in context is set in enabledUser/isDisabledByBruteForce
return;
}
@ -115,12 +115,12 @@ public class OTPFormAuthenticator extends AbstractUsernameFormAuthenticator impl
}
@Override
protected String tempDisabledError() {
protected String disabledByBruteForceError() {
return Messages.INVALID_TOTP;
}
@Override
protected String tempDisabledFieldError() {
protected String disabledByBruteForceFieldError() {
return Validation.FIELD_OTP_CODE;
}

View file

@ -31,6 +31,7 @@ import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.provider.ProviderConfigProperty;
import org.keycloak.services.ServicesLogger;
import org.keycloak.services.managers.AuthenticationManager;
import org.keycloak.services.managers.BruteForceProtector;
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.Response;
@ -74,6 +75,18 @@ public class ValidateUsername extends AbstractDirectGrantAuthenticator {
context.failure(AuthenticationFlowError.INVALID_USER, challengeResponse);
return;
}
if (context.getRealm().isBruteForceProtected()) {
BruteForceProtector protector = context.getProtector();
boolean isPermanentlyLockedOut = protector.isPermanentlyLockedOut(context.getSession(), context.getRealm(), user);
if (isPermanentlyLockedOut || protector.isTemporarilyDisabled(context.getSession(), context.getRealm(), user)) {
context.getEvent().user(user);
context.getEvent().error(isPermanentlyLockedOut ? Errors.USER_DISABLED : Errors.USER_TEMPORARILY_DISABLED);
Response challengeResponse = errorResponse(Response.Status.UNAUTHORIZED.getStatusCode(), "invalid_grant", "Invalid user credentials");
context.failure(AuthenticationFlowError.INVALID_USER, challengeResponse);
return;
}
}
if (!user.isEnabled()) {
context.getEvent().user(user);
context.getEvent().error(Errors.USER_DISABLED);
@ -81,15 +94,6 @@ public class ValidateUsername extends AbstractDirectGrantAuthenticator {
context.failure(AuthenticationFlowError.INVALID_USER, challengeResponse);
return;
}
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 = errorResponse(Response.Status.UNAUTHORIZED.getStatusCode(), "invalid_grant", "Invalid user credentials");
context.failure(AuthenticationFlowError.INVALID_USER, challengeResponse);
return;
}
}
context.setUser(user);
context.success();
}

View file

@ -30,6 +30,7 @@ import org.keycloak.events.Errors;
import org.keycloak.models.ModelDuplicateException;
import org.keycloak.models.UserModel;
import org.keycloak.services.ServicesLogger;
import org.keycloak.services.managers.BruteForceProtector;
/**
* @author <a href="mailto:pnalyvayko@agi.com">Peter Nalyvayko</a>
@ -118,6 +119,18 @@ public class ValidateX509CertificateUsername extends AbstractX509ClientCertifica
context.failure(AuthenticationFlowError.INVALID_USER, challengeResponse);
return;
}
if (context.getRealm().isBruteForceProtected()) {
BruteForceProtector protector = context.getProtector();
boolean isPermanentlyLockedOut = protector.isPermanentlyLockedOut(context.getSession(), context.getRealm(), user);
if (isPermanentlyLockedOut || protector.isTemporarilyDisabled(context.getSession(), context.getRealm(), user)) {
context.getEvent().user(user);
context.getEvent().error(isPermanentlyLockedOut ? Errors.USER_DISABLED : Errors.USER_TEMPORARILY_DISABLED);
Response challengeResponse = errorResponse(Response.Status.BAD_REQUEST.getStatusCode(), "invalid_grant", "Invalid user credentials");
context.failure(AuthenticationFlowError.INVALID_USER, challengeResponse);
return;
}
}
if (!user.isEnabled()) {
context.getEvent().user(user);
context.getEvent().error(Errors.USER_DISABLED);
@ -125,15 +138,6 @@ public class ValidateX509CertificateUsername extends AbstractX509ClientCertifica
context.failure(AuthenticationFlowError.INVALID_USER, challengeResponse);
return;
}
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 = errorResponse(Response.Status.BAD_REQUEST.getStatusCode(), "invalid_grant", "Account temporarily disabled");
context.failure(AuthenticationFlowError.INVALID_USER, challengeResponse);
return;
}
}
context.setUser(user);
context.success();
}

View file

@ -35,6 +35,7 @@ import org.keycloak.forms.login.LoginFormsProvider;
import org.keycloak.models.ModelDuplicateException;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.FormMessage;
import org.keycloak.services.managers.BruteForceProtector;
/**
* @author <a href="mailto:pnalyvayko@agi.com">Peter Nalyvayko</a>
@ -135,6 +136,23 @@ public class X509ClientCertificateAuthenticator extends AbstractX509ClientCertif
return;
}
if (context.getRealm().isBruteForceProtected()) {
BruteForceProtector protector = context.getProtector();
boolean isPermanentlyLockedOut = protector.isPermanentlyLockedOut(context.getSession(), context.getRealm(), user);
if (isPermanentlyLockedOut || protector.isTemporarilyDisabled(context.getSession(), context.getRealm(), user)) {
context.getEvent().user(user);
context.getEvent().error(isPermanentlyLockedOut ? Errors.USER_DISABLED : Errors.USER_TEMPORARILY_DISABLED);
// TODO use specific locale to load error messages
String errorMessage = "X509 certificate authentication's failed.";
// TODO is calling form().setErrors enough to show errors on login screen?
context.challenge(createErrorResponse(context, certs[0].getSubjectDN().getName(),
errorMessage, "Invalid user"));
context.attempted();
return;
}
}
if (!userEnabled(context, user)) {
// TODO use specific locale to load error messages
String errorMessage = "X509 certificate authentication's failed.";
@ -144,19 +162,6 @@ public class X509ClientCertificateAuthenticator extends AbstractX509ClientCertif
context.attempted();
return;
}
if (context.getRealm().isBruteForceProtected()) {
if (context.getProtector().isTemporarilyDisabled(context.getSession(), context.getRealm(), user)) {
context.getEvent().user(user);
context.getEvent().error(Errors.USER_TEMPORARILY_DISABLED);
// TODO use specific locale to load error messages
String errorMessage = "X509 certificate authentication's failed.";
// TODO is calling form().setErrors enough to show errors on login screen?
context.challenge(createErrorResponse(context, certs[0].getSubjectDN().getName(),
errorMessage, "User is temporarily disabled. Contact administrator."));
context.attempted();
return;
}
}
context.setUser(user);
// Check whether to display the identity confirmation

View file

@ -1235,8 +1235,11 @@ public class TokenEndpoint {
throw new CorsErrorResponseException(cors, Errors.INVALID_TOKEN, "Invalid Token", Response.Status.BAD_REQUEST);
}
if (realm.isBruteForceProtected()) {
if (session.getProvider(BruteForceProtector.class).isTemporarilyDisabled(session, realm, user)) {
event.error(Errors.USER_TEMPORARILY_DISABLED);
BruteForceProtector protector = session.getProvider(BruteForceProtector.class);
boolean isPermanentlyLockedOut = protector.isPermanentlyLockedOut(session, realm, user);
if (isPermanentlyLockedOut || protector.isTemporarilyDisabled(session, realm, user)) {
event.error(isPermanentlyLockedOut ? Errors.USER_DISABLED : Errors.USER_TEMPORARILY_DISABLED);
throw new CorsErrorResponseException(cors, Errors.INVALID_TOKEN, "Invalid Token", Response.Status.BAD_REQUEST);
}
}

View file

@ -33,6 +33,8 @@ import java.util.concurrent.CountDownLatch;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.TimeUnit;
import static org.keycloak.models.UserModel.DISABLED_REASON;
/**
* A single thread will log failures. This is so that we can avoid concurrent writes as we want an accurate failure count
*
@ -128,6 +130,7 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector
}
logger.debugv("user {0} locked permanently due to too many login attempts", user.getUsername());
user.setEnabled(false);
user.setSingleAttribute(DISABLED_REASON, DISABLED_BY_PERMANENT_LOCKOUT);
return;
}
@ -318,6 +321,19 @@ public class DefaultBruteForceProtector implements Runnable, BruteForceProtector
return false;
}
@Override
public boolean isPermanentlyLockedOut(KeycloakSession session, RealmModel realm, UserModel user) {
return !user.isEnabled() && DISABLED_BY_PERMANENT_LOCKOUT.equals(user.getFirstAttribute(DISABLED_REASON));
}
@Override
public void cleanUpPermanentLockout(KeycloakSession session, RealmModel realm, UserModel user) {
if (DISABLED_BY_PERMANENT_LOCKOUT.equals(user.getFirstAttribute(DISABLED_REASON))) {
user.removeAttribute(DISABLED_REASON);
}
}
@Override
public void close() {

View file

@ -99,7 +99,6 @@ import javax.ws.rs.core.Response.Status;
import javax.ws.rs.core.UriBuilder;
import java.net.URI;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedList;
@ -162,11 +161,13 @@ public class UserResource {
auth.users().requireManage(user);
try {
boolean wasPermanentlyLockedOut = false;
if (rep.isEnabled() != null && rep.isEnabled()) {
UserLoginFailureModel failureModel = session.loginFailures().getUserLoginFailure(realm, user.getId());
if (failureModel != null) {
failureModel.clearFailures();
}
wasPermanentlyLockedOut = session.getProvider(BruteForceProtector.class).isPermanentlyLockedOut(session, realm, user);
}
Response response = validateUserProfile(user, rep, session);
@ -175,6 +176,12 @@ public class UserResource {
}
updateUserFromRep(user, rep, session, true);
RepresentationToModel.createCredentials(rep, session, realm, user, true);
// we need to do it here as the attributes would be overwritten by what is in the rep
if (wasPermanentlyLockedOut) {
session.getProvider(BruteForceProtector.class).cleanUpPermanentLockout(session, realm, user);
}
adminEvent.operation(OperationType.UPDATE).resourcePath(session.getContext().getUri()).representation(rep).success();
if (session.getTransactionManager().isActive()) {

View file

@ -26,9 +26,11 @@ import org.keycloak.events.Details;
import org.keycloak.events.Errors;
import org.keycloak.events.EventType;
import org.keycloak.models.Constants;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.TimeBasedOTP;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.services.managers.BruteForceProtector;
import org.keycloak.testsuite.AbstractTestRealmKeycloakTest;
import org.keycloak.testsuite.AssertEvents;
import org.keycloak.testsuite.AssertEvents.ExpectedEvent;
@ -419,7 +421,14 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
// assert
expectPermanentlyDisabled();
assertFalse(adminClient.realm("test").users().search("test-user@localhost", 0, 1).get(0).isEnabled());
UserRepresentation user = adminClient.realm("test").users().search("test-user@localhost", 0, 1).get(0);
assertFalse(user.isEnabled());
assertUserDisabledReason(BruteForceProtector.DISABLED_BY_PERMANENT_LOCKOUT);
user.setEnabled(true);
updateUser(user);
assertUserDisabledReason(null);
} finally {
realm.setPermanentLockout(false);
testRealm().update(realm);
@ -563,7 +572,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
loginPage.login(username, "password");
loginPage.assertCurrent();
Assert.assertEquals("Account is disabled, contact your administrator.", loginPage.getError());
Assert.assertEquals("Invalid username or password.", loginPage.getInputError());
ExpectedEvent event = events.expectLogin()
.session((String) null)
.error(Errors.USER_DISABLED)
@ -708,4 +717,12 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
private void assertUserDisabledEvent() {
events.expect(EventType.LOGIN_ERROR).error(Errors.USER_TEMPORARILY_DISABLED).assertEvent();
}
private void assertUserDisabledReason(String expected) {
String actual = adminClient.realm("test").users()
.search("test-user@localhost", 0, 1)
.get(0)
.firstAttribute(UserModel.DISABLED_REASON);
assertEquals(expected, actual);
}
}