Skip to content

Commit a81ccf7

Browse files
committed
feat: Add reverse group lookup for LDAP servers that do not return memberOf
1 parent ab54092 commit a81ccf7

2 files changed

Lines changed: 29 additions & 28 deletions

File tree

extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/LDAPCredentialsValidator.java

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,10 @@ public AuthenticationResult validateCredentials(
206206
throw new BasicSecurityAuthenticationException(Access.DEFAULT_ERROR_MESSAGE);
207207
}
208208

209+
if (this.ldapConfig.isGroupSearchConfigured() && !hasMemberOfAttribute(userResult)) {
210+
enrichWithGroupSearch(userResult);
211+
}
212+
209213
byte[] salt = BasicAuthUtils.generateSalt();
210214
byte[] hash = hashGenerator.getOrComputePasswordHash(password, salt, this.ldapConfig.getCredentialIterations());
211215
LdapUserPrincipal newPrincipal = new LdapUserPrincipal(
@@ -253,14 +257,6 @@ SearchResult getLdapUserObject(BasicAuthLDAPConfig ldapConfig, DirContext contex
253257
finally {
254258
results.close();
255259
}
256-
257-
// Some LDAP servers do not return memberOf in search results. When memberOf is
258-
// absent and group search configuration is provided, fall back to a reverse
259-
// group lookup.
260-
if (ldapConfig.isGroupSearchConfigured() && !hasMemberOfAttribute(userResult)) {
261-
populateMemberOfFromGroupSearch(ldapConfig, context, userResult);
262-
}
263-
264260
return userResult;
265261
}
266262
catch (NamingException e) {
@@ -276,47 +272,41 @@ private static boolean hasMemberOfAttribute(SearchResult userResult)
276272
}
277273

278274
@SuppressWarnings("BanJNDI")
279-
private void populateMemberOfFromGroupSearch(
280-
BasicAuthLDAPConfig ldapConfig,
281-
DirContext context,
282-
SearchResult userResult
283-
)
275+
private void enrichWithGroupSearch(SearchResult userResult)
284276
{
277+
final ClassLoader currentClassLoader = Thread.currentThread().getContextClassLoader();
278+
InitialDirContext dirContext = null;
285279
try {
280+
Thread.currentThread().setContextClassLoader(this.getClass().getClassLoader());
281+
dirContext = new InitialDirContext(bindProperties(this.ldapConfig));
282+
286283
final String userDn = userResult.getNameInNamespace();
287284
final SearchControls sc = new SearchControls();
288285
sc.setSearchScope(SearchControls.SUBTREE_SCOPE);
289-
// Request only the DN by returning a minimal attribute. The group DN is
290-
// obtained via getNameInNamespace() and does not depend on any returned attribute.
291286
sc.setReturningAttributes(new String[]{"1.1"});
292287

293-
final String filter = StringUtils.format(ldapConfig.getGroupSearch(), encodeForLDAP(userDn, true));
294-
final NamingEnumeration<SearchResult> groupResults = context.search(
295-
ldapConfig.getGroupBaseDn(),
288+
final String filter = StringUtils.format(this.ldapConfig.getGroupSearch(), encodeForLDAP(userDn, true));
289+
final NamingEnumeration<SearchResult> groupResults = dirContext.search(
290+
this.ldapConfig.getGroupBaseDn(),
296291
filter,
297292
sc
298293
);
299294

300295
final BasicAttribute memberOfAttr = new BasicAttribute("memberOf");
301296
try {
302297
while (groupResults.hasMore()) {
303-
final SearchResult groupResult = groupResults.next();
304-
final String groupDn = groupResult.getNameInNamespace();
305-
memberOfAttr.add(groupDn);
298+
memberOfAttr.add(groupResults.next().getNameInNamespace());
306299
}
307300
}
308301
finally {
309302
groupResults.close();
310303
}
311304

312305
if (memberOfAttr.size() > 0) {
313-
if (userResult.getAttributes() != null) {
314-
userResult.getAttributes().put(memberOfAttr);
315-
} else {
316-
final BasicAttributes attrs = new BasicAttributes(true);
317-
attrs.put(memberOfAttr);
318-
userResult.setAttributes(attrs);
306+
if (userResult.getAttributes() == null) {
307+
userResult.setAttributes(new BasicAttributes(true));
319308
}
309+
userResult.getAttributes().put(memberOfAttr);
320310
LOG.debug(
321311
"Populated memberOf for user '%s' with %d groups from reverse group search",
322312
userDn,
@@ -327,6 +317,18 @@ private void populateMemberOfFromGroupSearch(
327317
catch (NamingException e) {
328318
LOG.error(e, "Exception during reverse group lookup, proceeding without group memberships");
329319
}
320+
finally {
321+
try {
322+
if (dirContext != null) {
323+
dirContext.close();
324+
}
325+
}
326+
catch (Exception ignored) {
327+
LOG.warn("Exception closing LDAP context");
328+
// ignored
329+
}
330+
Thread.currentThread().setContextClassLoader(currentClassLoader);
331+
}
330332
}
331333

332334
boolean validatePassword(BasicAuthLDAPConfig ldapConfig, LdapName userDn, char[] password)

extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/validator/LDAPCredentialsValidatorTest.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,6 @@ public Context getInitialContext(Hashtable<?, ?> environment) throws NamingExcep
171171
ArgumentMatchers.any(SearchControls.class))
172172
).thenReturn(makeNamingEnum(userResults));
173173

174-
// Group search results
175174
final SearchResult group1 = new SearchResult("cn=admins,ou=Groups,dc=example,dc=org", null, new BasicAttributes(true));
176175
group1.setNameInNamespace("cn=admins,ou=Groups,dc=example,dc=org");
177176
final SearchResult group2 = new SearchResult("cn=developers,ou=Groups,dc=example,dc=org", null, new BasicAttributes(true));

0 commit comments

Comments
 (0)