[java-identity-provider] branch main updated: IDP-1936 - Multiple IdPAttributes mapped to 1 name don't get combined.
Scott Cantor
cantor.2 at osu.edu
Thu Apr 7 16:40:43 UTC 2022
This is an automated email from the git hooks/post-receive script.
scantor pushed a commit to branch main
in repository java-identity-provider.
View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=f044477454b4e2cc1c1efa7b27971e58350e44d6
The following commit(s) were added to refs/heads/main by this push:
new f04447745 IDP-1936 - Multiple IdPAttributes mapped to 1 name don't get combined.
f04447745 is described below
commit f044477454b4e2cc1c1efa7b27971e58350e44d6
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Thu Apr 7 12:40:39 2022 -0400
IDP-1936 - Multiple IdPAttributes mapped to 1 name don't get combined.
https://shibboleth.atlassian.net/browse/IDP-1936
---
.../impl/AddAttributeStatementToAssertion.java | 42 +++++++++++++++++++-
.../impl/AddAttributeStatementToAssertion.java | 46 ++++++++++++++++++++--
.../impl/AddAttributeStatementToAssertionTest.java | 36 +++++++++++++----
.../impl/AddAttributeStatementToAssertionTest.java | 34 ++++++++++++----
4 files changed, 139 insertions(+), 19 deletions(-)
diff --git a/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/saml1/profile/impl/AddAttributeStatementToAssertion.java b/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/saml1/profile/impl/AddAttributeStatementToAssertion.java
index eb460edc7..138b9bdd2 100644
--- a/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/saml1/profile/impl/AddAttributeStatementToAssertion.java
+++ b/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/saml1/profile/impl/AddAttributeStatementToAssertion.java
@@ -19,6 +19,8 @@ package net.shibboleth.idp.saml.saml1.profile.impl;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Iterator;
+import java.util.Objects;
import java.util.function.Function;
import javax.annotation.Nonnull;
@@ -35,6 +37,7 @@ import net.shibboleth.utilities.java.support.component.ComponentSupport;
import net.shibboleth.utilities.java.support.logic.Constraint;
import net.shibboleth.utilities.java.support.service.ServiceableComponent;
+import org.opensaml.core.xml.XMLObject;
import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport;
import org.opensaml.profile.action.ActionSupport;
import org.opensaml.profile.action.EventIds;
@@ -114,6 +117,7 @@ public class AddAttributeStatementToAssertion extends BaseAddAttributeStatementT
}
//CheckStyle: ReturnCount ON
+// CheckStyle: CyclomaticComplexity OFF
/**
* Builds an attribute statement from a collection of attributes.
*
@@ -160,9 +164,45 @@ public class AddAttributeStatementToAssertion extends BaseAddAttributeStatementT
AttributeStatement.DEFAULT_ELEMENT_NAME);
final AttributeStatement statement = statementBuilder.buildObject();
- statement.getAttributes().addAll(encodedAttributes);
+
+ for (final Attribute attribute : encodedAttributes) {
+ final Attribute existing = findExistingAttribute(statement, attribute);
+ if (existing != null) {
+ final Iterator<XMLObject> newValues = attribute.getAttributeValues().iterator();
+ while (newValues.hasNext()) {
+ final XMLObject newValue = newValues.next();
+ newValues.remove();
+ existing.getAttributeValues().add(newValue);
+ }
+ } else {
+ statement.getAttributes().add(attribute);
+ }
+ }
+
return statement;
}
+// CheckStyle: CyclomaticComplexity ON
+
+ /**
+ * Find a matching {@link Attribute} in the statement, if any.
+ *
+ * @param statement input statement
+ * @param newAttribute the attribute to match
+ *
+ * @return a match, or null
+ */
+ @Nullable private Attribute findExistingAttribute(@Nonnull final AttributeStatement statement,
+ @Nonnull final Attribute newAttribute) {
+
+ for (final Attribute attr : statement.getAttributes()) {
+ if (Objects.equals(attr.getAttributeName(), newAttribute.getAttributeName())
+ && Objects.equals(attr.getAttributeNamespace(), newAttribute.getAttributeNamespace())) {
+ return attr;
+ }
+ }
+
+ return null;
+ }
/**
* Encodes {@link IdPAttribute} into zero or more {@link Attribute} objects if a proper encoder is available.
diff --git a/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/saml2/profile/impl/AddAttributeStatementToAssertion.java b/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/saml2/profile/impl/AddAttributeStatementToAssertion.java
index d331d07d8..ed1c2a01d 100644
--- a/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/saml2/profile/impl/AddAttributeStatementToAssertion.java
+++ b/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/saml2/profile/impl/AddAttributeStatementToAssertion.java
@@ -19,6 +19,8 @@ package net.shibboleth.idp.saml.saml2.profile.impl;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Iterator;
+import java.util.Objects;
import java.util.function.Function;
import javax.annotation.Nonnull;
@@ -35,6 +37,7 @@ import net.shibboleth.utilities.java.support.component.ComponentSupport;
import net.shibboleth.utilities.java.support.logic.Constraint;
import net.shibboleth.utilities.java.support.service.ServiceableComponent;
+import org.opensaml.core.xml.XMLObject;
import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport;
import org.opensaml.profile.action.ActionSupport;
import org.opensaml.profile.action.EventIds;
@@ -88,7 +91,7 @@ public class AddAttributeStatementToAssertion extends BaseAddAttributeStatementT
assertionLookupStrategy = Constraint.isNotNull(strategy, "Assertion lookup strategy cannot be null");
}
-//CheckStyle: ReturnCount OFF
+// CheckStyle: ReturnCount OFF
/** {@inheritDoc} */
@Override protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
try {
@@ -113,8 +116,9 @@ public class AddAttributeStatementToAssertion extends BaseAddAttributeStatementT
ActionSupport.buildEvent(profileRequestContext, IdPEventIds.UNABLE_ENCODE_ATTRIBUTE);
}
}
-//CheckStyle: ReturnCount ON
+// CheckStyle: ReturnCount ON
+// CheckStyle: CyclomaticComplexity OFF
/**
* Builds an attribute statement from a collection of attributes.
*
@@ -162,9 +166,45 @@ public class AddAttributeStatementToAssertion extends BaseAddAttributeStatementT
AttributeStatement.DEFAULT_ELEMENT_NAME);
final AttributeStatement statement = statementBuilder.buildObject();
- statement.getAttributes().addAll(encodedAttributes);
+
+ for (final Attribute attribute : encodedAttributes) {
+ final Attribute existing = findExistingAttribute(statement, attribute);
+ if (existing != null) {
+ final Iterator<XMLObject> newValues = attribute.getAttributeValues().iterator();
+ while (newValues.hasNext()) {
+ final XMLObject newValue = newValues.next();
+ newValues.remove();
+ existing.getAttributeValues().add(newValue);
+ }
+ } else {
+ statement.getAttributes().add(attribute);
+ }
+ }
+
return statement;
}
+// CheckStyle: CyclomaticComplexity ON
+
+ /**
+ * Find a matching {@link Attribute} in the statement, if any.
+ *
+ * @param statement input statement
+ * @param newAttribute the attribute to match
+ *
+ * @return a match, or null
+ */
+ @Nullable private Attribute findExistingAttribute(@Nonnull final AttributeStatement statement,
+ @Nonnull final Attribute newAttribute) {
+
+ for (final Attribute attr : statement.getAttributes()) {
+ if (Objects.equals(attr.getName(), newAttribute.getName())
+ && Objects.equals(attr.getNameFormat(), newAttribute.getNameFormat())) {
+ return attr;
+ }
+ }
+
+ return null;
+ }
/**
* Encodes a {@link IdPAttribute} into zero or more {@link Attribute} objects if a proper encoder is available.
diff --git a/idp-saml-impl/src/test/java/net/shibboleth/idp/saml/saml1/profile/impl/AddAttributeStatementToAssertionTest.java b/idp-saml-impl/src/test/java/net/shibboleth/idp/saml/saml1/profile/impl/AddAttributeStatementToAssertionTest.java
index 91f3eb645..284bc8643 100644
--- a/idp-saml-impl/src/test/java/net/shibboleth/idp/saml/saml1/profile/impl/AddAttributeStatementToAssertionTest.java
+++ b/idp-saml-impl/src/test/java/net/shibboleth/idp/saml/saml1/profile/impl/AddAttributeStatementToAssertionTest.java
@@ -91,6 +91,11 @@ public class AddAttributeStatementToAssertionTest extends OpenSAMLInitBaseTestCa
private AttributeTranscoderRegistryImpl registry;
+ /**
+ * Set up for tests.
+ *
+ * @throws ComponentInitializationException on error
+ */
@BeforeMethod public void setUp() throws ComponentInitializationException {
rc = new RequestContextBuilder().setOutboundMessage(
SAML1ActionTestingSupport.buildResponse()).buildRequestContext();
@@ -123,10 +128,17 @@ public class AddAttributeStatementToAssertionTest extends OpenSAMLInitBaseTestCa
rule2_1.put(SAML1AttributeTranscoder.PROP_NAME, MY_NAME_2);
rule2_1.put(SAML1AttributeTranscoder.PROP_NAMESPACE, MY_NAMESPACE);
+ final Map<String,Object> rule2_2 = new HashMap<>();
+ rule2_2.put(AttributeTranscoderRegistry.PROP_ID, MY_NAME_2);
+ rule2_2.put(AttributeTranscoderRegistry.PROP_TRANSCODER, transcoder);
+ rule2_2.put(SAML1AttributeTranscoder.PROP_NAME, MY_ALTNAME_1);
+ rule2_2.put(SAML1AttributeTranscoder.PROP_NAMESPACE, MY_NAMESPACE);
+
registry.setTranscoderRegistry(Arrays.asList(
new TranscodingRule(rule1_1),
new TranscodingRule(rule1_2),
- new TranscodingRule(rule2_1)));
+ new TranscodingRule(rule2_1),
+ new TranscodingRule(rule2_2)));
registry.setApplicationContext(new MockApplicationContext());
registry.initialize();
@@ -389,26 +401,34 @@ public class AddAttributeStatementToAssertionTest extends OpenSAMLInitBaseTestCa
boolean one = false, altone = false, two = false;
for (final Attribute samlAttr : attributeStatement.getAttributes()) {
- Assert.assertNotNull(samlAttr.getAttributeValues());
- Assert.assertEquals(samlAttr.getAttributeValues().size(), 1);
- final XMLObject xmlObject = samlAttr.getAttributeValues().get(0);
- Assert.assertTrue(xmlObject instanceof XSStringImpl);
if (samlAttr.getAttributeName().equals(MY_NAME_1)) {
+ Assert.assertEquals(samlAttr.getAttributeValues().size(), 1);
+ final XMLObject xmlObject = samlAttr.getAttributeValues().get(0);
Assert.assertEquals(((XSStringImpl) xmlObject).getValue(), MY_VALUE_1);
one = true;
} else if (samlAttr.getAttributeName().equals(MY_NAME_2)) {
+ Assert.assertEquals(samlAttr.getAttributeValues().size(), 1);
+ final XMLObject xmlObject = samlAttr.getAttributeValues().get(0);
Assert.assertEquals(((XSStringImpl) xmlObject).getValue(), MY_VALUE_2);
altone = true;
} else if (samlAttr.getAttributeName().equals(MY_ALTNAME_1)) {
- Assert.assertEquals(((XSStringImpl) xmlObject).getValue(), MY_VALUE_1);
- two = true;
+ Assert.assertEquals(samlAttr.getAttributeValues().size(), 2);
+ final String val1 = ((XSStringImpl) samlAttr.getAttributeValues().get(0)).getValue();
+ final String val2 = ((XSStringImpl) samlAttr.getAttributeValues().get(1)).getValue();
+ if (val1.equals(MY_VALUE_1)) {
+ Assert.assertEquals(val2, MY_VALUE_2);
+ two = true;
+ } else if (val2.equals(MY_VALUE_1)) {
+ Assert.assertEquals(val1, MY_VALUE_2);
+ two = true;
+ }
} else {
Assert.fail("Incorrect attribute name.");
}
}
if (!one || !altone || !two) {
- Assert.fail("Missing attribute");
+ Assert.fail("Missing attribute or value");
}
}
diff --git a/idp-saml-impl/src/test/java/net/shibboleth/idp/saml/saml2/profile/impl/AddAttributeStatementToAssertionTest.java b/idp-saml-impl/src/test/java/net/shibboleth/idp/saml/saml2/profile/impl/AddAttributeStatementToAssertionTest.java
index 738756655..395728a0f 100644
--- a/idp-saml-impl/src/test/java/net/shibboleth/idp/saml/saml2/profile/impl/AddAttributeStatementToAssertionTest.java
+++ b/idp-saml-impl/src/test/java/net/shibboleth/idp/saml/saml2/profile/impl/AddAttributeStatementToAssertionTest.java
@@ -93,6 +93,11 @@ public class AddAttributeStatementToAssertionTest extends OpenSAMLInitBaseTestCa
private AttributeTranscoderRegistryImpl registry;
+ /**
+ * Set up for tests.
+ *
+ * @throws ComponentInitializationException on error
+ */
@BeforeMethod public void setUp() throws ComponentInitializationException {
rc = new RequestContextBuilder().setOutboundMessage(
SAML2ActionTestingSupport.buildResponse()).buildRequestContext();
@@ -125,6 +130,12 @@ public class AddAttributeStatementToAssertionTest extends OpenSAMLInitBaseTestCa
rule2_1.put(SAML2AttributeTranscoder.PROP_NAME, MY_NAME_2);
rule2_1.put(SAML2AttributeTranscoder.PROP_NAME_FORMAT, MY_NAMESPACE);
+ final Map<String,Object> rule2_2 = new HashMap<>();
+ rule2_2.put(AttributeTranscoderRegistry.PROP_ID, MY_NAME_2);
+ rule2_2.put(AttributeTranscoderRegistry.PROP_TRANSCODER, transcoder);
+ rule2_2.put(SAML2AttributeTranscoder.PROP_NAME, MY_ALTNAME_1);
+ rule2_2.put(SAML2AttributeTranscoder.PROP_NAME_FORMAT, MY_NAMESPACE);
+
final Map<String,Object> rule3_1 = new HashMap<>();
rule3_1.put(AttributeTranscoderRegistry.PROP_ID, MY_NAME_3);
rule3_1.put(AttributeTranscoderRegistry.PROP_TRANSCODER, transcoder);
@@ -135,6 +146,7 @@ public class AddAttributeStatementToAssertionTest extends OpenSAMLInitBaseTestCa
new TranscodingRule(rule1_1),
new TranscodingRule(rule1_2),
new TranscodingRule(rule2_1),
+ new TranscodingRule(rule2_2),
new TranscodingRule(rule3_1)));
registry.setApplicationContext(new MockApplicationContext());
registry.initialize();
@@ -398,19 +410,27 @@ public class AddAttributeStatementToAssertionTest extends OpenSAMLInitBaseTestCa
boolean one = false, altone = false, two = false;
for (final Attribute samlAttr : attributeStatement.getAttributes()) {
- Assert.assertNotNull(samlAttr.getAttributeValues());
- Assert.assertEquals(samlAttr.getAttributeValues().size(), 1);
- final XMLObject xmlObject = samlAttr.getAttributeValues().get(0);
- Assert.assertTrue(xmlObject instanceof XSStringImpl);
if (samlAttr.getName().equals(MY_NAME_1)) {
+ Assert.assertEquals(samlAttr.getAttributeValues().size(), 1);
+ final XMLObject xmlObject = samlAttr.getAttributeValues().get(0);
Assert.assertEquals(((XSStringImpl) xmlObject).getValue(), MY_VALUE_1);
one = true;
} else if (samlAttr.getName().equals(MY_NAME_2)) {
+ Assert.assertEquals(samlAttr.getAttributeValues().size(), 1);
+ final XMLObject xmlObject = samlAttr.getAttributeValues().get(0);
Assert.assertEquals(((XSStringImpl) xmlObject).getValue(), MY_VALUE_2);
altone = true;
} else if (samlAttr.getName().equals(MY_ALTNAME_1)) {
- Assert.assertEquals(((XSStringImpl) xmlObject).getValue(), MY_VALUE_1);
- two = true;
+ Assert.assertEquals(samlAttr.getAttributeValues().size(), 2);
+ final String val1 = ((XSStringImpl) samlAttr.getAttributeValues().get(0)).getValue();
+ final String val2 = ((XSStringImpl) samlAttr.getAttributeValues().get(1)).getValue();
+ if (val1.equals(MY_VALUE_1)) {
+ Assert.assertEquals(val2, MY_VALUE_2);
+ two = true;
+ } else if (val2.equals(MY_VALUE_1)) {
+ Assert.assertEquals(val1, MY_VALUE_2);
+ two = true;
+ }
} else {
Assert.fail("Incorrect attribute name.");
}
@@ -418,7 +438,7 @@ public class AddAttributeStatementToAssertionTest extends OpenSAMLInitBaseTestCa
if (!one || !altone || !two) {
- Assert.fail("Missing attribute");
+ Assert.fail("Missing attribute/value");
}
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list