diff --git a/api/src/main/java/app/quickcase/spring/oidc/authentication/QuickcaseClientAuthentication.java b/api/src/main/java/app/quickcase/spring/oidc/authentication/QuickcaseClientAuthentication.java index 7593985..63ee44d 100644 --- a/api/src/main/java/app/quickcase/spring/oidc/authentication/QuickcaseClientAuthentication.java +++ b/api/src/main/java/app/quickcase/spring/oidc/authentication/QuickcaseClientAuthentication.java @@ -3,7 +3,6 @@ import java.util.Collection; import java.util.Optional; import java.util.Set; -import java.util.stream.Collectors; import app.quickcase.spring.oidc.AccessLevel; import app.quickcase.spring.oidc.SecurityClassification; @@ -23,12 +22,17 @@ private static OrganisationProfile clientProfile() { } private final String clientId; - - public QuickcaseClientAuthentication(String accessToken, - String clientId, - Collection authorities) { + private final Set roles; + + public QuickcaseClientAuthentication( + String accessToken, + String clientId, + Collection authorities, + Set roles + ) { super(authorities, accessToken); this.clientId = clientId; + this.roles = roles; this.setAuthenticated(true); } @@ -54,9 +58,7 @@ public String getName() { @Override public Set getRoles() { - return getAuthorities().stream() - .map(GrantedAuthority::getAuthority) - .collect(Collectors.toSet()); + return roles; } @Override diff --git a/api/src/main/java/app/quickcase/spring/oidc/authentication/converter/AccessTokenAuthenticationConverter.java b/api/src/main/java/app/quickcase/spring/oidc/authentication/converter/AccessTokenAuthenticationConverter.java index 3983016..10bd34c 100644 --- a/api/src/main/java/app/quickcase/spring/oidc/authentication/converter/AccessTokenAuthenticationConverter.java +++ b/api/src/main/java/app/quickcase/spring/oidc/authentication/converter/AccessTokenAuthenticationConverter.java @@ -7,11 +7,11 @@ import app.quickcase.spring.oidc.authentication.QuickcaseUserAuthentication; import app.quickcase.spring.oidc.claims.ClaimsParser; import app.quickcase.spring.oidc.claims.JwtClaimsParser; +import app.quickcase.spring.oidc.userinfo.UserInfo; import app.quickcase.spring.oidc.userinfo.UserInfoExtractor; - import org.springframework.security.oauth2.jwt.Jwt; -import static app.quickcase.spring.oidc.utils.StringUtils.authorities; +import static app.quickcase.spring.oidc.authentication.converter.QuickcaseAuthenticationConverter.authorities; import static app.quickcase.spring.oidc.utils.StringUtils.fromSpaceSeparated; /** @@ -48,12 +48,13 @@ private QuickcaseAuthentication clientAuthentication(Jwt source) { final String accessToken = source.getTokenValue(); final String subject = source.getSubject(); final Set scopes = fromSpaceSeparated(source.getClaimAsString("scope")); - return new QuickcaseClientAuthentication(accessToken, subject, authorities(scopes)); + return new QuickcaseClientAuthentication(accessToken, subject, authorities(scopes), scopes); } private QuickcaseAuthentication userAuthentication(Jwt source) { final ClaimsParser claims = new JwtClaimsParser(source.getClaims()); final Set scopes = fromSpaceSeparated(source.getClaimAsString("scope")); - return new QuickcaseUserAuthentication(source.getTokenValue(), authorities(scopes), userInfoExtractor.extract(claims)); + final UserInfo userInfo = userInfoExtractor.extract(claims); + return new QuickcaseUserAuthentication(source.getTokenValue(), authorities(scopes, userInfo.getRoles()), userInfo); } } diff --git a/api/src/main/java/app/quickcase/spring/oidc/authentication/converter/QuickcaseAuthenticationConverter.java b/api/src/main/java/app/quickcase/spring/oidc/authentication/converter/QuickcaseAuthenticationConverter.java index 181718b..82deda9 100644 --- a/api/src/main/java/app/quickcase/spring/oidc/authentication/converter/QuickcaseAuthenticationConverter.java +++ b/api/src/main/java/app/quickcase/spring/oidc/authentication/converter/QuickcaseAuthenticationConverter.java @@ -1,9 +1,35 @@ package app.quickcase.spring.oidc.authentication.converter; -import app.quickcase.spring.oidc.authentication.QuickcaseAuthentication; +import java.util.Set; +import java.util.stream.Stream; +import app.quickcase.spring.oidc.authentication.QuickcaseAuthentication; import org.springframework.core.convert.converter.Converter; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.oauth2.jwt.Jwt; +import static java.util.stream.Collectors.toSet; + public interface QuickcaseAuthenticationConverter extends Converter { + static String prefixScope(String scope) { + return "SCOPE_" + scope; + } + + static String prefixRole(String role) { + return "ROLE_" + role; + } + + static Set authorities(Set scopes) { + return authorities(scopes, Set.of()); + } + + static Set authorities(Set scopes, Set roles) { + return Stream.concat( + scopes.stream().map(QuickcaseAuthenticationConverter::prefixScope), + roles.stream().map(QuickcaseAuthenticationConverter::prefixRole) + ) + .map(SimpleGrantedAuthority::new) + .collect(toSet()); + } } diff --git a/api/src/main/java/app/quickcase/spring/oidc/authentication/converter/UserInfoAuthenticationConverter.java b/api/src/main/java/app/quickcase/spring/oidc/authentication/converter/UserInfoAuthenticationConverter.java index 779535d..ca020ee 100644 --- a/api/src/main/java/app/quickcase/spring/oidc/authentication/converter/UserInfoAuthenticationConverter.java +++ b/api/src/main/java/app/quickcase/spring/oidc/authentication/converter/UserInfoAuthenticationConverter.java @@ -7,10 +7,9 @@ import app.quickcase.spring.oidc.authentication.QuickcaseUserAuthentication; import app.quickcase.spring.oidc.userinfo.UserInfo; import app.quickcase.spring.oidc.userinfo.UserInfoService; - import org.springframework.security.oauth2.jwt.Jwt; -import static app.quickcase.spring.oidc.utils.StringUtils.authorities; +import static app.quickcase.spring.oidc.authentication.converter.QuickcaseAuthenticationConverter.authorities; import static app.quickcase.spring.oidc.utils.StringUtils.fromSpaceSeparated; public class UserInfoAuthenticationConverter implements QuickcaseAuthenticationConverter { @@ -43,7 +42,7 @@ private QuickcaseAuthentication clientAuthentication(Jwt source) { final String accessToken = source.getTokenValue(); final String subject = source.getSubject(); final Set scopes = fromSpaceSeparated(source.getClaimAsString("scope")); - return new QuickcaseClientAuthentication(accessToken, subject, authorities(scopes)); + return new QuickcaseClientAuthentication(accessToken, subject, authorities(scopes), scopes); } private QuickcaseUserAuthentication userAuthentication(Jwt source) { @@ -52,6 +51,6 @@ private QuickcaseUserAuthentication userAuthentication(Jwt source) { final Set scopes = fromSpaceSeparated(source.getClaimAsString("scope")); final UserInfo userInfo = userInfoService.loadUserInfo(subject, accessToken); - return new QuickcaseUserAuthentication(accessToken, authorities(scopes), userInfo); + return new QuickcaseUserAuthentication(accessToken, authorities(scopes, userInfo.getRoles()), userInfo); } } diff --git a/api/src/test/java/app/quickcase/spring/oidc/authentication/QuickcaseClientAuthenticationTest.java b/api/src/test/java/app/quickcase/spring/oidc/authentication/QuickcaseClientAuthenticationTest.java index f70b9fa..88d59f3 100644 --- a/api/src/test/java/app/quickcase/spring/oidc/authentication/QuickcaseClientAuthenticationTest.java +++ b/api/src/test/java/app/quickcase/spring/oidc/authentication/QuickcaseClientAuthenticationTest.java @@ -111,7 +111,8 @@ void getOrganisationProfile() { } private QuickcaseAuthentication clientAuthentication() { - final Set authorities = StringUtils.authorities("ROLE-1", "ROLE-2"); - return new QuickcaseClientAuthentication(ACCESS_TOKEN, CLIENT_ID, authorities); + final Set scopes = Set.of("ROLE-1", "ROLE-2"); + final Set authorities = StringUtils.authorities(scopes); + return new QuickcaseClientAuthentication(ACCESS_TOKEN, CLIENT_ID, authorities, scopes); } } \ No newline at end of file diff --git a/api/src/test/java/app/quickcase/spring/oidc/authentication/converter/AccessTokenAuthenticationConverterTest.java b/api/src/test/java/app/quickcase/spring/oidc/authentication/converter/AccessTokenAuthenticationConverterTest.java index 7407757..edce115 100644 --- a/api/src/test/java/app/quickcase/spring/oidc/authentication/converter/AccessTokenAuthenticationConverterTest.java +++ b/api/src/test/java/app/quickcase/spring/oidc/authentication/converter/AccessTokenAuthenticationConverterTest.java @@ -8,7 +8,6 @@ import app.quickcase.spring.oidc.userinfo.UserInfo; import app.quickcase.spring.oidc.userinfo.UserInfoExtractor; import app.quickcase.spring.oidc.userinfo.UserPreferences; - import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; @@ -18,9 +17,7 @@ import org.springframework.security.oauth2.jwt.Jwt; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.*; class AccessTokenAuthenticationConverterTest { private static final String ACCESS_TOKEN = "token123"; @@ -84,12 +81,22 @@ void shouldGetIdFromClient() { } @Test - @DisplayName("should use scopes as authorities") + @DisplayName("should use scopes as prefixed authorities") void shouldUseScopesAsAuthorities() { final QuickcaseAuthentication authentication = clientAuthentication(); - final GrantedAuthority[] scopes = authorities(SCOPE_1, SCOPE_2); - assertThat(authentication.getAuthorities(), containsInAnyOrder(scopes)); + assertThat(authentication.getAuthorities(), containsInAnyOrder(authorities( + "SCOPE_" + SCOPE_1, + "SCOPE_" + SCOPE_2 + ))); + } + + @Test + @DisplayName("should use scopes as roles") + void shouldUseScopesAsRoles() { + final QuickcaseAuthentication authentication = clientAuthentication(); + + assertThat(authentication.getRoles(), containsInAnyOrder(SCOPE_1, SCOPE_2)); } @Test @@ -131,6 +138,27 @@ private QuickcaseAuthentication userAuthentication(String openidScope) { return converter.convert(jwt); } + @Test + @DisplayName("should combine prefixed scopes and roles as authorities") + void shouldUseRolesAsAuthorities() { + final QuickcaseAuthentication authentication = userAuthentication(); + + assertThat(authentication.getAuthorities(), containsInAnyOrder(authorities( + "SCOPE_openid", + "SCOPE_" + SCOPE_2, + "ROLE_" + ROLE_1, + "ROLE_" + ROLE_2 + ))); + } + + @Test + @DisplayName("should expose user roles") + void shouldUseScopesAsRoles() { + final QuickcaseAuthentication authentication = userAuthentication(); + + assertThat(authentication.getRoles(), containsInAnyOrder(ROLE_1, ROLE_2)); + } + @Test @DisplayName("should populate user authentication from access token") void shouldGetIdFromUser() { @@ -141,7 +169,6 @@ void shouldGetIdFromUser() { assertThat(authentication.getId(), equalTo(USER_ID)); assertThat(authentication.getName(), equalTo(USER_NAME)); assertThat(authentication.getEmail().get(), equalTo(USER_EMAIL)); - assertThat(authentication.getAuthorities(), containsInAnyOrder(authorities(AccessTokenAuthenticationConverter.OPENID_SCOPE, SCOPE_2))); assertThat(authentication.getRoles(), containsInAnyOrder(ROLE_1, ROLE_2)); assertThat(authentication.getUserInfo() .get() diff --git a/api/src/test/java/app/quickcase/spring/oidc/authentication/converter/UserInfoAuthenticationConverterTest.java b/api/src/test/java/app/quickcase/spring/oidc/authentication/converter/UserInfoAuthenticationConverterTest.java index 02ccbdd..5ee6829 100644 --- a/api/src/test/java/app/quickcase/spring/oidc/authentication/converter/UserInfoAuthenticationConverterTest.java +++ b/api/src/test/java/app/quickcase/spring/oidc/authentication/converter/UserInfoAuthenticationConverterTest.java @@ -48,7 +48,7 @@ void setUp() { userInfoServiceStub = (sub, token) -> UserInfo.builder(sub) .name(USER_NAME) .email(USER_EMAIL) - .authorities(ROLE_1, ROLE_2) + .roles(ROLE_1, ROLE_2) .organisationProfile("org-a", orgA) .build(); @@ -79,12 +79,22 @@ void shouldGetIdFromClient() { } @Test - @DisplayName("should use scopes as authorities") + @DisplayName("should use scopes as prefixed authorities") void shouldUseScopesAsAuthorities() { final QuickcaseAuthentication authentication = clientAuthentication(); - final GrantedAuthority[] scopes = authorities(SCOPE_1, SCOPE_2); - assertThat(authentication.getAuthorities(), containsInAnyOrder(scopes)); + assertThat(authentication.getAuthorities(), containsInAnyOrder(authorities( + "SCOPE_" + SCOPE_1, + "SCOPE_" + SCOPE_2 + ))); + } + + @Test + @DisplayName("should use scopes as roles") + void shouldUseScopesAsRoles() { + final QuickcaseAuthentication authentication = clientAuthentication(); + + assertThat(authentication.getRoles(), containsInAnyOrder(SCOPE_1, SCOPE_2)); } @Test @@ -158,11 +168,24 @@ void shouldHaveUserInfo() { } @Test - @DisplayName("should use scopes as authorities") + @DisplayName("should combine prefixed scopes and roles as authorities") void shouldUseRolesAsAuthorities() { final QuickcaseAuthentication authentication = userAuthentication(); - assertThat(authentication.getAuthorities(), containsInAnyOrder(authorities("openid", SCOPE_2))); + assertThat(authentication.getAuthorities(), containsInAnyOrder(authorities( + "SCOPE_openid", + "SCOPE_" + SCOPE_2, + "ROLE_" + ROLE_1, + "ROLE_" + ROLE_2 + ))); + } + + @Test + @DisplayName("should expose user roles") + void shouldUseScopesAsRoles() { + final QuickcaseAuthentication authentication = userAuthentication(); + + assertThat(authentication.getRoles(), containsInAnyOrder(ROLE_1, ROLE_2)); } @Test