Skip to content

Commit

Permalink
Improve LDAP auto-configuration conditions
Browse files Browse the repository at this point in the history
At present, auto-configuration of `LdapContextSource` is conditional on
presence of a `ContextSource` bean. However, there are valid use cases
which require multiple `ContextSource` bean, for instance
`PooledContextSource`. With the current arrangement, the
auto-configuration of `LdapContextSource` will back off if user provides
a `PooledContextSource` bean, while it would still be reasonable to
reuse the auto-configured `LdapContextSource`.

This commit improves `LdapContextSource` factory method return value and
condition to back off only if users actually provide a
`LdapContextSource` bean themselves.

See gh-13143
  • Loading branch information
vpavic authored and snicoll committed May 11, 2018
1 parent efda5ef commit dfceede
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 5 deletions.
Expand Up @@ -53,7 +53,7 @@ public LdapAutoConfiguration(LdapProperties properties, Environment environment)

@Bean
@ConditionalOnMissingBean
public ContextSource ldapContextSource() {
public LdapContextSource ldapContextSource() {
LdapContextSource source = new LdapContextSource();
source.setUserDn(this.properties.getUsername());
source.setPassword(this.properties.getPassword());
Expand Down
Expand Up @@ -58,7 +58,6 @@
import org.springframework.core.env.PropertySource;
import org.springframework.core.io.Resource;
import org.springframework.core.type.AnnotatedTypeMetadata;
import org.springframework.ldap.core.ContextSource;
import org.springframework.ldap.core.support.LdapContextSource;
import org.springframework.util.StringUtils;

Expand Down Expand Up @@ -101,7 +100,7 @@ public EmbeddedLdapAutoConfiguration(EmbeddedLdapProperties embeddedProperties,
@Bean
@DependsOn("directoryServer")
@ConditionalOnMissingBean
public ContextSource ldapContextSource() {
public LdapContextSource ldapContextSource() {
LdapContextSource source = new LdapContextSource();
if (hasCredentials(this.embeddedProperties.getCredential())) {
source.setUserDn(this.embeddedProperties.getCredential().getUsername());
Expand Down
Expand Up @@ -20,9 +20,13 @@

import org.springframework.boot.autoconfigure.AutoConfigurations;
import org.springframework.boot.test.context.runner.ApplicationContextRunner;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.ldap.core.ContextSource;
import org.springframework.ldap.core.LdapTemplate;
import org.springframework.ldap.core.support.LdapContextSource;
import org.springframework.ldap.pool2.factory.PoolConfig;
import org.springframework.ldap.pool2.factory.PooledContextSource;
import org.springframework.test.util.ReflectionTestUtils;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -54,7 +58,8 @@ public void contextSourceWithDefaultUrl() {
public void contextSourceWithSingleUrl() {
this.contextRunner.withPropertyValues("spring.ldap.urls:ldap://localhost:123")
.run((context) -> {
ContextSource contextSource = context.getBean(ContextSource.class);
ContextSource contextSource = context
.getBean(LdapContextSource.class);
String[] urls = (String[]) ReflectionTestUtils.getField(contextSource,
"urls");
assertThat(urls).containsExactly("ldap://localhost:123");
Expand All @@ -67,7 +72,8 @@ public void contextSourceWithSeveralUrls() {
.withPropertyValues(
"spring.ldap.urls:ldap://localhost:123,ldap://mycompany:123")
.run((context) -> {
ContextSource contextSource = context.getBean(ContextSource.class);
ContextSource contextSource = context
.getBean(LdapContextSource.class);
LdapProperties ldapProperties = context.getBean(LdapProperties.class);
String[] urls = (String[]) ReflectionTestUtils.getField(contextSource,
"urls");
Expand Down Expand Up @@ -104,4 +110,32 @@ public void templateExists() {
.run(context -> assertThat(context).hasSingleBean(LdapTemplate.class));
}

@Test
public void contextSourceWithUserProvidedPooledContextSource() {
this.contextRunner.withUserConfiguration(PooledContextSourceConfig.class)
.run((context) -> {
LdapContextSource contextSource = context
.getBean(LdapContextSource.class);
String[] urls = (String[]) ReflectionTestUtils.getField(contextSource,
"urls");
assertThat(urls).containsExactly("ldap://localhost:389");
assertThat(contextSource.isAnonymousReadOnly()).isFalse();
context.getBean(PooledContextSource.class);
});
}

@Configuration
static class PooledContextSourceConfig {

@Bean
public PooledContextSource pooledContextSource(
LdapContextSource ldapContextSource) {
PooledContextSource pooledContextSource = new PooledContextSource(
new PoolConfig());
pooledContextSource.setContextSource(ldapContextSource);
return pooledContextSource;
}

}

}

0 comments on commit dfceede

Please sign in to comment.