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 20:08:38 UTC
(cloudstack) branch ghi6934-test-ldap-connection created (now c8a8518c8fd)
This is an automated email from the ASF dual-hosted git repository.
DaanHoogland pushed a change to branch ghi6934-test-ldap-connection
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
at c8a8518c8fd add test ldap test command and button
This branch includes the following new commits:
new c8a8518c8fd add test ldap test command and button
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: add test ldap test command and button
Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
DaanHoogland pushed a commit to branch ghi6934-test-ldap-connection
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit c8a8518c8fd7b127f3293c27dfe04b94f48e0c93
Author: Daan Hoogland <da...@apache.org>
AuthorDate: Sat Aug 22 22:07:47 2026 +0200
add test ldap test command and button
---
.../api/command/LdapTestConfigurationCmd.java | 88 ++++++++++++++
.../org/apache/cloudstack/ldap/LdapManager.java | 3 +
.../apache/cloudstack/ldap/LdapManagerImpl.java | 57 ++++++---
.../cloudstack/ldap/LdapManagerImplTest.java | 128 +++++++++++++++++++++
ui/public/locales/en.json | 1 +
ui/src/config/section/config.js | 28 +++++
ui/src/core/lazy_lib/icons_use.js | 2 +
7 files changed, 288 insertions(+), 19 deletions(-)
diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapTestConfigurationCmd.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapTestConfigurationCmd.java
new file mode 100644
index 00000000000..b98c7fedaab
--- /dev/null
+++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapTestConfigurationCmd.java
@@ -0,0 +1,88 @@
+// 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.api.command;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.DomainResponse;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.ldap.LdapManager;
+
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.user.Account;
+
+@APICommand(name = "testLdapConfiguration", description = "Tests connectivity to an LDAP server without saving a configuration", responseObject = SuccessResponse.class,
+ since = "4.23.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+public class LdapTestConfigurationCmd extends BaseCmd {
+
+ @Inject
+ private LdapManager _ldapManager;
+
+ @Parameter(name = ApiConstants.HOST_NAME, type = CommandType.STRING, required = true, description = "Hostname")
+ private String hostname;
+
+ @Parameter(name = ApiConstants.PORT, type = CommandType.INTEGER, description = "Port")
+ private int port;
+
+ @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, entityType = DomainResponse.class, description = "Linked Domain")
+ private Long domainId;
+
+ public LdapTestConfigurationCmd() {
+ super();
+ }
+
+ public LdapTestConfigurationCmd(final LdapManager ldapManager) {
+ super();
+ _ldapManager = ldapManager;
+ }
+
+ public String getHostname() {
+ return hostname;
+ }
+
+ public int getPort() {
+ return port;
+ }
+
+ public Long getDomainId() {
+ return domainId;
+ }
+
+ @Override
+ public void execute() throws ServerApiException {
+ SuccessResponse response = new SuccessResponse(getCommandName());
+ try {
+ _ldapManager.testConnection(this);
+ response.setSuccess(true);
+ response.setDisplayText("Successfully connected to the LDAP server");
+ } catch (InvalidParameterValueException e) {
+ response.setSuccess(false);
+ response.setDisplayText(e.getMessage());
+ }
+ setResponseObject(response);
+ }
+
+ @Override
+ public long getEntityOwnerId() {
+ return Account.ACCOUNT_ID_SYSTEM;
+ }
+}
diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManager.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManager.java
index 7e561ccf754..d432340df61 100644
--- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManager.java
+++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManager.java
@@ -21,6 +21,7 @@ import java.util.List;
import org.apache.cloudstack.api.command.LdapAddConfigurationCmd;
import org.apache.cloudstack.api.command.LdapDeleteConfigurationCmd;
import org.apache.cloudstack.api.command.LdapListConfigurationCmd;
+import org.apache.cloudstack.api.command.LdapTestConfigurationCmd;
import org.apache.cloudstack.api.command.LinkAccountToLdapCmd;
import org.apache.cloudstack.api.command.LinkDomainToLdapCmd;
import org.apache.cloudstack.api.command.UnlinkDomainFromLdapCmd;
@@ -41,6 +42,8 @@ public interface LdapManager extends PluggableService {
LdapConfigurationResponse addConfiguration(String hostname, int port, Long domainId) throws InvalidParameterValueException;
+ void testConnection(LdapTestConfigurationCmd cmd) throws InvalidParameterValueException;
+
boolean canAuthenticate(String principal, String password, final Long domainId);
LdapConfigurationResponse createLdapConfigurationResponse(LdapConfigurationVO configuration);
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..5cdc9d6e9a6 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
@@ -36,6 +36,7 @@ import org.apache.cloudstack.api.command.LdapDeleteConfigurationCmd;
import org.apache.cloudstack.api.command.LdapImportUsersCmd;
import org.apache.cloudstack.api.command.LdapListConfigurationCmd;
import org.apache.cloudstack.api.command.LdapListUsersCmd;
+import org.apache.cloudstack.api.command.LdapTestConfigurationCmd;
import org.apache.cloudstack.api.command.LdapUserSearchCmd;
import org.apache.cloudstack.api.command.LinkAccountToLdapCmd;
import org.apache.cloudstack.api.command.LinkDomainToLdapCmd;
@@ -173,30 +174,47 @@ public class LdapManagerImpl extends ComponentLifecycleBase implements LdapManag
// hostname:port is unique for domain binding
LdapConfigurationVO configuration = _ldapConfigurationDao.find(hostname, port, domainId);
if (configuration == null) {
- LdapContext context = null;
- try {
- final String providerUrl = "ldap://" + hostname + ":" + port;
- context = _ldapContextFactory.createBindContext(providerUrl,domainId);
- configuration = new LdapConfigurationVO(hostname, port, domainId);
- _ldapConfigurationDao.persist(configuration);
- logger.info("Added a new LDAP server with URL: {}{}", providerUrl, domainId == null ? "" : " for domain " + domainId);
- return createLdapConfigurationResponse(configuration);
- } catch (NamingException | IOException e) {
- logger.debug("NamingException while doing an LDAP bind", e);
- throw new InvalidParameterValueException("Unable to bind to the given LDAP server");
- } catch (RuntimeException e) {
- if (e.getMessage().contains("Invalid truststore")) {
- throw new InvalidParameterValueException("Invalid truststore or truststore password");
- }
- throw e;
- } finally {
- closeContext(context);
- }
+ testBind(hostname, port, domainId);
+ configuration = new LdapConfigurationVO(hostname, port, domainId);
+ _ldapConfigurationDao.persist(configuration);
+ logger.info("Added a new LDAP server with URL: ldap://{}:{}{}", hostname, port, domainId == null ? "" : " for domain " + domainId);
+ return createLdapConfigurationResponse(configuration);
} else {
throw new InvalidParameterValueException("Duplicate configuration");
}
}
+ @Override
+ public void testConnection(LdapTestConfigurationCmd cmd) throws InvalidParameterValueException {
+ int port = cmd.getPort();
+ if (port <= 0) {
+ port = 389;
+ }
+ testBind(cmd.getHostname(), port, cmd.getDomainId());
+ }
+
+ /**
+ * Binds to the given LDAP server without persisting a configuration, so both adding a new
+ * configuration and {@link #testConnection} can share the same connectivity check.
+ */
+ private void testBind(final String hostname, final int port, final Long domainId) throws InvalidParameterValueException {
+ LdapContext context = null;
+ try {
+ final String providerUrl = "ldap://" + hostname + ":" + port;
+ context = _ldapContextFactory.createBindContext(providerUrl, domainId);
+ } catch (NamingException | IOException e) {
+ logger.debug("NamingException while doing an LDAP bind", e);
+ throw new InvalidParameterValueException("Unable to bind to the given LDAP server");
+ } catch (RuntimeException e) {
+ if (e.getMessage().contains("Invalid truststore")) {
+ throw new InvalidParameterValueException("Invalid truststore or truststore password");
+ }
+ throw e;
+ } finally {
+ closeContext(context);
+ }
+ }
+
/**
* TODO decide if the principal is good enough to get the domain id or we need to add it as parameter
* @param principal ldap user
@@ -300,6 +318,7 @@ public class LdapManagerImpl extends ComponentLifecycleBase implements LdapManag
cmdList.add(LdapUserSearchCmd.class);
cmdList.add(LdapListUsersCmd.class);
cmdList.add(LdapAddConfigurationCmd.class);
+ cmdList.add(LdapTestConfigurationCmd.class);
cmdList.add(LdapDeleteConfigurationCmd.class);
cmdList.add(LdapListConfigurationCmd.class);
cmdList.add(LdapCreateAccountCmd.class);
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..91f13c49050
--- /dev/null
+++ b/plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/LdapManagerImplTest.java
@@ -0,0 +1,128 @@
+// 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.dao.DomainDao;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.user.AccountManager;
+import org.apache.cloudstack.api.command.LdapTestConfigurationCmd;
+import org.apache.cloudstack.ldap.dao.LdapConfigurationDao;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+import org.springframework.test.util.ReflectionTestUtils;
+
+import javax.naming.NamingException;
+
+import static org.junit.Assert.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+/**
+ * Regression tests for #6934: testing an LDAP connection must not persist a
+ * configuration, and adding one must still bind before persisting.
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class LdapManagerImplTest {
+
+ private static final long DOMAIN_ID = 1L;
+
+ private LdapManagerImpl ldapManager;
+
+ @Mock
+ private LdapConfigurationDao ldapConfigurationDao;
+
+ @Mock
+ private LdapContextFactory ldapContextFactory;
+
+ @Mock
+ private DomainDao domainDao;
+
+ @Mock
+ private AccountManager accountManager;
+
+ @Before
+ public void setup() {
+ ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, null, null);
+ ReflectionTestUtils.setField(ldapManager, "domainDao", domainDao);
+ ReflectionTestUtils.setField(ldapManager, "accountManager", accountManager);
+ }
+
+ @Test
+ public void testConnectionDoesNotPersistOnSuccess() throws Exception {
+ LdapTestConfigurationCmd cmd = buildCmd("ldap.example.com", 389, DOMAIN_ID);
+
+ ldapManager.testConnection(cmd);
+
+ verify(ldapContextFactory).createBindContext("ldap://ldap.example.com:389", DOMAIN_ID);
+ verify(ldapConfigurationDao, never()).persist(any());
+ }
+
+ @Test
+ public void testConnectionDefaultsPortWhenNotGiven() throws Exception {
+ LdapTestConfigurationCmd cmd = buildCmd("ldap.example.com", 0, DOMAIN_ID);
+
+ ldapManager.testConnection(cmd);
+
+ verify(ldapContextFactory).createBindContext("ldap://ldap.example.com:389", DOMAIN_ID);
+ }
+
+ @Test
+ public void testConnectionThrowsOnBindFailure() throws Exception {
+ LdapTestConfigurationCmd cmd = buildCmd("ldap.example.com", 389, DOMAIN_ID);
+ doThrow(new NamingException("bind failed")).when(ldapContextFactory).createBindContext(any(), anyLong());
+
+ assertThrows(InvalidParameterValueException.class, () -> ldapManager.testConnection(cmd));
+
+ verify(ldapConfigurationDao, never()).persist(any());
+ }
+
+ @Test
+ public void addConfigurationStillBindsBeforePersisting() throws Exception {
+ when(ldapConfigurationDao.find("ldap.example.com", 389, DOMAIN_ID)).thenReturn(null);
+ when(ldapConfigurationDao.persist(any())).thenAnswer(invocation -> invocation.getArgument(0));
+
+ ldapManager.addConfiguration("ldap.example.com", 389, DOMAIN_ID);
+
+ verify(ldapContextFactory).createBindContext("ldap://ldap.example.com:389", DOMAIN_ID);
+ verify(ldapConfigurationDao).persist(any());
+ }
+
+ @Test
+ public void addConfigurationDoesNotPersistOnBindFailure() throws Exception {
+ when(ldapConfigurationDao.find("ldap.example.com", 389, DOMAIN_ID)).thenReturn(null);
+ doThrow(new NamingException("bind failed")).when(ldapContextFactory).createBindContext(any(), anyLong());
+
+ assertThrows(InvalidParameterValueException.class, () -> ldapManager.addConfiguration("ldap.example.com", 389, DOMAIN_ID));
+
+ verify(ldapConfigurationDao, never()).persist(any());
+ }
+
+ private LdapTestConfigurationCmd buildCmd(String hostname, int port, long domainId) {
+ LdapTestConfigurationCmd cmd = new LdapTestConfigurationCmd();
+ ReflectionTestUtils.setField(cmd, "hostname", hostname);
+ ReflectionTestUtils.setField(cmd, "port", port);
+ ReflectionTestUtils.setField(cmd, "domainId", domainId);
+ return cmd;
+ }
+}
diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json
index f57460efa48..32507294de0 100644
--- a/ui/public/locales/en.json
+++ b/ui/public/locales/en.json
@@ -2676,6 +2676,7 @@
"label.tenantname": "Netris Tenant",
"label.term.type": "Term type",
"label.test": "Test",
+"label.test.ldap.configuration": "Test LDAP Connection",
"label.test.webhook.delivery": "Test Webhook Delivery",
"label.tftpdir": "TFTP root directory",
"label.theme.alert": "The setting is only visible to the current browser. To apply the setting, please download the JSON file and replace its content in the `theme` section of the `config.json` file under the path: `/public/config.json`",
diff --git a/ui/src/config/section/config.js b/ui/src/config/section/config.js
index a2c12ce236a..ce1193fc515 100644
--- a/ui/src/config/section/config.js
+++ b/ui/src/config/section/config.js
@@ -52,6 +52,34 @@ export default {
'hostname', 'port', 'domainid'
]
},
+ {
+ api: 'testLdapConfiguration',
+ icon: 'ExperimentOutlined',
+ label: 'label.test.ldap.configuration',
+ docHelp: 'adminguide/accounts.html#using-an-ldap-server-for-user-authentication',
+ listView: true,
+ args: [
+ 'hostname', 'port', 'domainid'
+ ]
+ },
+ {
+ api: 'testLdapConfiguration',
+ icon: 'ExperimentOutlined',
+ label: 'label.test.ldap.configuration',
+ dataView: true,
+ args: ['hostname', 'port', 'domainid'],
+ mapping: {
+ hostname: {
+ value: (record) => { return record.hostname }
+ },
+ port: {
+ value: (record) => { return record.port }
+ },
+ domainid: {
+ value: (record) => { return record.domainid }
+ }
+ }
+ },
{
api: 'deleteLdapConfiguration',
icon: 'delete-outlined',
diff --git a/ui/src/core/lazy_lib/icons_use.js b/ui/src/core/lazy_lib/icons_use.js
index 43c6f822de3..244a6d2bb4e 100644
--- a/ui/src/core/lazy_lib/icons_use.js
+++ b/ui/src/core/lazy_lib/icons_use.js
@@ -78,6 +78,7 @@ import {
EnvironmentOutlined,
ExceptionOutlined,
ExclamationCircleOutlined,
+ ExperimentOutlined,
EyeInvisibleOutlined,
EyeOutlined,
FieldTimeOutlined,
@@ -254,6 +255,7 @@ export default {
app.component('EnvironmentOutlined', EnvironmentOutlined)
app.component('ExceptionOutlined', ExceptionOutlined)
app.component('ExclamationCircleOutlined', ExclamationCircleOutlined)
+ app.component('ExperimentOutlined', ExperimentOutlined)
app.component('EyeInvisibleOutlined', EyeInvisibleOutlined)
app.component('EyeOutlined', EyeOutlined)
app.component('FieldTimeOutlined', FieldTimeOutlined)