From 82bc12631ad34b88876fa674d5fcc664cfe8f1ff Mon Sep 17 00:00:00 2001 From: "Chen, Taiyue" Date: Sat, 11 Apr 2026 11:52:46 +0800 Subject: [PATCH 1/2] [ldap]: refactor LDAP login logic and add two-factor authentication support 1. Extract common method findAccountThirdPartyAccountSourceRefByName - Extract the logic of finding third-party account source reference from login method into a standalone method - Unify account lookup logic in both login and collectUserInfoIntoContext methods 2. Add two-factor authentication support - getRequiredAdditionalAuthFeature method now returns a list containing both basicLoginControl and twoFactor - Support two-factor authentication for LDAP login Resolves: ZSV-11833 Change-Id: I7578626465676e6569686a786271786b64667179 --- .../java/org/zstack/ldap/LdapManager.java | 3 +- .../java/org/zstack/ldap/LdapManagerImpl.java | 68 ++++++++++++------- ...etTwoFactorAuthenticationSecretAction.java | 3 + ...etTwoFactorAuthenticationSecretAction.java | 3 + .../resources/springConfigXml/license.xml | 20 ------ ...Test.groovy => LdapDriverSimulator.groovy} | 2 +- .../ldap/LdapVirtualEndpointSpec.groovy | 2 +- 7 files changed, 54 insertions(+), 47 deletions(-) delete mode 100755 test/src/test/resources/springConfigXml/license.xml rename testlib/src/main/java/org/zstack/testlib/identity/ldap/{LdapDriverForTest.groovy => LdapDriverSimulator.groovy} (98%) diff --git a/plugin/ldap/src/main/java/org/zstack/ldap/LdapManager.java b/plugin/ldap/src/main/java/org/zstack/ldap/LdapManager.java index 43d9950d913..4fb4cd60a3a 100644 --- a/plugin/ldap/src/main/java/org/zstack/ldap/LdapManager.java +++ b/plugin/ldap/src/main/java/org/zstack/ldap/LdapManager.java @@ -2,6 +2,7 @@ import org.zstack.core.Platform; import org.zstack.header.errorcode.ErrorableValue; +import org.zstack.identity.imports.entity.AccountThirdPartyAccountSourceRefVO; import org.zstack.ldap.driver.LdapUtil; import org.zstack.ldap.entity.LdapServerVO; @@ -10,7 +11,7 @@ */ public interface LdapManager { boolean isValid(String uid, String password); - + ErrorableValue findAccountThirdPartyAccountSourceRefByName(String ldapLoginName, String ldapLoginPassword); ErrorableValue findCurrentLdapServerUuid(); ErrorableValue findCurrentLdapServer(); diff --git a/plugin/ldap/src/main/java/org/zstack/ldap/LdapManagerImpl.java b/plugin/ldap/src/main/java/org/zstack/ldap/LdapManagerImpl.java index 92de3125e5b..08ef261d69d 100755 --- a/plugin/ldap/src/main/java/org/zstack/ldap/LdapManagerImpl.java +++ b/plugin/ldap/src/main/java/org/zstack/ldap/LdapManagerImpl.java @@ -407,24 +407,13 @@ public LoginType getLoginType() { return loginType; } - @Override - public void login(LoginContext loginContext, ReturnValueCompletion completion) { + public ErrorableValue findAccountThirdPartyAccountSourceRefByName(String ldapLoginName, String ldapLoginPassword) { final ErrorableValue currentLdapServer = findCurrentLdapServer(); if (!currentLdapServer.isSuccess()) { - logger.debug("failed to login by LDAP: failed to find current LdapServer: " + currentLdapServer.error.getDetails()); - completion.fail(err(IdentityErrors.AUTHENTICATION_ERROR, - "Login validation failed in LDAP")); - return; + return ErrorableValue.ofErrorCode(currentLdapServer.error); } final LdapServerVO ldap = currentLdapServer.result; - String ldapLoginName = loginContext.getUsername(); - if (!isValid(ldapLoginName, loginContext.getPassword())) { - completion.fail(err(IdentityErrors.AUTHENTICATION_ERROR, - "Login validation failed in LDAP")); - return; - } - String dn = createDriver().getFullUserDn(ldap, ldap.getUsernameProperty(), ldapLoginName); AccountThirdPartyAccountSourceRefVO vo = Q.New(AccountThirdPartyAccountSourceRefVO.class) .eq(AccountThirdPartyAccountSourceRefVO_.credentials, dn) @@ -432,19 +421,35 @@ public void login(LoginContext loginContext, ReturnValueCompletion completion) { + final ErrorableValue accountThirdPartyAccountSourceRef = findAccountThirdPartyAccountSourceRefByName(loginContext.getUsername(), loginContext.getPassword()); + + if (!accountThirdPartyAccountSourceRef.isSuccess()) { + completion.fail(accountThirdPartyAccountSourceRef.error); return; } + if (!isValid(loginContext.getUsername(), loginContext.getPassword())) { + completion.fail(err(IdentityErrors.AUTHENTICATION_ERROR, + "Login validation failed in LDAP")); + return; + } + String accountUuid = accountThirdPartyAccountSourceRef.result.getAccountUuid(); final AccountState state = Q.New(AccountVO.class) - .eq(AccountVO_.uuid, vo.getAccountUuid()) + .eq(AccountVO_.uuid, accountUuid) .select(AccountVO_.state) .findValue(); if (state == null || state == AccountState.Staled) { completion.fail(operr( - "Account[uuid:%s] Not Found!!!", vo.getAccountUuid())); + "Account[uuid:%s] Not Found!!!", accountUuid)); return; } if (state == AccountState.Disabled) { @@ -453,7 +458,7 @@ public void login(LoginContext loginContext, ReturnValueCompletion property = findCurrentLdapServer(); - if (property.isSuccess()) { - final String fullUserDn = createDriver().getFullUserDn(property.result, username); - return "".equals(fullUserDn) ? null : fullUserDn; + final ErrorableValue currentLdapServer = findCurrentLdapServer(); + if (!currentLdapServer.isSuccess()) { + return null; + } + final LdapServerVO ldap = currentLdapServer.result; + + String dn = createDriver().getFullUserDn(ldap, ldap.getUsernameProperty(), username); + if (dn == null || dn.isEmpty()) { + return null; } - return null; + + String accountUuid = Q.New(AccountThirdPartyAccountSourceRefVO.class) + .select(AccountThirdPartyAccountSourceRefVO_.accountUuid) + .eq(AccountThirdPartyAccountSourceRefVO_.credentials, dn) + .eq(AccountThirdPartyAccountSourceRefVO_.accountSourceUuid, ldap.getUuid()) + .findValue(); + return accountUuid; } @Override public void collectUserInfoIntoContext(LoginContext loginContext) { - loginContext.setAccountUuid(getAccountIdByName(loginContext.getUsername())); + ErrorableValue accountThirdPartyAccountSourceRef = findAccountThirdPartyAccountSourceRefByName(loginContext.getUsername(), loginContext.getPassword()); + if (!accountThirdPartyAccountSourceRef.isSuccess()) { + return; + } + loginContext.setAccountUuid(accountThirdPartyAccountSourceRef.result.getAccountUuid()); } @Override public List getRequiredAdditionalAuthFeature() { - return Collections.singletonList(LoginAuthConstant.basicLoginControl); + return Arrays.asList(LoginAuthConstant.basicLoginControl, LoginAuthConstant.twoFactor); } } diff --git a/sdk/src/main/java/org/zstack/sdk/GetTwoFactorAuthenticationSecretAction.java b/sdk/src/main/java/org/zstack/sdk/GetTwoFactorAuthenticationSecretAction.java index 06643468410..32c32376d48 100644 --- a/sdk/src/main/java/org/zstack/sdk/GetTwoFactorAuthenticationSecretAction.java +++ b/sdk/src/main/java/org/zstack/sdk/GetTwoFactorAuthenticationSecretAction.java @@ -37,6 +37,9 @@ public Result throwExceptionIfError() { @Param(required = false, nonempty = false, nullElements = false, emptyString = true, noTrim = false) public java.lang.String verifyCode; + @Param(required = true, validValues = {"account","ldap"}, nonempty = true, nullElements = false, emptyString = true, noTrim = false) + public java.lang.String type; + @Param(required = false) public java.util.List systemTags; diff --git a/sdk/src/main/java/org/zstack/sdk/ResetTwoFactorAuthenticationSecretAction.java b/sdk/src/main/java/org/zstack/sdk/ResetTwoFactorAuthenticationSecretAction.java index 61fcfc0980e..a84fcecdc02 100644 --- a/sdk/src/main/java/org/zstack/sdk/ResetTwoFactorAuthenticationSecretAction.java +++ b/sdk/src/main/java/org/zstack/sdk/ResetTwoFactorAuthenticationSecretAction.java @@ -37,6 +37,9 @@ public Result throwExceptionIfError() { @Param(required = false, nonempty = false, nullElements = false, emptyString = true, noTrim = false) public java.lang.String verifyCode; + @Param(required = true, validValues = {"account","ldap"}, nonempty = true, nullElements = false, emptyString = true, noTrim = false) + public java.lang.String type; + @Param(required = false) public java.util.List systemTags; diff --git a/test/src/test/resources/springConfigXml/license.xml b/test/src/test/resources/springConfigXml/license.xml deleted file mode 100755 index 981f89749f4..00000000000 --- a/test/src/test/resources/springConfigXml/license.xml +++ /dev/null @@ -1,20 +0,0 @@ - - - - - - - - - diff --git a/testlib/src/main/java/org/zstack/testlib/identity/ldap/LdapDriverForTest.groovy b/testlib/src/main/java/org/zstack/testlib/identity/ldap/LdapDriverSimulator.groovy similarity index 98% rename from testlib/src/main/java/org/zstack/testlib/identity/ldap/LdapDriverForTest.groovy rename to testlib/src/main/java/org/zstack/testlib/identity/ldap/LdapDriverSimulator.groovy index be2e61daddd..9e33fedfe4d 100644 --- a/testlib/src/main/java/org/zstack/testlib/identity/ldap/LdapDriverForTest.groovy +++ b/testlib/src/main/java/org/zstack/testlib/identity/ldap/LdapDriverSimulator.groovy @@ -13,7 +13,7 @@ import org.zstack.ldap.entity.LdapServerVO_ import java.util.function.Function import java.util.function.Supplier -class LdapDriverForTest extends LdapUtil { +class LdapDriverSimulator extends LdapUtil { public Supplier> findAllEndpointsFunction public Function findEndpointByLdapVOFunction diff --git a/testlib/src/main/java/org/zstack/testlib/identity/ldap/LdapVirtualEndpointSpec.groovy b/testlib/src/main/java/org/zstack/testlib/identity/ldap/LdapVirtualEndpointSpec.groovy index 2204b82594b..a90b681b069 100644 --- a/testlib/src/main/java/org/zstack/testlib/identity/ldap/LdapVirtualEndpointSpec.groovy +++ b/testlib/src/main/java/org/zstack/testlib/identity/ldap/LdapVirtualEndpointSpec.groovy @@ -60,7 +60,7 @@ class LdapVirtualEndpointSpec extends Spec { endpointUuid = uuid envSpec.mockFactory(LdapUtil.class) { - def driver = new LdapDriverForTest() + def driver = new LdapDriverSimulator() driver.findAllEndpointsFunction = { findAllEndpoints() } driver.findEndpointByLdapVOFunction = { LdapServerVO ldap -> findEndpointByLdapVO(ldap) } return driver From 09e649d3ec6356abbd54b3337411f9f44b370e9b Mon Sep 17 00:00:00 2001 From: "Chen, Taiyue" Date: Sun, 12 Apr 2026 00:15:18 +0800 Subject: [PATCH 2/2] [testlib]: fix unexecute test cases Resolves: ZSV-11833 Change-Id: I696c6e746965616478786e6f6e6263636d766868 --- .../GetTwoFactorAuthenticationSecretAction.java | 2 +- .../ResetTwoFactorAuthenticationSecretAction.java | 2 +- test/src/test/resources/springConfigXml/Kvm.xml | 2 +- ...a => HypervisorMetadataCollectorSimulator.java} | 8 ++++---- ...ntForTest.groovy => VOpsClientSimulator.groovy} | 8 ++++---- .../testlib/vops/VOpsVirtualEndpointSpec.groovy | 14 +++++++------- 6 files changed, 18 insertions(+), 18 deletions(-) rename testlib/src/main/java/org/zstack/kvm/hypervisor/{HypervisorMetadataCollectorForTest.java => HypervisorMetadataCollectorSimulator.java} (88%) rename testlib/src/main/java/org/zstack/testlib/vops/{VOpsClientForTest.groovy => VOpsClientSimulator.groovy} (95%) diff --git a/sdk/src/main/java/org/zstack/sdk/GetTwoFactorAuthenticationSecretAction.java b/sdk/src/main/java/org/zstack/sdk/GetTwoFactorAuthenticationSecretAction.java index 32c32376d48..cd1de1986ad 100644 --- a/sdk/src/main/java/org/zstack/sdk/GetTwoFactorAuthenticationSecretAction.java +++ b/sdk/src/main/java/org/zstack/sdk/GetTwoFactorAuthenticationSecretAction.java @@ -38,7 +38,7 @@ public Result throwExceptionIfError() { public java.lang.String verifyCode; @Param(required = true, validValues = {"account","ldap"}, nonempty = true, nullElements = false, emptyString = true, noTrim = false) - public java.lang.String type; + public java.lang.String type = "account"; @Param(required = false) public java.util.List systemTags; diff --git a/sdk/src/main/java/org/zstack/sdk/ResetTwoFactorAuthenticationSecretAction.java b/sdk/src/main/java/org/zstack/sdk/ResetTwoFactorAuthenticationSecretAction.java index a84fcecdc02..19a5f07552b 100644 --- a/sdk/src/main/java/org/zstack/sdk/ResetTwoFactorAuthenticationSecretAction.java +++ b/sdk/src/main/java/org/zstack/sdk/ResetTwoFactorAuthenticationSecretAction.java @@ -38,7 +38,7 @@ public Result throwExceptionIfError() { public java.lang.String verifyCode; @Param(required = true, validValues = {"account","ldap"}, nonempty = true, nullElements = false, emptyString = true, noTrim = false) - public java.lang.String type; + public java.lang.String type = "account"; @Param(required = false) public java.util.List systemTags; diff --git a/test/src/test/resources/springConfigXml/Kvm.xml b/test/src/test/resources/springConfigXml/Kvm.xml index 70e706ddef2..137cad827f6 100755 --- a/test/src/test/resources/springConfigXml/Kvm.xml +++ b/test/src/test/resources/springConfigXml/Kvm.xml @@ -206,7 +206,7 @@ - + diff --git a/testlib/src/main/java/org/zstack/kvm/hypervisor/HypervisorMetadataCollectorForTest.java b/testlib/src/main/java/org/zstack/kvm/hypervisor/HypervisorMetadataCollectorSimulator.java similarity index 88% rename from testlib/src/main/java/org/zstack/kvm/hypervisor/HypervisorMetadataCollectorForTest.java rename to testlib/src/main/java/org/zstack/kvm/hypervisor/HypervisorMetadataCollectorSimulator.java index a8d9f37b4b4..226494749c2 100644 --- a/testlib/src/main/java/org/zstack/kvm/hypervisor/HypervisorMetadataCollectorForTest.java +++ b/testlib/src/main/java/org/zstack/kvm/hypervisor/HypervisorMetadataCollectorSimulator.java @@ -13,7 +13,7 @@ * * Created by Wenhao.Zhang on 23/03/01 */ -public class HypervisorMetadataCollectorForTest extends HypervisorMetadataCollectorImpl { +public class HypervisorMetadataCollectorSimulator extends HypervisorMetadataCollectorImpl { public static final String QEMU_VERSION_FOR_TEST = "4.2.0-632.g6a6222b.el7"; private Function> folderScannerSimulator; private Function collectMetadataSimulator; @@ -41,11 +41,11 @@ protected List scanFolder(Path rootPath) throws IO } public List scanFolderForDefault(Path rootPath) { - HypervisorMetadataDefinitionForTest d1 = new HypervisorMetadataDefinitionForTest(); + HypervisorMetadataDefinitionSimulator d1 = new HypervisorMetadataDefinitionSimulator(); d1.setArchitecture(CpuArchitecture.x86_64.name()); d1.setOsReleaseSimpleVersion("c76"); - HypervisorMetadataDefinitionForTest d2 = new HypervisorMetadataDefinitionForTest(); + HypervisorMetadataDefinitionSimulator d2 = new HypervisorMetadataDefinitionSimulator(); d2.setArchitecture(CpuArchitecture.x86_64.name()); d2.setOsReleaseSimpleVersion("c79"); @@ -74,7 +74,7 @@ public String collectMetadataForDefault(HypervisorMetadataDefinition definition) QEMU_VERSION_FOR_TEST, osReleaseVersion[0], osReleaseVersion[1], osReleaseVersion[2]); } - public static class HypervisorMetadataDefinitionForTest extends HypervisorMetadataDefinition { + public static class HypervisorMetadataDefinitionSimulator extends HypervisorMetadataDefinition { @Override public boolean isValid() { return true; diff --git a/testlib/src/main/java/org/zstack/testlib/vops/VOpsClientForTest.groovy b/testlib/src/main/java/org/zstack/testlib/vops/VOpsClientSimulator.groovy similarity index 95% rename from testlib/src/main/java/org/zstack/testlib/vops/VOpsClientForTest.groovy rename to testlib/src/main/java/org/zstack/testlib/vops/VOpsClientSimulator.groovy index 9cd6543cbfc..9c94925d647 100644 --- a/testlib/src/main/java/org/zstack/testlib/vops/VOpsClientForTest.groovy +++ b/testlib/src/main/java/org/zstack/testlib/vops/VOpsClientSimulator.groovy @@ -13,10 +13,10 @@ import java.util.function.Function import static org.zstack.externalservice.vops.VOpsCommands.*; -class VOpsClientForTest extends VOpsClient { +class VOpsClientSimulator extends VOpsClient { public final VOpsVirtualEndpointSpec parent - VOpsClientForTest(VOpsVirtualEndpointSpec parent) { + VOpsClientSimulator(VOpsVirtualEndpointSpec parent) { this.parent = parent } @@ -75,9 +75,9 @@ class VOpsClientForTest extends VOpsClient { } static class HttpForTest extends RestHttp { - final VOpsClientForTest client + final VOpsClientSimulator client - HttpForTest(Class returnClass, VOpsClientForTest client) { + HttpForTest(Class returnClass, VOpsClientSimulator client) { super(returnClass) this.client = client this.errorCodeBuilder = { Exception e, http2 -> client.errorFacade.throwableToOperationError(e) } diff --git a/testlib/src/main/java/org/zstack/testlib/vops/VOpsVirtualEndpointSpec.groovy b/testlib/src/main/java/org/zstack/testlib/vops/VOpsVirtualEndpointSpec.groovy index bf873b66d68..0e60f3b1f53 100644 --- a/testlib/src/main/java/org/zstack/testlib/vops/VOpsVirtualEndpointSpec.groovy +++ b/testlib/src/main/java/org/zstack/testlib/vops/VOpsVirtualEndpointSpec.groovy @@ -25,7 +25,7 @@ class VOpsVirtualEndpointSpec extends Spec { @Override SpecID create(String uuid, String sessionId) { - mockFactory(VOpsClient.class, { return new VOpsClientForTest(this) }) + mockFactory(VOpsClient.class, { return new VOpsClientSimulator(this) }) return id(endpointName, endpointUuid = uuid) } @@ -34,15 +34,15 @@ class VOpsVirtualEndpointSpec extends Spec { Test.functionForMockTestObjectFactory.remove(VOpsClient.class) } - public List> postHandlers = [] + public List> postHandlers = [] /** * @return a function to remove this handler */ BooleanSupplier registerPostHttpHandler( - Predicate predicate, - BiFunction, ErrorableValue> handler) { - def pair = new PostHandlerPair( + Predicate predicate, + BiFunction, ErrorableValue> handler) { + def pair = new PostHandlerPair( Objects.requireNonNull(predicate), Objects.requireNonNull(handler)) @@ -55,7 +55,7 @@ class VOpsVirtualEndpointSpec extends Spec { */ BooleanSupplier registerPostHttpHandler( String path, - BiFunction, ErrorableValue> handler) { + BiFunction, ErrorableValue> handler) { return registerPostHttpHandler({ it.path == path || it.pathWithoutIpAndPort == path }, handler) } @@ -65,7 +65,7 @@ class VOpsVirtualEndpointSpec extends Spec { BooleanSupplier registerPostHttpHandler( String path, HttpMethod method, - BiFunction, ErrorableValue> handler) { + BiFunction, ErrorableValue> handler) { return registerPostHttpHandler( { (it.path == path || it.pathWithoutIpAndPort == path) && it.method == method