[java-idp-oidc] branch main updated: JOIDC-19 - Applying consent to back-channel uses the wrong attribute names

[Henri Mikkonen]

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

hjmikkon pushed a commit to branch main
in repository java-idp-oidc.

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

The following commit(s) were added to refs/heads/main by this push:
       new  e092846a  JOIDC-19 - Applying consent to back-channel uses the wrong attribute names
e092846a is described below

commit e092846a45d240cabb26e4d60bcc817634d2b329
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Tue May 18 15:26:00 2021 +0300

    JOIDC-19 - Applying consent to back-channel uses the wrong attribute names
    
    https://issues.shibboleth.net/jira/browse/JOIDC-19
    
    The bug described in the issue was still existing when the consent was
    encoded into tokens (property 'idp.oidc.encodeConsentInTokens' set to true).
---
 .../oidc/op/profile/impl/AddAttributesToClaimsSet.java | 18 +++++++++---------
 .../op/profile/impl/AddAttributesToClaimsSetTest.java  | 16 ++++++++--------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/AddAttributesToClaimsSet.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/AddAttributesToClaimsSet.java
index 06077fdb..9ebd0601 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/AddAttributesToClaimsSet.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/AddAttributesToClaimsSet.java
@@ -311,6 +311,9 @@ public class AddAttributesToClaimsSet extends AbstractOIDCResponseAction {
 
         final List<JSONObject> claims = new ArrayList<>();
         
+        final OIDCAuthenticationResponseConsentContext consentCtx =
+                consentContextLookupStrategy.apply(profileRequestContext);
+        
         ServiceableComponent<AttributeTranscoderRegistry> component = null;
         try {
             component = transcoderRegistry.getServiceableComponent();
@@ -321,7 +324,12 @@ public class AddAttributesToClaimsSet extends AbstractOIDCResponseAction {
             
             for (final IdPAttribute attribute : attributeCtx.getIdPAttributes().values()) {
                 if (attribute != null && !attribute.getValues().isEmpty()) {
-                    encodeAttribute(component.getComponent(), profileRequestContext, attribute, claims);
+                    if (consentCtx != null && !consentCtx.getConsentedAttributes().contains(attribute.getId())) {
+                        log.debug("{} Consentable attribute {} has no consent. Not added to claims set",
+                                getLogPrefix(), attribute.getId());
+                    } else {
+                        encodeAttribute(component.getComponent(), profileRequestContext, attribute, claims);
+                    }
                 }
             }
         } catch (final AttributeEncodingException e) {
@@ -333,9 +341,6 @@ public class AddAttributesToClaimsSet extends AbstractOIDCResponseAction {
             }
         }
         
-        final OIDCAuthenticationResponseConsentContext consentCtx =
-                consentContextLookupStrategy.apply(profileRequestContext);
-        
         for (final JSONObject claim : claims) {
             for (final String name : claim.keySet()) {
                 if (reservedClaimNames != null && reservedClaimNames.contains(name)) {
@@ -343,11 +348,6 @@ public class AddAttributesToClaimsSet extends AbstractOIDCResponseAction {
                     continue;
                 }
                 
-                if (consentCtx != null && !consentCtx.getConsentedAttributes().contains(name)) {
-                    log.debug("{} Consentable attribute {} has no consent. Not added to claims set",
-                            getLogPrefix(), name);
-                    continue;
-                }
                 log.debug("{} Adding claim {} with value {}", getLogPrefix(), name, claim.get(name));
                 claimsSet.setClaim(name, claim.get(name));
             }
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/AddAttributesToClaimsSetTest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/AddAttributesToClaimsSetTest.java
index b8fd96eb..1def710d 100644
--- a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/AddAttributesToClaimsSetTest.java
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/AddAttributesToClaimsSetTest.java
@@ -88,12 +88,12 @@ public class AddAttributesToClaimsSetTest extends BaseOIDCResponseActionTest {
         final Map<String,Object> rule3 = new HashMap<>();
         rule3.put(AttributeTranscoderRegistry.PROP_ID, "test3");
         rule3.put(AttributeTranscoderRegistry.PROP_TRANSCODER, transcoder);
-        rule3.put(OIDCAttributeTranscoder.PROP_NAME, "test3");
+        rule3.put(OIDCAttributeTranscoder.PROP_NAME, "test3claim");
 
         final Map<String,Object> rule4 = new HashMap<>();
         rule4.put(AttributeTranscoderRegistry.PROP_ID, "test4");
         rule4.put(AttributeTranscoderRegistry.PROP_TRANSCODER, transcoder);
-        rule4.put(OIDCAttributeTranscoder.PROP_NAME, "test4");
+        rule4.put(OIDCAttributeTranscoder.PROP_NAME, "test4claim");
         
         registry.setNamingRegistry(Collections.singletonList(
                 new BasicNamingFunction<>(transcoder.getEncodedType(),
@@ -203,9 +203,9 @@ public class AddAttributesToClaimsSetTest extends BaseOIDCResponseActionTest {
         ActionTestingSupport.assertProceedEvent(event);
         Assert.assertTrue(respCtx.getIDToken().getClaim("test1").equals("value1 value2"));
         Assert.assertNull(respCtx.getIDToken().getClaim("test2"));
-        Assert.assertTrue(respCtx.getIDToken().getClaim("test3").equals("value3"));
-        Assert.assertTrue(respCtx.getIDToken().getClaim("test4").equals("value4"));
-        Assert.assertNull(respCtx.getIDToken().getClaim("test5"));
+        Assert.assertTrue(respCtx.getIDToken().getClaim("test3claim").equals("value3"));
+        Assert.assertTrue(respCtx.getIDToken().getClaim("test4claim").equals("value4"));
+        Assert.assertNull(respCtx.getIDToken().getClaim("test5claim"));
     }
 
     /**
@@ -226,9 +226,9 @@ public class AddAttributesToClaimsSetTest extends BaseOIDCResponseActionTest {
         ActionTestingSupport.assertProceedEvent(event);
         Assert.assertTrue(respCtx.getIDToken().getClaim("test1").equals("value1 value2"));
         Assert.assertNull(respCtx.getIDToken().getClaim("test2"));
-        Assert.assertNull(respCtx.getIDToken().getClaim("test3"));
-        Assert.assertTrue(respCtx.getIDToken().getClaim("test4").equals("value4"));
-        Assert.assertNull(respCtx.getIDToken().getClaim("test5"));
+        Assert.assertNull(respCtx.getIDToken().getClaim("test3claim"));
+        Assert.assertTrue(respCtx.getIDToken().getClaim("test4claim").equals("value4"));
+        Assert.assertNull(respCtx.getIDToken().getClaim("test5claim"));
     }
 
 }
\ No newline at end of file

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