Cosmetic fixes + test enhancement

italiangrid · Nov 15, 2024 · baae7b5 · baae7b5
1 parent de6f1ea
commit baae7b5
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 17 deletions.
diff --git 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<Jwt, Collection<GrantedAuthority>> {
 
-  @Autowired
   public StormJwtAuthoritiesConverter(StorageAreaConfiguration conf,
       ServiceConfigurationProperties props) {
     super(conf, props);
@@ -111,7 +109,7 @@ protected Collection<GrantedAuthority> 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
@@ -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<GrantedAuthority> 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<String> clientIdClaim =
+        Optional.ofNullable(userAuthority.getIdToken().getClaim("client_id"));
+    if (clientIdClaim.isPresent()) {
+      authorities.add(new JwtClientAuthority(idTokenIssuer, clientIdClaim.get()));
     }
 
     return authorities;

diff --git a/...va/org/italiangrid/storm/webdav/test/authz/integration/AuthorizationIntegrationTests.java b/...va/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
@@ -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