This is an automated email from the ASF dual-hosted git repository.
DaanHoogland pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 068d393d012 allow update of ldap linked account (#13949)
068d393d012 is described below
commit 068d393d0120189ad4220594d9503df04a091076
Author: dahn <[email protected]>
AuthorDate: Wed Sep 9 12:34:08 2026 +0200
allow update of ldap linked account (#13949)
Co-authored-by: Daan Hoogland <[email protected]>
---
.../apache/cloudstack/ldap/LdapManagerImpl.java | 26 ++-
.../cloudstack/ldap/LdapManagerImplTest.java | 185 +++++++++++++++++++++
2 files changed, 207 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..69bb116252a 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("account {} in domain {} is already linked to ldap {}
'{}'; 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..917c29b81ae
--- /dev/null
+++
b/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/LdapManagerImplTest.java
@@ -0,0 +1,185 @@
+// 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.doThrow;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+/**
+ * Tests {@link LdapManagerImpl#linkAccountToLdap}: re-linking an account
replaces its own
+ * mapping, refuses a group already claimed by another live account, and
leaves no mapping
+ * persisted 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);
+
+ LinkAccountToLdapCmd cmd =
buildCmd("cn=claimed,dc=my,dc=domain,dc=com");
+ assertThrows(CloudRuntimeException.class, () ->
ldapManager.linkAccountToLdap(cmd));
+
+ 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);
+ doThrow(new CloudRuntimeException("db
blip")).when(ldapTrustMapDaoMock).expunge(Long.valueOf(OLD_MAPPING_ID));
+
+ LinkAccountToLdapCmd cmd = buildCmd("cn=new,dc=my,dc=domain,dc=com");
+ assertThrows(CloudRuntimeException.class, () ->
ldapManager.linkAccountToLdap(cmd));
+
+ 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;
+ }
+}
