[java-shib-attribute] 02/03: IDP-2375 Aliased decoded IdPAttributes are lost during subsequent use
Rod Widdowson
rdw at steadingsoftware.com
Tue Apr 22 09:21:16 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=90a2f3b07c2b7d25242799c5c227c7d834c77246
commit 90a2f3b07c2b7d25242799c5c227c7d834c77246
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Mon Apr 21 17:08:39 2025 +0100
IDP-2375 Aliased decoded IdPAttributes are lost during subsequent use
https://shibboleth.atlassian.net/browse/IDP-2375
Change previous fix to allow setting of a map (rather than from a list). Allows the
callers to control what happens to duplicates
---
.../idp/attribute/IdPAttributeSupport.java | 88 ++++++++++++++++++++++
.../idp/attribute/context/AttributeContext.java | 80 +++-----------------
.../idp/attribute/AttributeContextTest.java | 29 ++++++-
.../filter/context/AttributeFilterContext.java | 45 +++--------
.../filter/context/AttributeFilterContextTest.java | 19 ++++-
.../attribute/filter/impl/AttributeFilterImpl.java | 2 +-
6 files changed, 154 insertions(+), 109 deletions(-)
diff --git a/shib-attribute-api/src/main/java/net/shibboleth/idp/attribute/IdPAttributeSupport.java b/shib-attribute-api/src/main/java/net/shibboleth/idp/attribute/IdPAttributeSupport.java
new file mode 100644
index 000000000..044d293b4
--- /dev/null
+++ b/shib-attribute-api/src/main/java/net/shibboleth/idp/attribute/IdPAttributeSupport.java
@@ -0,0 +1,88 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.shibboleth.idp.attribute;
+
+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;
+
+import net.shibboleth.shared.collection.CollectionSupport;
+
+/**
+ * Class for general support functions for managing IdPAttributes.
+ */
+public class IdPAttributeSupport {
+
+ /** Convert a Buckets of attributes into a Map of attributes.
+ * If an attribute with a duplicate Id is found then we issue a warning and
+ * an arbitrary attribute is chosen.
+ * @param attributes a collection of attributes.
+ * @return These attributes in a map where the key is the attribute's ID and the value the attributes
+ */
+ @Nonnull public static Map<String, IdPAttribute> toMapNoDuplicates(@Nullable Collection<IdPAttribute> attributes) {
+ if (attributes == null) {
+ return CollectionSupport.emptyMap();
+ }
+ return attributes.
+ stream().
+ collect(CollectionSupport.nonnullCollector(Collectors.toUnmodifiableMap(IdPAttribute::getId,
+ a -> a,
+ CollectionSupport.warningMergeFunction("AttrtibuteContext", true)))).get();
+ }
+
+
+ /** Convert a Buckets of attributes into a Map of attributes.
+ * If an attribute with a duplicate Id is found then we merge the values into a single attribute.
+ * @param attributes a collection of attributes.
+ * @return These attributes in a map where the key is the attribute's ID and the value the attributes
+ * @throws AttributeEncodingException if a clone operation fails.
+ */
+ @Nonnull public static Map<String, IdPAttribute> toMapMergeDuplicates(@Nullable Collection<IdPAttribute> attributes) {
+ final Map<String, IdPAttribute> accumulator;
+ if (attributes == null) {
+ return CollectionSupport.emptyMap();
+ }
+
+ accumulator = new HashMap<>(attributes.size());
+ for (IdPAttribute attribute:attributes) {
+
+ final IdPAttribute oldAttr = accumulator.get(attribute.getId());
+ if (oldAttr == null) {
+ accumulator.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<IdPAttributeValue>(newAttribute.getValues());
+ values.addAll(attribute.getValues());
+ newAttribute.setValues(values);
+ accumulator.remove(attribute.getId());
+ accumulator.put(newAttribute.getId(), newAttribute);
+ }
+ }
+
+ return Collections.unmodifiableMap(accumulator);
+ }
+}
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 de4ec9573..8082bb385 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,11 +14,8 @@
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 javax.annotation.Nonnull;
@@ -28,7 +25,7 @@ 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.idp.attribute.IdPAttributeSupport;
import net.shibboleth.shared.annotation.constraint.NotLive;
import net.shibboleth.shared.annotation.constraint.Unmodifiable;
import net.shibboleth.shared.collection.CollectionSupport;
@@ -66,29 +63,6 @@ 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.
*
@@ -99,16 +73,8 @@ public final class AttributeContext extends BaseContext {
@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));
+ DeprecationSupport.warn(ObjectType.METHOD, "setIdPAttributes(Collection)", "AttributeContext", "setIdPAttributes(Map)");
+ return setIdPAttributes(IdPAttributeSupport.toMapMergeDuplicates(newAttributes));
}
/**
@@ -118,17 +84,14 @@ public final class AttributeContext extends BaseContext {
*
* @return this context
*/
- @Nonnull public AttributeContext setIdPAttributesFromList(@Nullable final List<IdPAttribute> newAttributes) {
-
- if (newAttributes != null) {
- attributes = mergeIdPAttributes(newAttributes);
- } else {
- attributes = CollectionSupport.emptyMap();
- }
+ @Nonnull public AttributeContext setIdPAttributes(@Nullable final Map<String, IdPAttribute> newAttributes) {
+
+ attributes = Collections.unmodifiableMap(newAttributes);
return this;
}
+
/**
* Gets the map of unfiltered attributes, indexed by attribute ID, tracked by this context.
*
@@ -148,36 +111,17 @@ public final class AttributeContext extends BaseContext {
@Deprecated(since = "5.2.0", forRemoval = true)
@Nonnull public AttributeContext setUnfilteredIdPAttributes(@Nullable final Collection<IdPAttribute> newAttributes) {
- DeprecationSupport.warn(ObjectType.METHOD, "setUnfilteredIdPAttributes", "AttributeContext", "setUnfilteredIdPAttributesFromList");
+ DeprecationSupport.warn(ObjectType.METHOD, "setUnfilteredIdPAttributes(Collection)", "AttributeContext", "setUnfilteredIdPAttributes(Map)");
- if (newAttributes == null) {
- unfilteredAttributes = CollectionSupport.emptyMap();
- return this;
- }
- if (newAttributes instanceof List) {
- return setUnfilteredIdPAttributesFromList((List<IdPAttribute>) newAttributes);
- }
- return setUnfilteredIdPAttributesFromList(new ArrayList<IdPAttribute>(newAttributes));
+ return setUnfilteredIdPAttributes(IdPAttributeSupport.toMapMergeDuplicates(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();
- }
-
+ @Nonnull public AttributeContext setUnfilteredIdPAttributes(@Nullable final Map<String, IdPAttribute> newAttributes) {
+ unfilteredAttributes = Collections.unmodifiableMap(newAttributes);
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 fff738c1d..2b498c2a4 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
@@ -94,11 +94,25 @@ public class AttributeContextTest {
assertEquals(attrs.get("attribute2").getValues().size(), 4);
assertEquals(attrs.get("attribute3").getValues().size(), 3);
- context.setIdPAttributes(null);
+ context.setIdPAttributes((List<IdPAttribute>)null);
attrs = context.getIdPAttributes();
assertEquals(attrs.size(), 0);
}
+ @Test public void testAttributeNoMerge() {
+
+ final AttributeContext context = new AttributeContext();
+ context.setIdPAttributes(IdPAttributeSupport.toMapNoDuplicates(getMergeList()));
+
+ Map<String, IdPAttribute> attrs = context.getIdPAttributes();
+
+ assertEquals(attrs.size(), 3);
+ assertNull(attrs.get("atribute4"));
+ assertEquals(attrs.get("attribute1").getValues().size(), 2);
+ assertEquals(attrs.get("attribute2").getValues().size(), 2);
+ assertEquals(attrs.get("attribute3").getValues().size(), 3);
+ }
+
@Test public void testUnfilteredMerge() {
final AttributeContext context = new AttributeContext();
context.setUnfilteredIdPAttributes(getMergeList());
@@ -110,4 +124,17 @@ public class AttributeContextTest {
assertEquals(attrs.get("attribute2").getValues().size(), 4);
assertEquals(attrs.get("attribute3").getValues().size(), 3);
}
+
+ @Test public void testUnfilteredNoMerge() {
+ final AttributeContext context = new AttributeContext();
+ context.setUnfilteredIdPAttributes(IdPAttributeSupport.toMapNoDuplicates(getMergeList()));
+
+ Map<String, IdPAttribute> attrs = context.getUnfilteredIdPAttributes();
+ assertEquals(attrs.size(), 3);
+ assertNull(attrs.get("atribute4"));
+ assertEquals(attrs.get("attribute1").getValues().size(), 2);
+ assertEquals(attrs.get("attribute2").getValues().size(), 2);
+ 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 2ebe229e3..26fd84f98 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,13 +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;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
@@ -35,7 +32,7 @@ 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.IdPAttributeSupport;
import net.shibboleth.idp.attribute.filter.AttributeFilter;
import net.shibboleth.idp.attribute.filter.AttributeFilterException;
import net.shibboleth.shared.annotation.constraint.NotLive;
@@ -43,8 +40,8 @@ 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.primitive.LoggerFactory;
import net.shibboleth.shared.service.ReloadableService;
import net.shibboleth.shared.service.ServiceException;
import net.shibboleth.shared.service.ServiceableComponent;
@@ -224,25 +221,18 @@ public final class AttributeFilterContext extends BaseContext {
/**
* Sets the attributes which are to be filtered.
- *
+ *
* @param attributes attributes which are to be filtered
- *
+ *
* @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");
+ DeprecationSupport.warn(ObjectType.METHOD, "setPrefilteredIdPAttributes(Collection)", "AttributeFilterContext", "setPrefilteredIdPAttributes(Map)");
- if (attributes == null) {
- prefilteredAttributes = CollectionSupport.emptyMap();
- return this;
- }
- if (attributes instanceof List) {
- return setPrefilteredIdPAttributesFromList((List<IdPAttribute>) attributes);
- }
- return setPrefilteredIdPAttributesFromList(new ArrayList<IdPAttribute>(attributes));
+ return setPrefilteredIdPAttributes(IdPAttributeSupport.toMapMergeDuplicates(attributes));
}
/**
@@ -252,19 +242,14 @@ public final class AttributeFilterContext extends BaseContext {
*
* @return this context;
*/
- @Nonnull public AttributeFilterContext setPrefilteredIdPAttributesFromList(
- @Nullable final List<IdPAttribute> attributes) {
+ @Nonnull public AttributeFilterContext setPrefilteredIdPAttributes(
+ @Nullable final Map<String, IdPAttribute> attributes) {
- if (attributes != null) {
- prefilteredAttributes = AttributeContext.mergeIdPAttributes(attributes);
- } else {
- prefilteredAttributes = CollectionSupport.emptyMap();
- }
+ prefilteredAttributes = Collections.unmodifiableMap(attributes);
return this;
}
-
/**
* Gets the collection of attributes, indexed by ID, left after the filtering process has run.
*
@@ -287,17 +272,7 @@ public final class AttributeFilterContext extends BaseContext {
DeprecationSupport.warn(ObjectType.METHOD, "setFilteredIdPAttributes(Collection)", "AttributeFilterContext", "setFilteredIdPAttributes(Map)");
- if (attributes != null) {
- filteredAttributes = attributes.
- stream().
- collect(CollectionSupport.nonnullCollector(Collectors.toUnmodifiableMap(IdPAttribute::getId,
- e -> e,
- CollectionSupport.warningMergeFunction("AttrtibuteFilterContextFiltered", true)))).get();
- } else {
- filteredAttributes = CollectionSupport.emptyMap();
- }
-
- return this;
+ return setFilteredIdPAttributes(IdPAttributeSupport.toMapMergeDuplicates(attributes));
}
/**
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 f7f41d1be..f352e4206 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
@@ -22,6 +22,7 @@ import static org.testng.Assert.assertSame;
import static org.testng.Assert.assertTrue;
import java.util.Arrays;
+import java.util.Collection;
import java.util.List;
import java.util.Map;
@@ -31,6 +32,7 @@ import org.testng.Assert;
import org.testng.annotations.Test;
import net.shibboleth.idp.attribute.IdPAttribute;
+import net.shibboleth.idp.attribute.IdPAttributeSupport;
import net.shibboleth.idp.attribute.IdPAttributeValue;
import net.shibboleth.idp.attribute.StringAttributeValue;
import net.shibboleth.shared.collection.CollectionSupport;
@@ -77,7 +79,7 @@ public class AttributeFilterContextTest {
assertTrue(context.getPrefilteredIdPAttributes().containsKey("attribute3"));
assertEquals(context.getPrefilteredIdPAttributes().get("attribute3"), attribute3);
- context.setPrefilteredIdPAttributes(null);
+ context.setPrefilteredIdPAttributes((List<IdPAttribute>)null);
assertNotNull(context.getPrefilteredIdPAttributes());
assertTrue(context.getPrefilteredIdPAttributes().isEmpty());
}
@@ -168,15 +170,24 @@ public class AttributeFilterContextTest {
attribute3.setValues(CollectionSupport.listOf(av1, av2, av1));
final AttributeFilterContext context = new AttributeFilterContext();
- context.setPrefilteredIdPAttributes(
- CollectionSupport.listOf(attribute1a, attribute1b, attribute2a, attribute2b, attribute3));
+ Collection<IdPAttribute> attributes = CollectionSupport.listOf(attribute1a, attribute1b, attribute2a, attribute2b, attribute3);
+ context.setPrefilteredIdPAttributes( attributes );
- final Map<String, IdPAttribute> attrs = context.getPrefilteredIdPAttributes();
+ Map<String, IdPAttribute> attrs = context.getPrefilteredIdPAttributes();
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.setPrefilteredIdPAttributes(IdPAttributeSupport.toMapNoDuplicates(attributes));
+ attrs = context.getPrefilteredIdPAttributes();
+
+ assertEquals(attrs.size(), 3);
+ assertNull(attrs.get("atribute4"));
+ assertEquals(attrs.get("attribute1").getValues().size(), 2);
+ assertEquals(attrs.get("attribute2").getValues().size(), 2);
+ assertEquals(attrs.get("attribute3").getValues().size(), 3);
}
}
diff --git a/shib-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/impl/AttributeFilterImpl.java b/shib-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/impl/AttributeFilterImpl.java
index 8def4a0ef..bec49ecbf 100644
--- a/shib-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/impl/AttributeFilterImpl.java
+++ b/shib-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/impl/AttributeFilterImpl.java
@@ -163,7 +163,7 @@ public class AttributeFilterImpl extends AbstractIdentifiableInitializableCompon
filteredAttribute.setValues(values);
allFilteredAttributes.put(filteredAttribute.getId(), filteredAttribute);
}
- filterContext.setFilteredIdPAttributes(allFilteredAttributes.values());
+ filterContext.setFilteredIdPAttributes(allFilteredAttributes);
}
} finally {
if (timerStarted) {
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list