Mark user as disabled if reaching max login failures and permanent lockout is enabled

Closes #40159

Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
This commit is contained in:
Pedro Igor 2025-06-18 03:34:56 -03:00 committed by GitHub
parent 881c2114ea
commit 828f9f7916
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 92 additions and 12 deletions

View File

@ -45,6 +45,7 @@ public abstract class AbstractUserRepresentation {
@JsonDeserialize(using = StringListMapDeserializer.class)
protected Map<String, List<String>> attributes;
private UserProfileMetadata userProfileMetadata;
protected Boolean enabled;
public String getId() {
@ -154,4 +155,12 @@ public abstract class AbstractUserRepresentation {
public UserProfileMetadata getUserProfileMetadata() {
return userProfileMetadata;
}
public Boolean isEnabled() {
return enabled;
}
public void setEnabled(Boolean enabled) {
this.enabled = enabled;
}
}

View File

@ -30,7 +30,6 @@ public class UserRepresentation extends AbstractUserRepresentation{
protected String self; // link
protected String origin;
protected Long createdTimestamp;
protected Boolean enabled;
protected Boolean totp;
protected String federationLink;
protected String serviceAccountClientId; // For rep, it points to clientId (not DB ID)
@ -104,14 +103,6 @@ public class UserRepresentation extends AbstractUserRepresentation{
this.createdTimestamp = createdTimestamp;
}
public Boolean isEnabled() {
return enabled;
}
public void setEnabled(Boolean enabled) {
this.enabled = enabled;
}
@Deprecated
public Boolean isTotp() {
return totp;

View File

@ -242,7 +242,9 @@ public class ModelToRepresentation {
if (setUserAttributes) {
rep.setEmail(user.getEmail());
}
rep.setEnabled(user.isEnabled());
if (rep.isEnabled() == null) {
rep.setEnabled(user.isEnabled());
}
rep.setEmailVerified(user.isEmailVerified());
rep.setTotp(user.credentialManager().isConfiguredFor(OTPCredentialModel.TYPE));
rep.setDisableableCredentialTypes(user.credentialManager()
@ -290,7 +292,9 @@ public class ModelToRepresentation {
if (setUserAttributes) {
rep.setEmail(user.getEmail());
}
rep.setEnabled(user.isEnabled());
if (rep.isEnabled() == null) {
rep.setEnabled(user.isEnabled());
}
rep.setEmailVerified(user.isEmailVerified());
rep.setFederationLink(user.getFederationLink());
addAttributeToBriefRep(user, rep, IS_TEMP_ADMIN_ATTR_NAME);

View File

@ -41,6 +41,7 @@ import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.ModelToRepresentation;
import org.keycloak.representations.idm.AbstractUserRepresentation;
import org.keycloak.services.managers.BruteForceProtector;
import org.keycloak.storage.ReadOnlyException;
import org.keycloak.utils.StringUtil;
@ -250,6 +251,18 @@ public final class DefaultUserProfile implements UserProfile {
setAttributeIfExists(user, DISABLED_REASON, attributesRep);
setAttributeIfExists(user, IS_TEMP_ADMIN_ATTR_NAME, attributesRep);
RealmModel realm = session.getContext().getRealm();
if (realm.isBruteForceProtected() && !attributesRep.containsKey(DISABLED_REASON)) {
BruteForceProtector protector = session.getProvider(BruteForceProtector.class);
boolean lockedOut = protector.isPermanentlyLockedOut(session, realm, user);
if (lockedOut) {
rep.setEnabled(false);
attributesRep.put(DISABLED_REASON, List.of(BruteForceProtector.DISABLED_BY_PERMANENT_LOCKOUT));
}
}
rep.setId(user.getId());
rep.setAttributes(attributesRep.isEmpty() ? null : attributesRep);
rep.setUserProfileMetadata(createUserProfileMetadata(session, this));

View File

@ -250,8 +250,12 @@ public class DefaultBruteForceProtector implements BruteForceProtector {
@Override
public void cleanUpPermanentLockout(KeycloakSession session, RealmModel realm, UserModel user) {
if (DISABLED_BY_PERMANENT_LOCKOUT.equals(user.getFirstAttribute(DISABLED_REASON))) {
if (DISABLED_BY_PERMANENT_LOCKOUT.equals(user.getFirstAttribute(DISABLED_REASON)) || isPermanentlyLockedOut(session, realm, user)) {
user.removeAttribute(DISABLED_REASON);
if (!isTemporarilyDisabled(session, realm, user)) {
session.loginFailures().removeUserLoginFailure(realm, user.getId());
}
}
}

View File

@ -23,6 +23,7 @@ import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.keycloak.admin.client.resource.UsersResource;
import org.keycloak.events.Details;
import org.keycloak.events.Errors;
import org.keycloak.events.EventType;
@ -63,6 +64,7 @@ import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
/**
@ -553,6 +555,57 @@ public class BruteForceTest extends AbstractChangeImportedUserPasswordsTest {
assertEquals(Boolean.TRUE, testRealm().attackDetection().bruteForceUserStatus(userId).get("disabled"));
}
@Test
public void testUserDisabledAfterSwitchFromMixedToPermanentLockout() throws Exception {
UsersResource users = testRealm().users();
UserRepresentation user = users.search("test-user@localhost", null, null, null, 0, 1).get(0);
// temporarily lockout
loginInvalidPassword();
loginInvalidPassword();
expectTemporarilyDisabled();
assertUserNumberOfFailures(user.getId(), 2);
// user is still enabled during temporary lockout
assertTrue(users.get(user.getId()).toRepresentation().isEnabled());
assertTrue(users.search("test-user@localhost", true).get(0).isEnabled());
assertEquals(Boolean.TRUE, testRealm().attackDetection().bruteForceUserStatus(user.getId()).get("disabled"));
RealmRepresentation realm = testRealm().toRepresentation();
try {
// switch to permanent lockout before waiting to successful login
realm.setPermanentLockout(true);
testRealm().update(realm);
// expires the temporary lockout
this.setTimeOffset(60);
// after switching to permanent lockout the user status is disabled because there are login failures
// the user did not try to successfully authenticate yet to clear the login failures
user = users.get(user.getId()).toRepresentation();
assertFalse(user.isEnabled());
assertFalse(users.search("test-user@localhost", true).get(0).isEnabled());
assertEquals(Boolean.TRUE, testRealm().attackDetection().bruteForceUserStatus(user.getId()).get("disabled"));
expectPermanentlyDisabled();
// attempt to re-enable the user and login successfully
user.setEnabled(true);
users.get(user.getId()).update(user);
user = users.get(user.getId()).toRepresentation();
assertTrue(user.isEnabled());
assertTrue(users.search("test-user@localhost", true).get(0).isEnabled());
Map<String, Object> userAttackInfo = testRealm().attackDetection().bruteForceUserStatus(user.getId());
assertEquals(Boolean.FALSE, userAttackInfo.get("disabled"));
assertThat((Integer) userAttackInfo.get("numFailures"), is(0));
// login failures should be removed after re-enabling the user and the user able to authenticate
loginSuccess();
} finally {
resetTimeOffset();
realm.setPermanentLockout(false);
testRealm().update(realm);
}
}
@Test
public void testBrowserMissingPassword() throws Exception {
loginSuccess();
@ -977,6 +1030,12 @@ public class BruteForceTest extends AbstractChangeImportedUserPasswordsTest {
.detail(Details.USERNAME, username)
.removeDetail(Details.CONSENT);
event.assertEvent();
UserRepresentation user = testRealm().users().search(username, true).get(0);
user = testRealm().users().get(user.getId()).toRepresentation();
List<String> disabledReason = user.getAttributes().get(UserModel.DISABLED_REASON);
assertNotNull(disabledReason);
assertEquals(1, disabledReason.size());
assertEquals(BruteForceProtector.DISABLED_BY_PERMANENT_LOCKOUT, disabledReason.get(0));
}
public void loginSuccess() {