External-Mirrors/keycloak
2
0
Fork 0
mirror of https://github.com/keycloak/keycloak.git synced 2026-01-10 15:32:05 -03:30
Code Issues Packages Projects Releases Wiki Activity

Do not remove users in LDAP when queries return an empty result

Browse Source 
closes #34764

Signed-off-by: Christian Janker <christian.janker@gmx.at>
This commit is contained in:
Christian Janker 2024-11-10 22:48:11 +01:00 committed by Pedro Igor
parent 0fc0dcd119
commit 87db882a89
7 changed files with 128 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties
View File

@ -3391,4 +3391,6 @@ allResources=All resources
currentRealm=Current realm
recentlyUsed=Recently used
viewAll=View all
currentRealmExplain=This realm is selected
currentRealmExplain=This realm is selected
removeInvalidUsers=Remove invalid users during searches
removeInvalidUsersHelp=Remove invalid or deleted users from the KC database, which are not found in user storage. If this is true, then users, which were deleted from 3rd party user storage (EG. LDAP) will be deleted as well from the KC database whenever an attempt to lookup particular user is performed from KC. If false, then users previously imported from 3rd party user storage will be still kept in the KC database even if KC found that user is not anymore in the 3rd party user storage (for example user was deleted directly from LDAP). Set to false if you have problems with setup of your user storage (EG. setup of your LDAP server) and valid users are accidentally deleted from KC database.

29
js/apps/admin-ui/src/user-federation/ldap/LdapSettingsSynchronization.tsx
View File

@ -175,6 +175,35 @@ export const LdapSettingsSynchronization = ({
defaultValue={86400}
/>
)}
<FormGroup
label={t("removeInvalidUsers")}
labelIcon={
<HelpItem
helpText={t("removeInvalidUsersHelp")}
fieldLabelId="removeInvalidUsers"
/>
}
fieldId="kc-remove-invalid-users"
hasNoPaddingTop
>
<Controller
name="config.removeInvalidUsersEnabled"
defaultValue={["true"]}
control={form.control}
render={({ field }) => (
<Switch
id="kc-remove-invalid-users"
data-testid="remove-invalid-users"
isDisabled={false}
onChange={(_event, value) => field.onChange([`${value}`])}
isChecked={field.value[0] === "true"}
label={t("on")}
labelOff={t("off")}
aria-label={t("removeInvalidUsers")}
/>
)}
/>
</FormGroup>
</FormAccess>
</FormProvider>
);

15
model/storage-private/src/main/java/org/keycloak/storage/UserStorageManager.java
View File

@ -128,6 +128,7 @@ public class UserStorageManager extends AbstractStorageManager<UserStorageProvid
if (model == null) {
// remove linked user with unknown storage provider.
logger.debugf("Removed user with federation link of unknown storage provider '%s'", user.getUsername());
deleteInvalidUserCache(realm, user);
deleteInvalidUser(realm, user);
return null;
}
@ -149,7 +150,10 @@ public class UserStorageManager extends AbstractStorageManager<UserStorageProvid
UserModel validated = importedUserValidation.validate(realm, user);
if (validated == null) {
deleteInvalidUser(realm, user);
deleteInvalidUserCache(realm, user);
if (model.isRemoveInvalidUsersEnabled()) {
deleteInvalidUser(realm, user);
}
return null;
} else {
return validated;
@ -204,14 +208,16 @@ public class UserStorageManager extends AbstractStorageManager<UserStorageProvid
return result;
}
protected void deleteInvalidUser(final RealmModel realm, final UserModel user) {
String userId = user.getId();
String userName = user.getUsername();
protected void deleteInvalidUserCache(final RealmModel realm, final UserModel user) {
UserCache userCache = UserStorageUtil.userCache(session);
if (userCache != null) {
userCache.evict(realm, user);
}
}
protected void deleteInvalidUser(final RealmModel realm, final UserModel user) {
String userId = user.getId();
String userName = user.getUsername();
// This needs to be running in separate transaction because removing the user may end up with throwing
// PessimisticLockException which also rollbacks Jpa transaction, hence when it is running in current transaction
// it will become not usable for all consequent jpa calls. It will end up with Transaction is in rolled back
@ -234,7 +240,6 @@ public class UserStorageManager extends AbstractStorageManager<UserStorageProvid
});
}
protected Stream<UserModel> importValidation(RealmModel realm, Stream<UserModel> users) {
return users.map(user -> importValidation(realm, user)).filter(Objects::nonNull);
}

1
model/storage/src/main/java/org/keycloak/storage/AbstractStorageManager.java
View File

@ -182,7 +182,6 @@ public abstract class AbstractStorageManager<ProviderType extends Provider,
if (componentModel == null) {
return null;
}
return toStorageProviderModelTypeFunction.apply(componentModel);
}

15
model/storage/src/main/java/org/keycloak/storage/UserStorageProviderModel.java
View File

@ -33,6 +33,7 @@ public class UserStorageProviderModel extends CacheableStorageProviderModel {
public static final String FULL_SYNC_PERIOD = "fullSyncPeriod";
public static final String CHANGED_SYNC_PERIOD = "changedSyncPeriod";
public static final String LAST_SYNC = "lastSync";
public static final String REMOVE_INVALID_USERS_ENABELD = "removeInvalidUsersEnabled";
public UserStorageProviderModel() {
setProviderType(UserStorageProvider.class.getName());
@ -46,6 +47,7 @@ public class UserStorageProviderModel extends CacheableStorageProviderModel {
private transient Integer changedSyncPeriod;
private transient Integer lastSync;
private transient Boolean importEnabled;
private transient Boolean removeInvalidUsersEnabled;
public boolean isImportEnabled() {
if (importEnabled == null) {
@ -57,7 +59,6 @@ public class UserStorageProviderModel extends CacheableStorageProviderModel {
}
}
return importEnabled;
}
public void setImportEnabled(boolean flag) {
@ -116,4 +117,16 @@ public class UserStorageProviderModel extends CacheableStorageProviderModel {
this.lastSync = lastSync;
getConfig().putSingle(LAST_SYNC, Integer.toString(lastSync));
}
public boolean isRemoveInvalidUsersEnabled() {
if (removeInvalidUsersEnabled == null) {
String val = getConfig().getFirst(REMOVE_INVALID_USERS_ENABELD);
if (val == null) {
removeInvalidUsersEnabled = true;
} else {
removeInvalidUsersEnabled = Boolean.valueOf(val);
}
}
return removeInvalidUsersEnabled;
}
}

2
model/storage/src/main/java/org/keycloak/storage/UserStorageProviderSpi.java
View File

@ -81,6 +81,8 @@ public class UserStorageProviderSpi implements Spi {
.name("evictionDay").type(ProviderConfigProperty.STRING_TYPE).add()
.property()
.name("cacheInvalidBefore").type(ProviderConfigProperty.STRING_TYPE).add()
.property()
.name("removeInvalidUsersEnabled").type(ProviderConfigProperty.BOOLEAN_TYPE).add()
.build();
commonConfig = Collections.unmodifiableList(config);
}

70
testsuite/model/src/test/java/org/keycloak/testsuite/model/user/UserSyncTest.java
View File

@ -50,6 +50,7 @@ import org.keycloak.testsuite.util.LDAPTestUtils;
import javax.naming.directory.BasicAttribute;
import java.util.Collections;
import java.util.List;
import java.util.stream.IntStream;
import static org.hamcrest.CoreMatchers.equalTo;
@ -58,6 +59,7 @@ import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assume.assumeThat;
import static org.keycloak.storage.UserStorageProviderModel.REMOVE_INVALID_USERS_ENABELD;
@RequireProvider(UserProvider.class)
@RequireProvider(ClusterProvider.class)
@ -249,5 +251,73 @@ public class UserSyncTest extends KeycloakModelTest {
});
}
@Test
public void testInvalidUsersAreDeleted() {
withRealm(realmId, (session, realm) -> {
UserStorageProviderModel providerModel = new UserStorageProviderModel(realm.getComponent(userFederationId));
providerModel.setCachePolicy(CacheableStorageProviderModel.CachePolicy.NO_CACHE);
realm.updateComponent(providerModel);
return null;
});
// create user1 in LDAP
withRealm(realmId, (session, realm) -> {
ComponentModel ldapModel = LDAPTestUtils.getLdapProviderModel(realm);
LDAPStorageProvider ldapFedProvider = LDAPTestUtils.getLdapProvider(session, ldapModel);
int i = 1;
LDAPTestUtils.addLDAPUser(ldapFedProvider, realm, "user" + i, "User" + i + "FN", "User" + i + "LN", "user" + i + "@email.org", "my-street 9", "12" + i);
return null;
});
// import user
withRealm(realmId, (session, realm) -> {
UserModel user1 = session.users().getUserByUsername(realm, "user1");
user1.setSingleAttribute("LDAP_ID", "WRONG");
return user1;
});
// validate imported user
withRealm(realmId, (session, realm) -> {
session.users().getUserByUsername(realm, "user1");
UserModel deletedUser = UserStoragePrivateUtil.userLocalStorage(session).getUserByUsername(realm, "user1");
assertThat(deletedUser, is(nullValue()));
return deletedUser;
});
}
@Test
public void testInvalidUsersAreNotDeleted() {
withRealm(realmId, (session, realm) -> {
UserStorageProviderModel providerModel = new UserStorageProviderModel(realm.getComponent(userFederationId));
providerModel.setCachePolicy(CacheableStorageProviderModel.CachePolicy.NO_CACHE);
providerModel.getConfig().putSingle(REMOVE_INVALID_USERS_ENABELD, "false"); // prevent local delete
realm.updateComponent(providerModel);
return null;
});
// create user1 in LDAP
withRealm(realmId, (session, realm) -> {
ComponentModel ldapModel = LDAPTestUtils.getLdapProviderModel(realm);
LDAPStorageProvider ldapFedProvider = LDAPTestUtils.getLdapProvider(session, ldapModel);
int i = 1;
LDAPTestUtils.addLDAPUser(ldapFedProvider, realm, "user" + i, "User" + i + "FN", "User" + i + "LN", "user" + i + "@email.org", "my-street 9", "12" + i);
return null;
});
// import user
withRealm(realmId, (session, realm) -> {
UserModel user1 = session.users().getUserByUsername(realm, "user1");
user1.setSingleAttribute("LDAP_ID", "WRONG");
return user1;
});
// validate imported user
withRealm(realmId, (session, realm) -> {
session.users().getUserByUsername(realm, "user1");
UserModel deletedUser = UserStoragePrivateUtil.userLocalStorage(session).getUserByUsername(realm, "user1");
assertThat(deletedUser, is(notNullValue()));
return deletedUser;
});
}
}