diff --git a/gateway/src/main/java/org/georchestra/gateway/security/accessrules/AccessRulesConfiguration.java b/gateway/src/main/java/org/georchestra/gateway/security/accessrules/AccessRulesConfiguration.java index ec4c74ac..1063147b 100644 --- a/gateway/src/main/java/org/georchestra/gateway/security/accessrules/AccessRulesConfiguration.java +++ b/gateway/src/main/java/org/georchestra/gateway/security/accessrules/AccessRulesConfiguration.java @@ -19,6 +19,7 @@ package org.georchestra.gateway.security.accessrules; import org.georchestra.gateway.model.GatewayConfigProperties; +import org.georchestra.gateway.security.GeorchestraUserMapper; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -28,7 +29,8 @@ public class AccessRulesConfiguration { @Bean - AccessRulesCustomizer georchestraAccessRulesCustomizer(GatewayConfigProperties config) { - return new AccessRulesCustomizer(config); + AccessRulesCustomizer georchestraAccessRulesCustomizer(GatewayConfigProperties config, + GeorchestraUserMapper userMapper) { + return new AccessRulesCustomizer(config, userMapper); } } diff --git a/gateway/src/main/java/org/georchestra/gateway/security/accessrules/AccessRulesCustomizer.java b/gateway/src/main/java/org/georchestra/gateway/security/accessrules/AccessRulesCustomizer.java index 119e3107..0b3b8d54 100644 --- a/gateway/src/main/java/org/georchestra/gateway/security/accessrules/AccessRulesCustomizer.java +++ b/gateway/src/main/java/org/georchestra/gateway/security/accessrules/AccessRulesCustomizer.java @@ -25,6 +25,7 @@ import org.georchestra.gateway.model.GatewayConfigProperties; import org.georchestra.gateway.model.RoleBasedAccessRule; import org.georchestra.gateway.model.Service; +import org.georchestra.gateway.security.GeorchestraUserMapper; import org.georchestra.gateway.security.ServerHttpSecurityCustomizer; import org.springframework.security.config.web.server.ServerHttpSecurity; import org.springframework.security.config.web.server.ServerHttpSecurity.AuthorizeExchangeSpec; @@ -57,6 +58,7 @@ public class AccessRulesCustomizer implements ServerHttpSecurityCustomizer { private final @NonNull GatewayConfigProperties config; + private final @NonNull GeorchestraUserMapper userMapper; @Override public void customize(ServerHttpSecurity http) { @@ -145,7 +147,11 @@ void requireAuthenticatedUser(Access access) { @VisibleForTesting void hasAnyAuthority(Access access, List roles) { - access.hasAnyAuthority(roles.toArray(String[]::new)); + // Checks against the effective set of rules (both provided by the Authorization + // service and derived from roles mappings) + access.access( + GeorchestraUserRolesAuthorizationManager.hasAnyAuthority(userMapper, roles.toArray(String[]::new))); + // access.hasAnyAuthority(roles.toArray(String[]::new)); } @VisibleForTesting diff --git a/gateway/src/main/java/org/georchestra/gateway/security/accessrules/GeorchestraUserRolesAuthorizationManager.java b/gateway/src/main/java/org/georchestra/gateway/security/accessrules/GeorchestraUserRolesAuthorizationManager.java new file mode 100644 index 00000000..95ecac78 --- /dev/null +++ b/gateway/src/main/java/org/georchestra/gateway/security/accessrules/GeorchestraUserRolesAuthorizationManager.java @@ -0,0 +1,159 @@ +/* + * Copyright (C) 2022 by the geOrchestra PSC + * + * This file is part of geOrchestra. + * + * geOrchestra is free software: you can redistribute it and/or modify it under + * the terms of the GNU General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * geOrchestra is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * geOrchestra. If not, see . + */ +package org.georchestra.gateway.security.accessrules; + +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Stream; + +import org.georchestra.gateway.security.GeorchestraUserMapper; +import org.georchestra.security.model.GeorchestraUser; +import org.springframework.security.authorization.AuthorityAuthorizationDecision; +import org.springframework.security.authorization.AuthorityReactiveAuthorizationManager; +import org.springframework.security.authorization.AuthorizationDecision; +import org.springframework.security.authorization.ReactiveAuthorizationManager; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.authority.AuthorityUtils; +import org.springframework.util.Assert; + +import com.google.common.annotations.VisibleForTesting; + +import reactor.core.publisher.Mono; + +/** + * Variant of {@link AuthorityReactiveAuthorizationManager} that + * {@link #check(Mono, Object) checks} access based on the effectively resolved + * set of role names in a {@link GeorchestraUser}, as opposed to only on the + * {@link Authentication#getAuthorities() Authenticateion authorities}. + *

+ * This is so because the authorization provider (e.g. OAuth2/OIDC) returns an + * {@link Authentication} object from which {@link GeorchestraUserMapper} will + * derive additional roles to be sent to the downstream services as the + * {@code sec-roles} header, and we need to also account for those derived role + * names when granting access to an URI (see {@link AccessRulesCustomizer#apply} + * and {@link AccessRulesCustomizer#hasAnyAuthority}). + */ +class GeorchestraUserRolesAuthorizationManager implements ReactiveAuthorizationManager { + + private final GeorchestraUserMapper userMapper; + private final List authorities; + private final Set authorityFilter; + private final AuthorityAuthorizationDecision unauthorized; + + GeorchestraUserRolesAuthorizationManager(GeorchestraUserMapper userMapper, String... authorities) { + this.userMapper = userMapper; + this.authorities = AuthorityUtils.createAuthorityList(authorities); + this.authorityFilter = Set.of(authorities); + this.unauthorized = new AuthorityAuthorizationDecision(false, this.authorities); + } + + @Override + public Mono check(Mono authentication, T object) { + return authentication.map(this::authorize).map( + (granted) -> ((AuthorizationDecision) new AuthorityAuthorizationDecision(granted, this.authorities))) + .defaultIfEmpty(unauthorized); + } + + @VisibleForTesting + boolean authorize(Authentication authentication) { + if (!authentication.isAuthenticated()) { + return false; + } + Optional user = userMapper.resolve(authentication); + Stream effectiveRoles = user.map(GeorchestraUser::getRoles).map(List::stream).orElse(Stream.empty()); + Stream grandtedAuthorities = authentication.getAuthorities().stream() + .map(GrantedAuthority::getAuthority); + + return Stream.concat(effectiveRoles, grandtedAuthorities).sorted().distinct() + .anyMatch(authorityFilter::contains); + } + + /** + * Creates an instance of {@link GeorchestraUserRolesAuthorizationManager} with + * the provided authority. + * + * @param authority the authority to check for + * @param the type of object being authorized + * @return the new instance + */ + public static GeorchestraUserRolesAuthorizationManager hasAuthority(GeorchestraUserMapper userMapper, + String authority) { + Assert.notNull(authority, "authority cannot be null"); + return new GeorchestraUserRolesAuthorizationManager<>(userMapper, authority); + } + + /** + * Creates an instance of {@link GeorchestraUserRolesAuthorizationManager} with + * the provided authorities. + * + * @param authorities the authorities to check for + * @param the type of object being authorized + * @return the new instance + */ + public static GeorchestraUserRolesAuthorizationManager hasAnyAuthority(GeorchestraUserMapper userMapper, + String... authorities) { + Assert.notNull(authorities, "authorities cannot be null"); + for (String authority : authorities) { + Assert.notNull(authority, "authority cannot be null"); + } + return new GeorchestraUserRolesAuthorizationManager<>(userMapper, authorities); + } + + /** + * Creates an instance of {@link GeorchestraUserRolesAuthorizationManager} with + * the provided authority. + * + * @param role the authority to check for prefixed with "ROLE_" + * @param the type of object being authorized + * @return the new instance + */ + public static GeorchestraUserRolesAuthorizationManager hasRole(GeorchestraUserMapper userMapper, + String role) { + Assert.notNull(role, "role cannot be null"); + return hasAuthority(userMapper, "ROLE_" + role); + } + + /** + * Creates an instance of {@link GeorchestraUserRolesAuthorizationManager} with + * the provided authorities. + * + * @param roles the authorities to check for prefixed with "ROLE_" + * @param the type of object being authorized + * @return the new instance + */ + public static GeorchestraUserRolesAuthorizationManager hasAnyRole(GeorchestraUserMapper userMapper, + String... roles) { + Assert.notNull(roles, "roles cannot be null"); + for (String role : roles) { + Assert.notNull(role, "role cannot be null"); + } + return hasAnyAuthority(userMapper, toNamedRolesArray(roles)); + } + + private static String[] toNamedRolesArray(String... roles) { + String[] result = new String[roles.length]; + for (int i = 0; i < roles.length; i++) { + result[i] = "ROLE_" + roles[i]; + } + return result; + } + +} diff --git a/gateway/src/test/java/org/georchestra/gateway/security/accessrules/AccessRulesCustomizerTest.java b/gateway/src/test/java/org/georchestra/gateway/security/accessrules/AccessRulesCustomizerTest.java index 6ca35035..1727aaeb 100644 --- a/gateway/src/test/java/org/georchestra/gateway/security/accessrules/AccessRulesCustomizerTest.java +++ b/gateway/src/test/java/org/georchestra/gateway/security/accessrules/AccessRulesCustomizerTest.java @@ -39,6 +39,7 @@ import org.georchestra.gateway.model.GatewayConfigProperties; import org.georchestra.gateway.model.RoleBasedAccessRule; import org.georchestra.gateway.model.Service; +import org.georchestra.gateway.security.GeorchestraUserMapper; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; @@ -58,14 +59,16 @@ class AccessRulesCustomizerTest { @BeforeEach void setUp() throws Exception { config = new GatewayConfigProperties(); - customizer = new AccessRulesCustomizer(config); + customizer = new AccessRulesCustomizer(config, new GeorchestraUserMapper(List.of(), List.of())); http = spy(new ServerHttpSecurity() { }); } @Test void testConstructorDoesNotAcceptNullConfig() { - assertThrows(NullPointerException.class, () -> new AccessRulesCustomizer(null)); + assertThrows(NullPointerException.class, + () -> new AccessRulesCustomizer(null, new GeorchestraUserMapper(List.of(), List.of()))); + assertThrows(NullPointerException.class, () -> new AccessRulesCustomizer(config, null)); } @Test diff --git a/gateway/src/test/java/org/georchestra/gateway/security/accessrules/GeorchestraUserRolesAuthorizationManagerTest.java b/gateway/src/test/java/org/georchestra/gateway/security/accessrules/GeorchestraUserRolesAuthorizationManagerTest.java new file mode 100644 index 00000000..75a9fa6e --- /dev/null +++ b/gateway/src/test/java/org/georchestra/gateway/security/accessrules/GeorchestraUserRolesAuthorizationManagerTest.java @@ -0,0 +1,101 @@ +/* + * Copyright (C) 2022 by the geOrchestra PSC + * + * This file is part of geOrchestra. + * + * geOrchestra is free software: you can redistribute it and/or modify it under + * the terms of the GNU General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * geOrchestra is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * geOrchestra. If not, see . + */ +package org.georchestra.gateway.security.accessrules; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.Optional; + +import org.georchestra.gateway.security.GeorchestraUserMapper; +import org.georchestra.security.model.GeorchestraUser; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.security.authentication.TestingAuthenticationToken; + +class GeorchestraUserRolesAuthorizationManagerTest { + + GeorchestraUserMapper userMapper; + GeorchestraUser user; + GeorchestraUserRolesAuthorizationManager authManager; + + @BeforeEach + void setup() { + userMapper = mock(GeorchestraUserMapper.class); + user = new GeorchestraUser(); + when(userMapper.resolve(any())).thenReturn(Optional.of(user)); + + authManager = GeorchestraUserRolesAuthorizationManager.hasAnyAuthority(userMapper, "GDI_ADMIN", "SUPERUSER", + "ROLE_ADMIN"); + } + + private TestingAuthenticationToken authentication(String... roles) { + return new TestingAuthenticationToken("gabe", null, roles); + } + + @Test + void hasAnyAuthority_notAuthenticated() { + TestingAuthenticationToken authentication = authentication(); + authentication.setAuthenticated(false); + assertThat(authManager.authorize(authentication)).isFalse(); + } + + @Test + void hasAnyAuthority() { + TestingAuthenticationToken authentication = authentication("ROLE_USER"); + user.setRoles(List.of("ROLE_USER", "GDI_ADMIN")); + assertThat(authManager.authorize(authentication)).isTrue(); + + user.setRoles(List.of("ROLE_USER", "SUPERUSER")); + assertThat(authManager.authorize(authentication)).isTrue(); + + user.setRoles(List.of("ROLE_USER", "ROLE_ADMIN")); + assertThat(authManager.authorize(authentication)).isTrue(); + + user.setRoles(List.of("ROLE_USER")); + assertThat(authManager.authorize(authentication)).isFalse(); + } + + @Test + void hasAnyAuthority_joins_user_and_authentication_authorities() { + TestingAuthenticationToken authentication = authentication("GDI_ADMIN"); + user.setRoles(List.of("ROLE_USER")); + assertThat(authManager.authorize(authentication)).isTrue(); + } + + @Test + void hasAnyAuthority_noResolvedUser_nor_grantedAuthorities() { + TestingAuthenticationToken authentication = authentication(); + when(userMapper.resolve(any())).thenReturn(Optional.empty()); + + assertThat(authManager.authorize(authentication)).isFalse(); + } + + @Test + void hasAnyAuthority_noResolvedUser_resolved_grantedAuthorities() { + TestingAuthenticationToken authentication = authentication("GDI_ADMIN"); + when(userMapper.resolve(any())).thenReturn(Optional.empty()); + + assertThat(authManager.authorize(authentication)).isTrue(); + } + +}