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 <[email protected]> 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)
