[java-plugin-shibd-saml] branch main updated: Get issuer validation working, allow tests to validate assertion errors.
Scott Cantor
cantor.2 at osu.edu
Fri Oct 4 19:44:34 UTC 2024
This is an automated email from the git hooks/post-receive script.
scantor pushed a commit to branch main
in repository java-plugin-shibd-saml.
View the commit online:
http://git.shibboleth.net/view/?p=java-plugin-shibd-saml.git;a=commit;h=690cda2dbb9f38d60b861782282402c36239427f
The following commit(s) were added to refs/heads/main by this push:
new 690cda2 Get issuer validation working, allow tests to validate assertion errors.
690cda2 is described below
commit 690cda2dbb9f38d60b861782282402c36239427f
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Fri Oct 4 15:44:31 2024 -0400
Get issuer validation working, allow tests to validate assertion errors.
---
.../idp/flows/sp/consumer/saml2/saml2-beans.xml | 17 +++-
.../idp/flows/sp/consumer/saml2/saml2-flow.xml | 8 +-
.../flows/saml2/SAML2TokenConsumerFlowTest.java | 100 ++++++++++++++++++---
.../saml2/profile/impl/ValidateAssertions.java | 28 ++++++
4 files changed, 138 insertions(+), 15 deletions(-)
diff --git a/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/saml2/saml2-beans.xml b/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/saml2/saml2-beans.xml
index c6dbee6..4c8290b 100644
--- a/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/saml2/saml2-beans.xml
+++ b/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/saml2/saml2-beans.xml
@@ -111,6 +111,15 @@
<bean id="AssertionValidationContextBuilder" class="org.opensaml.saml.saml2.profile.impl.DefaultAssertionValidationContextBuilder">
<property name="clockSkew" value="%{sp.policy.clockSkew:PT3M}" />
<property name="lifetime" value="%{sp.policy.assertionLifetime:PT3M}" />
+ <property name="validIssuers">
+ <!-- This is wrapping the valid issuer into a set but via a Function. -->
+ <bean parent="shibboleth.ContextFunctions.Expression"
+ c:expression="T(net.shibboleth.shared.collection.CollectionSupport).singleton(#custom.apply(#input))"
+ p:customObject-ref="InboundEntityIDLookup" />
+ </property>
+ <property name="requireEntityIssuer">
+ <ref bean="shibboleth.Conditions.TRUE" />
+ </property>
<property name="checkAddress">
<bean class="net.shibboleth.saml.saml2.profile.config.logic.CheckAddressPredicate" />
</property>
@@ -145,10 +154,6 @@
p:validationContextBuilder-ref="AssertionValidationContextBuilder"
p:assertionValidator-ref="AssertionValidator" />
- <bean id="DecryptNameIDs" class="org.opensaml.saml.saml2.profile.impl.DecryptNameIDs" scope="prototype" />
-
- <bean id="DecryptAttributes" class="org.opensaml.saml.saml2.profile.impl.DecryptAttributes" scope="prototype" />
-
<bean id="SAMLStatementConsumer" class="net.shibboleth.sp.saml.saml2.profile.impl.SAMLTokenContextConsumer" />
<bean id="ProcessAssertionsForAuthentication"
@@ -167,6 +172,10 @@
</property>
</bean>
+ <bean id="DecryptNameIDs" class="org.opensaml.saml.saml2.profile.impl.DecryptNameIDs" scope="prototype" />
+
+ <bean id="DecryptAttributes" class="org.opensaml.saml.saml2.profile.impl.DecryptAttributes" scope="prototype" />
+
<bean id="StandardExtractionStrategy"
class="net.shibboleth.sp.saml.saml2.profile.impl.ExtractSAMLAttributes.StandardExtractionStrategy"
p:issuerAttributeId="%{sp.saml.issuerAttributeId:}"
diff --git a/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/saml2/saml2-flow.xml b/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/saml2/saml2-flow.xml
index 7eba489..ff88097 100644
--- a/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/saml2/saml2-flow.xml
+++ b/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/saml2/saml2-flow.xml
@@ -24,9 +24,9 @@
<evaluate expression="PopulateDecryptionParameters" />
<evaluate expression="DecryptAssertions" />
<evaluate expression="ValidateAssertions" />
+ <evaluate expression="ProcessAssertionsForAuthentication" />
<evaluate expression="DecryptNameIDs" />
<evaluate expression="DecryptAttributes" />
- <evaluate expression="ProcessAssertionsForAuthentication" />
<evaluate expression="ExtractSAMLAttributes" />
<evaluate expression="PrepareAgentResponse" />
<!-- <evaluate expression="PostAssertionPopulateAuditContext" />-->
@@ -37,6 +37,12 @@
<transition on="proceed" to="proceed" />
</action-state>
+ <global-transitions>
+ <!-- Remap some internal action errors. -->
+ <transition on="InvalidCredentials" to="InvalidMessage" />
+ <transition on="UnableToValidateAssertion" to="MessageProcessingError" />
+ </global-transitions>
+
<!-- The file really exists in this directory, but it's referenced from extending flow-directories -->
<bean-import resource="classpath:/META-INF/net/shibboleth/idp/flows/sp/consumer/saml2/saml2-beans.xml" />
diff --git a/sp-saml-conf-impl/src/test/java/net/shibboleth/sp/saml/flows/saml2/SAML2TokenConsumerFlowTest.java b/sp-saml-conf-impl/src/test/java/net/shibboleth/sp/saml/flows/saml2/SAML2TokenConsumerFlowTest.java
index fa785e1..aa6d228 100644
--- a/sp-saml-conf-impl/src/test/java/net/shibboleth/sp/saml/flows/saml2/SAML2TokenConsumerFlowTest.java
+++ b/sp-saml-conf-impl/src/test/java/net/shibboleth/sp/saml/flows/saml2/SAML2TokenConsumerFlowTest.java
@@ -28,6 +28,7 @@ import org.opensaml.profile.action.EventIds;
import org.opensaml.profile.context.ProfileRequestContext;
import org.opensaml.saml.saml2.core.Assertion;
import org.opensaml.saml.saml2.core.AuthnContext;
+import org.opensaml.saml.saml2.core.Issuer;
import org.opensaml.saml.saml2.core.NameID;
import org.opensaml.saml.saml2.core.NameIDType;
import org.opensaml.saml.saml2.core.Response;
@@ -44,6 +45,7 @@ import org.springframework.test.context.web.WebAppConfiguration;
import org.springframework.webflow.executor.FlowExecutionResult;
import org.testng.Assert;
import org.testng.annotations.BeforeClass;
+import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
import net.shibboleth.idp.authn.AuthnEventIds;
@@ -61,6 +63,7 @@ import net.shibboleth.sp.flows.AbstractSPFlowTest;
import net.shibboleth.sp.impl.StorageServiceStateTokenManager;
import net.shibboleth.sp.messaging.RemotedHttpServletRequest;
import net.shibboleth.sp.messaging.RemotedHttpServletResponse;
+import net.shibboleth.sp.profile.ConsumerConstants;
/**
* Unit test for the SP session-initiator flow.
@@ -115,6 +118,12 @@ public class SAML2TokenConsumerFlowTest extends AbstractSPFlowTest {
stateManager.setStorageService(storageService);
stateManager.initialize();
}
+
+ /** Pre-test work. */
+ @BeforeMethod
+ public void beforeMethod() {
+ setDefaultAuth();
+ }
/**
* Test flow without proper input.
@@ -123,8 +132,6 @@ public class SAML2TokenConsumerFlowTest extends AbstractSPFlowTest {
*/
@Test
public void testNoInput() throws IOException {
- setDefaultAuth();
-
setApplicationRequest(APPLICATION_ID, null);
final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
@@ -140,8 +147,6 @@ public class SAML2TokenConsumerFlowTest extends AbstractSPFlowTest {
*/
@Test
public void testErrorStatus() throws IOException {
- setDefaultAuth();
-
final Response response = buildSAMLResponse("https://idp.example.org", StatusCode.RESPONDER);
final DDF input = buildRemotedPOSTResponse(response);
setApplicationRequest(APPLICATION_ID, input);
@@ -159,8 +164,6 @@ public class SAML2TokenConsumerFlowTest extends AbstractSPFlowTest {
*/
@Test
public void testUnverified() throws IOException {
- setDefaultAuth();
-
final DDF input = buildRemotedPOSTResponse(buildSAMLResponse("https://unknown.example.org", StatusCode.SUCCESS));
setApplicationRequest(APPLICATION_ID, input);
@@ -177,8 +180,6 @@ public class SAML2TokenConsumerFlowTest extends AbstractSPFlowTest {
*/
@Test
public void testExpired() throws IOException {
- setDefaultAuth();
-
final Response response = buildSAMLResponse("https://idp.example.org", StatusCode.SUCCESS);
response.setIssueInstant(Instant.EPOCH);
final DDF input = buildRemotedPOSTResponse(response);
@@ -197,8 +198,6 @@ public class SAML2TokenConsumerFlowTest extends AbstractSPFlowTest {
*/
@Test
public void testBadDestination() throws IOException {
- setDefaultAuth();
-
final Response response = buildSAMLResponse("https://idp.example.org", StatusCode.SUCCESS);
response.setDestination(RESPONSE_URL + "/bad");
final DDF input = buildRemotedPOSTResponse(response);
@@ -210,6 +209,67 @@ public class SAML2TokenConsumerFlowTest extends AbstractSPFlowTest {
assertOutputMessageEvent(result, EventIds.INVALID_MESSAGE);
}
+ /**
+ * Test flow with expired assertion issue instant.
+ *
+ * @throws IOException
+ */
+ @Test
+ public void testAssertionExpired() throws IOException {
+ final Response response = buildSAMLResponse("https://idp.example.org", StatusCode.SUCCESS);
+ final DDF input = buildRemotedPOSTResponse(response);
+ setApplicationRequest(APPLICATION_ID, input);
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertFlowExecutionResult(result, FLOW_ID);
+ assertFlowExecutionOutcome(result.getOutcome());
+ final DDF output = assertOutputMessageEvent(result, EventIds.INVALID_MESSAGE);
+ validateAssertionError(output, "Assertion IssueInstant was expired");
+ }
+
+ /**
+ * Test flow with bad issuer value.
+ *
+ * @throws IOException
+ */
+ @Test
+ public void testAssertionBadIssuer() throws IOException {
+ final Response response = buildSAMLResponse("https://idp.example.org", StatusCode.SUCCESS);
+ response.getAssertions().get(0).setIssueInstant(Instant.now().minusSeconds(60));
+ response.getAssertions().get(0).setIssuer(SAML2ActionTestingSupport.buildIssuer(ISSUER + "bad"));
+ final DDF input = buildRemotedPOSTResponse(response);
+ setApplicationRequest(APPLICATION_ID, input);
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertFlowExecutionResult(result, FLOW_ID);
+ assertFlowExecutionOutcome(result.getOutcome());
+ final DDF output = assertOutputMessageEvent(result, EventIds.INVALID_MESSAGE);
+ validateAssertionError(output,
+ String.format("Issuer of Assertion '%s' did not match any valid issuers", response.getAssertions().get(0).getID()));
+ }
+
+ /**
+ * Test flow with bad issuer format.
+ *
+ * @throws IOException
+ */
+ @Test
+ public void testAssertionBadIssuerFormat() throws IOException {
+ final Response response = buildSAMLResponse("https://idp.example.org", StatusCode.SUCCESS);
+ response.getAssertions().get(0).setIssueInstant(Instant.now().minusSeconds(60));
+ final Issuer issuer = SAML2ActionTestingSupport.buildIssuer(ISSUER);
+ issuer.setFormat(NameIDType.EMAIL);
+ response.getAssertions().get(0).setIssuer(issuer);
+ final DDF input = buildRemotedPOSTResponse(response);
+ setApplicationRequest(APPLICATION_ID, input);
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertFlowExecutionResult(result, FLOW_ID);
+ assertFlowExecutionOutcome(result.getOutcome());
+ final DDF output = assertOutputMessageEvent(result, EventIds.INVALID_MESSAGE);
+ validateAssertionError(output, "Issuer had invalid Format: urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress");
+ }
+
/**
* Decode an encoded response and run sanity checks against it.
*
@@ -235,6 +295,25 @@ public class SAML2TokenConsumerFlowTest extends AbstractSPFlowTest {
return output;
}
+ /**
+ * Tests the output contains token validation error starting with the designated string.
+ *
+ * @param output output message
+ * @param messagePrefix error message to check for
+ */
+ private void validateAssertionError(@Nullable final DDF output, @Nullable final String messagePrefix) {
+ if (output == null) {
+ Assert.fail("No output message");
+ return;
+ }
+
+ final DDF errors = output.getmember(ConsumerConstants.VALIDATION_ERRORS);
+ Assert.assertTrue(errors.islist());
+
+ final String msg = errors.asList().get(0).string();
+ Assert.assertTrue(msg != null && msg.startsWith(messagePrefix));
+ }
+
/**
* Builds a SAML response with some tailored data.
@@ -296,6 +375,7 @@ public class SAML2TokenConsumerFlowTest extends AbstractSPFlowTest {
final DDF http = obj.addmember(RemotedHttpServletRequest.STRUCTURE_NAME).structure();
http.addmember(RemotedHttpServletRequest.METHOD).string("POST");
+ http.addmember(RemotedHttpServletRequest.REMOTE_ADDR).string("192.168.1.1");
http.addmember(RemotedHttpServletRequest.CONTENT_TYPE).string("application/x-www-form-urlencoded");
http.addmember(RemotedHttpServletRequest.REQUEST_URL).unsafe_string(RESPONSE_URL.getBytes(StandardCharsets.UTF_8));
diff --git a/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/ValidateAssertions.java b/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/ValidateAssertions.java
index de60155..e0aef8a 100644
--- a/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/ValidateAssertions.java
+++ b/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/ValidateAssertions.java
@@ -17,9 +17,14 @@ package net.shibboleth.sp.saml.saml2.profile.impl;
import javax.annotation.Nonnull;
import org.opensaml.profile.context.ProfileRequestContext;
+import org.opensaml.saml.common.assertion.ValidationContext;
+import org.opensaml.saml.common.assertion.ValidationResult;
+import org.opensaml.saml.saml2.core.Assertion;
import net.shibboleth.sp.context.AgentRequestContext;
+import net.shibboleth.sp.ddf.DDF;
import net.shibboleth.sp.messaging.RemotedHttpServletRequestResponseContext;
+import net.shibboleth.sp.profile.ConsumerConstants;
/**
* Subclass of base class action that encapsulates the execution step with the thread-local
@@ -47,5 +52,28 @@ public class ValidateAssertions extends org.opensaml.saml.saml2.profile.impl.Val
RemotedHttpServletRequestResponseContext.clearCurrent();
}
}
+
+ /** {@inheritDoc} */
+ @Override
+ protected void processResult(@Nonnull final ValidationContext validationContext,
+ @Nonnull final ValidationResult validationResult, @Nonnull final Assertion assertion,
+ @Nonnull final ProfileRequestContext profileRequestContext) {
+ super.processResult(validationContext, validationResult, assertion, profileRequestContext);
+
+ final AgentRequestContext agentRequestContext =
+ profileRequestContext.ensureSubcontext(AgentRequestContext.class);
+ DDF output = agentRequestContext.getOutput();
+ if (output == null) {
+ output = new DDF(null).structure();
+ agentRequestContext.setOutput(output);
+ }
+
+ final DDF errors = output.addmember(ConsumerConstants.VALIDATION_ERRORS);
+ if (!errors.islist()) {
+ errors.list();
+ }
+
+ validationContext.getValidationFailureMessages().forEach(msg -> errors.add(new DDF(null).string(msg)));
+ }
}
\ No newline at end of file
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list