[java-idp-plugin-oidc-rp] branch main updated: JOIDCRP-51 - Translate nonexistent ACR in responses

Phil Smart philip.smart at jisc.ac.uk
Wed Feb 14 09:56:22 UTC 2024


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=cbe7c95cf7d0146fd388ccbba65955c07cc8bf82

The following commit(s) were added to refs/heads/main by this push:
     new cbe7c95  JOIDCRP-51 - Translate nonexistent ACR in responses
cbe7c95 is described below

commit cbe7c95cf7d0146fd388ccbba65955c07cc8bf82
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Wed Feb 14 09:56:19 2024 +0000

    JOIDCRP-51 - Translate nonexistent ACR in responses
    
     - Allow empty AMR and ACR lists to be passed to the AMR and ACR
    translators.
    
    https://shibboleth.atlassian.net/browse/JOIDCRP-51
---
 .../oidc/rp/impl/ValidateOIDCAuthentication.java   |  41 +++++---
 .../rp/impl/ValidateOIDCAuthenticationTest.java    | 109 ++++++++++++++++++++-
 2 files changed, 133 insertions(+), 17 deletions(-)

diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateOIDCAuthentication.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateOIDCAuthentication.java
index 6fd6225..57d130a 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateOIDCAuthentication.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateOIDCAuthentication.java
@@ -19,6 +19,7 @@ import java.text.ParseException;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
@@ -73,6 +74,7 @@ import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.shared.annotation.constraint.NonnullBeforeExec;
 import net.shibboleth.shared.annotation.constraint.NonnullElements;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
+import net.shibboleth.shared.collection.CollectionSupport;
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.LoggerFactory;
 import net.shibboleth.shared.service.ReloadableService;
@@ -385,10 +387,14 @@ public class ValidateOIDCAuthentication extends AbstractValidationAction {
     @Override
     protected Subject populateSubject(@Nonnull final Subject subject) {   
                 
+        // Allow empty ACR/AMR lists to pass into the acrTranslator. So mapping can occur even if none are present 
+        // from the OP.
         final var localAcrTranslator = acrTranslator;
-        if (localAcrTranslator != null && unprocessedIdTokenClaims.getClaim("acr") != null
-                && unprocessedIdTokenClaims.getClaim("acr") instanceof final String acr) {
-            final Collection<Principal> translated = localAcrTranslator.apply(List.of(acr));
+        if (localAcrTranslator != null) {
+            final Object acrClaim = unprocessedIdTokenClaims.getClaim("acr");
+            final List<String> acrList = 
+                    acrClaim instanceof final String acr ? List.of(acr) : CollectionSupport.emptyList();
+            final Collection<Principal> translated = localAcrTranslator.apply(acrList);
             if (translated != null && !translated.isEmpty()) {
                 subject.getPrincipals().addAll(translated);
                 if (log.isDebugEnabled()) {
@@ -396,26 +402,29 @@ public class ValidateOIDCAuthentication extends AbstractValidationAction {
                             translated.stream().map(Principal::getName).toList());
                 }
             }
-        }
+        }        
         
         final var localAmrTranslator = amrTranslator;
-        if (localAmrTranslator != null && unprocessedIdTokenClaims.getClaim("amr") != null
-                && unprocessedIdTokenClaims.getClaim("amr") instanceof Collection) {
+        if (localAmrTranslator != null) {
+            final Object amrClaim = unprocessedIdTokenClaims.getClaim("amr");
+            List<String> amrs = Collections.emptyList();
             try {
-                final List<String> amrs = unprocessedIdTokenClaims.getStringListClaim("amr");
-                final Collection<Principal> translated = localAmrTranslator.apply(amrs);
-                if (translated != null && !translated.isEmpty()) {
-                    subject.getPrincipals().addAll(translated);
-                    if (log.isDebugEnabled()) {
-                        log.debug("{} Added translated AMR Principals: {}", getLogPrefix(),
-                                translated.stream().map(Principal::getName).toList());
-                    }
+                if (amrClaim instanceof Collection) {
+                    amrs = unprocessedIdTokenClaims.getStringListClaim("amr");
                 }
             } catch (final ParseException e) {
-                log.warn("Unable to parse AMR claims", e);
+                log.debug("Unable to parse AMR claims", e);
+            }
+            final Collection<Principal> translated = localAmrTranslator.apply(amrs);
+            if (translated != null && !translated.isEmpty()) {
+                subject.getPrincipals().addAll(translated);
+                if (log.isDebugEnabled()) {
+                    log.debug("{} Added translated AMR Principals: {}", getLogPrefix(),
+                            translated.stream().map(Principal::getName).toList());
+                }
             }
-            
         }
+
         // TODO What type of subject?  switch on subject_types_supported and subject_type.    
         final String localSubject = unprocessedIdTokenClaims.getSubject();
         assert localSubject != null;
diff --git a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateOIDCAuthenticationTest.java b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateOIDCAuthenticationTest.java
index 38c55eb..4757a75 100644
--- a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateOIDCAuthenticationTest.java
+++ b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateOIDCAuthenticationTest.java
@@ -190,7 +190,8 @@ public class ValidateOIDCAuthenticationTest  extends AbstractOIDCTest {
                    for (final String amr : amrs) {
                        if ("pwd".equals(amr)) {
                            principals.add(new
-                                   AuthenticationMethodPrincipal("urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport"));
+                                   AuthenticationMethodPrincipal(
+                                           "urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport"));
                        }
                    }
                    return principals;
@@ -213,6 +214,112 @@ public class ValidateOIDCAuthenticationTest  extends AbstractOIDCTest {
         assertEquals(subject.getPrincipals(AuthenticationMethodPrincipal.class).size(), 1);
         
     }
+    
+    @Test
+    public void testSuccess_WithNoACRClaimButACRLookupStrategy() throws Exception {
+        
+        // Replace the end user claims with a JWT that includes the no ACR value
+        final PlainJWT jwt = new PlainJWT(new JWTClaimsSet.Builder()
+                .issuer("https://op.example.com")
+                .audience(List.of("https://rp.example.com"))
+                .subject("jdoe")
+                .claim("nonce", "abadnonce")
+                .claim("azp", "https://rp.example.com")
+                .claim("name","jdoe")
+                .claim("amr", List.of("pwd", "otp"))
+                .expirationTime(Date.from(Instant.now().plusSeconds(120)))
+                .build());
+        final var claimsSet = jwt.getJWTClaimsSet();
+        assert claimsSet != null;
+        final EndUserClaimsContext endClaimsContext = new EndUserClaimsContext();
+        final ClaimsSet endUserClaims = new ClaimsSet();
+        endUserClaims.putAll(claimsSet.getClaims());
+        endClaimsContext.setUnprocessedIdTokenClaims(claimsSet);
+        endClaimsContext.setEndUserClaims(endUserClaims);
+        final var inboundMsgCtx = prc.getInboundMessageContext();
+        assert inboundMsgCtx != null;
+        inboundMsgCtx.addSubcontext(endClaimsContext,true);
+        
+        // Strategy exists, but it is working on an empty list of ACRs
+        partyConfig.setAuthenticationContextClassReferenceTranslationStrategyLookupStrategy(
+                context -> acrs -> {     
+                    final List<Principal> principals = new ArrayList<>();
+                    if (acrs.isEmpty()) {
+                        principals.add(new AuthnContextClassRefPrincipal("urn:mace:incommon:iap:silver"));                 
+                    }
+                    return principals;
+                });
+        
+        action.initialize();
+        final Event result = action.execute(src);
+        
+        assertNull(result);
+        assertNotNull(ac.getAuthenticationResult());
+        final var authnResult = ac.getAuthenticationResult();
+        assert authnResult != null;
+        assertNotNull(authnResult.getSubject());
+        final var subject = authnResult.getSubject();
+        assertEquals(subject.getPrincipals(OIDCSubjectIdentifierPrincipal.class).size(), 1);
+        assertEquals(subject.getPrincipals(IdPAttributePrincipal.class).size(), 1);
+        assertEquals(subject.getPrincipals(IdPAttributePrincipal.class)
+                .iterator().next().getName(),"givenName");
+        assertEquals(subject.getPrincipals(AuthnContextClassRefPrincipal.class).size(), 1);
+        
+    }
+    
+    @Test
+    public void testSuccess_WithNoAMRClaimButAMRLookupStrategy() throws Exception {
+        
+        // Replace the end user claims with a JWT that includes the no ACR value
+        final PlainJWT jwt = new PlainJWT(new JWTClaimsSet.Builder()
+                .issuer("https://op.example.com")
+                .audience(List.of("https://rp.example.com"))
+                .subject("jdoe")
+                .claim("nonce", "abadnonce")
+                .claim("azp", "https://rp.example.com")
+                .claim("name","jdoe")
+                .expirationTime(Date.from(Instant.now().plusSeconds(120)))
+                .build());
+        final var claimsSet = jwt.getJWTClaimsSet();
+        assert claimsSet != null;
+        final EndUserClaimsContext endClaimsContext = new EndUserClaimsContext();
+        final ClaimsSet endUserClaims = new ClaimsSet();
+        endUserClaims.putAll(claimsSet.getClaims());
+        endClaimsContext.setUnprocessedIdTokenClaims(claimsSet);
+        endClaimsContext.setEndUserClaims(endUserClaims);
+        final var inboundMsgCtx = prc.getInboundMessageContext();
+        assert inboundMsgCtx != null;
+        inboundMsgCtx.addSubcontext(endClaimsContext,true);
+        
+        
+        // Add PasswordProtectedTransport if empty list input. To test empty list is being sent to the strategy
+        partyConfig.setAuthenticationMethodsReferencesTranslationStrategyLookupStrategy(
+                context -> amrs -> {
+                    final List<Principal> principals = new ArrayList<>();
+                    if (amrs.isEmpty()) {
+                            principals.add(new
+                                    AuthenticationMethodPrincipal(
+                                            "urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport"));
+                    }                      
+                    return principals;
+                });
+        
+        action.initialize();
+        final Event result = action.execute(src);
+        
+        assertNull(result);
+        assertNotNull(ac.getAuthenticationResult());
+        final var authnResult = ac.getAuthenticationResult();
+        assert authnResult != null;
+        assertNotNull(authnResult.getSubject());
+        final var subject = authnResult.getSubject();
+        assertEquals(subject.getPrincipals(OIDCSubjectIdentifierPrincipal.class).size(), 1);
+        assertEquals(subject.getPrincipals(IdPAttributePrincipal.class).size(), 1);
+        assertEquals(subject.getPrincipals(IdPAttributePrincipal.class)
+                .iterator().next().getName(),"givenName");
+        assertEquals(subject.getPrincipals(AuthenticationMethodPrincipal.class).size(), 1);
+        
+    }
 
     @Test
     public void testSuccess() throws ComponentInitializationException {

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


More information about the commits mailing list