Skip to content

Commit c8a8518

Browse files
DaanHooglandDaanHoogland
committed
add test ldap test command and button
1 parent 5e5ae0c commit c8a8518

7 files changed

Lines changed: 288 additions & 19 deletions

File tree

‎plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapTestConfigurationCmd.java‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package org.apache.cloudstack.api.command;
18+
19+
import javax.inject.Inject;
20+
21+
import org.apache.cloudstack.api.APICommand;
22+
import org.apache.cloudstack.api.ApiConstants;
23+
import org.apache.cloudstack.api.BaseCmd;
24+
import org.apache.cloudstack.api.Parameter;
25+
import org.apache.cloudstack.api.ServerApiException;
26+
import org.apache.cloudstack.api.response.DomainResponse;
27+
import org.apache.cloudstack.api.response.SuccessResponse;
28+
import org.apache.cloudstack.ldap.LdapManager;
29+
30+
import com.cloud.exception.InvalidParameterValueException;
31+
import com.cloud.user.Account;
32+
33+
@APICommand(name = "testLdapConfiguration", description = "Tests connectivity to an LDAP server without saving a configuration", responseObject = SuccessResponse.class,
34+
since = "4.23.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
35+
public class LdapTestConfigurationCmd extends BaseCmd {
36+
37+
@Inject
38+
private LdapManager _ldapManager;
39+
40+
@Parameter(name = ApiConstants.HOST_NAME, type = CommandType.STRING, required = true, description = "Hostname")
41+
private String hostname;
42+
43+
@Parameter(name = ApiConstants.PORT, type = CommandType.INTEGER, description = "Port")
44+
private int port;
45+
46+
@Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, entityType = DomainResponse.class, description = "Linked Domain")
47+
private Long domainId;
48+
49+
public LdapTestConfigurationCmd() {
50+
super();
51+
}
52+
53+
public LdapTestConfigurationCmd(final LdapManager ldapManager) {
54+
super();
55+
_ldapManager = ldapManager;
56+
}
57+
58+
public String getHostname() {
59+
return hostname;
60+
}
61+
62+
public int getPort() {
63+
return port;
64+
}
65+
66+
public Long getDomainId() {
67+
return domainId;
68+
}
69+
70+
@Override
71+
public void execute() throws ServerApiException {
72+
SuccessResponse response = new SuccessResponse(getCommandName());
73+
try {
74+
_ldapManager.testConnection(this);
75+
response.setSuccess(true);
76+
response.setDisplayText("Successfully connected to the LDAP server");
77+
} catch (InvalidParameterValueException e) {
78+
response.setSuccess(false);
79+
response.setDisplayText(e.getMessage());
80+
}
81+
setResponseObject(response);
82+
}
83+
84+
@Override
85+
public long getEntityOwnerId() {
86+
return Account.ACCOUNT_ID_SYSTEM;
87+
}
88+
}

‎plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManager.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.apache.cloudstack.api.command.LdapAddConfigurationCmd;
2222
import org.apache.cloudstack.api.command.LdapDeleteConfigurationCmd;
2323
import org.apache.cloudstack.api.command.LdapListConfigurationCmd;
24+
import org.apache.cloudstack.api.command.LdapTestConfigurationCmd;
2425
import org.apache.cloudstack.api.command.LinkAccountToLdapCmd;
2526
import org.apache.cloudstack.api.command.LinkDomainToLdapCmd;
2627
import org.apache.cloudstack.api.command.UnlinkDomainFromLdapCmd;
@@ -41,6 +42,8 @@ enum LinkType { GROUP, OU }
4142

4243
LdapConfigurationResponse addConfiguration(String hostname, int port, Long domainId) throws InvalidParameterValueException;
4344

45+
void testConnection(LdapTestConfigurationCmd cmd) throws InvalidParameterValueException;
46+
4447
boolean canAuthenticate(String principal, String password, final Long domainId);
4548

4649
LdapConfigurationResponse createLdapConfigurationResponse(LdapConfigurationVO configuration);

‎plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java‎

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.apache.cloudstack.api.command.LdapImportUsersCmd;
3737
import org.apache.cloudstack.api.command.LdapListConfigurationCmd;
3838
import org.apache.cloudstack.api.command.LdapListUsersCmd;
39+
import org.apache.cloudstack.api.command.LdapTestConfigurationCmd;
3940
import org.apache.cloudstack.api.command.LdapUserSearchCmd;
4041
import org.apache.cloudstack.api.command.LinkAccountToLdapCmd;
4142
import org.apache.cloudstack.api.command.LinkDomainToLdapCmd;
@@ -173,30 +174,47 @@ private LdapConfigurationResponse addConfigurationInternal(final String hostname
173174
// hostname:port is unique for domain binding
174175
LdapConfigurationVO configuration = _ldapConfigurationDao.find(hostname, port, domainId);
175176
if (configuration == null) {
176-
LdapContext context = null;
177-
try {
178-
final String providerUrl = "ldap://" + hostname + ":" + port;
179-
context = _ldapContextFactory.createBindContext(providerUrl,domainId);
180-
configuration = new LdapConfigurationVO(hostname, port, domainId);
181-
_ldapConfigurationDao.persist(configuration);
182-
logger.info("Added a new LDAP server with URL: {}{}", providerUrl, domainId == null ? "" : " for domain " + domainId);
183-
return createLdapConfigurationResponse(configuration);
184-
} catch (NamingException | IOException e) {
185-
logger.debug("NamingException while doing an LDAP bind", e);
186-
throw new InvalidParameterValueException("Unable to bind to the given LDAP server");
187-
} catch (RuntimeException e) {
188-
if (e.getMessage().contains("Invalid truststore")) {
189-
throw new InvalidParameterValueException("Invalid truststore or truststore password");
190-
}
191-
throw e;
192-
} finally {
193-
closeContext(context);
194-
}
177+
testBind(hostname, port, domainId);
178+
configuration = new LdapConfigurationVO(hostname, port, domainId);
179+
_ldapConfigurationDao.persist(configuration);
180+
logger.info("Added a new LDAP server with URL: ldap://{}:{}{}", hostname, port, domainId == null ? "" : " for domain " + domainId);
181+
return createLdapConfigurationResponse(configuration);
195182
} else {
196183
throw new InvalidParameterValueException("Duplicate configuration");
197184
}
198185
}
199186

187+
@Override
188+
public void testConnection(LdapTestConfigurationCmd cmd) throws InvalidParameterValueException {
189+
int port = cmd.getPort();
190+
if (port <= 0) {
191+
port = 389;
192+
}
193+
testBind(cmd.getHostname(), port, cmd.getDomainId());
194+
}
195+
196+
/**
197+
* Binds to the given LDAP server without persisting a configuration, so both adding a new
198+
* configuration and {@link #testConnection} can share the same connectivity check.
199+
*/
200+
private void testBind(final String hostname, final int port, final Long domainId) throws InvalidParameterValueException {
201+
LdapContext context = null;
202+
try {
203+
final String providerUrl = "ldap://" + hostname + ":" + port;
204+
context = _ldapContextFactory.createBindContext(providerUrl, domainId);
205+
} catch (NamingException | IOException e) {
206+
logger.debug("NamingException while doing an LDAP bind", e);
207+
throw new InvalidParameterValueException("Unable to bind to the given LDAP server");
208+
} catch (RuntimeException e) {
209+
if (e.getMessage().contains("Invalid truststore")) {
210+
throw new InvalidParameterValueException("Invalid truststore or truststore password");
211+
}
212+
throw e;
213+
} finally {
214+
closeContext(context);
215+
}
216+
}
217+
200218
/**
201219
* TODO decide if the principal is good enough to get the domain id or we need to add it as parameter
202220
* @param principal ldap user
@@ -300,6 +318,7 @@ public List<Class<?>> getCommands() {
300318
cmdList.add(LdapUserSearchCmd.class);
301319
cmdList.add(LdapListUsersCmd.class);
302320
cmdList.add(LdapAddConfigurationCmd.class);
321+
cmdList.add(LdapTestConfigurationCmd.class);
303322
cmdList.add(LdapDeleteConfigurationCmd.class);
304323
cmdList.add(LdapListConfigurationCmd.class);
305324
cmdList.add(LdapCreateAccountCmd.class);

‎plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/LdapManagerImplTest.java‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package org.apache.cloudstack.ldap;
18+
19+
import com.cloud.domain.dao.DomainDao;
20+
import com.cloud.exception.InvalidParameterValueException;
21+
import com.cloud.user.AccountManager;
22+
import org.apache.cloudstack.api.command.LdapTestConfigurationCmd;
23+
import org.apache.cloudstack.ldap.dao.LdapConfigurationDao;
24+
import org.junit.Before;
25+
import org.junit.Test;
26+
import org.junit.runner.RunWith;
27+
import org.mockito.Mock;
28+
import org.mockito.junit.MockitoJUnitRunner;
29+
import org.springframework.test.util.ReflectionTestUtils;
30+
31+
import javax.naming.NamingException;
32+
33+
import static org.junit.Assert.assertThrows;
34+
import static org.mockito.ArgumentMatchers.any;
35+
import static org.mockito.ArgumentMatchers.anyLong;
36+
import static org.mockito.Mockito.doThrow;
37+
import static org.mockito.Mockito.never;
38+
import static org.mockito.Mockito.verify;
39+
import static org.mockito.Mockito.when;
40+
41+
/**
42+
* Regression tests for #6934: testing an LDAP connection must not persist a
43+
* configuration, and adding one must still bind before persisting.
44+
*/
45+
@RunWith(MockitoJUnitRunner.class)
46+
public class LdapManagerImplTest {
47+
48+
private static final long DOMAIN_ID = 1L;
49+
50+
private LdapManagerImpl ldapManager;
51+
52+
@Mock
53+
private LdapConfigurationDao ldapConfigurationDao;
54+
55+
@Mock
56+
private LdapContextFactory ldapContextFactory;
57+
58+
@Mock
59+
private DomainDao domainDao;
60+
61+
@Mock
62+
private AccountManager accountManager;
63+
64+
@Before
65+
public void setup() {
66+
ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, null, null);
67+
ReflectionTestUtils.setField(ldapManager, "domainDao", domainDao);
68+
ReflectionTestUtils.setField(ldapManager, "accountManager", accountManager);
69+
}
70+
71+
@Test
72+
public void testConnectionDoesNotPersistOnSuccess() throws Exception {
73+
LdapTestConfigurationCmd cmd = buildCmd("ldap.example.com", 389, DOMAIN_ID);
74+
75+
ldapManager.testConnection(cmd);
76+
77+
verify(ldapContextFactory).createBindContext("ldap://ldap.example.com:389", DOMAIN_ID);
78+
verify(ldapConfigurationDao, never()).persist(any());
79+
}
80+
81+
@Test
82+
public void testConnectionDefaultsPortWhenNotGiven() throws Exception {
83+
LdapTestConfigurationCmd cmd = buildCmd("ldap.example.com", 0, DOMAIN_ID);
84+
85+
ldapManager.testConnection(cmd);
86+
87+
verify(ldapContextFactory).createBindContext("ldap://ldap.example.com:389", DOMAIN_ID);
88+
}
89+
90+
@Test
91+
public void testConnectionThrowsOnBindFailure() throws Exception {
92+
LdapTestConfigurationCmd cmd = buildCmd("ldap.example.com", 389, DOMAIN_ID);
93+
doThrow(new NamingException("bind failed")).when(ldapContextFactory).createBindContext(any(), anyLong());
94+
95+
assertThrows(InvalidParameterValueException.class, () -> ldapManager.testConnection(cmd));
96+
97+
verify(ldapConfigurationDao, never()).persist(any());
98+
}
99+
100+
@Test
101+
public void addConfigurationStillBindsBeforePersisting() throws Exception {
102+
when(ldapConfigurationDao.find("ldap.example.com", 389, DOMAIN_ID)).thenReturn(null);
103+
when(ldapConfigurationDao.persist(any())).thenAnswer(invocation -> invocation.getArgument(0));
104+
105+
ldapManager.addConfiguration("ldap.example.com", 389, DOMAIN_ID);
106+
107+
verify(ldapContextFactory).createBindContext("ldap://ldap.example.com:389", DOMAIN_ID);
108+
verify(ldapConfigurationDao).persist(any());
109+
}
110+
111+
@Test
112+
public void addConfigurationDoesNotPersistOnBindFailure() throws Exception {
113+
when(ldapConfigurationDao.find("ldap.example.com", 389, DOMAIN_ID)).thenReturn(null);
114+
doThrow(new NamingException("bind failed")).when(ldapContextFactory).createBindContext(any(), anyLong());
115+
116+
assertThrows(InvalidParameterValueException.class, () -> ldapManager.addConfiguration("ldap.example.com", 389, DOMAIN_ID));
117+
118+
verify(ldapConfigurationDao, never()).persist(any());
119+
}
120+
121+
private LdapTestConfigurationCmd buildCmd(String hostname, int port, long domainId) {
122+
LdapTestConfigurationCmd cmd = new LdapTestConfigurationCmd();
123+
ReflectionTestUtils.setField(cmd, "hostname", hostname);
124+
ReflectionTestUtils.setField(cmd, "port", port);
125+
ReflectionTestUtils.setField(cmd, "domainId", domainId);
126+
return cmd;
127+
}
128+
}

‎ui/public/locales/en.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2676,6 +2676,7 @@
26762676
"label.tenantname": "Netris Tenant",
26772677
"label.term.type": "Term type",
26782678
"label.test": "Test",
2679+
"label.test.ldap.configuration": "Test LDAP Connection",
26792680
"label.test.webhook.delivery": "Test Webhook Delivery",
26802681
"label.tftpdir": "TFTP root directory",
26812682
"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`",

‎ui/src/config/section/config.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,34 @@ export default {
5252
'hostname', 'port', 'domainid'
5353
]
5454
},
55+
{
56+
api: 'testLdapConfiguration',
57+
icon: 'ExperimentOutlined',
58+
label: 'label.test.ldap.configuration',
59+
docHelp: 'adminguide/accounts.html#using-an-ldap-server-for-user-authentication',
60+
listView: true,
61+
args: [
62+
'hostname', 'port', 'domainid'
63+
]
64+
},
65+
{
66+
api: 'testLdapConfiguration',
67+
icon: 'ExperimentOutlined',
68+
label: 'label.test.ldap.configuration',
69+
dataView: true,
70+
args: ['hostname', 'port', 'domainid'],
71+
mapping: {
72+
hostname: {
73+
value: (record) => { return record.hostname }
74+
},
75+
port: {
76+
value: (record) => { return record.port }
77+
},
78+
domainid: {
79+
value: (record) => { return record.domainid }
80+
}
81+
}
82+
},
5583
{
5684
api: 'deleteLdapConfiguration',
5785
icon: 'delete-outlined',

‎ui/src/core/lazy_lib/icons_use.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ import {
7878
EnvironmentOutlined,
7979
ExceptionOutlined,
8080
ExclamationCircleOutlined,
81+
ExperimentOutlined,
8182
EyeInvisibleOutlined,
8283
EyeOutlined,
8384
FieldTimeOutlined,
@@ -254,6 +255,7 @@ export default {
254255
app.component('EnvironmentOutlined', EnvironmentOutlined)
255256
app.component('ExceptionOutlined', ExceptionOutlined)
256257
app.component('ExclamationCircleOutlined', ExclamationCircleOutlined)
258+
app.component('ExperimentOutlined', ExperimentOutlined)
257259
app.component('EyeInvisibleOutlined', EyeInvisibleOutlined)
258260
app.component('EyeOutlined', EyeOutlined)
259261
app.component('FieldTimeOutlined', FieldTimeOutlined)

0 commit comments

Comments
 (0)