libre.sh/keycloak-scim
4
0
Fork 0
Code Issues 15 Pull requests Releases 1 Activity Actions

KEYCLOAK-13074 Don't return LDAP group members if under IMPORT mode

Browse source 
If GroupLDAPStorageMapper is running under IMPORT mode, getGroupMembers should not return users in LDAP, which, according to how UserStorageManager.query works (getting both user federation and Keycloak storage), will cause duplicate users in the list.

A test has been added as well, which will fail before the fix in the mapper.
This commit is contained in:
Phy 2020-02-23 21:25:21 -05:00 committed by Marek Posolda
parent ed97d40939
commit 8aa5019efe
2 changed files with 27 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java
View file

@ -579,6 +579,11 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements
@Override
public List<UserModel> getGroupMembers(RealmModel realm, GroupModel kcGroup, int firstResult, int maxResults) {
if (config.getMode() == LDAPGroupMapperMode.IMPORT) {
// only results from Keycloak should be returned, or imported LDAP and KC items will duplicate
return Collections.emptyList();
}
// TODO: with ranged search in AD we can improve the search using the specific range (not done for the moment)
LDAPObject ldapGroup = loadLDAPGroupByName(kcGroup.getName());
if (ldapGroup == null) {

22
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperTest.java
View file

@ -417,6 +417,17 @@ public class LDAPGroupMapperTest extends AbstractLDAPTest {
long dbGroupCount = rob.getGroupsCount();
Assert.assertEquals(4, dbGroupCount);
// Check getGroupMembers
List<UserModel> group1Members = session.users().getGroupMembers(appRealm, group1, 0, 10);
List<UserModel> group11Members = session.users().getGroupMembers(appRealm, group11, 0, 10);
List<UserModel> group12Members = session.users().getGroupMembers(appRealm, group12, 0, 10);
Assert.assertEquals(0, group1Members.size());
Assert.assertEquals(1, group11Members.size());
Assert.assertEquals("robkeycloak", group11Members.get(0).getUsername());
Assert.assertEquals(1, group12Members.size());
Assert.assertEquals("robkeycloak", group12Members.get(0).getUsername());
// Delete some group mappings in LDAP and check that it doesn't have any effect and user still has groups
LDAPObject ldapGroup = groupMapper.loadLDAPGroupByName("group11");
groupMapper.deleteGroupMappingInLDAP(robLdap, ldapGroup);
@ -428,6 +439,17 @@ public class LDAPGroupMapperTest extends AbstractLDAPTest {
Assert.assertTrue(robGroups.contains(group11));
Assert.assertTrue(robGroups.contains(group12));
// Check getGroupMembers
group1Members = session.users().getGroupMembers(appRealm, group1, 0, 10);
group11Members = session.users().getGroupMembers(appRealm, group11, 0, 10);
group12Members = session.users().getGroupMembers(appRealm, group12, 0, 10);
Assert.assertEquals(0, group1Members.size());
Assert.assertEquals(1, group11Members.size());
Assert.assertEquals("robkeycloak", group11Members.get(0).getUsername());
Assert.assertEquals(1, group12Members.size());
Assert.assertEquals("robkeycloak", group12Members.get(0).getUsername());
// Delete group mappings through model and verifies that user doesn't have them anymore
rob.leaveGroup(group11);
rob.leaveGroup(group12);