Remove unused HttpResponse.setWriteCookiesOnTransactionComplete (keyc…

…loak#26326) Closes keycloak#26325 Signed-off-by: stianst <[email protected]>
tnorimat · Jan 20, 2024 · 656e680 · 656e680
1 parent 98be32d
commit 656e680
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 178 deletions.
diff --git a/.../src/main/java/org/keycloak/quarkus/runtime/integration/resteasy/QuarkusHttpResponse.java b/.../src/main/java/org/keycloak/quarkus/runtime/integration/resteasy/QuarkusHttpResponse.java
@@ -17,30 +17,23 @@
 
 package org.keycloak.quarkus.runtime.integration.resteasy;
 
-import java.util.HashSet;
-import java.util.Set;
-
 import jakarta.ws.rs.core.HttpHeaders;
-
 import org.jboss.resteasy.reactive.server.core.ResteasyReactiveRequestContext;
-import org.jboss.resteasy.reactive.server.spi.ServerHttpResponse;
 import org.jboss.resteasy.reactive.server.vertx.VertxResteasyReactiveRequestContext;
 import org.keycloak.http.HttpCookie;
 import org.keycloak.http.HttpResponse;
-import org.keycloak.models.KeycloakSession;
-import org.keycloak.models.KeycloakTransaction;
 
-public final class QuarkusHttpResponse implements HttpResponse, KeycloakTransaction {
+import java.util.HashSet;
+import java.util.Set;
+
+public final class QuarkusHttpResponse implements HttpResponse {
 
     private final ResteasyReactiveRequestContext requestContext;
 
     private Set<HttpCookie> cookies;
-    private boolean transactionActive;
-    private boolean writeCookiesOnTransactionComplete;
 
-    public QuarkusHttpResponse(KeycloakSession session, ResteasyReactiveRequestContext requestContext) {
+    public QuarkusHttpResponse(ResteasyReactiveRequestContext requestContext) {
         this.requestContext = requestContext;
-        session.getTransactionManager().enlistAfterCompletion(this);
     }
 
     @Override
@@ -75,72 +68,8 @@ public void setCookieIfAbsent(HttpCookie cookie) {
         }
 
         if (cookies.add(cookie)) {
-            if (writeCookiesOnTransactionComplete) {
-                // cookies are written after transaction completes
-                return;
-            }
-
             addHeader(HttpHeaders.SET_COOKIE, cookie.toHeaderValue());
         }
     }
 
-    @Override
-    public void setWriteCookiesOnTransactionComplete() {
-        this.writeCookiesOnTransactionComplete = true;
-    }
-
-    @Override
-    public void begin() {
-        transactionActive = true;
-    }
-
-    @Override
-    public void commit() {
-        if (!transactionActive) {
-            throw new IllegalStateException("Transaction not active. Response already committed or rolled back");
-        }
-
-        try {
-            addCookiesAfterTransaction();
-        } finally {
-            close();
-        }
-    }
-
-    @Override
-    public void rollback() {
-        close();
-    }
-
-    @Override
-    public void setRollbackOnly() {
-
-    }
-
-    @Override
-    public boolean getRollbackOnly() {
-        return false;
-    }
-
-    @Override
-    public boolean isActive() {
-        return transactionActive;
-    }
-
-    private void close() {
-        transactionActive = false;
-        cookies = null;
-    }
-
-    private void addCookiesAfterTransaction() {
-        if (cookies == null || !writeCookiesOnTransactionComplete) {
-            return;
-        }
-
-        // Ensure that cookies are only added when the transaction is complete, as otherwise cookies will be set for
-        // error pages, or will be added twice when running retries.
-        for (HttpCookie cookie : cookies) {
-            addHeader(HttpHeaders.SET_COOKIE, cookie.toHeaderValue());
-        }
-    }
 }
diff --git a/...c/main/java/org/keycloak/quarkus/runtime/integration/resteasy/QuarkusKeycloakContext.java b/...c/main/java/org/keycloak/quarkus/runtime/integration/resteasy/QuarkusKeycloakContext.java
@@ -17,6 +17,7 @@
 
 package org.keycloak.quarkus.runtime.integration.resteasy;
 
+import io.vertx.core.http.HttpServerRequest;
 import org.jboss.resteasy.reactive.server.core.CurrentRequestManager;
 import org.jboss.resteasy.reactive.server.core.ResteasyReactiveRequestContext;
 import org.keycloak.common.ClientConnection;
@@ -26,8 +27,6 @@
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.services.DefaultKeycloakContext;
 
-import io.vertx.core.http.HttpServerRequest;
-
 public final class QuarkusKeycloakContext extends DefaultKeycloakContext {
 
     private ClientConnection clientConnection;
@@ -43,7 +42,7 @@ protected HttpRequest createHttpRequest() {
 
     @Override
     protected HttpResponse createHttpResponse() {
-        return new QuarkusHttpResponse(getSession(), getResteasyReactiveRequestContext());
+        return new QuarkusHttpResponse(getResteasyReactiveRequestContext());
     }
 
     @Override

diff --git a/server-spi/src/main/java/org/keycloak/http/HttpResponse.java b/server-spi/src/main/java/org/keycloak/http/HttpResponse.java
@@ -58,12 +58,4 @@ public interface HttpResponse {
      */
     void setCookieIfAbsent(HttpCookie cookie);
 
-    /**
-     * Adding cookies at the end of the transaction helps when retrying a transaction might add the
-     * cookie multiple times. In some scenarios it must not be added at the end of the transaction,
-     * as at that time the response has already been sent to the caller ("committed"), so the code
-     * needs to make a choice. As retrying transactions is the exception, adding cookies at the end
-     * of the transaction is also the exception and needs to be switched on where necessary.
-     */
-    void setWriteCookiesOnTransactionComplete();
 }
diff --git a/services/src/main/java/org/keycloak/services/DefaultKeycloakContext.java b/services/src/main/java/org/keycloak/services/DefaultKeycloakContext.java
@@ -17,6 +17,7 @@
 
 package org.keycloak.services;
 
+import jakarta.ws.rs.core.HttpHeaders;
 import org.keycloak.common.ClientConnection;
 import org.keycloak.common.util.Resteasy;
 import org.keycloak.http.HttpRequest;
@@ -31,8 +32,6 @@
 import org.keycloak.sessions.AuthenticationSessionModel;
 import org.keycloak.urls.UrlType;
 
-import jakarta.ws.rs.core.HttpHeaders;
-import jakarta.ws.rs.core.UriInfo;
 import java.net.URI;
 import java.util.HashMap;
 import java.util.Locale;
@@ -176,7 +175,7 @@ protected HttpRequest createHttpRequest() {
     }
 
     protected HttpResponse createHttpResponse() {
-        return new HttpResponseImpl(session, getContextObject(org.jboss.resteasy.spi.HttpResponse.class));
+        return new HttpResponseImpl(getContextObject(org.jboss.resteasy.spi.HttpResponse.class));
     }
 
     protected KeycloakSession getSession() {

diff --git a/services/src/main/java/org/keycloak/services/HttpResponseImpl.java b/services/src/main/java/org/keycloak/services/HttpResponseImpl.java
@@ -17,26 +17,20 @@
 
 package org.keycloak.services;
 
-import java.util.HashSet;
-import java.util.Set;
-
 import jakarta.ws.rs.core.HttpHeaders;
-
 import org.keycloak.http.HttpCookie;
 import org.keycloak.http.HttpResponse;
-import org.keycloak.models.KeycloakSession;
-import org.keycloak.models.KeycloakTransaction;
 
-public class HttpResponseImpl implements HttpResponse, KeycloakTransaction {
+import java.util.HashSet;
+import java.util.Set;
+
+public class HttpResponseImpl implements HttpResponse {
 
     private final org.jboss.resteasy.spi.HttpResponse delegate;
     private Set<HttpCookie> cookies;
-    private boolean transactionActive;
-    private boolean writeCookiesOnTransactionComplete;
 
-    public HttpResponseImpl(KeycloakSession session, org.jboss.resteasy.spi.HttpResponse delegate) {
+    public HttpResponseImpl(org.jboss.resteasy.spi.HttpResponse delegate) {
         this.delegate = delegate;
-        session.getTransactionManager().enlistAfterCompletion(this);
     }
 
     @Override
@@ -51,13 +45,11 @@ public void setStatus(int statusCode) {
 
     @Override
     public void addHeader(String name, String value) {
-        checkCommitted();
         delegate.getOutputHeaders().add(name, value);
     }
 
     @Override
     public void setHeader(String name, String value) {
-        checkCommitted();
         delegate.getOutputHeaders().putSingle(name, value);
     }
 
@@ -72,83 +64,8 @@ public void setCookieIfAbsent(HttpCookie cookie) {
         }
 
         if (cookies.add(cookie)) {
-            if (writeCookiesOnTransactionComplete) {
-                // cookies are written after transaction completes
-                return;
-            }
-
             addHeader(HttpHeaders.SET_COOKIE, cookie.toHeaderValue());
         }
     }
 
-    @Override
-    public void setWriteCookiesOnTransactionComplete() {
-        this.writeCookiesOnTransactionComplete = true;
-    }
-
-    /**
-     * Validate that the response has not been committed.
-     * If the response is already committed, the headers and part of the response have been sent already.
-     * Therefore, additional headers including cookies won't be delivered to the caller.
-     */
-    private void checkCommitted() {
-        if (delegate.isCommitted()) {
-            throw new IllegalStateException("response already committed, can't be changed");
-        }
-    }
-
-    @Override
-    public void begin() {
-        transactionActive = true;
-    }
-
-    @Override
-    public void commit() {
-        if (!transactionActive) {
-            throw new IllegalStateException("Transaction not active. Response already committed or rolled back");
-        }
-
-        try {
-            addCookiesAfterTransaction();
-        } finally {
-            close();
-        }
-    }
-
-    @Override
-    public void rollback() {
-        close();
-    }
-
-    @Override
-    public void setRollbackOnly() {
-
-    }
-
-    @Override
-    public boolean getRollbackOnly() {
-        return false;
-    }
-
-    @Override
-    public boolean isActive() {
-        return transactionActive;
-    }
-
-    private void close() {
-        transactionActive = false;
-        cookies = null;
-    }
-
-    private void addCookiesAfterTransaction() {
-        if (cookies == null || !writeCookiesOnTransactionComplete) {
-            return;
-        }
-
-        // Ensure that cookies are only added when the transaction is complete, as otherwise cookies will be set for
-        // error pages, or will be added twice when running retries.
-        for (HttpCookie cookie : cookies) {
-            addHeader(HttpHeaders.SET_COOKIE, cookie.toHeaderValue());
-        }
-    }
 }