Skip updating account controls if no control is set when enabling/disabling users

Closes #37720

Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
This commit is contained in:
Pedro Igor 2025-05-09 04:11:21 -03:00 committed by GitHub
parent f40cb88db4
commit 953ba04018
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 51 additions and 20 deletions

View File

@ -38,6 +38,7 @@ import javax.naming.AuthenticationException;
import java.util.Objects;
import java.util.Set;
import java.util.function.Function;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Stream;
@ -63,6 +64,11 @@ public class MSADUserAccountControlStorageMapper extends AbstractLDAPStorageMapp
// See https://msdn.microsoft.com/en-us/library/windows/desktop/ms681385(v=vs.85).aspx
public static final Set<String> PASSWORD_UPDATE_MSAD_ERROR_CODES = Set.of("52D");
private final Function<LDAPObject, UserAccountControl> GET_USER_ACCOUNT_CONTROL = ldapUser -> {
String userAccountControl = ldapUser.getAttributeAsString(LDAPConstants.USER_ACCOUNT_CONTROL);
return UserAccountControl.of(userAccountControl);
};
public MSADUserAccountControlStorageMapper(ComponentModel mapperModel, LDAPStorageProvider ldapProvider) {
super(mapperModel, ldapProvider);
ldapProvider.setUpdater(this);
@ -211,9 +217,21 @@ public class MSADUserAccountControlStorageMapper extends AbstractLDAPStorageMapp
}
protected UserAccountControl getUserAccountControl(LDAPObject ldapUser) {
String userAccountControl = ldapUser.getAttributeAsString(LDAPConstants.USER_ACCOUNT_CONTROL);
long longValue = userAccountControl == null ? 0 : Long.parseLong(userAccountControl);
return new UserAccountControl(longValue);
UserAccountControl control = GET_USER_ACCOUNT_CONTROL.apply(ldapUser);
if (control.isAnySet()) {
return control;
}
RealmModel realm = session.getContext().getRealm();
if (realm == null) {
return control;
}
ldapUser = ldapProvider.loadLDAPUserByUuid(realm, ldapUser.getUuid());
return GET_USER_ACCOUNT_CONTROL.apply(ldapUser);
}
// Update user in LDAP if "updateInLDAP" is true. Otherwise it is assumed that LDAP update will be called at the end of transaction
@ -256,18 +274,20 @@ public class MSADUserAccountControlStorageMapper extends AbstractLDAPStorageMapp
@Override
public void setEnabled(boolean enabled) {
if (UserStorageProvider.EditMode.WRITABLE.equals(ldapProvider.getEditMode())) {
MSADUserAccountControlStorageMapper.logger.debugf("Going to propagate enabled=%s for ldapUser '%s' to MSAD", enabled, ldapUser.getDn().toString());
UserAccountControl control = getUserAccountControl(ldapUser);
if (enabled) {
control.remove(UserAccountControl.ACCOUNTDISABLE);
} else {
control.add(UserAccountControl.ACCOUNTDISABLE);
if (control.isAnySet()) {
MSADUserAccountControlStorageMapper.logger.debugf("Going to propagate enabled=%s for ldapUser '%s' to MSAD", enabled, ldapUser.getDn().toString());
if (enabled) {
control.remove(UserAccountControl.ACCOUNTDISABLE);
} else {
control.add(UserAccountControl.ACCOUNTDISABLE);
}
markUpdatedAttributeInTransaction(LDAPConstants.ENABLED);
updateUserAccountControl(false, ldapUser, control);
}
markUpdatedAttributeInTransaction(LDAPConstants.ENABLED);
updateUserAccountControl(false, ldapUser, control);
}
// Always update DB
super.setEnabled(enabled);

View File

@ -47,9 +47,18 @@ public class UserAccountControl {
public static final long TRUSTED_TO_AUTH_FOR_DELEGATION = 0x1000000L;
public static final long PARTIAL_SECRETS_ACCOUNT = 0x04000000L;
private static final UserAccountControl EMPTY = new UserAccountControl(0);
public static UserAccountControl of(String userAccountControl) {
if (userAccountControl == null) {
return EMPTY;
}
return new UserAccountControl(Long.parseLong(userAccountControl));
}
private long value;
public UserAccountControl(long value) {
private UserAccountControl(long value) {
this.value = value;
}
@ -72,4 +81,8 @@ public class UserAccountControl {
public long getValue() {
return value;
}
public boolean isAnySet() {
return value != 0;
}
}

View File

@ -323,9 +323,7 @@ public class LDAPMSADMapperTest extends AbstractLDAPTest {
Assert.assertTrue(Long.parseLong(pwdLastSet) > 0);
String userAccountControl = ldapJohn.getAttributeAsString(LDAPConstants.USER_ACCOUNT_CONTROL);
long longValue = userAccountControl == null ? 0 : Long.parseLong(userAccountControl);
Assert.assertFalse(new UserAccountControl(longValue).has(UserAccountControl.ACCOUNTDISABLE));
Assert.assertFalse(UserAccountControl.of(userAccountControl).has(UserAccountControl.ACCOUNTDISABLE));
});
// Logout and login again. Success
@ -346,7 +344,7 @@ public class LDAPMSADMapperTest extends AbstractLDAPTest {
LDAPObject ldapJohn = ctx.getLdapProvider().loadLDAPUserByUsername(appRealm, "johnkeycloak");
String userAccountControlStr = ldapJohn.getAttributeAsString(LDAPConstants.USER_ACCOUNT_CONTROL);
UserAccountControl control = new UserAccountControl(Long.parseLong(userAccountControlStr));
UserAccountControl control = UserAccountControl.of(userAccountControlStr);
control.add(UserAccountControl.ACCOUNTDISABLE);
ldapJohn.setSingleAttribute(LDAPConstants.USER_ACCOUNT_CONTROL, String.valueOf(control.getValue()));
@ -517,7 +515,7 @@ public class LDAPMSADMapperTest extends AbstractLDAPTest {
LDAPObject ldapJohn = ctx.getLdapProvider().loadLDAPUserByUsername(appRealm, "johnkeycloak");
String userAccountControlStr = ldapJohn.getAttributeAsString(LDAPConstants.USER_ACCOUNT_CONTROL);
UserAccountControl control = new UserAccountControl(Long.parseLong(userAccountControlStr));
UserAccountControl control = UserAccountControl.of(userAccountControlStr);
control.remove(UserAccountControl.ACCOUNTDISABLE);
ldapJohn.setSingleAttribute(LDAPConstants.USER_ACCOUNT_CONTROL, String.valueOf(control.getValue()));
ctx.getLdapProvider().getLdapIdentityStore().update(ldapJohn);
@ -565,7 +563,7 @@ public class LDAPMSADMapperTest extends AbstractLDAPTest {
}
// Need to remove double quotes TODO: Ideally fix fetchString method and all the tests, which uses it as it is dummy to need to remove quotes in each test individually...
UserAccountControl acControl = new UserAccountControl(Long.parseLong(userAccountControls.replace("\"","")));
UserAccountControl acControl = UserAccountControl.of(userAccountControls.replace("\"",""));
return !acControl.has(UserAccountControl.ACCOUNTDISABLE);
}
}