Skip to content

KNOX-3406: LDAP Proxy backend improve error handling #1355

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
handavid wants to merge 1 commit into
base: master
Choose a base branch
from
Open

KNOX-3406: LDAP Proxy backend improve error handling #1355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/tests/test_knox_ldap_proxy_search.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/LdapMessages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand Down
24 changes: 17 additions & 7 deletions ...-server/src/main/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackend.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -557,7 +562,7 @@ public List<Entry> search(String searchBase, SearchScope searchScope, String fil
}
}

private void addGroupMemberships(Entry entry, LdapConnection connection, Map<String, Entry> entryCache, Map<String, Set<String>> resolvedParentsCache) throws Exception {
private void addGroupMemberships(Entry entry, LdapConnection connection, Map<String, Entry> entryCache, Map<String, Set<String>> 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) {
Expand All @@ -568,7 +573,7 @@ private void addGroupMemberships(Entry entry, LdapConnection connection, Map<Str
}
}

private List<Entry> getUserGroupsEntries(LdapConnection connection, Entry user, Map<String, Entry> entryCache, Map<String, Set<String>> resolvedParentsCache) throws Exception {
private List<Entry> getUserGroupsEntries(LdapConnection connection, Entry user, Map<String, Entry> entryCache, Map<String, Set<String>> resolvedParentsCache) throws LdapException, CursorException, IOException {
List<Entry> groups = new ArrayList<>();
if (useMemberOf) {
// Use memberOf attribute for efficient AD lookups
Expand Down Expand Up @@ -887,6 +892,11 @@ protected List<Entry> 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) {
Expand All @@ -903,7 +913,7 @@ protected List<Entry> 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;
Expand Down
41 changes: 41 additions & 0 deletions ...ver/src/test/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackendTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -795,6 +806,36 @@ public void testIsSupportedSearchBase() {
assertFalse(ldapProxyBackend.isSupportedSearchBase("dc=other,dc=base,dc=org"));
}

@Test
public void testPerformPagedSearchThrowsOnFailure() throws Exception{
Map<String, String> 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<String, String> createConfigWithUserAttr(String attr) {
Expand Down