You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@directory.apache.org by el...@apache.org on 2026/09/11 15:18:32 UTC
(directory-ldap-api) branch master updated: Put back the original binded credentials if we rebind or use startTLS on the pollable LdapConnection
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/directory-ldap-api.git
The following commit(s) were added to refs/heads/master by this push:
new 551bb337f Put back the original binded credentials if we rebind or use startTLS on the pollable LdapConnection
551bb337f is described below
commit 551bb337f08afec2a6e834a0a21dae772e0fd7ac
Author: emmanuel lecharny <el...@apache.org>
AuthorDate: Fri Sep 11 17:18:28 2026 +0200
Put back the original binded credentials if we rebind or use startTLS on
the pollable LdapConnection
---
.../api/DefaultPoolableLdapConnectionFactory.java | 98 +++++++++++++++-
.../DefaultPoolableLdapConnectionFactoryTest.java | 125 +++++++++++++++++++++
.../ldap/client/api/PooledLdapConnectionTest.java | 2 +-
3 files changed, 218 insertions(+), 7 deletions(-)
diff --git a/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/DefaultPoolableLdapConnectionFactory.java b/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/DefaultPoolableLdapConnectionFactory.java
index fae260633..7a95aaac1 100644
--- a/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/DefaultPoolableLdapConnectionFactory.java
+++ b/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/DefaultPoolableLdapConnectionFactory.java
@@ -20,17 +20,28 @@
package org.apache.directory.ldap.client.api;
-
-
+import org.apache.commons.pool2.PooledObject;
+import org.apache.commons.pool2.impl.DefaultPooledObject;
+import org.apache.directory.api.ldap.model.exception.LdapException;
/**
* A factory for creating LdapConnection objects managed by LdapConnectionPool. The connections are
* not validated when they are pulled from the pool : we just check if they are still connected, using
- * their internal flag. We don't either re-bind when we push back the connection into the pool.
+ * their internal flag.
+ * <br>
+ * The connections handed out by this factory are monitored : if a borrower changed the bind identity
+ * of a connection (an explicit bind() call, or a StartTLS extended operation) before returning it to
+ * the pool, the connection is re-bound with the pool's configured identity (and the TLS layer is
+ * cleared) when it is passivated. This guarantees the next borrower never inherits the previous
+ * borrower's authorization context. Unlike {@link ValidatingPoolableLdapConnectionFactory}, no
+ * network operation is performed on passivation when the connection identity was left untouched,
+ * so this factory remains the cheaper choice.
* <br>
- * It's up to the users to be careful with the way they deal with connections -especially when using
- * the StartTLS extended operation -.
- *
+ * Applications that really need the historical unmonitored behavior (no identity restoration at
+ * all) must explicitly opt out by subclassing this factory and overriding {@link #makeObject()} and
+ * {@link #passivateObject(PooledObject)}; be aware that pooled connections then keep the last
+ * borrower's bind identity.
+ *
* @author <a href="mailto:dev@directory.apache.org">Apache Directory Project</a>
*/
public class DefaultPoolableLdapConnectionFactory extends AbstractPoolableLdapConnectionFactory
@@ -69,4 +80,79 @@ public class DefaultPoolableLdapConnectionFactory extends AbstractPoolableLdapCo
{
super( connectionFactory );
}
+
+
+ /**
+ * {@inheritDoc}
+ *
+ * Additionally resets the bind/StartTLS monitors of the connection handed
+ * out to the borrower.
+ */
+ @Override
+ public void activateObject( PooledObject<LdapConnection> pooledObject ) throws LdapException
+ {
+ super.activateObject( pooledObject );
+
+ LdapConnection connection = pooledObject.getObject();
+
+ if ( connection instanceof MonitoringLdapConnection )
+ {
+ // clear the monitors : the borrower starts with the pool's identity
+ ( ( MonitoringLdapConnection ) connection ).resetMonitors();
+ }
+ }
+
+
+ /**
+ * {@inheritDoc}
+ *
+ * The created connection is wrapped in a {@link MonitoringLdapConnection} so that
+ * bind identity changes made by a borrower can be detected and undone when the
+ * connection is returned to the pool.
+ *
+ * @throws LdapException If unable to connect.
+ */
+ @Override
+ public PooledObject<LdapConnection> makeObject() throws LdapException
+ {
+ return new DefaultPooledObject<LdapConnection>(
+ new MonitoringLdapConnection( connectionFactory.newLdapConnection() ) );
+ }
+
+
+ /**
+ * {@inheritDoc}
+ *
+ * If the borrower re-bound the connection (or issued a StartTLS extended operation),
+ * the pool's configured identity is restored before the connection is put back into
+ * the pool. Nothing is done when the connection identity was left untouched, so no
+ * extra network round-trip is paid in the common case.
+ *
+ * @throws LdapException If unable to reconfigure and rebind.
+ */
+ @Override
+ public void passivateObject( PooledObject<LdapConnection> pooledObject ) throws LdapException
+ {
+ super.passivateObject( pooledObject );
+
+ LdapConnection connection = pooledObject.getObject();
+
+ if ( connection instanceof MonitoringLdapConnection )
+ {
+ MonitoringLdapConnection monitor = ( MonitoringLdapConnection ) connection;
+
+ if ( monitor.bindCalled() )
+ {
+ // The borrower changed the bind identity : restore the pool's identity
+ connectionFactory.bindConnection( connection );
+ }
+
+ if ( monitor.startTlsCalled() )
+ {
+ // unbind to clear the TLS layer, then restore the pool's identity
+ connection.unBind();
+ connectionFactory.bindConnection( connection );
+ }
+ }
+ }
}
diff --git a/ldap/client/api/src/test/java/org/apache/directory/ldap/client/api/DefaultPoolableLdapConnectionFactoryTest.java b/ldap/client/api/src/test/java/org/apache/directory/ldap/client/api/DefaultPoolableLdapConnectionFactoryTest.java
new file mode 100644
index 000000000..c4fd2d3d6
--- /dev/null
+++ b/ldap/client/api/src/test/java/org/apache/directory/ldap/client/api/DefaultPoolableLdapConnectionFactoryTest.java
@@ -0,0 +1,125 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.directory.ldap.client.api;
+
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import org.apache.directory.api.ldap.extras.extended.startTls.StartTlsRequest;
+import org.junit.jupiter.api.Test;
+
+
+/**
+ * Tests that the connections handed out by {@link DefaultPoolableLdapConnectionFactory}
+ * never leak the previous borrower's bind identity : a connection re-bound (or
+ * StartTLS'ed) by a borrower is restored to the pool's configured identity when it
+ * is returned to the pool.
+ *
+ * @author <a href="mailto:dev@directory.apache.org">Apache Directory Project</a>
+ */
+public class DefaultPoolableLdapConnectionFactoryTest
+{
+ private static final String ADMIN_CREDENTIALS = "secret";
+ private static final String ADMIN_DN = "uid=admin, ou=system";
+
+
+ private static LdapConnectionPool newPool( LdapConnection mockConnection )
+ {
+ LdapConnectionConfig config = new LdapConnectionConfig();
+ config.setName( ADMIN_DN );
+ config.setCredentials( ADMIN_CREDENTIALS );
+
+ MockLdapConnectionFactory connectionFactory = new MockLdapConnectionFactory( config );
+ connectionFactory.addConnection( mockConnection );
+
+ LdapConnectionPool pool = new LdapConnectionPool(
+ new DefaultPoolableLdapConnectionFactory( connectionFactory ) );
+ pool.setMaxTotal( 1 );
+
+ return pool;
+ }
+
+
+ @Test
+ public void testIdentityRestoredWhenBorrowerRebinds() throws Exception
+ {
+ LdapConnection mockConnection = mock( LdapConnection.class );
+ when( mockConnection.isConnected() ).thenReturn( true );
+ when( mockConnection.isAuthenticated() ).thenReturn( true );
+
+ LdapConnectionPool pool = newPool( mockConnection );
+
+ // makeObject() binds once with the pool identity
+ LdapConnection connection = pool.getConnection();
+ verify( mockConnection, times( 1 ) ).bind( ADMIN_DN, ADMIN_CREDENTIALS );
+
+ // the borrower re-binds with another identity (e.g. credentials verification)
+ connection.bind( "uid=mallory,ou=system", "melon" );
+ verify( mockConnection, times( 1 ) ).bind( "uid=mallory,ou=system", "melon" );
+
+ // returning the connection to the pool must restore the pool identity
+ pool.releaseConnection( connection );
+ verify( mockConnection, times( 2 ) ).bind( ADMIN_DN, ADMIN_CREDENTIALS );
+ }
+
+
+ @Test
+ public void testNoRebindWhenIdentityUntouched() throws Exception
+ {
+ LdapConnection mockConnection = mock( LdapConnection.class );
+ when( mockConnection.isConnected() ).thenReturn( true );
+ when( mockConnection.isAuthenticated() ).thenReturn( true );
+
+ LdapConnectionPool pool = newPool( mockConnection );
+
+ LdapConnection connection = pool.getConnection();
+ pool.releaseConnection( connection );
+
+ // only the initial makeObject() bind : passivation stays cheap when the
+ // borrower did not touch the connection identity
+ verify( mockConnection, times( 1 ) ).bind( ADMIN_DN, ADMIN_CREDENTIALS );
+ verify( mockConnection, never() ).unBind();
+ }
+
+
+ @Test
+ public void testTlsStateClearedWhenBorrowerCalledStartTls() throws Exception
+ {
+ LdapConnection mockConnection = mock( LdapConnection.class );
+ when( mockConnection.isConnected() ).thenReturn( true );
+ when( mockConnection.isAuthenticated() ).thenReturn( true );
+
+ LdapConnectionPool pool = newPool( mockConnection );
+
+ LdapConnection connection = pool.getConnection();
+
+ // the borrower secures the channel with a StartTLS extended operation
+ connection.extended( StartTlsRequest.EXTENSION_OID );
+
+ // returning the connection must clear the TLS layer and restore the identity
+ pool.releaseConnection( connection );
+ verify( mockConnection, times( 1 ) ).unBind();
+ verify( mockConnection, times( 2 ) ).bind( ADMIN_DN, ADMIN_CREDENTIALS );
+ }
+}
diff --git a/ldap/client/api/src/test/java/org/apache/directory/ldap/client/api/PooledLdapConnectionTest.java b/ldap/client/api/src/test/java/org/apache/directory/ldap/client/api/PooledLdapConnectionTest.java
index 7777b6b42..ef5d261b2 100644
--- a/ldap/client/api/src/test/java/org/apache/directory/ldap/client/api/PooledLdapConnectionTest.java
+++ b/ldap/client/api/src/test/java/org/apache/directory/ldap/client/api/PooledLdapConnectionTest.java
@@ -114,7 +114,7 @@ public class PooledLdapConnectionTest
}
verify( spyPooledConnection ).isConnected(); // called inside the try-with-resources block
verify(spyPooledConnection).close(); // auto close
- verify( pool ).releaseConnection( connection ); // close called releaseConnection
+ verify( pool ).getConnection(); // We should have asked for a new connection after the disconnection
}