[java-identity-provider] branch main updated: IDP-1473 - Remove requirement for IdP session in CAS protocol.
Marvin S. Addison
marvin.addison at gmail.com
Wed Aug 30 21:29:30 UTC 2023
This is an automated email from the git hooks/post-receive script.
serac pushed a commit to branch main
in repository java-identity-provider.
View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=bc78ab36e9638e34f02134ece0d3cda8d189108f
The following commit(s) were added to refs/heads/main by this push:
new bc78ab36e IDP-1473 - Remove requirement for IdP session in CAS protocol.
bc78ab36e is described below
commit bc78ab36e9638e34f02134ece0d3cda8d189108f
Author: Marvin S. Addison <serac at vt.edu>
AuthorDate: Thu Oct 27 09:37:26 2022 -0400
IDP-1473 - Remove requirement for IdP session in CAS protocol.
https://shibboleth.atlassian.net/browse/IDP-1473
Also move session tracking to login flow to put it on the front channel
where browser-based storage mechanisms are more likely to be available.
---
.../net/shibboleth/idp/cas/ticket/TicketState.java | 10 ++--
.../idp/cas/flow/impl/GrantProxyTicketAction.java | 54 ++++++++++--------
.../cas/flow/impl/GrantServiceTicketAction.java | 39 +++++--------
.../impl/UpdateIdPSessionWithSPSessionAction.java | 5 +-
.../idp/cas/flow/impl/ValidateTicketAction.java | 5 +-
.../idp/cas/ticket/impl/AbstractTicketService.java | 66 +++++++++++++---------
.../impl/AbstractTicketSerializer.java | 38 ++++++++-----
.../flow/impl/GrantServiceTicketActionTest.java | 7 ++-
.../cas/ticket/impl/SimpleTicketServiceTest.java | 30 +++++++---
.../impl/ServiceTicketSerializerTest.java | 18 ++++++
.../net/shibboleth/idp/flows/cas/login-beans.xml | 5 ++
.../net/shibboleth/idp/flows/cas/login-flow.xml | 1 +
.../idp/flows/cas/validate-abstract-beans.xml | 5 --
.../idp/flows/cas/validate-abstract-flow.xml | 9 ++-
.../idp/test/flows/cas/LoginFlowTest.java | 6 +-
.../test/flows/cas/ServiceValidateFlowTest.java | 34 -----------
16 files changed, 180 insertions(+), 152 deletions(-)
diff --git a/idp-cas-api/src/main/java/net/shibboleth/idp/cas/ticket/TicketState.java b/idp-cas-api/src/main/java/net/shibboleth/idp/cas/ticket/TicketState.java
index fd52cef67..bdd68e0f2 100644
--- a/idp-cas-api/src/main/java/net/shibboleth/idp/cas/ticket/TicketState.java
+++ b/idp-cas-api/src/main/java/net/shibboleth/idp/cas/ticket/TicketState.java
@@ -36,7 +36,7 @@ import net.shibboleth.shared.primitive.StringSupport;
public class TicketState {
/** ID of session in which ticket is created. */
- @Nonnull private String sessId;
+ @Nullable private String sessId;
/** Canonical authenticated principal name. */
@Nonnull private String authenticatedPrincipalName;
@@ -59,11 +59,11 @@ public class TicketState {
* @param authnMethod principal authentication method ID/name/description
*/
public TicketState(
- @Nonnull final String sessionId,
+ @Nullable final String sessionId,
@Nonnull final String principalName,
@Nonnull final Instant authnInstant,
@Nonnull final String authnMethod) {
- sessId = Constraint.isNotNull(sessionId, "SessionID cannot be null");
+ sessId = sessionId;
authenticatedPrincipalName = Constraint.isNotNull(principalName, "PrincipalName cannot be null");
authenticationInstant = Constraint.isNotNull(authnInstant, "AuthnInstant cannot be null");
authenticationMethod = Constraint.isNotNull(authnMethod, "AuthnMethod cannot be null");
@@ -74,7 +74,7 @@ public class TicketState {
*
* @return IdP session ID.
*/
- @Nonnull public String getSessionId() {
+ @Nullable public String getSessionId() {
return sessId;
}
@@ -135,7 +135,7 @@ public class TicketState {
public boolean equals(final Object o) {
if (o instanceof TicketState) {
final TicketState other = (TicketState) o;
- return sessId.equals(other.sessId) &&
+ return Objects.equals(sessId, other.sessId) &&
authenticatedPrincipalName.equals(other.authenticatedPrincipalName) &&
authenticationInstant.equals(other.authenticationInstant) &&
authenticationMethod.equals(other.authenticationMethod);
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/GrantProxyTicketAction.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/GrantProxyTicketAction.java
index c679ba81c..cc8c9ea9f 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/GrantProxyTicketAction.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/GrantProxyTicketAction.java
@@ -167,30 +167,40 @@ public class GrantProxyTicketAction extends AbstractCASProtocolAction<ProxyTicke
}
if (validateIdPSessionPredicate.test(profileRequestContext)) {
- IdPSession session = null;
- try {
- log.debug("{} Attempting to retrieve session {}", getLogPrefix(), proxyGrantingTicket.getSessionId());
- session = sessionResolver.resolveSingle(new CriteriaSet(new SessionIdCriterion(
- Constraint.isNotNull(proxyGrantingTicket.getSessionId(),
- "ProxyGrantingTicket session id was null"))));
- } catch (final ResolverException e) {
- log.warn("{} IdPSession resolution error: {}", getLogPrefix(), e);
- }
- boolean expired = true;
- if (session == null) {
- log.info("{} IdPSession {} not found", getLogPrefix(), proxyGrantingTicket.getSessionId());
- } else {
+ final String sessionId = proxyGrantingTicket.getSessionId();
+ if (sessionId != null) {
+ IdPSession session;
try {
- expired = !session.checkTimeout();
- log.debug("{} Session {} expired={}", getLogPrefix(), proxyGrantingTicket.getSessionId(), expired);
- } catch (final SessionException e) {
- log.warn("{} Error performing session timeout check: {}. Assuming session has expired.",
- getLogPrefix(), e);
+ log.debug("{} Attempting to retrieve session {}", getLogPrefix(), sessionId);
+ session = sessionResolver.resolveSingle(new CriteriaSet(new SessionIdCriterion(sessionId)));
+ } catch (final ResolverException e) {
+ log.info("{} Failed resolving IdP session {}: {}", getLogPrefix(), sessionId, e.getMessage());
+ ActionSupport.buildEvent(profileRequestContext, ProtocolError.SessionExpired.event(this));
+ return;
}
- }
- if (expired) {
- ActionSupport.buildEvent(profileRequestContext, ProtocolError.SessionExpired.event(this));
- return;
+ if (session == null) {
+ log.info("{} IdPSession {} not found", getLogPrefix(), sessionId);
+ ActionSupport.buildEvent(profileRequestContext, ProtocolError.SessionExpired.event(this));
+ return;
+ } else {
+ boolean expired = true;
+ try {
+ expired = !session.checkTimeout();
+ log.debug("{} IdPSession {} expired={}", getLogPrefix(), sessionId, expired);
+ } catch (final SessionException e) {
+ log.warn("{} Error performing session timeout check: {}. Assuming session has expired.",
+ getLogPrefix(), e);
+ }
+ if (expired) {
+ ActionSupport.buildEvent(profileRequestContext, ProtocolError.SessionExpired.event(this));
+ return;
+ }
+ }
+ } else {
+ log.warn("{} Cannot validate session because the PGT is not bound to a session. " +
+ "This is likely a sign of a configuration problem. The validateIdPSessionPredicate is " +
+ "configured to return true, but the session storage mechanism is configured such that " +
+ "IdP sessions are not available to CAS tickets.", getLogPrefix());
}
}
final ProxyTicket pt;
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/GrantServiceTicketAction.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/GrantServiceTicketAction.java
index 7be32cb1a..a309b390f 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/GrantServiceTicketAction.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/GrantServiceTicketAction.java
@@ -87,10 +87,7 @@ public class GrantServiceTicketAction extends AbstractCASProtocolAction<ServiceT
/** Security config. */
@NonnullBeforeExec private SecurityConfiguration securityConfig;
-
- /** IdP's session. */
- @NonnullBeforeExec private IdPSession session;
-
+
/** Authentication result. */
@NonnullBeforeExec private AuthenticationResult authnResult;
@@ -167,21 +164,11 @@ public class GrantServiceTicketAction extends AbstractCASProtocolAction<ServiceT
return false;
}
- session = getIdPSession(profileRequestContext);
- if (session == null) {
- // TODO: I think this should be revisited, unclear why the TicketState later needs this.
- // It may be needed specifically if the check below for an active AuthnResult fails, but that's
- // a secondary requirement that would only happen when absolutely needed.
- log.warn("{} No IdP session found", getLogPrefix());
- ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
- return false;
- }
-
final AuthenticationContext authnCtx = authnCtxLookupFunction.apply(profileRequestContext);
if (authnCtx != null) {
authnResult = authnCtx.getAuthenticationResult();
} else {
- authnResult = getLatestAuthenticationResult();
+ authnResult = getLatestAuthenticationResult(profileRequestContext);
}
if (authnResult == null) {
@@ -213,8 +200,9 @@ public class GrantServiceTicketAction extends AbstractCASProtocolAction<ServiceT
try {
log.debug("{} Granting service ticket for {}", getLogPrefix(), request.getService());
+ final IdPSession session = getIdPSession(profileRequestContext);
final TicketState state = new TicketState(
- Constraint.isNotNull(session.getId(), "Session ID was non null"),
+ session != null ? session.getId() : null,
getPrincipalName(profileRequestContext),
authnResult.getAuthenticationInstant(),
authnResult.getAuthenticationFlowId());
@@ -244,6 +232,7 @@ public class GrantServiceTicketAction extends AbstractCASProtocolAction<ServiceT
}
try {
+ setCASTicket(profileRequestContext, ticket);
setCASResponse(profileRequestContext, response);
} catch (final EventException e) {
ActionSupport.buildEvent(profileRequestContext, e.getEventID());
@@ -257,7 +246,7 @@ public class GrantServiceTicketAction extends AbstractCASProtocolAction<ServiceT
* Get the IdP session.
*
* @param prc profile request context
- *
+ *
* @return IdP session
*/
@Nullable private IdPSession getIdPSession(@Nonnull final ProfileRequestContext prc) {
@@ -280,21 +269,23 @@ public class GrantServiceTicketAction extends AbstractCASProtocolAction<ServiceT
}
/**
- * Gets the most recent authentication result from the IdP session.
+ * Gets the most recent authentication result from the current IdP session.
*
+ * @param prc Profile request context.
* @return Latest authentication result.
*
* @throws IllegalStateException If no authentication results are found.
*/
- @Nullable private AuthenticationResult getLatestAuthenticationResult() {
+ @Nullable private AuthenticationResult getLatestAuthenticationResult(final ProfileRequestContext prc) {
AuthenticationResult latest = null;
-
- for (final AuthenticationResult result : session.getAuthenticationResults()) {
- if (latest == null || result.getAuthenticationInstant().isAfter(latest.getAuthenticationInstant())) {
- latest = result;
+ final IdPSession session = getIdPSession(prc);
+ if (session != null) {
+ for (final AuthenticationResult result : session.getAuthenticationResults()) {
+ if (latest == null || result.getAuthenticationInstant().isAfter(latest.getAuthenticationInstant())) {
+ latest = result;
+ }
}
}
-
return latest;
}
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/UpdateIdPSessionWithSPSessionAction.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/UpdateIdPSessionWithSPSessionAction.java
index 02ae27fc6..d53fed3b1 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/UpdateIdPSessionWithSPSessionAction.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/UpdateIdPSessionWithSPSessionAction.java
@@ -109,12 +109,15 @@ public class UpdateIdPSessionWithSPSessionAction<RequestType,ResponseType>
if (!service.isSingleLogoutParticipant()) {
return false;
}
-
ticket = getCASTicket(profileRequestContext);
} catch (final EventException e) {
ActionSupport.buildEvent(profileRequestContext, e.getEventID());
return false;
}
+ if (ticket.getSessionId() == null) {
+ log.debug("{} Cannot update IdP session because the ticket is not bound to a session", getLogPrefix());
+ return false;
+ }
return true;
}
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/ValidateTicketAction.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/ValidateTicketAction.java
index 9d9bec53e..183cfbb0b 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/ValidateTicketAction.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/ValidateTicketAction.java
@@ -111,7 +111,7 @@ public class ValidateTicketAction extends AbstractCASProtocolAction<TicketValida
final String ticketId = request.getTicket();
log.debug("Attempting to validate {}", ticketId);
if (ticketId.startsWith(LoginConfiguration.DEFAULT_TICKET_PREFIX)) {
- ticket = casTicketService.removeServiceTicket(request.getTicket());
+ ticket = casTicketService.removeServiceTicket(ticketId);
} else if (ticketId.startsWith(ProxyConfiguration.DEFAULT_TICKET_PREFIX)) {
ticket = casTicketService.removeProxyTicket(ticketId);
} else {
@@ -119,8 +119,7 @@ public class ValidateTicketAction extends AbstractCASProtocolAction<TicketValida
return;
}
if (ticket != null) {
- log.debug("{} Found and removed {}/{} from ticket store", getLogPrefix(), ticket,
- ticket.getSessionId());
+ log.debug("{} Found and removed {} from ticket store", getLogPrefix(), ticketId);
}
} catch (final RuntimeException e) {
log.debug("{} CAS ticket retrieval failed with error: {}", getLogPrefix(), e);
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/impl/AbstractTicketService.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/impl/AbstractTicketService.java
index dd7caa105..271c130d2 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/impl/AbstractTicketService.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/impl/AbstractTicketService.java
@@ -40,6 +40,7 @@ import net.shibboleth.idp.cas.ticket.serialization.impl.ProxyTicketSerializer;
import net.shibboleth.idp.cas.ticket.serialization.impl.ServiceTicketSerializer;
import net.shibboleth.shared.logic.Constraint;
import net.shibboleth.shared.primitive.LoggerFactory;
+
/**
* Abstract base class for ticket services that rely on {@link StorageService} for ticket storage.
*
@@ -170,17 +171,22 @@ public abstract class AbstractTicketService implements TicketService {
* @param <T> Type of ticket.
*/
protected <T extends Ticket> void store(@Nonnull final T ticket) {
- final String context = Constraint.isNotNull(context(ticket.getClass()),
- "Could not find context for ticket of type " + ticket.getClass());
try {
- final String sessionId = Constraint.isNotNull(ticket.getSessionId(), "No session Id");
+ final String sessionId = ticket.getSessionId();
final long expiry = ticket.getExpirationInstant().toEpochMilli();
- log.debug("Storing mapping of {} to {} in context {}", ticket, sessionId, context);
- if (!storageService.create(context, ticket.getId(), sessionId, expiry)) {
- throw new RuntimeException("Failed to store ticket " + ticket);
+ final String ticketCtx;
+ if (sessionId != null) {
+ final String context = context(ticket.getClass());
+ log.debug("Storing mapping of {} to {} in context {}", ticket, sessionId, context);
+ if (!storageService.create(context, ticket.getId(), sessionId, expiry)) {
+ throw new RuntimeException("Failed to store ticket " + ticket);
+ }
+ ticketCtx = sessionId;
+ } else {
+ ticketCtx = ticket.getId();
}
- log.debug("Storing {} in context {}", ticket, sessionId);
- if (!storageService.create(sessionId, ticket.getId(), ticket,
+ log.debug("Storing {} in context {}", ticket, ticketCtx);
+ if (!storageService.create(ticketCtx, ticket.getId(), ticket,
(StorageSerializer<T>) serializer(ticket.getClass()), expiry)) {
throw new RuntimeException("Failed to store ticket " + ticket);
}
@@ -202,20 +208,21 @@ public abstract class AbstractTicketService implements TicketService {
log.debug("Reading {}", id);
final T ticket;
try {
- final String context = Constraint.isNotNull(context(clazz),
- "Could not find context for ticket of type " + clazz);
- final StorageRecord<T> sessionRecord = storageService.read(context, id);
- if (sessionRecord == null) {
- log.debug("{} not found in context {}", id, context);
- return null;
+ final String context;
+ final StorageRecord<T> sessionRecord = storageService.read(context(clazz), id);
+ if (sessionRecord != null) {
+ context = sessionRecord.getValue();
+ log.debug("{} bound to session {}", id, context);
+ } else {
+ log.debug("{} not bound to any session. Using ticket ID for context.", id);
+ context = id;
}
- final String sessionId = sessionRecord.getValue();
- final StorageRecord<T> ticketRecord = storageService.read(sessionId, id);
+ final StorageRecord<T> ticketRecord = storageService.read(context, id);
if (ticketRecord == null) {
- log.debug("{} not found in context {}", id, sessionId);
+ log.debug("{} not found in context {}", id, context);
return null;
}
- ticket = ticketRecord.getValue(serializer(clazz), sessionId, id);
+ ticket = ticketRecord.getValue(serializer(clazz), context, id);
} catch (final IOException e) {
throw new RuntimeException("Error reading ticket.");
}
@@ -237,16 +244,21 @@ public abstract class AbstractTicketService implements TicketService {
return null;
}
try {
- final String context = Constraint.isNotNull(context(clazz),
- "Could not find context for ticket of type " + clazz);
- log.debug("Attempting to delete {} from context {}", id, context);
- if (!storageService.delete(context, id)) {
- log.info("Failed deleting {} from context {}.", id, context);
+ final String context = context(clazz);
+ final String sessionId = ticket.getSessionId();
+ final String ticketCtx;
+ if (sessionId != null) {
+ log.debug("Attempting to delete {} from context {}", id, context);
+ if (!storageService.delete(context, id)) {
+ log.info("Failed deleting {} from context {}.", id, context);
+ }
+ ticketCtx = sessionId;
+ } else {
+ ticketCtx = id;
}
- final String sessionId = Constraint.isNotNull(ticket.getSessionId(), "No session Id");
- log.debug("Attempting to delete {} from context {}", id, sessionId);
- if (!storageService.delete(sessionId, id)) {
- log.info("Failed deleting {} from context {}.", id, sessionId);
+ log.debug("Attempting to delete {} from context {}", id, ticketCtx);
+ if (!storageService.delete(ticketCtx, id)) {
+ log.info("Failed deleting {} from context {}.", id, ticketCtx);
}
} catch (final IOException e) {
throw new RuntimeException("Error deleting ticket " + id, e);
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/serialization/impl/AbstractTicketSerializer.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/serialization/impl/AbstractTicketSerializer.java
index 5e118cc43..d64648297 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/serialization/impl/AbstractTicketSerializer.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/ticket/serialization/impl/AbstractTicketSerializer.java
@@ -105,17 +105,22 @@ public abstract class AbstractTicketSerializer<T extends Ticket> implements Stor
final StringWriter buffer = new StringWriter(200);
try (final JsonGenerator gen = generatorFactory.createGenerator(buffer)) {
gen.writeStartObject()
- .write(SERVICE_FIELD, ticket.getService())
- .write(EXPIRATION_FIELD, ticket.getExpirationInstant().toEpochMilli());
- final TicketState state = ticket.getTicketState();
- if (state != null) {
- gen.writeStartObject(STATE_FIELD)
- .write(SESSION_FIELD, state.getSessionId())
- .write(PRINCIPAL_FIELD, state.getPrincipalName())
- .write(AUTHN_INSTANT_FIELD, state.getAuthenticationInstant().toEpochMilli())
- .write(AUTHN_METHOD_FIELD, state.getAuthenticationMethod());
-
- final Set<String> consentedIds = state.getConsentedAttributeIds();
+ .write(SERVICE_FIELD, ticket.getService())
+ .write(EXPIRATION_FIELD, ticket.getExpirationInstant().toEpochMilli());
+
+ if (ticket.getTicketState() != null) {
+ final TicketState state = ticket.getTicketState();
+ gen.writeStartObject(STATE_FIELD);
+ if (state.getSessionId() != null) {
+ gen.write(SESSION_FIELD, state.getSessionId());
+ } else {
+ gen.writeNull(SESSION_FIELD);
+ }
+ gen.write(PRINCIPAL_FIELD, state.getPrincipalName())
+ .write(AUTHN_INSTANT_FIELD, state.getAuthenticationInstant().toEpochMilli())
+ .write(AUTHN_METHOD_FIELD, state.getAuthenticationMethod());
+
+ final Set<String> consentedIds = state.getConsentedAttributeIds();
if (consentedIds != null) {
gen.writeStartArray(CONSENTED_ATTRS_FIELD);
for (final String id : consentedIds) {
@@ -154,8 +159,13 @@ public abstract class AbstractTicketSerializer<T extends Ticket> implements Stor
final JsonObject so = to.getJsonObject(STATE_FIELD);
final TicketState state;
if (so != null) {
- final String sessionField =
- Constraint.isNotNull(so.getString(SESSION_FIELD), "Session field was not present");
+ final JsonValue sessionField = so.get(SESSION_FIELD);
+ final String sessionId;
+ if (!JsonValue.NULL.equals(sessionField)) {
+ sessionId = ((JsonString) sessionField).getString();
+ } else {
+ sessionId = null;
+ }
final String principalField =
Constraint.isNotNull(so.getString(PRINCIPAL_FIELD), "Principal field was not present");
final JsonNumber authnInstantField =
@@ -166,7 +176,7 @@ public abstract class AbstractTicketSerializer<T extends Ticket> implements Stor
final String authnMethodField =
Constraint.isNotNull(so.getString(AUTHN_METHOD_FIELD), "Authn Method field was not present");
state = new TicketState(
- sessionField,
+ sessionId,
principalField,
authnInstant,
authnMethodField);
diff --git a/idp-cas-impl/src/test/java/net/shibboleth/idp/cas/flow/impl/GrantServiceTicketActionTest.java b/idp-cas-impl/src/test/java/net/shibboleth/idp/cas/flow/impl/GrantServiceTicketActionTest.java
index 80af4ba42..ecfc7ade1 100644
--- a/idp-cas-impl/src/test/java/net/shibboleth/idp/cas/flow/impl/GrantServiceTicketActionTest.java
+++ b/idp-cas-impl/src/test/java/net/shibboleth/idp/cas/flow/impl/GrantServiceTicketActionTest.java
@@ -22,6 +22,7 @@ import net.shibboleth.idp.cas.protocol.ServiceTicketResponse;
import net.shibboleth.idp.cas.ticket.ServiceTicket;
import net.shibboleth.idp.cas.ticket.TicketState;
+import net.shibboleth.idp.session.IdPSession;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.webflow.execution.RequestContext;
import org.testng.annotations.DataProvider;
@@ -53,10 +54,12 @@ public class GrantServiceTicketActionTest extends AbstractFlowActionTest {
@Test(dataProvider = "messages")
public void testExecute(final ServiceTicketRequest request) throws Exception {
+ final IdPSession session = mockSession("1234567890", true);
+ final AuthenticationResult result = new AuthenticationResult("Password", new UsernamePrincipal("bob"));
final RequestContext context = new TestContextBuilder(LoginConfiguration.PROFILE_ID)
.addProtocolContext(request, null)
- .addAuthenticationContext(new AuthenticationResult("Password", new UsernamePrincipal("bob")))
- .addSessionContext(mockSession("1234567890", true))
+ .addAuthenticationContext(result)
+ .addSessionContext(session)
.addSubjectContext(TEST_PRINCIPAL_NAME)
.addRelyingPartyContext(request.getService(), true, new LoginConfiguration())
.build();
diff --git a/idp-cas-impl/src/test/java/net/shibboleth/idp/cas/ticket/impl/SimpleTicketServiceTest.java b/idp-cas-impl/src/test/java/net/shibboleth/idp/cas/ticket/impl/SimpleTicketServiceTest.java
index 7db701d7b..91f418e1a 100644
--- a/idp-cas-impl/src/test/java/net/shibboleth/idp/cas/ticket/impl/SimpleTicketServiceTest.java
+++ b/idp-cas-impl/src/test/java/net/shibboleth/idp/cas/ticket/impl/SimpleTicketServiceTest.java
@@ -57,12 +57,10 @@ public class SimpleTicketServiceTest {
@Test
public void testCreateRemoveServiceTicket() throws Exception {
- final ServiceTicket st = createServiceTicket();
- assert st != null;
- final TicketState ts = st.getTicketState();
- assert ts != null;
- assertNotNull(ts.getSessionId());
- assertNotNull(ts.getPrincipalName());
+ final ServiceTicket st = createServiceTicket(TEST_SESSION_ID);
+ assertNotNull(st);
+ assertNotNull(st.getTicketState().getSessionId());
+ assertNotNull(st.getTicketState().getPrincipalName());
final ServiceTicket st2 = ticketService.removeServiceTicket(st.getId());
assert st2 != null;
assertEquals(st, st2);
@@ -72,6 +70,20 @@ public class SimpleTicketServiceTest {
assertNull(ticketService.removeServiceTicket(st.getId()));
}
+ @Test
+ public void testCreateRemoveServiceTicketNoSession() throws Exception {
+ final ServiceTicket st = createServiceTicket(null);
+ assertNotNull(st);
+ assertNull(st.getTicketState().getSessionId());
+ assertNotNull(st.getTicketState().getPrincipalName());
+ final ServiceTicket st2 = ticketService.removeServiceTicket(st.getId());
+ assertEquals(st, st2);
+ assertEquals(st.getExpirationInstant(), st2.getExpirationInstant());
+ assertEquals(st.getService(), st2.getService());
+ assertEquals(st.getTicketState(), st2.getTicketState());
+ assertNull(ticketService.removeServiceTicket(st.getId()));
+ }
+
@Test
public void testCreateFetchRemoveProxyGrantingTicket() throws Exception {
final ProxyGrantingTicket pgt = createProxyGrantingTicket();
@@ -112,12 +124,12 @@ public class SimpleTicketServiceTest {
assertNull(ticketService.removeProxyTicket(pt.getId()));
}
- @Nonnull private ServiceTicket createServiceTicket() {
+ @Nonnull private ServiceTicket createServiceTicket(final String sessionId) {
return ticketService.createServiceTicket(
new TicketIdentifierGenerationStrategy("ST", 25).generateIdentifier(),
expiry(),
TEST_SERVICE,
- new TicketState(TEST_SESSION_ID, "bob", expiry(), "Password"),
+ new TicketState(sessionId, "bob", expiry(), "Password"),
false);
}
@@ -125,7 +137,7 @@ public class SimpleTicketServiceTest {
return ticketService.createProxyGrantingTicket(
new TicketIdentifierGenerationStrategy("PGT", 50).generateIdentifier(),
expiry(),
- createServiceTicket(),
+ createServiceTicket(TEST_SESSION_ID),
TEST_PGTURL);
}
diff --git a/idp-cas-impl/src/test/java/net/shibboleth/idp/cas/ticket/serialization/impl/ServiceTicketSerializerTest.java b/idp-cas-impl/src/test/java/net/shibboleth/idp/cas/ticket/serialization/impl/ServiceTicketSerializerTest.java
index 7a9bde15d..ad0ae3842 100644
--- a/idp-cas-impl/src/test/java/net/shibboleth/idp/cas/ticket/serialization/impl/ServiceTicketSerializerTest.java
+++ b/idp-cas-impl/src/test/java/net/shibboleth/idp/cas/ticket/serialization/impl/ServiceTicketSerializerTest.java
@@ -69,6 +69,24 @@ public class ServiceTicketSerializerTest {
assertEquals(st2.getTicketState(), st1.getTicketState());
}
+ @Test
+ public void testSerializeWithTicketStateNullSessionId() throws Exception {
+ final ServiceTicket st1 = new ServiceTicket(
+ "ST-0123456789-e6342d467a4414e599aa3c323528e96f",
+ "https://nobody.example.org",
+ Instant.now().truncatedTo(ChronoUnit.MILLIS),
+ true);
+ st1.setTicketState(new TicketState(null, "bob", Instant.now().truncatedTo(ChronoUnit.MILLIS), "Password"));
+ final String serialized = serializer.serialize(st1);
+ final ServiceTicket st2 = serializer.deserialize(1, "notused", st1.getId(), serialized, null);
+ assertNull(st2.getSessionId());
+ assertEquals(st2.getId(), st1.getId());
+ assertEquals(st2.getService(), st1.getService());
+ assertEquals(st2.getExpirationInstant(), st1.getExpirationInstant());
+ assertEquals(st2.isRenew(), st1.isRenew());
+ assertEquals(st2.getTicketState(), st1.getTicketState());
+ }
+
@Test
public void testSerializeWithConsent() throws Exception {
final ServiceTicket st1 = new ServiceTicket(
diff --git a/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/cas/login-beans.xml b/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/cas/login-beans.xml
index 1f13f0ded..7430fe7e5 100644
--- a/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/cas/login-beans.xml
+++ b/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/cas/login-beans.xml
@@ -40,6 +40,11 @@
class="net.shibboleth.idp.cas.flow.impl.GrantServiceTicketAction" scope="prototype"
c:ticketService="#{getObject('shibboleth.CASTicketService') ?: getObject('shibboleth.DefaultCASTicketService')}" />
+ <bean id="UpdateIdPSessionWithSPSession"
+ class="net.shibboleth.idp.cas.flow.impl.UpdateIdPSessionWithSPSessionAction" scope="prototype"
+ c:lifetime="%{idp.session.defaultSPlifetime:PT2H}"
+ c:resolver-ref="shibboleth.SessionManager" />
+
<bean id="LoginConfigLookup"
class="net.shibboleth.idp.cas.config.ConfigLookupFunction" scope="prototype"
c:clazz="net.shibboleth.idp.cas.config.LoginConfiguration" />
diff --git a/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/cas/login-flow.xml b/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/cas/login-flow.xml
index 84fcd9b56..acf4b4d16 100644
--- a/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/cas/login-flow.xml
+++ b/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/cas/login-flow.xml
@@ -98,6 +98,7 @@
<action-state id="GrantServiceTicket">
<evaluate expression="GrantServiceTicket" />
+ <evaluate expression="UpdateIdPSessionWithSPSession" />
<evaluate expression="PublishProtocolResponse" />
<evaluate expression="PopulateOutboundInterceptContext" />
<evaluate expression="PopulateClientStorageSaveContext" />
diff --git a/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/cas/validate-abstract-beans.xml b/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/cas/validate-abstract-beans.xml
index 1cd74b21f..88b6b285f 100644
--- a/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/cas/validate-abstract-beans.xml
+++ b/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/cas/validate-abstract-beans.xml
@@ -50,11 +50,6 @@
class="net.shibboleth.idp.cas.flow.impl.PrepareTicketValidationResponseAction" scope="prototype"
p:transcoderRegistry-ref="shibboleth.AttributeRegistryService" />
- <bean id="UpdateIdPSessionWithSPSession"
- class="net.shibboleth.idp.cas.flow.impl.UpdateIdPSessionWithSPSessionAction" scope="prototype"
- c:lifetime="%{idp.session.defaultSPlifetime:PT2H}"
- c:resolver-ref="shibboleth.SessionManager" />
-
<bean id="PopulateAuditContext" parent="shibboleth.AbstractPopulateAuditContext"
p:fieldExtractors="#{getObject('shibboleth.CASValidationAuditExtractors') ?: getObject('shibboleth.DefaultCASValidationAuditExtractors')}" />
diff --git a/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/cas/validate-abstract-flow.xml b/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/cas/validate-abstract-flow.xml
index 4abf2339c..3902362f3 100644
--- a/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/cas/validate-abstract-flow.xml
+++ b/idp-conf-impl/src/main/resources/net/shibboleth/idp/flows/cas/validate-abstract-flow.xml
@@ -66,7 +66,7 @@
<decision-state id="CheckResolveAttributes">
<if test="ValidateConfigLookup.apply(opensamlProfileRequestContext).isResolveAttributes(opensamlProfileRequestContext)"
then="ResolveAttributes"
- else="UpdateIdPSessionWithSPSession" />
+ else="PopulateOutboundInterceptContext" />
</decision-state>
<action-state id="ResolveAttributes">
@@ -79,7 +79,7 @@
<decision-state id="CheckConsentCondition">
<if test="flowRequestContext.getActiveFlow().getApplicationContext().containsBean('shibboleth.consent.AttributeQuery.Condition') and flowRequestContext.getActiveFlow().getApplicationContext().getBean('shibboleth.consent.AttributeQuery.Condition').apply(opensamlProfileRequestContext)"
- then="ConsentSetup" else="UpdateIdPSessionWithSPSession" />
+ then="ConsentSetup" else="PopulateOutboundInterceptContext" />
</decision-state>
<action-state id="ConsentSetup">
@@ -90,11 +90,10 @@
<subflow-state id="ConsentFlow" subflow="intercept/attribute-release-query">
<input name="calledAsSubflow" value="true" />
- <transition on="proceed" to="UpdateIdPSessionWithSPSession"/>
+ <transition on="proceed" to="PopulateOutboundInterceptContext"/>
</subflow-state>
- <action-state id="UpdateIdPSessionWithSPSession">
- <evaluate expression="UpdateIdPSessionWithSPSession" />
+ <action-state id="PopulateOutboundInterceptContext">
<evaluate expression="PopulateOutboundInterceptContext" />
<evaluate expression="'proceed'" />
<transition on="proceed" to="CheckOutboundInterceptContext" />
diff --git a/idp-conf-impl/src/test/java/net/shibboleth/idp/test/flows/cas/LoginFlowTest.java b/idp-conf-impl/src/test/java/net/shibboleth/idp/test/flows/cas/LoginFlowTest.java
index 945861429..53e3ecac4 100644
--- a/idp-conf-impl/src/test/java/net/shibboleth/idp/test/flows/cas/LoginFlowTest.java
+++ b/idp-conf-impl/src/test/java/net/shibboleth/idp/test/flows/cas/LoginFlowTest.java
@@ -142,7 +142,9 @@ public class LoginFlowTest extends AbstractFlowTest {
@Test
public void testLoginStartSession() throws Exception {
- final String service = "https://start.example.org/";
+ // The service below is registered for single logout,
+ // which triggers attaching an SPSession to the IdPSession
+ final String service = "https://slo.example.org/";
externalContext.getMockRequestParameterMap().put("service", service);
overrideEndStateOutput(FLOW_ID, "RedirectToService");
@@ -158,6 +160,8 @@ public class LoginFlowTest extends AbstractFlowTest {
final IdPSession session = sessionManager.resolveSingle(
new CriteriaSet(new SessionIdCriterion(sid)));
assert session!=null;
+ assertEquals(session.getSPSessions().size(), 1);
+ assertEquals(session.getSPSessions().iterator().next().getId(), service);
final ProfileRequestContext prc = (ProfileRequestContext) outcome.getOutput().get(END_STATE_OUTPUT_ATTR_NAME);
assertNotNull(prc.getSubcontext(SubjectContext.class));
diff --git a/idp-conf-impl/src/test/java/net/shibboleth/idp/test/flows/cas/ServiceValidateFlowTest.java b/idp-conf-impl/src/test/java/net/shibboleth/idp/test/flows/cas/ServiceValidateFlowTest.java
index 2b101bc68..d3bf84520 100644
--- a/idp-conf-impl/src/test/java/net/shibboleth/idp/test/flows/cas/ServiceValidateFlowTest.java
+++ b/idp-conf-impl/src/test/java/net/shibboleth/idp/test/flows/cas/ServiceValidateFlowTest.java
@@ -179,40 +179,6 @@ public class ServiceValidateFlowTest extends AbstractFlowTest {
assertEquals(updatedSession.getSPSessions().size(), 0);
}
- @Test
- public void testSuccessWithSLOParticipant() throws Exception {
- final String principal = "john";
- final IdPSession session = sessionManager.createSession(principal);
- final String sid = session.getId();
- assert sid!=null;
- final ServiceTicket ticket = ticketService.createServiceTicket(
- "ST-1415133132-ompog68ygxKyX9BPwPuw0hESQBjuA",
- Instant.now().plusSeconds(5),
- "https://slo.example.org/",
- new TicketState(sid, principal, Instant.now(), "Password"),
- false);
-
- externalContext.getMockRequestParameterMap().put("service", ticket.getService());
- externalContext.getMockRequestParameterMap().put("ticket", ticket.getId());
- overrideEndStateOutput(FLOW_ID, "ValidateSuccess");
-
- final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
-
- final String responseBody = response.getContentAsString();
- final FlowExecutionOutcome outcome = result.getOutcome();
- assertEquals(outcome.getId(), "ValidateSuccess");
- assertTrue(responseBody.contains("<cas:authenticationSuccess>"));
- assertTrue(responseBody.contains("<cas:user>john</cas:user>"));
- assertFalse(responseBody.contains("<cas:proxyGrantingTicket>"));
- assertFalse(responseBody.contains("<cas:proxies>"));
- assertPopulatedAttributeContext((ProfileRequestContext) outcome.getOutput().get(END_STATE_OUTPUT_ATTR_NAME));
-
- final IdPSession updatedSession = sessionResolver.resolveSingle(
- new CriteriaSet(new SessionIdCriterion(sid)));
- assert updatedSession!=null;
- assertEquals(updatedSession.getSPSessions().size(), 1);
- }
-
@Test
public void testFailureTicketExpired() throws Exception {
externalContext.getMockRequestParameterMap().put("service", "https://test.example.org/");
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list