From 3a804315cdead9711369ccb988be82e7930f8db2 Mon Sep 17 00:00:00 2001
From: Kristaps Berzinch <kristapsberzinch@gmail.com>
Date: Tue, 17 Sep 2024 20:35:59 -0400
Subject: [PATCH 1/4] Store state in cookie instead of service URL

---
 .../broker/cas/CasIdentityProvider.java       | 58 +++++++------------
 .../keycloak/broker/cas/util/UrlHelper.java   | 25 ++------
 2 files changed, 26 insertions(+), 57 deletions(-)

diff --git a/src/main/java/io/github/johnjcool/keycloak/broker/cas/CasIdentityProvider.java b/src/main/java/io/github/johnjcool/keycloak/broker/cas/CasIdentityProvider.java
index dc060e0..b7d12e7 100644
--- a/src/main/java/io/github/johnjcool/keycloak/broker/cas/CasIdentityProvider.java
+++ b/src/main/java/io/github/johnjcool/keycloak/broker/cas/CasIdentityProvider.java
@@ -1,6 +1,5 @@
 package io.github.johnjcool.keycloak.broker.cas;
 
-import static io.github.johnjcool.keycloak.broker.cas.util.UrlHelper.PROVIDER_PARAMETER_STATE;
 import static io.github.johnjcool.keycloak.broker.cas.util.UrlHelper.PROVIDER_PARAMETER_TICKET;
 import static io.github.johnjcool.keycloak.broker.cas.util.UrlHelper.createAuthenticationUrl;
 import static io.github.johnjcool.keycloak.broker.cas.util.UrlHelper.createLogoutUrl;
@@ -8,15 +7,11 @@
 
 import io.github.johnjcool.keycloak.broker.cas.model.ServiceResponse;
 import io.github.johnjcool.keycloak.broker.cas.model.Success;
+import jakarta.ws.rs.CookieParam;
 import jakarta.ws.rs.GET;
 import jakarta.ws.rs.Path;
 import jakarta.ws.rs.QueryParam;
-import jakarta.ws.rs.core.Context;
-import jakarta.ws.rs.core.HttpHeaders;
-import jakarta.ws.rs.core.MediaType;
-import jakarta.ws.rs.core.Response;
-import jakarta.ws.rs.core.Response.Status;
-import jakarta.ws.rs.core.UriInfo;
+import jakarta.ws.rs.core.*;
 import jakarta.xml.bind.JAXBContext;
 import jakarta.xml.bind.JAXBException;
 import jakarta.xml.bind.Unmarshaller;
@@ -48,6 +43,8 @@ public class CasIdentityProvider extends AbstractIdentityProvider<CasIdentityPro
 
   public static final String USER_ATTRIBUTES = "UserAttributes";
 
+  private static final String STATE_COOKIE_NAME = "__Host-cas_state";
+
   private static final Unmarshaller unmarshaller;
 
   static {
@@ -65,13 +62,15 @@ public CasIdentityProvider(
 
   @Override
   public Response performLogin(final AuthenticationRequest request) {
-    try {
-      URI authenticationUrl = createAuthenticationUrl(getConfig(), request).build();
-      return Response.seeOther(authenticationUrl).build();
-    } catch (Exception e) {
-      throw new IdentityBrokerException(
-          "Could not send authentication request to cas provider.", e);
-    }
+    return Response.seeOther(createAuthenticationUrl(getConfig(), request).build())
+        .cookie(
+            new NewCookie.Builder(STATE_COOKIE_NAME)
+                .value(request.getState().getEncoded())
+                .httpOnly(true)
+                .secure(true)
+                .path("/")
+                .build())
+        .build();
   }
 
   @Override
@@ -80,7 +79,7 @@ public Response keycloakInitiatedBrowserLogout(
       final UserSessionModel userSession,
       final UriInfo uriInfo,
       final RealmModel realm) {
-    URI logoutUrl = createLogoutUrl(getConfig(), userSession, realm, uriInfo).build();
+    URI logoutUrl = createLogoutUrl(getConfig(), realm, uriInfo).build();
     return Response.status(302).location(logoutUrl).build();
   }
 
@@ -101,7 +100,6 @@ public Endpoint callback(
   public static final class Endpoint {
     private final AuthenticationCallback callback;
     private final RealmModel realm;
-    private final EventBuilder event;
     private final KeycloakSession session;
     private final ClientConnection clientConnection;
     private final HttpHeaders headers;
@@ -115,7 +113,6 @@ public static final class Endpoint {
         final CasIdentityProvider provider) {
       this.callback = callback;
       this.realm = realm;
-      this.event = event;
       this.provider = provider;
       this.session = provider.session;
       this.headers = session.getContext().getRequestHeaders();
@@ -126,19 +123,12 @@ public static final class Endpoint {
     @GET
     public Response authResponse(
         @QueryParam(PROVIDER_PARAMETER_TICKET) final String ticket,
-        @QueryParam(PROVIDER_PARAMETER_STATE) final String state) {
-      try {
-        BrokeredIdentityContext federatedIdentity =
-            getFederatedIdentity(config, ticket, session.getContext().getUri(), state);
-
-        return callback.authenticated(federatedIdentity);
-      } catch (Exception e) {
-        logger.error("Failed to complete CAS authentication", e);
-      }
-      event.event(EventType.LOGIN);
-      event.error(Errors.IDENTITY_PROVIDER_LOGIN_FAILURE);
-      return ErrorPage.error(
-          session, null, Status.EXPECTATION_FAILED, Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR);
+        @CookieParam(STATE_COOKIE_NAME) final Cookie stateCookie) {
+      BrokeredIdentityContext federatedIdentity =
+          getFederatedIdentity(
+              config, ticket, session.getContext().getUri(), stateCookie.getValue());
+
+      return callback.authenticated(federatedIdentity);
     }
 
     @GET
@@ -177,11 +167,7 @@ private BrokeredIdentityContext getFederatedIdentity(
       logger.debug("Current state value: " + state);
       try (SimpleHttp.Response response =
           SimpleHttp.doGet(
-                  createValidateServiceUrl(config, ticket, uriInfo, state)
-                      .build()
-                      .toURL()
-                      .toString()
-                      .replace("+", "%2B"),
+                  createValidateServiceUrl(config, ticket, uriInfo).build().toURL().toString(),
                   session)
               .asResponse()) {
         if (response.getStatus() != 200) {
@@ -216,7 +202,7 @@ private BrokeredIdentityContext getFederatedIdentity(
         user.getContextData().put(USER_ATTRIBUTES, success.getAttributes());
         user.setIdp(provider);
         AuthenticationSessionModel authSession =
-            this.callback.getAndVerifyAuthenticationSession(state.replace(' ', '+'));
+            this.callback.getAndVerifyAuthenticationSession(state);
         session.getContext().setAuthenticationSession(authSession);
         user.setAuthenticationSession(authSession);
         return user;
diff --git a/src/main/java/io/github/johnjcool/keycloak/broker/cas/util/UrlHelper.java b/src/main/java/io/github/johnjcool/keycloak/broker/cas/util/UrlHelper.java
index a3975bb..0e6e89b 100644
--- a/src/main/java/io/github/johnjcool/keycloak/broker/cas/util/UrlHelper.java
+++ b/src/main/java/io/github/johnjcool/keycloak/broker/cas/util/UrlHelper.java
@@ -6,7 +6,6 @@
 import jakarta.ws.rs.core.UriInfo;
 import org.keycloak.broker.provider.AuthenticationRequest;
 import org.keycloak.models.RealmModel;
-import org.keycloak.models.UserSessionModel;
 import org.keycloak.services.resources.IdentityBrokerService;
 import org.keycloak.services.resources.RealmsResource;
 
@@ -15,7 +14,6 @@ public final class UrlHelper {
   private static final String PROVIDER_PARAMETER_RENEW = "renew";
   private static final String PROVIDER_PARAMETER_GATEWAY = "gateway";
   public static final String PROVIDER_PARAMETER_TICKET = "ticket";
-  public static final String PROVIDER_PARAMETER_STATE = "state";
 
   private UrlHelper() {
     // util
@@ -25,9 +23,7 @@ public static UriBuilder createAuthenticationUrl(
       final CasIdentityProviderConfig config, final AuthenticationRequest request) {
     UriBuilder builder =
         UriBuilder.fromUri(config.getCasServerLoginUrl())
-            .queryParam(
-                PROVIDER_PARAMETER_SERVICE,
-                createServiceUrl(request.getRedirectUri(), request.getState().getEncoded()));
+            .queryParam(PROVIDER_PARAMETER_SERVICE, request.getRedirectUri());
     if (config.isRenew()) {
       builder.queryParam(PROVIDER_PARAMETER_RENEW, config.isRenew());
     }
@@ -38,16 +34,11 @@ public static UriBuilder createAuthenticationUrl(
   }
 
   public static UriBuilder createValidateServiceUrl(
-      final CasIdentityProviderConfig config,
-      final String ticket,
-      final UriInfo uriInfo,
-      final String state) {
+      final CasIdentityProviderConfig config, final String ticket, final UriInfo uriInfo) {
     UriBuilder builder =
         UriBuilder.fromUri(config.getCasServiceValidateUrl())
             .queryParam(PROVIDER_PARAMETER_TICKET, ticket)
-            .queryParam(
-                PROVIDER_PARAMETER_SERVICE,
-                createServiceUrl(uriInfo.getAbsolutePath().toString(), state));
+            .queryParam(PROVIDER_PARAMETER_SERVICE, uriInfo.getAbsolutePath().toString());
     if (config.isRenew()) {
       builder.queryParam(PROVIDER_PARAMETER_RENEW, config.isRenew());
     }
@@ -55,22 +46,14 @@ public static UriBuilder createValidateServiceUrl(
   }
 
   public static UriBuilder createLogoutUrl(
-      final CasIdentityProviderConfig config,
-      final UserSessionModel userSession,
-      final RealmModel realm,
-      final UriInfo uriInfo) {
+      final CasIdentityProviderConfig config, final RealmModel realm, final UriInfo uriInfo) {
     final String redirect =
         RealmsResource.brokerUrl(uriInfo)
             .path(IdentityBrokerService.class, "getEndpoint")
             .path(CasIdentityProvider.Endpoint.class, "logoutResponse")
-            .queryParam(PROVIDER_PARAMETER_STATE, userSession.getId())
             .build(realm.getName(), config.getAlias())
             .toString();
     return UriBuilder.fromUri(config.getCasServerLogoutUrl())
         .queryParam(PROVIDER_PARAMETER_SERVICE, redirect);
   }
-
-  private static String createServiceUrl(final String serviceUrlPrefix, final String state) {
-    return String.format("%s?%s=%s", serviceUrlPrefix, PROVIDER_PARAMETER_STATE, state);
-  }
 }

From 05f2a5bff5edb0c26162c0968729bf837ea46988 Mon Sep 17 00:00:00 2001
From: Kristaps Berzinch <kristapsberzinch@gmail.com>
Date: Wed, 18 Sep 2024 20:09:23 -0400
Subject: [PATCH 2/4] Remove unneeded event parameter

---
 .../johnjcool/keycloak/broker/cas/CasIdentityProvider.java     | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/main/java/io/github/johnjcool/keycloak/broker/cas/CasIdentityProvider.java b/src/main/java/io/github/johnjcool/keycloak/broker/cas/CasIdentityProvider.java
index b7d12e7..f41c010 100644
--- a/src/main/java/io/github/johnjcool/keycloak/broker/cas/CasIdentityProvider.java
+++ b/src/main/java/io/github/johnjcool/keycloak/broker/cas/CasIdentityProvider.java
@@ -94,7 +94,7 @@ public Endpoint callback(
       final RealmModel realm,
       final org.keycloak.broker.provider.IdentityProvider.AuthenticationCallback callback,
       final EventBuilder event) {
-    return new Endpoint(callback, realm, event, this);
+    return new Endpoint(callback, realm, this);
   }
 
   public static final class Endpoint {
@@ -109,7 +109,6 @@ public static final class Endpoint {
     Endpoint(
         final AuthenticationCallback callback,
         final RealmModel realm,
-        final EventBuilder event,
         final CasIdentityProvider provider) {
       this.callback = callback;
       this.realm = realm;

From 3b86ad24d62030ce86cee4b698c5541effc0344e Mon Sep 17 00:00:00 2001
From: Kristaps Berzinch <kristapsberzinch@gmail.com>
Date: Wed, 18 Sep 2024 20:15:40 -0400
Subject: [PATCH 3/4] Remove unneeded local variables

---
 .../keycloak/broker/cas/CasIdentityProvider.java    | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/main/java/io/github/johnjcool/keycloak/broker/cas/CasIdentityProvider.java b/src/main/java/io/github/johnjcool/keycloak/broker/cas/CasIdentityProvider.java
index f41c010..0b753dd 100644
--- a/src/main/java/io/github/johnjcool/keycloak/broker/cas/CasIdentityProvider.java
+++ b/src/main/java/io/github/johnjcool/keycloak/broker/cas/CasIdentityProvider.java
@@ -17,7 +17,6 @@
 import jakarta.xml.bind.Unmarshaller;
 import java.io.IOException;
 import java.io.StringReader;
-import java.net.URI;
 import org.jboss.logging.Logger;
 import org.keycloak.broker.provider.AbstractIdentityProvider;
 import org.keycloak.broker.provider.AuthenticationRequest;
@@ -79,8 +78,9 @@ public Response keycloakInitiatedBrowserLogout(
       final UserSessionModel userSession,
       final UriInfo uriInfo,
       final RealmModel realm) {
-    URI logoutUrl = createLogoutUrl(getConfig(), realm, uriInfo).build();
-    return Response.status(302).location(logoutUrl).build();
+    return Response.status(302)
+        .location(createLogoutUrl(getConfig(), realm, uriInfo).build())
+        .build();
   }
 
   @Override
@@ -123,11 +123,8 @@ public static final class Endpoint {
     public Response authResponse(
         @QueryParam(PROVIDER_PARAMETER_TICKET) final String ticket,
         @CookieParam(STATE_COOKIE_NAME) final Cookie stateCookie) {
-      BrokeredIdentityContext federatedIdentity =
-          getFederatedIdentity(
-              config, ticket, session.getContext().getUri(), stateCookie.getValue());
-
-      return callback.authenticated(federatedIdentity);
+      return callback.authenticated(getFederatedIdentity(
+        config, ticket, session.getContext().getUri(), stateCookie.getValue()));
     }
 
     @GET

From e7394181876244f8190b32ddc422d0a8ceb89b2c Mon Sep 17 00:00:00 2001
From: Kristaps Berzinch <kristapsberzinch@gmail.com>
Date: Wed, 18 Sep 2024 20:17:05 -0400
Subject: [PATCH 4/4] Format

---
 .../johnjcool/keycloak/broker/cas/CasIdentityProvider.java   | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/main/java/io/github/johnjcool/keycloak/broker/cas/CasIdentityProvider.java b/src/main/java/io/github/johnjcool/keycloak/broker/cas/CasIdentityProvider.java
index 0b753dd..3833611 100644
--- a/src/main/java/io/github/johnjcool/keycloak/broker/cas/CasIdentityProvider.java
+++ b/src/main/java/io/github/johnjcool/keycloak/broker/cas/CasIdentityProvider.java
@@ -123,8 +123,9 @@ public static final class Endpoint {
     public Response authResponse(
         @QueryParam(PROVIDER_PARAMETER_TICKET) final String ticket,
         @CookieParam(STATE_COOKIE_NAME) final Cookie stateCookie) {
-      return callback.authenticated(getFederatedIdentity(
-        config, ticket, session.getContext().getUri(), stateCookie.getValue()));
+      return callback.authenticated(
+          getFederatedIdentity(
+              config, ticket, session.getContext().getUri(), stateCookie.getValue()));
     }
 
     @GET