[java-opensaml] branch master updated: OSJ-308 - Honor ordered BindingCriterion precedence
Scott Cantor
cantor.2 at osu.edu
Mon Jun 8 23:21:19 UTC 2020
This is an automated email from the git hooks/post-receive script.
scantor pushed a commit to branch master
in repository java-opensaml.
View the commit online:
http://git.shibboleth.net/view/?p=java-opensaml.git;a=commit;h=a69d6fbd9481b01dc78585f3e26df10abbde5435
The following commit(s) were added to refs/heads/master by this push:
new a69d6fbd9 OSJ-308 - Honor ordered BindingCriterion precedence
a69d6fbd9 is described below
commit a69d6fbd9481b01dc78585f3e26df10abbde5435
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Apr 6 19:36:39 2020 -0400
OSJ-308 - Honor ordered BindingCriterion precedence
https://issues.shibboleth.net/jira/browse/OSJ-308
---
.../common/binding/AbstractEndpointResolver.java | 76 +++++++++++++++++++---
.../opensaml/saml/criterion/BindingCriterion.java | 4 +-
.../binding/impl/DefaultEndpointResolver.java | 11 ++--
.../binding/impl/DefaultEndpointResolverTest.java | 70 +++++++++++++++++++-
4 files changed, 145 insertions(+), 16 deletions(-)
diff --git a/opensaml-saml-api/src/main/java/org/opensaml/saml/common/binding/AbstractEndpointResolver.java b/opensaml-saml-api/src/main/java/org/opensaml/saml/common/binding/AbstractEndpointResolver.java
index 837e81a0e..881111af0 100644
--- a/opensaml-saml-api/src/main/java/org/opensaml/saml/common/binding/AbstractEndpointResolver.java
+++ b/opensaml-saml-api/src/main/java/org/opensaml/saml/common/binding/AbstractEndpointResolver.java
@@ -22,6 +22,7 @@ import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
+import java.util.stream.Collectors;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
@@ -29,9 +30,11 @@ import javax.xml.namespace.QName;
import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
import net.shibboleth.utilities.java.support.component.AbstractIdentifiedInitializableComponent;
+import net.shibboleth.utilities.java.support.component.ComponentSupport;
import net.shibboleth.utilities.java.support.resolver.CriteriaSet;
import net.shibboleth.utilities.java.support.resolver.ResolverException;
+import org.opensaml.saml.criterion.BindingCriterion;
import org.opensaml.saml.criterion.EndpointCriterion;
import org.opensaml.saml.criterion.RoleDescriptorCriterion;
import org.opensaml.saml.saml2.metadata.Endpoint;
@@ -50,16 +53,20 @@ import org.slf4j.LoggerFactory;
* <p>The supported {@link net.shibboleth.utilities.java.support.resolver.Criterion} types and their use follows:</p>
*
* <dl>
- * <dt>{@link EndpointCriterion} (required)
+ * <dt>{@link EndpointCriterion} (required)</dt>
* <dd>Contains a "template" for the eventual {@link Endpoint}(s) to resolve that identifies at minimum the
* type of endpoint object (via schema type or element name) to resolve. It MAY contain other attributes that
* will be used in matching candidate endpoints for suitability, such as index, binding, location, etc. If so
- * marked, it may also be resolved as a trusted endpoint without additional verification required.
+ * marked, it may also be resolved as a trusted endpoint without additional verification required.</dd>
*
- * <dt>{@link RoleDescriptorCriterion}
+ * <dt>{@link BindingCriterion}</dt>
+ * <dd>Ordered list of bindings to filter and sort the endpoints. This overrides the ordering from the
+ * metadata and possibly overrides the normal default endpoint in favor of higher-precedence bindings.</dd>
+ *
+ * <dt>{@link RoleDescriptorCriterion}</dt>
* <dd>If present, provides access to the candidate endpoint(s) to attempt resolution against. Strictly optional,
* but if absent, the supplied endpoint (from {@link EndpointCriterion}) is returned as the sole result,
- * whatever its completeness/usability, allowing for subclass validation.
+ * whatever its completeness/usability, allowing for subclass validation.</dd>
* </dl>
*
* <p>Subclasses should override the {{@link #doCheckEndpoint(CriteriaSet, Endpoint)} method to implement
@@ -73,11 +80,43 @@ public abstract class AbstractEndpointResolver<EndpointType extends Endpoint>
/** Class logger. */
@Nonnull private Logger log = LoggerFactory.getLogger(AbstractEndpointResolver.class);
+ /** Sorting rule for results. */
+ private boolean inMetadataOrder;
+
/** Constructor. */
public AbstractEndpointResolver() {
super.setId(getClass().getName());
+ inMetadataOrder = true;
}
+ /**
+ * Get whether the results should be sorted by metadata order or based on the order of
+ * bindings provided to the lookup.
+ *
+ * @return true iff the {@link BindingCriterion} should be ignored for the purposes of sorting the results
+ *
+ * @since 4.1.0
+ */
+ public boolean isInMetadataOrder() {
+ return inMetadataOrder;
+ }
+
+ /**
+ * Set whether the results should be sorted by metadata order or based on the order of
+ * bindings provided to the lookup.
+ *
+ * <p>Defaults to true</p>
+ *
+ * @param flag flag to set
+ *
+ * @since 4.1.0
+ */
+ public void setInMetadataOrder(final boolean flag) {
+ ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+
+ inMetadataOrder = flag;
+ }
+
/** {@inheritDoc} */
@Override
@Nonnull @NonnullElements public Iterable<EndpointType> resolve(@Nullable final CriteriaSet criteria)
@@ -87,7 +126,7 @@ public abstract class AbstractEndpointResolver<EndpointType extends Endpoint>
if (canUseRequestedEndpoint(criteria)) {
final EndpointType endpoint = (EndpointType) criteria.get(EndpointCriterion.class).getEndpoint();
if (doCheckEndpoint(criteria, endpoint)) {
- return Collections.<EndpointType>singletonList(endpoint);
+ return Collections.singletonList(endpoint);
}
log.debug("{} Requested endpoint was rejected by extended validation process", getLogPrefix());
return Collections.emptyList();
@@ -210,16 +249,37 @@ public abstract class AbstractEndpointResolver<EndpointType extends Endpoint>
endpointType = epCriterion.getEndpoint().getElementQName();
}
- // Return the endpoints in the metadata of the candidate type.
final List<Endpoint> endpoints = role.getRole().getEndpoints(endpointType);
+
+ // Check for none.
if (endpoints.isEmpty()) {
log.debug("{} No endpoints in metadata of type {}", getLogPrefix(), endpointType);
- } else {
+ return new ArrayList<>();
+ }
+
+ final BindingCriterion bindingCriterion = criteria.get(BindingCriterion.class);
+ if (inMetadataOrder || bindingCriterion == null || bindingCriterion.getBindings().isEmpty()) {
+ // No second-level sort. Return the endpoints in the metadata of the candidate type,
+ // default endpoint first.
log.debug("{} Returning {} candidate endpoints of type {}", getLogPrefix(), endpoints.size(),
endpointType);
+ return sortCandidates(endpoints);
}
- return sortCandidates(endpoints);
+ // The binding(s) enforce a top-level sort on the endpoints, which is achieved
+ // by iterating over the full set multiple times with a binding filter applied.
+
+ final List<EndpointType> sortedResults = new ArrayList<>(endpoints.size());
+ for (final String binding : bindingCriterion.getBindings()) {
+ sortedResults.addAll(
+ sortCandidates(
+ endpoints.stream()
+ .filter(ep -> binding.equals(ep.getBinding()))
+ .collect(Collectors.toUnmodifiableList())));
+ }
+ log.debug("{} Returning {} candidate endpoints of type {}", getLogPrefix(), sortedResults.size(),
+ endpointType);
+ return sortedResults;
}
/**
diff --git a/opensaml-saml-api/src/main/java/org/opensaml/saml/criterion/BindingCriterion.java b/opensaml-saml-api/src/main/java/org/opensaml/saml/criterion/BindingCriterion.java
index 973375aba..228b0dff4 100644
--- a/opensaml-saml-api/src/main/java/org/opensaml/saml/criterion/BindingCriterion.java
+++ b/opensaml-saml-api/src/main/java/org/opensaml/saml/criterion/BindingCriterion.java
@@ -43,9 +43,9 @@ public final class BindingCriterion implements Criterion {
}
/**
- * Get the SAML binding URI.
+ * Get ordered list of SAML binding URIs.
*
- * @return the SAML binding URI
+ * @return ordered list of SAML binding URIs
*/
@Nonnull @NonnullElements @Unmodifiable @NotLive public List<String> getBindings() {
return bindings;
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/common/binding/impl/DefaultEndpointResolver.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/common/binding/impl/DefaultEndpointResolver.java
index 1e5da3d07..8f483c0aa 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/common/binding/impl/DefaultEndpointResolver.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/common/binding/impl/DefaultEndpointResolver.java
@@ -55,10 +55,13 @@ public class DefaultEndpointResolver<EndpointType extends Endpoint> extends Abst
@Override
protected boolean doCheckEndpoint(@Nonnull final CriteriaSet criteria, @Nonnull final EndpointType endpoint) {
- // Make sure the candidate binding, if set, is one of the bindings specified.
- final BindingCriterion bindingCriterion = criteria.get(BindingCriterion.class);
- if (bindingCriterion != null && !checkBindingCriterion(bindingCriterion, endpoint)) {
- return false;
+ if (isInMetadataOrder()) {
+ // Make sure the candidate binding, if set, is one of the bindings specified.
+ // When the sort isn't based on metadata, this has already happened.
+ final BindingCriterion bindingCriterion = criteria.get(BindingCriterion.class);
+ if (bindingCriterion != null && !checkBindingCriterion(bindingCriterion, endpoint)) {
+ return false;
+ }
}
// Compare individual fields to a comparison template.
diff --git a/opensaml-saml-impl/src/test/java/org/opensaml/saml/common/binding/impl/DefaultEndpointResolverTest.java b/opensaml-saml-impl/src/test/java/org/opensaml/saml/common/binding/impl/DefaultEndpointResolverTest.java
index 3871beba8..57baf42b3 100644
--- a/opensaml-saml-impl/src/test/java/org/opensaml/saml/common/binding/impl/DefaultEndpointResolverTest.java
+++ b/opensaml-saml-impl/src/test/java/org/opensaml/saml/common/binding/impl/DefaultEndpointResolverTest.java
@@ -25,6 +25,7 @@ import java.net.URL;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import java.util.stream.Collectors;
import javax.annotation.Nonnull;
@@ -37,12 +38,12 @@ import net.shibboleth.utilities.java.support.xml.XMLParserException;
import org.opensaml.core.xml.XMLObjectBaseTestCase;
import org.opensaml.core.xml.io.Unmarshaller;
import org.opensaml.core.xml.io.UnmarshallingException;
-import org.opensaml.saml.common.binding.impl.DefaultEndpointResolver;
import org.opensaml.saml.common.xml.SAMLConstants;
import org.opensaml.saml.criterion.BindingCriterion;
import org.opensaml.saml.criterion.EndpointCriterion;
import org.opensaml.saml.criterion.RoleDescriptorCriterion;
import org.opensaml.saml.saml2.metadata.AssertionConsumerService;
+import org.opensaml.saml.saml2.metadata.Endpoint;
import org.opensaml.saml.saml2.metadata.SPSSODescriptor;
import org.testng.Assert;
import org.testng.annotations.BeforeClass;
@@ -104,7 +105,7 @@ public class DefaultEndpointResolverTest extends XMLObjectBaseTestCase {
@Test
public void testSignedRequestBadBinding() throws ResolverException {
final CriteriaSet crits = new CriteriaSet(new EndpointCriterion<>(endpointCrit.getEndpoint(), true),
- new BindingCriterion(Collections.<String>emptyList()));
+ new BindingCriterion(Collections.emptyList()));
final AssertionConsumerService ep = resolver.resolveSingle(crits);
Assert.assertNull(ep);
}
@@ -268,8 +269,73 @@ public class DefaultEndpointResolverTest extends XMLObjectBaseTestCase {
eps.add(ep);
}
Assert.assertEquals(eps.size(), 4);
+ Assert.assertEquals(
+ eps.stream().map(Endpoint::getBinding).collect(Collectors.toUnmodifiableList()),
+ List.of(SAMLConstants.SAML2_ARTIFACT_BINDING_URI,
+ SAMLConstants.SAML2_POST_BINDING_URI,
+ SAMLConstants.SAML2_POST_BINDING_URI,
+ SAMLConstants.SAML2_ARTIFACT_BINDING_URI));
}
+ /**
+ * All endpoints of the right type in a supplied binding order.
+ *
+ * @throws UnmarshallingException ...
+ * @throws ResolverException ...
+ */
+ @Test
+ public void testMultipleBindingMetadataOrdered() throws UnmarshallingException, ResolverException {
+ endpointCrit.getEndpoint().setLocation(null);
+ endpointCrit.getEndpoint().setBinding(null);
+ final RoleDescriptorCriterion roleCrit =
+ new RoleDescriptorCriterion(loadMetadata("/org/opensaml/saml/common/binding/SPWithEndpoints.xml"));
+ final CriteriaSet crits = new CriteriaSet(endpointCrit, roleCrit,
+ new BindingCriterion(List.of(SAMLConstants.SAML2_POST_BINDING_URI, SAMLConstants.SAML2_ARTIFACT_BINDING_URI)));
+ final List<AssertionConsumerService> eps = new ArrayList<>();
+ for (final AssertionConsumerService ep : resolver.resolve(crits)) {
+ eps.add(ep);
+ }
+ Assert.assertEquals(eps.size(), 4);
+ Assert.assertEquals(
+ eps.stream().map(Endpoint::getBinding).collect(Collectors.toUnmodifiableList()),
+ List.of(SAMLConstants.SAML2_ARTIFACT_BINDING_URI,
+ SAMLConstants.SAML2_POST_BINDING_URI,
+ SAMLConstants.SAML2_POST_BINDING_URI,
+ SAMLConstants.SAML2_ARTIFACT_BINDING_URI));
+ }
+
+ /**
+ * All endpoints of the right type in a supplied binding order.
+ *
+ * @throws UnmarshallingException ...
+ * @throws ResolverException ...
+ * @throws ComponentInitializationException
+ */
+ @Test
+ public void testMultipleBindingImposeOrdered() throws UnmarshallingException, ResolverException, ComponentInitializationException {
+ final DefaultEndpointResolver<AssertionConsumerService> overridden = new DefaultEndpointResolver<>();
+ overridden.setInMetadataOrder(false);
+ overridden.initialize();
+
+ endpointCrit.getEndpoint().setLocation(null);
+ endpointCrit.getEndpoint().setBinding(null);
+ final RoleDescriptorCriterion roleCrit =
+ new RoleDescriptorCriterion(loadMetadata("/org/opensaml/saml/common/binding/SPWithEndpoints.xml"));
+ final CriteriaSet crits = new CriteriaSet(endpointCrit, roleCrit,
+ new BindingCriterion(List.of(SAMLConstants.SAML2_POST_BINDING_URI, SAMLConstants.SAML2_ARTIFACT_BINDING_URI)));
+ final List<AssertionConsumerService> eps = new ArrayList<>();
+ for (final AssertionConsumerService ep : overridden.resolve(crits)) {
+ eps.add(ep);
+ }
+ Assert.assertEquals(eps.size(), 4);
+ Assert.assertEquals(
+ eps.stream().map(Endpoint::getBinding).collect(Collectors.toUnmodifiableList()),
+ List.of(SAMLConstants.SAML2_POST_BINDING_URI,
+ SAMLConstants.SAML2_POST_BINDING_URI,
+ SAMLConstants.SAML2_ARTIFACT_BINDING_URI,
+ SAMLConstants.SAML2_ARTIFACT_BINDING_URI));
+ }
+
/**
* All endpoints of the right type and binding.
* @throws UnmarshallingException ...
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list