Skip to content

Commit

Permalink
Calling getTopLevelGroups is slow inside GroupLDAPStorageMapper#getLD…
Browse files Browse the repository at this point in the history
…APGroupMappingsConverted (keycloak#8430)


Closes keycloak#14820 
---------
Co-authored-by: Michal Hajas <[email protected]>
  • Loading branch information
bohmber authored Sep 20, 2023
1 parent f8a9e01 commit bb2f59d
Show file tree
Hide file tree
Showing 20 changed files with 472 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
import java.util.Objects;
import java.util.Set;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand Down Expand Up @@ -338,7 +337,9 @@ private void updateKeycloakGroupTreeEntry(RealmModel realm, GroupTreeResolver.Gr

private void dropNonExistingKcGroups(RealmModel realm, SynchronizationResult syncResult, Set<String> visitedGroupIds) {
// Remove keycloak groups, which don't exist in LDAP
getAllKcGroups(realm)
GroupModel parent = getKcGroupsPathGroup(realm);

getAllKcGroups(realm, parent)
.filter(kcGroup -> !visitedGroupIds.contains(kcGroup.getId()))
.forEach(kcGroup -> {
logger.debugf("Removing Keycloak group '%s', which doesn't exist in LDAP", kcGroup.getName());
Expand All @@ -361,22 +362,22 @@ private void updateAttributesOfKCGroup(GroupModel kcGroup, LDAPObject ldapGroup)
}


protected GroupModel findKcGroupByLDAPGroup(RealmModel realm, LDAPObject ldapGroup) {
protected GroupModel findKcGroupByLDAPGroup(RealmModel realm, GroupModel parent, LDAPObject ldapGroup) {
String groupNameAttr = config.getGroupNameLdapAttribute();
String groupName = ldapGroup.getAttributeAsString(groupNameAttr);

if (config.isPreserveGroupsInheritance()) {
// Override if better effectivity or different algorithm is needed
return getAllKcGroups(realm)
return getAllKcGroups(realm, parent)
.filter(group -> Objects.equals(group.getName(), groupName)).findFirst().orElse(null);
} else {
// Without preserved inheritance, it's always at groups path
return KeycloakModelUtils.findGroupByPath(realm, getKcGroupPathFromLDAPGroupName(groupName));
return session.groups().getGroupByName(realm, parent, groupName);
}
}

protected GroupModel findKcGroupOrSyncFromLDAP(RealmModel realm, LDAPObject ldapGroup, UserModel user) {
GroupModel kcGroup = findKcGroupByLDAPGroup(realm, ldapGroup);
protected GroupModel findKcGroupOrSyncFromLDAP(RealmModel realm, GroupModel parent, LDAPObject ldapGroup, UserModel user) {
GroupModel kcGroup = findKcGroupByLDAPGroup(realm, parent, ldapGroup);

if (kcGroup == null) {

Expand All @@ -385,7 +386,7 @@ protected GroupModel findKcGroupOrSyncFromLDAP(RealmModel realm, LDAPObject ldap
// Better to sync all groups from LDAP with preserved inheritance
if (!syncFromLDAPPerformedInThisTransaction) {
syncDataFromFederationProviderToKeycloak(realm);
kcGroup = findKcGroupByLDAPGroup(realm, ldapGroup);
kcGroup = findKcGroupByLDAPGroup(realm, parent, ldapGroup);
}
} else {
String groupNameAttr = config.getGroupNameLdapAttribute();
Expand Down Expand Up @@ -656,14 +657,16 @@ public void onImportUserFromLDAP(LDAPObject ldapUser, UserModel user, RealmModel
if (mode == LDAPGroupMapperMode.IMPORT && isCreate) {

List<LDAPObject> ldapGroups = getLDAPGroupMappings(ldapUser);

// Import role mappings from LDAP into Keycloak DB
for (LDAPObject ldapGroup : ldapGroups) {

GroupModel kcGroup = findKcGroupOrSyncFromLDAP(realm, ldapGroup, user);
if (kcGroup != null) {
logger.debugf("User '%s' joins group '%s' during import from LDAP", user.getUsername(), kcGroup.getName());
user.joinGroup(kcGroup);
if (!ldapGroups.isEmpty()) {
GroupModel parent = getKcGroupsPathGroup(realm);
// Import role mappings from LDAP into Keycloak DB
for (LDAPObject ldapGroup : ldapGroups) {

GroupModel kcGroup = findKcGroupOrSyncFromLDAP(realm, parent, ldapGroup, user);
if (kcGroup != null) {
logger.debugf("User '%s' joins group '%s' during import from LDAP", user.getUsername(), kcGroup.getName());
user.joinGroup(kcGroup);
}
}
}
}
Expand Down Expand Up @@ -760,13 +763,17 @@ protected Stream<GroupModel> getLDAPGroupMappingsConverted() {
}

List<LDAPObject> ldapGroups = getLDAPGroupMappings(ldapUser);
if (!ldapGroups.isEmpty()) {
GroupModel parent = getKcGroupsPathGroup(realm);

cachedLDAPGroupMappings = ldapGroups.stream()
.map(ldapGroup -> findKcGroupOrSyncFromLDAP(realm, ldapGroup, this))
.filter(Objects::nonNull)
.collect(Collectors.toSet());
cachedLDAPGroupMappings = ldapGroups.stream()
.map(ldapGroup -> findKcGroupOrSyncFromLDAP(realm, parent, ldapGroup, this))
.filter(Objects::nonNull)
.collect(Collectors.toSet());

return cachedLDAPGroupMappings.stream();
return cachedLDAPGroupMappings.stream();
}
return Stream.empty();
}
}

Expand All @@ -783,7 +790,7 @@ protected String getKcGroupPathFromLDAPGroupName(String ldapGroupName) {
* Provides KC group defined as groups path or null (top-level group) if corresponding group is not available.
*/
protected GroupModel getKcGroupsPathGroup(RealmModel realm) {
return config.isTopLevelGroupsPath() ? null : KeycloakModelUtils.findGroupByPath(realm, config.getGroupsPath());
return config.isTopLevelGroupsPath() ? null : KeycloakModelUtils.findGroupByPath(session.groups(), realm, config.getGroupsPath());
}

/**
Expand Down Expand Up @@ -813,9 +820,7 @@ protected Stream<GroupModel> getKcSubGroups(RealmModel realm, GroupModel parentG
/**
* Provides a stream of all KC groups (with their sub groups) from groups path configured by the "Groups Path" configuration property.
*/
protected Stream<GroupModel> getAllKcGroups(RealmModel realm) {
GroupModel topParentGroup = getKcGroupsPathGroup(realm);

protected Stream<GroupModel> getAllKcGroups(RealmModel realm, GroupModel topParentGroup) {
Stream<GroupModel> allGroups = realm.getGroupsStream();
if (topParentGroup == null) return allGroups;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ public void validateConfiguration(KeycloakSession session, RealmModel realm, Com
LDAPUtils.validateCustomLdapFilter(config.getConfig().getFirst(GroupMapperConfig.GROUPS_LDAP_FILTER));

String group = new GroupMapperConfig(config).getGroupsPath();
if (!GroupMapperConfig.DEFAULT_LDAP_GROUPS_PATH.equals(group) && KeycloakModelUtils.findGroupByPath(realm, group) == null) {
if (!GroupMapperConfig.DEFAULT_LDAP_GROUPS_PATH.equals(group) && KeycloakModelUtils.findGroupByPath(session.groups(), realm, group) == null) {
throw new ComponentValidationException("ldapErrorMissingGroupsPathGroup");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.keycloak.models.cache.infinispan.stream.GroupListPredicate;
import org.keycloak.models.cache.infinispan.stream.HasRolePredicate;
import org.keycloak.models.cache.infinispan.stream.InClientPredicate;
import org.keycloak.models.cache.infinispan.stream.InGroupPredicate;
import org.keycloak.models.cache.infinispan.stream.InRealmPredicate;

import java.util.Set;
Expand Down Expand Up @@ -96,6 +97,10 @@ public void groupQueriesInvalidations(String realmId, Set<String> invalidations)
addInvalidations(GroupListPredicate.create().realm(realmId), invalidations);
}

public void groupNameInvalidations(String groupId, Set<String> invalidations) {
addInvalidations(InGroupPredicate.create().group(groupId), invalidations);
}

public void clientAdded(String realmId, String clientUUID, String clientId, Set<String> invalidations) {
invalidations.add(RealmCacheSession.getRealmClientsQueryCacheKey(realmId));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ public void registerGroupInvalidation(String id) {

private void invalidateGroup(String id, String realmId, boolean invalidateQueries) {
invalidateGroup(id);
cache.groupNameInvalidations(id, invalidations);
if (invalidateQueries) {
cache.groupQueriesInvalidations(realmId, invalidations);
}
Expand Down Expand Up @@ -564,6 +565,14 @@ static String getTopGroupsQueryCacheKey(String realm) {
return realm + ".top.groups";
}

static String getGroupByNameCacheKey(String realm, String parentId, String name) {
if (parentId != null) {
return realm + ".group." + parentId + "." + name;
} else {
return realm + ".group.top." + name;
}
}

static String getRolesCacheKey(String container) {
return container + ROLES_QUERY_SUFFIX;
}
Expand Down Expand Up @@ -886,6 +895,33 @@ public GroupModel getGroupById(RealmModel realm, String id) {
return adapter;
}

@Override
public GroupModel getGroupByName(RealmModel realm, GroupModel parent, String name) {
String cacheKey = getGroupByNameCacheKey(realm.getId(), parent != null? parent.getId(): null, name);
GroupNameQuery query = cache.get(cacheKey, GroupNameQuery.class);
if (query != null) {
logger.tracev("Group by name cache hit: {0}", name);
}
if (query == null) {
Long loaded = cache.getCurrentRevision(cacheKey);
GroupModel model = getGroupDelegate().getGroupByName(realm, parent, name);
if (model == null) return null;
if (invalidations.contains(model.getId())) return model;
query = new GroupNameQuery(loaded, cacheKey, model.getId(), realm);
cache.addRevisioned(query, startupRevision);
return model;
} else if (invalidations.contains(cacheKey)) {
return getGroupDelegate().getGroupByName(realm, parent, name);
} else {
String groupId = query.getGroupId();
if (invalidations.contains(groupId)) {
return getGroupDelegate().getGroupByName(realm, parent, name);
}
return getGroupById(realm, groupId);
}

}

@Override
public void moveGroup(RealmModel realm, GroupModel group, GroupModel toParent) {
invalidateGroup(group.getId(), realm.getId(), true);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Copyright 2022 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.keycloak.models.cache.infinispan.entities;

import org.keycloak.models.RealmModel;

public class GroupNameQuery extends AbstractRevisioned implements InRealm {
private final String realm;
private final String realmName;
private final String groupId;

public GroupNameQuery(Long revisioned, String id, String groupId, RealmModel realm) {
super(revisioned, id);
this.realm = realm.getId();
this.realmName = realm.getName();
this.groupId = groupId;
}

public String getGroupId() {
return groupId;
}

public String getRealm() {
return realm;
}

@Override
public String toString() {
return "GroupNameQuery{" +
"id='" + getId() + "'" +
"realmName='" + realmName + '\'' +
'}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public String toString() {
@Override
public void addInvalidations(RealmCacheManager realmCache, Set<String> invalidations) {
realmCache.groupQueriesInvalidations(realmId, invalidations);
realmCache.groupNameInvalidations(groupId, invalidations);
if (newParentId != null) {
invalidations.add(newParentId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public String toString() {
@Override
public void addInvalidations(RealmCacheManager realmCache, Set<String> invalidations) {
realmCache.groupQueriesInvalidations(realmId, invalidations);
realmCache.groupNameInvalidations(groupId, invalidations);
if (parentId != null) {
invalidations.add(parentId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public String toString() {

@Override
public void addInvalidations(RealmCacheManager realmCache, Set<String> invalidations) {
// Nothing. ID already invalidated
realmCache.groupNameInvalidations(groupId, invalidations);
}

public static class ExternalizerImpl implements Externalizer<GroupUpdatedEvent> {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* Copyright 2022 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.keycloak.models.cache.infinispan.stream;

import org.keycloak.models.cache.infinispan.entities.GroupNameQuery;
import org.keycloak.models.cache.infinispan.entities.Revisioned;

import java.io.IOException;
import java.io.ObjectInput;
import java.io.ObjectOutput;
import java.io.Serializable;
import java.util.Map;
import java.util.function.Predicate;
import org.infinispan.commons.marshall.Externalizer;
import org.infinispan.commons.marshall.MarshallUtil;
import org.infinispan.commons.marshall.SerializeWith;

@SerializeWith(InGroupPredicate.ExternalizerImpl.class)
public class InGroupPredicate implements Predicate<Map.Entry<String, Revisioned>>, Serializable {
private String group;

public static InGroupPredicate create() {
return new InGroupPredicate();
}

public InGroupPredicate group(String id) {
group = id;
return this;
}

@Override
public boolean test(Map.Entry<String, Revisioned> entry) {
Object value = entry.getValue();
if (value == null) return false;
if (!(value instanceof GroupNameQuery)) return false;

return group.equals(((GroupNameQuery)value).getGroupId());
}

public static class ExternalizerImpl implements Externalizer<InGroupPredicate> {

private static final int VERSION_1 = 1;

@Override
public void writeObject(ObjectOutput output, InGroupPredicate obj) throws IOException {
output.writeByte(VERSION_1);

MarshallUtil.marshallString(obj.group, output);
}

@Override
public InGroupPredicate readObject(ObjectInput input) throws IOException, ClassNotFoundException {
switch (input.readByte()) {
case VERSION_1:
return readObjectVersion1(input);
default:
throw new IOException("Unknown version");
}
}

public InGroupPredicate readObjectVersion1(ObjectInput input) throws IOException, ClassNotFoundException {
InGroupPredicate res = new InGroupPredicate();
res.group = MarshallUtil.unmarshallString(input);

return res;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright 2022 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.keycloak.models.sessions.infinispan.entities.wildfly;

import org.keycloak.models.cache.infinispan.stream.InGroupPredicate;

public class InGroupPredicateWFExternalizer extends InfinispanExternalizerAdapter<InGroupPredicate> {

public InGroupPredicateWFExternalizer() {
super(InGroupPredicate.class, new InGroupPredicate.ExternalizerImpl());
}
}
Loading

0 comments on commit bb2f59d

Please sign in to comment.