diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java index f077f0bfbc..d75812f506 100755 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java @@ -389,7 +389,8 @@ public class LDAPStorageProvider implements UserStorageProvider, result = result.filter(filterLocalUsers(realm)); } return StreamsUtil.paginatedStream( - result.map(ldapObject -> importUserFromLDAP(session, realm, ldapObject, false)) + // search users but not force import returning null as they were returned before by the DB + result.map(ldapObject -> importUserFromLDAP(session, realm, ldapObject, ImportType.NOT_FORCED_RETURN_NULL)) .filter(Objects::nonNull), firstResult, maxResults); } @@ -459,7 +460,8 @@ public class LDAPStorageProvider implements UserStorageProvider, .flatMap(Function.identity()) .skip(firstResult) .limit(maxResults) - .map(ldapUser -> importUserFromLDAP(session, realm, ldapUser)); + // do no force the import and return the current existing user if available + .map(ldapUser -> importUserFromLDAP(session, realm, ldapUser, ImportType.NOT_FORCED_RETURN_EXISTING)); } private Stream loadUsersByUniqueAttributeChunk(RealmModel realm, String uidName, Collection uids) { @@ -480,7 +482,8 @@ public class LDAPStorageProvider implements UserStorageProvider, .flatMap(Function.identity()) .skip(firstResult) .limit(maxResults) - .map(ldapUser -> importUserFromLDAP(session, realm, ldapUser)); + // do no force the import and return the current existing user if available + .map(ldapUser -> importUserFromLDAP(session, realm, ldapUser, ImportType.NOT_FORCED_RETURN_EXISTING)); } private Condition createSearchCondition(LDAPQueryConditionsBuilder conditionsBuilder, String name, boolean equals, String value) { @@ -652,7 +655,7 @@ public class LDAPStorageProvider implements UserStorageProvider, } protected UserModel importUserFromLDAP(KeycloakSession session, RealmModel realm, LDAPObject ldapUser) { - return importUserFromLDAP(session, realm, ldapUser, true); + return importUserFromLDAP(session, realm, ldapUser, ImportType.FORCED); } private void doImportUser(final RealmModel realm, final UserModel user, final LDAPObject ldapUser) { @@ -687,9 +690,18 @@ public class LDAPStorageProvider implements UserStorageProvider, user.getUsername(), user.getEmail(), ldapUser.getUuid(), userDN); } - protected UserModel importUserFromLDAP(KeycloakSession session, RealmModel realm, LDAPObject ldapUser, boolean forcedImport) { + protected enum ImportType { + FORCED, // the import is forced + NOT_FORCED_RETURN_NULL, // the import is not forced and null is returned when a previous user exists + NOT_FORCED_RETURN_EXISTING // the import is not forced and existing user is returned + }; + + protected UserModel importUserFromLDAP(KeycloakSession session, RealmModel realm, LDAPObject ldapUser, ImportType importType) { String ldapUsername = LDAPUtils.getUsername(ldapUser, ldapIdentityStore.getConfig()); LDAPUtils.checkUuid(ldapUser, ldapIdentityStore.getConfig()); + if (importType == null) { + importType = ImportType.FORCED; + } UserModel imported = null; UserModel existingLocalUser = null; @@ -701,14 +713,17 @@ public class LDAPStorageProvider implements UserStorageProvider, .findFirst().orElse(null); if (existingLocalUser != null) { imported = existingLocalUser; - // Need to evict the existing user from cache + if (importType == ImportType.NOT_FORCED_RETURN_NULL) { + // import not forced and return null + return null; + } else if (importType == ImportType.NOT_FORCED_RETURN_EXISTING) { + // import not forced and return the current DB user + return proxy(realm, imported, ldapUser, false); + } + // import is forced, need to evict the existing user from cache if (UserStorageUtil.userCache(session) != null) { UserStorageUtil.userCache(session).evict(realm, existingLocalUser); } - if (!forcedImport) { - // if import is not forced return null as it was already imported - return null; - } } else { imported = userProvider.addUser(realm, ldapUsername); } @@ -720,7 +735,7 @@ public class LDAPStorageProvider implements UserStorageProvider, doImportUser(realm, imported, ldapUser); } catch (ModelDuplicateException e) { logger.warnf(e, "Duplicated user importing from LDAP. LDAP Entry DN: [%s], LDAP_ID: [%s]", ldapUser.getDn(), ldapUser.getUuid()); - if (!forcedImport && existingLocalUser == null) { + if (importType != ImportType.FORCED && existingLocalUser == null) { // try to continue if import was not forced, delete created db user if necessary if (model.isImportEnabled() && imported != null) { userProvider.removeUser(realm, imported); diff --git a/js/apps/admin-ui/src/groups/Members.tsx b/js/apps/admin-ui/src/groups/Members.tsx index b0eb7dc6da..6eb72dd95a 100644 --- a/js/apps/admin-ui/src/groups/Members.tsx +++ b/js/apps/admin-ui/src/groups/Members.tsx @@ -24,7 +24,6 @@ import { useState } from "react"; import { useTranslation } from "react-i18next"; import { Link, useLocation } from "react-router-dom"; import { useAdminClient } from "../admin-client"; -import { GroupPath } from "../components/group/GroupPath"; import { KeycloakSpinner } from "@keycloak/keycloak-ui-shared"; import { useAccess } from "../context/access/Access"; import { useRealm } from "../context/realm-context/RealmContext"; @@ -34,24 +33,7 @@ import { MemberModal } from "./MembersModal"; import { useSubGroups } from "./SubGroupsContext"; import { getLastId } from "./groupIdUtils"; -type MembersOf = UserRepresentation & { - membership: GroupRepresentation[]; -}; - -const MemberOfRenderer = (member: MembersOf) => { - return ( - <> - {member.membership.map((group, index) => ( - <> - - {member.membership[index + 1] ? ", " : ""} - - ))} - - ); -}; - -const UserDetailLink = (user: MembersOf) => { +const UserDetailLink = (user: UserRepresentation) => { const { realm } = useRealm(); const { t } = useTranslation(); return ( @@ -94,9 +76,6 @@ export const Members = () => { const [key, setKey] = useState(0); const refresh = () => setKey(new Date().getTime()); - const getMembership = async (id: string) => - await adminClient.users.listGroups({ id: id! }); - // this queries the subgroups using the new search paradigm but doesn't // account for pagination and therefore isn't going to scale well const getSubGroups = async (groupId?: string, count = 0) => { @@ -128,6 +107,7 @@ export const Members = () => { let members = await adminClient.groups.listMembers({ id: id!, + briefRepresentation: true, first, max, }); @@ -138,19 +118,19 @@ export const Members = () => { currentGroup.subGroupCount, ); await Promise.all( - subGroups.map((g) => adminClient.groups.listMembers({ id: g.id! })), + subGroups.map((g) => + adminClient.groups.listMembers({ + id: g.id!, + briefRepresentation: true, + }), + ), ).then((values: UserRepresentation[][]) => { values.forEach((users) => (members = members.concat(users))); }); members = uniqBy(members, (member) => member.username); } - const memberOfPromises = await Promise.all( - members.map((member) => getMembership(member.id!)), - ); - return members.map((member: UserRepresentation, i) => { - return { ...member, membership: memberOfPromises[i] }; - }); + return members; }; if (!currentGroup) { @@ -305,11 +285,6 @@ export const Members = () => { displayKey: "lastName", cellFormatters: [emptyFormatter()], }, - { - name: "membership", - displayKey: "membership", - cellRenderer: MemberOfRenderer, - }, ]} emptyState={ { */ public listMembers = this.makeRequest< - { id: string; first?: number; max?: number }, + { id: string; first?: number; max?: number; briefRepresentation?: boolean }, UserRepresentation[] >({ method: "GET",