Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public List<LdapUser> getUsersInGroup(String groupName, LdapContext context, Lon
searchControls.setReturningAttributes(_ldapConfiguration.getReturnAttributes(domainId));

NamingEnumeration<SearchResult> results = context.search(basedn, generateADGroupSearchFilter(groupName, domainId), searchControls);
final List<LdapUser> users = new ArrayList<LdapUser>();
final List<LdapUser> users = new ArrayList<>();
while (results.hasMoreElements()) {
final SearchResult result = results.nextElement();
users.add(createUser(result, domainId));
Expand All @@ -58,10 +58,8 @@ public List<LdapUser> getUsersInGroup(String groupName, LdapContext context, Lon
}

String generateADGroupSearchFilter(String groupName, Long domainId) {
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);
Expand All @@ -75,10 +73,18 @@ String generateADGroupSearchFilter(String groupName, Long domainId) {
result.append(memberOfFilter);
result.append(")");

logger.debug("group search filter = " + result);
logger.debug("group search filter = {}", result);
return result.toString();
}

Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method should be marked with @Override annotation since it overrides the parent class method in OpenLdapUserManagerImpl. Adding the annotation makes the inheritance relationship explicit and allows the compiler to verify the override.

Suggested change
@Override

Copilot uses AI. Check for mistakes.
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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> ldapGroups = getMappedLdapGroups(domainId);
if (null != ldapGroups && ldapGroups.size() > 0) {
if (!ldapGroups.isEmpty()) {
ldapGroupsFilter.append("(|");
for (String ldapGroup : ldapGroups) {
ldapGroupsFilter.append(getMemberOfGroupString(ldapGroup, memberOfAttribute));
Expand All @@ -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<String> getMappedLdapGroups(Long domainId) {
List <String> ldapGroups = new ArrayList<>();
// first get the trustmaps
Expand All @@ -134,37 +140,31 @@ private List<String> 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
Expand All @@ -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)) {
Expand All @@ -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);
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling getUserMemberOfAttribute() statically on LdapConfiguration suggests this method was changed from instance to static. If this is an instance method being called statically, this will cause a compilation error. Verify that the method signature has been properly updated to be static, or revert to calling it on the _ldapConfiguration instance.

Suggested change
return LdapConfiguration.getUserMemberOfAttribute(domainId);
return _ldapConfiguration.getUserMemberOfAttribute(domainId);

Copilot uses AI. Check for mistakes.
}

@Override
Expand All @@ -243,7 +234,7 @@ public List<LdapUser> getUsersInGroup(String groupName, LdapContext context, Lon

NamingEnumeration<SearchResult> result = context.search(_ldapConfiguration.getBaseDn(domainId), generateGroupSearchFilter(groupName, domainId), controls);

final List<LdapUser> users = new ArrayList<LdapUser>();
final List<LdapUser> users = new ArrayList<>();
//Expecting only one result which has all the users
if (result.hasMoreElements()) {
Attribute attribute = result.nextElement().getAttributes().get(attributeName);
Expand All @@ -254,7 +245,7 @@ public List<LdapUser> 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());
}
}
}
Expand Down Expand Up @@ -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 {
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing IOException from the throws clause is a breaking change to the method signature. If any calling code explicitly catches IOException from this method, it will cause compilation errors. Verify that all callers have been updated accordingly, or consider whether this exception should remain for backward compatibility.

Copilot uses AI. Check for mistakes.
final SearchControls searchControls = new SearchControls();

searchControls.setSearchScope(_ldapConfiguration.getScope());
searchControls.setReturningAttributes(_ldapConfiguration.getReturnAttributes(domainId));

NamingEnumeration<SearchResult> results = context.search(basedn, searchString, searchControls);
if(logger.isDebugEnabled()) {
logger.debug("searching user(s) with filter: \"" + searchString + "\"");
}
final List<LdapUser> users = new ArrayList<LdapUser>();
logger.debug("searching user(s) with filter: \"{}\"", searchString);
final List<LdapUser> users = new ArrayList<>();
while (results.hasMoreElements()) {
final SearchResult result = results.nextElement();
users.add(createUser(result, domainId));
Expand Down Expand Up @@ -324,7 +313,7 @@ public List<LdapUser> 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<LdapUser> users = new ArrayList<LdapUser>();
final List<LdapUser> users = new ArrayList<>();
NamingEnumeration<SearchResult> results;
do {
results = context.search(basedn, generateSearchFilter(username, domainId), searchControls);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,8 @@ 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));
}

}

@Test
Expand All @@ -69,7 +68,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));
}
}

Expand Down
Loading