Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

For convenience added convertToUperCase on LdapAuthenticationProviderConfigurer #10153

Closed
wants to merge 2 commits into from

Conversation

domi-87
Copy link

@domi-87 domi-87 commented Aug 2, 2021

Since LdapAuthenticationProviderConfigurer currently does not offer the possibility to pass the flag convertToUpperCase to the DefaultLdapAuthoritiesPopulator, I have extended this feature on the LdapAuthenticationProviderConfigurer accordingly. Previously, if you only wanted to set convertToUpperCase to 'false', you had to initialize the DefaultLdapAuthoritiesPopulator yourself and set all values accordingly.

Until now:

protected void configure(AuthenticationManagerBuilder auth) throws Exception {
  final var ldapAuth = auth.ldapAuthentication();

  final var authoritiesPopulator = new DefaultLdapAuthoritiesPopulator(contextSource, groupSearchBase);
  authoritiesPopulator.setGroupRoleAttribute(groupRoleAttribute);
  authoritiesPopulator.setGroupSearchFilter(groupSearchFilter);
  authoritiesPopulator.setSearchSubtree(groupSearchSubtree);
  authoritiesPopulator.setRolePrefix(rolePrefix);
  authoritiesPopulator.setConvertToUpperCase(false);
  ldapAuth.ldapAuthoritiesPopulator(authoritiesPopulator);
}

New:

protected void configure(AuthenticationManagerBuilder auth) throws Exception {
  final var ldapAuth = auth.ldapAuthentication();
  ldapAuth.convertToUpperCase(false);
}

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 2, 2021
@eleftherias eleftherias self-assigned this Aug 10, 2021
@eleftherias
Copy link
Contributor

Thanks for the suggestion @domi-87.

Using the AuthenticationManagerBuilder is no longer the preferred way of configuring LDAP authentication, which is why we aren't looking to add any additional functionality to it.

Currently the preferred way to set convertToUpperCase is to do something like this:

@Bean
LdapAuthenticationProvider authenticationProvider(LdapAuthenticator authenticator) {
	DefaultLdapAuthoritiesPopulator authoritiesPopulator = new DefaultLdapAuthoritiesPopulator(
			this.contextSource, this.groupSearchBase);
	authoritiesPopulator.setConvertToUpperCase(false);
	LdapAuthenticationProvider provider = new LdapAuthenticationProvider(authenticator, authoritiesPopulator);
	provider.setUserDetailsContextMapper(new PersonContextMapper());
	return provider;
}

We are planning to introduce a LdapAuthenticationManagerFactoryBean, which will make it easier to create an AuthenticationManager that can perform LDAP authentication.
You can track the progress in gh-10138.

For those reasons I am going to close this PR, but I will keep your suggestion in mind while working on the LdapAuthenticationManagerFactoryBean.

@eleftherias eleftherias added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants