From 596b8c161113321e26f646920e2fae706b45dbe7 Mon Sep 17 00:00:00 2001 From: David Darras <117278497+davdarras@users.noreply.github.com> Date: Thu, 28 Nov 2024 13:35:29 +0100 Subject: [PATCH] fix: allow multiple app roles for same oidc token role (#97) --- .../security/GrantedAuthorityConverter.java | 36 +++-- .../GrantedAuthorityConverterTest.java | 133 ++++++++++++++++++ pom.xml | 2 +- 3 files changed, 159 insertions(+), 12 deletions(-) create mode 100644 platine-management-api/src/test/java/fr/insee/survey/datacollectionmanagement/configuration/auth/security/GrantedAuthorityConverterTest.java diff --git a/platine-management-api/src/main/java/fr/insee/survey/datacollectionmanagement/configuration/auth/security/GrantedAuthorityConverter.java b/platine-management-api/src/main/java/fr/insee/survey/datacollectionmanagement/configuration/auth/security/GrantedAuthorityConverter.java index 66b6fb3..aac51c7 100644 --- a/platine-management-api/src/main/java/fr/insee/survey/datacollectionmanagement/configuration/auth/security/GrantedAuthorityConverter.java +++ b/platine-management-api/src/main/java/fr/insee/survey/datacollectionmanagement/configuration/auth/security/GrantedAuthorityConverter.java @@ -15,12 +15,12 @@ @AllArgsConstructor public class GrantedAuthorityConverter implements Converter> { - private final Map grantedRoles; + private final Map> roles; ApplicationConfig applicationConfig; public GrantedAuthorityConverter(ApplicationConfig applicationConfig) { this.applicationConfig = applicationConfig; - this.grantedRoles = new HashMap<>(); + this.roles = new HashMap<>(); fillGrantedRoles(applicationConfig.getRoleAdmin(), AuthorityRoleEnum.ADMIN); fillGrantedRoles(applicationConfig.getRoleRespondent(), AuthorityRoleEnum.RESPONDENT); fillGrantedRoles(applicationConfig.getRoleInternalUser(), AuthorityRoleEnum.INTERNAL_USER); @@ -31,23 +31,37 @@ public GrantedAuthorityConverter(ApplicationConfig applicationConfig) { @Override public Collection convert(@NonNull Jwt jwt) { Map claims = jwt.getClaims(); - List roles = (List) claims.get(applicationConfig.getRoleClaim()); + List userRoles = (List) claims.get(applicationConfig.getRoleClaim()); - return roles.stream() + if(userRoles == null) { + return new ArrayList<>(); + } + + return userRoles.stream() .filter(Objects::nonNull) .filter(role -> !role.isBlank()) - .filter(grantedRoles::containsKey) - .map(grantedRoles::get) + .filter(roles::containsKey) + .map(roles::get) + .flatMap(Collection::stream) + .distinct() .collect(Collectors.toCollection(ArrayList::new)); } - private void fillGrantedRoles(List listRoles, AuthorityRoleEnum roleEnum) { + private void fillGrantedRoles(List configRoles, AuthorityRoleEnum authorityRole) { - for (String role : listRoles ) { - this.grantedRoles.putIfAbsent(role, - new SimpleGrantedAuthority(roleEnum.securityRole())); - } + for (String configRole : configRoles ) { + if(configRole == null || configRole.isBlank()) { + return; + } + this.roles.compute(configRole, (key, grantedAuthorities) -> { + if(grantedAuthorities == null) { + grantedAuthorities = new ArrayList<>(); + } + grantedAuthorities.add(new SimpleGrantedAuthority(authorityRole.securityRole())); + return grantedAuthorities; + }); + } } } diff --git a/platine-management-api/src/test/java/fr/insee/survey/datacollectionmanagement/configuration/auth/security/GrantedAuthorityConverterTest.java b/platine-management-api/src/test/java/fr/insee/survey/datacollectionmanagement/configuration/auth/security/GrantedAuthorityConverterTest.java new file mode 100644 index 0000000..cefe9b8 --- /dev/null +++ b/platine-management-api/src/test/java/fr/insee/survey/datacollectionmanagement/configuration/auth/security/GrantedAuthorityConverterTest.java @@ -0,0 +1,133 @@ +package fr.insee.survey.datacollectionmanagement.configuration.auth.security; + +import fr.insee.survey.datacollectionmanagement.configuration.ApplicationConfig; +import fr.insee.survey.datacollectionmanagement.constants.AuthorityRoleEnum; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.oauth2.jwt.Jwt; + +import java.time.Instant; +import java.util.*; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; + +class GrantedAuthorityConverterTest { + + private GrantedAuthorityConverter converter; + + private ApplicationConfig applicationConfig; + + private static final List JWT_ROLE_INTERNAL_USER = List.of("internal_user", "intern_user"); + private static final List JWT_ROLE_RESPONDENT = List.of("respondent", "resp"); + private static final List JWT_ROLE_ADMIN = List.of("admin", "adm"); + private static final List JWT_ROLE_WEBCLIENT = List.of("webclient","webcli"); + + @BeforeEach + void init() { + applicationConfig = new ApplicationConfig(); + } + + @Test + @DisplayName("Given a JWT, when converting null or empty JWT role, then converting ignore these roles") + void testConverter01() { + applicationConfig.setRoleAdmin(List.of("")); + applicationConfig.setRoleInternalUser(List.of()); + applicationConfig.setRoleWebClient(List.of("")); + List nullList = new ArrayList<>(); + nullList.add(null); + applicationConfig.setRoleRespondent(nullList); + converter = new GrantedAuthorityConverter(applicationConfig); + List tokenRoles = new ArrayList<>(); + tokenRoles.add(null); + tokenRoles.add(""); + + Jwt jwt = createJwt(tokenRoles); + Collection authorities = converter.convert(jwt); + assertThat(authorities).isEmpty(); + } + + @Test + @DisplayName("Given a JWT, when converting roles, then convert only JWT roles matching roles in role properties") + void testConverter02() { + applicationConfig.setRoleAdmin(JWT_ROLE_ADMIN); + applicationConfig.setRoleInternalUser(JWT_ROLE_INTERNAL_USER); + applicationConfig.setRoleWebClient(List.of("webclient", "adm")); + applicationConfig.setRoleRespondent(JWT_ROLE_RESPONDENT); + converter = new GrantedAuthorityConverter(applicationConfig); + List tokenRoles = List.of("dummyRole1", "internal_user", "dummyRole2", "webclient", "adm"); + + Jwt jwt = createJwt(tokenRoles); + Collection authorities = converter.convert(jwt); + assertThat(authorities) + .hasSize(3) + .containsExactlyInAnyOrder( + new SimpleGrantedAuthority(AuthorityRoleEnum.INTERNAL_USER.securityRole()), + new SimpleGrantedAuthority(AuthorityRoleEnum.ADMIN.securityRole()), + new SimpleGrantedAuthority(AuthorityRoleEnum.WEB_CLIENT.securityRole())); + } + + @Test + @DisplayName("Given a JWT, when converting roles, then accept a config role can be used for multiple app roles") + void testConverter03() { + String dummyRole = "dummyRole"; + String dummyRole2 = "dummyRole2"; + applicationConfig.setRoleAdmin(List.of(dummyRole)); + applicationConfig.setRoleInternalUser(List.of(dummyRole2)); + applicationConfig.setRoleWebClient(List.of(dummyRole)); + applicationConfig.setRoleRespondent(List.of("")); + converter = new GrantedAuthorityConverter(applicationConfig); + + List tokenRoles = List.of(dummyRole, "role-not-used", dummyRole2, "role-not-used-2"); + Jwt jwt = createJwt(tokenRoles); + + Collection authorities = converter.convert(jwt); + assertThat(authorities) + .hasSize(3) + .contains( + new SimpleGrantedAuthority(AuthorityRoleEnum.INTERNAL_USER.securityRole()), + new SimpleGrantedAuthority(AuthorityRoleEnum.ADMIN.securityRole()), + new SimpleGrantedAuthority(AuthorityRoleEnum.WEB_CLIENT.securityRole())); + } + + @ParameterizedTest + @MethodSource("provideJWTRoleWithAppRoleAssociated") + @DisplayName("Given a JWT, when converting roles, then assure each JWT role is converted to equivalent app role") + void testConverter04(List jwtRoles, AuthorityRoleEnum appRole) { + applicationConfig.setRoleAdmin(JWT_ROLE_ADMIN); + applicationConfig.setRoleInternalUser(JWT_ROLE_INTERNAL_USER); + applicationConfig.setRoleWebClient(JWT_ROLE_WEBCLIENT); + applicationConfig.setRoleRespondent(JWT_ROLE_RESPONDENT); + converter = new GrantedAuthorityConverter(applicationConfig); + + Jwt jwt = createJwt(jwtRoles); + Collection authorities = converter.convert(jwt); + assertThat(authorities) + .hasSize(1) + .contains(new SimpleGrantedAuthority(appRole.securityRole())); + } + + private static Stream provideJWTRoleWithAppRoleAssociated() { + return Stream.of( + Arguments.of(JWT_ROLE_INTERNAL_USER, AuthorityRoleEnum.INTERNAL_USER), + Arguments.of(JWT_ROLE_ADMIN, AuthorityRoleEnum.ADMIN), + Arguments.of(JWT_ROLE_WEBCLIENT, AuthorityRoleEnum.WEB_CLIENT), + Arguments.of(JWT_ROLE_RESPONDENT, AuthorityRoleEnum.RESPONDENT)); + } + + private Jwt createJwt(List tokenRoles) { + Map jwtHeaders = new HashMap<>(); + jwtHeaders.put("header", "headerValue"); + + Map claims = new HashMap<>(); + claims.put(applicationConfig.getRoleClaim(), tokenRoles); + + return new Jwt("user-id", Instant.now(), Instant.MAX, jwtHeaders, claims); + } +} diff --git a/pom.xml b/pom.xml index 2ea9773..cb59e29 100644 --- a/pom.xml +++ b/pom.xml @@ -24,7 +24,7 @@ platine-management-api - 2.7.1 + 2.7.2 21 21