[java-plugin-shibd-oidc] branch main updated: Add error response checking

Codeberg noreply at shibboleth.net
Fri Dec 5 11:42:21 UTC 2025


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

codeberg pushed a commit to branch main
in repository java-plugin-shibd-oidc.

View the commit online:
https://codeberg.org/Shibboleth/java-plugin-shibd-oidc/commit/878aac3e884c4bb0e0308f7f7205c6f236a00ef2

The following commit(s) were added to refs/heads/main by this push:
     new 878aac3  Add error response checking
878aac3 is described below

commit 878aac3e884c4bb0e0308f7f7205c6f236a00ef2
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Fri Dec 5 11:42:10 2025 +0000

    Add error response checking
---
 .../META-INF/net.shibboleth.idp/postconfig.xml     |   4 +-
 .../idp/flows/sp/consumer/oidc/oidc-beans.xml      |   3 +
 .../idp/flows/sp/consumer/oidc/oidc-flow.xml       |  31 +-----
 .../sp/oidc/flows/OIDCTokenConsumerFlowTest.java   |  55 ++++++++++-
 .../impl/ValidateAuthenticationResponseResult.java | 104 +++++++++++++++++++++
 5 files changed, 168 insertions(+), 29 deletions(-)

diff --git a/sp-oidc-conf-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml b/sp-oidc-conf-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml
index 309012e..ef54bda 100644
--- a/sp-oidc-conf-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml
+++ b/sp-oidc-conf-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml
@@ -71,11 +71,13 @@
             <set>
                 <value>code</value>
                 <value>state</value>
+                <value>error</value>
+                <value>error_description</value>
+                <value>error_uri</value>
             </set>
         </property>
         <property name="requiredParameters">
             <set>
-                <value>code</value>
                 <value>state</value>
             </set>
         </property>
diff --git a/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/oidc/oidc-beans.xml b/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/oidc/oidc-beans.xml
index 9cd44e5..487b097 100644
--- a/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/oidc/oidc-beans.xml
+++ b/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/oidc/oidc-beans.xml
@@ -59,6 +59,9 @@
 			<bean class="net.shibboleth.sp.oidc.profile.impl.SetIssuerToPeerContextConsumer"/>
 		</property>
 	</bean>
+    
+    <bean id="ValidateAuthenticationResponseResult" scope="prototype"
+        class="net.shibboleth.sp.oidc.profile.impl.ValidateAuthenticationResponseResult" />
         
     <bean id="ValidateResponseStateMatchesRequest" scope="prototype"
         class="net.shibboleth.sp.oidc.profile.impl.ValidateResponseState"
diff --git a/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/oidc/oidc-flow.xml b/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/oidc/oidc-flow.xml
index 6f1e973..d17369e 100644
--- a/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/oidc/oidc-flow.xml
+++ b/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/oidc/oidc-flow.xml
@@ -10,33 +10,13 @@
        <evaluate expression="ProcessNonceCorrelationCookie" />
        <evaluate expression="ProcessIssuerCorrelationCookie" />
        <evaluate expression="ValidateResponseStateMatchesRequest" />
+       <evaluate expression="ValidateAuthenticationResponseResult" />
        
        <evaluate expression="ProviderMetadataLookup" />        
        <evaluate expression="InitializeRelyingPartyContextFromOIDCPeer" />
        <evaluate expression="SelectRelyingPartyConfiguration" />
        <evaluate expression="SelectProfileConfiguration" />
        
-       
-       <!--  SAML equivs for reference -->
-       
-       
-
-      <!--  <evaluate expression="PopulateSignatureValidationParameters" />
-        <evaluate expression="PopulateClientTLSValidationParameters" />
-        <evaluate expression="PopulateInboundMessageContextWithSAMLSelf" />
-        
-        <evaluate expression="HandleResponse" />
-        <evaluate expression="PopulateDecryptionParameters" />
-        <evaluate expression="DecryptAssertions" />
-        <evaluate expression="ValidateAssertions" />
-        <evaluate expression="ProcessAssertionsForAuthentication" />
-        <evaluate expression="DecryptNameIDs" />
-        <evaluate expression="DecryptAttributes" />
-        <evaluate expression="ExtractSAMLAttributes" /> -->
-        <!-- <evaluate expression="PrepareAgentResponse" /> -->
-<!--        <evaluate expression="PostAssertionPopulateAuditContext" />-->
-<!--        <evaluate expression="PostResponsePopulateAuditContext" />-->
-        
         <evaluate expression="'proceed'" />
         
         <transition on="proceed" to="AuthorizationCodeFlow" />
@@ -51,7 +31,7 @@
         <evaluate expression="DecryptIDTokenJWE" />
         <!--Validation of the JWT signature is optional if TLS server validation was performed -->
         <evaluate expression="IDTokenSignatureValidation" />
-    <evaluate expression="ValidateIDTokenClaims" />
+        <evaluate expression="ValidateIDTokenClaims" />
       <!--   <evaluate expression="TokenResponsePopulateAuditContext" /> -->
         <evaluate expression="'proceed'" />
         <transition on="proceed" to="CheckUserInfoClaimsRequired" />
@@ -94,11 +74,10 @@
     </action-state>
     
     <action-state id="FinalizeResponse">
-        <evaluate expression="ProcessEndUserClaims" />        
-  <!--      <evaluate expression="PostResponsePopulateAuditContext" />-->
+        <evaluate expression="ProcessEndUserClaims" />       
+
    <!--     <evaluate expression="ValidateOIDCAuthentication" /> -->
-   <!-- <evaluate expression="PopulateSubjectCanonicalizationContext" />
-        <evaluate expression="WriteAuditLog" /> -->
+
         <evaluate expression="ExtractOIDCClaims"/>
         <evaluate expression="PrepareAgentResponse"/>
         <evaluate expression="'proceed'" />        
diff --git a/sp-oidc-conf-impl/src/test/java/net/shibboleth/sp/oidc/flows/OIDCTokenConsumerFlowTest.java b/sp-oidc-conf-impl/src/test/java/net/shibboleth/sp/oidc/flows/OIDCTokenConsumerFlowTest.java
index 57e0f90..e69f1a5 100644
--- a/sp-oidc-conf-impl/src/test/java/net/shibboleth/sp/oidc/flows/OIDCTokenConsumerFlowTest.java
+++ b/sp-oidc-conf-impl/src/test/java/net/shibboleth/sp/oidc/flows/OIDCTokenConsumerFlowTest.java
@@ -42,6 +42,7 @@ import org.mockito.Mockito;
 import org.opensaml.core.xml.XMLObject;
 import org.opensaml.core.xml.io.UnmarshallingException;
 import org.opensaml.core.xml.util.XMLObjectSupport;
+import org.opensaml.profile.action.EventIds;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.opensaml.saml.saml2.core.NameID;
 import org.opensaml.saml.saml2.core.NameIDType;
@@ -62,11 +63,14 @@ import com.nimbusds.jose.JWSAlgorithm;
 import com.nimbusds.jwt.JWT;
 import com.nimbusds.jwt.JWTClaimsSet;
 import com.nimbusds.oauth2.sdk.AuthorizationCode;
+import com.nimbusds.oauth2.sdk.AuthorizationResponse;
+import com.nimbusds.oauth2.sdk.ErrorObject;
 import com.nimbusds.oauth2.sdk.ResponseMode;
 import com.nimbusds.oauth2.sdk.id.State;
 import com.nimbusds.oauth2.sdk.token.AccessToken;
 import com.nimbusds.oauth2.sdk.token.BearerAccessToken;
 import com.nimbusds.oauth2.sdk.token.RefreshToken;
+import com.nimbusds.openid.connect.sdk.AuthenticationErrorResponse;
 import com.nimbusds.openid.connect.sdk.AuthenticationSuccessResponse;
 import com.nimbusds.openid.connect.sdk.OIDCTokenResponse;
 import com.nimbusds.openid.connect.sdk.UserInfoSuccessResponse;
@@ -257,6 +261,28 @@ public class OIDCTokenConsumerFlowTest extends AbstractSPFlowTest {
         assertOutputMessageEvent(result, AuthnEventIds.NO_CREDENTIALS);
     }
     
+    /**
+     * Test successful flow.
+     * 
+     * @throws IOException 
+     */
+    @Test
+    public void testFail_ErrorResponse() throws IOException {
+        final DDF input = buildRemotedQueryStringResponse(buildErrorResponse(TestConstants.STATE_STRING));
+        
+        // Add cookies
+        input.addmember("http.headers.Cookie")
+            .unsafe_string(TestConstants.COOKIE_HEADER_BAD_STATE.getBytes("UTF-8"));
+
+        setApplicationRequest("test-oidc-application-with-ro", input);
+
+        final FlowExecutionResult result = flowExecutor.launchExecution(TestConstants.FLOW_ID, null, externalContext);
+        assertFlowExecutionResult(result, TestConstants.FLOW_ID);
+        assertFlowExecutionOutcome(result.getOutcome());
+        assertOutputMessageEvent(result, EventIds.MESSAGE_PROC_ERROR);
+    }
+  
+    
     /**
      * Construct a successful OIDC token response with signed id_token.
      * 
@@ -556,17 +582,41 @@ public class OIDCTokenConsumerFlowTest extends AbstractSPFlowTest {
         );
     }
     
+    /**
+     * Build a invalid_request error response, from openID connect core.
+     * 
+     * @return the error response
+     */
+    private AuthenticationErrorResponse buildErrorResponse(final String stateString) {
+
+        final URI redirectURI = URI.create(TestConstants.RESPONSE_URL);
+        final State state = new State(stateString);
+        
+        final ErrorObject error = new ErrorObject(
+            "invalid_request",
+            "Unsupported response_type value",
+            400,
+            null);
+        
+        return new AuthenticationErrorResponse(
+            redirectURI,
+            error,
+            state,
+            ResponseMode.QUERY);
+
+    }
+    
     
     /**
      * Encodes an OIDC response into the query string of a redirect URI embedded in a remoted message.
      * 
-     * @param response succesful OIDC response to encode
+     * @param response OIDC response to encode
      * 
      * @return input object suitable for token consumer flow
      * 
      * @throws IOException on error 
      */
-    @Nonnull private DDF buildRemotedQueryStringResponse(@Nonnull final AuthenticationSuccessResponse response) 
+    @Nonnull private DDF buildRemotedQueryStringResponse(@Nonnull final AuthorizationResponse response) 
             throws IOException {                
 
         final DDF obj = new DDF(null).structure();
@@ -582,4 +632,5 @@ public class OIDCTokenConsumerFlowTest extends AbstractSPFlowTest {
         return obj;
     }
     
+    
 }
\ No newline at end of file
diff --git a/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/ValidateAuthenticationResponseResult.java b/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/ValidateAuthenticationResponseResult.java
new file mode 100644
index 0000000..a6965d5
--- /dev/null
+++ b/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/ValidateAuthenticationResponseResult.java
@@ -0,0 +1,104 @@
+/*
+ * 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.sp.oidc.profile.impl;
+
+import javax.annotation.Nonnull;
+
+import org.opensaml.messaging.context.MessageContext;
+import org.opensaml.profile.action.ActionSupport;
+import org.opensaml.profile.action.EventIds;
+import org.opensaml.profile.context.ProfileRequestContext;
+import org.slf4j.Logger;
+
+import com.nimbusds.oauth2.sdk.ErrorResponse;
+import com.nimbusds.openid.connect.sdk.AuthenticationErrorResponse;
+import com.nimbusds.openid.connect.sdk.AuthenticationResponse;
+
+import net.shibboleth.shared.annotation.constraint.NonnullBeforeExec;
+import net.shibboleth.shared.primitive.LoggerFactory;
+import net.shibboleth.sp.profile.AbstractAgentAction;
+
+/**
+ * An action that checks the inbound authentication response indicated success as opposed to an error.
+ *  
+ * @event {@link EventIds#PROCEED_EVENT_ID}
+ * @event {@link EventIds#INVALID_MSG_CTX}
+ * @event {@link EventIds#MESSAGE_PROC_ERROR}
+ */
+public class ValidateAuthenticationResponseResult extends AbstractAgentAction {
+
+    /** Class logger. */
+    @Nonnull private final Logger log = LoggerFactory.getLogger(ValidateAuthenticationResponseResult.class);
+
+    /** Context containing the result to examine. */
+    @NonnullBeforeExec private AuthenticationResponse authenticationResponse;
+    
+    @Override
+    protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext) {        
+    
+        final MessageContext inMsgContext = profileRequestContext.getInboundMessageContext();
+        if (inMsgContext == null) {
+            log.debug("{} Inbound message context was null", getLogPrefix());
+            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MSG_CTX);
+            return false;
+        }
+       
+        if (inMsgContext.getMessage() instanceof final AuthenticationResponse response) {
+            authenticationResponse = response;
+        } else {
+            log.debug("{} Inbound message was not an authentication response", getLogPrefix());
+            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MSG_CTX);
+            return false;
+        }        
+        return true;
+    }
+
+    @Override
+    protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
+       
+        if (!authenticationResponse.indicatesSuccess()) {
+            final AuthenticationErrorResponse error = authenticationResponse.toErrorResponse();
+            assert error != null;
+            if (log.isErrorEnabled()) {
+	            log.error("{} Authentication Response contained an error: {}", 
+	                    getLogPrefix(), buildErrorResponseString(error));
+            }
+            ActionSupport.buildEvent(profileRequestContext, EventIds.MESSAGE_PROC_ERROR);
+        } else {
+            log.debug("{} OpenID Provider signalled a successful response", getLogPrefix());
+        }
+       
+    }
+    
+    /**
+     * Build an error response string from the {@link ErrorResponse} object.
+     * 
+     * @param error the error
+     * 
+     * @return a string representation of the error
+     */
+    private String buildErrorResponseString(@Nonnull final AuthenticationErrorResponse error) {
+        final StringBuilder builder = new StringBuilder();
+        if (error.getErrorObject() != null) {
+            builder.append("Code -> '").append(error.getErrorObject().getCode()).append("'");
+            builder.append(", ");
+            builder.append("Description -> '").append(error.getErrorObject().getDescription()).append("'");
+        } else {
+            builder.append("Unknown error response");
+        }
+        return builder.toString();
+    }
+    
+}

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


More information about the commits mailing list