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

ldap - avoid providing sensitive (hashed password) on the /whoami endpoint #110

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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 @@ -38,7 +38,8 @@ public interface AccountManager {
* @param mappedUser the user {@link ResolveGeorchestraUserGlobalFilter}
* resolved by calling
* {@link GeorchestraUserMapper#resolve(Authentication)}
* @return the stored version of the user if it exists, otherwise an empty Optional
* @return the stored version of the user if it exists, otherwise an empty
* Optional
*/
Optional<GeorchestraUser> find(GeorchestraUser mappedUser);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package org.georchestra.gateway.security.ldap;

import org.springframework.security.ldap.userdetails.LdapUserDetailsMapper;

public class NoPasswordLdapUserDetailsMapper extends LdapUserDetailsMapper {

@Override
protected String mapPassword(Object passwordValue) {
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static java.util.Objects.requireNonNull;

import org.georchestra.ds.users.AccountDao;
import org.georchestra.gateway.security.ldap.NoPasswordLdapUserDetailsMapper;
import org.georchestra.gateway.security.ldap.extended.ExtendedLdapAuthenticationProvider;
import org.georchestra.gateway.security.ldap.extended.ExtendedPasswordPolicyAwareContextSource;
import org.springframework.ldap.core.support.BaseLdapPathContextSource;
Expand Down Expand Up @@ -72,7 +73,7 @@ public ExtendedLdapAuthenticationProvider build() {

final GrantedAuthoritiesMapper rolesMapper = ldapAuthoritiesMapper();
provider.setAuthoritiesMapper(rolesMapper);
provider.setUserDetailsContextMapper(new LdapUserDetailsMapper());
provider.setUserDetailsContextMapper(new NoPasswordLdapUserDetailsMapper());
provider.setAccountDao(accountDao);
return provider;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import org.springframework.context.ApplicationContext;
import org.springframework.ldap.NameNotFoundException;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.test.context.DynamicPropertyRegistry;
import org.springframework.test.context.DynamicPropertySource;
import org.springframework.test.web.reactive.server.WebTestClient;

import java.util.Map;
Expand Down Expand Up @@ -46,6 +48,12 @@ public class CreateAccountUserCustomizerIT {
ldap.stop();
}

@DynamicPropertySource
static void registerLdap(DynamicPropertyRegistry registry) {
registry.add("testcontainers.georchestra.ldap.host", () -> "127.0.0.1");
registry.add("testcontainers.georchestra.ldap.port", ldap::getMappedLdapPort);
}

private static final Map<String, String> NOT_EXISTING_ACCOUNT_HEADERS = Map.of( //
"sec-georchestra-preauthenticated", "true", //
"preauth-username", "pmartin", //
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
import org.springframework.boot.test.autoconfigure.web.reactive.AutoConfigureWebTestClient;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.test.context.DynamicPropertyRegistry;
import org.springframework.test.context.DynamicPropertySource;
import org.springframework.test.web.reactive.server.WebTestClient;

@SpringBootTest(classes = GeorchestraGatewayApplication.class)
Expand All @@ -34,6 +36,12 @@ class PreauthHttpHeadersBase64EncodedCreateAccountIT {
ldap.stop();
}

@DynamicPropertySource
static void registerLdap(DynamicPropertyRegistry registry) {
registry.add("testcontainers.georchestra.ldap.host", () -> "127.0.0.1");
registry.add("testcontainers.georchestra.ldap.port", ldap::getMappedLdapPort);
}

@Test
void testPreauthenticatedHeaders_AccentedChars() throws Exception {
testClient.get().uri("/whoami")//
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package org.georchestra.gateway.security.ldap.basic;

import org.georchestra.gateway.app.GeorchestraGatewayApplication;
import org.georchestra.testcontainers.ldap.GeorchestraLdapContainer;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Disabled;
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.test.context.ActiveProfiles;
import org.springframework.test.web.reactive.server.WebTestClient;

@SpringBootTest(classes = GeorchestraGatewayApplication.class)
@ActiveProfiles({ "basicldap" })
@AutoConfigureWebTestClient(timeout = "PT20S")
@Disabled("ExtendedLdapAuthenticationProvider being built instead of a Basic one after https://github.com/georchestra/georchestra-gateway/pull/50/files ?")
public class BasicLdapAuthenticationIT {

public static GeorchestraLdapContainer ldap = new GeorchestraLdapContainer();

private @Autowired WebTestClient testClient;

public static @BeforeAll void startUpContainers() {
ldap.start();
}

public static @AfterAll void shutDownContainers() {
ldap.stop();
}

public @Test void testWhoamiNoPasswordRevealed() {
testClient.get().uri("/whoami")//
.header("Authorization", "Basic dGVzdGFkbWluOnRlc3RhZG1pbg==") // testadmin:testadmin
.exchange()//
.expectStatus()//
.is2xxSuccessful()//
.expectBody(String.class)//
.value(Matchers.not(Matchers.containsString("{SHA}")));
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package org.georchestra.gateway.security.ldap.extended;

import org.georchestra.gateway.app.GeorchestraGatewayApplication;
import org.georchestra.gateway.filter.headers.providers.JsonPayloadHeadersContributor;
import org.georchestra.gateway.model.GatewayConfigProperties;
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.context.DynamicPropertyRegistry;
import org.springframework.test.context.DynamicPropertySource;
import org.springframework.test.web.reactive.server.WebTestClient;
import org.springframework.web.context.WebApplicationContext;
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)
@ActiveProfiles({ "createaccount" })
@AutoConfigureWebTestClient(timeout = "PT20S")
public class ExtendedLdapAuthenticationIT {
public static GeorchestraLdapContainer ldap = new GeorchestraLdapContainer();

private @Autowired WebTestClient testClient;

public static @BeforeAll void startUpContainers() {
ldap.start();
}

public static @AfterAll void shutDownContainers() {
ldap.stop();
}

@DynamicPropertySource
static void registerLdap(DynamicPropertyRegistry registry) {
registry.add("testcontainers.georchestra.ldap.host", () -> "127.0.0.1");
registry.add("testcontainers.georchestra.ldap.port", ldap::getMappedLdapPort);
}

public @Test void testWhoami() {
testClient.get().uri("/whoami")//
.header("Authorization", "Basic dGVzdGFkbWluOnRlc3RhZG1pbg==") // testadmin:testadmin
.exchange()//
.expectStatus()//
.is2xxSuccessful()//
.expectBody()//
.jsonPath("$.GeorchestraUser.username").isEqualTo("testadmin");
}

public @Test void testWhoamiUsingEmail() {
testClient.get().uri("/whoami")//
.header("Authorization", "Basic cHNjK3Rlc3RhZG1pbkBnZW9yY2hlc3RyYS5vcmc6dGVzdGFkbWlu") // [email protected]:testadmin
.exchange()//
.expectStatus()//
.is2xxSuccessful()//
.expectBody()//
.jsonPath("$.GeorchestraUser.username").isEqualTo("testadmin");
}

public @Test void testWhoamiNoPasswordRevealed() {
testClient.get().uri("/whoami")//
.header("Authorization", "Basic dGVzdGFkbWluOnRlc3RhZG1pbg==") // testadmin:testadmin
.exchange()//
.expectStatus()//
.is2xxSuccessful()//
.expectBody()//
.jsonPath(
"$.['org.georchestra.gateway.security.ldap.extended.GeorchestraUserNamePasswordAuthenticationToken'].principal.password")
.isEmpty();
}

}
39 changes: 39 additions & 0 deletions gateway/src/test/resources/application-basicldap.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# This YAML file configures the geOrchestra gateway with
# a basic LDAP (extended set to false).
#
georchestra:
gateway:
default-headers:
# Default security headers to append to proxied requests
proxy: true
username: true
roles: true
org: true
orgname: true
global-access-rules:
- intercept-url:
- "/**"
- "/proxy/?url=*"
anonymous: true
security:
ldap:
default:
enabled: true
extended: false
url: ldap://${ldapHost}:${ldapPort}/
baseDn: dc=georchestra,dc=org
adminDn: cn=admin,dc=georchestra,dc=org
adminPassword: secret
spring:
cloud:
gateway:
enabled: true
default-filters:
- SecureHeaders
- TokenRelay
- RemoveSecurityHeaders
# AddSecHeaders appends sec-* headers to proxied requests based on the
# georchestra.gateway.default-headers and georchestra.gateway.servies.<service>.headers config properties
- AddSecHeaders
httpclient.wiretap: true
httpserver.wiretap: false
8 changes: 7 additions & 1 deletion gateway/src/test/resources/application-createaccount.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
# This YAML file configures the geOrchestra gateway with
# * a geOrchestra extended LDAP
# * authentication via HTTP headers
#
# It is mainly used for integration testing the preauthentication mechanisms.
#
georchestra:
gateway:
default-headers:
Expand All @@ -21,7 +27,7 @@ georchestra:
default:
enabled: true
extended: true
url: ldap://${ldapHost}:${ldapPort}/
url: ldap://${testcontainers.georchestra.ldap.host}:${testcontainers.georchestra.ldap.port}/
baseDn: dc=georchestra,dc=org
adminDn: cn=admin,dc=georchestra,dc=org
adminPassword: secret
Expand Down
Loading