From c3e08306e4a9e4d2e906e1e1a4301293857815c1 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Wed, 15 Oct 2025 17:14:35 +0200 Subject: [PATCH 1/3] add isPerson check to query --- .../org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java | 3 +++ .../org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java index e96606dca2f9..41268a4430de 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java @@ -58,6 +58,8 @@ public List getUsersInGroup(String groupName, LdapContext context, Lon } String generateADGroupSearchFilter(String groupName, Long domainId) { + final String isPersonFilter = "(objectCategory=person)"; + final StringBuilder userObjectFilter = new StringBuilder(); userObjectFilter.append("(objectClass="); userObjectFilter.append(_ldapConfiguration.getUserObject(domainId)); @@ -71,6 +73,7 @@ String generateADGroupSearchFilter(String groupName, Long domainId) { final StringBuilder result = new StringBuilder(); result.append("(&"); + result.append(isPersonFilter); result.append(userObjectFilter); result.append(memberOfFilter); result.append(")"); diff --git a/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java b/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java index 58b14ec3684e..8b69e9d7280f 100644 --- a/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java +++ b/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java @@ -54,7 +54,7 @@ public void testGenerateADSearchFilterWithNestedGroupsEnabled() { String [] groups = {"dev", "dev-hyd"}; for (String group: groups) { String result = adLdapUserManager.generateADGroupSearchFilter(group, 1L); - assertTrue(("(&(objectClass=user)(memberOf:1.2.840.113556.1.4.1941:=CN=" + group + ",DC=cloud,DC=citrix,DC=com))").equals(result)); + assertTrue(("(&(objectCategory=person)(objectClass=user)(memberOf:1.2.840.113556.1.4.1941:=CN=" + group + ",DC=cloud,DC=citrix,DC=com))").equals(result)); } } @@ -69,7 +69,7 @@ public void testGenerateADSearchFilterWithNestedGroupsDisabled() { String [] groups = {"dev", "dev-hyd"}; for (String group: groups) { String result = adLdapUserManager.generateADGroupSearchFilter(group, 1L); - assertTrue(("(&(objectClass=user)(memberOf=CN=" + group + ",DC=cloud,DC=citrix,DC=com))").equals(result)); + assertTrue(("(&(objectCategory=person)(objectClass=user)(memberOf=CN=" + group + ",DC=cloud,DC=citrix,DC=com))").equals(result)); } } From 613beaf75689e74a4c943133a3c76e061a3ed1dd Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 16 Oct 2025 16:58:53 +0200 Subject: [PATCH 2/3] space --- .../org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java b/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java index 8b69e9d7280f..fb0004640ad0 100644 --- a/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java +++ b/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java @@ -56,7 +56,6 @@ public void testGenerateADSearchFilterWithNestedGroupsEnabled() { String result = adLdapUserManager.generateADGroupSearchFilter(group, 1L); assertTrue(("(&(objectCategory=person)(objectClass=user)(memberOf:1.2.840.113556.1.4.1941:=CN=" + group + ",DC=cloud,DC=citrix,DC=com))").equals(result)); } - } @Test From ac14522e0416c9c7157877d3670c48caa342b8b1 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Mon, 20 Oct 2025 15:50:16 +0200 Subject: [PATCH 3/3] created an ad specific usertype filter --- .../ldap/ADLdapUserManagerImpl.java | 19 ++- .../ldap/OpenLdapUserManagerImpl.java | 139 ++++++++---------- .../ldap/ADLdapUserManagerImplTest.java | 4 +- 3 files changed, 77 insertions(+), 85 deletions(-) diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java index 41268a4430de..bf5d503e8416 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java @@ -49,7 +49,7 @@ public List getUsersInGroup(String groupName, LdapContext context, Lon searchControls.setReturningAttributes(_ldapConfiguration.getReturnAttributes(domainId)); NamingEnumeration results = context.search(basedn, generateADGroupSearchFilter(groupName, domainId), searchControls); - final List users = new ArrayList(); + final List users = new ArrayList<>(); while (results.hasMoreElements()) { final SearchResult result = results.nextElement(); users.add(createUser(result, domainId)); @@ -58,12 +58,8 @@ public List getUsersInGroup(String groupName, LdapContext context, Lon } String generateADGroupSearchFilter(String groupName, Long domainId) { - final String isPersonFilter = "(objectCategory=person)"; - final StringBuilder userObjectFilter = new StringBuilder(); - userObjectFilter.append("(objectClass="); - userObjectFilter.append(_ldapConfiguration.getUserObject(domainId)); - userObjectFilter.append(")"); + final StringBuilder userObjectFilter = getUserObjectFilter(domainId); final StringBuilder memberOfFilter = new StringBuilder(); String groupCnName = _ldapConfiguration.getCommonNameAttribute() + "=" +groupName + "," + _ldapConfiguration.getBaseDn(domainId); @@ -73,15 +69,22 @@ String generateADGroupSearchFilter(String groupName, Long domainId) { final StringBuilder result = new StringBuilder(); result.append("(&"); - result.append(isPersonFilter); result.append(userObjectFilter); result.append(memberOfFilter); result.append(")"); - logger.debug("group search filter = " + result); + logger.debug("group search filter = {}", result); return result.toString(); } + StringBuilder getUserObjectFilter(Long domainId) { + final StringBuilder userObjectFilter = new StringBuilder(); + userObjectFilter.append("(&(objectCategory=person)"); + userObjectFilter.append(super.getUserObjectFilter(domainId)); + userObjectFilter.append(")"); + return userObjectFilter; + } + protected boolean isUserDisabled(SearchResult result) throws NamingException { boolean isDisabledUser = false; String userAccountControl = LdapUtils.getAttributeValue(result.getAttributes(), _ldapConfiguration.getUserAccountControlAttribute()); diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java index d0b6bc4bd34d..80d394d7478c 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java @@ -75,23 +75,15 @@ protected LdapUser createUser(final SearchResult result, Long domainId) throws N } private String generateSearchFilter(final String username, Long domainId) { - final StringBuilder userObjectFilter = new StringBuilder(); - userObjectFilter.append("(objectClass="); - userObjectFilter.append(_ldapConfiguration.getUserObject(domainId)); - userObjectFilter.append(")"); + final StringBuilder userObjectFilter = getUserObjectFilter(domainId); - final StringBuilder usernameFilter = new StringBuilder(); - usernameFilter.append("("); - usernameFilter.append(_ldapConfiguration.getUsernameAttribute(domainId)); - usernameFilter.append("="); - usernameFilter.append((username == null ? "*" : LdapUtils.escapeLDAPSearchFilter(username))); - usernameFilter.append(")"); + final StringBuilder usernameFilter = getUsernameFilter(username, domainId); String memberOfAttribute = getMemberOfAttribute(domainId); StringBuilder ldapGroupsFilter = new StringBuilder(); // this should get the trustmaps for this domain List ldapGroups = getMappedLdapGroups(domainId); - if (null != ldapGroups && ldapGroups.size() > 0) { + if (!ldapGroups.isEmpty()) { ldapGroupsFilter.append("(|"); for (String ldapGroup : ldapGroups) { ldapGroupsFilter.append(getMemberOfGroupString(ldapGroup, memberOfAttribute)); @@ -104,21 +96,35 @@ private String generateSearchFilter(final String username, Long domainId) { if (null != pricipleGroup) { principleGroupFilter.append(getMemberOfGroupString(pricipleGroup, memberOfAttribute)); } - final StringBuilder result = new StringBuilder(); - result.append("(&"); - result.append(userObjectFilter); - result.append(usernameFilter); - result.append(ldapGroupsFilter); - result.append(principleGroupFilter); - result.append(")"); - - String returnString = result.toString(); - if (logger.isTraceEnabled()) { - logger.trace("constructed ldap query: " + returnString); - } + + String returnString = "(&" + + userObjectFilter + + usernameFilter + + ldapGroupsFilter + + principleGroupFilter + + ")"; + logger.trace("constructed ldap query: {}", returnString); return returnString; } + private StringBuilder getUsernameFilter(String username, Long domainId) { + final StringBuilder usernameFilter = new StringBuilder(); + usernameFilter.append("("); + usernameFilter.append(_ldapConfiguration.getUsernameAttribute(domainId)); + usernameFilter.append("="); + usernameFilter.append((username == null ? "*" : LdapUtils.escapeLDAPSearchFilter(username))); + usernameFilter.append(")"); + return usernameFilter; + } + + StringBuilder getUserObjectFilter(Long domainId) { + final StringBuilder userObjectFilter = new StringBuilder(); + userObjectFilter.append("(objectClass="); + userObjectFilter.append(_ldapConfiguration.getUserObject(domainId)); + userObjectFilter.append(")"); + return userObjectFilter; + } + private List getMappedLdapGroups(Long domainId) { List ldapGroups = new ArrayList<>(); // first get the trustmaps @@ -134,37 +140,31 @@ private List getMappedLdapGroups(Long domainId) { private String getMemberOfGroupString(String group, String memberOfAttribute) { final StringBuilder memberOfFilter = new StringBuilder(); if (null != group) { - if(logger.isDebugEnabled()) { - logger.debug("adding search filter for '" + group + - "', using '" + memberOfAttribute + "'"); - } - memberOfFilter.append("(" + memberOfAttribute + "="); - memberOfFilter.append(group); - memberOfFilter.append(")"); + logger.debug("adding search filter for '{}', using '{}'", group, memberOfAttribute); + memberOfFilter.append("(") + .append(memberOfAttribute) + .append("=") + .append(group) + .append(")"); } return memberOfFilter.toString(); } private String generateGroupSearchFilter(final String groupName, Long domainId) { - final StringBuilder groupObjectFilter = new StringBuilder(); - groupObjectFilter.append("(objectClass="); - groupObjectFilter.append(_ldapConfiguration.getGroupObject(domainId)); - groupObjectFilter.append(")"); - - final StringBuilder groupNameFilter = new StringBuilder(); - groupNameFilter.append("("); - groupNameFilter.append(_ldapConfiguration.getCommonNameAttribute()); - groupNameFilter.append("="); - groupNameFilter.append((groupName == null ? "*" : LdapUtils.escapeLDAPSearchFilter(groupName))); - groupNameFilter.append(")"); - - final StringBuilder result = new StringBuilder(); - result.append("(&"); - result.append(groupObjectFilter); - result.append(groupNameFilter); - result.append(")"); - - return result.toString(); + String groupObjectFilter = "(objectClass=" + + _ldapConfiguration.getGroupObject(domainId) + + ")"; + + String groupNameFilter = "(" + + _ldapConfiguration.getCommonNameAttribute() + + "=" + + (groupName == null ? "*" : LdapUtils.escapeLDAPSearchFilter(groupName)) + + ")"; + + return "(&" + + groupObjectFilter + + groupNameFilter + + ")"; } @Override @@ -186,17 +186,9 @@ public LdapUser getUser(final String username, final String type, final String n basedn = _ldapConfiguration.getBaseDn(domainId); } - final StringBuilder userObjectFilter = new StringBuilder(); - userObjectFilter.append("(objectClass="); - userObjectFilter.append(_ldapConfiguration.getUserObject(domainId)); - userObjectFilter.append(")"); + final StringBuilder userObjectFilter = getUserObjectFilter(domainId); - final StringBuilder usernameFilter = new StringBuilder(); - usernameFilter.append("("); - usernameFilter.append(_ldapConfiguration.getUsernameAttribute(domainId)); - usernameFilter.append("="); - usernameFilter.append((username == null ? "*" : LdapUtils.escapeLDAPSearchFilter(username))); - usernameFilter.append(")"); + final StringBuilder usernameFilter = getUsernameFilter(username, domainId); final StringBuilder memberOfFilter = new StringBuilder(); if ("GROUP".equals(type)) { @@ -205,18 +197,17 @@ public LdapUser getUser(final String username, final String type, final String n memberOfFilter.append(")"); } - final StringBuilder searchQuery = new StringBuilder(); - searchQuery.append("(&"); - searchQuery.append(userObjectFilter); - searchQuery.append(usernameFilter); - searchQuery.append(memberOfFilter); - searchQuery.append(")"); + String searchQuery = "(&" + + userObjectFilter + + usernameFilter + + memberOfFilter + + ")"; - return searchUser(basedn, searchQuery.toString(), context, domainId); + return searchUser(basedn, searchQuery, context, domainId); } protected String getMemberOfAttribute(final Long domainId) { - return _ldapConfiguration.getUserMemberOfAttribute(domainId); + return LdapConfiguration.getUserMemberOfAttribute(domainId); } @Override @@ -243,7 +234,7 @@ public List getUsersInGroup(String groupName, LdapContext context, Lon NamingEnumeration result = context.search(_ldapConfiguration.getBaseDn(domainId), generateGroupSearchFilter(groupName, domainId), controls); - final List users = new ArrayList(); + final List users = new ArrayList<>(); //Expecting only one result which has all the users if (result.hasMoreElements()) { Attribute attribute = result.nextElement().getAttributes().get(attributeName); @@ -254,7 +245,7 @@ public List getUsersInGroup(String groupName, LdapContext context, Lon try{ users.add(getUserForDn(userdn, context, domainId)); } catch (NamingException e){ - logger.info("Userdn: " + userdn + " Not Found:: Exception message: " + e.getMessage()); + logger.info("Userdn: {} Not Found:: Exception message: {}", userdn, e.getMessage()); } } } @@ -286,17 +277,15 @@ protected boolean isUserDisabled(SearchResult result) throws NamingException { return false; } - public LdapUser searchUser(final String basedn, final String searchString, final LdapContext context, Long domainId) throws NamingException, IOException { + public LdapUser searchUser(final String basedn, final String searchString, final LdapContext context, Long domainId) throws NamingException { final SearchControls searchControls = new SearchControls(); searchControls.setSearchScope(_ldapConfiguration.getScope()); searchControls.setReturningAttributes(_ldapConfiguration.getReturnAttributes(domainId)); NamingEnumeration results = context.search(basedn, searchString, searchControls); - if(logger.isDebugEnabled()) { - logger.debug("searching user(s) with filter: \"" + searchString + "\""); - } - final List users = new ArrayList(); + logger.debug("searching user(s) with filter: \"{}\"", searchString); + final List users = new ArrayList<>(); while (results.hasMoreElements()) { final SearchResult result = results.nextElement(); users.add(createUser(result, domainId)); @@ -324,7 +313,7 @@ public List searchUsers(final String username, final LdapContext conte byte[] cookie = null; int pageSize = _ldapConfiguration.getLdapPageSize(domainId); context.setRequestControls(new Control[]{new PagedResultsControl(pageSize, Control.NONCRITICAL)}); - final List users = new ArrayList(); + final List users = new ArrayList<>(); NamingEnumeration results; do { results = context.search(basedn, generateSearchFilter(username, domainId), searchControls); diff --git a/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java b/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java index fb0004640ad0..f2ac1dffaf95 100644 --- a/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java +++ b/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java @@ -54,7 +54,7 @@ public void testGenerateADSearchFilterWithNestedGroupsEnabled() { String [] groups = {"dev", "dev-hyd"}; for (String group: groups) { String result = adLdapUserManager.generateADGroupSearchFilter(group, 1L); - assertTrue(("(&(objectCategory=person)(objectClass=user)(memberOf:1.2.840.113556.1.4.1941:=CN=" + group + ",DC=cloud,DC=citrix,DC=com))").equals(result)); + assertTrue(("(&(&(objectCategory=person)(objectClass=user))(memberOf:1.2.840.113556.1.4.1941:=CN=" + group + ",DC=cloud,DC=citrix,DC=com))").equals(result)); } } @@ -68,7 +68,7 @@ public void testGenerateADSearchFilterWithNestedGroupsDisabled() { String [] groups = {"dev", "dev-hyd"}; for (String group: groups) { String result = adLdapUserManager.generateADGroupSearchFilter(group, 1L); - assertTrue(("(&(objectCategory=person)(objectClass=user)(memberOf=CN=" + group + ",DC=cloud,DC=citrix,DC=com))").equals(result)); + assertTrue(("(&(&(objectCategory=person)(objectClass=user))(memberOf=CN=" + group + ",DC=cloud,DC=citrix,DC=com))").equals(result)); } }