Skip to content

LDAP: honour nested groups for MSAD #11696

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 24, 2025
Merged

LDAP: honour nested groups for MSAD #11696

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bde238a
call msad if needed
Sep 22, 2025
dd376a3
Update log statement for nested search in MSAD
DaanHoogland Sep 24, 2025
1d22549
wrong double quote
DaanHoogland Sep 24, 2025
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
8 changes: 6 additions & 2 deletions ...r-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,14 @@ protected boolean isUserDisabled(SearchResult result) throws NamingException {
}

protected String getMemberOfAttribute(final Long domainId) {
String rc;
if(_ldapConfiguration.isNestedGroupsEnabled(domainId)) {
return MICROSOFT_AD_NESTED_MEMBERS_FILTER;
rc = MICROSOFT_AD_NESTED_MEMBERS_FILTER;
} else {
return MICROSOFT_AD_MEMBERS_FILTER;
rc = MICROSOFT_AD_MEMBERS_FILTER;
}
logger.trace(“using memberOf filter = {} for domain with id {}”, rc, domainId);

return rc;
}
}
5 changes: 4 additions & 1 deletion .../user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,12 @@

import com.cloud.utils.Pair;
import org.apache.cloudstack.ldap.dao.LdapConfigurationDao;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

public class LdapConfiguration implements Configurable{
private final static String factory = "com.sun.jndi.ldap.LdapCtxFactory";
protected Logger logger = LogManager.getLogger(getClass());

private static final ConfigKey<Long> ldapReadTimeout = new ConfigKey<Long>(
Long.class,
Expand Down Expand Up @@ -325,7 +328,7 @@ public LdapUserManager.Provider getLdapProvider(final Long domainId) {
try {
provider = LdapUserManager.Provider.valueOf(ldapProvider.valueIn(domainId).toUpperCase());
} catch (IllegalArgumentException ex) {
//openldap is the default
logger.warn("no LDAP provider found for domain {}, using openldap as default", domainId);
provider = LdapUserManager.Provider.OPENLDAP;
}
return provider;
Expand Down
4 changes: 2 additions & 2 deletions ...authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ protected LdapUser createUser(final SearchResult result, Long domainId) throws N
final String firstname = LdapUtils.getAttributeValue(attributes, _ldapConfiguration.getFirstnameAttribute(domainId));
final String lastname = LdapUtils.getAttributeValue(attributes, _ldapConfiguration.getLastnameAttribute(domainId));
final String principal = result.getNameInNamespace();
final List<String> memberships = LdapUtils.getAttributeValues(attributes, _ldapConfiguration.getUserMemberOfAttribute(domainId));
final List<String> memberships = LdapUtils.getAttributeValues(attributes, getMemberOfAttribute(domainId));

String domain = principal.replace("cn=" + LdapUtils.getAttributeValue(attributes, _ldapConfiguration.getCommonNameAttribute()) + ",", "");
domain = domain.replace("," + _ldapConfiguration.getBaseDn(domainId), "");
Expand All @@ -87,7 +87,7 @@ private String generateSearchFilter(final String username, Long domainId) {
usernameFilter.append((username == null ? "*" : LdapUtils.escapeLDAPSearchFilter(username)));
usernameFilter.append(")");

String memberOfAttribute = _ldapConfiguration.getUserMemberOfAttribute(domainId);
String memberOfAttribute = getMemberOfAttribute(domainId);
Copy link
Member

@weizhouapache weizhouapache Sep 23, 2025

Choose a reason for hiding this comment

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

@DaanHoogland
there is a method in the same file

    protected String getMemberOfAttribute(final Long domainId) {
        return _ldapConfiguration.getUserMemberOfAttribute(domainId);
    }

which is used only once

        if ("GROUP".equals(type)) {
            memberOfFilter.append("(").append(getMemberOfAttribute(domainId)).append("=");
            memberOfFilter.append(name);
            memberOfFilter.append(")"); 
        }

is it correct ?

Copy link
Contributor Author

@DaanHoogland DaanHoogland Sep 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

yes, but it is overloaded in the AD user manager, so at this point it is not known which one will be called.

StringBuilder ldapGroupsFilter = new StringBuilder();
// this should get the trustmaps for this domain
List<String> ldapGroups = getMappedLdapGroups(domainId);
Expand Down
Loading