[java-identity-provider] 01/01: Remove hard requirement for IdP session in CAS protocol.

Marvin S. Addison marvin.addison at gmail.com
Thu Oct 27 14:44:05 UTC 2022


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

serac pushed a commit to branch dev/cas-no-session
in repository java-identity-provider.

View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=13c2773e6067f7a1d0cef23b70dcf0096405fea3

commit 13c2773e6067f7a1d0cef23b70dcf0096405fea3
Author: Marvin S. Addison <serac at vt.edu>
AuthorDate: Thu Oct 27 09:37:26 2022 -0400

    Remove hard requirement for IdP session in CAS protocol.
---
 .../net/shibboleth/idp/cas/ticket/TicketState.java | 10 ++--
 .../idp/cas/flow/impl/GrantProxyTicketAction.java  | 42 +++++++++-------
 .../cas/flow/impl/GrantServiceTicketAction.java    | 30 ++++-------
 .../impl/UpdateIdPSessionWithSPSessionAction.java  |  5 +-
 .../idp/cas/flow/impl/ValidateTicketAction.java    |  5 +-
 .../idp/cas/ticket/impl/AbstractTicketService.java | 58 ++++++++++++++--------
 .../impl/AbstractTicketSerializer.java             | 28 ++++++++---
 .../flow/impl/GrantServiceTicketActionTest.java    |  7 ++-
 .../cas/ticket/impl/SimpleTicketServiceTest.java   | 22 ++++++--
 .../impl/ServiceTicketSerializerTest.java          | 18 +++++++
 10 files changed, 140 insertions(+), 85 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 dd0a78173..f932bcb3f 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
@@ -39,7 +39,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;
@@ -62,11 +62,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");
@@ -77,7 +77,7 @@ public class TicketState {
      *
      * @return IdP session ID.
      */
-    @Nonnull public String getSessionId() {
+    @Nullable public String getSessionId() {
         return sessId;
     }
 
@@ -138,7 +138,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 80d170796..c5dd4199f 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
@@ -157,28 +157,32 @@ public class GrantProxyTicketAction extends AbstractCASProtocolAction<ProxyTicke
         }
         
         if (validateIdPSessionPredicate.test(profileRequestContext)) {
-            IdPSession session = null;
-            try {
-                log.debug("{} Attempting to retrieve session {}", getLogPrefix(), pgt.getSessionId());
-                session = sessionResolver.resolveSingle(new CriteriaSet(new SessionIdCriterion(pgt.getSessionId())));
-            } catch (final ResolverException e) {
-                log.warn("{} IdPSession resolution error: {}", getLogPrefix(), e);
-            }
-            boolean expired = true;
-            if (session == null) {
-                log.info("{} IdPSession {} not found", getLogPrefix(), pgt.getSessionId());
-            } else {
+            if (pgt.getSessionId() != null) {
+                IdPSession session = null;
                 try {
-                    expired = !session.checkTimeout();
-                    log.debug("{} Session {} expired={}", getLogPrefix(), pgt.getSessionId(), expired);
-                } catch (final SessionException e) {
-                    log.warn("{} Error performing session timeout check: {}. Assuming session has expired.",
+                    log.debug("{} Attempting to retrieve session {}", getLogPrefix(), pgt.getSessionId());
+                    session = sessionResolver.resolveSingle(new CriteriaSet(new SessionIdCriterion(pgt.getSessionId())));
+                } catch (final ResolverException e) {
+                    log.warn("{} IdPSession resolution error: {}", getLogPrefix(), e);
+                }
+                boolean expired = true;
+                if (session == null) {
+                    log.info("{} IdPSession {} not found", getLogPrefix(), pgt.getSessionId());
+                } else {
+                    try {
+                        expired = !session.checkTimeout();
+                        log.debug("{} Session {} expired={}", getLogPrefix(), pgt.getSessionId(), 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;
+                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", 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 f606b53f2..5e7f6df0b 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
@@ -89,10 +89,7 @@ public class GrantServiceTicketAction extends AbstractCASProtocolAction<ServiceT
     
     /** Security config. */
     @Nullable private SecurityConfiguration securityConfig;
-    
-    /** IdP's session. */
-    @Nullable private IdPSession session;
-    
+
     /** Authentication result. */
     @Nullable private AuthenticationResult authnResult;
 
@@ -163,21 +160,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) {
@@ -206,8 +193,9 @@ public class GrantServiceTicketAction extends AbstractCASProtocolAction<ServiceT
         final ServiceTicket ticket;
         try {
             log.debug("{} Granting service ticket for {}", getLogPrefix(), request.getService());
+            final IdPSession session = getIdPSession(profileRequestContext);
             final TicketState state = new TicketState(
-                    session.getId(),
+                    session != null ? session.getId() : null,
                     getPrincipalName(profileRequestContext),
                     authnResult.getAuthenticationInstant(),
                     authnResult.getAuthenticationFlowId());
@@ -247,7 +235,7 @@ public class GrantServiceTicketAction extends AbstractCASProtocolAction<ServiceT
      * Get the IdP session.
      *
      * @param prc profile request context
-     * 
+     *
      * @return IdP session
      */
     @Nullable private IdPSession getIdPSession(final ProfileRequestContext prc) {
@@ -270,21 +258,21 @@ 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;
-        
+        final IdPSession session = getIdPSession(prc);
         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 ee0b131a2..ab9d81376 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
@@ -93,12 +93,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 26ed95bbf..3752401c8 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
@@ -112,7 +112,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 {
@@ -120,8 +120,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 1327bc230..d3afc45d0 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
@@ -173,16 +173,22 @@ public abstract class AbstractTicketService implements TicketService {
      * @param <T> Type of ticket.
      */
     protected <T extends Ticket> void store(final T ticket) {
-        final String context = context(ticket.getClass());
         try {
             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);
             }
@@ -204,19 +210,21 @@ public abstract class AbstractTicketService implements TicketService {
         log.debug("Reading {}", id);
         final T ticket;
         try {
-            final String context = context(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.");
         }
@@ -239,14 +247,20 @@ public abstract class AbstractTicketService implements TicketService {
         }
         try {
             final String context = context(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 sessionId = ticket.getSessionId();
-            log.debug("Attempting to delete {} from context {}", id, sessionId);
-            if (!storageService.delete(sessionId, id)) {
-                log.info("Failed deleting {} from context {}.", id, sessionId);
+            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;
+            }
+            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 d114a98c7..54354839e 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
@@ -37,6 +37,7 @@ import javax.json.JsonValue;
 import javax.json.stream.JsonGenerator;
 import javax.json.stream.JsonGeneratorFactory;
 
+import com.fasterxml.jackson.databind.JsonNode;
 import net.shibboleth.idp.cas.ticket.Ticket;
 import net.shibboleth.idp.cas.ticket.TicketState;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
@@ -106,15 +107,19 @@ 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());
+                .write(SERVICE_FIELD, ticket.getService())
+                .write(EXPIRATION_FIELD, ticket.getExpirationInstant().toEpochMilli());
             
             if (ticket.getTicketState() != null) {
-                gen.writeStartObject(STATE_FIELD)
-                        .write(SESSION_FIELD, ticket.getTicketState().getSessionId())
-                        .write(PRINCIPAL_FIELD, ticket.getTicketState().getPrincipalName())
-                        .write(AUTHN_INSTANT_FIELD, ticket.getTicketState().getAuthenticationInstant().toEpochMilli())
-                        .write(AUTHN_METHOD_FIELD, ticket.getTicketState().getAuthenticationMethod());
+                gen.writeStartObject(STATE_FIELD);
+                if (ticket.getTicketState().getSessionId() != null) {
+                    gen.write(SESSION_FIELD, ticket.getTicketState().getSessionId());
+                } else {
+                    gen.writeNull(SESSION_FIELD);
+                }
+                gen.write(PRINCIPAL_FIELD, ticket.getTicketState().getPrincipalName())
+                    .write(AUTHN_INSTANT_FIELD, ticket.getTicketState().getAuthenticationInstant().toEpochMilli())
+                    .write(AUTHN_METHOD_FIELD, ticket.getTicketState().getAuthenticationMethod());
                 
                 if (ticket.getTicketState().getConsentedAttributeIds() != null) {
                     gen.writeStartArray(CONSENTED_ATTRS_FIELD);
@@ -151,8 +156,15 @@ public abstract class AbstractTicketSerializer<T extends Ticket> implements Stor
             final JsonObject so = to.getJsonObject(STATE_FIELD);
             final TicketState state;
             if (so != null) {
+                final JsonValue session = so.get(SESSION_FIELD);
+                final String sessionId;
+                if (!JsonValue.NULL.equals(session)) {
+                    sessionId = ((JsonString) session).getString();
+                } else {
+                    sessionId = null;
+                }
                 state = new TicketState(
-                        so.getString(SESSION_FIELD),
+                        sessionId,
                         so.getString(PRINCIPAL_FIELD),
                         Instant.ofEpochMilli(so.getJsonNumber(AUTHN_INSTANT_FIELD).longValueExact()),
                         so.getString(AUTHN_METHOD_FIELD));
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 e5c5b2079..9475a987b 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
@@ -23,6 +23,7 @@ import net.shibboleth.idp.cas.config.LoginConfiguration;
 import net.shibboleth.idp.cas.protocol.ServiceTicketRequest;
 import net.shibboleth.idp.cas.protocol.ServiceTicketResponse;
 import net.shibboleth.idp.cas.ticket.ServiceTicket;
+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 47bc187ed..2ef63a1a9 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
@@ -55,7 +55,7 @@ public class SimpleTicketServiceTest {
 
     @Test
     public void testCreateRemoveServiceTicket() throws Exception {
-        final ServiceTicket st = createServiceTicket();
+        final ServiceTicket st = createServiceTicket(TEST_SESSION_ID);
         assertNotNull(st);
         assertNotNull(st.getTicketState().getSessionId());
         assertNotNull(st.getTicketState().getPrincipalName());
@@ -67,6 +67,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();
@@ -100,12 +114,12 @@ public class SimpleTicketServiceTest {
         assertNull(ticketService.removeProxyTicket(pt.getId()));
     }
 
-    private ServiceTicket createServiceTicket() {
+    private ServiceTicket createServiceTicket(final String sessionId) {
         return ticketService.createServiceTicket(
                 new TicketIdentifierGenerationStrategy("ST", 25).generateIdentifier(),
                 expiry(),
                 TEST_SERVICE,
-                new TicketState(TEST_SESSION_ID, "bob", Instant.now().truncatedTo(ChronoUnit.MILLIS), "Password"),
+                new TicketState(sessionId, "bob", Instant.now().truncatedTo(ChronoUnit.MILLIS), "Password"),
                 false);
     }
 
@@ -113,7 +127,7 @@ public class SimpleTicketServiceTest {
         return ticketService.createProxyGrantingTicket(
                 new TicketIdentifierGenerationStrategy("PGT", 50).generateIdentifier(),
                 expiry(),
-                createServiceTicket());
+                createServiceTicket(TEST_SESSION_ID));
     }
 
     private static Instant expiry() {
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 21a13590c..33821112e 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(

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


More information about the commits mailing list