Change ActiveDirectoryLdapAuthenticationProvider to use LdapClient

[therepanic]

Replaces SpringSecurityLdapTemplate with LdapClient for user search operations.

Closes: gh-17291

[therepanic]

This is what you should do, otherwise the tests will fail.

[jzheaux]

I think the reason is because SpringSecurityLdapTemplate propagates the javax.naming exception while LdapClient wraps them. For passivity reasons, let's do something closer to:

if (ex.getCause() instanceof NamingException original) {
    throw original;
}
throw badCredentials(ex);

In this way, the original naming exception is propagated like it was before.

[therepanic]

I'm not entirely sure whether this is a breaking change or not. Because when switching to LdapClient, we don't accept any(Object[].class) as the fourth argument, so we don't need to mock it.

[jzheaux]

I think this is okay, though I'd recommend that we still use eq to test the filter so that the expectations are still the same level of specificity:

String customSearchFilter = "(&(objectClass=user)(sAMAccountName={0}))";
String domain = "mydomain.eu";
String encoded = MessageFormat.format(customSearchFilter, this.joe.getPrincipal() + "@" + domain);
...
given(this.ctx.search(any(Name.class), eq(encoded), any(SearchControls.class)))
		.willReturn(new MockNamingEnumeration(sr));
...

[therepanic]

I don't quite understand whether this is breaking Change or not, please take a look at #18627 (comment)

[rwinch]

FYI I rebased based off origin/main

[jzheaux]

Thanks for the PR, @therepanic! I've left some feedback inline.

[jzheaux]

I think this is okay, though I'd recommend that we still use eq to test the filter so that the expectations are still the same level of specificity:

String customSearchFilter = "(&(objectClass=user)(sAMAccountName={0}))";
String domain = "mydomain.eu";
String encoded = MessageFormat.format(customSearchFilter, this.joe.getPrincipal() + "@" + domain);
...
given(this.ctx.search(any(Name.class), eq(encoded), any(SearchControls.class)))
		.willReturn(new MockNamingEnumeration(sr));
...

[jzheaux]

I think the reason is because SpringSecurityLdapTemplate propagates the javax.naming exception while LdapClient wraps them. For passivity reasons, let's do something closer to:

if (ex.getCause() instanceof NamingException original) {
    throw original;
}
throw badCredentials(ex);

In this way, the original naming exception is propagated like it was before.

[jzheaux]

This seems like something we should consider as an enhancement to LdapClient; a way to indicate that no results is an error. Will you please file an issue?

[jzheaux]

This might be slightly more readable as:

LdapQuery query = LdapQueryBuilder.query()
		.base(searchRoot)
		.searchScope(SearchScope.SUBTREE)
		.filter(this.searchFilter, bindPrincipal, username);
DirContextOperations result = ldapClient.search().query(query).toEntry();