Revert new behaviour around setting secure flag for cookies (#26650)

Closes #26649

Signed-off-by: stianst <stianst@gmail.com>
This commit is contained in:
Stian Thorgersen 2024-01-31 19:33:56 +01:00 committed by GitHub
parent 37acb2fd09
commit 64b5f42c4a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 12 additions and 22 deletions

View file

@ -48,7 +48,7 @@ public class DefaultCookieProvider implements CookieProvider {
HttpCookie newCookie = new HttpCookie(1, name, value, path, null, null, maxAge, secure, httpOnly, sameSite); HttpCookie newCookie = new HttpCookie(1, name, value, path, null, null, maxAge, secure, httpOnly, sameSite);
context.getHttpResponse().setCookieIfAbsent(newCookie); context.getHttpResponse().setCookieIfAbsent(newCookie);
logger.tracef("Setting cookie: name: %s, path: %s, same-site: %s, http-only: %s, max-age: %d", name, path, sameSite, httpOnly, maxAge); logger.tracef("Setting cookie: name: %s, path: %s, same-site: %s, secure: %s, http-only: %s, max-age: %d", name, path, sameSite, secure, httpOnly, maxAge);
if (legacyCookiesEnabled && cookieType.supportsSameSiteLegacy()) { if (legacyCookiesEnabled && cookieType.supportsSameSiteLegacy()) {
if (ServerCookie.SameSiteAttributeValue.NONE.equals(sameSite)) { if (ServerCookie.SameSiteAttributeValue.NONE.equals(sameSite)) {
@ -57,7 +57,7 @@ public class DefaultCookieProvider implements CookieProvider {
HttpCookie legacyCookie = new HttpCookie(1, legacyName, value, path, null, null, maxAge, secure, httpOnly, null); HttpCookie legacyCookie = new HttpCookie(1, legacyName, value, path, null, null, maxAge, secure, httpOnly, null);
context.getHttpResponse().setCookieIfAbsent(legacyCookie); context.getHttpResponse().setCookieIfAbsent(legacyCookie);
logger.tracef("Setting legacy cookie: name: %s, path: %s, same-site: %s, http-only: %s, max-age: %d", legacyName, path, sameSite, httpOnly, maxAge); logger.tracef("Setting legacy cookie: name: %s, path: %s, same-site: %s, secure: %s, http-only: %s, max-age: %d", legacyName, path, sameSite, secure, httpOnly, maxAge);
} }
} else { } else {
expireLegacy(cookieType); expireLegacy(cookieType);
@ -127,16 +127,6 @@ public class DefaultCookieProvider implements CookieProvider {
return true; return true;
} }
if ("https".equals(requestUri.getScheme())) {
return true;
}
// Browsers consider 127.0.0.1, localhost and *.localhost as secure contexts
String frontendHostname = context.getUri(UrlType.FRONTEND).getRequestUri().getHost();
if (frontendHostname.equals("127.0.0.1") || frontendHostname.equals("localhost") || frontendHostname.endsWith(".localhost")) {
return true;
}
return false; return false;
} }

View file

@ -53,15 +53,15 @@ public class DefaultCookieProviderTest extends AbstractKeycloakTest {
cookies.set(CookieType.WELCOME_CSRF, "my-welcome-csrf"); cookies.set(CookieType.WELCOME_CSRF, "my-welcome-csrf");
}); });
Assert.assertEquals(12, response.getCookies().size()); Assert.assertEquals(12, response.getCookies().size());
assertCookie(response, "AUTH_SESSION_ID", "my-auth-session-id", "/auth/realms/master/", -1, true, true, "None", true); assertCookie(response, "AUTH_SESSION_ID", "my-auth-session-id", "/auth/realms/master/", -1, false, true, "None", true);
assertCookie(response, "KC_AUTH_STATE", "my-auth-state", "/auth/realms/master/", 111, true, false, null, false); assertCookie(response, "KC_AUTH_STATE", "my-auth-state", "/auth/realms/master/", 111, false, false, null, false);
assertCookie(response, "KC_RESTART", "my-auth-restart", "/auth/realms/master/", -1, true, true, null, false); assertCookie(response, "KC_RESTART", "my-auth-restart", "/auth/realms/master/", -1, false, true, null, false);
assertCookie(response, "KC_STATE_CHECKER", "my-auth-detached", "/auth/realms/master/", 222, true, true, null, false); assertCookie(response, "KC_STATE_CHECKER", "my-auth-detached", "/auth/realms/master/", 222, false, true, null, false);
assertCookie(response, "KEYCLOAK_IDENTITY", "my-identity", "/auth/realms/master/", 333, true, true, "None", true); assertCookie(response, "KEYCLOAK_IDENTITY", "my-identity", "/auth/realms/master/", 333, false, true, "None", true);
assertCookie(response, "KEYCLOAK_LOCALE", "my-locale", "/auth/realms/master/", -1, true, true, null, false); assertCookie(response, "KEYCLOAK_LOCALE", "my-locale", "/auth/realms/master/", -1, false, true, null, false);
assertCookie(response, "KEYCLOAK_REMEMBER_ME", "my-username", "/auth/realms/master/", 31536000, true, true, null, false); assertCookie(response, "KEYCLOAK_REMEMBER_ME", "my-username", "/auth/realms/master/", 31536000, false, true, null, false);
assertCookie(response, "KEYCLOAK_SESSION", "my-session", "/auth/realms/master/", 444, true, false, "None", true); assertCookie(response, "KEYCLOAK_SESSION", "my-session", "/auth/realms/master/", 444, false, false, "None", true);
assertCookie(response, "WELCOME_STATE_CHECKER", "my-welcome-csrf", "/auth/realms/master/testing/run-on-server", 300, true, true, "Strict", false); assertCookie(response, "WELCOME_STATE_CHECKER", "my-welcome-csrf", "/auth/realms/master/testing/run-on-server", 300, false, true, "Strict", false);
} }
@Test @Test
@ -125,7 +125,7 @@ public class DefaultCookieProviderTest extends AbstractKeycloakTest {
Assert.assertEquals(value, cookie.getValue()); Assert.assertEquals(value, cookie.getValue());
Assert.assertEquals(path, cookie.getPath()); Assert.assertEquals(path, cookie.getPath());
Assert.assertEquals(maxAge, cookie.getMaxAge()); Assert.assertEquals(maxAge, cookie.getMaxAge());
Assert.assertEquals(secure, cookie.isSecure()); Assert.assertEquals(secure || "None".equals(sameSite), cookie.isSecure());
Assert.assertEquals(httpOnly, cookie.isHttpOnly()); Assert.assertEquals(httpOnly, cookie.isHttpOnly());
String setHeader = (String) response.getHeaders().get("Set-Cookie").stream().filter(v -> ((String) v).startsWith(name)).findFirst().get(); String setHeader = (String) response.getHeaders().get("Set-Cookie").stream().filter(v -> ((String) v).startsWith(name)).findFirst().get();