[java-shib-attribute] branch main updated: JSATTR-32 - Make LDAP connector config immutable
Daniel Fisher
dfisher at vt.edu
Mon Mar 31 02:46:39 UTC 2025
This is an automated email from the git hooks/post-receive script.
dfisher pushed a commit to branch main
in repository java-shib-attribute.
View the commit online:
http://git.shibboleth.net/view/?p=java-shib-attribute.git;a=commit;h=a446432403ba621c4fbeb4f7ea7f79918aad6981
The following commit(s) were added to refs/heads/main by this push:
new a44643240 JSATTR-32 - Make LDAP connector config immutable
a44643240 is described below
commit a446432403ba621c4fbeb4f7ea7f79918aad6981
Author: Daniel Fisher <dfisher at vt.edu>
AuthorDate: Fri Jul 26 15:38:36 2024 -0400
JSATTR-32 - Make LDAP connector config immutable
https://shibboleth.atlassian.net/browse/JSATTR-32
Invoke freeze on configuration objects to prevent modification.
---
.../resolver/dc/ldap/impl/LDAPDataConnector.java | 2 +-
.../dc/ldap/impl/LDAPDataConnectorParser.java | 68 +++++++++++-----------
.../dc/ldap/impl/LDAPDataConnectorParserTest.java | 37 ++++++++++--
.../ldap-attribute-resolver-spring-context.xml | 2 +-
4 files changed, 69 insertions(+), 40 deletions(-)
diff --git a/shib-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/dc/ldap/impl/LDAPDataConnector.java b/shib-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/dc/ldap/impl/LDAPDataConnector.java
index 51aa0a1ee..5c98ecf99 100644
--- a/shib-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/dc/ldap/impl/LDAPDataConnector.java
+++ b/shib-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/dc/ldap/impl/LDAPDataConnector.java
@@ -93,7 +93,7 @@ public class LDAPDataConnector extends AbstractSearchDataConnector<ExecutableSea
* @return search operation for executing searches
*/
public SearchOperation getSearchOperation() {
- return searchOperation;
+ return SearchOperation.copy(searchOperation, true);
}
/**
diff --git a/shib-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/dc/ldap/impl/LDAPDataConnectorParser.java b/shib-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/dc/ldap/impl/LDAPDataConnectorParser.java
index 60e7f45aa..c73d3ae7a 100644
--- a/shib-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/dc/ldap/impl/LDAPDataConnectorParser.java
+++ b/shib-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/dc/ldap/impl/LDAPDataConnectorParser.java
@@ -19,6 +19,7 @@ import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
@@ -31,6 +32,7 @@ import org.ldaptive.ConnectionStrategy;
import org.ldaptive.Credential;
import org.ldaptive.DefaultConnectionFactory;
import org.ldaptive.FilterTemplate;
+import org.ldaptive.Freezable;
import org.ldaptive.PooledConnectionFactory;
import org.ldaptive.RandomConnectionStrategy;
import org.ldaptive.RoundRobinConnectionStrategy;
@@ -160,6 +162,7 @@ public class LDAPDataConnectorParser extends AbstractDataConnectorParser {
if (pooledConnectionFactory != null) {
builder.addPropertyValue("connectionFactory", pooledConnectionFactory);
} else {
+ connectionFactory.setInitMethodName("freeze");
builder.addPropertyValue("connectionFactory", connectionFactory.getBeanDefinition());
}
}
@@ -296,7 +299,7 @@ public class LDAPDataConnectorParser extends AbstractDataConnectorParser {
verifier.addConstructorArgValue(getLogPrefix());
sslConfig.addPropertyValue("hostnameVerifier", verifier.getBeanDefinition());
}
-
+
sslConfig.addPropertyValue("credentialConfig", createCredentialConfig(parserContext));
connectionConfig.addPropertyValue("sslConfig", sslConfig.getBeanDefinition());
final BeanDefinitionBuilder connectionInitializer =
@@ -846,23 +849,16 @@ public class LDAPDataConnectorParser extends AbstractDataConnectorParser {
searchRequest.setReturnAttributes("1.1");
searchRequest.setSearchScope(SearchScope.OBJECT);
searchRequest.setSizeLimit(1);
- if (validateDN != null) {
- searchRequest.setBaseDn(validateDN);
- } else {
- searchRequest.setBaseDn("");
- }
+ searchRequest.setBaseDn(Objects.requireNonNullElse(validateDN, ""));
final FilterTemplate searchFilter = new FilterTemplate();
- if (validateFilter != null) {
- searchFilter.setFilter(validateFilter);
- } else {
- searchFilter.setFilter("(objectClass=*)");
- }
+ searchFilter.setFilter(Objects.requireNonNullElse(validateFilter, "(objectClass=*)"));
searchRequest.setFilter(searchFilter);
final SearchConnectionValidator validator = new SearchConnectionValidator();
if (validatePeriod != null) {
validator.setValidatePeriod(Duration.parse(validatePeriod));
}
- validator.setSearchRequest(searchRequest);
+ validator.setRequest(searchRequest);
+ validator.freeze();
return validator;
}
@@ -894,11 +890,14 @@ public class LDAPDataConnectorParser extends AbstractDataConnectorParser {
@Nonnull public static List<LdapEntryHandler> buildSearchEntryHandlers(
@Nullable final String lowercaseAttributeNames) {
final List<LdapEntryHandler> handlers = new ArrayList<>();
- handlers.add(new DnAttributeEntryHandler());
+ final DnAttributeEntryHandler dnAttributeEntryHandler = new DnAttributeEntryHandler();
+ dnAttributeEntryHandler.freeze();
+ handlers.add(dnAttributeEntryHandler);
if (Boolean.valueOf(lowercaseAttributeNames)) {
- final CaseChangeEntryHandler entryHandler = new CaseChangeEntryHandler();
- entryHandler.setAttributeNameCaseChange(CaseChange.LOWER);
- handlers.add(entryHandler);
+ final CaseChangeEntryHandler caseChangeEntryHandler = new CaseChangeEntryHandler();
+ caseChangeEntryHandler.setAttributeNameCaseChange(CaseChange.LOWER);
+ caseChangeEntryHandler.freeze();
+ handlers.add(caseChangeEntryHandler);
}
return handlers;
}
@@ -913,8 +912,11 @@ public class LDAPDataConnectorParser extends AbstractDataConnectorParser {
@Nullable public static List<SearchResultHandler> buildReferralHandlers(
@Nullable final String followReferrals) {
if (followReferrals != null && Boolean.valueOf(followReferrals)) {
- return CollectionSupport.listOf(
- new FollowSearchReferralHandler(), new FollowSearchResultReferenceHandler());
+ final FollowSearchReferralHandler referralHandler = new FollowSearchReferralHandler();
+ referralHandler.freeze();
+ final FollowSearchResultReferenceHandler referenceHandler = new FollowSearchResultReferenceHandler();
+ referenceHandler.freeze();
+ return CollectionSupport.listOf(referralHandler, referenceHandler);
}
return null;
}
@@ -929,6 +931,7 @@ public class LDAPDataConnectorParser extends AbstractDataConnectorParser {
@Nonnull public static SaslConfig buildSaslConfig(@Nonnull final String mechanism) {
final SaslConfig config = new SaslConfig();
config.setMechanism(Mechanism.valueOf(mechanism));
+ config.freeze();
return config;
}
@@ -938,23 +941,22 @@ public class LDAPDataConnectorParser extends AbstractDataConnectorParser {
*/
@Nonnull public static ConnectionStrategy buildConnectionStrategy(@Nullable final String connectionStrategy) {
+ final ConnectionStrategy strategy;
if (connectionStrategy == null) {
- return new ActivePassiveConnectionStrategy();
- }
- switch (connectionStrategy) {
- case "ROUND_ROBIN":
- return new RoundRobinConnectionStrategy();
-
- case "RANDOM":
- return new RandomConnectionStrategy();
-
- case "ACTIVE_PASSIVE":
- return new ActivePassiveConnectionStrategy();
-
- default:
- LOG.warn("Unexpected connectionStrategy {}", connectionStrategy);
- return new ActivePassiveConnectionStrategy();
+ strategy = new ActivePassiveConnectionStrategy();
+ } else {
+ strategy = switch (connectionStrategy) {
+ case "ROUND_ROBIN" -> new RoundRobinConnectionStrategy();
+ case "RANDOM" -> new RandomConnectionStrategy();
+ case "ACTIVE_PASSIVE" -> new ActivePassiveConnectionStrategy();
+ default -> {
+ LOG.warn("Unexpected connectionStrategy {}", connectionStrategy);
+ yield new ActivePassiveConnectionStrategy();
+ }
+ };
}
+ ((Freezable) strategy).freeze();
+ return strategy;
}
}
}
diff --git a/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/dc/ldap/impl/LDAPDataConnectorParserTest.java b/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/dc/ldap/impl/LDAPDataConnectorParserTest.java
index 2c47949aa..fc231c399 100644
--- a/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/dc/ldap/impl/LDAPDataConnectorParserTest.java
+++ b/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/dc/ldap/impl/LDAPDataConnectorParserTest.java
@@ -19,6 +19,7 @@ import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
import java.io.IOException;
import java.time.Duration;
@@ -38,6 +39,7 @@ import org.ldaptive.RandomConnectionStrategy;
import org.ldaptive.SearchConnectionValidator;
import org.ldaptive.SearchOperation;
import org.ldaptive.filter.EqualityFilter;
+import org.ldaptive.handler.DnAttributeEntryHandler;
import org.ldaptive.pool.IdlePruneStrategy;
import org.ldaptive.referral.FollowSearchReferralHandler;
import org.ldaptive.referral.FollowSearchResultReferenceHandler;
@@ -124,9 +126,6 @@ public class LDAPDataConnectorParserTest {
10389,
new ClassPathResource("/net/shibboleth/idp/attribute/resolver/spring/dc/ldap/server.keystore"), or);
directoryServer.start();
- System.setProperty(
- "org.ldaptive.sasl.defaultSaslClient",
- TestSaslClient.class.getName());
}
/**
@@ -143,7 +142,6 @@ public class LDAPDataConnectorParserTest {
assert(directoryServer != null);
directoryServer.stop(true);
}
- System.clearProperty("org.ldaptive.sasl.defaultSaslClient");
}
@Test public void v2Config() throws Exception {
@@ -203,21 +201,25 @@ public class LDAPDataConnectorParserTest {
assertEquals(dataConnector.getNoRetryDelay(), Duration.ZERO);
final DefaultConnectionFactory connFactory = (DefaultConnectionFactory) dataConnector.getConnectionFactory();
assertNotNull(connFactory);
+ assertNotMutable(() -> connFactory.setConnectionConfig(null));
final ConnectionConfig connConfig = connFactory.getConnectionConfig();
assertNotNull(connConfig);
assertEquals(connConfig.getLdapUrl(), "ldap://localhost:10389");
assertFalse(connConfig.getUseStartTLS());
+ assertNotMutable(() -> connConfig.setSslConfig(null));
final BindConnectionInitializer connInitializer = (BindConnectionInitializer) connConfig.getConnectionInitializers()[0];
assertEquals(connInitializer.getBindDn(), "cn=Directory Manager");
assertEquals(connInitializer.getBindCredential().getString(), "password");
assertEquals(connConfig.getConnectTimeout(), Duration.ofSeconds(3));
assertEquals(connConfig.getResponseTimeout(), Duration.ofSeconds(3));
+ assertNotMutable(() -> connInitializer.setBindDn(""));
final SslConfig sslConfig = connFactory.getConnectionConfig().getSslConfig();
assertNotNull(sslConfig);
final CredentialConfig credentialConfig = sslConfig.getCredentialConfig();
assertNotNull(credentialConfig);
+ assertNotMutable(sslConfig::setTrustManagers);
final SearchOperation searchOperation = dataConnector.getSearchOperation();
assertNotNull(searchOperation);
@@ -257,6 +259,7 @@ public class LDAPDataConnectorParserTest {
assertEquals(Duration.ZERO, dataConnector.getNoRetryDelay());
final PooledConnectionFactory connFactory = (PooledConnectionFactory) dataConnector.getConnectionFactory();
assertNotNull(connFactory);
+ assertNotMutable(() -> connFactory.setConnectionConfig(null));
// note that default value changed from null to PT1M
assertEquals(connFactory.getBlockWaitTime(), Duration.ofMinutes(1));
assertEquals(connFactory.getName(), "resolver-pool-myLDAP");
@@ -266,21 +269,25 @@ public class LDAPDataConnectorParserTest {
// note that pooled connection factories have a validator by default
assertNotNull(connFactory.getValidator());
assertEquals(connFactory.getValidator().getValidatePeriod(), Duration.ofMinutes(30));
- // .... but that we align the failfast with our default
+ assertNotMutable(() -> ((SearchConnectionValidator) connFactory.getValidator()).setValidatePeriod(Duration.ofHours(1)));
+ // .... but that we align the failfast with our default
assertFalse(connFactory.getFailFastInitialize());
final IdlePruneStrategy pruneStrategy = (IdlePruneStrategy) connFactory.getPruneStrategy();
assertNotNull(pruneStrategy);
assertEquals(pruneStrategy.getPrunePeriod(), Duration.ofMinutes(5));
assertEquals(pruneStrategy.getIdleTime(), Duration.ofMinutes(10));
+ assertNotMutable(() -> pruneStrategy.setIdleTime(Duration.ofHours(1)));
final ConnectionConfig connConfig = connFactory.getConnectionConfig();
assertNotNull(connConfig);
assertEquals(connConfig.getLdapUrl(), "ldap://localhost:10389");
assertFalse(connConfig.getUseStartTLS());
+ assertNotMutable(() -> connConfig.setSslConfig(null));
final BindConnectionInitializer connInitializer = (BindConnectionInitializer) connConfig.getConnectionInitializers()[0];
assertEquals(connInitializer.getBindDn(), "cn=Directory Manager");
assertEquals(connInitializer.getBindCredential().getString(), "password");
+ assertNotMutable(() -> connInitializer.setBindDn(""));
assertEquals(connConfig.getConnectTimeout(), Duration.ofSeconds(3));
assertEquals(connConfig.getResponseTimeout(), Duration.ofSeconds(3));
@@ -288,6 +295,7 @@ public class LDAPDataConnectorParserTest {
assertNotNull(sslConfig);
final CredentialConfig credentialConfig = sslConfig.getCredentialConfig();
assertNotNull(credentialConfig);
+ assertNotMutable(() -> sslConfig.setCredentialConfig(null));
final SearchOperation searchOperation = dataConnector.getSearchOperation();
assertNotNull(searchOperation);
@@ -340,6 +348,7 @@ public class LDAPDataConnectorParserTest {
assertEquals(dataConnector.getConnectionFactory().getConnectionConfig().getConnectionStrategy().getClass(),
RandomConnectionStrategy.class);
+ assertNotMutable(() -> dataConnector.getConnectionFactory().getConnectionConfig().setConnectionStrategy(null));
}
@Test public void v2SaslConfig() throws Exception {
@@ -366,6 +375,7 @@ public class LDAPDataConnectorParserTest {
assertEquals(saslConfig.getSecurityStrength()[0], SecurityStrength.HIGH);
assertEquals(saslConfig.getRealm(), "shibboleth.net");
assertEquals(saslConfig.getProperties(), Map.of("org.ldaptive.sasl.gssapi.jaas.refreshConfig", "true"));
+ assertNotMutable(() -> saslConfig.setRealm("new_realm"));
}
@Test public void v2ReferralConfig() throws Exception {
@@ -393,8 +403,10 @@ public class LDAPDataConnectorParserTest {
assertEquals(searchOperation.getSearchResultHandlers().length, 2);
final FollowSearchReferralHandler referralHandler = (FollowSearchReferralHandler) searchOperation.getSearchResultHandlers()[0];
assertNotNull(referralHandler);
+ assertNotMutable(() -> referralHandler.setRequest(null));
final FollowSearchResultReferenceHandler referenceHandler = (FollowSearchResultReferenceHandler) searchOperation.getSearchResultHandlers()[1];
assertNotNull(referenceHandler);
+ assertNotMutable(() -> referenceHandler.setRequest(null));
}
@Test public void v2SaslExternalConfig() throws Exception {
@@ -415,6 +427,7 @@ public class LDAPDataConnectorParserTest {
final SaslConfig saslConfig = connInitializer.getBindSaslConfig();
assertNotNull(saslConfig);
assertEquals(saslConfig.getMechanism(), Mechanism.EXTERNAL);
+ assertNotMutable(() -> saslConfig.setRealm("new_realm"));
}
@Test public void refConfig() throws Exception {
@@ -543,6 +556,7 @@ public class LDAPDataConnectorParserTest {
assertNotNull(pruneStrategy);
assertEquals(pruneStrategy.getPrunePeriod(), Duration.ofMinutes(5));
assertEquals(pruneStrategy.getIdleTime(), Duration.ofMinutes(10));
+ assertNotMutable(() -> pruneStrategy.setIdleTime(Duration.ofDays(1)));
final ConnectionConfig connConfig = connFactory.getConnectionConfig();
assertNotNull(connConfig);
@@ -566,6 +580,10 @@ public class LDAPDataConnectorParserTest {
assertEquals(searchOperation.getRequest().getTimeLimit(), Duration.ofSeconds(7));
assertNull(searchOperation.getSearchResultHandlers());
+ if (searchOperation.getEntryHandlers() != null) {
+ assertNotMutable(() -> ((DnAttributeEntryHandler) searchOperation.getEntryHandlers()[0]).setAddIfExists(true));
+ }
+
final ConnectionFactoryValidator validator = (ConnectionFactoryValidator) dataConnector.getValidator();
assertNotNull(validator);
assertFalse(validator.isThrowValidateError());
@@ -581,4 +599,13 @@ public class LDAPDataConnectorParserTest {
final Cache<String, Map<String, IdPAttribute>> resultCache = dataConnector.getResultsCache();
assertNotNull(resultCache);
}
+
+ private void assertNotMutable(final Runnable runnable) {
+ try {
+ runnable.run();
+ fail("Should have thrown exception");
+ } catch (Exception e) {
+ assertEquals(e.getClass(), IllegalStateException.class);
+ }
+ }
}
diff --git a/shib-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/dc/ldap/ldap-attribute-resolver-spring-context.xml b/shib-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/dc/ldap/ldap-attribute-resolver-spring-context.xml
index ad2b39c53..55b8014c4 100644
--- a/shib-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/dc/ldap/ldap-attribute-resolver-spring-context.xml
+++ b/shib-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/dc/ldap/ldap-attribute-resolver-spring-context.xml
@@ -31,7 +31,7 @@
</property>
<property name="validator">
<bean class="org.ldaptive.SearchConnectionValidator" p:validatePeriod="PT15M">
- <property name="searchRequest">
+ <property name="request">
<bean class="org.ldaptive.SearchRequest">
<constructor-arg value="dc=shibboleth,dc=net" />
<constructor-arg value="(ou=people)" />
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list