Remove unused HttpResponse.setWriteCookiesOnTransactionComplete (#26326)

Closes #26325

Signed-off-by: stianst <stianst@gmail.com>
This commit is contained in:
Stian Thorgersen 2024-01-20 11:31:10 +01:00 committed by GitHub
parent 98be32d9ff
commit 656e680019
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 15 additions and 179 deletions

View file

@ -17,30 +17,23 @@
package org.keycloak.quarkus.runtime.integration.resteasy; package org.keycloak.quarkus.runtime.integration.resteasy;
import java.util.HashSet;
import java.util.Set;
import jakarta.ws.rs.core.HttpHeaders; import jakarta.ws.rs.core.HttpHeaders;
import org.jboss.resteasy.reactive.server.core.ResteasyReactiveRequestContext; 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.jboss.resteasy.reactive.server.vertx.VertxResteasyReactiveRequestContext;
import org.keycloak.http.HttpCookie; import org.keycloak.http.HttpCookie;
import org.keycloak.http.HttpResponse; 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 final ResteasyReactiveRequestContext requestContext;
private Set<HttpCookie> cookies; private Set<HttpCookie> cookies;
private boolean transactionActive;
private boolean writeCookiesOnTransactionComplete;
public QuarkusHttpResponse(KeycloakSession session, ResteasyReactiveRequestContext requestContext) { public QuarkusHttpResponse(ResteasyReactiveRequestContext requestContext) {
this.requestContext = requestContext; this.requestContext = requestContext;
session.getTransactionManager().enlistAfterCompletion(this);
} }
@Override @Override
@ -75,72 +68,8 @@ public final class QuarkusHttpResponse implements HttpResponse, KeycloakTransact
} }
if (cookies.add(cookie)) { if (cookies.add(cookie)) {
if (writeCookiesOnTransactionComplete) {
// cookies are written after transaction completes
return;
}
addHeader(HttpHeaders.SET_COOKIE, cookie.toHeaderValue()); 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());
}
}
} }

View file

@ -17,6 +17,7 @@
package org.keycloak.quarkus.runtime.integration.resteasy; 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.CurrentRequestManager;
import org.jboss.resteasy.reactive.server.core.ResteasyReactiveRequestContext; import org.jboss.resteasy.reactive.server.core.ResteasyReactiveRequestContext;
import org.keycloak.common.ClientConnection; import org.keycloak.common.ClientConnection;
@ -26,8 +27,6 @@ import org.keycloak.http.HttpResponse;
import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSession;
import org.keycloak.services.DefaultKeycloakContext; import org.keycloak.services.DefaultKeycloakContext;
import io.vertx.core.http.HttpServerRequest;
public final class QuarkusKeycloakContext extends DefaultKeycloakContext { public final class QuarkusKeycloakContext extends DefaultKeycloakContext {
private ClientConnection clientConnection; private ClientConnection clientConnection;
@ -43,7 +42,7 @@ public final class QuarkusKeycloakContext extends DefaultKeycloakContext {
@Override @Override
protected HttpResponse createHttpResponse() { protected HttpResponse createHttpResponse() {
return new QuarkusHttpResponse(getSession(), getResteasyReactiveRequestContext()); return new QuarkusHttpResponse(getResteasyReactiveRequestContext());
} }
@Override @Override

View file

@ -58,12 +58,4 @@ public interface HttpResponse {
*/ */
void setCookieIfAbsent(HttpCookie cookie); 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();
} }

View file

@ -17,6 +17,7 @@
package org.keycloak.services; package org.keycloak.services;
import jakarta.ws.rs.core.HttpHeaders;
import org.keycloak.common.ClientConnection; import org.keycloak.common.ClientConnection;
import org.keycloak.common.util.Resteasy; import org.keycloak.common.util.Resteasy;
import org.keycloak.http.HttpRequest; import org.keycloak.http.HttpRequest;
@ -31,8 +32,6 @@ import org.keycloak.models.UserModel;
import org.keycloak.sessions.AuthenticationSessionModel; import org.keycloak.sessions.AuthenticationSessionModel;
import org.keycloak.urls.UrlType; import org.keycloak.urls.UrlType;
import jakarta.ws.rs.core.HttpHeaders;
import jakarta.ws.rs.core.UriInfo;
import java.net.URI; import java.net.URI;
import java.util.HashMap; import java.util.HashMap;
import java.util.Locale; import java.util.Locale;
@ -176,7 +175,7 @@ public class DefaultKeycloakContext implements KeycloakContext {
} }
protected HttpResponse createHttpResponse() { 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() { protected KeycloakSession getSession() {

View file

@ -17,26 +17,20 @@
package org.keycloak.services; 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.HashSet;
import java.util.Set; import java.util.Set;
import jakarta.ws.rs.core.HttpHeaders; public class HttpResponseImpl implements HttpResponse {
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 {
private final org.jboss.resteasy.spi.HttpResponse delegate; private final org.jboss.resteasy.spi.HttpResponse delegate;
private Set<HttpCookie> cookies; 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; this.delegate = delegate;
session.getTransactionManager().enlistAfterCompletion(this);
} }
@Override @Override
@ -51,13 +45,11 @@ public class HttpResponseImpl implements HttpResponse, KeycloakTransaction {
@Override @Override
public void addHeader(String name, String value) { public void addHeader(String name, String value) {
checkCommitted();
delegate.getOutputHeaders().add(name, value); delegate.getOutputHeaders().add(name, value);
} }
@Override @Override
public void setHeader(String name, String value) { public void setHeader(String name, String value) {
checkCommitted();
delegate.getOutputHeaders().putSingle(name, value); delegate.getOutputHeaders().putSingle(name, value);
} }
@ -72,83 +64,8 @@ public class HttpResponseImpl implements HttpResponse, KeycloakTransaction {
} }
if (cookies.add(cookie)) { if (cookies.add(cookie)) {
if (writeCookiesOnTransactionComplete) {
// cookies are written after transaction completes
return;
}
addHeader(HttpHeaders.SET_COOKIE, cookie.toHeaderValue()); 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());
}
}
} }