[java-idp-plugin-oidc-rp] branch main updated: JOIDCRP-68 - Claim sanitization fails with ill behaving OP

[Phil Smart]

This is an automated email from the git hooks/post-receive script.

philsmart pushed a commit to branch main
in repository java-idp-plugin-oidc-rp.

View the commit online:
http://git.shibboleth.net/view/?p=java-idp-plugin-oidc-rp.git;a=commit;h=8e0885c060a92f970c048163dd7fad674ed194f5

The following commit(s) were added to refs/heads/main by this push:
     new 8e0885c  JOIDCRP-68 - Claim sanitization fails with ill behaving OP
8e0885c is described below

commit 8e0885c060a92f970c048163dd7fad674ed194f5
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Wed Oct 9 11:20:07 2024 +0100

    JOIDCRP-68 - Claim sanitization fails with ill behaving OP
    
     - Remove claims with null values
    
    https://shibboleth.atlassian.net/browse/JOIDCRP-68
---
 .../rp/impl/DefaultClaimSanitizationStrategy.java  |  9 ++++---
 .../impl/DefaultClaimSanitizationStrategyTest.java | 31 ++++++++++++++++++++++
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/DefaultClaimSanitizationStrategy.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/DefaultClaimSanitizationStrategy.java
index c0421ce..4f266d7 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/DefaultClaimSanitizationStrategy.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/DefaultClaimSanitizationStrategy.java
@@ -28,8 +28,9 @@ import net.shibboleth.oidc.security.jwt.claims.impl.IDTokenClaims;
 import net.shibboleth.oidc.security.jwt.claims.impl.JWTClaims;
 
 /**
- * Produce a claims set from the JWT claims set without the validation claims, leaving the identity,
- * authorization, and misc. claims.
+ * Produce a claims set from the JWT claims set without either the validation claims or claims with null values.
+ * Leaving the identity, authorization, and misc. claims.
+ * 
  */
 public class DefaultClaimSanitizationStrategy implements UnaryOperator<ClaimsSet> {
     
@@ -60,7 +61,9 @@ public class DefaultClaimSanitizationStrategy implements UnaryOperator<ClaimsSet
         final ClaimsSet sanitizedClaims = new ClaimsSet();
         final Map<String, Object> filteredMap = jwtClaims.toJSONObject().entrySet()
             .stream()
-            .filter(c -> !validationClaims.contains(c.getKey()))
+            .filter(c -> c.getValue() != null)
+            .filter(c -> !validationClaims.contains(c.getKey()))            
+            .filter(c -> c.getKey() != null)
                 .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
         sanitizedClaims.putAll(filteredMap);
         return sanitizedClaims;
diff --git a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/DefaultClaimSanitizationStrategyTest.java b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/DefaultClaimSanitizationStrategyTest.java
index 4c9598d..bf567b7 100644
--- a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/DefaultClaimSanitizationStrategyTest.java
+++ b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/DefaultClaimSanitizationStrategyTest.java
@@ -60,5 +60,36 @@ public class DefaultClaimSanitizationStrategyTest {
         assertEquals(sanClaims.getClaim(JWTClaims.SUBJECT_CLAIM.getClaimName()),"joe");
         assertEquals(sanClaims.getClaim("given_name"),"Joe");
     }
+    
+    @Test
+    public void testNullValidationClaim() {
+        final DefaultClaimSanitizationStrategy strategy = new DefaultClaimSanitizationStrategy();
+        
+        final JWTClaimsSet idToken = new JWTClaimsSet.Builder()
+                     .subject("joe")
+                     .issuer("https://op.example.com")
+                     .audience(List.of("https://rp.example.com"))
+                     .issueTime(Date.from(Instant.now()))
+                     .expirationTime(Date.from(Instant.now().plus(Duration.ofSeconds(10))))
+                     .claim(IDTokenClaims.NONCE.getClaimName(), "noncevalue")
+                     // add a null claim here
+                     .claim("given_name", null)
+                     .claim("surname", "bloggs")
+                     .build();
+        
+        final ClaimsSet idTokenClaims = new ClaimsSet();
+        idTokenClaims.putAll(idToken.getClaims());
+        final ClaimsSet sanClaims = strategy.apply(idTokenClaims);
+        assertNull(sanClaims.getAudience());
+        assertNull(sanClaims.getIssuer());
+        assertNull(sanClaims.getClaim(IDTokenClaims.NONCE.getClaimName()));
+        assertNull(sanClaims.getClaim(JWTClaims.EXPIRATION_TIME_CLAIM.getClaimName()));
+        assertNull(sanClaims.getClaim(JWTClaims.ISSUED_AT_CLAIM.getClaimName()));
+        // check the null claim has been removed
+        assertNull(sanClaims.getClaim("given_name"));
+        // Check a non-null claim still exists
+        assertEquals(sanClaims.getClaim("surname"), "bloggs");
+
+    }
 
 }

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.