diff --git a/.github/workflows/tests/test_knox_ldap_proxy_search.py b/.github/workflows/tests/test_knox_ldap_proxy_search.py index 064d42d3b..2baf65081 100644 --- a/.github/workflows/tests/test_knox_ldap_proxy_search.py +++ b/.github/workflows/tests/test_knox_ldap_proxy_search.py @@ -68,7 +68,7 @@ def setUp(self) -> None: knox_host(), port=KNOX_LDAP_PORT, use_ssl=True, tls=tls, get_info=ldap3.NONE ) self.connection = ldap3.Connection( - server, user=BIND_DN, password=BIND_PASSWORD, auto_bind=True + server, user=BIND_DN, password=BIND_PASSWORD, auto_bind=True, raise_exceptions=True ) def tearDown(self) -> None: diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/LdapMessages.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/LdapMessages.java index 8683f4645..c27056800 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/LdapMessages.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/LdapMessages.java @@ -114,8 +114,8 @@ public interface LdapMessages { void ldapPagedSearchExceededMaxResultSetSize(int resultSetSize, int maxResultSetSize); @Message(level = MessageLevel.DEBUG, - text = "LDAP Paged Search Completed: {0} | {1}") - void ldapPagedSearchCompleted(String baseDn, String filter); + text = "LDAP Paged Search Completed: {0} | {1}. Found {2} entries") + void ldapPagedSearchCompleted(String baseDn, String filter, int numResults); @Message(level = MessageLevel.ERROR, text = "LDAP Search failed: {0} | {1}, {2}") diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackend.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackend.java index 32fd033b2..e00c1a0a6 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackend.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackend.java @@ -27,7 +27,10 @@ import org.apache.directory.api.ldap.model.entry.Entry; import org.apache.directory.api.ldap.model.entry.Value; import org.apache.directory.api.ldap.model.exception.LdapException; +import org.apache.directory.api.ldap.model.exception.LdapOperationException; +import org.apache.directory.api.ldap.model.message.LdapResult; import org.apache.directory.api.ldap.model.message.Response; +import org.apache.directory.api.ldap.model.message.ResultCodeEnum; import org.apache.directory.api.ldap.model.message.SearchRequest; import org.apache.directory.api.ldap.model.message.SearchRequestImpl; import org.apache.directory.api.ldap.model.message.SearchResultDone; @@ -483,14 +486,16 @@ public Entry getUser(String username, SchemaManager schemaManager) throws Except connection = getConnection(); // Search for user using configurable attribute String filter = userSearchFilter.replace("{username}", username); + Entry sourceEntry = null; try (EntryCursor cursor = connection.search(remoteUserSearchBase, filter, SearchScope.SUBTREE, "*")) { if (cursor.next()) { - Entry sourceEntry = cursor.get(); - addGroupMemberships(sourceEntry, connection, createEntryCache(), createResolvedParentsCache()); - return remoteSchemaConverter.convertRemoteEntryToProxyEntry(sourceEntry, schemaManager); - + sourceEntry = cursor.get(); } } + if (sourceEntry != null) { + addGroupMemberships(sourceEntry, connection, createEntryCache(), createResolvedParentsCache()); + return remoteSchemaConverter.convertRemoteEntryToProxyEntry(sourceEntry, schemaManager); + } return null; } finally { releaseConnection(connection); @@ -557,7 +562,7 @@ public List search(String searchBase, SearchScope searchScope, String fil } } - private void addGroupMemberships(Entry entry, LdapConnection connection, Map entryCache, Map> resolvedParentsCache) throws Exception { + private void addGroupMemberships(Entry entry, LdapConnection connection, Map entryCache, Map> resolvedParentsCache) throws LdapException, CursorException, IOException { // The memberOf attribute is already populated on the entry. Further work is only needed // when using recursive group resolution or non using memberOf to find groups if (recursiveGroupResolution || !useMemberOf) { @@ -568,7 +573,7 @@ private void addGroupMemberships(Entry entry, LdapConnection connection, Map getUserGroupsEntries(LdapConnection connection, Entry user, Map entryCache, Map> resolvedParentsCache) throws Exception { + private List getUserGroupsEntries(LdapConnection connection, Entry user, Map entryCache, Map> resolvedParentsCache) throws LdapException, CursorException, IOException { List groups = new ArrayList<>(); if (useMemberOf) { // Use memberOf attribute for efficient AD lookups @@ -887,6 +892,11 @@ protected List performPagedSearch(LdapConnection connection, String baseD } if (cursor.isDone()) { SearchResultDone done = cursor.getSearchResultDone(); + + LdapResult ldapResult = done.getLdapResult(); + if (ldapResult.getResultCode() != ResultCodeEnum.SUCCESS) { + throw new LdapOperationException(ldapResult.getResultCode(), ldapResult.getDiagnosticMessage()); + } PagedResults responseControl = (PagedResults) done.getControl(PagedResults.OID); if (responseControl != null) { @@ -903,7 +913,7 @@ protected List performPagedSearch(LdapConnection connection, String baseD if (maxResultSetSize != 0 && results.size() >= maxResultSetSize) { LOG.ldapPagedSearchExceededMaxResultSetSize(results.size(), maxResultSetSize); } else { - LOG.ldapPagedSearchCompleted(baseDn, filter); + LOG.ldapPagedSearchCompleted(baseDn, filter, results.size()); } return results; diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackendTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackendTest.java index 7264527db..bcfc899e1 100644 --- a/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackendTest.java +++ b/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackendTest.java @@ -17,18 +17,29 @@ */ package org.apache.knox.gateway.services.ldap.backend; +import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; +import org.apache.directory.api.ldap.model.cursor.SearchCursor; import org.apache.directory.api.ldap.model.entry.Entry; import org.apache.directory.api.ldap.model.entry.Value; +import org.apache.directory.api.ldap.model.exception.LdapOperationException; +import org.apache.directory.api.ldap.model.message.LdapResult; +import org.apache.directory.api.ldap.model.message.ResultCodeEnum; import org.apache.directory.api.ldap.model.message.SearchRequest; +import org.apache.directory.api.ldap.model.message.SearchResultDone; import org.apache.directory.api.ldap.model.message.SearchScope; import org.apache.directory.api.ldap.model.name.Dn; import org.apache.directory.api.ldap.model.schema.SchemaManager; +import org.apache.directory.ldap.client.api.LdapConnection; import org.apache.directory.server.core.api.CoreSession; import org.apache.directory.server.core.api.DirectoryService; import org.apache.directory.server.core.api.InstanceLayout; @@ -795,6 +806,36 @@ public void testIsSupportedSearchBase() { assertFalse(ldapProxyBackend.isSupportedSearchBase("dc=other,dc=base,dc=org")); } + @Test + public void testPerformPagedSearchThrowsOnFailure() throws Exception{ + Map config = new HashMap<>(ldapBackendConfig); + config.put("pageSize", Integer.toString(PAGE_SIZE)); + ldapProxyBackend = new LdapProxyBackend("testbackend", config); + + // Mock out unsuccessful paging result + LdapResult mockResult = createMock(LdapResult.class); + expect(mockResult.getResultCode()).andReturn(ResultCodeEnum.UNWILLING_TO_PERFORM).atLeastOnce(); + expect(mockResult.getDiagnosticMessage()).andReturn("test error message"); + SearchResultDone mockDone = createMock(SearchResultDone.class); + expect(mockDone.getLdapResult()).andReturn(mockResult); + SearchCursor mockCursor = createMock(SearchCursor.class); + expect(mockCursor.next()).andReturn(false); + expect(mockCursor.isDone()).andReturn(true); + expect(mockCursor.getSearchResultDone()).andReturn(mockDone); + LdapConnection mockConnection = createMock(LdapConnection.class); + expect(mockConnection.search(anyObject(SearchRequest.class))).andReturn(mockCursor); + replay(mockResult, + mockDone, + mockCursor, + mockConnection); + + LdapOperationException exception = assertThrows( + LdapOperationException.class, + () -> ldapProxyBackend.performPagedSearch(mockConnection, "ou=people,dc=proxy,dc=org", "(objectclass=inetOrgPerson)", SearchScope.SUBTREE, "*")); + assertEquals(ResultCodeEnum.UNWILLING_TO_PERFORM, exception.getResultCode()); + assertEquals("test error message", exception.getMessage()); + } + // Helper methods for refactoring private Map createConfigWithUserAttr(String attr) {