From c8cbf831d0d3fe5ab522a7426081374ca1a148bb Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Tue, 27 Sep 2011 16:35:18 +0000
Subject: [PATCH] Fix OPENDJ-290: LDAP PTA valid auth attempt rejected if AD reset connection
---
opends/tests/unit-tests-testng/src/server/org/opends/server/extensions/LDAPPassThroughAuthenticationPolicyTestCase.java | 123 +++++++++++++++++++++++++++---
opends/src/server/org/opends/server/extensions/LDAPPassThroughAuthenticationPolicyFactory.java | 100 +++++++++++++++++++-----
2 files changed, 188 insertions(+), 35 deletions(-)
diff --git a/opends/src/server/org/opends/server/extensions/LDAPPassThroughAuthenticationPolicyFactory.java b/opends/src/server/org/opends/server/extensions/LDAPPassThroughAuthenticationPolicyFactory.java
index dc8d1e5..8dece5f 100644
--- a/opends/src/server/org/opends/server/extensions/LDAPPassThroughAuthenticationPolicyFactory.java
+++ b/opends/src/server/org/opends/server/extensions/LDAPPassThroughAuthenticationPolicyFactory.java
@@ -74,8 +74,7 @@
// TODO: handle password policy response controls? AD?
// TODO: custom aliveness pings
// TODO: cache password
- // TODO: handle idle timeouts (connection resets): implement retry logic in
- // connection pools.
+ // TODO: improve debug logging and error messages.
/**
* A simplistic load-balancer connection factory implementation using
@@ -591,7 +590,7 @@
*/
private final class PooledConnection implements Connection
{
- private final Connection connection;
+ private Connection connection;
private boolean connectionIsClosed = false;
@@ -622,6 +621,8 @@
{
connectionPool.offer(connection);
}
+
+ connection = null;
availableConnections.release();
}
}
@@ -639,11 +640,25 @@
{
return connection.search(baseDN, scope, filter);
}
- catch (final DirectoryException e)
+ catch (final DirectoryException e1)
{
- // Don't put the connection back in the pool if it has failed.
- closeConnectionIfServiceError(e);
- throw e;
+ // Fail immediately if the result indicates that the operation failed
+ // for a reason other than connection/server failure.
+ reconnectIfConnectionFailure(e1);
+
+ // The connection has failed, so retry the operation using the new
+ // connection.
+ try
+ {
+ return connection.search(baseDN, scope, filter);
+ }
+ catch (final DirectoryException e2)
+ {
+ // If the connection has failed again then give up: don't put the
+ // connection back in the pool.
+ closeIfConnectionFailure(e2);
+ throw e2;
+ }
}
}
@@ -660,29 +675,68 @@
{
connection.simpleBind(username, password);
}
- catch (final DirectoryException e)
+ catch (final DirectoryException e1)
{
- // Don't put the connection back in the pool if it has failed.
- closeConnectionIfServiceError(e);
- throw e;
- }
- }
+ // Fail immediately if the result indicates that the operation failed
+ // for a reason other than connection/server failure.
+ reconnectIfConnectionFailure(e1);
-
-
- private void closeConnectionIfServiceError(final DirectoryException e)
- {
- if (isServiceError(e.getResultCode()))
- {
- if (!connectionIsClosed)
+ // The connection has failed, so retry the operation using the new
+ // connection.
+ try
{
- connectionIsClosed = true;
- connection.close();
- availableConnections.release();
+ connection.simpleBind(username, password);
+ }
+ catch (final DirectoryException e2)
+ {
+ // If the connection has failed again then give up: don't put the
+ // connection back in the pool.
+ closeIfConnectionFailure(e2);
+ throw e2;
}
}
}
+
+
+ private void closeIfConnectionFailure(final DirectoryException e)
+ throws DirectoryException
+ {
+ if (isServiceError(e.getResultCode()))
+ {
+ connectionIsClosed = true;
+ connection.close();
+ connection = null;
+ availableConnections.release();
+ }
+ }
+
+
+
+ private void reconnectIfConnectionFailure(final DirectoryException e)
+ throws DirectoryException
+ {
+ if (!isServiceError(e.getResultCode()))
+ {
+ throw e;
+ }
+
+ // The connection has failed (e.g. idle timeout), so repeat the
+ // request on a new connection.
+ connection.close();
+ try
+ {
+ connection = factory.getConnection();
+ }
+ catch (final DirectoryException e2)
+ {
+ // Give up - the server is unreachable.
+ connectionIsClosed = true;
+ connection = null;
+ availableConnections.release();
+ throw e2;
+ }
+ }
}
diff --git a/opends/tests/unit-tests-testng/src/server/org/opends/server/extensions/LDAPPassThroughAuthenticationPolicyTestCase.java b/opends/tests/unit-tests-testng/src/server/org/opends/server/extensions/LDAPPassThroughAuthenticationPolicyTestCase.java
index 871a605..a9019c6 100644
--- a/opends/tests/unit-tests-testng/src/server/org/opends/server/extensions/LDAPPassThroughAuthenticationPolicyTestCase.java
+++ b/opends/tests/unit-tests-testng/src/server/org/opends/server/extensions/LDAPPassThroughAuthenticationPolicyTestCase.java
@@ -49,6 +49,7 @@
import org.opends.server.core.DirectoryServer;
import org.opends.server.extensions.LDAPPassThroughAuthenticationPolicyFactory.Connection;
import org.opends.server.extensions.LDAPPassThroughAuthenticationPolicyFactory.ConnectionFactory;
+import org.opends.server.extensions.LDAPPassThroughAuthenticationPolicyFactory.ConnectionPool;
import org.opends.server.extensions.LDAPPassThroughAuthenticationPolicyFactory.LDAPConnectionFactory;
import org.opends.server.protocols.asn1.ASN1;
import org.opends.server.protocols.asn1.ASN1Writer;
@@ -1612,8 +1613,10 @@
"(uid=aduser)", searchResultCode));
if (isServiceError(searchResultCode))
{
- // The connection will fail and be closed immediately.
+ // The connection will fail and be closed immediately, and the pool will
+ // retry on new connection.
provider.expectEvent(new CloseEvent(ceSearch));
+ provider.expectEvent(new GetConnectionEvent(fe, searchResultCode));
}
// Obtain policy and state.
@@ -2006,6 +2009,7 @@
new SearchEvent(ceSearch1, "o=ad", SearchScope.WHOLE_SUBTREE,
"(uid=aduser)", ResultCode.UNAVAILABLE))
.expectEvent(new CloseEvent(ceSearch1))
+ .expectEvent(new GetConnectionEvent(fe1, ResultCode.UNAVAILABLE)) // pool retry
.expectEvent(ceSearch2)
.expectEvent(
new SimpleBindEvent(ceSearch2, searchBindDNString,
@@ -2021,10 +2025,12 @@
// Now simulate shost1 going down as well.
// phost1 will have been marked as failed and won't be retried.
- provider.expectEvent(
- new SearchEvent(ceSearch2, "o=ad", SearchScope.WHOLE_SUBTREE,
- "(uid=aduser)", ResultCode.UNAVAILABLE)).expectEvent(
- new CloseEvent(ceSearch2));
+ provider
+ .expectEvent(
+ new SearchEvent(ceSearch2, "o=ad", SearchScope.WHOLE_SUBTREE,
+ "(uid=aduser)", ResultCode.UNAVAILABLE))
+ .expectEvent(new CloseEvent(ceSearch2))
+ .expectEvent(new GetConnectionEvent(fe2, ResultCode.UNAVAILABLE)); // pool retry
// Now simulate phost1 coming back and fail back to it.
@@ -2209,6 +2215,7 @@
new SearchEvent(ceSearch1, "o=ad", SearchScope.WHOLE_SUBTREE,
"(uid=aduser)", ResultCode.UNAVAILABLE))
.expectEvent(new CloseEvent(ceSearch1))
+ .expectEvent(new GetConnectionEvent(fe1, ResultCode.UNAVAILABLE)) // pool retry
.expectEvent(ceSearch2)
.expectEvent(
new SimpleBindEvent(ceSearch2, searchBindDNString,
@@ -2227,7 +2234,8 @@
provider.expectEvent(
new SearchEvent(ceSearch2, "o=ad", SearchScope.WHOLE_SUBTREE,
"(uid=aduser)", ResultCode.UNAVAILABLE)).expectEvent(
- new CloseEvent(ceSearch2));
+ new CloseEvent(ceSearch2))
+ .expectEvent(new GetConnectionEvent(fe2, ResultCode.UNAVAILABLE)); // pool retry
// Now simulate phost1 coming back and fail back to it.
@@ -3325,8 +3333,10 @@
: adDNString, userPassword, bindResultCode));
if (isServiceError(bindResultCode))
{
- // The connection will fail and be closed immediately.
+ // The connection will fail and be closed immediately, and the pool will
+ // retry on new connection.
provider.expectEvent(new CloseEvent(ceBind));
+ provider.expectEvent(new GetConnectionEvent(fe, bindResultCode));
}
// Connection should be cached until the policy is finalized or until the
@@ -3851,6 +3861,7 @@
new SearchEvent(ceSearch1, "o=ad", SearchScope.WHOLE_SUBTREE,
"(uid=aduser)", ResultCode.UNAVAILABLE))
.expectEvent(new CloseEvent(ceSearch1))
+ .expectEvent(new GetConnectionEvent(fe1, ResultCode.UNAVAILABLE)) // pool retry
.expectEvent(ceSearch2)
.expectEvent(
new SimpleBindEvent(ceSearch2, searchBindDNString, "searchPassword"))
@@ -3862,10 +3873,12 @@
ResultCode.SUCCESS));
// Repeat, but fail on shost1 as well, leaving no available servers.
- provider.expectEvent(
- new SearchEvent(ceSearch2, "o=ad", SearchScope.WHOLE_SUBTREE,
- "(uid=aduser)", ResultCode.UNAVAILABLE)).expectEvent(
- new CloseEvent(ceSearch2));
+ provider
+ .expectEvent(
+ new SearchEvent(ceSearch2, "o=ad", SearchScope.WHOLE_SUBTREE,
+ "(uid=aduser)", ResultCode.UNAVAILABLE))
+ .expectEvent(new CloseEvent(ceSearch2))
+ .expectEvent(new GetConnectionEvent(fe2, ResultCode.UNAVAILABLE)); // pool retry
// Obtain policy and state.
final LDAPPassThroughAuthenticationPolicyFactory factory = new LDAPPassThroughAuthenticationPolicyFactory(
@@ -4049,7 +4062,93 @@
- // TODO: detect when servers come back online
+ /**
+ * Test for issue OPENDJ-290
+ * (https://bugster.forgerock.org/jira/browse/OPENDJ-290).
+ *
+ * @throws Exception
+ * If an unexpected exception occurred.
+ */
+ @Test(enabled = true)
+ public void testIssueOPENDJ290() throws Exception
+ {
+ // Mock configuration.
+ final LDAPPassThroughAuthenticationPolicyCfg cfg = mockCfg()
+ .withPrimaryServer(phost1)
+ .withMappingPolicy(MappingPolicy.MAPPED_SEARCH)
+ .withMappedAttribute("uid").withBaseDN("o=ad");
+
+ // Create all the events.
+ final MockProvider provider = new MockProvider();
+
+ // First of all the connection factories are created.
+ final GetLDAPConnectionFactoryEvent fe1 = new GetLDAPConnectionFactoryEvent(
+ phost1, cfg);
+ provider.expectEvent(fe1);
+
+ // Get connection then bind twice creating two pooled connections.
+ final GetConnectionEvent ce1 = new GetConnectionEvent(fe1);
+ final GetConnectionEvent ce2 = new GetConnectionEvent(fe1);
+ provider
+ .expectEvent(ce1)
+ .expectEvent(
+ new SimpleBindEvent(ce1, searchBindDNString, "searchPassword"))
+ .expectEvent(ce2)
+ .expectEvent(
+ new SimpleBindEvent(ce2, searchBindDNString, "searchPassword"));
+
+ // Once two pooled connections are created, retry but this time simulate the
+ // first connection failing. The attempt should automatically retry on a new
+ // connection.
+ final GetConnectionEvent ce3 = new GetConnectionEvent(fe1);
+ provider
+ .expectEvent(
+ new SimpleBindEvent(ce1, searchBindDNString, "searchPassword",
+ ResultCode.UNAVAILABLE))
+ .expectEvent(new CloseEvent(ce1))
+ .expectEvent(ce3)
+ .expectEvent(
+ new SimpleBindEvent(ce3, searchBindDNString, "searchPassword"));
+
+ // Use a connection pool directly for this test.
+ ConnectionPool pool = new ConnectionPool(provider.getLDAPConnectionFactory(
+ "phost1", 11, cfg));
+
+ // Authenticate three times, the third time was failing because the pool
+ // would not retry the operation on a new connection.
+ ByteString username = ByteString.valueOf(searchBindDNString);
+ ByteString password = ByteString.valueOf("searchPassword");
+
+ Connection c1 = pool.getConnection();
+ c1.simpleBind(username, password);
+
+ // Don't release c1 because we want to force creation of a second connection.
+
+ Connection c2 = pool.getConnection();
+ c2.simpleBind(username, password);
+
+ // Release both the connections.
+ c1.close();
+ c2.close();
+
+ // This was failing because the pool would not retry with a new connection.
+ Connection c3 = pool.getConnection();
+ c3.simpleBind(username, password);
+ c3.close();
+
+ // There should be no more pending events.
+ provider.assertAllExpectedEventsReceived();
+
+ // Cached connections should be closed when the pool is closed.
+ provider.expectEvent(new CloseEvent(ce2));
+ provider.expectEvent(new CloseEvent(ce3));
+
+ // Tear down and check final state.
+ pool.close();
+ provider.assertAllExpectedEventsReceived();
+ }
+
+
MockPolicyCfg mockCfg()
{
--
Gitblit v1.10.0