diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManager.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManager.java index 5d58b42e54..2c24426864 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManager.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManager.java @@ -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; @@ -378,10 +377,6 @@ public List getUserGroups(String username) throws Exception { searchRequest.setFilter("(uid=" + username + ")"); searchRequest.addAttributes("*"); - RolesLookupBypassControlImpl bypassControl = new RolesLookupBypassControlImpl(); - bypassControl.setBypassRolesLookup(true); - searchRequest.addControl(bypassControl); - List groups = new ArrayList<>(); try (Cursor cursor = directoryService.getAdminSession().search(searchRequest)) { if (cursor.next()) { diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManagerTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManagerTest.java index b277cb8351..19708dea2e 100644 --- a/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManagerTest.java +++ b/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManagerTest.java @@ -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; @@ -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; @@ -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 chain = new ArrayList<>(serverManager.directoryService.getInterceptors()); - chain.add(0, rolesInterceptor); - chain.add(1, entriesInterceptor); - serverManager.directoryService.setInterceptors(chain); - - List 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); diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/interceptor/ConfigurableEntriesTestInterceptor.java b/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/interceptor/ConfigurableEntriesTestInterceptor.java index a8507c614f..27ac17f86b 100644 --- a/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/interceptor/ConfigurableEntriesTestInterceptor.java +++ b/gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/interceptor/ConfigurableEntriesTestInterceptor.java @@ -35,7 +35,7 @@ public class ConfigurableEntriesTestInterceptor extends BaseInterceptor { private List entries; private EntryFilteringCursor cursor; - public ConfigurableEntriesTestInterceptor(String name) { + ConfigurableEntriesTestInterceptor(String name) { super(name); } diff --git a/gateway-service-auth/src/main/java/org/apache/knox/gateway/service/auth/AbstractAuthResource.java b/gateway-service-auth/src/main/java/org/apache/knox/gateway/service/auth/AbstractAuthResource.java index 4e844d6436..273cfd771e 100644 --- a/gateway-service-auth/src/main/java/org/apache/knox/gateway/service/auth/AbstractAuthResource.java +++ b/gateway-service-auth/src/main/java/org/apache/knox/gateway/service/auth/AbstractAuthResource.java @@ -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; @@ -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; @@ -57,6 +59,8 @@ 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; @@ -64,6 +68,7 @@ public abstract class AbstractAuthResource { 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); @@ -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 names = config.getLDAPInterceptorNames(); + if (names == null || names.isEmpty()) { + return false; + } + for (String name : names) { + final Map 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 */ @@ -101,14 +123,21 @@ public Response doGetImpl() { } getResponse().setHeader(authHeaderActorIDName, primaryPrincipalName); - // Populate actor groups/roles headers final Set 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 roles = lookupRoles(primaryPrincipalName, matchingGroupNames); - if (!matchingGroupNames.isEmpty() || !roles.isEmpty()) { - final boolean useRoles = !roles.isEmpty(); - final List groupStrings = GroupUtils.getGroupStrings(useRoles ? roles : matchingGroupNames, groupHeaderLengthLimit, groupHeaderSizeLimit); + final Collection headerValues; + final boolean useRoles; + if (rolesLookupInterceptorEnabled) { + headerValues = matchingGroupNames; + useRoles = !headerValues.isEmpty(); + } else { + final Collection roles = lookupRoles(primaryPrincipalName, matchingGroupNames); + useRoles = !roles.isEmpty(); + headerValues = useRoles ? roles : matchingGroupNames; + } + if (!headerValues.isEmpty()) { + final List 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)); @@ -119,15 +148,14 @@ public Response doGetImpl() { private Collection lookupRoles(String userName, Collection groups) { Collection 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; } } diff --git a/gateway-service-auth/src/test/java/org/apache/knox/gateway/service/auth/PreAuthResourceTest.java b/gateway-service-auth/src/test/java/org/apache/knox/gateway/service/auth/PreAuthResourceTest.java index 78c019d626..ea47f0df35 100644 --- a/gateway-service-auth/src/test/java/org/apache/knox/gateway/service/auth/PreAuthResourceTest.java +++ b/gateway-service-auth/src/test/java/org/apache/knox/gateway/service/auth/PreAuthResourceTest.java @@ -18,6 +18,7 @@ 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; @@ -25,6 +26,7 @@ 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; @@ -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; @@ -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 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))); @@ -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 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 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 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 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.>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();