[java-idp-plugin-oidc-rp] branch main updated: JOIDCRP-73 - Aliased decoded IdPAttributes are lost during subsequent use
Phil Smart
philip.smart at jisc.ac.uk
Fri May 2 09:23:15 UTC 2025
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=efa39de691dfe537e1fb21f6a0327985c788a1f6
The following commit(s) were added to refs/heads/main by this push:
new efa39de JOIDCRP-73 - Aliased decoded IdPAttributes are lost during subsequent use
efa39de is described below
commit efa39de691dfe537e1fb21f6a0327985c788a1f6
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Fri May 2 10:23:13 2025 +0100
JOIDCRP-73 - Aliased decoded IdPAttributes are lost during subsequent
use
- Inlined the IdP Attribute map merging methods from
IdPAttributeSupport in shib-attribute-api 5.2.
- Used the inlined methods to merge attributes with duplicate IDs
before they are set onto the attribute context.
https://shibboleth.atlassian.net/browse/JOIDCRP-73
---
.../oidc/rp/impl/ValidateOIDCAuthentication.java | 102 +++++++++++++++++---
.../rp/impl/ValidateOIDCAuthenticationTest.java | 107 ++++++++++++++++++++-
2 files changed, 192 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 57d130a..d2a8ce6 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
@@ -21,9 +21,11 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
+import java.util.stream.Collectors;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
@@ -48,6 +50,7 @@ import com.nimbusds.openid.connect.sdk.claims.ClaimsSet;
import net.minidev.json.JSONObject;
import net.shibboleth.idp.attribute.AttributeDecodingException;
import net.shibboleth.idp.attribute.IdPAttribute;
+import net.shibboleth.idp.attribute.IdPAttributeValue;
import net.shibboleth.idp.attribute.context.AttributeContext;
import net.shibboleth.idp.attribute.filter.AttributeFilter;
import net.shibboleth.idp.attribute.filter.AttributeFilterException;
@@ -332,22 +335,28 @@ public class ValidateOIDCAuthentication extends AbstractValidationAction {
final var localAttributeExtractionStrategy = attributeExtractionStrategy;
if (localAttributeExtractionStrategy != null) {
log.debug("{} Applying custom extraction strategy function", getLogPrefix());
- if (attributeContext == null) {
- final RelyingPartyContext rpc = profileRequestContext.getSubcontext(RelyingPartyContext.class);
- assert rpc != null;
- attributeContext = rpc.ensureSubcontext(AttributeContext.class);
- }
- final var localAttributeContext = attributeContext;
- assert localAttributeContext != null;
- final Collection<IdPAttribute> attributes = new ArrayList<>(localAttributeContext.getIdPAttributes().values());
- final Collection<IdPAttribute> newAttributes = localAttributeExtractionStrategy.apply(profileRequestContext);
+
+ final Collection<IdPAttribute> newAttributes =
+ localAttributeExtractionStrategy.apply(profileRequestContext);
if (newAttributes != null) {
if (log.isDebugEnabled()) {
log.debug("{} Extracted attributes with custom strategy: {}", getLogPrefix(),
- newAttributes.stream().map(IdPAttribute::getId).toList());
+ newAttributes.stream().map(IdPAttribute::getId).collect(Collectors.toUnmodifiableList()));
+ }
+
+ if (attributeContext != null) {
+ // Need to merge the new collection with the original map.
+ final Map<String,IdPAttribute> newMap = toMapMergeDuplicates(newAttributes);
+ assert attributeContext != null;
+ attributeContext.setIdPAttributes(
+ withMapMergeDuplicates(newMap, attributeContext.getIdPAttributes().values()).values());
+ } else {
+ // No existing attributes, so produce a merged map out of the custom extraction result
+ // and store to new context.
+ attributeContext = profileRequestContext.ensureSubcontext(RelyingPartyContext.class)
+ .ensureSubcontext(AttributeContext.class);
+ attributeContext.setIdPAttributes(toMapMergeDuplicates(newAttributes).values());
}
- attributes.addAll(newAttributes);
- localAttributeContext.setIdPAttributes(attributes);
}
}
@@ -382,6 +391,7 @@ public class ValidateOIDCAuthentication extends AbstractValidationAction {
}
}
+
// Checkstyle: CyclomaticComplexity OFF
@Override
@@ -511,9 +521,8 @@ public class ValidateOIDCAuthentication extends AbstractValidationAction {
final RelyingPartyContext rpc = profileRequestContext.getSubcontext(RelyingPartyContext.class);
assert rpc != null;
final var ac = attributeContext = rpc.ensureSubcontext(AttributeContext.class);
- assert ac != null;
- ac.setUnfilteredIdPAttributes(mapped.values());
- ac.setIdPAttributes(null);
+ assert ac != null;
+ ac.setUnfilteredIdPAttributes(toMapMergeDuplicates(mapped.values()).values()).setIdPAttributes(null);
filterAttributes(profileRequestContext);
}
}
@@ -605,5 +614,68 @@ public class ValidateOIDCAuthentication extends AbstractValidationAction {
}
}
}
+
+
+
+ /**
+ * Convert a collection of {@link IdPAttribute} objects into a mutable {@link Map} keyed by
+ * {@link IdPAttribute#getId()}.
+ *
+ * <p>If an attribute with a duplicate ID is found, then we merge the values into a single attribute.</p>
+ *
+ * @param attributes collection of attributes
+ *
+ * @return the input attributes in a map keyed by {@link IdPAttribute#getId()}
+ */
+ //TODO this method is in IdPAttributeSupport in IdP 5.2. So we can remove this when we switch.
+ @Nonnull @Live private static Map<String,IdPAttribute> toMapMergeDuplicates(
+ @Nullable final Collection<IdPAttribute> attributes) {
+
+ final Map<String, IdPAttribute> accumulator = new HashMap<>();
+ return withMapMergeDuplicates(accumulator, attributes);
+ }
+
+ /**
+ * Add a collection of {@link IdPAttribute} objects into a mutable input {@link Map} keyed by
+ * {@link IdPAttribute#getId()}.
+ *
+ * <p>If an attribute with a duplicate ID is found, then we merge the values into a single attribute.</p>
+ *
+ * @param existingAttributes an existing mutable map to merge into
+ * @param newAttributes collection to merge into map
+ *
+ * @return the input map after merging is done
+ */
+ //TODO this method is in IdPAttributeSupport in IdP 5.2. So we can remove this when we switch.
+ @Nonnull @Live private static Map<String,IdPAttribute> withMapMergeDuplicates(
+ @Nonnull @Live final Map<String,IdPAttribute> existingAttributes,
+ @Nullable final Collection<IdPAttribute> newAttributes) {
+
+ if (newAttributes == null) {
+ return existingAttributes;
+ }
+
+ for (final IdPAttribute attribute : newAttributes) {
+
+ final IdPAttribute oldAttr = existingAttributes.get(attribute.getId());
+ if (oldAttr == null) {
+ existingAttributes.put(attribute.getId(), attribute);
+ } else {
+ final IdPAttribute newAttribute;
+ try {
+ newAttribute = oldAttr.clone();
+ } catch (final Exception e) {
+ throw new RuntimeException(e);
+ }
+ final List<IdPAttributeValue> values = new ArrayList<>(newAttribute.getValues());
+ values.addAll(attribute.getValues());
+ newAttribute.setValues(values);
+ existingAttributes.remove(attribute.getId());
+ existingAttributes.put(newAttribute.getId(), newAttribute);
+ }
+ }
+
+ return existingAttributes;
+ }
}
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 4757a75..b7a6fc5 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
@@ -18,10 +18,12 @@ package net.shibboleth.idp.plugin.authn.oidc.rp.impl;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertNull;
+import static org.testng.Assert.assertTrue;
import java.security.Principal;
import java.time.Instant;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
@@ -42,6 +44,8 @@ import com.nimbusds.jwt.PlainJWT;
import com.nimbusds.openid.connect.sdk.claims.ClaimsSet;
import net.shibboleth.idp.attribute.IdPAttribute;
+import net.shibboleth.idp.attribute.IdPAttributeValue;
+import net.shibboleth.idp.attribute.StringAttributeValue;
import net.shibboleth.idp.attribute.filter.AttributeFilterPolicy;
import net.shibboleth.idp.attribute.filter.AttributeRule;
import net.shibboleth.idp.attribute.filter.PolicyRequirementRule;
@@ -98,10 +102,18 @@ public class ValidateOIDCAuthenticationTest extends AbstractOIDCTest {
ruleset1.put(AttributeTranscoderRegistry.PROP_DESCRIPTION + '.' + Locale.ENGLISH.toLanguageTag(), "Given Name");
ruleset1.put(OIDCAttributeTranscoder.PROP_NAME,"name");
ruleset1.put("name", "bar");
-
+
+ // RuleSet for an aliased attribute
+ final Map<String,Object> ruleset2 = new HashMap<>();
+ ruleset2.put(AttributeTranscoderRegistry.PROP_ID, "givenName");
+ ruleset2.put(AttributeTranscoderRegistry.PROP_TRANSCODER, transcoder);
+ ruleset2.put(AttributeTranscoderRegistry.PROP_DISPLAY_NAME, "givenName");
+ ruleset2.put(AttributeTranscoderRegistry.PROP_DESCRIPTION + '.' + Locale.ENGLISH.toLanguageTag(), "Given Name");
+ ruleset2.put(OIDCAttributeTranscoder.PROP_NAME,"aliasedName");
+ ruleset2.put("name", "bar");
registry.setTranscoderRegistry(CollectionSupport.listOf(
- new TranscodingRule(ruleset1)));
+ new TranscodingRule(ruleset1), new TranscodingRule(ruleset2)));
registry.setApplicationContext(new MockApplicationContext());
registry.initialize();
assertEquals(registry.getDisplayNames(new IdPAttribute("givenName")).size(), 1);
@@ -338,6 +350,97 @@ public class ValidateOIDCAuthenticationTest extends AbstractOIDCTest {
.iterator().next().getName(),"givenName");
+ }
+
+ /* Both aliasedName and name transcode to givenName. Make sure those are not lost in the merged result.*/
+ @Test
+ public void testSuccess_WithAliasedAttribute() throws Exception {
+
+ 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("aliasedName", "another-name")
+ .claim("acr","urn:mace:incommon:iap:silver")
+ .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();
+ // We put all the claims in the JWT in here, not just the 'sanitized' ones
+ endUserClaims.putAll(claimsSet.getClaims());
+ endClaimsContext.setUnprocessedIdTokenClaims(claimsSet);
+ endClaimsContext.setEndUserClaims(endUserClaims);
+
+ final var inboundMsgCtx = prc.getInboundMessageContext();
+ assert inboundMsgCtx != null;
+ inboundMsgCtx.removeSubcontext(EndUserClaimsContext.class);
+ inboundMsgCtx.addSubcontext(endClaimsContext);
+
+ 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");
+
+ final IdPAttributePrincipal attrPrincipal =
+ subject.getPrincipals(IdPAttributePrincipal.class).iterator().next();
+ final List<IdPAttributeValue> values = attrPrincipal.getAttribute().getValues();
+ assertEquals(values.size(),2);
+ // One should be 'jdoe' and the other 'other-name'
+ final List<Object> nativeValues = values.stream().map(attr -> attr.getNativeValue()).toList();
+ assertTrue(nativeValues.containsAll(Arrays.asList("jdoe","another-name")));
+
+
+ }
+
+ @Test
+ public void testSuccess_WithCustomAttributeExtraction() throws Exception {
+
+
+ // Create a strategy that duplicates an existing attribute
+ action.setAttributeExtractionStrategy( prc -> {
+ final var attribute = new IdPAttribute("givenName");
+ attribute.setValues(Collections.singletonList(new StringAttributeValue("another-name")));
+ return Collections.singleton(attribute);
+ });
+
+ 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");
+
+ final IdPAttributePrincipal attrPrincipal =
+ subject.getPrincipals(IdPAttributePrincipal.class).iterator().next();
+ final List<IdPAttributeValue> values = attrPrincipal.getAttribute().getValues();
+ assertEquals(values.size(),2);
+ // One should be 'jdoe' and the other 'other-name'
+ final List<Object> nativeValues = values.stream().map(attr -> attr.getNativeValue()).toList();
+ assertTrue(nativeValues.containsAll(Arrays.asList("jdoe","another-name")));
+
+
}
@Test
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list