diff --git a/common/src/main/java/org/keycloak/common/util/ServerCookie.java b/common/src/main/java/org/keycloak/common/util/ServerCookie.java index 1ce9901d5b..ee79f654fa 100755 --- a/common/src/main/java/org/keycloak/common/util/ServerCookie.java +++ b/common/src/main/java/org/keycloak/common/util/ServerCookie.java @@ -33,7 +33,9 @@ public class ServerCookie implements Serializable { private static final String tspecials2 = "()<>@,;:\\\"/[]?={} \t"; public enum SameSiteAttributeValue { - NONE("None"); // we currently support only SameSite=None; this might change in the future + NONE("None"), + LAX("Lax"), + STRICT("Strict"); private final String specValue; SameSiteAttributeValue(String specValue) { diff --git a/docs/documentation/release_notes/topics/24_0_0.adoc b/docs/documentation/release_notes/topics/24_0_0.adoc index 9f49648f64..223b10bc09 100644 --- a/docs/documentation/release_notes/topics/24_0_0.adoc +++ b/docs/documentation/release_notes/topics/24_0_0.adoc @@ -170,3 +170,8 @@ link:{upgradingguide_link}[{upgradingguide_name}]. = Authorization Policy In previous versions of Keycloak when the last member of a User, Group or Client policy was deleted then that policy would also be deleted. Unfortunately this could lead to an escalation of privileges if the policy was used in an aggregate policy. To avoid privilege escalation the effect policies are no longer deleted and an administrator will need to update those policies. + += Updates to cookies + +Cookie handling code has been refactored and improved, including a new Cookie Provider. This provides better consistency +for cookies handled by Keycloak, and the ability to introduce configuration options around cookies if needed. \ No newline at end of file diff --git a/docs/documentation/upgrading/topics/keycloak/changes-24_0_0.adoc b/docs/documentation/upgrading/topics/keycloak/changes-24_0_0.adoc index db887c8651..609877ba44 100644 --- a/docs/documentation/upgrading/topics/keycloak/changes-24_0_0.adoc +++ b/docs/documentation/upgrading/topics/keycloak/changes-24_0_0.adoc @@ -285,3 +285,15 @@ After removal of the Map Store the following modules were renamed: * `org.keycloak:keycloak-model-legacy` to `org.keycloak:keycloak-model-storage` * `org.keycloak:keycloak-model-legacy-private` to `org.keycloak:keycloak-model-storage-private` * `org.keycloak:keycloak-model-legacy-services` to `org.keycloak:keycloak-model-storage-services` + += Updates to cookies + +As part of refactoring cookie handling in Keycloak there are some changes to how cookies are set: + +* All cookies will now have the secure attribute set if the request is through a secure context +* KEYCLOAK_LOCALE and WELCOME_STATE_CHECKER cookies now set SameSite=Strict + +For custom extensions there may be some changes needed: + +* LocaleSelectorProvider.KEYCLOAK_LOCALE is deprecated as cookies are now managed through the CookieProvider +* HttpResponse.setWriteCookiesOnTransactionComplete has been removed diff --git a/server-spi-private/src/main/java/org/keycloak/cookie/CookieMaxAge.java b/server-spi-private/src/main/java/org/keycloak/cookie/CookieMaxAge.java new file mode 100644 index 0000000000..cfd6f980a1 --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/cookie/CookieMaxAge.java @@ -0,0 +1,9 @@ +package org.keycloak.cookie; + +public interface CookieMaxAge { + + int EXPIRED = 0; + + int SESSION = -1; + +} diff --git a/server-spi-private/src/main/java/org/keycloak/cookie/CookiePath.java b/server-spi-private/src/main/java/org/keycloak/cookie/CookiePath.java new file mode 100644 index 0000000000..33c5ce1002 --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/cookie/CookiePath.java @@ -0,0 +1,6 @@ +package org.keycloak.cookie; + +enum CookiePath { + REALM, + REQUEST +} diff --git a/server-spi-private/src/main/java/org/keycloak/cookie/CookieProvider.java b/server-spi-private/src/main/java/org/keycloak/cookie/CookieProvider.java new file mode 100644 index 0000000000..c3680b3539 --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/cookie/CookieProvider.java @@ -0,0 +1,15 @@ +package org.keycloak.cookie; + +import org.keycloak.provider.Provider; + +public interface CookieProvider extends Provider { + + void set(CookieType cookieType, String value); + + void set(CookieType cookieType, String value, int maxAge); + + String get(CookieType cookieType); + + void expire(CookieType cookieType); + +} diff --git a/server-spi-private/src/main/java/org/keycloak/cookie/CookieProviderFactory.java b/server-spi-private/src/main/java/org/keycloak/cookie/CookieProviderFactory.java new file mode 100644 index 0000000000..5880ff3a4e --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/cookie/CookieProviderFactory.java @@ -0,0 +1,6 @@ +package org.keycloak.cookie; + +import org.keycloak.provider.ProviderFactory; + +public interface CookieProviderFactory extends ProviderFactory { +} diff --git a/server-spi-private/src/main/java/org/keycloak/cookie/CookieScope.java b/server-spi-private/src/main/java/org/keycloak/cookie/CookieScope.java new file mode 100644 index 0000000000..42ec185996 --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/cookie/CookieScope.java @@ -0,0 +1,39 @@ +package org.keycloak.cookie; + +import org.keycloak.common.util.ServerCookie; + +enum CookieScope { + // Internal cookies are only available for direct requests to Keycloak + INTERNAL(ServerCookie.SameSiteAttributeValue.STRICT, true), + + // Federation cookies are available after redirect from applications, and are also available in an iframe context + // unless the browser blocks third-party cookies + FEDERATION(ServerCookie.SameSiteAttributeValue.NONE, true), + + // Federation cookies that are also available from JavaScript + FEDERATION_JS(ServerCookie.SameSiteAttributeValue.NONE, false), + + // Legacy cookies do not set the SameSite attribute and will default to SameSite=Lax in modern browsers + @Deprecated + LEGACY(null, true), + + // Legacy cookies that are also available from JavaScript + @Deprecated + LEGACY_JS(null, false); + + private final ServerCookie.SameSiteAttributeValue sameSite; + private final boolean httpOnly; + + CookieScope(ServerCookie.SameSiteAttributeValue sameSite, boolean httpOnly) { + this.sameSite = sameSite; + this.httpOnly = httpOnly; + } + + public ServerCookie.SameSiteAttributeValue getSameSite() { + return sameSite; + } + + public boolean isHttpOnly() { + return httpOnly; + } +} diff --git a/server-spi-private/src/main/java/org/keycloak/cookie/CookieSpi.java b/server-spi-private/src/main/java/org/keycloak/cookie/CookieSpi.java new file mode 100644 index 0000000000..e518843d44 --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/cookie/CookieSpi.java @@ -0,0 +1,27 @@ +package org.keycloak.cookie; + +import org.keycloak.provider.Provider; +import org.keycloak.provider.ProviderFactory; +import org.keycloak.provider.Spi; + +public class CookieSpi implements Spi { + @Override + public boolean isInternal() { + return true; + } + + @Override + public String getName() { + return "cookie"; + } + + @Override + public Class getProviderClass() { + return CookieProvider.class; + } + + @Override + public Class getProviderFactoryClass() { + return CookieProviderFactory.class; + } +} diff --git a/server-spi-private/src/main/java/org/keycloak/cookie/CookieType.java b/server-spi-private/src/main/java/org/keycloak/cookie/CookieType.java new file mode 100644 index 0000000000..8c8dc79b30 --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/cookie/CookieType.java @@ -0,0 +1,39 @@ +package org.keycloak.cookie; + +public enum CookieType { + + KEYCLOAK_LOCALE(CookiePath.REALM, CookieScope.INTERNAL, CookieMaxAge.SESSION), + WELCOME_STATE_CHECKER(CookiePath.REQUEST, CookieScope.INTERNAL, 300), + KC_AUTH_STATE(CookiePath.REALM, CookieScope.LEGACY_JS), // TODO Change CookieScope + KC_RESTART(CookiePath.REALM, CookieScope.LEGACY, CookieMaxAge.SESSION); // TODO Change CookieScope + + private final CookiePath path; + private final CookieScope scope; + + private final Integer defaultMaxAge; + + CookieType(CookiePath path, CookieScope scope) { + this.path = path; + this.scope = scope; + this.defaultMaxAge = null; + } + + CookieType(CookiePath path, CookieScope scope, int defaultMaxAge) { + this.path = path; + this.scope = scope; + this.defaultMaxAge = defaultMaxAge; + } + + public CookiePath getPath() { + return path; + } + + public CookieScope getScope() { + return scope; + } + + public Integer getDefaultMaxAge() { + return defaultMaxAge; + } + +} diff --git a/server-spi-private/src/main/resources/META-INF/services/org.keycloak.provider.Spi b/server-spi-private/src/main/resources/META-INF/services/org.keycloak.provider.Spi index 5d26788ba9..64c2a0d586 100755 --- a/server-spi-private/src/main/resources/META-INF/services/org.keycloak.provider.Spi +++ b/server-spi-private/src/main/resources/META-INF/services/org.keycloak.provider.Spi @@ -93,3 +93,4 @@ org.keycloak.services.clientpolicy.ClientPolicyManagerSpi org.keycloak.userprofile.UserProfileSpi org.keycloak.device.DeviceRepresentationSpi org.keycloak.health.LoadBalancerCheckSpi +org.keycloak.cookie.CookieSpi \ No newline at end of file diff --git a/server-spi/src/main/java/org/keycloak/locale/LocaleSelectorProvider.java b/server-spi/src/main/java/org/keycloak/locale/LocaleSelectorProvider.java index e6597d3e6d..bcf1628557 100644 --- a/server-spi/src/main/java/org/keycloak/locale/LocaleSelectorProvider.java +++ b/server-spi/src/main/java/org/keycloak/locale/LocaleSelectorProvider.java @@ -24,6 +24,7 @@ import java.util.Locale; public interface LocaleSelectorProvider extends Provider { + @Deprecated(since = "24.0.0", forRemoval = true) String LOCALE_COOKIE = "KEYCLOAK_LOCALE"; String KC_LOCALE_PARAM = "kc_locale"; diff --git a/services/src/main/java/org/keycloak/cookie/DefaultCookieProvider.java b/services/src/main/java/org/keycloak/cookie/DefaultCookieProvider.java new file mode 100644 index 0000000000..38c34ca67f --- /dev/null +++ b/services/src/main/java/org/keycloak/cookie/DefaultCookieProvider.java @@ -0,0 +1,126 @@ +package org.keycloak.cookie; + +import jakarta.ws.rs.core.Cookie; +import org.keycloak.common.util.ServerCookie; +import org.keycloak.http.HttpCookie; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; +import org.keycloak.services.resources.RealmsResource; +import org.keycloak.urls.UrlType; + +import java.net.URI; + +public class DefaultCookieProvider implements CookieProvider { + + private static final String LEGACY_SUFFIX = "_LEGACY"; + + private KeycloakSession session; + + private boolean legacyCookiesEnabled; + + public DefaultCookieProvider(KeycloakSession session, boolean legacyCookiesEnabled) { + this.session = session; + this.legacyCookiesEnabled = legacyCookiesEnabled; + } + + @Override + public void set(CookieType cookieType, String value) { + if (cookieType.getDefaultMaxAge() == null) { + throw new IllegalArgumentException(cookieType + " has no default max-age"); + } + + set(cookieType, value, cookieType.getDefaultMaxAge()); + } + + @Override + public void set(CookieType cookieType, String value, int maxAge) { + String name = cookieType.name(); + ServerCookie.SameSiteAttributeValue sameSite = cookieType.getScope().getSameSite(); + boolean secure = resolveSecure(sameSite); + String path = resolvePath(cookieType); + boolean httpOnly = cookieType.getScope().isHttpOnly(); + + HttpCookie newCookie = new HttpCookie(1, name, value, path, null, null, maxAge, secure, httpOnly, sameSite); + session.getContext().getHttpResponse().setCookieIfAbsent(newCookie); + + if (legacyCookiesEnabled) { + if (ServerCookie.SameSiteAttributeValue.NONE.equals(sameSite)) { + String legacyName = name + LEGACY_SUFFIX; + HttpCookie legacyCookie = new HttpCookie(1, legacyName, value, path, null, null, maxAge, secure, httpOnly, null); + session.getContext().getHttpResponse().setCookieIfAbsent(legacyCookie); + } + } else { + expireLegacy(cookieType); + } + } + + @Override + public String get(CookieType cookieType) { + Cookie cookie = session.getContext().getRequestHeaders().getCookies().get(cookieType.name()); + return cookie != null ? cookie.getValue() : null; + } + + @Override + public void expire(CookieType cookieType) { + Cookie cookie = session.getContext().getRequestHeaders().getCookies().get(cookieType.name()); + expire(cookie, cookieType); + + expireLegacy(cookieType); + } + + private void expireLegacy(CookieType cookieType) { + String legacyName = cookieType.name() + LEGACY_SUFFIX; + Cookie legacyCookie = session.getContext().getRequestHeaders().getCookies().get(legacyName); + expire(legacyCookie, cookieType); + } + + private void expire(Cookie cookie, CookieType cookieType) { + if (cookie != null) { + String path = resolvePath(cookieType); + HttpCookie newCookie = new HttpCookie(1, cookie.getName(), "", path, null, null, 0, false, false, null); + session.getContext().getHttpResponse().setCookieIfAbsent(newCookie); + } + } + + @Override + public void close() { + } + + private String resolvePath(CookieType cookieType) { + switch (cookieType.getPath()) { + case REALM: + return RealmsResource.realmBaseUrl(session.getContext().getUri()).path("/").build(session.getContext().getRealm().getName()).getRawPath(); + case REQUEST: + return session.getContext().getUri().getRequestUri().getRawPath(); + default: + throw new IllegalArgumentException("Unsupported enum value " + cookieType.getPath().name()); + } + } + + private boolean resolveSecure(ServerCookie.SameSiteAttributeValue sameSite) { + URI requestUri = session.getContext().getUri().getRequestUri(); + + // SameSite=none requires secure context + if (ServerCookie.SameSiteAttributeValue.NONE.equals(sameSite)) { + return true; + } + + RealmModel realm = session.getContext().getRealm(); + if (realm != null && realm.getSslRequired().isRequired(requestUri.getHost())) { + return true; + } + + if ("https".equals(requestUri.getScheme())) { + return true; + } + + // Browsers consider 127.0.0.1, localhost and *.localhost as secure contexts + String frontendHostname = session.getContext().getUri(UrlType.FRONTEND).getRequestUri().getHost(); + if (frontendHostname.equals("127.0.0.1") || frontendHostname.equals("localhost") || frontendHostname.endsWith(".localhost")) { + return true; + } + + return false; + } + +} diff --git a/services/src/main/java/org/keycloak/cookie/DefaultCookieProviderFactory.java b/services/src/main/java/org/keycloak/cookie/DefaultCookieProviderFactory.java new file mode 100644 index 0000000000..b16b0ee4a2 --- /dev/null +++ b/services/src/main/java/org/keycloak/cookie/DefaultCookieProviderFactory.java @@ -0,0 +1,34 @@ +package org.keycloak.cookie; + +import org.keycloak.Config; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.KeycloakSessionFactory; + +public class DefaultCookieProviderFactory implements CookieProviderFactory { + + private boolean legacyCookies; + + @Override + public CookieProvider create(KeycloakSession session) { + return new DefaultCookieProvider(session, legacyCookies); + } + + @Override + public void init(Config.Scope config) { + legacyCookies = config.getBoolean("legacyCookies", false); + } + + @Override + public void postInit(KeycloakSessionFactory factory) { + } + + @Override + public void close() { + } + + @Override + public String getId() { + return "default"; + } + +} diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/AuthenticationStateCookie.java b/services/src/main/java/org/keycloak/forms/login/freemarker/AuthenticationStateCookie.java index db597c38bb..904faad8c4 100644 --- a/services/src/main/java/org/keycloak/forms/login/freemarker/AuthenticationStateCookie.java +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/AuthenticationStateCookie.java @@ -26,6 +26,8 @@ import com.fasterxml.jackson.annotation.JsonProperty; import jakarta.ws.rs.core.UriInfo; import org.jboss.logging.Logger; import org.keycloak.common.ClientConnection; +import org.keycloak.cookie.CookieProvider; +import org.keycloak.cookie.CookieType; import org.keycloak.models.KeycloakContext; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; @@ -67,30 +69,23 @@ public class AuthenticationStateCookie { this.remainingTabs = remainingTabs; } - public static void generateAndSetCookie(KeycloakSession session, RealmModel realm, RootAuthenticationSessionModel rootAuthSession, int cookieMaxAge) { - UriInfo uriInfo = session.getContext().getHttpRequest().getUri(); - String path = AuthenticationManager.getRealmCookiePath(realm, uriInfo); - boolean secureOnly = realm.getSslRequired().isRequired(session.getContext().getConnection()); - + public static void generateAndSetCookie(KeycloakSession session, RootAuthenticationSessionModel rootAuthSession, int cookieMaxAge) { AuthenticationStateCookie cookie = new AuthenticationStateCookie(); cookie.setAuthSessionId(rootAuthSession.getId()); cookie.setRemainingTabs(rootAuthSession.getAuthenticationSessions().keySet()); try { String encoded = JsonSerialization.writeValueAsString(cookie); - logger.tracef("Generating new %s cookie. Cookie: %s, Cookie lifespan: %d", KC_AUTH_STATE, encoded, cookieMaxAge); + logger.tracef("Generating new %s cookie. Cookie: %s, Cookie lifespan: %d", CookieType.KC_AUTH_STATE, encoded, cookieMaxAge); - CookieHelper.addCookie(KC_AUTH_STATE, encoded, path, null, null, cookieMaxAge, secureOnly, false, session); + session.getProvider(CookieProvider.class).set(CookieType.KC_AUTH_STATE, encoded, cookieMaxAge); } catch (IOException ioe) { throw new IllegalStateException("Exception thrown when encoding cookie", ioe); } } - public static void expireCookie(RealmModel realm, KeycloakSession session) { - UriInfo uriInfo = session.getContext().getHttpRequest().getUri(); - String path = AuthenticationManager.getRealmCookiePath(realm, uriInfo); - boolean secureOnly = realm.getSslRequired().isRequired(session.getContext().getConnection()); - CookieHelper.addCookie(KC_AUTH_STATE, "", path, null, null, 0, secureOnly, false, session); + public static void expireCookie(KeycloakSession session) { + session.getProvider(CookieProvider.class).expire(CookieType.KC_AUTH_STATE); } @Override diff --git a/services/src/main/java/org/keycloak/locale/DefaultLocaleSelectorProvider.java b/services/src/main/java/org/keycloak/locale/DefaultLocaleSelectorProvider.java index 2faf7b80b7..b8febd0770 100644 --- a/services/src/main/java/org/keycloak/locale/DefaultLocaleSelectorProvider.java +++ b/services/src/main/java/org/keycloak/locale/DefaultLocaleSelectorProvider.java @@ -17,12 +17,13 @@ package org.keycloak.locale; import org.jboss.logging.Logger; +import org.keycloak.cookie.CookieProvider; +import org.keycloak.cookie.CookieType; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.sessions.AuthenticationSessionModel; -import jakarta.ws.rs.core.Cookie; import jakarta.ws.rs.core.HttpHeaders; import java.util.List; import java.util.Locale; @@ -131,12 +132,12 @@ public class DefaultLocaleSelectorProvider implements LocaleSelectorProvider { return null; } - Cookie localeCookie = httpHeaders.getCookies().get(LOCALE_COOKIE); + String localeCookie = session.getProvider(CookieProvider.class).get(CookieType.KEYCLOAK_LOCALE); if (localeCookie == null) { return null; } - return findLocale(realm, localeCookie.getValue()); + return findLocale(realm, localeCookie); } private Locale getAcceptLanguageHeaderLocale(RealmModel realm, HttpHeaders httpHeaders) { diff --git a/services/src/main/java/org/keycloak/locale/DefaultLocaleUpdaterProvider.java b/services/src/main/java/org/keycloak/locale/DefaultLocaleUpdaterProvider.java index 1f18a13d01..4bc65c9f18 100644 --- a/services/src/main/java/org/keycloak/locale/DefaultLocaleUpdaterProvider.java +++ b/services/src/main/java/org/keycloak/locale/DefaultLocaleUpdaterProvider.java @@ -16,16 +16,14 @@ */ package org.keycloak.locale; -import jakarta.ws.rs.core.UriInfo; import org.jboss.logging.Logger; +import org.keycloak.cookie.CookieProvider; +import org.keycloak.cookie.CookieType; import org.keycloak.events.Details; import org.keycloak.events.EventBuilder; import org.keycloak.events.EventType; import org.keycloak.models.KeycloakSession; -import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; -import org.keycloak.services.managers.AuthenticationManager; -import org.keycloak.services.util.CookieHelper; import org.keycloak.storage.ReadOnlyException; public class DefaultLocaleUpdaterProvider implements LocaleUpdaterProvider { @@ -61,21 +59,13 @@ public class DefaultLocaleUpdaterProvider implements LocaleUpdaterProvider { @Override public void updateLocaleCookie(String locale) { - RealmModel realm = session.getContext().getRealm(); - UriInfo uriInfo = session.getContext().getUri(); - - boolean secure = realm.getSslRequired().isRequired(uriInfo.getRequestUri().getHost()); - CookieHelper.addCookie(LocaleSelectorProvider.LOCALE_COOKIE, locale, AuthenticationManager.getRealmCookiePath(realm, uriInfo), null, null, -1, secure, true, session); + session.getProvider(CookieProvider.class).set(CookieType.KEYCLOAK_LOCALE, locale); logger.debugv("Updating locale cookie to {0}", locale); } @Override public void expireLocaleCookie() { - RealmModel realm = session.getContext().getRealm(); - UriInfo uriInfo = session.getContext().getUri(); - - boolean secure = realm.getSslRequired().isRequired(session.getContext().getConnection()); - CookieHelper.addCookie(LocaleSelectorProvider.LOCALE_COOKIE, "", AuthenticationManager.getRealmCookiePath(realm, uriInfo), null, "Expiring cookie", 0, secure, true, session); + session.getProvider(CookieProvider.class).expire(CookieType.KEYCLOAK_LOCALE); } @Override diff --git a/services/src/main/java/org/keycloak/protocol/AuthorizationEndpointBase.java b/services/src/main/java/org/keycloak/protocol/AuthorizationEndpointBase.java index 9e88d70ef1..ae771ba8a2 100755 --- a/services/src/main/java/org/keycloak/protocol/AuthorizationEndpointBase.java +++ b/services/src/main/java/org/keycloak/protocol/AuthorizationEndpointBase.java @@ -17,14 +17,16 @@ package org.keycloak.protocol; +import jakarta.ws.rs.core.HttpHeaders; +import jakarta.ws.rs.core.Response; import org.jboss.logging.Logger; -import org.keycloak.http.HttpRequest; import org.keycloak.authentication.AuthenticationProcessor; import org.keycloak.common.ClientConnection; import org.keycloak.events.Details; import org.keycloak.events.Errors; import org.keycloak.events.EventBuilder; import org.keycloak.forms.login.LoginFormsProvider; +import org.keycloak.http.HttpRequest; import org.keycloak.models.AuthenticationFlowModel; import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; @@ -42,9 +44,6 @@ import org.keycloak.services.resources.LoginActionsService; import org.keycloak.sessions.AuthenticationSessionModel; import org.keycloak.sessions.RootAuthenticationSessionModel; -import jakarta.ws.rs.core.HttpHeaders; -import jakarta.ws.rs.core.Response; - /** * Common base class for Authorization REST endpoints implementation, which have to be implemented by each protocol. * @@ -120,7 +119,7 @@ public abstract class AuthorizationEndpointBase { } else { // KEYCLOAK-8043: forward the request with prompt=none to the default provider. if ("true".equals(authSession.getAuthNote(AuthenticationProcessor.FORWARDED_PASSIVE_LOGIN))) { - RestartLoginCookie.setRestartCookie(session, realm, clientConnection, session.getContext().getUri(), authSession); + RestartLoginCookie.setRestartCookie(session, authSession); if (redirectToAuthentication) { return processor.redirectToFlow(); } @@ -146,7 +145,7 @@ public abstract class AuthorizationEndpointBase { return processor.finishAuthentication(protocol); } else { try { - RestartLoginCookie.setRestartCookie(session, realm, clientConnection, session.getContext().getUri(), authSession); + RestartLoginCookie.setRestartCookie(session, authSession); if (redirectToAuthentication) { return processor.redirectToFlow(); } diff --git a/services/src/main/java/org/keycloak/protocol/RestartLoginCookie.java b/services/src/main/java/org/keycloak/protocol/RestartLoginCookie.java index d52fbcfa82..178a0330d0 100644 --- a/services/src/main/java/org/keycloak/protocol/RestartLoginCookie.java +++ b/services/src/main/java/org/keycloak/protocol/RestartLoginCookie.java @@ -21,19 +21,15 @@ import com.fasterxml.jackson.annotation.JsonProperty; import org.jboss.logging.Logger; import org.keycloak.Token; import org.keycloak.TokenCategory; -import org.keycloak.common.ClientConnection; +import org.keycloak.cookie.CookieProvider; +import org.keycloak.cookie.CookieType; import org.keycloak.models.ClientModel; -import org.keycloak.models.KeycloakContext; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; -import org.keycloak.services.managers.AuthenticationManager; import org.keycloak.services.managers.AuthenticationSessionManager; -import org.keycloak.services.util.CookieHelper; import org.keycloak.sessions.AuthenticationSessionModel; import org.keycloak.sessions.RootAuthenticationSessionModel; -import jakarta.ws.rs.core.Cookie; -import jakarta.ws.rs.core.UriInfo; import java.util.HashMap; import java.util.Map; @@ -120,24 +116,18 @@ public class RestartLoginCookie implements Token { } } - public static void setRestartCookie(KeycloakSession session, RealmModel realm, ClientConnection connection, UriInfo uriInfo, AuthenticationSessionModel authSession) { + public static void setRestartCookie(KeycloakSession session, AuthenticationSessionModel authSession) { RestartLoginCookie restart = new RestartLoginCookie(authSession); String encoded = session.tokens().encode(restart); - String path = AuthenticationManager.getRealmCookiePath(realm, uriInfo); - boolean secureOnly = realm.getSslRequired().isRequired(connection); - CookieHelper.addCookie(KC_RESTART, encoded, path, null, null, -1, secureOnly, true, session); + session.getProvider(CookieProvider.class).set(CookieType.KC_RESTART, encoded); } - public static void expireRestartCookie(RealmModel realm, UriInfo uriInfo, KeycloakSession session) { - KeycloakContext context = session.getContext(); - ClientConnection connection = context.getConnection(); - String path = AuthenticationManager.getRealmCookiePath(realm, uriInfo); - boolean secureOnly = realm.getSslRequired().isRequired(connection); - CookieHelper.addCookie(KC_RESTART, "", path, null, null, 0, secureOnly, true, session); + public static void expireRestartCookie(KeycloakSession session) { + session.getProvider(CookieProvider.class).expire(CookieType.KC_RESTART); } - public static Cookie getRestartCookie(KeycloakSession session){ - Cookie cook = session.getContext().getRequestHeaders().getCookies().get(KC_RESTART); + public static String getRestartCookie(KeycloakSession session){ + String cook = session.getProvider(CookieProvider.class).get(CookieType.KC_RESTART); if (cook == null) { logger.debug("KC_RESTART cookie doesn't exist"); return null; @@ -147,10 +137,7 @@ public class RestartLoginCookie implements Token { public static AuthenticationSessionModel restartSession(KeycloakSession session, RealmModel realm, RootAuthenticationSessionModel rootSession, String expectedClientId, - Cookie cook) throws Exception { - - String encodedCookie = cook.getValue(); - + String encodedCookie) throws Exception { RestartLoginCookie cookie = session.tokens().decode(encodedCookie, RestartLoginCookie.class); if (cookie == null) { logger.debug("Failed to verify encoded RestartLoginCookie"); diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java index a7801e3823..6fe7345951 100644 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java @@ -17,6 +17,7 @@ package org.keycloak.services.managers; +import jakarta.ws.rs.core.UriInfo; import org.jboss.logging.Logger; import org.keycloak.common.util.ServerCookie.SameSiteAttributeValue; import org.keycloak.common.util.Time; @@ -33,13 +34,6 @@ import org.keycloak.sessions.AuthenticationSessionModel; import org.keycloak.sessions.RootAuthenticationSessionModel; import org.keycloak.sessions.StickySessionEncoderProvider; -import jakarta.ws.rs.core.UriInfo; -import java.util.List; -import java.util.Objects; -import java.util.Set; -import java.util.function.Predicate; -import java.util.stream.Collectors; - /** * @author Marek Posolda @@ -191,9 +185,8 @@ public class AuthenticationSessionManager { // expire restart cookie if (expireRestartCookie) { - UriInfo uriInfo = session.getContext().getUri(); - RestartLoginCookie.expireRestartCookie(realm, uriInfo, session); - AuthenticationStateCookie.expireCookie(realm, session); + RestartLoginCookie.expireRestartCookie(session); + AuthenticationStateCookie.expireCookie(session); // With browser session, this makes sure that info/error pages will be rendered correctly when locale is changed on them session.getProvider(LoginFormsProvider.class).setDetachedAuthSession(); @@ -242,7 +235,7 @@ public class AuthenticationSessionManager { log.tracef("Removed authentication session of root session '%s' with tabId '%s'. But there are remaining tabs in the root session. Root authentication session will expire in %d seconds", rootAuthSession.getId(), authSession.getTabId(), authSessionExpiresIn); - AuthenticationStateCookie.generateAndSetCookie(session, realm, rootAuthSession, authSessionExpiresIn); + AuthenticationStateCookie.generateAndSetCookie(session, rootAuthSession, authSessionExpiresIn); } } diff --git a/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java b/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java index 5e94939aca..84527a6094 100644 --- a/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java +++ b/services/src/main/java/org/keycloak/services/resources/SessionCodeChecks.java @@ -385,7 +385,7 @@ public class SessionCodeChecks { logger.debug("Authentication session not found. Trying to restart from cookie."); AuthenticationSessionModel authSession = null; - Cookie cook = RestartLoginCookie.getRestartCookie(session); + String cook = RestartLoginCookie.getRestartCookie(session); if (cook == null) { event.error(Errors.COOKIE_NOT_FOUND); return ErrorPage.error(session, authSession, Response.Status.BAD_REQUEST, Messages.COOKIE_NOT_FOUND); diff --git a/services/src/main/java/org/keycloak/services/resources/WelcomeResource.java b/services/src/main/java/org/keycloak/services/resources/WelcomeResource.java index 79c999aea7..615630edfd 100755 --- a/services/src/main/java/org/keycloak/services/resources/WelcomeResource.java +++ b/services/src/main/java/org/keycloak/services/resources/WelcomeResource.java @@ -24,7 +24,6 @@ import jakarta.ws.rs.PathParam; import jakarta.ws.rs.Produces; import jakarta.ws.rs.WebApplicationException; import jakarta.ws.rs.core.Context; -import jakarta.ws.rs.core.Cookie; import jakarta.ws.rs.core.HttpHeaders; import jakarta.ws.rs.core.MultivaluedMap; import jakarta.ws.rs.core.Response; @@ -37,13 +36,14 @@ import org.keycloak.common.Version; import org.keycloak.common.util.Base64Url; import org.keycloak.common.util.MimeTypeUtil; import org.keycloak.common.util.SecretGenerator; +import org.keycloak.cookie.CookieProvider; +import org.keycloak.cookie.CookieType; import org.keycloak.http.HttpRequest; import org.keycloak.models.KeycloakSession; import org.keycloak.services.ForbiddenException; import org.keycloak.services.ServicesLogger; import org.keycloak.services.managers.ApplianceBootstrap; import org.keycloak.services.util.CacheControlUtil; -import org.keycloak.services.util.CookieHelper; import org.keycloak.theme.Theme; import org.keycloak.theme.freemarker.FreeMarkerProvider; import org.keycloak.urls.UrlType; @@ -283,28 +283,17 @@ public class WelcomeResource { private String setCsrfCookie() { String stateChecker = Base64Url.encode(SecretGenerator.getInstance().randomBytes()); - String cookiePath = session.getContext().getUri().getPath(); - boolean secureOnly = session.getContext().getUri().getRequestUri().getScheme().equalsIgnoreCase("https"); - CookieHelper.addCookie(KEYCLOAK_STATE_CHECKER, stateChecker, cookiePath, null, null, 300, secureOnly, true, session); + session.getProvider(CookieProvider.class).set(CookieType.WELCOME_STATE_CHECKER, stateChecker); return stateChecker; } private void expireCsrfCookie() { - String cookiePath = session.getContext().getUri().getPath(); - boolean secureOnly = session.getContext().getUri().getRequestUri().getScheme().equalsIgnoreCase("https"); - CookieHelper.addCookie(KEYCLOAK_STATE_CHECKER, "", cookiePath, null, null, 0, secureOnly, true, session); + session.getProvider(CookieProvider.class).expire(CookieType.WELCOME_STATE_CHECKER); } private void csrfCheck(final MultivaluedMap formData) { String formStateChecker = formData.getFirst("stateChecker"); - HttpRequest request = session.getContext().getHttpRequest(); - HttpHeaders headers = request.getHttpHeaders(); - Cookie cookie = headers.getCookies().get(KEYCLOAK_STATE_CHECKER); - if (cookie == null) { - throw new ForbiddenException(); - } - - String cookieStateChecker = cookie.getValue(); + String cookieStateChecker = session.getProvider(CookieProvider.class).get(CookieType.WELCOME_STATE_CHECKER); if (cookieStateChecker == null || !cookieStateChecker.equals(formStateChecker)) { throw new ForbiddenException(); diff --git a/services/src/main/resources/META-INF/services/org.keycloak.cookie.CookieProviderFactory b/services/src/main/resources/META-INF/services/org.keycloak.cookie.CookieProviderFactory new file mode 100644 index 0000000000..ecc46e0217 --- /dev/null +++ b/services/src/main/resources/META-INF/services/org.keycloak.cookie.CookieProviderFactory @@ -0,0 +1 @@ +org.keycloak.cookie.DefaultCookieProviderFactory \ No newline at end of file diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/i18n/LoginPageTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/i18n/LoginPageTest.java index 1b3d0b45c4..e442106c3e 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/i18n/LoginPageTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/i18n/LoginPageTest.java @@ -16,13 +16,9 @@ */ package org.keycloak.testsuite.i18n; -import java.io.IOException; -import java.net.URI; -import java.net.URISyntaxException; -import java.util.Arrays; -import java.util.Locale; - +import jakarta.ws.rs.core.Response; import org.apache.http.impl.client.CloseableHttpClient; +import org.jboss.arquillian.graphene.page.Page; import org.jboss.resteasy.client.jaxrs.ResteasyClient; import org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder; import org.jboss.resteasy.client.jaxrs.engines.ApacheHttpClient43Engine; @@ -33,6 +29,8 @@ import org.keycloak.OAuth2Constants; import org.keycloak.adapters.HttpClientBuilder; import org.keycloak.admin.client.resource.UserResource; import org.keycloak.common.util.KeycloakUriBuilder; +import org.keycloak.cookie.CookieType; +import org.keycloak.cookie.DefaultCookieProvider; import org.keycloak.events.Details; import org.keycloak.events.EventType; import org.keycloak.forms.login.freemarker.DetachedInfoStateChecker; @@ -46,18 +44,21 @@ import org.keycloak.testsuite.pages.AppPage; import org.keycloak.testsuite.pages.ErrorPage; import org.keycloak.testsuite.pages.LanguageComboboxAwarePage; import org.keycloak.testsuite.pages.LoginPage; - -import jakarta.ws.rs.core.Response; -import org.jboss.arquillian.graphene.page.Page; import org.keycloak.testsuite.pages.LoginPasswordUpdatePage; import org.keycloak.testsuite.pages.OAuthGrantPage; import org.keycloak.testsuite.util.IdentityProviderBuilder; import org.openqa.selenium.Cookie; +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.Arrays; +import java.util.Locale; + import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.Assert.assertEquals; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.not; +import static org.junit.Assert.assertEquals; /** * @author Michael Gerber @@ -231,7 +232,7 @@ public class LoginPageTest extends AbstractI18NTest { assertEquals("Deutsch", loginPage.getLanguageDropdownText()); - Cookie localeCookie = driver.manage().getCookieNamed(LocaleSelectorProvider.LOCALE_COOKIE); + Cookie localeCookie = driver.manage().getCookieNamed(CookieType.KEYCLOAK_LOCALE.name()); assertEquals("de", localeCookie.getValue()); UserResource user = ApiUtil.findUserByUsernameId(testRealm(), "test-user@localhost"); @@ -276,7 +277,7 @@ public class LoginPageTest extends AbstractI18NTest { loginPage.open(); // Cookie should be removed as last user to login didn't have a locale - localeCookie = driver.manage().getCookieNamed(LocaleSelectorProvider.LOCALE_COOKIE); + localeCookie = driver.manage().getCookieNamed(CookieType.KEYCLOAK_LOCALE.name()); Assert.assertNull(localeCookie); }