diff --git a/src/main/java/org/italiangrid/storm/webdav/oauth/StormJwtAuthoritiesConverter.java b/src/main/java/org/italiangrid/storm/webdav/oauth/StormJwtAuthoritiesConverter.java index b6bc8eaa..6a68bd03 100644 --- a/src/main/java/org/italiangrid/storm/webdav/oauth/StormJwtAuthoritiesConverter.java +++ b/src/main/java/org/italiangrid/storm/webdav/oauth/StormJwtAuthoritiesConverter.java @@ -29,7 +29,6 @@ import org.italiangrid.storm.webdav.oauth.authority.JwtIssuerAuthority; import org.italiangrid.storm.webdav.oauth.authority.JwtScopeAuthority; import org.italiangrid.storm.webdav.oauth.authority.JwtSubjectAuthority; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.convert.converter.Converter; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.oauth2.jwt.Jwt; @@ -42,7 +41,6 @@ public class StormJwtAuthoritiesConverter extends GrantedAuthoritiesMapperSupport implements Converter> { - @Autowired public StormJwtAuthoritiesConverter(StorageAreaConfiguration conf, ServiceConfigurationProperties props) { super(conf, props); @@ -111,7 +109,7 @@ protected Collection extractAuthorities(Jwt jwt) { authorities.add(new JwtIssuerAuthority(issuer)); authorities.add(new JwtSubjectAuthority(issuer, jwt.getSubject())); - if(jwt.getClaim("client_id") != null) { + if (jwt.getClaim("client_id") != null) { authorities.add(new JwtClientAuthority(issuer, jwt.getClaim("client_id"))); } diff --git a/src/main/java/org/italiangrid/storm/webdav/oidc/OidcGrantedAuthoritiesMapper.java b/src/main/java/org/italiangrid/storm/webdav/oidc/OidcGrantedAuthoritiesMapper.java index 21cc2fa6..72a0e6fd 100644 --- a/src/main/java/org/italiangrid/storm/webdav/oidc/OidcGrantedAuthoritiesMapper.java +++ b/src/main/java/org/italiangrid/storm/webdav/oidc/OidcGrantedAuthoritiesMapper.java @@ -17,6 +17,7 @@ import java.util.Collection; import java.util.List; +import java.util.Optional; import java.util.Set; import org.italiangrid.storm.webdav.config.ServiceConfigurationProperties; @@ -67,8 +68,10 @@ protected Collection mapAuthorities(OidcUserAuthority userAuth authorities.addAll(grantGroupAuthorities(userAuthority)); authorities.add(new JwtIssuerAuthority(idTokenIssuer)); authorities.add(new JwtSubjectAuthority(idTokenIssuer, userAuthority.getIdToken().getSubject())); - if(userAuthority.getIdToken().getClaim("client_id") != null) { - authorities.add(new JwtClientAuthority(idTokenIssuer, userAuthority.getIdToken().getClaim("client_id"))); + Optional clientIdClaim = + Optional.ofNullable(userAuthority.getIdToken().getClaim("client_id")); + if (clientIdClaim.isPresent()) { + authorities.add(new JwtClientAuthority(idTokenIssuer, clientIdClaim.get())); } return authorities; diff --git a/src/test/java/org/italiangrid/storm/webdav/test/authz/integration/AuthorizationIntegrationTests.java b/src/test/java/org/italiangrid/storm/webdav/test/authz/integration/AuthorizationIntegrationTests.java index 67851d89..db4c60c4 100644 --- a/src/test/java/org/italiangrid/storm/webdav/test/authz/integration/AuthorizationIntegrationTests.java +++ b/src/test/java/org/italiangrid/storm/webdav/test/authz/integration/AuthorizationIntegrationTests.java @@ -57,7 +57,8 @@ public class AuthorizationIntegrationTests { public static final String UNKNOWN_ISSUER = "https://unknown.example"; public static final String EXAMPLE_ISSUER = "https://issuer.example"; - public static final String JWT_CLIENT_ID = "1234"; + public static final String AUTHORIZED_JWT_CLIENT_ID = "1234"; + public static final String UNAUTHORIZED_JWT_CLIENT_ID = "5678"; @Autowired @@ -184,7 +185,7 @@ void readAccessAsJwtWithAllowedClient() throws Exception { Jwt token = Jwt.withTokenValue("test") .header("kid", "rsa1") .issuer(EXAMPLE_ISSUER) - .claim("client_id", JWT_CLIENT_ID) + .claim("client_id", AUTHORIZED_JWT_CLIENT_ID) .build(); mvc.perform(get(SLASH_WLCG_SLASH_FILE).with(jwt().jwt(token).authorities(authConverter))) @@ -197,7 +198,7 @@ void readAccessWithoutMatchedJWTIsDenied() throws Exception { Jwt token = Jwt.withTokenValue("test") .header("kid", "rsa1") .issuer(EXAMPLE_ISSUER) - .claim("client_id", "5678") + .claim("client_id", UNAUTHORIZED_JWT_CLIENT_ID) .build(); mvc.perform(get(SLASH_WLCG_SLASH_FILE).with(jwt().jwt(token).authorities(authConverter))) @@ -206,7 +207,7 @@ void readAccessWithoutMatchedJWTIsDenied() throws Exception { token = Jwt.withTokenValue("test") .header("kid", "rsa1") .issuer(UNKNOWN_ISSUER) - .claim("client_id", "1234") + .claim("client_id", AUTHORIZED_JWT_CLIENT_ID) .build(); mvc.perform(get(SLASH_WLCG_SLASH_FILE).with(jwt().jwt(token).authorities(authConverter))) @@ -215,7 +216,7 @@ void readAccessWithoutMatchedJWTIsDenied() throws Exception { token = Jwt.withTokenValue("test") .header("kid", "rsa1") .issuer(UNKNOWN_ISSUER) - .claim("client_id", "5678") + .claim("client_id", UNAUTHORIZED_JWT_CLIENT_ID) .build(); mvc.perform(get(SLASH_WLCG_SLASH_FILE).with(jwt().jwt(token).authorities(authConverter))) @@ -233,7 +234,7 @@ void writeAccessAsJwtWithAllowedClient() throws Exception { Jwt token = Jwt.withTokenValue("test") .header("kid", "rsa1") .issuer(EXAMPLE_ISSUER) - .claim("client_id", JWT_CLIENT_ID) + .claim("client_id", AUTHORIZED_JWT_CLIENT_ID) .build(); mvc.perform(put(SLASH_WLCG_SLASH_FILE).with(jwt().jwt(token).authorities(authConverter))) @@ -246,7 +247,7 @@ void writeAccessWithoutMatchedJWTIsDenied() throws Exception { Jwt token = Jwt.withTokenValue("test") .header("kid", "rsa1") .issuer(EXAMPLE_ISSUER) - .claim("client_id", "5678") + .claim("client_id", UNAUTHORIZED_JWT_CLIENT_ID) .build(); mvc.perform(put(SLASH_WLCG_SLASH_FILE).with(jwt().jwt(token).authorities(authConverter))) @@ -255,7 +256,7 @@ void writeAccessWithoutMatchedJWTIsDenied() throws Exception { token = Jwt.withTokenValue("test") .header("kid", "rsa1") .issuer(UNKNOWN_ISSUER) - .claim("client_id", "1234") + .claim("client_id", AUTHORIZED_JWT_CLIENT_ID) .build(); mvc.perform(put(SLASH_WLCG_SLASH_FILE).with(jwt().jwt(token).authorities(authConverter))) @@ -264,7 +265,7 @@ void writeAccessWithoutMatchedJWTIsDenied() throws Exception { token = Jwt.withTokenValue("test") .header("kid", "rsa1") .issuer(UNKNOWN_ISSUER) - .claim("client_id", "5678") + .claim("client_id", UNAUTHORIZED_JWT_CLIENT_ID) .build(); mvc.perform(put(SLASH_WLCG_SLASH_FILE).with(jwt().jwt(token).authorities(authConverter))) diff --git a/src/test/java/org/italiangrid/storm/webdav/test/authz/pdp/AuthzPdpTests.java b/src/test/java/org/italiangrid/storm/webdav/test/authz/pdp/AuthzPdpTests.java index d0e1118b..342326fc 100644 --- a/src/test/java/org/italiangrid/storm/webdav/test/authz/pdp/AuthzPdpTests.java +++ b/src/test/java/org/italiangrid/storm/webdav/test/authz/pdp/AuthzPdpTests.java @@ -58,6 +58,8 @@ public class AuthzPdpTests { public static final String TEST_ISSUER = "https://test.example"; public static final String TEST2_ISSUER = "https://test2.example"; + public static final String AUTHORIZED_JWT_CLIENT_ID = "1234"; + public static final String UNAUTHORIZED_JWT_CLIENT_ID = "5678"; @Mock PathAuthorizationPolicyRepository repo; @@ -267,12 +269,12 @@ void oauthClientHolderPolicyApplied() { when(request.getMethod()).thenReturn("GET"); when(authentication.getAuthorities()) - .thenReturn(authorities(new JwtClientAuthority(TEST_ISSUER, "1234"))); + .thenReturn(authorities(new JwtClientAuthority(TEST_ISSUER, AUTHORIZED_JWT_CLIENT_ID))); PathAuthorizationPolicy oauthTestPolicy = PathAuthorizationPolicy.builder() .withPermit() - .withPrincipalMatcher( - AuthorityHolder.fromAuthority(new JwtClientAuthority(TEST_ISSUER, "1234"))) + .withPrincipalMatcher(AuthorityHolder + .fromAuthority(new JwtClientAuthority(TEST_ISSUER, AUTHORIZED_JWT_CLIENT_ID))) .withRequestMatcher(new AntPathRequestMatcher("/test/**", "GET")) .build(); @@ -290,6 +292,12 @@ void oauthClientHolderPolicyApplied() { assertThat(result.getDecision(), is(PERMIT)); assertThat(result.getPolicy().isPresent(), is(true)); assertThat(result.getPolicy().get(), is(oauthTestPolicy)); + + when(authentication.getAuthorities()) + .thenReturn(authorities(new JwtClientAuthority(TEST_ISSUER, UNAUTHORIZED_JWT_CLIENT_ID))); + + result = pdp.authorizeRequest(newAuthorizationRequest(request, authentication)); + assertThat(result.getDecision(), is(DENY)); } @Test