This is an automated email from the ASF dual-hosted git repository. DaanHoogland pushed a commit to branch ghi8732-ldap-createaccount-group-scope in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit cbc13c477daf62cae0f7e64dded2d9ec50e83a64 Author: Daan Hoogland <[email protected]> AuthorDate: Fri Aug 21 17:29:10 2026 +0200 allow creating ldap accounts after linking accounts to the same the domain --- .../cloudstack/ldap/OpenLdapUserManagerImpl.java | 35 ++++-- .../ldap/OpenLdapUserManagerImplTest.java | 130 +++++++++++++++++++++ 2 files changed, 155 insertions(+), 10 deletions(-) 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 d0b6bc4bd34..1eee84947a2 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 @@ -74,7 +74,12 @@ public class OpenLdapUserManagerImpl implements LdapUserManager { return new LdapUser(username, email, firstname, lastname, principal, domain, disabled, memberships); } - private String generateSearchFilter(final String username, Long domainId) { + /** + * @param restrictToLinkedGroups scope to groups already linked in this domain; only valid for + * browsing/importing. Applied to a single known username, it wrongly + * blocks creating one ldap account once another is linked to a group. + */ + private String generateSearchFilter(final String username, Long domainId, final boolean restrictToLinkedGroups) { final StringBuilder userObjectFilter = new StringBuilder(); userObjectFilter.append("(objectClass="); userObjectFilter.append(_ldapConfiguration.getUserObject(domainId)); @@ -89,14 +94,16 @@ public class OpenLdapUserManagerImpl implements LdapUserManager { 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) { - ldapGroupsFilter.append("(|"); - for (String ldapGroup : ldapGroups) { - ldapGroupsFilter.append(getMemberOfGroupString(ldapGroup, memberOfAttribute)); + if (restrictToLinkedGroups) { + // this should get the trustmaps for this domain + List<String> ldapGroups = getMappedLdapGroups(domainId); + if (null != ldapGroups && ldapGroups.size() > 0) { + ldapGroupsFilter.append("(|"); + for (String ldapGroup : ldapGroups) { + ldapGroupsFilter.append(getMemberOfGroupString(ldapGroup, memberOfAttribute)); + } + ldapGroupsFilter.append(')'); } - ldapGroupsFilter.append(')'); } // make sure only users in the principle group are retrieved String pricipleGroup = _ldapConfiguration.getSearchGroupPrinciple(domainId); @@ -167,9 +174,13 @@ public class OpenLdapUserManagerImpl implements LdapUserManager { return result.toString(); } + /** + * Looks up one known username, unscoped by linked groups, so an existing group link + * elsewhere in the domain can't block finding this user. + */ @Override public LdapUser getUser(final String username, final LdapContext context, Long domainId) throws NamingException, IOException { - List<LdapUser> result = searchUsers(username, context, domainId); + List<LdapUser> result = searchUsers(username, context, domainId, false); if (result!= null && result.size() == 1) { return result.get(0); } else { @@ -311,6 +322,10 @@ public class OpenLdapUserManagerImpl implements LdapUserManager { @Override public List<LdapUser> searchUsers(final String username, final LdapContext context, Long domainId) throws NamingException, IOException { + return searchUsers(username, context, domainId, true); + } + + private List<LdapUser> searchUsers(final String username, final LdapContext context, Long domainId, final boolean restrictToLinkedGroups) throws NamingException, IOException { final SearchControls searchControls = new SearchControls(); @@ -327,7 +342,7 @@ public class OpenLdapUserManagerImpl implements LdapUserManager { final List<LdapUser> users = new ArrayList<LdapUser>(); NamingEnumeration<SearchResult> results; do { - results = context.search(basedn, generateSearchFilter(username, domainId), searchControls); + results = context.search(basedn, generateSearchFilter(username, domainId, restrictToLinkedGroups), searchControls); while (results.hasMoreElements()) { final SearchResult result = results.nextElement(); if (!isUserDisabled(result)) { diff --git a/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/OpenLdapUserManagerImplTest.java b/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/OpenLdapUserManagerImplTest.java new file mode 100644 index 00000000000..dfbc5fd9e4c --- /dev/null +++ b/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/OpenLdapUserManagerImplTest.java @@ -0,0 +1,130 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.ldap; + +import com.cloud.user.Account; +import org.apache.cloudstack.ldap.dao.LdapTrustMapDao; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import javax.naming.NamingEnumeration; +import javax.naming.NamingException; +import javax.naming.directory.SearchControls; +import javax.naming.directory.SearchResult; +import javax.naming.ldap.LdapContext; + +import java.util.Collections; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Regression tests: creating an ldap account must not fail just because another + * account is already linked to an ldap group in the domain; browsing/importing + * still honours that group scope. + */ +@RunWith(MockitoJUnitRunner.class) +public class OpenLdapUserManagerImplTest { + + private static final Long DOMAIN_ID = 1L; + private static final String LINKED_GROUP = "cn=test admins,ou=groups,dc=my,dc=domain,dc=com"; + + private OpenLdapUserManagerImpl openLdapUserManager; + + private MockedStatic<LdapConfiguration> ldapConfigurationMockedStatic; + + @Mock + private LdapConfiguration ldapConfigurationMock; + + @Mock + private LdapTrustMapDao ldapTrustMapDaoMock; + + @Mock + private LdapContext ldapContextMock; + + @Before + public void setup() throws Exception { + // getUserMemberOfAttribute is static, unlike its LdapConfiguration siblings; mock statically. + ldapConfigurationMockedStatic = Mockito.mockStatic(LdapConfiguration.class, Mockito.CALLS_REAL_METHODS); + when(LdapConfiguration.getUserMemberOfAttribute(any())).thenReturn("memberOf"); + + openLdapUserManager = new OpenLdapUserManagerImpl(ldapConfigurationMock); + openLdapUserManager._ldapTrustMapDao = ldapTrustMapDaoMock; + + when(ldapConfigurationMock.getScope()).thenReturn(SearchControls.SUBTREE_SCOPE); + when(ldapConfigurationMock.getReturnAttributes(any())).thenReturn(new String[]{"uid"}); + when(ldapConfigurationMock.getSearchGroupPrinciple(any())).thenReturn(null); + when(ldapConfigurationMock.getBaseDn(any())).thenReturn("dc=my,dc=domain,dc=com"); + when(ldapConfigurationMock.getUsernameAttribute(any())).thenReturn("uid"); + when(ldapConfigurationMock.getUserObject(any())).thenReturn("inetOrgPerson"); + when(ldapConfigurationMock.getLdapPageSize(any())).thenReturn(1000); + + LdapTrustMapVO linkedGroup = new LdapTrustMapVO(DOMAIN_ID, LdapManager.LinkType.GROUP, LINKED_GROUP, Account.Type.NORMAL, 5L); + when(ldapTrustMapDaoMock.searchByDomainId(anyLong())).thenReturn(Collections.singletonList(linkedGroup)); + + NamingEnumeration<SearchResult> noResults = mock(NamingEnumeration.class); + when(noResults.hasMoreElements()).thenReturn(false); + when(ldapContextMock.search(any(String.class), any(String.class), any(SearchControls.class))).thenReturn(noResults); + when(ldapContextMock.getResponseControls()).thenReturn(null); + } + + @After + public void tearDown() { + ldapConfigurationMockedStatic.close(); + } + + @Test + public void getUserDoesNotRestrictToAlreadyLinkedGroups() throws Exception { + try { + openLdapUserManager.getUser("test_user", ldapContextMock, DOMAIN_ID); + } catch (NamingException expected) { + // no results stubbed; only the filter sent matters here + } + + String filter = capturedSearchFilter(); + assertFalse("a lookup for one specific username must not be scoped to already-linked groups: " + filter, + filter.contains("memberOf=" + LINKED_GROUP)); + } + + @Test + public void getUsersStillRestrictsToAlreadyLinkedGroups() throws Exception { + openLdapUserManager.getUsers("test_user", ldapContextMock, DOMAIN_ID); + + String filter = capturedSearchFilter(); + assertTrue("browsing/importing users should still be scoped to already-linked groups: " + filter, + filter.contains("memberOf=" + LINKED_GROUP)); + } + + private String capturedSearchFilter() throws Exception { + ArgumentCaptor<String> filterCaptor = ArgumentCaptor.forClass(String.class); + verify(ldapContextMock, atLeastOnce()).search(any(String.class), filterCaptor.capture(), any(SearchControls.class)); + return filterCaptor.getValue(); + } +}
