[java-plugin-shibd-oidc] 04/04: JSHIBDOIDC-26 - Add client address to state

Codeberg noreply at shibboleth.net
Wed May 6 15:47:31 UTC 2026


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/1ab7d4144d85d8b768897ee55b2b9d3a0a66a922

commit 1ab7d4144d85d8b768897ee55b2b9d3a0a66a922
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Wed May 6 16:44:41 2026 +0100

    JSHIBDOIDC-26 - Add client address to state
    
     - if enabled by profile config option, add client address to the state
    
    https://shibboleth.atlassian.net/browse/JSHIBDOIDC-26
---
 .../net/shibboleth/sp/service/agent/postconfig.xml |  4 +-
 .../shibboleth/idp/module/conf/sp/oidc.properties  |  3 +
 .../impl/CreateAuthenticationStateData.java        | 77 +++++++++++++++++-----
 .../profile/impl/InitializePeerEntityContext.java  |  3 +
 4 files changed, 70 insertions(+), 17 deletions(-)

diff --git a/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/sp/service/agent/postconfig.xml b/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/sp/service/agent/postconfig.xml
index fba5ada..fcb58b8 100644
--- a/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/sp/service/agent/postconfig.xml
+++ b/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/sp/service/agent/postconfig.xml
@@ -100,8 +100,8 @@
         p:tlsServerValidationSufficient="%{sp.oidc.idToken.tlsServerValidationOnly:false}"
         p:userInfoHttpRequestMethod="%{sp.oidc.userinfo.httpRequestMethod:GET}"
         p:scopes="%{sp.oidc.scopes:#{null}}"
-        p:responseModes="%{sp.oauth2.responseModes:}"
-        p:validateAcrValue="%{sp.oidc.idToken.validateAcrValue:true}">
+        p:validateAcrValue="%{sp.oidc.idToken.validateAcrValue:true}"
+        p:checkAddressPredicate="%{sp.oidc.checkAddress:false}">
     </bean>
     
      <util:constant id="OIDC.SSO.FEATURE_ESSENTIAL_ACR_REQUEST"
diff --git a/sp-oidc-conf-impl/src/main/resources/net/shibboleth/idp/module/conf/sp/oidc.properties b/sp-oidc-conf-impl/src/main/resources/net/shibboleth/idp/module/conf/sp/oidc.properties
index 5439d38..d132139 100644
--- a/sp-oidc-conf-impl/src/main/resources/net/shibboleth/idp/module/conf/sp/oidc.properties
+++ b/sp-oidc-conf-impl/src/main/resources/net/shibboleth/idp/module/conf/sp/oidc.properties
@@ -15,6 +15,9 @@
 
 # Global Request And Response Settings
 
+# Turn on to enable address check during response validation
+#sp.oidc.checkAddress = false
+
 # Comma seperated list of additional scopes e.g. PROFILE or EMAIL. The openid scope is added by default.
 #sp.oidc.scopes = 
 # The HTTP method use to send the authorization request
diff --git a/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/CreateAuthenticationStateData.java b/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/CreateAuthenticationStateData.java
index 2097c62..a560775 100644
--- a/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/CreateAuthenticationStateData.java
+++ b/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/CreateAuthenticationStateData.java
@@ -24,12 +24,17 @@ import org.opensaml.profile.action.EventIds;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.slf4j.Logger;
 
+import net.shibboleth.idp.profile.IdPEventIds;
+import net.shibboleth.oidc.profile.config.OIDCSSORelyingPartyConfiguration;
+import net.shibboleth.profile.context.RelyingPartyContext;
 import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
+import net.shibboleth.shared.annotation.constraint.NonnullBeforeExec;
 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.messaging.RemotedHttpServletRequest;
 import net.shibboleth.sp.profile.AbstractApplicationAction;
 import net.shibboleth.sp.profile.SPConstants;
 import net.shibboleth.sp.state.StateData;
@@ -49,12 +54,24 @@ public class CreateAuthenticationStateData extends AbstractApplicationAction {
     /** Class logger. */
     @Nonnull private final Logger log = LoggerFactory.getLogger(CreateAuthenticationStateData.class);
 
-    /** Lookup strategy for the contents of the state token, as stored in a {@link StateData} or subclass thereof. */
+    /** 
+     * Lookup strategy for the contents of the state token, as stored in a {@link StateData} or subclass thereof. 
+     * This is augmented with other state data as defined in this action. 
+     */
     @NonnullAfterInit private Function<ProfileRequestContext,StateData> stateDataLookupStrategy;
     
-    /** Strategy used to create the {@link StateDataContext} to populate. */
+    /** Strategy used to locate or create the {@link StateDataContext} to populate. */
     @Nonnull private Function<ProfileRequestContext,StateDataContext> stateDataContextCreationStrategy;
     
+    /** Applicable stashed profile configuration. */
+    @NonnullBeforeExec private OIDCSSORelyingPartyConfiguration profileConfiguration;
+    
+    /** The stashed state data context.*/
+    @NonnullBeforeExec private StateDataContext stateDataContext;
+    
+    /** The stashed input DDF from the Agent.*/
+    @NonnullBeforeExec private DDF input;
+    
     /**
      * Constructor.
      */
@@ -81,7 +98,7 @@ public class CreateAuthenticationStateData extends AbstractApplicationAction {
         super.doInitialize();
         
         if (stateDataLookupStrategy == null) {
-            throw new ComponentInitializationException("StateValueLookupStrategy cannot be null");
+            throw new ComponentInitializationException("StateDataLookupStrategy cannot be null");
         }
     }
     
@@ -96,28 +113,50 @@ public class CreateAuthenticationStateData extends AbstractApplicationAction {
         stateDataLookupStrategy = Constraint.isNotNull(strategy, "State data lookup strategy cannot be null");
     }
     
-
     /** {@inheritDoc} */
     @Override
-    protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
-                
-        final StateDataContext stateDataContext = stateDataContextCreationStrategy.apply(profileRequestContext);
+    protected boolean doPreExecute(final ProfileRequestContext profileRequestContext) {
+        if (!super.doPreExecute(profileRequestContext)) {
+            return false;
+        }
+        
+        final RelyingPartyContext rpCtx = profileRequestContext.getSubcontext(RelyingPartyContext.class);
+        if (rpCtx != null && rpCtx.getConfiguration() != null &&
+                rpCtx.getProfileConfig() instanceof final OIDCSSORelyingPartyConfiguration 
+                rpConfig) {
+            profileConfiguration = rpConfig;
+        }
+        if (profileConfiguration == null) {
+            log.error("{} Profile configuration not found", getLogPrefix());   
+            ActionSupport.buildEvent(profileRequestContext, IdPEventIds.INVALID_PROFILE_CONFIG);
+            return false;            
+        }
+        
+        stateDataContext = stateDataContextCreationStrategy.apply(profileRequestContext);
         
         if (stateDataContext == null) {
-            log.error("{} Error creating StateDataContext", getLogPrefix());
+            log.error("{} Error creating or locating StateDataContext", getLogPrefix());
             ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
-            return;
-        }       
+            return false;
+        } 
         
-        // Before we create new state data, pull out the target URL if either in exsiting state or from the Agent input
-        final DDF input = ensureAgentRequestContext().getInput();
+        input = ensureAgentRequestContext().getInput();
         
         if (input == null) {
-            log.error("{} No input message", getLogPrefix());
+            log.error("{} No input message from Agent", getLogPrefix());
             ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
-            return;
+            return false;
         }
         
+        
+        return true;
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {                
+
+        // Before we create new state data, pull out the target URL if either in existing state or from the Agent input        
         byte[] target = input.getmember(SPConstants.TARGET).unsafe_string();
         if (target == null) {
             final StateData oldStateData = stateDataContext.getStateData();
@@ -135,7 +174,15 @@ public class CreateAuthenticationStateData extends AbstractApplicationAction {
         }
         
         // Now add target to state
-        stateData.setRawResource(target);        
+        stateData.setRawResource(target);
+        
+        // Now add client address if enabled
+        if (profileConfiguration.isCheckAddress(profileRequestContext)) {
+            stateData.setClientAddress(
+                    input.getmember(RemotedHttpServletRequest.STRUCTURE_NAME)
+                        .getmember(RemotedHttpServletRequest.REMOTE_ADDR)
+                        .string());
+        }
        
         stateDataContext.setStateData(stateData);
         log.debug("{} Created authentication state data for preservation '{}'", getLogPrefix(), stateData);
diff --git a/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/InitializePeerEntityContext.java b/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/InitializePeerEntityContext.java
index a50e3d0..7f72caf 100644
--- a/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/InitializePeerEntityContext.java
+++ b/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/InitializePeerEntityContext.java
@@ -36,6 +36,9 @@ import net.shibboleth.sp.state.StateData;
 /**
  * An action that initializes the {@link OIDCPeerEntityContext} based on the issuer value from the {@link StateData}.
  * 
+ * <p>It is important the issuer value can not be tampered with. For example, if StateData, it should
+ * be protected when stored, whatever that means in a given deployment.</p>
+ * 
  * @event {@link EventIds#INVALID_MSG_CTX} 
  * @event {@link EventIds#INVALID_PROFILE_CTX}
  */

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


More information about the commits mailing list