[java-plugin-shibd] branch main updated: Switch to session-bound record object for cached authentication.
Scott Cantor
cantor.2 at osu.edu
Thu May 30 20:09:44 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.
View the commit online:
http://git.shibboleth.net/view/?p=java-plugin-shibd.git;a=commit;h=3f14a3adb51318e048d0c75bfaa13d9b39436468
The following commit(s) were added to refs/heads/main by this push:
new 3f14a3a Switch to session-bound record object for cached authentication.
3f14a3a is described below
commit 3f14a3adb51318e048d0c75bfaa13d9b39436468
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Thu May 30 16:09:38 2024 -0400
Switch to session-bound record object for cached authentication.
---
.../idp/flows/sp/abstract/sp-abstract-beans.xml | 14 +--
.../sp/authn/impl/CachedAgentAuthentication.java | 31 +++++
.../authn/impl/ValidateCachedAuthentication.java | 136 ++++-----------------
.../impl/ValidateCachedAuthenticationTest.java | 133 ++++----------------
4 files changed, 80 insertions(+), 234 deletions(-)
diff --git a/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/abstract/sp-abstract-beans.xml b/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/abstract/sp-abstract-beans.xml
index 3ac8dbb..e4e9add 100644
--- a/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/abstract/sp-abstract-beans.xml
+++ b/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/abstract/sp-abstract-beans.xml
@@ -18,7 +18,7 @@
p:metricStrategy="#{getObject('shibboleth.metrics.MetricStrategy')}" />
<bean id="LogEvent" class="org.opensaml.profile.action.impl.LogEvent" scope="prototype"
- p:suppressedEvents="#{getObject('shibboleth.SuppressedEvents') ?: getObject('shibboleth.DefaultSuppressedEvents')}">
+ p:suppressedEvents="#{getObject('shibboleth.SuppressedEvents') ?: getObject('shibboleth.DefaultSuppressedEvents')}">
<property name="eventContextLookupStrategy">
<bean class="net.shibboleth.idp.profile.context.navigate.WebFlowCurrentEventLookupFunction" />
</property>
@@ -38,17 +38,7 @@
<bean id="ValidateCachedAuthentication"
class="net.shibboleth.sp.authn.impl.ValidateCachedAuthentication" scope="prototype"
- p:httpServletRequestSupplier-ref="shibboleth.HttpServletRequestSupplier"
- p:cookieName="%{sp.agent.authn.cached.cookieName:__Host-shibsp_agent_token}"
- p:cookieManager-ref="sp.CookieManager"
- p:dataSealer-ref="shibboleth.DataSealer" />
-
- <bean id="sp.CookieManager" parent="shibboleth.PersistentCookieManager"
- p:secure="true"
- p:httpOnly="true"
- p:cookieDomain=""
- p:cookiePath="/"
- p:maxAge="%{sp.agent.authn.cached.maxAge:3600}" />
+ p:httpServletRequestSupplier-ref="shibboleth.HttpServletRequestSupplier" />
<bean id="ValidateAgentCredentials"
class="net.shibboleth.sp.authn.impl.ValidateAgentCredentials" scope="prototype"
diff --git a/sp-server-impl/src/main/java/net/shibboleth/sp/authn/impl/CachedAgentAuthentication.java b/sp-server-impl/src/main/java/net/shibboleth/sp/authn/impl/CachedAgentAuthentication.java
new file mode 100644
index 0000000..69da947
--- /dev/null
+++ b/sp-server-impl/src/main/java/net/shibboleth/sp/authn/impl/CachedAgentAuthentication.java
@@ -0,0 +1,31 @@
+/*
+ * 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.authn.impl;
+
+import java.time.Instant;
+
+import javax.annotation.Nonnull;
+
+/**
+ * A record capturing cached authentication state for an agent.
+ *
+ * @param agentId agent ID
+ * @param address address for which the session is valid
+ * @param expiration session expiration
+ *
+ */
+public record CachedAgentAuthentication(@Nonnull String agentId, @Nonnull String address, @Nonnull Instant expiration) {
+
+}
\ No newline at end of file
diff --git a/sp-server-impl/src/main/java/net/shibboleth/sp/authn/impl/ValidateCachedAuthentication.java b/sp-server-impl/src/main/java/net/shibboleth/sp/authn/impl/ValidateCachedAuthentication.java
index e50e4d1..f7adf35 100644
--- a/sp-server-impl/src/main/java/net/shibboleth/sp/authn/impl/ValidateCachedAuthentication.java
+++ b/sp-server-impl/src/main/java/net/shibboleth/sp/authn/impl/ValidateCachedAuthentication.java
@@ -14,6 +14,7 @@
package net.shibboleth.sp.authn.impl;
+import java.time.Instant;
import java.util.function.Function;
import javax.annotation.Nonnull;
@@ -25,35 +26,21 @@ import org.opensaml.profile.context.ProfileRequestContext;
import org.slf4j.Logger;
import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpSession;
import net.shibboleth.idp.profile.AbstractProfileAction;
-import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
import net.shibboleth.shared.annotation.constraint.NonnullBeforeExec;
import net.shibboleth.shared.annotation.constraint.NotEmpty;
-import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.logic.Constraint;
-import net.shibboleth.shared.net.CookieManager;
-import net.shibboleth.shared.net.URISupport;
import net.shibboleth.shared.primitive.LoggerFactory;
-import net.shibboleth.shared.primitive.StringSupport;
-import net.shibboleth.shared.security.DataExpiredException;
-import net.shibboleth.shared.security.DataSealer;
-import net.shibboleth.shared.security.DataSealerException;
-import net.shibboleth.shared.servlet.HttpServletSupport;
import net.shibboleth.sp.Agent;
import net.shibboleth.sp.context.AgentRequestContext;
/**
- * An action that checks for a sealed cookie authenticating request without the need for
- * validating a shared secret or other credentials.
+ * An action that checks for a record in the {@link HttpSession} to bypass agent authentication.
*
* <p>For safety's sake, the default event signal indicates that authentication should
* <strong>not</strong> be bypassed. An explicit event is used to signal bypass.</p>
*
- * <p>The cookie is an address-bound bearer token containing the agent authorized to use it.
- * The data being sealed, it has an internal expiration independently of the cookie's own.</p>
- *
- * <p>TODO: adding a key proof via MAC in some way</p>
- *
* @event {@link EventIds#PROCEED_EVENT_ID}
* @event {@link EventIds#INVALID_PROFILE_CTX}
* @event {@link #BYPASS_AUTHENTICATION}
@@ -61,6 +48,9 @@ import net.shibboleth.sp.context.AgentRequestContext;
*/
public class ValidateCachedAuthentication extends AbstractProfileAction {
+ /** Session attribute holding agent session record. */
+ @Nonnull @NotEmpty public static final String AGENT_SESSION_ATTRIBUTE = "net.shibboleth.sp.agent.cachedAuthentication";
+
/** Bypass event indicating cookie was accepted. */
@Nonnull @NotEmpty public static final String BYPASS_AUTHENTICATION = "BypassAuthentication";
@@ -70,15 +60,6 @@ public class ValidateCachedAuthentication extends AbstractProfileAction {
/** Lookup strategy for {@link AgentRequestContext}. */
@Nonnull private Function<ProfileRequestContext,AgentRequestContext> agentRequestContextLookupStrategy;
- /** Cookie name to use. */
- @NonnullAfterInit private String cookieName;
-
- /** CookieManager to use. */
- @NonnullAfterInit private CookieManager cookieManager;
-
- /** DataSealer to use. */
- @NonnullAfterInit private DataSealer dataSealer;
-
/** Cached agent from context. */
@NonnullBeforeExec private Agent agent;
@@ -100,49 +81,6 @@ public class ValidateCachedAuthentication extends AbstractProfileAction {
"AgentRequestContext lookup strategy cannot be null");
}
- /**
- * Sets the cookie name to use for cached authentication.
- *
- * @param name cookie name
- */
- public void setCookieName(@Nonnull @NotEmpty final String name) {
- checkSetterPreconditions();
-
- cookieName = Constraint.isNotNull(StringSupport.trimOrNull(name), "Cookie name cannot be null or empty");
- }
-
- /**
- * Sets the {@link CookieManager} to use.
- *
- * @param manager cookie manager
- */
- public void setCookieManager(@Nonnull final CookieManager manager) {
- checkSetterPreconditions();
-
- cookieManager = Constraint.isNotNull(manager, "CookieManager cannot be null");
- }
-
- /**
- * Sets the {@link DataSealer} to use.
- *
- * @param sealer data sealer
- */
- public void setDataSealer(@Nonnull final DataSealer sealer) {
- checkSetterPreconditions();
-
- dataSealer = Constraint.isNotNull(sealer, "DataSealer cannot be null");
- }
-
- /** {@inheritDoc} */
- @Override
- protected void doInitialize() throws ComponentInitializationException {
- super.doInitialize();
-
- if (cookieName == null || cookieManager == null || dataSealer == null) {
- throw new ComponentInitializationException("CookieManager, DataSealer, and cookie name must be set");
- }
- }
-
/** {@inheritDoc} */
@Override
protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
@@ -171,53 +109,29 @@ public class ValidateCachedAuthentication extends AbstractProfileAction {
@Override
protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
- try {
- assert cookieName != null;
- final String wrapped = cookieManager.getCookieValue(cookieName, null);
- if (wrapped == null) {
- log.debug("{} No cookie from agent '{}', skipping cached authentication check", getLogPrefix(),
- agent.getId());
- return;
- }
-
- final String unwrapped = dataSealer.unwrap(URISupport.doURLDecode(wrapped));
- if (isValid(profileRequestContext, unwrapped)) {
- log.info("{} Accepted cookie from agent '{}', skipping full authentication", getLogPrefix(),
- agent.getId());
+ final HttpServletRequest request = getHttpServletRequest();
+ final HttpSession session = request != null ? request.getSession(false) : null;
+ if (session == null) {
+ log.debug("{} No HttpSession associated with request from agent '{}', skipping cached authentication check",
+ getLogPrefix(), agent.getId());
+ return;
+ }
+
+ final Object attr = session.getAttribute(AGENT_SESSION_ATTRIBUTE);
+ if (attr instanceof CachedAgentAuthentication cached) {
+ assert request != null;
+ if (cached.agentId().equals(agent.getId()) && cached.address().equals(request.getRemoteAddr()) &&
+ cached.expiration().isAfter(Instant.now())) {
+ log.info("{} Accepted cached authentication session from agent '{}'", getLogPrefix(), agent.getId());
ActionSupport.buildEvent(profileRequestContext, BYPASS_AUTHENTICATION);
} else {
- log.debug("{} Rejected cookie from agent '{}', full authentication will proceed", getLogPrefix(),
- agent.getId());
+ log.info("{} Rejected cached authentication session record from agent '{}': {}",
+ getLogPrefix(), agent.getId(), cached);
}
- } catch (final DataExpiredException e) {
- log.debug("{} Cookie from agent '{}' expired, authentication not bypassed", getLogPrefix(), agent.getId());
- } catch (final DataSealerException e) {
- log.warn("{} Error decrypting cookie from agent '{}', authentication not bypassed", getLogPrefix(),
- agent.getId(), e);
- }
- }
-
- private boolean isValid(@Nonnull final ProfileRequestContext profileRequestContext, @Nonnull final String cookie) {
- final String[] split = cookie.split("!");
- if (split == null || split.length != 2) {
- log.warn("{} Cookie malformed from agent '{}'", getLogPrefix(), agent.getId());
- return false;
- }
-
- if (!split[0].equals(agent.getId())) {
- log.warn("{} Cookie from agent '{}' issued to agent '{}'", getLogPrefix(), agent.getId(), split[0]);
- return false;
+ } else {
+ log.debug("{} No session record for request from agent '{}', full authentication will proceed",
+ getLogPrefix(), agent.getId());
}
-
- final HttpServletRequest request = getHttpServletRequest();
- final String addr = request != null ? HttpServletSupport.getRemoteAddr(request) : null;
- if (!split[1].equals(addr)) {
- log.warn("{} Cookie from agent '{}' and address {} issued to address {}", getLogPrefix(), agent.getId(),
- split[1], addr);
- return false;
- }
-
- return true;
}
}
\ No newline at end of file
diff --git a/sp-server-impl/src/test/java/net/shibboleth/sp/authn/impl/ValidateCachedAuthenticationTest.java b/sp-server-impl/src/test/java/net/shibboleth/sp/authn/impl/ValidateCachedAuthenticationTest.java
index 6585158..0d9f681 100644
--- a/sp-server-impl/src/test/java/net/shibboleth/sp/authn/impl/ValidateCachedAuthenticationTest.java
+++ b/sp-server-impl/src/test/java/net/shibboleth/sp/authn/impl/ValidateCachedAuthenticationTest.java
@@ -17,37 +17,25 @@ package net.shibboleth.sp.authn.impl;
import java.time.Instant;
import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
import org.opensaml.profile.action.EventIds;
import org.opensaml.profile.context.ProfileRequestContext;
-import org.springframework.core.io.ClassPathResource;
import org.springframework.mock.web.MockHttpServletRequest;
-import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.mock.web.MockHttpSession;
import org.springframework.webflow.execution.Event;
import org.springframework.webflow.execution.RequestContext;
-import org.testng.Assert;
-import org.testng.annotations.BeforeClass;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
-import com.google.common.net.UrlEscapers;
-
-import jakarta.servlet.http.Cookie;
import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.http.HttpServletResponse;
import net.shibboleth.idp.authn.context.AuthenticationContext;
import net.shibboleth.idp.profile.context.navigate.WebflowRequestContextProfileRequestContextLookup;
import net.shibboleth.idp.profile.testing.ActionTestingSupport;
import net.shibboleth.idp.profile.testing.RequestContextBuilder;
import net.shibboleth.shared.annotation.constraint.NotEmpty;
import net.shibboleth.shared.component.ComponentInitializationException;
-import net.shibboleth.shared.net.CookieManager;
import net.shibboleth.shared.primitive.NonnullSupplier;
-import net.shibboleth.shared.resource.Resource;
-import net.shibboleth.shared.security.DataSealer;
import net.shibboleth.shared.security.DataSealerException;
-import net.shibboleth.shared.security.impl.BasicKeystoreKeyStrategy;
import net.shibboleth.sp.context.AgentRequestContext;
import net.shibboleth.sp.impl.BasicAgent;
@@ -59,55 +47,13 @@ public class ValidateCachedAuthenticationTest {
@Nonnull @NotEmpty protected static final String COOKIE_NAME = "_cookieName";
- private Resource keystoreResource;
- private Resource versionResource;
- private CookieManager cookieManager;
- private DataSealer dataSealer;
-
private MockHttpServletRequest request;
- private MockHttpServletResponse response;
private RequestContext src;
private ProfileRequestContext prc;
private BasicAgent agent;
private ValidateCachedAuthentication action;
- private DataSealer createDataSealer(@Nullable @NotEmpty final String nodePrefix)
- throws DataSealerException, ComponentInitializationException {
- final BasicKeystoreKeyStrategy strategy = new BasicKeystoreKeyStrategy();
-
- strategy.setKeyAlias("secret");
- strategy.setKeyPassword("kpassword");
-
- strategy.setKeystorePassword("password");
- strategy.setKeystoreResource(keystoreResource);
-
- strategy.setKeyVersionResource(versionResource);
-
- strategy.initialize();
-
- final DataSealer sealer = new DataSealer();
- sealer.setKeyStrategy(strategy);
- sealer.setNodePrefix(nodePrefix);
- sealer.initialize();
- return sealer;
- }
-
- @BeforeClass
- public void beforeClass() throws DataSealerException, ComponentInitializationException {
- ClassPathResource resource =
- new ClassPathResource("/net/shibboleth/sp/authn/impl/SealerKeyStore.jks");
- Assert.assertTrue(resource.exists());
- keystoreResource = TestResourceConverter.of(resource);
-
- resource =
- new ClassPathResource("/net/shibboleth/sp/authn/impl/SealerKeyStore.kver");
- Assert.assertTrue(resource.exists());
- versionResource = TestResourceConverter.of(resource);
-
- dataSealer = createDataSealer(null);
- }
-
@BeforeMethod
public void setUp() throws ComponentInitializationException {
src = new RequestContextBuilder().buildRequestContext();
@@ -116,12 +62,6 @@ public class ValidateCachedAuthenticationTest {
request = new MockHttpServletRequest();
request.setRemoteAddr("127.0.0.1");
- response = new MockHttpServletResponse();
-
- cookieManager = new CookieManager();
- cookieManager.setHttpServletRequestSupplier(new NonnullSupplier<>() { @Nonnull public HttpServletRequest get() {return request;}});
- cookieManager.setHttpServletResponseSupplier(new NonnullSupplier<>() { @Nonnull public HttpServletResponse get() {return response;}});
- cookieManager.initialize();
agent = new BasicAgent();
agent.setId("foo");
@@ -129,9 +69,6 @@ public class ValidateCachedAuthenticationTest {
action = new ValidateCachedAuthentication();
action.setHttpServletRequestSupplier(new NonnullSupplier<>() { @Nonnull public HttpServletRequest get() {return request;}});
- action.setCookieName(COOKIE_NAME);
- action.setCookieManager(cookieManager);
- action.setDataSealer(dataSealer);
action.initialize();
}
@Test
@@ -152,27 +89,30 @@ public class ValidateCachedAuthenticationTest {
}
@Test
- public void testNoCookie() throws ComponentInitializationException {
+ public void testNoSession() throws ComponentInitializationException {
prc.ensureSubcontext(AgentRequestContext.class).setAgent(agent);
final Event event = action.execute(src);
ActionTestingSupport.assertProceedEvent(event);
}
@Test
- public void testBadCookie() throws ComponentInitializationException {
+ public void testNoRecord() throws ComponentInitializationException {
prc.ensureSubcontext(AgentRequestContext.class).setAgent(agent);
- request.setCookies(new Cookie(COOKIE_NAME, "zork"));
+ request.setSession(new MockHttpSession());
final Event event = action.execute(src);
ActionTestingSupport.assertProceedEvent(event);
}
@Test
- public void testExoiredCookie() throws ComponentInitializationException, DataSealerException {
+ public void testExoired() throws ComponentInitializationException, DataSealerException {
prc.ensureSubcontext(AgentRequestContext.class).setAgent(agent);
-
- request.setCookies(buildCookie(agent.getId(), "127.0.0.1", Instant.now().minusSeconds(30)));
+
+ final MockHttpSession session = new MockHttpSession();
+ session.setAttribute(ValidateCachedAuthentication.AGENT_SESSION_ATTRIBUTE,
+ new CachedAgentAuthentication(agent.getId(), "127.0.0.1", Instant.now().minusSeconds(30)));
+ request.setSession(session);
final Event event = action.execute(src);
ActionTestingSupport.assertProceedEvent(event);
@@ -182,7 +122,10 @@ public class ValidateCachedAuthenticationTest {
public void testWrongAgent() throws ComponentInitializationException, DataSealerException {
prc.ensureSubcontext(AgentRequestContext.class).setAgent(agent);
- request.setCookies(buildCookie("wrong", "127.0.0.1", Instant.now().plusSeconds(3600)));
+ final MockHttpSession session = new MockHttpSession();
+ session.setAttribute(ValidateCachedAuthentication.AGENT_SESSION_ATTRIBUTE,
+ new CachedAgentAuthentication("wrong", "127.0.0.1", Instant.now().plusSeconds(3600)));
+ request.setSession(session);
final Event event = action.execute(src);
ActionTestingSupport.assertProceedEvent(event);
@@ -192,17 +135,10 @@ public class ValidateCachedAuthenticationTest {
public void testWrongAddress() throws ComponentInitializationException, DataSealerException {
prc.ensureSubcontext(AgentRequestContext.class).setAgent(agent);
- request.setCookies(buildCookie(agent.getId(), "127.0.0.2", Instant.now().plusSeconds(3600)));
-
- final Event event = action.execute(src);
- ActionTestingSupport.assertProceedEvent(event);
- }
-
- @Test
- public void testWrongFormat() throws ComponentInitializationException, DataSealerException {
- prc.ensureSubcontext(AgentRequestContext.class).setAgent(agent);
-
- request.setCookies(buildCookie(null, "127.0.0.1", Instant.now().plusSeconds(3600)));
+ final MockHttpSession session = new MockHttpSession();
+ session.setAttribute(ValidateCachedAuthentication.AGENT_SESSION_ATTRIBUTE,
+ new CachedAgentAuthentication("wrong", "127.0.0.2", Instant.now().plusSeconds(3600)));
+ request.setSession(session);
final Event event = action.execute(src);
ActionTestingSupport.assertProceedEvent(event);
@@ -212,38 +148,13 @@ public class ValidateCachedAuthenticationTest {
public void testSuccess() throws ComponentInitializationException, DataSealerException {
prc.ensureSubcontext(AgentRequestContext.class).setAgent(agent);
- request.setCookies(buildCookie(agent.getId(), "127.0.0.1", Instant.now().plusSeconds(3600)));
+ final MockHttpSession session = new MockHttpSession();
+ session.setAttribute(ValidateCachedAuthentication.AGENT_SESSION_ATTRIBUTE,
+ new CachedAgentAuthentication(agent.getId(), "127.0.0.1", Instant.now().plusSeconds(3600)));
+ request.setSession(session);
final Event event = action.execute(src);
ActionTestingSupport.assertEvent(event, ValidateCachedAuthentication.BYPASS_AUTHENTICATION);
}
- /**
- * Build a sealed cookie to order.
- *
- * @param agentId agent ID
- * @param address client adddress
- * @param expires data expiration
- *
- * @return the cookie
- *
- * @throws DataSealerException if sealer fails
- */
- @Nonnull private Cookie buildCookie(@Nullable final String agentId, @Nullable final String address,
- @Nonnull final Instant expires) throws DataSealerException {
-
- final StringBuilder builder = new StringBuilder();
- if (agentId != null) {
- builder.append(agentId);
- if (address != null) {
- builder.append('!').append(address);
- }
- } else if (address != null) {
- builder.append(address);
- }
-
- final String wrapped = dataSealer.wrap(builder.toString(), expires);
- return new Cookie(COOKIE_NAME, UrlEscapers.urlFormParameterEscaper().escape(wrapped));
- }
-
}
\ 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