[java-plugin-shibd-saml] branch dev/StateMgmtWIP updated: Error handling cleanup, unit tests for ACR validation.

Codeberg noreply at shibboleth.net
Mon May 4 18:33:33 UTC 2026


This is an automated email from the git hooks/post-receive script.

codeberg pushed a commit to branch dev/StateMgmtWIP
in repository java-plugin-shibd-saml.

View the commit online:
https://codeberg.org/Shibboleth/java-plugin-shibd-saml/commit/3e788e8f6e469ced57784e656e22a25bebf95e33

The following commit(s) were added to refs/heads/dev/StateMgmtWIP by this push:
     new 3e788e8  Error handling cleanup, unit tests for ACR validation.
3e788e8 is described below

commit 3e788e8f6e469ced57784e656e22a25bebf95e33
Author: Scott Cantor <scott at restingparrotsoftware.com>
AuthorDate: Mon May 4 14:33:18 2026 -0400

    Error handling cleanup, unit tests for ACR validation.
---
 .../idp/flows/sp/consumer/saml2/saml2-beans.xml    |  3 +-
 .../flows/saml2/SAML2TokenConsumerFlowTest.java    | 64 +++++++++++++++++++++-
 .../impl/ProcessAssertionsForAuthentication.java   | 42 ++++++++++++--
 3 files changed, 100 insertions(+), 9 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 bff40be..404171d 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
@@ -229,7 +229,8 @@
 
     <bean id="ProcessAssertionsForAuthentication"
             class="net.shibboleth.sp.saml.saml2.profile.impl.ProcessAssertionsForAuthentication" scope="prototype"
-            p:sAMLConsumer-ref="SAMLStatementConsumer">
+            p:sAMLConsumer-ref="SAMLStatementConsumer"
+            p:principalEvalPredicateFactoryRegistry-ref="shibboleth.AuthnComparisonRegistry">
         <property name="responseResolver">
             <bean parent="shibboleth.Functions.Compose">
                 <constructor-arg name="g">
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 5574839..b493276 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
@@ -565,6 +565,38 @@ public class SAML2TokenConsumerFlowTest extends AbstractSPFlowTest {
                         response.getAssertions().get(0).getID()));
     }
 
+    /**
+     * Test absent SubjectConfirmation.
+     * 
+     * @throws IOException 
+     */
+    @Test
+    public void testMissingConfirmation() throws IOException {
+        final Response response = buildSAMLResponse(ISSUER, StatusCode.SUCCESS, null);
+        Constraint.isNotNull(response.getAssertions().get(0).getSubject(), "No Subject").getSubjectConfirmations().clear();
+        sign(response);
+        final DDF input = buildRemotedPOSTResponse(response, null, null);
+        setApplicationRequest(APPLICATION_ID, input);
+
+        validateError(EventIds.INVALID_MESSAGE, "No valid assertions suitable for authentication were found");
+    }
+
+    /**
+     * Test no statements.
+     * 
+     * @throws IOException 
+     */
+    @Test
+    public void testMissingStatement() throws IOException {
+        final Response response = buildSAMLResponse(ISSUER, StatusCode.SUCCESS, null);
+        response.getAssertions().get(0).getAuthnStatements().clear();
+        sign(response);
+        final DDF input = buildRemotedPOSTResponse(response, null, null);
+        setApplicationRequest(APPLICATION_ID, input);
+
+        validateError(EventIds.INVALID_MESSAGE, "No valid assertions suitable for authentication were found");
+    }
+    
     /**
      * Test failure due to state recovery address.
      * 
@@ -612,7 +644,7 @@ public class SAML2TokenConsumerFlowTest extends AbstractSPFlowTest {
     }
     
     /**
-     * Test failure due to state recovery authority.
+     * Test failure due to state response URL.
      * 
      * @throws IOException
      * @throws ResolverException 
@@ -633,6 +665,30 @@ public class SAML2TokenConsumerFlowTest extends AbstractSPFlowTest {
         validateError(EventIds.INVALID_MESSAGE, null);
     }
     
+
+    /**
+     * Test failure due to state recovery address.
+     * 
+     * @throws IOException
+     * @throws ResolverException 
+     */
+    @Test
+    public void testFailedStateAuthnContext() throws IOException, ResolverException {
+        
+        final StateData data = buildStateData("foo");
+        data.setAcrs(CollectionSupport.singletonList(AuthnContext.X509_AUTHN_CTX));
+        
+        final String stateToken = getStateToken(data);
+        
+        final Response response = buildSAMLResponse(ISSUER, StatusCode.SUCCESS, "foo");
+        sign(response);
+        final DDF input = buildRemotedPOSTResponse(response, stateToken, response2.getCookies()[0]);
+
+        setApplicationRequest(APPLICATION_ID, input);
+        
+        validateError(EventIds.INVALID_MESSAGE, "No suitable AuthnContextClassRef found");
+    }
+    
     /**
      * Test successful flow.
      * 
@@ -664,7 +720,9 @@ public class SAML2TokenConsumerFlowTest extends AbstractSPFlowTest {
     @Test
     public void testSuccessWithState() throws IOException, ResolverException {
         
-        final String stateToken = getStateToken(buildStateData("foo"));
+        final StateData data = buildStateData("foo");
+        data.setAcrs(CollectionSupport.singletonList(AuthnContext.PPT_AUTHN_CTX));
+        final String stateToken = getStateToken(data);
         
         final Response response = buildSAMLResponse(ISSUER, StatusCode.SUCCESS, "foo");
         sign(response);
@@ -951,7 +1009,7 @@ public class SAML2TokenConsumerFlowTest extends AbstractSPFlowTest {
      * 
      * @return state object
      */
-    @Nonnull private StateData buildStateData(@Nonnull final String requestID) {
+    @Nonnull private SAMLStateData buildStateData(@Nonnull final String requestID) {
         final SAMLStateData data = new SAMLStateData();
         data.setRequestID(requestID);
         data.setAuthenticationAuthority(ISSUER);
diff --git a/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/ProcessAssertionsForAuthentication.java b/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/ProcessAssertionsForAuthentication.java
index ccda3f9..987333d 100644
--- a/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/ProcessAssertionsForAuthentication.java
+++ b/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/ProcessAssertionsForAuthentication.java
@@ -46,7 +46,6 @@ import org.slf4j.Logger;
 import net.shibboleth.idp.authn.AuthnEventIds;
 import net.shibboleth.idp.authn.context.RequestedPrincipalContext;
 import net.shibboleth.idp.authn.principal.PrincipalEvalPredicateFactoryRegistry;
-import net.shibboleth.idp.profile.AbstractProfileAction;
 import net.shibboleth.idp.saml.authn.principal.AuthnContextClassRefPrincipal;
 import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.shared.annotation.constraint.NonnullBeforeExec;
@@ -54,6 +53,9 @@ import net.shibboleth.shared.component.ComponentInitializationException;
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.LoggerFactory;
 import net.shibboleth.sp.context.StateDataContext;
+import net.shibboleth.sp.ddf.DDF;
+import net.shibboleth.sp.profile.AbstractApplicationAction;
+import net.shibboleth.sp.profile.ConsumerConstants;
 import net.shibboleth.sp.saml.saml2.SAMLStateData;
 
 /**
@@ -68,10 +70,9 @@ import net.shibboleth.sp.saml.saml2.SAMLStateData;
  * 
  * @event {@link EventIds#PROCEED_EVENT_ID}
  * @event {@link EventIds#INVALID_MESSAGE}
- * @event {@link AuthnEventIds#INVALID_CREDENTIALS}
  * @post the selected statement is passed into the supplied {@link BiConsumer}
  */
-public class ProcessAssertionsForAuthentication extends AbstractProfileAction {
+public class ProcessAssertionsForAuthentication extends AbstractApplicationAction {
     
     /** Logger. */
     @Nonnull private final Logger log = LoggerFactory.getLogger(ProcessAssertionsForAuthentication.class);
@@ -202,7 +203,8 @@ public class ProcessAssertionsForAuthentication extends AbstractProfileAction {
                 .collect(Collectors.toList());
         if (assertions.isEmpty()) {
             log.info("{} No valid SAML Assertions suitable for authentication were found", getLogPrefix());
-            ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_CREDENTIALS);
+            addValidationError("No valid assertions suitable for authentication were found");
+            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MESSAGE);
             return;
         }
         
@@ -253,7 +255,8 @@ public class ProcessAssertionsForAuthentication extends AbstractProfileAction {
         
         if (authnAssertion == null) {
             log.info("{} Could not select a single valid SAML Assertion for authentication", getLogPrefix());
-            ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_CREDENTIALS);
+            addValidationError("No suitable AuthnContextClassRef found");
+            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MESSAGE);
             return;
         }
 
@@ -262,6 +265,15 @@ public class ProcessAssertionsForAuthentication extends AbstractProfileAction {
         samlConsumer.accept(profileRequestContext, authnStatement);
     }
     
+    /**
+     * Process an {@link AuthnContext} for compatibility with the request as brokered by the populated
+     * {@link RequestedPrincipalContext}.
+     * 
+     * @param helperContext populated context to drive context evaluation
+     * @param authnContext input context object from assertion statement
+     * 
+     * @return true iff the context carries an ACR that is compatible with the request
+     */
     private boolean isAcceptable(@Nonnull final RequestedPrincipalContext helperContext,
             @Nullable final AuthnContext authnContext) {
         
@@ -281,6 +293,26 @@ public class ProcessAssertionsForAuthentication extends AbstractProfileAction {
         }
     }
     
+    /**
+     * Adds a validation error for the agent regarding the failure.
+     * 
+     * @param msg error message
+     */
+    private void addValidationError(@Nonnull final String msg) {
+        DDF output = ensureAgentRequestContext().getOutput();
+        if (output == null) {
+            output = new DDF(null).structure();
+            ensureAgentRequestContext().setOutput(output);
+        }
+        
+        final DDF errors = output.addmember(ConsumerConstants.VALIDATION_ERRORS);
+        if (!errors.islist()) {
+            errors.list();
+        }
+        
+        errors.add(new DDF(null).string(msg));
+    }
+    
     /**
      * Predicate for valid assertions.
      */

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list