From 67d03adf88de35b2aa2b42abac6e30564a5572f5 Mon Sep 17 00:00:00 2001 From: Pierre Mauduit Date: Fri, 17 Nov 2023 12:44:37 +0100 Subject: [PATCH] json headers - organization as json payload is not transmitted fix Resolve and store user organization for extended LDAP configs Tests: adding an IT --- .../ResolveGeorchestraUserGlobalFilter.java | 14 ++- .../ldap/extended/DemultiplexingUsersApi.java | 37 +++++-- .../extended/ExtendedGeorchestraUser.java | 40 +++++++ ...tendedLdapAuthenticationConfiguration.java | 36 ++++-- ...eorchestraLdapAuthenticatedUserMapper.java | 2 +- gateway/src/main/resources/application.yml | 2 +- .../admin/CreateAccountUserCustomizerIT.java | 3 +- .../ResolveGeorchestraUserGlobalFilterIT.java | 104 ++++++++++++++++++ ...hestraLdapAuthenticatedUserMapperTest.java | 14 ++- .../resources/application-georheaders.yml | 47 ++++++++ 10 files changed, 271 insertions(+), 28 deletions(-) create mode 100644 gateway/src/main/java/org/georchestra/gateway/security/ldap/extended/ExtendedGeorchestraUser.java create mode 100644 gateway/src/test/java/org/georchestra/gateway/security/ResolveGeorchestraUserGlobalFilterIT.java create mode 100644 gateway/src/test/resources/application-georheaders.yml diff --git a/gateway/src/main/java/org/georchestra/gateway/security/ResolveGeorchestraUserGlobalFilter.java b/gateway/src/main/java/org/georchestra/gateway/security/ResolveGeorchestraUserGlobalFilter.java index 8138db60..474a9fbf 100644 --- a/gateway/src/main/java/org/georchestra/gateway/security/ResolveGeorchestraUserGlobalFilter.java +++ b/gateway/src/main/java/org/georchestra/gateway/security/ResolveGeorchestraUserGlobalFilter.java @@ -18,9 +18,12 @@ */ package org.georchestra.gateway.security; +import org.georchestra.gateway.model.GeorchestraOrganizations; import org.georchestra.gateway.model.GeorchestraTargetConfig; import org.georchestra.gateway.model.GeorchestraUsers; +import org.georchestra.gateway.security.ldap.extended.ExtendedGeorchestraUser; import org.georchestra.security.model.GeorchestraUser; +import org.georchestra.security.model.Organization; import org.springframework.cloud.gateway.filter.GatewayFilterChain; import org.springframework.cloud.gateway.filter.GlobalFilter; import org.springframework.cloud.gateway.filter.RouteToRequestUrlFilter; @@ -75,7 +78,16 @@ public class ResolveGeorchestraUserGlobalFilter implements GlobalFilter, Ordered .filter(Authentication.class::isInstance)// .map(Authentication.class::cast)// .map(resolver::resolve)// - .map(user -> GeorchestraUsers.store(exchange, user.orElse(null)))// + .map(user -> { + GeorchestraUser usr = user.orElse(null); + GeorchestraUsers.store(exchange, usr); + if (usr != null && usr instanceof ExtendedGeorchestraUser) { + ExtendedGeorchestraUser eu = (ExtendedGeorchestraUser) usr; + Organization org = eu.getOrg(); + GeorchestraOrganizations.store(exchange, org); + } + return exchange; + })// .defaultIfEmpty(exchange)// .flatMap(chain::filter); } diff --git a/gateway/src/main/java/org/georchestra/gateway/security/ldap/extended/DemultiplexingUsersApi.java b/gateway/src/main/java/org/georchestra/gateway/security/ldap/extended/DemultiplexingUsersApi.java index fc111cf6..66b7a53f 100644 --- a/gateway/src/main/java/org/georchestra/gateway/security/ldap/extended/DemultiplexingUsersApi.java +++ b/gateway/src/main/java/org/georchestra/gateway/security/ldap/extended/DemultiplexingUsersApi.java @@ -19,14 +19,17 @@ package org.georchestra.gateway.security.ldap.extended; +import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; +import org.georchestra.security.api.OrganizationsApi; import org.georchestra.security.api.UsersApi; import org.georchestra.security.model.GeorchestraUser; +import org.georchestra.security.model.Organization; import com.google.common.annotations.VisibleForTesting; @@ -48,10 +51,11 @@ @RequiredArgsConstructor class DemultiplexingUsersApi { - private final @NonNull Map targets; + private final @NonNull Map usersByConfigName; + private final @NonNull Map orgsByConfigName; public @VisibleForTesting Set getTargetNames() { - return new HashSet<>(targets.keySet()); + return new HashSet<>(usersByConfigName.keySet()); } /** @@ -64,15 +68,28 @@ class DemultiplexingUsersApi { * @return the {@link GeorchestraUser} returned by the service's * {@link UsersApi}, or {@link Optional#empty() empty} if not found */ - public Optional findByUsername(@NonNull String serviceName, @NonNull String username) { - UsersApi target = targets.get(serviceName); - Objects.requireNonNull(target, () -> "No UsersApi found for config named " + serviceName); - return target.findByUsername(username); + public Optional findByUsername(@NonNull String serviceName, @NonNull String username) { + UsersApi usersApi = usersByConfigName.get(serviceName); + Objects.requireNonNull(usersApi, () -> "No UsersApi found for config named " + serviceName); + Optional user = usersApi.findByUsername(username); + + return extend(serviceName, user); + } + + public Optional findByEmail(@NonNull String serviceName, @NonNull String email) { + UsersApi usersApi = usersByConfigName.get(serviceName); + Objects.requireNonNull(usersApi, () -> "No UsersApi found for config named " + serviceName); + Optional user = usersApi.findByEmail(email); + return extend(serviceName, user); } - public Optional findByEmail(@NonNull String serviceName, @NonNull String email) { - UsersApi target = targets.get(serviceName); - Objects.requireNonNull(target, () -> "No UsersApi found for config named " + serviceName); - return target.findByEmail(email); + private Optional extend(String serviceName, Optional user) { + OrganizationsApi orgsApi = orgsByConfigName.get(serviceName); + Objects.requireNonNull(orgsApi, () -> "No OrganizationsApi found for config named " + serviceName); + + Organization org = user.map(GeorchestraUser::getOrganization).flatMap(orgsApi::findByShortName).orElse(null); + + return user.map(ExtendedGeorchestraUser::new).map(u -> u.setOrg(org)); } + } diff --git a/gateway/src/main/java/org/georchestra/gateway/security/ldap/extended/ExtendedGeorchestraUser.java b/gateway/src/main/java/org/georchestra/gateway/security/ldap/extended/ExtendedGeorchestraUser.java new file mode 100644 index 00000000..c839c98e --- /dev/null +++ b/gateway/src/main/java/org/georchestra/gateway/security/ldap/extended/ExtendedGeorchestraUser.java @@ -0,0 +1,40 @@ +package org.georchestra.gateway.security.ldap.extended; + +import org.georchestra.security.model.GeorchestraUser; +import org.georchestra.security.model.Organization; + +import com.fasterxml.jackson.annotation.JsonIgnore; + +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.NonNull; +import lombok.RequiredArgsConstructor; +import lombok.Setter; +import lombok.experimental.Accessors; +import lombok.experimental.Delegate; + +/** + * {@link GeorchestraUser} with resolved {@link #getOrg() Organization} + */ +@SuppressWarnings("serial") +@RequiredArgsConstructor +@Accessors(chain = true) +public class ExtendedGeorchestraUser extends GeorchestraUser { + + @JsonIgnore + private final @NonNull @Delegate GeorchestraUser user; + + @JsonIgnore + private @Getter @Setter Organization org; + + public @Override boolean equals(Object o) { + if (!(o instanceof GeorchestraUser)) { + return false; + } + return super.equals(o); + } + + public @Override int hashCode() { + return super.hashCode(); + } +} diff --git a/gateway/src/main/java/org/georchestra/gateway/security/ldap/extended/ExtendedLdapAuthenticationConfiguration.java b/gateway/src/main/java/org/georchestra/gateway/security/ldap/extended/ExtendedLdapAuthenticationConfiguration.java index afe7ec7a..88d37b3e 100644 --- a/gateway/src/main/java/org/georchestra/gateway/security/ldap/extended/ExtendedLdapAuthenticationConfiguration.java +++ b/gateway/src/main/java/org/georchestra/gateway/security/ldap/extended/ExtendedLdapAuthenticationConfiguration.java @@ -32,9 +32,7 @@ import org.georchestra.ds.roles.RoleDao; import org.georchestra.ds.roles.RoleDaoImpl; import org.georchestra.ds.roles.RoleProtected; -import org.georchestra.ds.security.UserMapper; -import org.georchestra.ds.security.UserMapperImpl; -import org.georchestra.ds.security.UsersApiImpl; +import org.georchestra.ds.security.*; import org.georchestra.ds.users.AccountDao; import org.georchestra.ds.users.AccountDaoImpl; import org.georchestra.ds.users.UserRule; @@ -42,6 +40,7 @@ import org.georchestra.gateway.security.ldap.LdapConfigProperties; import org.georchestra.gateway.security.ldap.LdapConfigProperties.Server; import org.georchestra.gateway.security.ldap.basic.LdapAuthenticatorProviderBuilder; +import org.georchestra.security.api.OrganizationsApi; import org.georchestra.security.api.UsersApi; import org.georchestra.security.model.GeorchestraUser; import org.springframework.beans.factory.BeanInitializationException; @@ -107,25 +106,44 @@ private GeorchestraLdapAuthenticationProvider createLdapProvider(ExtendedLdapCon @Bean DemultiplexingUsersApi demultiplexingUsersApi(List configs) { - Map targets = new HashMap<>(); + Map usersByConfigName = new HashMap<>(); + Map orgsByConfigName = new HashMap<>(); for (ExtendedLdapConfig config : configs) { try { - targets.put(config.getName(), createUsersApi(config)); + LdapTemplate ldapTemplate = ldapTemplate(config); + AccountDao accountsDao = accountsDao(ldapTemplate, config); + UsersApi usersApi = createUsersApi(config, ldapTemplate, accountsDao); + OrganizationsApi orgsApi = createOrgsApi(config, ldapTemplate, accountsDao); + usersByConfigName.put(config.getName(), usersApi); + orgsByConfigName.put(config.getName(), orgsApi); } catch (Exception ex) { throw new BeanInitializationException( "Error creating georchestra users api for ldap config " + config.getName(), ex); } } - return new DemultiplexingUsersApi(targets); + return new DemultiplexingUsersApi(usersByConfigName, orgsByConfigName); } ////////////////////////////////////////////// /// Low level LDAP account management beans ////////////////////////////////////////////// - private UsersApi createUsersApi(ExtendedLdapConfig ldapConfig) throws Exception { - final LdapTemplate ldapTemplate = ldapTemplate(ldapConfig); - final AccountDao accountsDao = accountsDao(ldapTemplate, ldapConfig); + private OrganizationsApi createOrgsApi(ExtendedLdapConfig ldapConfig, LdapTemplate ldapTemplate, + AccountDao accountsDao) throws Exception { + OrganizationsApiImpl impl = new OrganizationsApiImpl(); + OrgsDaoImpl orgsDao = new OrgsDaoImpl(); + orgsDao.setLdapTemplate(ldapTemplate); + orgsDao.setAccountDao(accountsDao); + orgsDao.setBasePath(ldapConfig.getBaseDn()); + orgsDao.setOrgSearchBaseDN(ldapConfig.getOrgsRdn()); + orgsDao.setPendingOrgSearchBaseDN(ldapConfig.getPendingOrgsRdn()); + impl.setOrgsDao(orgsDao); + impl.setOrgMapper(new OrganizationMapperImpl()); + return impl; + } + + private UsersApi createUsersApi(ExtendedLdapConfig ldapConfig, LdapTemplate ldapTemplate, AccountDao accountsDao) + throws Exception { final RoleDao roleDao = roleDao(ldapTemplate, ldapConfig, accountsDao); final UserMapper ldapUserMapper = createUserMapper(roleDao); diff --git a/gateway/src/main/java/org/georchestra/gateway/security/ldap/extended/GeorchestraLdapAuthenticatedUserMapper.java b/gateway/src/main/java/org/georchestra/gateway/security/ldap/extended/GeorchestraLdapAuthenticatedUserMapper.java index e837f316..8235aa69 100644 --- a/gateway/src/main/java/org/georchestra/gateway/security/ldap/extended/GeorchestraLdapAuthenticatedUserMapper.java +++ b/gateway/src/main/java/org/georchestra/gateway/security/ldap/extended/GeorchestraLdapAuthenticatedUserMapper.java @@ -67,7 +67,7 @@ Optional map(GeorchestraUserNamePasswordAuthenticationToken tok final String ldapConfigName = token.getConfigName(); final String username = principal.getUsername(); - Optional user = users.findByUsername(ldapConfigName, username); + Optional user = users.findByUsername(ldapConfigName, username); if (user.isEmpty()) { user = users.findByEmail(ldapConfigName, username); } diff --git a/gateway/src/main/resources/application.yml b/gateway/src/main/resources/application.yml index 1fc93b7e..3850640a 100644 --- a/gateway/src/main/resources/application.yml +++ b/gateway/src/main/resources/application.yml @@ -50,7 +50,7 @@ spring: - SecureHeaders - TokenRelay - RemoveSecurityHeaders - # AddSecHeaders appends sec-* headers to proxied requests based on the + # AddSecHeaders appends sec-* headers to proxied requests based on the currently authenticated user - AddSecHeaders global-filter: websocket-routing: diff --git a/gateway/src/test/java/org/georchestra/gateway/accounts/admin/CreateAccountUserCustomizerIT.java b/gateway/src/test/java/org/georchestra/gateway/accounts/admin/CreateAccountUserCustomizerIT.java index 62b8e011..f9606761 100644 --- a/gateway/src/test/java/org/georchestra/gateway/accounts/admin/CreateAccountUserCustomizerIT.java +++ b/gateway/src/test/java/org/georchestra/gateway/accounts/admin/CreateAccountUserCustomizerIT.java @@ -161,7 +161,6 @@ private WebTestClient.RequestHeadersUriSpec prepareWebTestClientHeaders( .expectStatus()// .is2xxSuccessful()// .expectBody()// - .jsonPath("$.GeorchestraUser").isNotEmpty() - .jsonPath("$.GeorchestraUser.organization").isEqualTo(null); + .jsonPath("$.GeorchestraUser").isNotEmpty().jsonPath("$.GeorchestraUser.organization").isEqualTo(null); } } diff --git a/gateway/src/test/java/org/georchestra/gateway/security/ResolveGeorchestraUserGlobalFilterIT.java b/gateway/src/test/java/org/georchestra/gateway/security/ResolveGeorchestraUserGlobalFilterIT.java new file mode 100644 index 00000000..f7803287 --- /dev/null +++ b/gateway/src/test/java/org/georchestra/gateway/security/ResolveGeorchestraUserGlobalFilterIT.java @@ -0,0 +1,104 @@ +package org.georchestra.gateway.security; + +import org.georchestra.gateway.app.GeorchestraGatewayApplication; +import org.georchestra.gateway.filter.headers.providers.JsonPayloadHeadersContributor; +import org.georchestra.gateway.model.GatewayConfigProperties; +import org.georchestra.gateway.model.HeaderMappings; +import org.georchestra.testcontainers.ldap.GeorchestraLdapContainer; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.reactive.AutoConfigureWebTestClient; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.context.ApplicationContext; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.reactive.server.WebTestClient; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.utility.DockerImageName; + +import java.util.Arrays; +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.assertNotNull; + +@SpringBootTest(classes = GeorchestraGatewayApplication.class) +@AutoConfigureWebTestClient(timeout = "PT20S") +@ActiveProfiles("georheaders") +public class ResolveGeorchestraUserGlobalFilterIT { + + public static GeorchestraLdapContainer ldap = new GeorchestraLdapContainer(); + + private @Autowired WebTestClient testClient; + + private @Autowired GatewayConfigProperties gatewayConfig; + + private @Autowired ApplicationContext context; + + public static GenericContainer httpEcho = new GenericContainer(DockerImageName.parse("ealen/echo-server")) { + @Override + protected void doStart() { + super.doStart(); + Integer mappedPort = this.getMappedPort(80); + System.setProperty("httpEchoHost", this.getHost()); + System.setProperty("httpEchoPort", mappedPort.toString()); + System.out.println("Automatically set system property httpEchoHost=" + this.getHost()); + System.out.println("Automatically set system property httpEchoPort=" + mappedPort); + } + }; + + public static @BeforeAll void startUpContainers() { + httpEcho.setExposedPorts(Arrays.asList(new Integer[] { 80 })); + httpEcho.start(); + ldap.start(); + } + + public static @AfterAll void shutDownContainers() { + ldap.stop(); + httpEcho.stop(); + } + + public @Test void testReceivedHeadersAsJson() { + gatewayConfig.getDefaultHeaders().setJsonUser(Optional.of(true)); + gatewayConfig.getDefaultHeaders().setJsonOrganization(Optional.of(true)); + assertNotNull(context.getBean(JsonPayloadHeadersContributor.class)); + + testClient.get().uri("/echo/")// + .header("Authorization", "Basic dGVzdGFkbWluOnRlc3RhZG1pbg==") // testadmin:testadmin + .exchange()// + .expectStatus()// + .is2xxSuccessful()// + .expectBody()// + .jsonPath(".request.headers.sec-user").exists().jsonPath(".request.headers.sec-organization").exists(); + } + + public @Test void testJsonUserNoOrganization() { + gatewayConfig.getDefaultHeaders().setJsonUser(Optional.of(true)); + gatewayConfig.getDefaultHeaders().setJsonOrganization(Optional.of(false)); + + testClient.get().uri("/echo/")// + .header("Authorization", "Basic dGVzdGFkbWluOnRlc3RhZG1pbg==") // testadmin:testadmin + .exchange()// + .expectStatus()// + .is2xxSuccessful()// + .expectBody()// + .jsonPath(".request.headers.sec-user").exists()// + .jsonPath(".request.headers.sec-organization").doesNotHaveJsonPath(); + + } + + public @Test void testNoJsonUserJsonOrganization() { + gatewayConfig.getDefaultHeaders().setJsonUser(Optional.of(false)); + gatewayConfig.getDefaultHeaders().setJsonOrganization(Optional.of(true)); + + testClient.get().uri("/echo/")// + .header("Authorization", "Basic dGVzdGFkbWluOnRlc3RhZG1pbg==") // testadmin:testadmin + .exchange()// + .expectStatus()// + .is2xxSuccessful()// + .expectBody()// + .jsonPath(".request.headers.sec-user").doesNotHaveJsonPath()// + .jsonPath(".request.headers.sec-organization").exists(); + + } +} diff --git a/gateway/src/test/java/org/georchestra/gateway/security/ldap/extended/GeorchestraLdapAuthenticatedUserMapperTest.java b/gateway/src/test/java/org/georchestra/gateway/security/ldap/extended/GeorchestraLdapAuthenticatedUserMapperTest.java index d76f382d..56e4996a 100644 --- a/gateway/src/test/java/org/georchestra/gateway/security/ldap/extended/GeorchestraLdapAuthenticatedUserMapperTest.java +++ b/gateway/src/test/java/org/georchestra/gateway/security/ldap/extended/GeorchestraLdapAuthenticatedUserMapperTest.java @@ -19,8 +19,8 @@ package org.georchestra.gateway.security.ldap.extended; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; @@ -35,6 +35,7 @@ import java.util.Map; import java.util.Optional; +import org.georchestra.security.api.OrganizationsApi; import org.georchestra.security.api.UsersApi; import org.georchestra.security.model.GeorchestraUser; import org.junit.jupiter.api.BeforeEach; @@ -53,12 +54,15 @@ class GeorchestraLdapAuthenticatedUserMapperTest { private GeorchestraLdapAuthenticatedUserMapper mapper; private UsersApi mockUsers; + private OrganizationsApi mockOrgs; @BeforeEach void before() { mockUsers = mock(UsersApi.class); + mockOrgs = mock(OrganizationsApi.class); when(mockUsers.findByUsername(anyString())).thenReturn(Optional.empty()); - DemultiplexingUsersApi demultiplexingUsers = new DemultiplexingUsersApi(Map.of("default", mockUsers)); + DemultiplexingUsersApi demultiplexingUsers = new DemultiplexingUsersApi(Map.of("default", mockUsers), + Map.of("default", mockOrgs)); mapper = new GeorchestraLdapAuthenticatedUserMapper(demultiplexingUsers); } @@ -98,7 +102,8 @@ void testNotAnLdapUserDetails() { @Test void testLdapUserDetails() { - GeorchestraUser expected = mock(GeorchestraUser.class); + GeorchestraUser expected = new GeorchestraUser(); + expected.setUsername("testuser"); LdapUserDetailsImpl principal = mock(LdapUserDetailsImpl.class); when(principal.getUsername()).thenReturn("ldapuser"); when(mockUsers.findByUsername(eq("ldapuser"))).thenReturn(Optional.of(expected)); @@ -107,7 +112,8 @@ void testLdapUserDetails() { Optional resolve = mapper .resolve(new GeorchestraUserNamePasswordAuthenticationToken("default", auth)); assertNotNull(resolve); - assertSame(expected, resolve.orElseThrow()); + GeorchestraUser actual = resolve.orElseThrow(); + assertEquals(expected, actual); verify(mockUsers, atLeastOnce()).findByUsername(eq("ldapuser")); verifyNoMoreInteractions(mockUsers); diff --git a/gateway/src/test/resources/application-georheaders.yml b/gateway/src/test/resources/application-georheaders.yml new file mode 100644 index 00000000..2069611a --- /dev/null +++ b/gateway/src/test/resources/application-georheaders.yml @@ -0,0 +1,47 @@ +georchestra: + gateway: + default-headers: + # Default security headers to append to proxied requests + proxy: true + username: true + roles: true + org: true + orgname: true + #jsonUser: true + jsonOrganization: true + security: + ldap: + default: + enabled: true + extended: true + url: ldap://${ldapHost}:${ldapPort}/ + baseDn: dc=georchestra,dc=org + adminDn: cn=admin,dc=georchestra,dc=org + adminPassword: secret + users: + rdn: ou=users + searchFilter: (uid={0}) + pendingUsersSearchBaseDN: ou=pendingusers + protectedUsers: geoserver_privileged_user + roles: + rdn: ou=roles + searchFilter: (member={0}) + protectedRoles: ADMINISTRATOR, EXTRACTORAPP, GN_.*, ORGADMIN, REFERENT, USER, SUPERUSER + orgs: + rdn: ou=orgs + orgTypes: Association,Company,NGO,Individual,Other + pendingOrgSearchBaseDN: ou=pendingorgs + services: + echo: + target: http://${httpEchoHost}:${httpEchoPort} + access-rules: + - intercept-url: /echo/** + anonymous: false +spring: + cloud: + gateway: + routes: + - id: echo + uri: http://${httpEchoHost}:${httpEchoPort} + predicates: + - Path=/echo/