libre.sh/keycloak-scim
2
0
Fork 0
Code Issues 14 Pull requests Releases Packages Activity Actions 6

Improve response time when displaying group members using LDAP Provider

Browse source 
Closes #31786

Signed-off-by: rmartinc <rmartinc@redhat.com>
Co-authored-by: rmartinc <rmartinc@redhat.com>
This commit is contained in:
Stefan Guilhen 2024-09-20 16:20:24 -03:00 committed by GitHub
parent fe9c4dd7ed
commit be13366c17
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 36 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java
View file

@ -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<LDAPObject> loadUsersByUniqueAttributeChunk(RealmModel realm, String uidName, Collection<String> 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);

43
js/apps/admin-ui/src/groups/Members.tsx
View file

@ -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) => (
<>
<GroupPath key={group.id + "-" + member.id} group={group} />
{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={
<ListEmptyState

2
js/libs/keycloak-admin-client/src/resources/groups.ts
View file

@ -139,7 +139,7 @@ export class Groups extends Resource<{ realm?: string }> {
*/
public listMembers = this.makeRequest<
{ id: string; first?: number; max?: number },
{ id: string; first?: number; max?: number; briefRepresentation?: boolean },
UserRepresentation[]
>({
method: "GET",