[java-plugin-shibd] 02/02: Adjust contract of state mapping action to address edge cases.
Scott Cantor
cantor.2 at osu.edu
Fri Apr 11 15:50:20 UTC 2025
This is an automated email from the git hooks/post-receive script.
scantor pushed a commit to branch main
in repository java-plugin-shibd.
View the commit online:
http://git.shibboleth.net/view/?p=java-plugin-shibd.git;a=commit;h=f0b4f42aa7c3deb0dca1763d068f5adf3be404e5
commit f0b4f42aa7c3deb0dca1763d068f5adf3be404e5
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Fri Apr 11 11:50:13 2025 -0400
Adjust contract of state mapping action to address edge cases.
---
.../session-initiator/session-initiator-beans.xml | 3 +-
.../shibboleth/idp/module/conf/sp/sp.properties | 1 +
.../sp/messaging/RemotedHttpServletRequest.java | 18 +++++++++--
.../sp/profile/impl/MapResourceToStateToken.java | 35 +++++++++++++++++-----
.../profile/impl/MapResourceToStateTokenTest.java | 5 ++--
5 files changed, 48 insertions(+), 14 deletions(-)
diff --git a/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/session-initiator/session-initiator-beans.xml b/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/session-initiator/session-initiator-beans.xml
index 8b9e448..cbff124 100644
--- a/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/session-initiator/session-initiator-beans.xml
+++ b/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/session-initiator/session-initiator-beans.xml
@@ -12,6 +12,7 @@
<bean id="MapResourceToStateToken"
class="net.shibboleth.sp.profile.impl.MapResourceToStateToken" scope="prototype"
- p:createOutputObjects="true" />
+ p:createOutputObjects="true"
+ p:errorFatal="%{sp.stateToken.errorsFatal:false}" />
</beans>
diff --git a/sp-conf-impl/src/main/resources/net/shibboleth/idp/module/conf/sp/sp.properties b/sp-conf-impl/src/main/resources/net/shibboleth/idp/module/conf/sp/sp.properties
index a2cfc72..2e817bc 100644
--- a/sp-conf-impl/src/main/resources/net/shibboleth/idp/module/conf/sp/sp.properties
+++ b/sp-conf-impl/src/main/resources/net/shibboleth/idp/module/conf/sp/sp.properties
@@ -32,6 +32,7 @@ sp.service.agents.checkInterval = PT5M
# Default state token management (SAML RelayState, etc.)
+#sp.stateToken.errorsFatal = false
# Set to shibboleth.CookieStateTokenManager to switch to cookie-based mechanism
#sp.stateToken.Manager = shibboleth.StorageStateTokenManager
# Controls storage back-end for storage-based state tokens
diff --git a/sp-server-api/src/main/java/net/shibboleth/sp/messaging/RemotedHttpServletRequest.java b/sp-server-api/src/main/java/net/shibboleth/sp/messaging/RemotedHttpServletRequest.java
index 2e05328..275e65e 100644
--- a/sp-server-api/src/main/java/net/shibboleth/sp/messaging/RemotedHttpServletRequest.java
+++ b/sp-server-api/src/main/java/net/shibboleth/sp/messaging/RemotedHttpServletRequest.java
@@ -631,15 +631,27 @@ public class RemotedHttpServletRequest implements HttpServletRequest {
}
final ByteBuffer wrapper = ByteBuffer.wrap(buffer);
-
+
try {
- return UTF_8.decode(wrapper).toString();
+ final String decoded = UTF_8.decode(wrapper).toString();
+ final int delim = decoded.indexOf('?');
+ if (delim > 0) {
+ return decoded.substring(0, delim);
+ } else {
+ return decoded;
+ }
} catch (final CharacterCodingException e) {
}
try {
- return ISO_8859_1.decode(wrapper).toString();
+ final String decoded = ISO_8859_1.decode(wrapper).toString();
+ final int delim = decoded.indexOf('?');
+ if (delim > 0) {
+ return decoded.substring(0, delim);
+ } else {
+ return decoded;
+ }
} catch (final CharacterCodingException e) {
}
diff --git a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/MapResourceToStateToken.java b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/MapResourceToStateToken.java
index d5e1dbb..a8c8747 100644
--- a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/MapResourceToStateToken.java
+++ b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/MapResourceToStateToken.java
@@ -18,12 +18,12 @@ import java.io.IOException;
import javax.annotation.Nonnull;
+import org.opensaml.profile.action.ActionSupport;
import org.opensaml.profile.action.EventIds;
import org.opensaml.profile.context.ProfileRequestContext;
import org.slf4j.Logger;
import net.shibboleth.shared.annotation.constraint.NonnullBeforeExec;
-import net.shibboleth.shared.annotation.constraint.NotEmpty;
import net.shibboleth.shared.primitive.LoggerFactory;
import net.shibboleth.sp.context.AgentRequestContext;
import net.shibboleth.sp.ddf.DDF;
@@ -36,15 +36,16 @@ import net.shibboleth.sp.profile.SPConstants;
* Action that maps a target/resource URL into a state token.
*
* <p>The existence of a {@link SPConstants#STATE} member will cause the action
- * to be skipped, as will the absence of a {@link SPConstants#TARGET} member to
- * translate.</p>
+ * to be skipped, while the absence of {@link SPConstants#TARGET} will result in
+ * failure.</p>
+ *
+ * <p>Errors may be ignored or result in an {@link EventIds#IO_ERROR} event.</p>
*
* @event {@link EventIds#PROCEED_EVENT_ID}
+ * @event {@link EventIds#INVALID_MESSAGE}
+ * @event {@link EventIds#IO_ERROR}
*/
public class MapResourceToStateToken extends AbstractApplicationAction {
-
- /** Static byte array with query string separator. */
- @Nonnull @NotEmpty public static byte[] QUERY_SEPERATOR = {'?'};
/** Class logger. */
@Nonnull private Logger log = LoggerFactory.getLogger(MapResourceToStateToken.class);
@@ -52,6 +53,9 @@ public class MapResourceToStateToken extends AbstractApplicationAction {
/** Whether to create the output objects into which the message will be encoded. */
private boolean createOutputObjects;
+ /** Whether an error constructing a state token is fatal. */
+ private boolean errorFatal;
+
/** Agent input. */
@NonnullBeforeExec private DDF input;
@@ -71,6 +75,19 @@ public class MapResourceToStateToken extends AbstractApplicationAction {
createOutputObjects = flag;
}
+ /**
+ * Sets whether an error computing a state token should result in a fatal event.
+ *
+ * <p>Defaults to false.</p>
+ *
+ * @param flag flag to set
+ */
+ public void setErrorFatal(final boolean flag) {
+ checkSetterPreconditions();
+
+ errorFatal = flag;
+ }
+
/** {@inheritDoc} */
@Override
protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
@@ -91,7 +108,8 @@ public class MapResourceToStateToken extends AbstractApplicationAction {
target = input.getmember(SPConstants.TARGET).unsafe_string();
if (target == null) {
- log.debug("{} Input message did not contain {} member", getLogPrefix(), SPConstants.TARGET);
+ log.warn("{} Input message did not contain {} member", getLogPrefix(), SPConstants.TARGET);
+ ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MESSAGE);
return false;
}
@@ -124,6 +142,9 @@ public class MapResourceToStateToken extends AbstractApplicationAction {
log.debug("{} Target resource preserved to state token: {}", getLogPrefix(), token);
} catch (final IOException e) {
log.warn("{} Exception preserving target resource to state token", getLogPrefix(), e);
+ if (errorFatal) {
+ ActionSupport.buildEvent(profileRequestContext, EventIds.IO_ERROR);
+ }
}
} finally {
RemotedHttpServletRequestResponseContext.clearCurrent();
diff --git a/sp-server-impl/src/test/java/net/shibboleth/sp/profile/impl/MapResourceToStateTokenTest.java b/sp-server-impl/src/test/java/net/shibboleth/sp/profile/impl/MapResourceToStateTokenTest.java
index c8ea8ed..4300171 100644
--- a/sp-server-impl/src/test/java/net/shibboleth/sp/profile/impl/MapResourceToStateTokenTest.java
+++ b/sp-server-impl/src/test/java/net/shibboleth/sp/profile/impl/MapResourceToStateTokenTest.java
@@ -20,6 +20,7 @@ import java.time.Duration;
import javax.annotation.Nonnull;
+import org.opensaml.profile.action.EventIds;
import org.opensaml.storage.impl.MemoryStorageService;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
@@ -83,9 +84,7 @@ public class MapResourceToStateTokenTest extends BaseAgplicationActionTest {
arc.setInput(input);
final Event event = action.execute(src);
- ActionTestingSupport.assertProceedEvent(event);
-
- Assert.assertNull(input.getmember(SPConstants.STATE).string());
+ ActionTestingSupport.assertEvent(event, EventIds.INVALID_MESSAGE);
}
@Test
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list