You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by da...@apache.org on 2026/08/22 16:19:21 UTC
(cloudstack) branch ghi11185-update-ldap-domain-on-account created (now 4825b6d001a)
This is an automated email from the ASF dual-hosted git repository.
DaanHoogland pushed a change to branch ghi11185-update-ldap-domain-on-account
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
at 4825b6d001a allow update of ldap linked account
This branch includes the following new commits:
new 4825b6d001a allow update of ldap linked account
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(cloudstack) 01/01: allow update of ldap linked account
Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
DaanHoogland pushed a commit to branch ghi11185-update-ldap-domain-on-account
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 4825b6d001ae2b1a2a398d94aac255d9e1ed99fd
Author: Daan Hoogland <da...@apache.org>
AuthorDate: Sat Aug 22 18:18:57 2026 +0200
allow update of ldap linked account
---
.../apache/cloudstack/ldap/LdapManagerImpl.java | 26 ++-
.../cloudstack/ldap/LdapManagerImplTest.java | 183 +++++++++++++++++++++
2 files changed, 205 insertions(+), 4 deletions(-)
diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java
index a93b7a9e133..52bb12a2772 100644
--- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java
+++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java
@@ -63,6 +63,8 @@ import com.cloud.user.DomainManager;
import com.cloud.user.dao.AccountDao;
import com.cloud.utils.Pair;
import com.cloud.utils.component.ComponentLifecycleBase;
+import com.cloud.utils.db.Transaction;
+import com.cloud.utils.db.TransactionCallback;
import com.cloud.utils.exception.CloudRuntimeException;
@Component
@@ -473,8 +475,11 @@ public class LdapManagerImpl extends ComponentLifecycleBase implements LdapManag
}
long accountId = account.getAccountId();
- clearOldAccountMapping(cmd);
- LdapTrustMapVO vo = _ldapTrustMapDao.persist(new LdapTrustMapVO(cmd.getDomainId(), linkType, cmd.getLdapDomain(), cmd.getAccountType(), accountId));
+ LdapTrustMapVO vo = Transaction.execute((TransactionCallback<LdapTrustMapVO>) status -> {
+ clearOldAccountMapping(cmd, accountId);
+ clearAccountsOwnMapping(cmd.getDomainId(), accountId);
+ return _ldapTrustMapDao.persist(new LdapTrustMapVO(cmd.getDomainId(), linkType, cmd.getLdapDomain(), cmd.getAccountType(), accountId));
+ });
return new LinkAccountToLdapResponse(domain.getUuid(), vo.getType().toString(), vo.getName(), vo.getAccountType().ordinal(), account.getUuid(), cmd.getAccountName());
}
@@ -515,10 +520,23 @@ public class LdapManagerImpl extends ComponentLifecycleBase implements LdapManag
return linkAccountToLdapAndGetResponse(cmd);
}
- private void clearOldAccountMapping(LinkAccountToLdapCmd cmd) {
+ /**
+ * Replaces the account's existing LDAP mapping, if any, so {@link #linkAccountToLdap}
+ * can update the ldapDomain/type of an existing link instead of failing on the
+ * domain_id/account_id unique key.
+ */
+ private void clearAccountsOwnMapping(Long domainId, long accountId) {
+ LdapTrustMapVO ownVo = _ldapTrustMapDao.findByAccount(domainId, accountId);
+ if (ownVo != null) {
+ logger.warn(String.format("account %d in domain %d is already linked to ldap %s '%s'; replacing with the new mapping", accountId, domainId, ownVo.getType(), ownVo.getName()));
+ _ldapTrustMapDao.expunge(ownVo.getId());
+ }
+ }
+
+ private void clearOldAccountMapping(LinkAccountToLdapCmd cmd, long accountId) {
// first find if exists log warning and update
LdapTrustMapVO oldVo = _ldapTrustMapDao.findGroupInDomain(cmd.getDomainId(), cmd.getLdapDomain());
- if (oldVo != null) {
+ if (oldVo != null && oldVo.getAccountId() != accountId) {
// deal with edge cases, i.e. check if the old account is indeed deleted etc.
if (oldVo.getAccountId() != 0L) {
AccountVO oldAcount = accountDao.findByIdIncludingRemoved(oldVo.getAccountId());
diff --git a/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/LdapManagerImplTest.java b/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/LdapManagerImplTest.java
new file mode 100644
index 00000000000..7c729d604ed
--- /dev/null
+++ b/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/LdapManagerImplTest.java
@@ -0,0 +1,183 @@
+// 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.domain.DomainVO;
+import com.cloud.domain.dao.DomainDao;
+import com.cloud.user.Account;
+import com.cloud.user.AccountVO;
+import com.cloud.user.dao.AccountDao;
+import com.cloud.utils.exception.CloudRuntimeException;
+import org.apache.cloudstack.acl.RoleService;
+import org.apache.cloudstack.api.command.LinkAccountToLdapCmd;
+import org.apache.cloudstack.api.response.LinkAccountToLdapResponse;
+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.Mock;
+import org.mockito.Mockito;
+import org.mockito.MockedStatic;
+import org.mockito.junit.MockitoJUnitRunner;
+import org.springframework.test.util.ReflectionTestUtils;
+
+import java.util.Date;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+/**
+ * Regression tests for #11185: linkAccountToLdap must be usable to update the
+ * ldapDomain/type of an account's existing LDAP mapping, instead of failing on the
+ * domain_id/account_id unique key, and must not persist a new mapping if clearing
+ * the old one fails.
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class LdapManagerImplTest {
+
+ private static final Long DOMAIN_ID = 1L;
+ private static final long ACCOUNT_ID = 24L;
+ private static final long OLD_MAPPING_ID = 5L;
+
+ private LdapManagerImpl ldapManager;
+
+ private MockedStatic<LdapConfiguration> ldapConfigurationMockedStatic;
+
+ @Mock
+ private LdapTrustMapDao ldapTrustMapDaoMock;
+
+ @Mock
+ private DomainDao domainDaoMock;
+
+ @Mock
+ private AccountDao accountDaoMock;
+
+ @Mock
+ private RoleService roleServiceMock;
+
+ @Before
+ public void setup() {
+ ldapConfigurationMockedStatic = Mockito.mockStatic(LdapConfiguration.class, Mockito.CALLS_REAL_METHODS);
+ when(LdapConfiguration.getBaseDn(DOMAIN_ID)).thenReturn("dc=my,dc=domain,dc=com");
+
+ ldapManager = new LdapManagerImpl();
+ ldapManager._ldapTrustMapDao = ldapTrustMapDaoMock;
+ ReflectionTestUtils.setField(ldapManager, "domainDao", domainDaoMock);
+ ReflectionTestUtils.setField(ldapManager, "accountDao", accountDaoMock);
+ when(domainDaoMock.findById(DOMAIN_ID)).thenReturn(new DomainVO());
+
+ AccountVO existingAccount = new AccountVO("jdoe", DOMAIN_ID, null, Account.Type.NORMAL, null, "acct-uuid");
+ ReflectionTestUtils.setField(existingAccount, "id", ACCOUNT_ID);
+ when(accountDaoMock.findActiveAccount("jdoe", DOMAIN_ID)).thenReturn(existingAccount);
+ when(ldapTrustMapDaoMock.persist(any())).thenAnswer(invocation -> invocation.getArgument(0));
+ }
+
+ @After
+ public void tearDown() {
+ ldapConfigurationMockedStatic.close();
+ }
+
+ @Test
+ public void relinkingAccountReplacesItsOwnExistingMapping() {
+ LdapTrustMapVO ownMapping = new LdapTrustMapVO(DOMAIN_ID, LdapManager.LinkType.GROUP, "cn=old,dc=my,dc=domain,dc=com", Account.Type.NORMAL, ACCOUNT_ID);
+ ReflectionTestUtils.setField(ownMapping, "id", OLD_MAPPING_ID);
+ when(ldapTrustMapDaoMock.findByAccount(DOMAIN_ID, ACCOUNT_ID)).thenReturn(ownMapping);
+
+ LinkAccountToLdapResponse response = ldapManager.linkAccountToLdap(buildCmd("cn=new,dc=my,dc=domain,dc=com"));
+
+ verify(ldapTrustMapDaoMock, times(1)).expunge(Long.valueOf(OLD_MAPPING_ID));
+ assertEquals("cn=new,dc=my,dc=domain,dc=com", response.getLdapDomain());
+ }
+
+ @Test
+ public void firstLinkOfAccountDoesNotExpungeAnything() {
+ when(ldapTrustMapDaoMock.findByAccount(DOMAIN_ID, ACCOUNT_ID)).thenReturn(null);
+
+ ldapManager.linkAccountToLdap(buildCmd("cn=first,dc=my,dc=domain,dc=com"));
+
+ verify(ldapTrustMapDaoMock, never()).expunge(any(Long.class));
+ }
+
+ @Test
+ public void relinkingAccountToItsCurrentGroupDoesNotThrow() {
+ LdapTrustMapVO ownMapping = new LdapTrustMapVO(DOMAIN_ID, LdapManager.LinkType.GROUP, "cn=same,dc=my,dc=domain,dc=com", Account.Type.NORMAL, ACCOUNT_ID);
+ ReflectionTestUtils.setField(ownMapping, "id", OLD_MAPPING_ID);
+ when(ldapTrustMapDaoMock.findGroupInDomain(DOMAIN_ID, "cn=same,dc=my,dc=domain,dc=com")).thenReturn(ownMapping);
+ when(ldapTrustMapDaoMock.findByAccount(DOMAIN_ID, ACCOUNT_ID)).thenReturn(ownMapping);
+
+ LinkAccountToLdapResponse response = ldapManager.linkAccountToLdap(buildCmd("cn=same,dc=my,dc=domain,dc=com"));
+
+ assertEquals("cn=same,dc=my,dc=domain,dc=com", response.getLdapDomain());
+ }
+
+ @Test
+ public void relinkingAccountRefusesGroupClaimedByAnotherLiveAccount() {
+ long otherAccountId = 99L;
+ LdapTrustMapVO otherMapping = new LdapTrustMapVO(DOMAIN_ID, LdapManager.LinkType.GROUP, "cn=claimed,dc=my,dc=domain,dc=com", Account.Type.NORMAL, otherAccountId);
+ when(ldapTrustMapDaoMock.findGroupInDomain(DOMAIN_ID, "cn=claimed,dc=my,dc=domain,dc=com")).thenReturn(otherMapping);
+ AccountVO otherAccount = new AccountVO();
+ when(accountDaoMock.findByIdIncludingRemoved(otherAccountId)).thenReturn(otherAccount);
+
+ assertThrows(CloudRuntimeException.class, () -> ldapManager.linkAccountToLdap(buildCmd("cn=claimed,dc=my,dc=domain,dc=com")));
+
+ verify(ldapTrustMapDaoMock, never()).persist(any());
+ }
+
+ @Test
+ public void relinkingAccountAllowsGroupOnceOtherClaimingAccountIsRemoved() {
+ long removedAccountId = 99L;
+ LdapTrustMapVO otherMapping = new LdapTrustMapVO(DOMAIN_ID, LdapManager.LinkType.GROUP, "cn=stale,dc=my,dc=domain,dc=com", Account.Type.NORMAL, removedAccountId);
+ ReflectionTestUtils.setField(otherMapping, "id", OLD_MAPPING_ID);
+ when(ldapTrustMapDaoMock.findGroupInDomain(DOMAIN_ID, "cn=stale,dc=my,dc=domain,dc=com")).thenReturn(otherMapping);
+ AccountVO removedAccount = new AccountVO();
+ ReflectionTestUtils.setField(removedAccount, "removed", new Date());
+ when(accountDaoMock.findByIdIncludingRemoved(removedAccountId)).thenReturn(removedAccount);
+
+ LinkAccountToLdapResponse response = ldapManager.linkAccountToLdap(buildCmd("cn=stale,dc=my,dc=domain,dc=com"));
+
+ assertEquals("cn=stale,dc=my,dc=domain,dc=com", response.getLdapDomain());
+ }
+
+ @Test
+ public void relinkingAccountDoesNotPersistWhenClearingOldMappingFails() {
+ LdapTrustMapVO ownMapping = new LdapTrustMapVO(DOMAIN_ID, LdapManager.LinkType.GROUP, "cn=old,dc=my,dc=domain,dc=com", Account.Type.NORMAL, ACCOUNT_ID);
+ ReflectionTestUtils.setField(ownMapping, "id", OLD_MAPPING_ID);
+ when(ldapTrustMapDaoMock.findByAccount(DOMAIN_ID, ACCOUNT_ID)).thenReturn(ownMapping);
+ Mockito.doThrow(new CloudRuntimeException("db blip")).when(ldapTrustMapDaoMock).expunge(Long.valueOf(OLD_MAPPING_ID));
+
+ assertThrows(CloudRuntimeException.class, () -> ldapManager.linkAccountToLdap(buildCmd("cn=new,dc=my,dc=domain,dc=com")));
+
+ verify(ldapTrustMapDaoMock, never()).persist(any());
+ }
+
+ private LinkAccountToLdapCmd buildCmd(String ldapDomain) {
+ LinkAccountToLdapCmd cmd = new LinkAccountToLdapCmd();
+ cmd.roleService = roleServiceMock;
+ ReflectionTestUtils.setField(cmd, "domainId", DOMAIN_ID);
+ ReflectionTestUtils.setField(cmd, "type", "GROUP");
+ ReflectionTestUtils.setField(cmd, "ldapDomain", ldapDomain);
+ ReflectionTestUtils.setField(cmd, "accountName", "jdoe");
+ ReflectionTestUtils.setField(cmd, "accountType", Account.Type.NORMAL.ordinal());
+ return cmd;
+ }
+}