Skip to content

Commit 6bf7e96

Browse files
benbroadawaybenbroadaway
committed
ldap: add test coverage for common actions, suport AD (name) and standard LDAP (cn) group schemas for getGroup action
1 parent 5610c72 commit 6bf7e96

5 files changed

Lines changed: 549 additions & 2 deletions

File tree

‎tasks/ldap/pom.xml‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@
1313
<artifactId>ldap-task</artifactId>
1414
<packaging>jar</packaging>
1515

16+
<properties>
17+
<lldap.image>lldap/lldap:v0.6.1-alpine</lldap.image>
18+
</properties>
19+
1620
<dependencies>
1721
<dependency>
1822
<groupId>com.walmartlabs.concord</groupId>
@@ -39,14 +43,72 @@
3943
<artifactId>slf4j-api</artifactId>
4044
<scope>provided</scope>
4145
</dependency>
46+
47+
<dependency>
48+
<groupId>org.junit.jupiter</groupId>
49+
<artifactId>junit-jupiter-api</artifactId>
50+
<scope>test</scope>
51+
</dependency>
52+
<dependency>
53+
<groupId>ch.qos.logback</groupId>
54+
<artifactId>logback-classic</artifactId>
55+
<scope>test</scope>
56+
</dependency>
57+
<dependency>
58+
<groupId>org.testcontainers</groupId>
59+
<artifactId>testcontainers</artifactId>
60+
<scope>test</scope>
61+
</dependency>
62+
<dependency>
63+
<groupId>org.testcontainers</groupId>
64+
<artifactId>testcontainers-junit-jupiter</artifactId>
65+
<scope>test</scope>
66+
</dependency>
67+
<dependency>
68+
<groupId>org.testcontainers</groupId>
69+
<artifactId>testcontainers-ldap</artifactId>
70+
<version>2.0.5</version>
71+
<scope>test</scope>
72+
</dependency>
73+
<dependency>
74+
<groupId>org.mockito</groupId>
75+
<artifactId>mockito-core</artifactId>
76+
<scope>test</scope>
77+
</dependency>
78+
<dependency>
79+
<groupId>org.mockito</groupId>
80+
<artifactId>mockito-junit-jupiter</artifactId>
81+
<scope>test</scope>
82+
</dependency>
4283
</dependencies>
4384

85+
<dependencyManagement>
86+
<dependencies>
87+
<dependency>
88+
<groupId>org.testcontainers</groupId>
89+
<artifactId>testcontainers-bom</artifactId>
90+
<version>2.0.5</version>
91+
<type>pom</type>
92+
<scope>import</scope>
93+
</dependency>
94+
</dependencies>
95+
</dependencyManagement>
96+
4497
<build>
4598
<plugins>
4699
<plugin>
47100
<groupId>dev.ybrig.concord</groupId>
48101
<artifactId>concord-maven-plugin</artifactId>
49102
</plugin>
103+
<plugin>
104+
<groupId>org.apache.maven.plugins</groupId>
105+
<artifactId>maven-surefire-plugin</artifactId>
106+
<configuration>
107+
<environmentVariables>
108+
<LLDAP_IMAGE>${lldap.image}</LLDAP_IMAGE>
109+
</environmentVariables>
110+
</configuration>
111+
</plugin>
50112
</plugins>
51113
</build>
52114
</project>

‎tasks/ldap/src/main/java/com/walmartlabs/concord/plugins/ldap/LdapTaskCommon.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,8 @@ private SearchResult getUser(LdapConnectionCfg cfg, String searchBase, String us
143143

144144
private SearchResult getGroup(LdapConnectionCfg cfg, String searchBase, String group, List<String> securityGroupTypes, boolean securityEnabled) {
145145
try {
146-
// create custom filter for group
147-
String searchFilter = "(name=" + group + ")";
146+
// Support both AD-style 'name' and standard LDAP 'cn' attributes
147+
String searchFilter = "(|(name=" + group + ")(cn=" + group + "))";
148148

149149
// use private method search
150150
NamingEnumeration<SearchResult> results = withRetry(MAX_RETRIES, RETRY_DELAY, () -> search(cfg, searchBase, searchFilter));
@@ -161,6 +161,13 @@ private SearchResult getGroup(LdapConnectionCfg cfg, String searchBase, String g
161161
if (groupType != null && securityGroupTypes.stream().anyMatch(groupType::equals) == securityEnabled) {
162162
return result;
163163
}
164+
165+
// Standard LDAP servers (non-AD) don't expose distinguishedName or groupType
166+
// as attributes; treat any matching group as a result when security filtering
167+
// is not requested
168+
if (dn == null && groupType == null && !securityEnabled) {
169+
return result;
170+
}
164171
}
165172

166173
return null;

‎tasks/ldap/src/test/java/com/walmartlabs/concord/plugins/ldap/LdapActiveDirectoryIT.java‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
package com.walmartlabs.concord.plugins.ldap;
2+
3+
/*-
4+
* *****
5+
* Concord
6+
* -----
7+
* Copyright (C) 2017 - 2026 Walmart Inc., Concord Authors
8+
* -----
9+
* Licensed under the Apache License, Version 2.0 (the "License");
10+
* you may not use this file except in compliance with the License.
11+
* You may obtain a copy of the License at
12+
*
13+
* http://www.apache.org/licenses/LICENSE-2.0
14+
*
15+
* Unless required by applicable law or agreed to in writing, software
16+
* distributed under the License is distributed on an "AS IS" BASIS,
17+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
18+
* See the License for the specific language governing permissions and
19+
* limitations under the License.
20+
* =====
21+
*/
22+
23+
import com.walmartlabs.concord.runtime.v2.sdk.MapBackedVariables;
24+
import org.junit.jupiter.api.Disabled;
25+
import org.junit.jupiter.api.Test;
26+
import org.slf4j.Logger;
27+
import org.slf4j.LoggerFactory;
28+
29+
import java.util.Arrays;
30+
import java.util.List;
31+
import java.util.Map;
32+
33+
import static org.junit.jupiter.api.Assertions.*;
34+
35+
/**
36+
* Manual integration tests against a real Active Directory instance.
37+
* <p>
38+
* All tests in this class are {@link Disabled} and must be run explicitly.
39+
* Supply connection details as system properties, e.g.:
40+
* <pre>
41+
* mvn test -pl tasks/ldap -Dtest=LdapActiveDirectoryIT#testGetGroup \
42+
* -DadServer=ldap://ad.corp.example.com \
43+
* -DbindUserDn="CN=svc_ldap,OU=ServiceAccounts,DC=corp,DC=example,DC=com" \
44+
* -DbindPassword=secret \
45+
* -DsearchBase="DC=corp,DC=example,DC=com" \
46+
* -DgroupName="My Security Group" \
47+
* -DsecurityEnabled=true \
48+
* -DsecurityGroupTypes="-2147483640,-2147483646"
49+
* </pre>
50+
*
51+
* <table border="1">
52+
* <caption>System properties</caption>
53+
* <tr><th>Property</th><th>Description</th><th>Required</th></tr>
54+
* <tr><td>{@code adServer}</td><td>LDAP URL, e.g. {@code ldap://ad.corp.example.com}</td><td>yes</td></tr>
55+
* <tr><td>{@code bindUserDn}</td><td>Full DN of the bind user</td><td>yes</td></tr>
56+
* <tr><td>{@code bindPassword}</td><td>Bind user password</td><td>yes</td></tr>
57+
* <tr><td>{@code searchBase}</td><td>LDAP search base, e.g. {@code DC=corp,DC=example,DC=com}</td><td>yes</td></tr>
58+
* <tr><td>{@code groupName}</td><td>AD group name to look up</td><td>yes</td></tr>
59+
* <tr><td>{@code securityEnabled}</td><td>{@code true} for security groups, {@code false} otherwise (default: {@code false})</td><td>no</td></tr>
60+
* <tr><td>{@code securityGroupTypes}</td><td>Comma-separated list of AD {@code groupType} values to treat as security groups</td><td>no</td></tr>
61+
* </table>
62+
*/
63+
@Disabled("Requires a real Active Directory instance — run manually with the system properties documented in the class Javadoc")
64+
class LdapActiveDirectoryIT {
65+
66+
private static final Logger log = LoggerFactory.getLogger(LdapActiveDirectoryIT.class);
67+
68+
@Test
69+
void testGetGroup() {
70+
String adServer = requireProp("adServer");
71+
String bindUserDn = requireProp("bindUserDn");
72+
String bindPassword = requireProp("bindPassword");
73+
String searchBase = requireProp("searchBase");
74+
String groupName = requireProp("groupName");
75+
boolean securityEnabled = Boolean.parseBoolean(System.getProperty("securityEnabled", "false"));
76+
List<String> securityGroupTypes = parseList(System.getProperty("securityGroupTypes", ""));
77+
78+
var vars = new MapBackedVariables(Map.of(
79+
"action", "getGroup",
80+
"ldapAdServer", adServer,
81+
"bindUserDn", bindUserDn,
82+
"bindPassword", bindPassword,
83+
"searchBase", searchBase,
84+
"group", groupName,
85+
"securityEnabled", securityEnabled,
86+
"securityGroupTypes", securityGroupTypes
87+
));
88+
89+
var result = new LdapTaskCommon().execute(TaskParams.of(vars, Map.of(), Map.of()));
90+
91+
assertTrue((Boolean) result.get("success"),
92+
"Expected group '" + groupName + "' to be found in AD");
93+
assertNotNull(result.get("result"), "Expected a non-null group result");
94+
95+
@SuppressWarnings("unchecked")
96+
var attrs = (Map<String, Object>) ((Map<String, Object>) result.get("result")).get("attributes");
97+
assertNotNull(attrs, "Expected attributes to be present in the result");
98+
for (Map.Entry<String, Object> e : attrs.entrySet()) {
99+
log.info("{} = {}", e.getKey(), e.getValue());
100+
}
101+
}
102+
103+
private static String requireProp(String name) {
104+
String value = System.getProperty(name);
105+
if (value == null || value.isBlank()) {
106+
throw new IllegalStateException(
107+
"Required system property '" + name + "' is not set. "
108+
+ "See the class Javadoc for usage.");
109+
}
110+
return value;
111+
}
112+
113+
private static List<String> parseList(String csv) {
114+
if (csv == null || csv.isBlank()) {
115+
return List.of();
116+
}
117+
return Arrays.stream(csv.split(","))
118+
.map(String::trim)
119+
.filter(s -> !s.isEmpty())
120+
.toList();
121+
}
122+
}

0 commit comments

Comments
 (0)