8000 Remove legacy cookies entirely by jonkoops · Pull Request #16734 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

Remove legacy cookies entirely #16734

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jonkoops wants to merge 1 commit into from
Closed

Remove legacy cookies entirely #16734

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8000
28 changes: 1 addition & 27 deletions services/src/main/java/org/keycloak/services/util/CookieHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@
*/
public class CookieHelper {

public static final String LEGACY_COOKIE = "_LEGACY";

private static final Logger logger = Logger.getLogger(CookieHelper.class);

/**
Expand Down Expand Up @@ -71,11 +69,6 @@ public static void addCookie(String name, String value, String path, String doma
ServerCookie.appendCookieValue(cookieBuf, 1, name, value, path, domain, comment, maxAge, secure_sameSite, httpOnly, sameSite);
String cookie = cookieBuf.toString();
response.addHeader(HttpHeaders.SET_COOKIE, cookie);

// a workaround for browser in older Apple OSs – browsers ignore cookies with SameSite=None
if (sameSiteParam == SameSiteAttributeValue.NONE) {
addCookie(name + LEGACY_COOKIE, value, path, domain, comment, maxAge, secure, httpOnly, null, session);
}
}

/**
Expand All @@ -95,16 +88,6 @@ public static void addCookie(String name, String value, String path, String doma


public static Set<String> getCookieValue(KeycloakSession session, String name) {
Set<String> ret = getInternalCookieValue(session, name);
if (ret.size() == 0) {
String legacy = name + LEGACY_COOKIE;
logger.debugv("Could not find any cookies with name '{0}', trying '{1}'", name, legacy);
ret = getInternalCookieValue(session, legacy);
}
return ret;
}

private static Set<String> getInternalCookieValue(KeycloakSession session, String name) {
HttpHeaders headers = session.getContext().getHttpRequest().getHttpHeaders();
Set<String> cookiesVal = new HashSet<>();

Expand All @@ -122,7 +105,6 @@ private static Set<String> getInternalCookieValue(KeycloakSession session, Strin
return cookiesVal;
}


public static Set<String> parseCookie(String header, String name) {
if (header == null || name == null) {
return Collections.emptySet();
Expand All @@ -141,14 +123,6 @@ public static Set<String> parseCookie(String header, String name) {
}

public static Cookie getCookie(Map<String, Cookie> cookies, String name) {
Cookie cookie = cookies.get(name);
if (cookie != null) {
return cookie;
}
else {
String legacy = name + LEGACY_COOKIE;
logger.debugv("Could not find cookie {0}, trying {1}", name, legacy);
return cookies.get(legacy);
}
return cookies.get(name);
}
}
6 changes: 1 addition & 5 deletions services/src/main/resources/org/keycloak/protocol/oidc/endpoints/login-status-iframe.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,7 @@
}

function getCookie() {
var cookie = getCookieByName("KEYCLOAK_SESSION");
if (cookie === null) {
return getCookieByName("KEYCLOAK_SESSION_LEGACY");
}
return cookie;
return getCookieByName("KEYCLOAK_SESSION");
}

function getCookieByName(name) {
Expand Down
3 changes: 1 addition & 2 deletions ...c/main/java/org/keycloak/testsuite/arquillian/undertow/lb/SimpleUndertowLoadBalancer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
import java.util.StringTokenizer;

import static org.keycloak.services.managers.AuthenticationSessionManager.AUTH_SESSION_ID;
import static org.keycloak.services.util.CookieHelper.LEGACY_COOKIE;

/**
* Loadbalancer on embedded undertow. Supports sticky session over "AUTH_SESSION_ID" cookie and failover to different node when sticky node not available.
Expand Down Expand Up @@ -177,7 +176,7 @@ static Map<String, URI> parseNodes(String nodes) {
private HttpHandler createHandler() throws Exception {

// TODO: configurable options if needed
String[] sessionIds = {AUTH_SESSION_ID, AUTH_SESSION_ID + LEGACY_COOKIE};
String[] sessionIds = {AUTH_SESSION_ID};
int connectionsPerThread = 20;
int problemServerRetry = 5; // In case of unavailable node, we will try to ping him every 5 seconds to check if it's back
int maxTime = 3600000; // 1 hour for proxy request timeout, so we can debug the backend keycloak servers
Expand Down
4 changes: 1 addition & 3 deletions ...illian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerLogoutTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,8 @@ public void logoutAfterBrowserRestart() {
String idToken = response.getIdToken();

// simulate browser restart by deleting an identity cookie
log.debugf("Deleting %s and %s cookies", AuthenticationManager.KEYCLOAK_IDENTITY_COOKIE,
AuthenticationManager.KEYCLOAK_IDENTITY_COOKIE + CookieHelper.LEGACY_COOKIE);
log.debugf("Deleting %s cookie", AuthenticationManager.KEYCLOAK_IDENTITY_COOKIE);
driver.manage().deleteCookieNamed(AuthenticationManager.KEYCLOAK_IDENTITY_COOKIE);
driver.manage().deleteCookieNamed(AuthenticationManager.KEYCLOAK_IDENTITY_COOKIE + CookieHelper.LEGACY_COOKIE);

logoutFromRealm(getConsumerRoot(), bc.consumerRealmName(), null, idToken);
driver.navigate().to(getAccountUrl(getProviderRoot(), REALM_PROV_NAME));
Expand Down
3 changes: 0 additions & 3 deletions ...rc/test/java/org/keycloak/testsuite/cluster/AuthenticationSessionFailoverClusterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,6 @@ protected void failoverTest(boolean expectSuccessfulFailover) throws IOException

public static String getAuthSessionCookieValue(WebDriver driver) {
Cookie authSessionCookie = driver.manage().getCookieNamed(AuthenticationSessionManager.AUTH_SESSION_ID);
if (authSessionCookie == null) {
authSessionCookie = driver.manage().getCookieNamed(AuthenticationSessionManager.AUTH_SESSION_ID + CookieHelper.LEGACY_COOKIE);
}
Assert.assertNotNull(authSessionCookie);
return authSessionCookie.getValue();
}
Expand Down
27 changes: 0 additions & 27 deletions ...ration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cookies/CookieTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
import static org.keycloak.services.managers.AuthenticationManager.KEYCLOAK_IDENTITY_COOKIE;
import static org.keycloak.services.managers.AuthenticationManager.KEYCLOAK_SESSION_COOKIE;
import static org.keycloak.services.managers.AuthenticationSessionManager.AUTH_SESSION_ID;
import static org.keycloak.services.util.CookieHelper.LEGACY_COOKIE;
import static org.keycloak.testsuite.admin.AbstractAdminTest.loadJson;
import static org.keycloak.testsuite.util.URLAssert.assertCurrentUrlStartsWithLoginUrlOf;

Expand Down Expand Up @@ -86,11 +85,6 @@ public void testCookieValue() throws Exception {
testCookieValue(KEYCLOAK_IDENTITY_COOKIE);
}

@Test
public void testLegacyCookieValue() throws Exception {
testCookieValue(KEYCLOAK_IDENTITY_COOKIE + LEGACY_COOKIE);
}

private void testCookieValue(String cookieName) throws Exception {
final String accountClientId = realmsResouce().realm("test").clients().findByClientId("account").get(0).getId();
final String clientSecret = realmsResouce().realm("test").clients().get(accountClientId).getSecret().getValue();
Expand Down Expand Up @@ -165,27 +159,6 @@ public void testCookieValueLoggedOut() throws Exception {
}
}

@Test
public void legacyCookiesTest() {
ContainerAssume.assumeAuthServerSSL();

accountPage.navigateTo();
assertCurrentUrlStartsWithLoginUrlOf(accountPage);

loginPage.login("test-user@localhost", "password");

Cookie sameSiteIdentityCookie = driver.manage().getCookieNamed(KEYCLOAK_IDENTITY_COOKIE);
Cookie legacyIdentityCookie = driver.manage().getCookieNamed(KEYCLOAK_IDENTITY_COOKIE + LEGACY_COOKIE);
Cookie sameSiteSessionCookie = driver.manage().getCookieNamed(KEYCLOAK_SESSION_COOKIE);
Cookie legacySessionCookie = driver.manage().getCookieNamed(KEYCLOAK_SESSION_COOKIE + LEGACY_COOKIE);
Cookie sameSiteAuthSessionIdCookie = driver.manage().getCookieNamed(AUTH_SESSION_ID);
Cookie legacyAuthSessionIdCookie = driver.manage().getCookieNamed(AUTH_SESSION_ID + LEGACY_COOKIE);

assertSameSiteCookies(sameSiteIdentityCookie, legacyIdentityCookie);
assertSameSiteCookies(sameSiteSessionCookie, legacySessionCookie);
assertSameSiteCookies(sameSiteAuthSessionIdCookie, legacyAuthSessionIdCookie);
}

private void assertSameSiteCookies(Cookie sameSiteCookie, Cookie legacyCookie) {
assertNotNull("SameSite cookie shouldn't be null", sameSiteCookie);
assertNotNull("Legacy cookie shouldn't be null", legacyCookie);
Expand Down
6 changes: 0 additions & 6 deletions ...n/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@
import static org.junit.Assert.fail;
import static org.keycloak.models.UserModel.RequiredAction.UPDATE_PROFILE;
import static org.keycloak.services.managers.AuthenticationManager.KEYCLOAK_SESSION_COOKIE;
import static org.keycloak.services.util.CookieHelper.LEGACY_COOKIE;
import static org.keycloak.storage.UserStorageProviderModel.CACHE_POLICY;
import static org.keycloak.storage.UserStorageProviderModel.EVICTION_DAY;
import static org.keycloak.storage.UserStorageProviderModel.EVICTION_HOUR;
Expand Down Expand Up @@ -254,16 +253,11 @@ public void testLoginSuccessWithSpecialCharacter() {
69DA assertCurrentUrlStartsWith(testRealmAccountPage);

Cookie sameSiteSessionCookie = driver.manage().getCookieNamed(KEYCLOAK_SESSION_COOKIE);
Cookie legacySessionCookie = driver.manage().getCookieNamed(KEYCLOAK_SESSION_COOKIE + LEGACY_COOKIE);

String cookieValue = sameSiteSessionCookie.getValue();
Assert.assertThat(cookieValue.contains("spécial"), is(false));
Assert.assertThat(cookieValue.contains("sp%C3%A9cial"), is(true));

String legacyCookieValue = legacySessionCookie.getValue();
Assert.assertThat(legacyCookieValue.contains("spécial"), is(false));
Assert.assertThat(legacyCookieValue.contains("sp%C3%A9cial"), is(true));

testRealmAccountPage.logOut();
}

Expand Down
0