keycloak-aplcache

[KEYCLOAK-5052] - LDAP group names containing / in the name

11/22/2018 5:55:40 PM

Details

diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPOperationManager.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPOperationManager.java
index 83f4b2d..f4b8186 100644
--- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPOperationManager.java
+++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPOperationManager.java
@@ -42,6 +42,7 @@ import javax.naming.directory.SearchResult;
 import javax.naming.ldap.Control;
 import javax.naming.ldap.InitialLdapContext;
 import javax.naming.ldap.LdapContext;
+import javax.naming.ldap.LdapName;
 import javax.naming.ldap.PagedResultsControl;
 import javax.naming.ldap.PagedResultsResponseControl;
 import java.io.IOException;
@@ -195,7 +196,7 @@ public class LDAPOperationManager {
                     int max = 5;
                     for (int i=0 ; i<max ; i++) {
                         try {
-                            context.rename(oldDn, dn);
+                            context.rename(new LdapName(oldDn), new LdapName(dn));
                             return dn;
                         } catch (NameAlreadyBoundException ex) {
                             if (!fallback) {
@@ -250,7 +251,7 @@ public class LDAPOperationManager {
             return execute(new LdapOperation<List<SearchResult>>() {
                 @Override
                 public List<SearchResult> execute(LdapContext context) throws NamingException {
-                    NamingEnumeration<SearchResult> search = context.search(baseDN, filter, cons);
+                    NamingEnumeration<SearchResult> search = context.search(new LdapName(baseDN), filter, cons);
 
                     while (search.hasMoreElements()) {
                         result.add(search.nextElement());
@@ -295,7 +296,7 @@ public class LDAPOperationManager {
                         PagedResultsControl pagedControls = new PagedResultsControl(identityQuery.getLimit(), cookie, Control.CRITICAL);
                         context.setRequestControls(new Control[] { pagedControls });
 
-                        NamingEnumeration<SearchResult> search = context.search(baseDN, filter, cons);
+                        NamingEnumeration<SearchResult> search = context.search(new LdapName(baseDN), filter, cons);
 
                         while (search.hasMoreElements()) {
                             result.add(search.nextElement());
@@ -407,7 +408,7 @@ public class LDAPOperationManager {
 
                 @Override
                 public SearchResult execute(LdapContext context) throws NamingException {
-                    NamingEnumeration<SearchResult> search = context.search(baseDN, filter, cons);
+                    NamingEnumeration<SearchResult> search = context.search(new LdapName(baseDN), filter, cons);
 
                     try {
                         if (search.hasMoreElements()) {
@@ -451,7 +452,7 @@ public class LDAPOperationManager {
             NamingEnumeration<Binding> enumeration = null;
 
             try {
-                enumeration = context.listBindings(dn);
+                enumeration = context.listBindings(new LdapName(dn));
 
                 while (enumeration.hasMore()) {
                     Binding binding = enumeration.next();
@@ -460,7 +461,7 @@ public class LDAPOperationManager {
                     destroySubcontext(context, name);
                 }
 
-                context.unbind(dn);
+                context.unbind(new LdapName(dn));
             } finally {
                 try {
                     enumeration.close();
@@ -550,7 +551,7 @@ public class LDAPOperationManager {
 
                 @Override
                 public Void execute(LdapContext context) throws NamingException {
-                    context.modifyAttributes(dn, mods);
+                    context.modifyAttributes(new LdapName(dn), mods);
                     return null;
                 }
 
@@ -595,7 +596,7 @@ public class LDAPOperationManager {
             execute(new LdapOperation<Void>() {
                 @Override
                 public Void execute(LdapContext context) throws NamingException {
-                    DirContext subcontext = context.createSubcontext(name, attributes);
+                    DirContext subcontext = context.createSubcontext(new LdapName(name), attributes);
 
                     subcontext.close();
 
diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java b/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java
index 2846917..1046d8d 100755
--- a/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java
+++ b/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java
@@ -55,7 +55,6 @@ import java.security.PublicKey;
 import java.security.SecureRandom;
 import java.security.cert.X509Certificate;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -432,15 +431,18 @@ public final class KeycloakModelUtils {
     }
 
 
-    private static GroupModel findSubGroup(String[] path, int index, GroupModel parent) {
+    private static GroupModel findSubGroup(String[] segments, int index, GroupModel parent) {
         for (GroupModel group : parent.getSubGroups()) {
-            if (group.getName().equals(path[index])) {
-                if (path.length == index + 1) {
+            String groupName = group.getName();
+            String[] pathSegments = formatPathSegments(segments, index, groupName);
+
+            if (groupName.equals(pathSegments[index])) {
+                if (pathSegments.length == index + 1) {
                     return group;
                 }
                 else {
-                    if (index + 1 < path.length) {
-                        GroupModel found = findSubGroup(path, index + 1, group);
+                    if (index + 1 < pathSegments.length) {
+                        GroupModel found = findSubGroup(pathSegments, index + 1, group);
                         if (found != null) return found;
                     } else {
                         return null;
@@ -452,6 +454,44 @@ public final class KeycloakModelUtils {
         return null;
     }
 
+    /**
+     * Given the {@code pathParts} of a group with the given {@code groupName}, format the {@pathParts} in order to ignore
+     * group names containing a {@code /} character.
+     *
+     * @param segments the path segments
+     * @param index the index pointing to the position to start looking for the group name
+     * @param groupName the groupName
+     * @return a new array of strings with the correct segments in case the group has a name containing slashes
+     */
+    private static String[] formatPathSegments(String[] segments, int index, String groupName) {
+        String[] nameSegments = groupName.split("/");
+
+        if (nameSegments.length > 1 && segments.length >= nameSegments.length) {
+            for (int i = 0; i < nameSegments.length; i++) {
+                if (!nameSegments[i].equals(segments[index + i])) {
+                    return segments;
+                }
+            }
+
+            int numMergedIndexes = nameSegments.length - 1;
+            String[] newPath = new String[segments.length - numMergedIndexes];
+
+            for (int i = 0; i < newPath.length; i++) {
+                if (i == index) {
+                    newPath[i] = groupName;
+                } else if (i > index) {
+                    newPath[i] = segments[i + numMergedIndexes];
+                } else {
+                    newPath[i] = segments[i];
+                }
+            }
+
+            return newPath;
+        }
+
+        return segments;
+    }
+
     public static GroupModel findGroupByPath(RealmModel realm, String path) {
         if (path == null) {
             return null;
@@ -466,14 +506,17 @@ public final class KeycloakModelUtils {
         if (split.length == 0) return null;
         GroupModel found = null;
         for (GroupModel group : realm.getTopLevelGroups()) {
-            if (group.getName().equals(split[0])) {
-                if (split.length == 1) {
+            String groupName = group.getName();
+            String[] pathSegments = formatPathSegments(split, 0, groupName);
+
+            if (groupName.equals(pathSegments[0])) {
+                if (pathSegments.length == 1) {
                     found = group;
                     break;
                 }
                 else {
-                    if (split.length > 1) {
-                        found = findSubGroup(split, 1, group);
+                    if (pathSegments.length > 1) {
+                        found = findSubGroup(pathSegments, 1, group);
                         if (found != null) break;
                     }
                 }
diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/resource/TestLDAPResource.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/resource/TestLDAPResource.java
index f4c7f68..246b2e0 100644
--- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/resource/TestLDAPResource.java
+++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/resource/TestLDAPResource.java
@@ -132,12 +132,27 @@ public class TestLDAPResource {
         LDAPObject defaultGroup1 = LDAPTestUtils.createLDAPGroup(session, realm, ldapModel, "defaultGroup1", descriptionAttrName, "Default Group1 - description");
         LDAPObject defaultGroup11 = LDAPTestUtils.createLDAPGroup(session, realm, ldapModel, "defaultGroup11");
         LDAPObject defaultGroup12 = LDAPTestUtils.createLDAPGroup(session, realm, ldapModel, "defaultGroup12", descriptionAttrName, "Default Group12 - description");
+        LDAPTestUtils.createLDAPGroup(session, realm, ldapModel, "Team 2016/2017", descriptionAttrName, "A group with slashes in the name");
+        LDAPObject teamChild20182019 = LDAPTestUtils.createLDAPGroup(session, realm, ldapModel, "Team Child 2018/2019", descriptionAttrName, "A child group with slashes in the name");
+        LDAPObject teamSubChild20202021 = LDAPTestUtils.createLDAPGroup(session, realm, ldapModel, "Team SubChild 2020/2021", descriptionAttrName, "A sub child group with slashes in the name");
+        LDAPObject defaultGroup13 = LDAPTestUtils.createLDAPGroup(session, realm, ldapModel, "defaultGroup13", descriptionAttrName, "Default Group13 - description");
+        LDAPObject teamSubChild20222023 = LDAPTestUtils.createLDAPGroup(session, realm, ldapModel, "Team SubChild 2022/2023/A/B/C/D/E", descriptionAttrName, "A sub child group with slashes in the name");
+        LDAPObject defaultGroup14 = LDAPTestUtils.createLDAPGroup(session, realm, ldapModel, "defaultGroup14", descriptionAttrName, "Default Group14 - description");
+        LDAPObject teamRoot20242025 = LDAPTestUtils.createLDAPGroup(session, realm, ldapModel, "Team Root 2024/2025/A/B/C/D", descriptionAttrName, "A sub child group with slashes in the name");
+        LDAPObject defaultGroup15 = LDAPTestUtils.createLDAPGroup(session, realm, ldapModel, "defaultGroup15", descriptionAttrName, "Default Group15 - description");
+        LDAPObject teamSubChild20262027 = LDAPTestUtils.createLDAPGroup(session, realm, ldapModel, "Team SubChild 2026/2027", descriptionAttrName, "A sub child group with slashes in the name");
 
         LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, "not-used", group1, group11, false);
         LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, "not-used", group1, group12, true);
 
         LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, "not-used", defaultGroup1, defaultGroup11, false);
         LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, "not-used", defaultGroup1, defaultGroup12, true);
+        LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, "not-used", defaultGroup1, teamChild20182019, true);
+        LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, "not-used", teamChild20182019, teamSubChild20202021, true);
+        LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, "not-used", defaultGroup13, teamSubChild20222023, true);
+        LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, "not-used", teamSubChild20222023, defaultGroup14, true);
+        LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, "not-used", teamRoot20242025, defaultGroup15, true);
+        LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, "not-used", defaultGroup15, teamSubChild20262027, true);
 
         // Sync LDAP groups to Keycloak DB
         ComponentModel mapperModel = LDAPTestUtils.getSubcomponentByName(realm, ldapModel, "groupsMapper");
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperTest.java
index b052c09..76b227b 100755
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperTest.java
@@ -116,6 +116,20 @@ public class LDAPGroupMapperTest extends AbstractLDAPTest {
             GroupModel group12 = KeycloakModelUtils.findGroupByPath(appRealm, "/group1/group12");
             john.joinGroup(group12);
             mary.joinGroup(group12);
+
+            // This group should already exists as it was imported from LDAP
+            GroupModel groupWithSlashesInName = KeycloakModelUtils.findGroupByPath(appRealm, "Team 2016/2017");
+            john.joinGroup(groupWithSlashesInName);
+            mary.joinGroup(groupWithSlashesInName);
+
+            // This group should already exists as it was imported from LDAP
+            GroupModel groupChildWithSlashesInName = KeycloakModelUtils.findGroupByPath(appRealm, "defaultGroup1/Team Child 2018/2019");
+            john.joinGroup(groupChildWithSlashesInName);
+            mary.joinGroup(groupChildWithSlashesInName);
+
+            Assert.assertEquals("Team SubChild 2020/2021", KeycloakModelUtils.findGroupByPath(appRealm, "defaultGroup1/Team Child 2018/2019/Team SubChild 2020/2021").getName());
+            Assert.assertEquals("defaultGroup14", KeycloakModelUtils.findGroupByPath(appRealm, "defaultGroup13/Team SubChild 2022/2023/A/B/C/D/E/defaultGroup14").getName());
+            Assert.assertEquals("Team SubChild 2026/2027", KeycloakModelUtils.findGroupByPath(appRealm, "Team Root 2024/2025/A/B/C/D/defaultGroup15/Team SubChild 2026/2027").getName());
         });
 
 
@@ -155,16 +169,20 @@ public class LDAPGroupMapperTest extends AbstractLDAPTest {
             GroupModel group1 = KeycloakModelUtils.findGroupByPath(appRealm, "/group1");
             GroupModel group11 = KeycloakModelUtils.findGroupByPath(appRealm, "/group1/group11");
             GroupModel group12 = KeycloakModelUtils.findGroupByPath(appRealm, "/group1/group12");
+            GroupModel groupTeam20162017 = KeycloakModelUtils.findGroupByPath(appRealm, "Team 2016/2017");
+            GroupModel groupTeamChild20182019 = KeycloakModelUtils.findGroupByPath(appRealm, "defaultGroup1/Team Child 2018/2019");
             UserModel john = session.users().getUserByUsername("johnkeycloak", appRealm);
             UserModel mary = session.users().getUserByUsername("marykeycloak", appRealm);
 
             Set<GroupModel> johnGroups = john.getGroups();
-            Assert.assertEquals(2, johnGroups.size());
+            Assert.assertEquals(4, johnGroups.size());
             long groupCount = john.getGroupsCount();
-            Assert.assertEquals(2, groupCount);
+            Assert.assertEquals(4, groupCount);
             Assert.assertTrue(johnGroups.contains(group1));
             Assert.assertFalse(johnGroups.contains(group11));
             Assert.assertTrue(johnGroups.contains(group12));
+            Assert.assertTrue(johnGroups.contains(groupTeam20162017));
+            Assert.assertTrue(johnGroups.contains(groupTeamChild20182019));
 
             Set<GroupModel> johnGroupsWithGr = john.getGroups("gr", 0, 10);
             Assert.assertEquals(2, johnGroupsWithGr.size());
@@ -178,24 +196,38 @@ public class LDAPGroupMapperTest extends AbstractLDAPTest {
             Set<GroupModel> johnGroupsWith12 = john.getGroups("12", 0, 10);
             Assert.assertEquals(1, johnGroupsWith12.size());
 
+            Set<GroupModel> johnGroupsWith2017 = john.getGroups("2017", 0, 10);
+            Assert.assertEquals(1, johnGroupsWith2017.size());
+
+            Set<GroupModel> johnGroupsWith2018 = john.getGroups("2018", 0, 10);
+            Assert.assertEquals(1, johnGroupsWith2017.size());
+
             // 4 - Check through userProvider
             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);
+            List<UserModel> groupTeam20162017Members = session.users().getGroupMembers(appRealm, groupTeam20162017, 0, 10);
+            List<UserModel> groupTeam20182019Members = session.users().getGroupMembers(appRealm, groupTeamChild20182019, 0, 10);
 
             Assert.assertEquals(1, group1Members.size());
             Assert.assertEquals("johnkeycloak", group1Members.get(0).getUsername());
             Assert.assertEquals(1, group11Members.size());
             Assert.assertEquals("marykeycloak", group11Members.get(0).getUsername());
             Assert.assertEquals(2, group12Members.size());
+            Assert.assertEquals(2, groupTeam20162017Members.size());
+            Assert.assertEquals(2, groupTeam20182019Members.size());
 
             // 4 - Delete some group mappings and check they are deleted
 
             john.leaveGroup(group1);
             john.leaveGroup(group12);
+            john.leaveGroup(groupTeam20162017);
+            john.leaveGroup(groupTeamChild20182019);
 
             mary.leaveGroup(group1);
             mary.leaveGroup(group12);
+            mary.leaveGroup(groupTeam20162017);
+            mary.leaveGroup(groupTeamChild20182019);
 
             johnGroups = john.getGroups();
             Assert.assertEquals(0, johnGroups.size());
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPSpecialCharsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPSpecialCharsTest.java
index 68ba17a..2bc884a 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPSpecialCharsTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPSpecialCharsTest.java
@@ -96,7 +96,8 @@ public class LDAPSpecialCharsTest extends AbstractLDAPTest {
 
             String descriptionAttrName = getGroupDescriptionLDAPAttrName(ctx.getLdapProvider());
 
-            LDAPObject groupSpecialCharacters = LDAPTestUtils.createLDAPGroup(session, appRealm, ctx.getLdapModel(), "group-spec,ia*l_characžter)s", descriptionAttrName, "group-special-characters");
+            LDAPTestUtils.createLDAPGroup(session, appRealm, ctx.getLdapModel(), "group-spec,ia*l_characžter)s", descriptionAttrName, "group-special-characters");
+            LDAPTestUtils.createLDAPGroup(session, appRealm, ctx.getLdapModel(), "group/with/three/slashes", descriptionAttrName, "group-with-three-slashes");
 
             // Resync LDAP groups to Keycloak DB
             ComponentModel mapperModel = LDAPTestUtils.getSubcomponentByName(appRealm, ctx.getLdapModel(), "groupsMapper");
@@ -170,10 +171,15 @@ public class LDAPSpecialCharsTest extends AbstractLDAPTest {
 
             specialUser.joinGroup(specialGroup);
 
+            GroupModel groupWithSlashes = KeycloakModelUtils.findGroupByPath(appRealm, "/group/with/three/slashes");
+            Assert.assertNotNull(groupWithSlashes);
+
+            specialUser.joinGroup(groupWithSlashes);
+
             // 2 - Check that group mappings are in LDAP and hence available through federation
 
             Set<GroupModel> userGroups = specialUser.getGroups();
-            Assert.assertEquals(1, userGroups.size());
+            Assert.assertEquals(2, userGroups.size());
             Assert.assertTrue(userGroups.contains(specialGroup));
 
             // 3 - Check through userProvider
@@ -182,9 +188,15 @@ public class LDAPSpecialCharsTest extends AbstractLDAPTest {
             Assert.assertEquals(1, groupMembers.size());
             Assert.assertEquals("jamees,key*cložak)ppp", groupMembers.get(0).getUsername());
 
+            groupMembers = session.users().getGroupMembers(appRealm, groupWithSlashes, 0, 10);
+
+            Assert.assertEquals(1, groupMembers.size());
+            Assert.assertEquals("jamees,key*cložak)ppp", groupMembers.get(0).getUsername());
+
             // 4 - Delete some group mappings and check they are deleted
 
             specialUser.leaveGroup(specialGroup);
+            specialUser.leaveGroup(groupWithSlashes);
 
             userGroups = specialUser.getGroups();
             Assert.assertEquals(0, userGroups.size());