[java-opensaml] branch master updated: OSJ-292 - Mishandling of ProxyRestriction content
Scott Cantor
cantor.2 at osu.edu
Wed Nov 20 19:46:36 EST 2019
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=ac2643c07dcbbc4b2690fe12714582b30664ad3b
The following commit(s) were added to refs/heads/master by this push:
new ac2643c OSJ-292 - Mishandling of ProxyRestriction content
ac2643c is described below
commit ac2643c07dcbbc4b2690fe12714582b30664ad3b
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed Nov 20 19:46:27 2019 -0500
OSJ-292 - Mishandling of ProxyRestriction content
https://issues.shibboleth.net/jira/browse/OSJ-292
---
.../opensaml/saml/saml2/core/ProxyRestriction.java | 22 +++---
.../saml/saml2/core/impl/ProxyRestrictionImpl.java | 24 +++---
.../impl/AddProxyRestrictionToAssertions.java | 44 +++++++----
.../impl/AddProxyRestrictionToAssertionsTest.java | 91 +++++++++++++++++++---
4 files changed, 136 insertions(+), 45 deletions(-)
diff --git a/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/core/ProxyRestriction.java b/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/core/ProxyRestriction.java
index e176012..6534a00 100644
--- a/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/core/ProxyRestriction.java
+++ b/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/core/ProxyRestriction.java
@@ -19,50 +19,54 @@ package org.opensaml.saml.saml2.core;
import java.util.List;
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
import javax.xml.namespace.QName;
import org.opensaml.saml.common.xml.SAMLConstants;
+import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
+
/**
* SAML 2.0 Core ProxyRestriction.
*/
public interface ProxyRestriction extends Condition {
/** Element local name. */
- public static final String DEFAULT_ELEMENT_LOCAL_NAME = "ProxyRestriction";
+ @Nonnull @NotEmpty public static final String DEFAULT_ELEMENT_LOCAL_NAME = "ProxyRestriction";
/** Default element name. */
- public static final QName DEFAULT_ELEMENT_NAME = new QName(SAMLConstants.SAML20_NS, DEFAULT_ELEMENT_LOCAL_NAME,
- SAMLConstants.SAML20_PREFIX);
+ @Nonnull public static final QName DEFAULT_ELEMENT_NAME =
+ new QName(SAMLConstants.SAML20_NS, DEFAULT_ELEMENT_LOCAL_NAME, SAMLConstants.SAML20_PREFIX);
/** Local name of the XSI type. */
- public static final String TYPE_LOCAL_NAME = "ProxyRestrictionType";
+ @Nonnull @NotEmpty public static final String TYPE_LOCAL_NAME = "ProxyRestrictionType";
/** QName of the XSI type. */
- public static final QName TYPE_NAME = new QName(SAMLConstants.SAML20_NS, TYPE_LOCAL_NAME,
+ @Nonnull public static final QName TYPE_NAME = new QName(SAMLConstants.SAML20_NS, TYPE_LOCAL_NAME,
SAMLConstants.SAML20_PREFIX);
/** Count attribute name. */
- public static final String COUNT_ATTRIB_NAME = "Count";
+ @Nonnull @NotEmpty public static final String COUNT_ATTRIB_NAME = "Count";
/**
* Gets the number of times the assertion may be proxied.
*
* @return the number of times the assertion may be proxied
*/
- public Integer getProxyCount();
+ @Nullable public Integer getProxyCount();
/**
* Sets the number of times the assertion may be proxied.
*
* @param newProxyCount the number of times the assertion may be proxied
*/
- public void setProxyCount(Integer newProxyCount);
+ public void setProxyCount(@Nullable Integer newProxyCount);
/**
* Gets the list of audiences to whom the assertion may be proxied.
*
* @return the list of audiences to whom the assertion may be proxied
*/
- public List<Audience> getAudiences();
+ @Nullable public List<Audience> getAudiences();
}
\ No newline at end of file
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/core/impl/ProxyRestrictionImpl.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/core/impl/ProxyRestrictionImpl.java
index ab8bdb7..b8b1d29 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/core/impl/ProxyRestrictionImpl.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/core/impl/ProxyRestrictionImpl.java
@@ -25,22 +25,27 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
import org.opensaml.core.xml.AbstractXMLObject;
import org.opensaml.core.xml.XMLObject;
import org.opensaml.core.xml.util.XMLObjectChildrenList;
import org.opensaml.saml.saml2.core.Audience;
import org.opensaml.saml.saml2.core.ProxyRestriction;
+import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
+
/**
* Concrete implementation of {@link org.opensaml.saml.saml2.core.ProxyRestriction}.
*/
public class ProxyRestrictionImpl extends AbstractXMLObject implements ProxyRestriction {
/** Audiences of the Restriction. */
- private final XMLObjectChildrenList<Audience> audiences;
+ @Nonnull private final XMLObjectChildrenList<Audience> audiences;
/** Count of the Restriction. */
- private Integer proxyCount;
+ @Nullable private Integer proxyCount;
/**
* Constructor.
@@ -49,26 +54,26 @@ public class ProxyRestrictionImpl extends AbstractXMLObject implements ProxyRest
* @param elementLocalName the local name of the XML element this Object represents
* @param namespacePrefix the prefix for the given namespace
*/
- protected ProxyRestrictionImpl(final String namespaceURI, final String elementLocalName,
- final String namespacePrefix) {
+ protected ProxyRestrictionImpl(@Nullable @NotEmpty final String namespaceURI,
+ @Nonnull @NotEmpty final String elementLocalName, @Nullable @NotEmpty final String namespacePrefix) {
super(namespaceURI, elementLocalName, namespacePrefix);
audiences = new XMLObjectChildrenList<>(this);
}
/** {@inheritDoc} */
- public List<Audience> getAudiences() {
+ @Nullable public List<Audience> getAudiences() {
return audiences;
}
/** {@inheritDoc} */
- public Integer getProxyCount() {
+ @Nullable public Integer getProxyCount() {
return proxyCount;
}
/** {@inheritDoc} */
- public void setProxyCount(final Integer newProxyCount) {
- if (newProxyCount >= 0) {
- this.proxyCount = prepareForAssignment(this.proxyCount, newProxyCount);
+ public void setProxyCount(@Nullable final Integer newProxyCount) {
+ if (newProxyCount == null || newProxyCount >= 0) {
+ proxyCount = prepareForAssignment(proxyCount, newProxyCount);
} else {
throw new IllegalArgumentException("Count must be a non-negative integer.");
}
@@ -81,4 +86,5 @@ public class ProxyRestrictionImpl extends AbstractXMLObject implements ProxyRest
children.addAll(audiences);
return Collections.unmodifiableList(children);
}
+
}
\ No newline at end of file
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/profile/impl/AddProxyRestrictionToAssertions.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/profile/impl/AddProxyRestrictionToAssertions.java
index e60d5b8..16762ac 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/profile/impl/AddProxyRestrictionToAssertions.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/profile/impl/AddProxyRestrictionToAssertions.java
@@ -65,11 +65,14 @@ public class AddProxyRestrictionToAssertions extends AbstractConditionalProfileA
@Nullable private Function<ProfileRequestContext,Collection<String>> proxyAudiencesLookupStrategy;
/** Strategy used to obtain the proxy count to add. */
- @Nullable private Function<ProfileRequestContext,Long> proxyCountLookupStrategy;
+ @Nullable private Function<ProfileRequestContext,Integer> proxyCountLookupStrategy;
/** Response to modify. */
@Nullable private Response response;
+ /** ProxyCount to add. */
+ @Nullable private Integer proxyCount;
+
/** Audiences to add. */
@Nullable private Collection<String> audiences;
@@ -107,7 +110,7 @@ public class AddProxyRestrictionToAssertions extends AbstractConditionalProfileA
*
* @param strategy lookup strategy
*/
- public void setProxyCountLookupStrategy(@Nonnull final Function<ProfileRequestContext,Long> strategy) {
+ public void setProxyCountLookupStrategy(@Nonnull final Function<ProfileRequestContext,Integer> strategy) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
proxyCountLookupStrategy = Constraint.isNotNull(strategy, "Proxy count lookup strategy cannot be null");
@@ -128,9 +131,16 @@ public class AddProxyRestrictionToAssertions extends AbstractConditionalProfileA
/** {@inheritDoc} */
@Override
protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
+
+ if (!super.doPreExecute(profileRequestContext)) {
+ return false;
+ }
+
+ proxyCount = proxyCountLookupStrategy.apply(profileRequestContext);
audiences = proxyAudiencesLookupStrategy.apply(profileRequestContext);
- if (audiences == null || audiences.isEmpty()) {
- log.debug("{} No audiences to add, nothing to do", getLogPrefix());
+
+ if (proxyCount == null && (audiences == null || audiences.isEmpty())) {
+ log.debug("{} No restrictions to add, nothing to do", getLogPrefix());
return false;
}
@@ -147,7 +157,7 @@ public class AddProxyRestrictionToAssertions extends AbstractConditionalProfileA
return false;
}
- return super.doPreExecute(profileRequestContext);
+ return true;
}
/** {@inheritDoc} */
@@ -170,19 +180,19 @@ public class AddProxyRestrictionToAssertions extends AbstractConditionalProfileA
private void addProxyRestriction(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final Conditions conditions) {
final ProxyRestriction condition = getProxyRestriction(conditions);
-
- final SAMLObjectBuilder<Audience> audienceBuilder = (SAMLObjectBuilder<Audience>)
- XMLObjectProviderRegistrySupport.getBuilderFactory().<Audience>getBuilderOrThrow(
- Audience.DEFAULT_ELEMENT_NAME);
- for (final String audienceId : audiences) {
- log.debug("{} Adding {} as an Audience of the ProxyRestriction", getLogPrefix(), audienceId);
- final Audience audience = audienceBuilder.buildObject();
- audience.setURI(audienceId);
- condition.getAudiences().add(audience);
+ condition.setProxyCount(proxyCount);
+
+ if (audiences != null && !audiences.isEmpty()) {
+ final SAMLObjectBuilder<Audience> audienceBuilder = (SAMLObjectBuilder<Audience>)
+ XMLObjectProviderRegistrySupport.getBuilderFactory().<Audience>getBuilderOrThrow(
+ Audience.DEFAULT_ELEMENT_NAME);
+ for (final String audienceId : audiences) {
+ log.debug("{} Adding {} as an Audience of the ProxyRestriction", getLogPrefix(), audienceId);
+ final Audience audience = audienceBuilder.buildObject();
+ audience.setURI(audienceId);
+ condition.getAudiences().add(audience);
+ }
}
-
- final Long count = proxyCountLookupStrategy.apply(profileRequestContext);
- condition.setProxyCount(count != null ? count.intValue() : 0);
}
/**
diff --git a/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/profile/impl/AddProxyRestrictionToAssertionsTest.java b/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/profile/impl/AddProxyRestrictionToAssertionsTest.java
index d390367..59571c8 100644
--- a/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/profile/impl/AddProxyRestrictionToAssertionsTest.java
+++ b/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/profile/impl/AddProxyRestrictionToAssertionsTest.java
@@ -20,6 +20,8 @@ package org.opensaml.saml.saml2.profile.impl;
import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
import net.shibboleth.utilities.java.support.logic.FunctionSupport;
+import java.util.List;
+
import org.opensaml.core.OpenSAMLInitBaseTestCase;
import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport;
import org.opensaml.profile.RequestContextBuilder;
@@ -36,8 +38,6 @@ import org.testng.Assert;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
-import com.google.common.collect.ImmutableList;
-
/** {@link AddProxyRestrictionToAssertions} unit test. */
public class AddProxyRestrictionToAssertionsTest extends OpenSAMLInitBaseTestCase {
@@ -46,17 +46,17 @@ public class AddProxyRestrictionToAssertionsTest extends OpenSAMLInitBaseTestCas
private AddProxyRestrictionToAssertions action;
- @BeforeMethod public void setUp() throws ComponentInitializationException {
+ @BeforeMethod public void setUp() {
action = new AddProxyRestrictionToAssertions();
- action.setProxyAudiencesLookupStrategy(FunctionSupport.constant(ImmutableList.of(AUDIENCE1, AUDIENCE2)));
- action.setProxyCountLookupStrategy(FunctionSupport.constant(1L));
- action.initialize();
+ action.setProxyAudiencesLookupStrategy(FunctionSupport.constant(List.of(AUDIENCE1, AUDIENCE2)));
+ action.setProxyCountLookupStrategy(FunctionSupport.constant(1));
}
/** Test that action errors out properly if there is no response. */
@Test public void testNoResponse() throws Exception {
final ProfileRequestContext prc = new RequestContextBuilder().buildProfileRequestContext();
+ action.initialize();
action.execute(prc);
ActionTestingSupport.assertEvent(prc, EventIds.INVALID_MSG_CTX);
}
@@ -66,15 +66,78 @@ public class AddProxyRestrictionToAssertionsTest extends OpenSAMLInitBaseTestCas
final ProfileRequestContext prc = new RequestContextBuilder().setOutboundMessage(
SAML2ActionTestingSupport.buildResponse()).buildProfileRequestContext();
+ action.initialize();
action.execute(prc);
ActionTestingSupport.assertProceedEvent(prc);
}
/**
* Test that the condition is properly added if there is a single assertion, without a Conditions element, in the
+ * response with no audiences.
+ *
+ * @throws ComponentInitializationException
+ */
+ @Test public void testCountOnly() throws ComponentInitializationException {
+ final Assertion assertion = SAML2ActionTestingSupport.buildAssertion();
+
+ final Response response = SAML2ActionTestingSupport.buildResponse();
+ response.getAssertions().add(assertion);
+
+ final ProfileRequestContext prc = new RequestContextBuilder().setOutboundMessage(response).buildProfileRequestContext();
+
+ action.setProxyAudiencesLookupStrategy(FunctionSupport.constant(null));
+ action.initialize();
+ action.execute(prc);
+ ActionTestingSupport.assertProceedEvent(prc);
+
+ Assert.assertNotNull(response.getAssertions());
+ Assert.assertEquals(response.getAssertions().size(), 1);
+
+ Assert.assertNotNull(assertion.getConditions());
+ Assert.assertNotNull(assertion.getConditions().getProxyRestriction());
+ final ProxyRestriction proxy = assertion.getConditions().getProxyRestriction();
+ Assert.assertEquals(proxy.getProxyCount(), Integer.valueOf(1));
+ Assert.assertTrue(proxy.getAudiences().isEmpty());
+ }
+
+ /**
+ * Test that the condition is properly added if there is a single assertion, without a Conditions element, in the
+ * response with no count.
+ *
+ * @throws ComponentInitializationException
+ */
+ @Test public void testAudiencesOnly() throws ComponentInitializationException {
+ final Assertion assertion = SAML2ActionTestingSupport.buildAssertion();
+
+ final Response response = SAML2ActionTestingSupport.buildResponse();
+ response.getAssertions().add(assertion);
+
+ final ProfileRequestContext prc = new RequestContextBuilder().setOutboundMessage(response).buildProfileRequestContext();
+
+ action.setProxyCountLookupStrategy(FunctionSupport.constant(null));
+ action.initialize();
+ action.execute(prc);
+ ActionTestingSupport.assertProceedEvent(prc);
+
+ Assert.assertNotNull(response.getAssertions());
+ Assert.assertEquals(response.getAssertions().size(), 1);
+
+ Assert.assertNotNull(assertion.getConditions());
+ Assert.assertNotNull(assertion.getConditions().getProxyRestriction());
+ final ProxyRestriction proxy = assertion.getConditions().getProxyRestriction();
+ Assert.assertNull(proxy.getProxyCount());
+ Assert.assertEquals(proxy.getAudiences().size(), 2);
+ Assert.assertEquals(proxy.getAudiences().get(0).getURI(), AUDIENCE1);
+ Assert.assertEquals(proxy.getAudiences().get(1).getURI(), AUDIENCE2);
+ }
+
+ /**
+ * Test that the condition is properly added if there is a single assertion, without a Conditions element, in the
* response.
+ *
+ * @throws ComponentInitializationException
*/
- @Test public void testSingleAssertion() throws Exception {
+ @Test public void testSingleAssertion() throws ComponentInitializationException {
final Assertion assertion = SAML2ActionTestingSupport.buildAssertion();
final Response response = SAML2ActionTestingSupport.buildResponse();
@@ -82,6 +145,7 @@ public class AddProxyRestrictionToAssertionsTest extends OpenSAMLInitBaseTestCas
final ProfileRequestContext prc = new RequestContextBuilder().setOutboundMessage(response).buildProfileRequestContext();
+ action.initialize();
action.execute(prc);
ActionTestingSupport.assertProceedEvent(prc);
@@ -100,8 +164,10 @@ public class AddProxyRestrictionToAssertionsTest extends OpenSAMLInitBaseTestCas
/**
* Test that the condition is properly added if there is a single assertion, with a Conditions element, in the
* response.
+ *
+ * @throws ComponentInitializationException
*/
- @Test public void testSingleAssertionWithExistingCondition() throws Exception {
+ @Test public void testSingleAssertionWithExistingCondition() throws ComponentInitializationException {
final SAMLObjectBuilder<Conditions> conditionsBuilder = (SAMLObjectBuilder<Conditions>)
XMLObjectProviderRegistrySupport.getBuilderFactory().<Conditions>getBuilderOrThrow(
Conditions.DEFAULT_ELEMENT_NAME);
@@ -115,6 +181,7 @@ public class AddProxyRestrictionToAssertionsTest extends OpenSAMLInitBaseTestCas
final ProfileRequestContext prc = new RequestContextBuilder().setOutboundMessage(response).buildProfileRequestContext();
+ action.initialize();
action.execute(prc);
ActionTestingSupport.assertProceedEvent(prc);
@@ -127,8 +194,11 @@ public class AddProxyRestrictionToAssertionsTest extends OpenSAMLInitBaseTestCas
Assert.assertEquals(proxy.getAudiences().get(1).getURI(), AUDIENCE2);
}
- /** Test that the condition is properly added if there are multiple assertions in the response. */
- @Test public void testMultipleAssertion() throws Exception {
+ /** Test that the condition is properly added if there are multiple assertions in the response.
+ *
+ * @throws ComponentInitializationException
+ */
+ @Test public void testMultipleAssertion() throws ComponentInitializationException {
final Response response = SAML2ActionTestingSupport.buildResponse();
response.getAssertions().add(SAML2ActionTestingSupport.buildAssertion());
response.getAssertions().add(SAML2ActionTestingSupport.buildAssertion());
@@ -136,6 +206,7 @@ public class AddProxyRestrictionToAssertionsTest extends OpenSAMLInitBaseTestCas
final ProfileRequestContext prc = new RequestContextBuilder().setOutboundMessage(response).buildProfileRequestContext();
+ action.initialize();
action.execute(prc);
ActionTestingSupport.assertProceedEvent(prc);
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list