This is an automated email from the ASF dual-hosted git repository.
nvazquez pushed a commit to branch 4.20
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.20 by this push:
new 50869a45b8c allow creating ldap on existing linked domain (#13945)
50869a45b8c is described below
commit 50869a45b8c2a066f8f0d4f4e16b77fff4a02b82
Author: dahn <[email protected]>
AuthorDate: Mon Nov 9 21:43:37 2026 +0100
allow creating ldap on existing linked domain (#13945)
* allow creating ldap accounts after linking accounts to the same the domain
* sonarqube
* tests
* null guard on mock
---
.../cloudstack/ldap/OpenLdapUserManagerImpl.java | 62 +++++---
.../ldap/OpenLdapUserManagerImplTest.java | 172 +++++++++++++++++++++
2 files changed, 212 insertions(+), 22 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 98d3be2f10c..06a9adcc69b 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);
@@ -166,10 +173,14 @@ 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);
- if (result!= null && result.size() == 1) {
+ List<LdapUser> result = searchUsers(username, context, domainId,
false);
+ if (result.size() == 1) {
return result.get(0);
} else {
throw new NamingException("No user found for username " +
username);
@@ -314,6 +325,10 @@ public class OpenLdapUserManagerImpl implements
LdapUserManager {
@Override
@SuppressWarnings("BanJNDI")
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();
@@ -330,27 +345,30 @@ 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)) {
users.add(createUser(result, domainId));
}
}
- Control[] contextControls = context.getResponseControls();
- if (contextControls != null) {
- for (Control control : contextControls) {
- if (control instanceof PagedResultsResponseControl) {
- PagedResultsResponseControl prrc =
(PagedResultsResponseControl) control;
- cookie = prrc.getCookie();
- }
- }
- } else {
- logger.info("No controls were sent from the ldap server");
- }
+ cookie = extractPagedResultsCookie(context.getResponseControls());
context.setRequestControls(new Control[] {new
PagedResultsControl(pageSize, cookie, Control.CRITICAL)});
} while (cookie != null);
return users;
}
+
+ private byte[] extractPagedResultsCookie(Control[] contextControls) {
+ if (contextControls == null) {
+ logger.info("No controls were sent from the ldap server");
+ return null;
+ }
+ for (Control control : contextControls) {
+ if (control instanceof PagedResultsResponseControl) {
+ return ((PagedResultsResponseControl) control).getCookie();
+ }
+ }
+ return null;
+ }
}
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..5e9895e2f18
--- /dev/null
+++
b/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/OpenLdapUserManagerImplTest.java
@@ -0,0 +1,172 @@
+// 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.Control;
+import javax.naming.ldap.LdapContext;
+import javax.naming.ldap.PagedResultsResponseControl;
+
+import java.util.Collections;
+import java.util.List;
+
+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.times;
+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() {
+ if (ldapConfigurationMockedStatic != null) {
+ 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));
+ }
+
+ @Test
+ public void getUsersDoesNotAddGroupScopeWhenDomainHasNoLinkedGroups()
throws Exception {
+
when(ldapTrustMapDaoMock.searchByDomainId(anyLong())).thenReturn(Collections.emptyList());
+
+ openLdapUserManager.getUsers("test_user", ldapContextMock, DOMAIN_ID);
+
+ String filter = capturedSearchFilter();
+ assertFalse("no linked groups in the domain means no group scoping
should be applied: " + filter,
+ filter.contains("memberOf="));
+ }
+
+ @Test
+ public void searchStopsAfterOnePageWhenNoPagedResultsControlIsReturned()
throws Exception {
+ Control unrelatedControl = mock(Control.class);
+ when(ldapContextMock.getResponseControls()).thenReturn(new
Control[]{unrelatedControl});
+
+ List<LdapUser> result = openLdapUserManager.searchUsers("test_user",
ldapContextMock, DOMAIN_ID);
+
+ assertTrue(result.isEmpty());
+ verify(ldapContextMock, times(1)).search(any(String.class),
any(String.class), any(SearchControls.class));
+ }
+
+ @Test
+ public void searchFollowsCookieFromPagedResultsControlToNextPage() throws
Exception {
+ PagedResultsResponseControl pagedControl =
mock(PagedResultsResponseControl.class);
+ when(pagedControl.getCookie()).thenReturn(new byte[]{1, 2, 3});
+ when(ldapContextMock.getResponseControls())
+ .thenReturn(new Control[]{pagedControl})
+ .thenReturn(null);
+
+ List<LdapUser> result = openLdapUserManager.searchUsers("test_user",
ldapContextMock, DOMAIN_ID);
+
+ assertTrue(result.isEmpty());
+ verify(ldapContextMock, times(2)).search(any(String.class),
any(String.class), any(SearchControls.class));
+ }
+
+ 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();
+ }
+}
