Skip to content

Commit

Permalink
Merge pull request #82 from georchestra/fix-georchestra-headers-as-json
Browse files Browse the repository at this point in the history
fix: geOrchestra json headers - organization as json payload is not transmitted
  • Loading branch information
pmauduit committed Nov 17, 2023
2 parents 1b50552 + 67d03ad commit 95ee6ec
Show file tree
Hide file tree
Showing 10 changed files with 271 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -48,10 +51,11 @@
@RequiredArgsConstructor
class DemultiplexingUsersApi {

private final @NonNull Map<String, UsersApi> targets;
private final @NonNull Map<String, UsersApi> usersByConfigName;
private final @NonNull Map<String, OrganizationsApi> orgsByConfigName;

public @VisibleForTesting Set<String> getTargetNames() {
return new HashSet<>(targets.keySet());
return new HashSet<>(usersByConfigName.keySet());
}

/**
Expand All @@ -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<GeorchestraUser> 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<ExtendedGeorchestraUser> findByUsername(@NonNull String serviceName, @NonNull String username) {
UsersApi usersApi = usersByConfigName.get(serviceName);
Objects.requireNonNull(usersApi, () -> "No UsersApi found for config named " + serviceName);
Optional<GeorchestraUser> user = usersApi.findByUsername(username);

return extend(serviceName, user);
}

public Optional<ExtendedGeorchestraUser> findByEmail(@NonNull String serviceName, @NonNull String email) {
UsersApi usersApi = usersByConfigName.get(serviceName);
Objects.requireNonNull(usersApi, () -> "No UsersApi found for config named " + serviceName);
Optional<GeorchestraUser> user = usersApi.findByEmail(email);
return extend(serviceName, user);
}

public Optional<GeorchestraUser> 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<ExtendedGeorchestraUser> extend(String serviceName, Optional<GeorchestraUser> 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));
}

}
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,15 @@
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;
import org.georchestra.gateway.security.GeorchestraUserMapperExtension;
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;
Expand Down Expand Up @@ -107,25 +106,44 @@ private GeorchestraLdapAuthenticationProvider createLdapProvider(ExtendedLdapCon

@Bean
DemultiplexingUsersApi demultiplexingUsersApi(List<ExtendedLdapConfig> configs) {
Map<String, UsersApi> targets = new HashMap<>();
Map<String, UsersApi> usersByConfigName = new HashMap<>();
Map<String, OrganizationsApi> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ Optional<GeorchestraUser> map(GeorchestraUserNamePasswordAuthenticationToken tok
final String ldapConfigName = token.getConfigName();
final String username = principal.getUsername();

Optional<GeorchestraUser> user = users.findByUsername(ldapConfigName, username);
Optional<ExtendedGeorchestraUser> user = users.findByUsername(ldapConfigName, username);
if (user.isEmpty()) {
user = users.findByEmail(ldapConfigName, username);
}
Expand Down
2 changes: 1 addition & 1 deletion gateway/src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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();

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
}

Expand Down Expand Up @@ -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));
Expand All @@ -107,7 +112,8 @@ void testLdapUserDetails() {
Optional<GeorchestraUser> 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);
Expand Down
Loading

0 comments on commit 95ee6ec

Please sign in to comment.