[java-shib-attribute] branch main updated: IDP-2375 Aliased decoded IdPAttributes are lost during subsequent use
Rod Widdowson
rdw at steadingsoftware.com
Mon Apr 21 09:58:51 UTC 2025
This is an automated email from the git hooks/post-receive script.
rdw pushed a commit to branch main
in repository java-shib-attribute.
View the commit online:
http://git.shibboleth.net/view/?p=java-shib-attribute.git;a=commit;h=9852b77c4168dbdceca855d5a5680bf8cfce6f88
The following commit(s) were added to refs/heads/main by this push:
new 9852b77c4 IDP-2375 Aliased decoded IdPAttributes are lost during subsequent use
9852b77c4 is described below
commit 9852b77c4168dbdceca855d5a5680bf8cfce6f88
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Mon Apr 21 10:55:37 2025 +0100
IDP-2375 Aliased decoded IdPAttributes are lost during subsequent use
https://shibboleth.atlassian.net/browse/IDP-2375
Add explicit duplicate handling to the AttributeContext and AttributeFilteContext.
Plus tests.
---
.../idp/attribute/context/AttributeContext.java | 100 +++++++++++++++++----
.../idp/attribute/AttributeContextTest.java | 28 +++++-
.../filter/context/AttributeFilterContext.java | 66 ++++++++++++--
.../filter/context/AttributeFilterContextTest.java | 26 ++++--
4 files changed, 180 insertions(+), 40 deletions(-)
diff --git a/shib-attribute-api/src/main/java/net/shibboleth/idp/attribute/context/AttributeContext.java b/shib-attribute-api/src/main/java/net/shibboleth/idp/attribute/context/AttributeContext.java
index 373ec9465..de4ec9573 100644
--- a/shib-attribute-api/src/main/java/net/shibboleth/idp/attribute/context/AttributeContext.java
+++ b/shib-attribute-api/src/main/java/net/shibboleth/idp/attribute/context/AttributeContext.java
@@ -14,9 +14,12 @@
package net.shibboleth.idp.attribute.context;
+import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
import java.util.Map;
-import java.util.stream.Collectors;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
@@ -25,9 +28,12 @@ import javax.annotation.concurrent.NotThreadSafe;
import org.opensaml.messaging.context.BaseContext;
import net.shibboleth.idp.attribute.IdPAttribute;
+import net.shibboleth.idp.attribute.IdPAttributeValue;
import net.shibboleth.shared.annotation.constraint.NotLive;
import net.shibboleth.shared.annotation.constraint.Unmodifiable;
import net.shibboleth.shared.collection.CollectionSupport;
+import net.shibboleth.shared.primitive.DeprecationSupport;
+import net.shibboleth.shared.primitive.DeprecationSupport.ObjectType;
/**
* A {@link BaseContext} that tracks a set of attributes. Usually the tracked attributes are about a particular user and
@@ -60,6 +66,29 @@ public final class AttributeContext extends BaseContext {
return attributes;
}
+ /** Helper function to construct a {@link Map<String, IdPAttribute>} from a {@link List<IdPAttribute>}
+ * handling dulicates by appending the {@link IdPAttributeValue}.
+ * @param newAttributes The list of Atttribute, may contain duplicates
+ * @return the map.
+ */
+ @Nonnull @NotLive public static Map<String, IdPAttribute> mergeIdPAttributes(@Nonnull List<IdPAttribute> newAttributes) {
+
+ final Map<String, IdPAttribute> accumulator;
+ accumulator = new HashMap<>(newAttributes.size());
+ for (IdPAttribute attribute:newAttributes) {
+ IdPAttribute oldAttr = accumulator.get(attribute.getId());
+ if (oldAttr == null) {
+ accumulator.put(attribute.getId(), attribute);
+ } else {
+ List<IdPAttributeValue> oldValues = new ArrayList<IdPAttributeValue>(oldAttr.getValues());
+ oldValues.addAll(attribute.getValues());
+ oldAttr.setValues(oldValues);
+ }
+ }
+
+ return Collections.unmodifiableMap(accumulator);
+ }
+
/**
* Sets the attributes tracked by this context.
*
@@ -67,22 +96,39 @@ public final class AttributeContext extends BaseContext {
*
* @return this context
*/
+ @Deprecated(since = "5.2.0", forRemoval = true)
@Nonnull public AttributeContext setIdPAttributes(@Nullable final Collection<IdPAttribute> newAttributes) {
+
+ DeprecationSupport.warn(ObjectType.METHOD, "setIdPAttributes", "AttributeContext", "setIdPAttributesFromList");
+
+ if (newAttributes == null) {
+ attributes = CollectionSupport.emptyMap();
+ return this;
+ }
+ if (newAttributes instanceof List) {
+ return setIdPAttributesFromList((List<IdPAttribute>) newAttributes);
+ }
+ return setIdPAttributesFromList(new ArrayList<IdPAttribute>(newAttributes));
+ }
+
+ /**
+ * Sets the attributes tracked by this context.
+ *
+ * @param newAttributes the attributes
+ *
+ * @return this context
+ */
+ @Nonnull public AttributeContext setIdPAttributesFromList(@Nullable final List<IdPAttribute> newAttributes) {
if (newAttributes != null) {
- attributes = newAttributes.
- stream().
- collect(CollectionSupport.nonnullCollector(Collectors.toUnmodifiableMap(IdPAttribute::getId,
- a -> a,
- CollectionSupport.warningMergeFunction("AttrtibuteContext", true)))).get();
+ attributes = mergeIdPAttributes(newAttributes);
} else {
attributes = CollectionSupport.emptyMap();
}
return this;
}
-
-
+
/**
* Gets the map of unfiltered attributes, indexed by attribute ID, tracked by this context.
*
@@ -99,21 +145,39 @@ public final class AttributeContext extends BaseContext {
*
* @return this context
*/
- @Nonnull public AttributeContext setUnfilteredIdPAttributes(
- @Nullable final Collection<IdPAttribute> newAttributes) {
- if (null != newAttributes) {
- unfilteredAttributes = newAttributes.
- stream().
- collect(CollectionSupport.nonnullCollector(Collectors.toUnmodifiableMap(IdPAttribute::getId,
- a -> a,
- CollectionSupport.warningMergeFunction("AttrtibuteContextUnfiltered", true)))).get();
+ @Deprecated(since = "5.2.0", forRemoval = true)
+ @Nonnull public AttributeContext setUnfilteredIdPAttributes(@Nullable final Collection<IdPAttribute> newAttributes) {
+
+ DeprecationSupport.warn(ObjectType.METHOD, "setUnfilteredIdPAttributes", "AttributeContext", "setUnfilteredIdPAttributesFromList");
+
+ if (newAttributes == null) {
+ unfilteredAttributes = CollectionSupport.emptyMap();
+ return this;
+ }
+ if (newAttributes instanceof List) {
+ return setUnfilteredIdPAttributesFromList((List<IdPAttribute>) newAttributes);
+ }
+ return setUnfilteredIdPAttributesFromList(new ArrayList<IdPAttribute>(newAttributes));
+ }
+
+ /**
+ * Sets the unfiltered attributes tracked by this context.
+ *
+ * @param newAttributes the attributes
+ *
+ * @return this context
+ */
+ @Nonnull public AttributeContext setUnfilteredIdPAttributesFromList(@Nullable final List<IdPAttribute> newAttributes) {
+
+ if (newAttributes != null) {
+ unfilteredAttributes = mergeIdPAttributes(newAttributes);
} else {
unfilteredAttributes = CollectionSupport.emptyMap();
}
-
+
return this;
}
-
+
/**
* Gets whether attribute release consent was obtained from the subject during this request but not stored.
*
diff --git a/shib-attribute-api/src/test/java/net/shibboleth/idp/attribute/AttributeContextTest.java b/shib-attribute-api/src/test/java/net/shibboleth/idp/attribute/AttributeContextTest.java
index b2053b57d..fff738c1d 100644
--- a/shib-attribute-api/src/test/java/net/shibboleth/idp/attribute/AttributeContextTest.java
+++ b/shib-attribute-api/src/test/java/net/shibboleth/idp/attribute/AttributeContextTest.java
@@ -18,6 +18,7 @@ import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNull;
import java.util.Arrays;
+import java.util.List;
import java.util.Map;
import org.testng.Assert;
@@ -28,6 +29,7 @@ import net.shibboleth.shared.collection.CollectionSupport;
/** Unit test for {@link AttributeContext} class. */
+ at SuppressWarnings( "removal" )
public class AttributeContextTest {
/**
@@ -57,8 +59,8 @@ public class AttributeContextTest {
context.setIdPAttributes(CollectionSupport.emptySet());
contextAttributes(context, 0);
}
- @Test(enabled = false) public void testAttributeMerge() {
+ private List<IdPAttribute> getMergeList() {
final IdPAttribute attribute1a = new IdPAttribute("attribute1");
final IdPAttribute attribute1b = new IdPAttribute("attribute1");
final IdPAttribute attribute2a = new IdPAttribute("attribute2");
@@ -76,18 +78,36 @@ public class AttributeContextTest {
attribute2b.setValues(CollectionSupport.listOf(av3, av4));
attribute3.setValues(CollectionSupport.listOf(av1, av2, av1));
+ return CollectionSupport.listOf(attribute1a, attribute1b, attribute2a, attribute2b, attribute3);
+ }
+
+ @Test public void testAttributeMerge() {
final AttributeContext context = new AttributeContext();
- context.setIdPAttributes(
- CollectionSupport.listOf(attribute1a, attribute1b, attribute2a, attribute2b, attribute3));
+ context.setIdPAttributes(getMergeList());
- final Map<String, IdPAttribute> attrs = context.getIdPAttributes();
+ Map<String, IdPAttribute> attrs = context.getIdPAttributes();
assertEquals(attrs.size(), 3);
assertNull(attrs.get("atribute4"));
assertEquals(attrs.get("attribute1").getValues().size(), 4);
assertEquals(attrs.get("attribute2").getValues().size(), 4);
assertEquals(attrs.get("attribute3").getValues().size(), 3);
+
+ context.setIdPAttributes(null);
+ attrs = context.getIdPAttributes();
+ assertEquals(attrs.size(), 0);
}
+ @Test public void testUnfilteredMerge() {
+ final AttributeContext context = new AttributeContext();
+ context.setUnfilteredIdPAttributes(getMergeList());
+
+ Map<String, IdPAttribute> attrs = context.getUnfilteredIdPAttributes();
+ assertEquals(attrs.size(), 3);
+ assertNull(attrs.get("atribute4"));
+ assertEquals(attrs.get("attribute1").getValues().size(), 4);
+ assertEquals(attrs.get("attribute2").getValues().size(), 4);
+ assertEquals(attrs.get("attribute3").getValues().size(), 3);
+ }
}
diff --git a/shib-attribute-filter-api/src/main/java/net/shibboleth/idp/attribute/filter/context/AttributeFilterContext.java b/shib-attribute-filter-api/src/main/java/net/shibboleth/idp/attribute/filter/context/AttributeFilterContext.java
index cdeca6ef8..2ebe229e3 100644
--- a/shib-attribute-filter-api/src/main/java/net/shibboleth/idp/attribute/filter/context/AttributeFilterContext.java
+++ b/shib-attribute-filter-api/src/main/java/net/shibboleth/idp/attribute/filter/context/AttributeFilterContext.java
@@ -14,7 +14,10 @@
package net.shibboleth.idp.attribute.filter.context;
+import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Collectors;
@@ -32,13 +35,16 @@ import org.opensaml.saml.metadata.resolver.MetadataResolver;
import org.slf4j.Logger;
import net.shibboleth.idp.attribute.IdPAttribute;
+import net.shibboleth.idp.attribute.context.AttributeContext;
import net.shibboleth.idp.attribute.filter.AttributeFilter;
import net.shibboleth.idp.attribute.filter.AttributeFilterException;
import net.shibboleth.shared.annotation.constraint.NotLive;
import net.shibboleth.shared.annotation.constraint.Unmodifiable;
import net.shibboleth.shared.collection.CollectionSupport;
import net.shibboleth.shared.logic.Constraint;
+import net.shibboleth.shared.primitive.DeprecationSupport;
import net.shibboleth.shared.primitive.LoggerFactory;
+import net.shibboleth.shared.primitive.DeprecationSupport.ObjectType;
import net.shibboleth.shared.service.ReloadableService;
import net.shibboleth.shared.service.ServiceException;
import net.shibboleth.shared.service.ServiceableComponent;
@@ -223,25 +229,45 @@ public final class AttributeFilterContext extends BaseContext {
*
* @return this context;
*/
+ @Deprecated(since = "5.2.0", forRemoval = true)
@Nonnull public AttributeFilterContext setPrefilteredIdPAttributes(
@Nullable final Collection<IdPAttribute> attributes) {
+ DeprecationSupport.warn(ObjectType.METHOD, "setPrefilteredIdPAttributes", "AttributeFilterContext", "setPrefilteredIdPAttributesFromList");
+
+ if (attributes == null) {
+ prefilteredAttributes = CollectionSupport.emptyMap();
+ return this;
+ }
+ if (attributes instanceof List) {
+ return setPrefilteredIdPAttributesFromList((List<IdPAttribute>) attributes);
+ }
+ return setPrefilteredIdPAttributesFromList(new ArrayList<IdPAttribute>(attributes));
+ }
+
+ /**
+ * Sets the attributes which are to be filtered.
+ *
+ * @param attributes attributes which are to be filtered
+ *
+ * @return this context;
+ */
+ @Nonnull public AttributeFilterContext setPrefilteredIdPAttributesFromList(
+ @Nullable final List<IdPAttribute> attributes) {
+
if (attributes != null) {
- prefilteredAttributes = attributes.
- stream().
- collect(CollectionSupport.nonnullCollector(
- Collectors.toUnmodifiableMap(IdPAttribute::getId, e -> e,
- CollectionSupport.warningMergeFunction("AttrtibuteFilterContextPrefiltered", true)))).get();
+ prefilteredAttributes = AttributeContext.mergeIdPAttributes(attributes);
} else {
prefilteredAttributes = CollectionSupport.emptyMap();
}
-
+
return this;
}
+
/**
* Gets the collection of attributes, indexed by ID, left after the filtering process has run.
- *
+ *
* @return attributes left after the filtering process has run
*/
@Nonnull @Unmodifiable @NotLive public Map<String, IdPAttribute> getFilteredIdPAttributes() {
@@ -250,14 +276,17 @@ public final class AttributeFilterContext extends BaseContext {
/**
* Sets the attributes that have been filtered.
- *
+ *
* @param attributes attributes that have been filtered
- *
+ *
* @return this context
*/
+ @Deprecated(since = "5.2.0", forRemoval = true)
@Nonnull public AttributeFilterContext setFilteredIdPAttributes(
@Nullable final Collection<IdPAttribute> attributes) {
+ DeprecationSupport.warn(ObjectType.METHOD, "setFilteredIdPAttributes(Collection)", "AttributeFilterContext", "setFilteredIdPAttributes(Map)");
+
if (attributes != null) {
filteredAttributes = attributes.
stream().
@@ -271,6 +300,25 @@ public final class AttributeFilterContext extends BaseContext {
return this;
}
+ /**
+ * Sets the attributes that have been filtered.
+ *
+ * @param attributes attributes that have been filtered
+ *
+ * @return this context
+ */
+ @Nonnull public AttributeFilterContext setFilteredIdPAttributes(
+ @Nullable final Map<String, IdPAttribute> attributes) {
+
+ if (attributes == null) {
+ filteredAttributes = CollectionSupport.emptyMap();
+ } else {
+ filteredAttributes = Collections.unmodifiableMap(attributes);
+ }
+
+ return this;
+ }
+
/**
* Get supplemental source of metadata for filtering rules.
*
diff --git a/shib-attribute-filter-api/src/test/java/net/shibboleth/idp/attribute/filter/context/AttributeFilterContextTest.java b/shib-attribute-filter-api/src/test/java/net/shibboleth/idp/attribute/filter/context/AttributeFilterContextTest.java
index d5f81ff8e..f7f41d1be 100644
--- a/shib-attribute-filter-api/src/test/java/net/shibboleth/idp/attribute/filter/context/AttributeFilterContextTest.java
+++ b/shib-attribute-filter-api/src/test/java/net/shibboleth/idp/attribute/filter/context/AttributeFilterContextTest.java
@@ -21,26 +21,22 @@ import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertSame;
import static org.testng.Assert.assertTrue;
-
import java.util.Arrays;
import java.util.List;
import java.util.Map;
-import net.shibboleth.idp.attribute.IdPAttribute;
-import net.shibboleth.idp.attribute.IdPAttributeValue;
-import net.shibboleth.idp.attribute.StringAttributeValue;
-import net.shibboleth.shared.collection.CollectionSupport;
-
import org.opensaml.messaging.context.navigate.ChildContextLookup;
import org.opensaml.saml.common.messaging.context.SAMLMetadataContext;
import org.testng.Assert;
import org.testng.annotations.Test;
import net.shibboleth.idp.attribute.IdPAttribute;
+import net.shibboleth.idp.attribute.IdPAttributeValue;
+import net.shibboleth.idp.attribute.StringAttributeValue;
import net.shibboleth.shared.collection.CollectionSupport;
/** Unit test for {@link AttributeFilterContext}. */
- at SuppressWarnings("javadoc")
+ at SuppressWarnings({"javadoc","removal"})
public class AttributeFilterContextTest {
/** Test that post-construction state is what is expected. */
@@ -107,7 +103,19 @@ public class AttributeFilterContextTest {
assertTrue(context.getFilteredIdPAttributes().containsKey("attribute3"));
assertEquals(context.getFilteredIdPAttributes().get("attribute3"), attribute3);
- context.setFilteredIdPAttributes(null);
+ context.setFilteredIdPAttributes(context.getFilteredIdPAttributes());
+ assertEquals(context.getFilteredIdPAttributes().size(), 2);
+ assertFalse(context.getFilteredIdPAttributes().containsKey("attribute1"));
+ assertTrue(context.getFilteredIdPAttributes().containsKey("attribute2"));
+ assertEquals(context.getFilteredIdPAttributes().get("attribute2"), attribute2);
+ assertTrue(context.getFilteredIdPAttributes().containsKey("attribute3"));
+ assertEquals(context.getFilteredIdPAttributes().get("attribute3"), attribute3);
+
+ context.setFilteredIdPAttributes((List<IdPAttribute>)null);
+ assertNotNull(context.getFilteredIdPAttributes());
+ assertTrue(context.getFilteredIdPAttributes().isEmpty());
+
+ context.setFilteredIdPAttributes((Map<String, IdPAttribute>)null);
assertNotNull(context.getFilteredIdPAttributes());
assertTrue(context.getFilteredIdPAttributes().isEmpty());
}
@@ -139,7 +147,7 @@ public class AttributeFilterContextTest {
assertSame(context.getRequesterMetadataContext(), mas);
}
- @Test(enabled = false) public void testAttributeMerge() {
+ @Test public void testAttributeMerge() {
final IdPAttribute attribute1a = new IdPAttribute("attribute1");
final IdPAttribute attribute1b = new IdPAttribute("attribute1");
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list