Skip to content

KNOX-3374: Revert LDAPServer Manager changes. Skip roles lookup if ro… #1301

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
hanicz wants to merge 1 commit into
base: master
Choose a base branch
from
Open

KNOX-3374: Revert LDAPServer Manager changes. Skip roles lookup if ro… #1301

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
5 changes: 0 additions & 5 deletions ...way-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import org.apache.knox.gateway.config.GatewayConfig;
import org.apache.knox.gateway.i18n.messages.MessagesFactory;
import org.apache.knox.gateway.services.ldap.control.RolesLookupBypassControlFactory;
import org.apache.knox.gateway.services.ldap.control.RolesLookupBypassControlImpl;
import org.apache.knox.gateway.services.ldap.interceptor.InterceptorFactory;
import org.apache.knox.gateway.services.security.AliasService;

Expand Down Expand Up @@ -378,10 +377,6 @@ public List<String> getUserGroups(String username) throws Exception {
searchRequest.setFilter("(uid=" + username + ")");
searchRequest.addAttributes("*");

RolesLookupBypassControlImpl bypassControl = new RolesLookupBypassControlImpl();
bypassControl.setBypassRolesLookup(true);
searchRequest.addControl(bypassControl);

List<String> groups = new ArrayList<>();
try (Cursor<Entry> cursor = directoryService.getAdminSession().search(searchRequest)) {
if (cursor.next()) {
Expand Down
47 changes: 0 additions & 47 deletions ...server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,16 @@

import org.apache.directory.api.ldap.codec.api.ControlFactory;
import org.apache.directory.api.ldap.model.cursor.EntryCursor;
import org.apache.directory.api.ldap.model.entry.DefaultEntry;
import org.apache.directory.api.ldap.model.entry.Entry;
import org.apache.directory.api.ldap.model.exception.LdapAuthenticationException;
import org.apache.directory.api.ldap.model.exception.LdapException;
import org.apache.directory.api.ldap.model.message.Control;
import org.apache.directory.api.ldap.model.message.SearchScope;
import org.apache.directory.api.ldap.model.schema.SchemaManager;
import org.apache.directory.ldap.client.api.LdapConnection;
import org.apache.directory.ldap.client.api.LdapNetworkConnection;
import org.apache.directory.server.core.api.interceptor.Interceptor;
import org.apache.knox.gateway.config.GatewayConfig;
import org.apache.knox.gateway.services.security.AliasService;
import org.apache.knox.gateway.services.ldap.control.RolesLookupBypassControlFactory;
import org.apache.knox.gateway.services.ldap.interceptor.ConfigurableEntriesTestInterceptor;
import org.apache.knox.gateway.services.ldap.interceptor.LDAPRolesLookupInterceptor;
import org.apache.knox.gateway.services.ldap.model.constants.SchemaConstants;
import org.easymock.EasyMock;
import org.apache.directory.api.ldap.model.name.Dn;
Expand All @@ -45,15 +40,12 @@
import java.io.File;
import java.net.ServerSocket;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import static org.easymock.EasyMock.anyObject;
import static org.easymock.EasyMock.anyString;
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.replay;
import static org.junit.Assert.assertEquals;
Expand Down Expand Up @@ -416,45 +408,6 @@ public void testStartRegistersRolesLookupBypassControl() throws Exception {
assertTrue(controlFactoryMap.get(SchemaConstants.ROLES_LOOKUP_BYPASS_CONTROL_OID) instanceof RolesLookupBypassControlFactory);
}

@Test
public void testGetUserGroupsReturnsRawGroupsEvenWhenRolesInterceptorRewritesMemberOf() throws Exception {
GatewayConfig mockConfig = EasyMock.createNiceMock(GatewayConfig.class);
expect(mockConfig.getGatewayDataDir()).andReturn(tempWorkDir.getParent()).anyTimes();
expect(mockConfig.getLDAPPort()).andReturn(port).anyTimes();
expect(mockConfig.getLDAPBaseDN()).andReturn("dc=test,dc=com").anyTimes();
expect(mockConfig.getLDAPInterceptorNames()).andReturn(List.of()).anyTimes();
replay(mockConfig);

serverManager.initialize(mockConfig);
serverManager.start();

SchemaManager schemaManager = serverManager.directoryService.getSchemaManager();
Entry userEntry = new DefaultEntry(schemaManager);
userEntry.setDn("uid=admin,ou=people,dc=test,dc=com");
userEntry.add("uid", "admin");
userEntry.add("memberOf", "cn=me-test-group-a,ou=groups,dc=test,dc=com");
userEntry.add("memberOf", "cn=me-test-group-b,ou=groups,dc=test,dc=com");
ConfigurableEntriesTestInterceptor entriesInterceptor = new ConfigurableEntriesTestInterceptor("testEntries");
entriesInterceptor.setEntries(List.of(userEntry));
entriesInterceptor.init(serverManager.directoryService);

LDAPRolesLookupService mockRolesService = EasyMock.createNiceMock(LDAPRolesLookupService.class);
expect(mockRolesService.lookupRoles(anyString(), anyObject()))
.andReturn(Arrays.asList("console:admin", "ws-1:viewer", "ws-2:user")).anyTimes();
replay(mockRolesService);
LDAPRolesLookupInterceptor rolesInterceptor = new LDAPRolesLookupInterceptor(mockRolesService);
rolesInterceptor.init(serverManager.directoryService);

List<Interceptor> chain = new ArrayList<>(serverManager.directoryService.getInterceptors());
chain.add(0, rolesInterceptor);
chain.add(1, entriesInterceptor);
serverManager.directoryService.setInterceptors(chain);

List<String> groups = serverManager.getUserGroups("admin");

assertEquals(Arrays.asList("me-test-group-a", "me-test-group-b"), groups);
}

@Test(expected = LdapException.class)
public void testBindRequiredRejectsAnonymous() throws Exception {
useBindPassword(BIND_PASSWORD);
Expand Down
2 changes: 1 addition & 1 deletion ...org/apache/knox/gateway/services/ldap/interceptor/ConfigurableEntriesTestInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public class ConfigurableEntriesTestInterceptor extends BaseInterceptor {
private List<Entry> entries;
private EntryFilteringCursor cursor;

public ConfigurableEntriesTestInterceptor(String name) {
ConfigurableEntriesTestInterceptor(String name) {
super(name);
}

Expand Down
54 changes: 41 additions & 13 deletions ...service-auth/src/main/java/org/apache/knox/gateway/service/auth/AbstractAuthResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/
package org.apache.knox.gateway.service.auth;

import org.apache.knox.gateway.config.GatewayConfig;
import org.apache.knox.gateway.i18n.messages.MessagesFactory;
import org.apache.knox.gateway.security.SubjectUtils;
import org.apache.knox.gateway.services.GatewayServices;
Expand All @@ -32,6 +33,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
Expand All @@ -57,13 +59,16 @@ public abstract class AbstractAuthResource {
private static final String DEFAULT_GROUP_HEADER_SIZE_LIMIT = "-1"; // turned off by default, to be backward compatible
private static final String ACTOR_GROUPS_HEADER_FORMAT = "%s-%d";

private static final String ROLES_LOOKUP_INTERCEPTOR_TYPE = "rolesLookup";

protected String authHeaderActorIDName;
protected String authHeaderActorGroupsPrefix;
private int groupHeaderLengthLimit;
private int groupHeaderSizeLimit;
protected Pattern groupFilterPattern;
protected String authHeaderActorRolesName;
private LDAPRolesLookupService ldapRolesLookupService;
private boolean rolesLookupInterceptorEnabled;

protected void initialize() {
authHeaderActorIDName = getInitParameter(AUTH_ACTOR_ID_HEADER_NAME, DEFAULT_AUTH_ACTOR_ID_HEADER_NAME);
Expand All @@ -77,7 +82,24 @@ protected void initialize() {
if (gatewayServices != null) {
ldapRolesLookupService = gatewayServices.getService(ServiceType.LDAP_ROLES_LOOKUP_SERVICE);
}
rolesLookupInterceptorEnabled = isRolesLookupInterceptorConfigured((GatewayConfig) getContext().getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE));
}

private static boolean isRolesLookupInterceptorConfigured(GatewayConfig config) {
if (config == null) {
return false;
}
final List<String> names = config.getLDAPInterceptorNames();
if (names == null || names.isEmpty()) {
return false;
}
for (String name : names) {
final Map<String, String> cfg = config.getLDAPInterceptorConfig(name);
if (cfg != null && ROLES_LOOKUP_INTERCEPTOR_TYPE.equalsIgnoreCase(cfg.get("interceptorType"))) {
return true;
}
}
return false;
}

/* abstract method to get the response instance */
Expand All @@ -101,14 +123,21 @@ public Response doGetImpl() {
}
getResponse().setHeader(authHeaderActorIDName, primaryPrincipalName);

// Populate actor groups/roles headers
final Set<String> matchingGroupNames = subject == null ? Collections.emptySet()
: SubjectUtils.getGroupPrincipals(subject).stream().filter(group -> groupFilterPattern.matcher(group.getName()).matches()).map(group -> group.getName())
.collect(Collectors.toSet());
final Collection<String> roles = lookupRoles(primaryPrincipalName, matchingGroupNames);
if (!matchingGroupNames.isEmpty() || !roles.isEmpty()) {
final boolean useRoles = !roles.isEmpty();
final List<String> groupStrings = GroupUtils.getGroupStrings(useRoles ? roles : matchingGroupNames, groupHeaderLengthLimit, groupHeaderSizeLimit);
final Collection<String> headerValues;
final boolean useRoles;
if (rolesLookupInterceptorEnabled) {
headerValues = matchingGroupNames;
useRoles = !headerValues.isEmpty();
} else {
final Collection<String> roles = lookupRoles(primaryPrincipalName, matchingGroupNames);
useRoles = !roles.isEmpty();
headerValues = useRoles ? roles : matchingGroupNames;
}
if (!headerValues.isEmpty()) {
final List<String> groupStrings = GroupUtils.getGroupStrings(headerValues, groupHeaderLengthLimit, groupHeaderSizeLimit);
for (int i = 0; i < groupStrings.size(); i++) {
final String headerName = useRoles ? authHeaderActorGroupsPrefix : String.format(Locale.ROOT, ACTOR_GROUPS_HEADER_FORMAT, authHeaderActorGroupsPrefix, i + 1);
getResponse().addHeader(headerName, groupStrings.get(i));
Expand All @@ -119,15 +148,14 @@ public Response doGetImpl() {

private Collection<String> lookupRoles(String userName, Collection<String> groups) {
Collection<String> roles = null;
try {
if (ldapRolesLookupService != null && ldapRolesLookupService.enabled()) {
roles = ldapRolesLookupService.lookupRoles(userName, groups);
}
} catch (Exception e) {
// Couldn't lookup roles: log and return null so that the API will return the groups
LOG.ldapRolesLookupFailed(userName, e);
try {
if (ldapRolesLookupService != null && ldapRolesLookupService.enabled()) {
roles = ldapRolesLookupService.lookupRoles(userName, groups);
}
return roles == null ? Collections.emptySet() : roles;
} catch (Exception e) {
LOG.ldapRolesLookupFailed(userName, e);
}
return roles == null ? Collections.emptySet() : roles;
}

}
105 changes: 94 additions & 11 deletions ...-service-auth/src/test/java/org/apache/knox/gateway/service/auth/PreAuthResourceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@
package org.apache.knox.gateway.service.auth;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;

Expand All @@ -34,12 +36,15 @@
import javax.servlet.http.HttpServletResponse;
import javax.ws.rs.core.Response;

import org.apache.knox.gateway.config.GatewayConfig;
import org.apache.knox.gateway.security.GroupPrincipal;
import org.apache.knox.gateway.security.PrimaryPrincipal;
import org.apache.knox.gateway.security.SubjectUtils;
import org.apache.knox.gateway.services.GatewayServices;
import org.apache.knox.gateway.services.ServiceType;
import org.apache.knox.gateway.services.ldap.LDAPRolesLookupService;
import org.easymock.Capture;
import org.easymock.CaptureType;
import org.easymock.EasyMock;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -79,14 +84,11 @@ private void configureCommonExpectations(String actorIdHeaderName, String groups

if (gatewayServices != null) {
EasyMock.expect(context.getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE)).andReturn(gatewayServices);
final LDAPRolesLookupService rolesLookupService = gatewayServices.getService(ServiceType.LDAP_ROLES_LOOKUP_SERVICE);
if (rolesLookupService != null && rolesLookupService.enabled()) {
Collection<String> roles = rolesLookupService.lookupRoles(USER_NAME, groups);
if (roles != null && !roles.isEmpty()) {
final String expectedGroupsHeaderPrefix = (groupsHeaderPrefix == null ? PreAuthResource.DEFAULT_AUTH_ACTOR_GROUPS_HEADER_PREFIX : groupsHeaderPrefix);
response.addHeader(EasyMock.eq(expectedGroupsHeaderPrefix), EasyMock.anyString());
EasyMock.expectLastCall().anyTimes();
}
groups.forEach(group -> subject.getPrincipals().add(new GroupPrincipal(group)));
if (!groups.isEmpty()) {
final String expectedGroupsHeaderPrefix = (groupsHeaderPrefix == null ? PreAuthResource.DEFAULT_AUTH_ACTOR_GROUPS_HEADER_PREFIX : groupsHeaderPrefix);
response.addHeader(EasyMock.eq(expectedGroupsHeaderPrefix), EasyMock.anyString());
EasyMock.expectLastCall();
}
} else if (!groups.isEmpty()) {
groups.forEach(group -> subject.getPrincipals().add(new GroupPrincipal(group)));
Expand Down Expand Up @@ -173,13 +175,94 @@ public void testPopulatingGroupsWithRoles() throws Exception {
EasyMock.verify(response);
}

private GatewayServices configureLdapRolesLookupExpectations() throws Exception {
@Test
public void testInheritedRolesFromSubjectAllAppearInSingleRolesHeader() throws Exception {
final String directRole = "platform:admin";
final String inheritedRole = "ml-workspace:viewer";
subject.getPrincipals().add(new GroupPrincipal(directRole));
subject.getPrincipals().add(new GroupPrincipal(inheritedRole));

final LDAPRolesLookupService rolesLookupService = EasyMock.createMock(LDAPRolesLookupService.class);
EasyMock.expect(rolesLookupService.enabled()).andReturn(true).anyTimes();

final GatewayServices gatewayServices = EasyMock.createNiceMock(GatewayServices.class);
EasyMock.expect(gatewayServices.getService(ServiceType.LDAP_ROLES_LOOKUP_SERVICE)).andReturn(rolesLookupService).anyTimes();

context = EasyMock.createNiceMock(ServletContext.class);
EasyMock.expect(context.getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE)).andReturn(gatewayServices).anyTimes();
EasyMock.expect(context.getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE)).andReturn(mockConfigWithRolesLookupInterceptor()).anyTimes();

response = EasyMock.createMock(HttpServletResponse.class);
response.setHeader(PreAuthResource.DEFAULT_AUTH_ACTOR_ID_HEADER_NAME, USER_NAME);
EasyMock.expectLastCall();
final Capture<String> headerValue = EasyMock.newCapture(CaptureType.ALL);
response.addHeader(EasyMock.eq(PreAuthResource.DEFAULT_AUTH_ACTOR_GROUPS_HEADER_PREFIX), EasyMock.capture(headerValue));
EasyMock.expectLastCall();

EasyMock.replay(rolesLookupService, gatewayServices, context, response);

final PreAuthResource preAuthResource = new PreAuthResource();
preAuthResource.context = context;
preAuthResource.response = response;
executeResourceWithSubject(preAuthResource);

final List<String> allHeaders = headerValue.getValues();
final String joined = String.join(",", allHeaders);
assertTrue("directRole missing from actor-groups header: " + joined, joined.contains(directRole));
assertTrue("inheritedRole missing from actor-groups header (KNOX-3374): " + joined, joined.contains(inheritedRole));
EasyMock.verify(response, rolesLookupService);
}

private static GatewayConfig mockConfigWithRolesLookupInterceptor() {
final String name = "rolesLookupInterceptor";
final GatewayConfig config = EasyMock.createNiceMock(GatewayConfig.class);
EasyMock.expect(config.getLDAPInterceptorNames()).andReturn(Collections.singletonList(name)).anyTimes();
EasyMock.expect(config.getLDAPInterceptorConfig(name)).andReturn(Collections.singletonMap("interceptorType", "rolesLookup")).anyTimes();
EasyMock.replay(config);
return config;
}

@Test
public void testFallsBackToDirectRolesLookupWhenSubjectHasNoGroups() throws Exception {
final String role1 = "platform:admin";
final String role2 = "ml-workspace:viewer";
final Set<String> groups = Collections.singleton("engineering");

final LDAPRolesLookupService rolesLookupService = EasyMock.createMock(LDAPRolesLookupService.class);
EasyMock.expect(rolesLookupService.enabled()).andReturn(true).anyTimes();
EasyMock.expect(rolesLookupService.lookupRoles(EasyMock.eq(USER_NAME), EasyMock.eq(Collections.emptySet())))
.andReturn(Arrays.asList(role1, role2));

final GatewayServices gatewayServices = EasyMock.createNiceMock(GatewayServices.class);
EasyMock.expect(gatewayServices.getService(ServiceType.LDAP_ROLES_LOOKUP_SERVICE)).andReturn(rolesLookupService).anyTimes();

context = EasyMock.createNiceMock(ServletContext.class);
EasyMock.expect(context.getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE)).andReturn(gatewayServices).anyTimes();

response = EasyMock.createMock(HttpServletResponse.class);
response.setHeader(PreAuthResource.DEFAULT_AUTH_ACTOR_ID_HEADER_NAME, USER_NAME);
EasyMock.expectLastCall();
final Capture<String> headerValue = EasyMock.newCapture(CaptureType.ALL);
response.addHeader(EasyMock.eq(PreAuthResource.DEFAULT_AUTH_ACTOR_GROUPS_HEADER_PREFIX), EasyMock.capture(headerValue));
EasyMock.expectLastCall();

EasyMock.replay(rolesLookupService, gatewayServices, context, response);

final PreAuthResource preAuthResource = new PreAuthResource();
preAuthResource.context = context;
preAuthResource.response = response;
executeResourceWithSubject(preAuthResource);

final String joined = String.join(",", headerValue.getValues());
assertTrue("role1 missing from fallback lookup: " + joined, joined.contains(role1));
assertTrue("role2 missing from fallback lookup: " + joined, joined.contains(role2));
EasyMock.verify(response, rolesLookupService);
}

private GatewayServices configureLdapRolesLookupExpectations() throws Exception {
final LDAPRolesLookupService rolesLookupService = EasyMock.createNiceMock(LDAPRolesLookupService.class);
EasyMock.expect(rolesLookupService.enabled()).andReturn(true).anyTimes();
EasyMock.expect(rolesLookupService.lookupRoles(EasyMock.eq(USER_NAME), EasyMock.anyObject())).andReturn(Arrays.asList(role1, role2)).anyTimes();
EasyMock.expect(rolesLookupService.lookupRoles(EasyMock.eq(USER_NAME), EasyMock.<Collection<String>>anyObject()))
.andReturn(Arrays.asList("platform:admin", "ml-workspace:viewer")).anyTimes();

final GatewayServices gatewayServices = EasyMock.createNiceMock(GatewayServices.class);
EasyMock.expect(gatewayServices.getService(ServiceType.LDAP_ROLES_LOOKUP_SERVICE)).andReturn(rolesLookupService).anyTimes();
Expand Down
Loading