From 656e680019bb8844fe59393098fda729c8953a9a Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Sat, 20 Jan 2024 11:31:10 +0100 Subject: [PATCH] Remove unused HttpResponse.setWriteCookiesOnTransactionComplete (#26326) Closes #26325 Signed-off-by: stianst --- .../resteasy/QuarkusHttpResponse.java | 81 +--------------- .../resteasy/QuarkusKeycloakContext.java | 5 +- .../java/org/keycloak/http/HttpResponse.java | 8 -- .../services/DefaultKeycloakContext.java | 5 +- .../keycloak/services/HttpResponseImpl.java | 95 ++----------------- 5 files changed, 15 insertions(+), 179 deletions(-) diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/integration/resteasy/QuarkusHttpResponse.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/integration/resteasy/QuarkusHttpResponse.java index 5b48164a8c..d469f9593b 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/integration/resteasy/QuarkusHttpResponse.java +++ b/quarkus/runtime/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 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 final class QuarkusHttpResponse implements HttpResponse, KeycloakTransact } 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/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/integration/resteasy/QuarkusKeycloakContext.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/integration/resteasy/QuarkusKeycloakContext.java index 348e0c0345..df9f1e16e1 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/integration/resteasy/QuarkusKeycloakContext.java +++ b/quarkus/runtime/src/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.http.HttpResponse; 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 @@ public final class QuarkusKeycloakContext extends DefaultKeycloakContext { @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 index e1b7b0b9b6..85c0da03cf 100644 --- 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 index f650d7666f..c1edd3ef5f 100755 --- 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.models.UserModel; 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 @@ public class DefaultKeycloakContext implements KeycloakContext { } 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 index dfee67307e..f391a2c512 100644 --- 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 jakarta.ws.rs.core.HttpHeaders; +import org.keycloak.http.HttpCookie; +import org.keycloak.http.HttpResponse; + 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 { +public class HttpResponseImpl implements HttpResponse { private final org.jboss.resteasy.spi.HttpResponse delegate; private Set 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 class HttpResponseImpl implements HttpResponse, KeycloakTransaction { @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 class HttpResponseImpl implements HttpResponse, KeycloakTransaction { } 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()); - } - } }