[java-oidc-common] branch main updated: JCOMOIDC-54 - Add untrusted audience validation to validator

Phil Smart philip.smart at jisc.ac.uk
Thu Dec 15 13:27:29 UTC 2022


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

philsmart pushed a commit to branch main
in repository java-oidc-common.

View the commit online:
http://git.shibboleth.net/view/?p=java-oidc-common.git;a=commit;h=3fa67faa7ead2a08eeab2ca39dc9e71c4a07f9fb

The following commit(s) were added to refs/heads/main by this push:
     new 3fa67fa  JCOMOIDC-54 - Add untrusted audience validation to validator
3fa67fa is described below

commit 3fa67faa7ead2a08eeab2ca39dc9e71c4a07f9fb
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Thu Dec 15 13:27:27 2022 +0000

    JCOMOIDC-54 - Add untrusted audience validation to validator
    
     - Add additional audiences check
     - Add flag to enable the check. Disabled by default.
    
    https://shibboleth.atlassian.net/browse/JCOMOIDC-54
---
 .../jwt/claims/impl/AudienceClaimsValidator.java   | 117 +++++++++++++--------
 .../claims/impl/AudienceClaimsValidatorTest.java   | 109 +++++++++++++------
 2 files changed, 146 insertions(+), 80 deletions(-)

diff --git a/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/jwt/claims/impl/AudienceClaimsValidator.java b/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/jwt/claims/impl/AudienceClaimsValidator.java
index b0fc706..58aeb69 100644
--- a/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/jwt/claims/impl/AudienceClaimsValidator.java
+++ b/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/jwt/claims/impl/AudienceClaimsValidator.java
@@ -21,6 +21,7 @@ package net.shibboleth.oidc.security.jwt.claims.impl;
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
+import java.util.Set;
 import java.util.function.BiFunction;
 import java.util.stream.Collectors;
 
@@ -49,17 +50,24 @@ public class AudienceClaimsValidator extends AbstractClaimsValidator{
     @NonnullAfterInit private BiFunction<ProfileRequestContext,JWTClaimsSet, String> audienceLookupStrategy;
     
     /** 
-     * A strategy to lookup a list of untrusted audiences that, if contained in the 'aud' claim, 
-     * should invalidate the token.
+     * A strategy to lookup a list of additional audiences expected in the 'aud' claim. If they are not found, 
+     * the token is rejected.
      */
-    @Nonnull private BiFunction<ProfileRequestContext, JWTClaimsSet, List<String>> untrustedAudiencesLookupStrategy;
+    @Nonnull private BiFunction<ProfileRequestContext, JWTClaimsSet, Set<String>> additionalAudiencesLookupStrategy;
     
     /** Allow a missing aud claim. */
     private boolean allowMissing;
     
+    /** 
+     * If enabled, any extra audience must be contained in the List returned from the additionalAudiencesLookupStrategy.
+     * Defaults to {@literal false}.
+     */
+    private boolean extraAudienceValidation;
+    
     /** Constructor. */
     public AudienceClaimsValidator() {
-        untrustedAudiencesLookupStrategy = (prc, claims) -> Collections.emptyList();
+        additionalAudiencesLookupStrategy = (prc, claims) -> Collections.emptySet();
+        extraAudienceValidation = false;
     }
     
     /** {@inheritDoc} */
@@ -85,49 +93,48 @@ public class AudienceClaimsValidator extends AbstractClaimsValidator{
     }
     
     /**
-     * Set the strategy used to lookup the list of untrusted audiences. 
+     * Set the strategy used to lookup the list of additional audiences to find in the 'aud' claim. 
      * 
      * @param strategy the strategy
      * 
      * @since 2.2.0
      */
-    public void setUntrustedAudiencesLookupStrategy(
-            @Nonnull final BiFunction<ProfileRequestContext, JWTClaimsSet, List<String>> strategy) {
+    public void setAdditionalAudiencesLookupStrategy(
+            @Nonnull final BiFunction<ProfileRequestContext, JWTClaimsSet, Set<String>> strategy) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
         ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
 
-        untrustedAudiencesLookupStrategy = Constraint.isNotNull(strategy,
-                "Untrusted audiences lookup strategy can not be null");
+        additionalAudiencesLookupStrategy = Constraint.isNotNull(strategy,
+                "Additional audiences lookup strategy can not be null");
     }
     
     /**
-     * Set a list of untrusted audiences. Can be empty but never {@literal null}.
+     * Set whether a missing 'aud' claim is valid or not.
      * 
-     * @param untrusted the list of untrusted audiences
+     * <p>Defaults to false.</p>
      * 
-     * @since 2.2.0
+     * @param flag flag to set
+     * 
+     * @since 2.1.0
      */
-    public void setUntrustedAudiences(@Nonnull final List<String> untrusted) {
-        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
-        
-        Constraint.isNotNull(untrusted, "Untrusted audiences list can not be null");
-        
-        untrustedAudiencesLookupStrategy = (prc, claims) -> 
-            Collections.unmodifiableList(untrusted.stream().filter(Objects::nonNull).collect(Collectors.toList()));
+    public void setAllowMissing(final boolean flag) {
+        allowMissing = flag;
     }
     
     /**
-     * Set whether a missing 'aud' claim is valid or not.
+     * Set whether to enable extra audience validation.
      * 
      * <p>Defaults to false.</p>
      * 
-     * @param flag flag to set
+     * @param flag the flag to set
      * 
-     * @since 2.1.0
+     * @since 2.2.0
      */
-    public void setAllowMissing(final boolean flag) {
-        allowMissing = flag;
+    public void setExtraAudienceValidation(final boolean flag) {
+        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
+
+        extraAudienceValidation = flag;
     }
 
     @Override
@@ -139,32 +146,50 @@ public class AudienceClaimsValidator extends AbstractClaimsValidator{
             throw new JWTValidationException("Audience value not returned by lookup function");
         }
         
-        List<String> untrustedAudiences = untrustedAudiencesLookupStrategy.apply(context, claims);
-        if (untrustedAudiences == null) {
-            untrustedAudiences = Collections.emptyList();
+        Set<String> additionalAudiences = additionalAudiencesLookupStrategy.apply(context, claims);
+        if (additionalAudiences == null) {
+            additionalAudiences = Collections.emptySet();
         }
         
-        final List<String> audList = claims.getAudience();        
+        final List<String> audList = claims.getAudience();  
         
-        if (audList != null && !audList.isEmpty()) {
-            boolean audMatch = false;
-            boolean untrustedMatch = false;
-            for (final String aud : audList) {
-                if (aud == null) {
-                    continue;
-                }
-                if (acceptedAudience.equals(aud)) {
-                    audMatch = true;
-                }
-                if (untrustedAudiences.contains(aud)) {
-                    untrustedMatch = true;
-                }
+        if ((audList == null || audList.isEmpty())) {
+            if (!allowMissing) {
+                // No audience but an audience is required
+                throw new JWTValidationException("JWT missing required audience");
+            } else {
+                // No audience, but that is allowable, so nothing to validate
+                return;
             }
-            if (!audMatch || untrustedMatch) {
-                throw new JWTValidationException("JWT audience rejected: " + audList);
+        }
+        
+        // Filter nulls from audList
+        final List<String> audListFiltered = audList.stream().filter(Objects::nonNull).collect(Collectors.toList());       
+               
+        final boolean acceptedAudienceMatch = audListFiltered.contains(acceptedAudience);
+
+        if (!acceptedAudienceMatch) {
+            throw new JWTValidationException("JWT audience rejected, accepted audience not found in: " 
+                    + audListFiltered);
+        }
+        
+        // If there is more than one audience and validation is enabled, we must have these listed 
+        // explicitly in additionalAudiences. 
+        if (audListFiltered.size() > 1 && extraAudienceValidation) {
+            
+            // Remove the accepted audience
+            final List<String> audListFilteredWithoutAcceptedAud = 
+                    audListFiltered.stream().filter(aud -> !aud.equals(acceptedAudience)).collect(Collectors.toList());
+            
+            // Test all additional audiences are known or trusted
+            final boolean additionalAudiencesTrusted = 
+                    audListFilteredWithoutAcceptedAud.stream().allMatch(additionalAudiences::contains);
+        
+            if (!additionalAudiencesTrusted) {
+                throw new JWTValidationException(
+                        "JWT audience rejected, additional audiences not trusted: " 
+                                + audListFilteredWithoutAcceptedAud);
             }
-        } else if (!allowMissing) {
-            throw new JWTValidationException("JWT missing required audience");
         }
     }
 
diff --git a/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/jwt/claims/impl/AudienceClaimsValidatorTest.java b/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/jwt/claims/impl/AudienceClaimsValidatorTest.java
index 67babd4..ed9ca1f 100644
--- a/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/jwt/claims/impl/AudienceClaimsValidatorTest.java
+++ b/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/jwt/claims/impl/AudienceClaimsValidatorTest.java
@@ -20,6 +20,7 @@ package net.shibboleth.oidc.security.jwt.claims.impl;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Set;
 
 import javax.annotation.Nonnull;
 
@@ -30,7 +31,6 @@ import com.nimbusds.jwt.JWTClaimsSet;
 
 import net.shibboleth.oidc.jwt.claims.JWTValidationException;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
-import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
 
 /** Test for the {@link AudienceClaimsValidator}. */
 public class AudienceClaimsValidatorTest extends AbstractClaimsValidatorTest {
@@ -47,135 +47,176 @@ public class AudienceClaimsValidatorTest extends AbstractClaimsValidatorTest {
     }
     
     @Test
-    public void doValidateTest() throws JWTValidationException, ComponentInitializationException {
+    public void doValidTest() throws JWTValidationException, ComponentInitializationException {
         final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder().audience("client-id").build();
         validator.setId("test-validator");
         validator.setAudienceLookupStrategy((prc,claims) -> "client-id");
+        validator.setExtraAudienceValidation(true);
         validator.initialize();
         validator.validate(claimsSet, prc);
     }
     
-    @Test
-    public void doValidateTestMultipleAudiences() throws JWTValidationException, ComponentInitializationException {
+    @Test(expectedExceptions = JWTValidationException.class)
+    public void doRejectedTest_MultipleAudiencesNotTrusted() 
+            throws JWTValidationException, ComponentInitializationException {
         final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder().
                 audience(List.of("client-id","another-audience")).build();
         validator.setId("test-validator");
         validator.setAudienceLookupStrategy((prc,claims) -> "client-id");
+        validator.setExtraAudienceValidation(true);
         validator.initialize();
         validator.validate(claimsSet, prc);
     }
     
     @Test(expectedExceptions = JWTValidationException.class)
-    public void doInValidateTestMultipleAudiences() throws JWTValidationException, ComponentInitializationException {
+    public void doRejectedTest_MultipleAudiences() 
+            throws JWTValidationException, ComponentInitializationException {
         final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder().
                 audience(List.of("wrong-client","another-audience")).build();
         validator.setId("test-validator");
         validator.setAudienceLookupStrategy((prc,claims) -> "client-id");
+        validator.setExtraAudienceValidation(true);
         validator.initialize();
         validator.validate(claimsSet, prc);
     }
     
     @Test(expectedExceptions = JWTValidationException.class)
-    public void doInValidateTestEmptyAudiences() throws JWTValidationException, ComponentInitializationException {
+    public void doRejectedTest_EmptyAudiences() 
+            throws JWTValidationException, ComponentInitializationException {
         final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder().audience(Collections.emptyList()).build();
         validator.setId("test-validator");
         validator.setAudienceLookupStrategy((prc,claims) -> "client-id");
+        validator.setExtraAudienceValidation(true);
         validator.initialize();
         validator.validate(claimsSet, prc);
     }
     
     @Test(expectedExceptions = JWTValidationException.class)
-    public void doInValidateTest() throws JWTValidationException, ComponentInitializationException {
+    public void doRejectedTest() throws JWTValidationException, ComponentInitializationException {
         final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder().audience("client-id").build();
         validator.setId("test-validator");
         validator.setAudienceLookupStrategy((prc,claims) -> "client-id-different");
+        validator.setExtraAudienceValidation(false);
         validator.initialize();
         validator.validate(claimsSet, prc);
     }
-    
-    @Test(expectedExceptions = JWTValidationException.class)
-    public void doInValidateTest_UntrustedAudience() 
+        
+    @Test
+    public void doValidTest_NoAudienceAllow() 
             throws JWTValidationException, ComponentInitializationException {
         final JWTClaimsSet claimsSet = 
-                new JWTClaimsSet.Builder().audience(List.of("client-id", "untrusted-client")).build();
+                new JWTClaimsSet.Builder().build();
         validator.setId("test-validator");
         validator.setAudienceLookupStrategy((prc,claims) -> "client-id");
-        validator.setUntrustedAudiences(List.of("untrusted-client"));
+        validator.setExtraAudienceValidation(true);
+        validator.setAllowMissing(true);
         validator.initialize();
         validator.validate(claimsSet, prc);
     }
     
-    @Test
-    public void doValidateTest_UntrustedAudiencesNotInClaim() 
+    @Test(expectedExceptions = JWTValidationException.class)
+    public void doValidTest_NoAudienceDisallow() 
             throws JWTValidationException, ComponentInitializationException {
         final JWTClaimsSet claimsSet = 
-                new JWTClaimsSet.Builder().audience(List.of("client-id", "client-id-2")).build();
+                new JWTClaimsSet.Builder().build();
         validator.setId("test-validator");
         validator.setAudienceLookupStrategy((prc,claims) -> "client-id");
-        validator.setUntrustedAudiences(List.of("untrusted-client"));
+        validator.setExtraAudienceValidation(true);
+        validator.setAllowMissing(false);
         validator.initialize();
         validator.validate(claimsSet, prc);
     }
     
-    @Test(expectedExceptions = ConstraintViolationException.class)
-    public void doValidateTest_SetNullUntrustedAudienceList() 
+    @Test(expectedExceptions = JWTValidationException.class)
+    public void doValidTest_NullClaims() 
             throws JWTValidationException, ComponentInitializationException {
-        validator.setUntrustedAudiences(null);
+        final List<String> nullClaims = new ArrayList<>();
+        nullClaims.add(null);
+        final JWTClaimsSet claimsSet = 
+                new JWTClaimsSet.Builder().audience(nullClaims).build();
+        validator.setExtraAudienceValidation(true);
+        validator.setId("test-validator");
+        validator.setAudienceLookupStrategy((prc,claims) -> "client-id");
+        validator.initialize();
+        validator.validate(claimsSet, prc);
     }
     
     @Test
-    public void doValidateTest_NoAudienceAllow() 
+    public void doValidTest_AdditionalAudiencesNotInToken() 
             throws JWTValidationException, ComponentInitializationException {
         final JWTClaimsSet claimsSet = 
-                new JWTClaimsSet.Builder().build();
+                new JWTClaimsSet.Builder().audience("client-id").build();
         validator.setId("test-validator");
         validator.setAudienceLookupStrategy((prc,claims) -> "client-id");
-        validator.setAllowMissing(true);
+        // Token only contains one audience, so this is not going to be checked
+        validator.setAdditionalAudiencesLookupStrategy((prc, claims)-> Set.of("additional-client-1"));
+        validator.setExtraAudienceValidation(true);
         validator.initialize();
         validator.validate(claimsSet, prc);
     }
+
     
-    @Test(expectedExceptions = JWTValidationException.class)
-    public void doValidateTest_NoAudienceDisallow() 
+    @Test
+    public void doValidTest_AdditionalAudiencesFound() 
             throws JWTValidationException, ComponentInitializationException {
         final JWTClaimsSet claimsSet = 
-                new JWTClaimsSet.Builder().build();
+                new JWTClaimsSet.Builder().audience(
+                        List.of("client-id","additional-client-1","additional-client-2")).build();
         validator.setId("test-validator");
         validator.setAudienceLookupStrategy((prc,claims) -> "client-id");
-        validator.setAllowMissing(false);
+        validator.setAdditionalAudiencesLookupStrategy(
+                (prc, claims)-> Set.of("additional-client-1", "additional-client-2"));
+        validator.setExtraAudienceValidation(true);
         validator.initialize();
         validator.validate(claimsSet, prc);
     }
     
     @Test(expectedExceptions = JWTValidationException.class)
-    public void doValidateTest_NullClaims() 
+    public void doValidTest_AdditionalAudiencesFound_AcceptedAudienceNotFound() 
             throws JWTValidationException, ComponentInitializationException {
-        final List<String> nullClaims = new ArrayList<>();
-        nullClaims.add(null);
         final JWTClaimsSet claimsSet = 
-                new JWTClaimsSet.Builder().audience(nullClaims).build();
+                new JWTClaimsSet.Builder().audience(
+                        List.of("client-id","additional-client-1","additional-client-2")).build();
         validator.setId("test-validator");
-        validator.setAudienceLookupStrategy((prc,claims) -> "client-id");
-        validator.setUntrustedAudiences(List.of("untrusted-client"));
+        validator.setAudienceLookupStrategy((prc,claims) -> "client-id-not-found");
+        validator.setAdditionalAudiencesLookupStrategy(
+                (prc, claims)-> Set.of("additional-client-1", "additional-client-2"));
+        validator.setExtraAudienceValidation(true);
         validator.initialize();
         validator.validate(claimsSet, prc);
     }
     
     @Test(expectedExceptions = JWTValidationException.class)
-    public void doInValidateTestNullAudienceInContext() 
+    public void doRejectedTest_NullAudienceInContext() 
             throws JWTValidationException, ComponentInitializationException {
         final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder().audience("client-id").build();
         validator.setId("test-validator");
         validator.setAudienceLookupStrategy((prc,claims) -> null);
+        validator.setExtraAudienceValidation(true);
         validator.initialize();
         validator.validate(claimsSet, prc);
     }
     
     @Test(expectedExceptions = JWTValidationException.class)
-    public void doInValidateTestNullAudienceInJWT() throws JWTValidationException, ComponentInitializationException {
+    public void doRejectedTest_NullAudienceInJWT() 
+            throws JWTValidationException, ComponentInitializationException {
         final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder().build();
         validator.setId("test-validator");
         validator.setAudienceLookupStrategy((prc,claims) -> "client-id");
+        validator.setExtraAudienceValidation(true);
+        validator.initialize();
+        validator.validate(claimsSet, prc);
+    }
+    
+    @Test
+    public void doValidTest_AdditionalAudiencesNotChecked() 
+            throws JWTValidationException, ComponentInitializationException {
+        final JWTClaimsSet claimsSet = 
+                new JWTClaimsSet.Builder().audience(
+                        List.of("client-id","additional-client-1","additional-client-2")).build();
+        validator.setId("test-validator");
+        validator.setAudienceLookupStrategy((prc,claims) -> "client-id");
+        validator.setExtraAudienceValidation(false);
         validator.initialize();
         validator.validate(claimsSet, prc);
     }

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


More information about the commits mailing list