Skip to content
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.

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

fix: geOrchestra json headers - organization as json payload is not transmitted #82

Merged
merged 1 commit into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading