[java-identity-provider] branch main updated: IDP-1841 - CAS does not fill rpUIContext on Logout

Scott Cantor cantor.2 at osu.edu
Tue Apr 25 18:18:11 UTC 2023


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

scantor 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=a32e4207c7043bdd590d4271083da6d11da9422e

The following commit(s) were added to refs/heads/main by this push:
     new a32e4207c IDP-1841 - CAS does not fill rpUIContext on Logout
a32e4207c is described below

commit a32e4207c7043bdd590d4271083da6d11da9422e
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Tue Apr 25 14:18:08 2023 -0400

    IDP-1841 - CAS does not fill rpUIContext on Logout
    
    https://shibboleth.atlassian.net/browse/IDP-1841
    
    Expose dedicated prop in CASSPSession for service URL.
    Populate session ID with relying party ID when possible.
    Update CAS logout view to use serviceURL prop for form target.
    Fix a couple of annotations.
---
 .../flow/impl/BuildSAMLMetadataContextAction.java  |  4 ++-
 .../cas/flow/impl/GrantServiceTicketAction.java    | 13 ++++++---
 .../impl/UpdateIdPSessionWithSPSessionAction.java  | 31 +++++++++++++++++++---
 .../idp/cas/session/impl/CASSPSession.java         | 21 ++++++++++++---
 .../cas/session/impl/CASSPSessionSerializer.java   | 18 +++++++++++--
 .../session/impl/CASSPSessionSerializerTest.java   |  4 ++-
 .../net/shibboleth/idp/views/cas/logoutService.vm  |  2 +-
 .../net/shibboleth/idp/session/BasicSPSession.java |  2 +-
 8 files changed, 78 insertions(+), 17 deletions(-)

diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/BuildSAMLMetadataContextAction.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/BuildSAMLMetadataContextAction.java
index f89483bee..9f705b57c 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/BuildSAMLMetadataContextAction.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/flow/impl/BuildSAMLMetadataContextAction.java
@@ -70,7 +70,9 @@ public class BuildSAMLMetadataContextAction<RequestType,ResponseType>
         relyingPartyIdFromMetadata = flag;
     }
     
-    /** null safe getter
+    /**
+     * Null safe getter.
+     * 
      * @return Returns the service.
      */
     @SuppressWarnings("null")
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 f7dbac3eb..f83c23d25 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
@@ -119,8 +119,9 @@ public class GrantServiceTicketAction extends AbstractCASProtocolAction<ServiceT
         authnCtxLookupFunction = new ChildContextLookup<>(AuthenticationContext.class);
         final Function<ProfileRequestContext, String> plf = new SubjectContextPrincipalLookupFunction().compose(
                 new ChildContextLookup<>(SubjectContext.class));
-        final Function<ProfileRequestContext,AttributeContext> aclf = new ChildContextLookup<>(AttributeContext.class).compose(
-                new ChildContextLookup<>(RelyingPartyContext.class));
+        final Function<ProfileRequestContext,AttributeContext> aclf =
+                new ChildContextLookup<>(AttributeContext.class).compose(
+                        new ChildContextLookup<>(RelyingPartyContext.class));
         assert plf != null && aclf != null;
         principalLookupFunction = plf;
         attributeContextLookupStrategy = aclf;
@@ -141,7 +142,9 @@ public class GrantServiceTicketAction extends AbstractCASProtocolAction<ServiceT
         attributeContextLookupStrategy =
                 Constraint.isNotNull(strategy, "AttributeContext lookup strategy cannot be null");
     }
-
+    
+// Checkstyle: CyclomaticComplexity OFF
+    /** {@inheritDoc} */
     @Override
     protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
         if (!super.doPreExecute(profileRequestContext)) {
@@ -202,8 +205,10 @@ public class GrantServiceTicketAction extends AbstractCASProtocolAction<ServiceT
         }
 
         return true;
-    }    
+    }
+// Checkstyle: CyclomaticComplexity ON
     
+    /** {@inheritDoc} */
     @Override
     protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
                 
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 9aabeb818..c17b5ea17 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
@@ -19,6 +19,7 @@ package net.shibboleth.idp.cas.flow.impl;
 
 import java.time.Duration;
 import java.time.Instant;
+import java.util.function.Function;
 
 import javax.annotation.Nonnull;
 
@@ -35,6 +36,7 @@ import net.shibboleth.idp.session.SPSession;
 import net.shibboleth.idp.session.SessionException;
 import net.shibboleth.idp.session.SessionResolver;
 import net.shibboleth.idp.session.criterion.SessionIdCriterion;
+import net.shibboleth.profile.context.navigate.RelyingPartyIdLookupFunction;
 import net.shibboleth.shared.annotation.constraint.NonnullBeforeExec;
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.LoggerFactory;
@@ -43,9 +45,10 @@ import net.shibboleth.shared.resolver.ResolverException;
 
 /**
  * Conditionally updates the {@link IdPSession} with a {@link CASSPSession} to support SLO.
- * If the service granted access to indicates participation in SLO via {@link Service#singleLogoutParticipant},
+ * 
+ * <p>If the service granted access to indicates participation in SLO via {@link Service#isSingleLogoutParticipant()},
  * then a {@link CASSPSession} is created to track the SP session in order that it may receive SLO messages upon
- * a request to the CAS <code>/logout</code> URI.
+ * a request to the CAS <code>/logout</code> URI.</p>
  * 
  * @param <RequestType> request
  * @param <ResponseType> response
@@ -64,6 +67,9 @@ public class UpdateIdPSessionWithSPSessionAction<RequestType,ResponseType>
     /** Lifetime of sessions to create. */
     @Nonnull private final Duration sessionLifetime;
 
+    /** Strategy for obtaining the relying party ID for sessions. */
+    @Nonnull private Function<ProfileRequestContext,String> relyingPartyIdLookupStrategy;
+    
     /** Ticket. */
     @NonnullBeforeExec private Ticket ticket;
     
@@ -80,6 +86,18 @@ public class UpdateIdPSessionWithSPSessionAction<RequestType,ResponseType>
             @Nonnull final Duration lifetime) {
         sessionResolver = Constraint.isNotNull(resolver, "Session resolver cannot be null");
         sessionLifetime = Constraint.isNotNull(lifetime, "Lifetime cannot be null");
+        relyingPartyIdLookupStrategy = new RelyingPartyIdLookupFunction();
+    }
+    
+    /**
+     * Set lookup strategy for relying party ID.
+     * 
+     * @param strategy lookup strategy
+     * 
+     * @since 5.0.0
+     */
+    public void setRelyingPartyIdLookupStrategy(@Nonnull final Function<ProfileRequestContext,String> strategy) {
+        relyingPartyIdLookupStrategy = Constraint.isNotNull(strategy, "RelyingParty ID lookup strategy cannot be null");
     }
 
     @Override
@@ -117,13 +135,18 @@ public class UpdateIdPSessionWithSPSessionAction<RequestType,ResponseType>
         if (session != null) {
             final Instant now = Instant.now();
             assert now != null;
+            
             final Instant expiration = now.plus(sessionLifetime); 
             assert expiration != null;
+            
+            final String relyingPartyId = relyingPartyIdLookupStrategy.apply(profileRequestContext);
+            
             final SPSession sps = new CASSPSession(
-                    ticket.getService(),
+                    relyingPartyId != null ? relyingPartyId : ticket.getService(),
                     now,
                     expiration,
-                    ticket.getId());
+                    ticket.getId(),
+                    ticket.getService());
             log.debug("{} Created SP session {}", getLogPrefix(), sps);
             try {
                 session.addSPSession(sps);
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/session/impl/CASSPSession.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/session/impl/CASSPSession.java
index 29e62818f..1a561f16e 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/session/impl/CASSPSession.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/session/impl/CASSPSession.java
@@ -37,6 +37,9 @@ public class CASSPSession extends BasicSPSession {
 
     /** Validated ticket that started the SP session. */
     @Nonnull @NotEmpty private final String ticket;
+    
+    /** Full service URL. */
+    @Nonnull @NotEmpty private final String serviceURL;
 
     /**
      * Creates a new CAS SP session.
@@ -45,14 +48,17 @@ public class CASSPSession extends BasicSPSession {
      * @param creation   creation time of session
      * @param expiration expiration time of session
      * @param ticketId   ticket ID used to gain access to the service
+     * @param url full URL of service, as distinct from the possibly shortened ID
      */
     public CASSPSession(
             @Nonnull @NotEmpty final String id,
             @Nonnull final Instant creation,
             @Nonnull final Instant expiration,
-            @Nonnull @NotEmpty final String ticketId) {
+            @Nonnull @NotEmpty final String ticketId,
+            @Nonnull @NotEmpty final String url) {
         super(id, creation, expiration);
         ticket = Constraint.isNotNull(StringSupport.trimOrNull(ticketId), "Ticket ID cannot be null or empty");
+        serviceURL = Constraint.isNotNull(StringSupport.trimOrNull(url), "Service URL cannot be null or empty");
     }
 
     /** 
@@ -63,10 +69,19 @@ public class CASSPSession extends BasicSPSession {
     @Nonnull @NotEmpty public String getTicketId() {
         return ticket;
     }
+    
+    /**
+     * Get the service URL.
+     * 
+     * @return service URL
+     */
+    @Nonnull @NotEmpty public String getServiceURL() {
+        return serviceURL;
+    }
 
     /** {@inheritDoc} */
     @Override
-    public String getSPSessionKey() {
+    @Nullable public String getSPSessionKey() {
         return ticket;
     }
 
@@ -84,7 +99,7 @@ public class CASSPSession extends BasicSPSession {
 
     @Override
     public String toString() {
-        return "CASSPSession: " + getId() + " via " + ticket;
+        return "CASSPSession: " + getId() + " via " + ticket + " at service URL " + serviceURL;
     }
     
 }
\ No newline at end of file
diff --git a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/session/impl/CASSPSessionSerializer.java b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/session/impl/CASSPSessionSerializer.java
index 1286d4213..4aaec425a 100644
--- a/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/session/impl/CASSPSessionSerializer.java
+++ b/idp-cas-impl/src/main/java/net/shibboleth/idp/cas/session/impl/CASSPSessionSerializer.java
@@ -41,6 +41,9 @@ public class CASSPSessionSerializer extends AbstractSPSessionSerializer {
     /** Field name of CAS ticket. */
     @Nonnull @NotEmpty private static final String TICKET_FIELD = "st";
 
+    /** Field name of CAS service URL. */
+    @Nonnull @NotEmpty private static final String SERVICE_URL_FIELD = "surl";
+
     /**
      * Constructor.
      *
@@ -55,15 +58,26 @@ public class CASSPSessionSerializer extends AbstractSPSessionSerializer {
         if (!(instance instanceof CASSPSession)) {
             throw new IllegalArgumentException("Expected instance of CASSPSession but got " + instance);
         }
-        generator.write(TICKET_FIELD, ((CASSPSession) instance).getTicketId());
+        
+        final CASSPSession casSession = (CASSPSession) instance;
+        generator.write(TICKET_FIELD, casSession.getTicketId());
+        if (!casSession.getServiceURL().equals(casSession.getId())) {
+            generator.write(SERVICE_URL_FIELD, casSession.getServiceURL());
+        }
     }
 
     /** {@inheritDoc} */
     @Override
     @Nonnull protected SPSession doDeserialize(@Nonnull final JsonObject obj, @Nonnull @NotEmpty final String id,
             @Nonnull final Instant creation, @Nonnull final Instant expiration) throws IOException {
+        
         final String ticketField = Constraint.isNotNull(obj.getString(TICKET_FIELD), "No ticket field");
-        return new CASSPSession(id, creation, expiration, ticketField);
+        
+        // Default the service URL to the session ID if absent.
+        final String serviceURL = obj.getString(SERVICE_URL_FIELD, id);
+        assert serviceURL != null;
+        
+        return new CASSPSession(id, creation, expiration, ticketField, serviceURL);
     }
     
 }
\ No newline at end of file
diff --git a/idp-cas-impl/src/test/java/net/shibboleth/idp/cas/session/impl/CASSPSessionSerializerTest.java b/idp-cas-impl/src/test/java/net/shibboleth/idp/cas/session/impl/CASSPSessionSerializerTest.java
index f285c6c70..f79608166 100644
--- a/idp-cas-impl/src/test/java/net/shibboleth/idp/cas/session/impl/CASSPSessionSerializerTest.java
+++ b/idp-cas-impl/src/test/java/net/shibboleth/idp/cas/session/impl/CASSPSessionSerializerTest.java
@@ -43,7 +43,8 @@ public class CASSPSessionSerializerTest {
                 "https://foo.example.com/shibboleth",
                 other,
                 exp,
-                "ST-1234126-ABC1346DEADBEEF");
+                "ST-1234126-ABC1346DEADBEEF",
+                "https://foo.example.com/callback");
         final String serialized = serializer.serialize(original);
         final CASSPSession deserialized =
                 (CASSPSession) serializer.deserialize(1, "context", "key", serialized, exp.toEpochMilli());
@@ -51,6 +52,7 @@ public class CASSPSessionSerializerTest {
         assertEquals(deserialized.getCreationInstant(), original.getCreationInstant());
         assertEquals(deserialized.getExpirationInstant(), original.getExpirationInstant());
         assertEquals(deserialized.getTicketId(), original.getTicketId());
+        assertEquals(deserialized.getServiceURL(), original.getServiceURL());
     }
 
 }
\ No newline at end of file
diff --git a/idp-conf-impl/src/main/resources/net/shibboleth/idp/views/cas/logoutService.vm b/idp-conf-impl/src/main/resources/net/shibboleth/idp/views/cas/logoutService.vm
index 9a39d68f0..084c550d8 100644
--- a/idp-conf-impl/src/main/resources/net/shibboleth/idp/views/cas/logoutService.vm
+++ b/idp-conf-impl/src/main/resources/net/shibboleth/idp/views/cas/logoutService.vm
@@ -5,7 +5,7 @@
     <title>Logout of $logoutPropCtx.session.id</title>
 </head>
 <body onload="document.getElementById('logout_prop').submit()">
-<form id="logout_prop" method="POST" action="$logoutPropCtx.session.id">
+<form id="logout_prop" method="POST" action="$logoutPropCtx.session.serviceURL">
     <input type="hidden" name="logoutRequest" value="
         <samlp:LogoutRequest xmlns:samlp='urn:oasis:names:tc:SAML:2.0:protocol' ID='$messageID' Version='2.0' IssueInstant='$issueInstant'>
         <saml:NameID xmlns:saml='urn:oasis:names:tc:SAML:2.0:assertion'>somebody</saml:NameID>
diff --git a/idp-session-api/src/main/java/net/shibboleth/idp/session/BasicSPSession.java b/idp-session-api/src/main/java/net/shibboleth/idp/session/BasicSPSession.java
index 6fdeaa67a..b8660532c 100644
--- a/idp-session-api/src/main/java/net/shibboleth/idp/session/BasicSPSession.java
+++ b/idp-session-api/src/main/java/net/shibboleth/idp/session/BasicSPSession.java
@@ -75,7 +75,7 @@ public class BasicSPSession implements SPSession {
     }
 
     /** {@inheritDoc} */
-    public String getSPSessionKey() {
+    @Nullable public String getSPSessionKey() {
         // A basic session doesn't have a secondary lookup key.
         return null;
     }

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


More information about the commits mailing list