Skip to content

GH-1328: Added SameSite support for session cookie. #1369

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

Open
Alexandermjos wants to merge 5 commits into
base: master
Choose a base branch
from
Open

GH-1328: Added SameSite support for session cookie. #1369

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0a8cb43
GH-1328: Added SameSite support for session cookie.
Alexandermjos Jan 5, 2022
6d0d0cc
Resolves #1328: Refactored sameSite value from String to Enum. Dont s…
Alexandermjos Jan 30, 2022
16d7f92
Merge branch 'master' into 1328-sync-1.8.0-v2
Alexandermjos Nov 3, 2022
183053a
Update unittest after syncing feature branch from master
Alexandermjos Nov 3, 2022
7f25a12
GH-1328: Change casing after feedback from pull request
Alexandermjos Nov 3, 2022
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
4 changes: 2 additions & 2 deletions framework/src/play/data/validation/ValidationPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ static void save() {
if (Validation.errors().isEmpty()) {
// Only send "delete cookie" header when the cookie was present in the request
if(Http.Request.current().cookies.containsKey(Scope.COOKIE_PREFIX + "_ERRORS") || !Scope.SESSION_SEND_ONLY_IF_CHANGED) {
Http.Response.current().setCookie(Scope.COOKIE_PREFIX + "_ERRORS", "", null, "/", 0, Scope.COOKIE_SECURE, Scope.SESSION_HTTPONLY);
Http.Response.current().setCookie(Scope.COOKIE_PREFIX + "_ERRORS", "", null, "/", 0, Scope.COOKIE_SECURE, Scope.SESSION_HTTPONLY, null);
}
return;
}
Expand All @@ -179,7 +179,7 @@ static void save() {
}
}
String errorsData = URLEncoder.encode(errors.toString(), "utf-8");
Http.Response.current().setCookie(Scope.COOKIE_PREFIX + "_ERRORS", errorsData, null, "/", null, Scope.COOKIE_SECURE, Scope.SESSION_HTTPONLY);
Http.Response.current().setCookie(Scope.COOKIE_PREFIX + "_ERRORS", errorsData, null, "/", null, Scope.COOKIE_SECURE, Scope.SESSION_HTTPONLY, null);
} catch (Exception e) {
throw new UnexpectedException("Errors serializationProblem", e);
}
Expand Down
4 changes: 2 additions & 2 deletions framework/src/play/i18n/Lang.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public static void change(String locale) {
Response response = Response.current();
if (response != null) {
// We have a current response in scope - set the language-cookie to store the selected language for the next requests
response.setCookie(Play.configuration.getProperty("application.lang.cookie", "PLAY_LANG"), locale, null, "/", null, Scope.COOKIE_SECURE);
response.setCookie(Play.configuration.getProperty("application.lang.cookie", "PLAY_LANG"), locale, null, "/", null, Scope.COOKIE_SECURE, Scope.SESSION_SAMESITE);
}
}

Expand Down Expand Up @@ -155,7 +155,7 @@ private static void resolveFrom(Request request) {
return;
}
// could not use locale from cookie - clear the locale-cookie
Response.current().setCookie(cn, "", null, "/", null, Scope.COOKIE_SECURE);
Response.current().setCookie(cn, "", null, "/", null, Scope.COOKIE_SECURE, Scope.SESSION_SAMESITE);

}

Expand Down
6 changes: 3 additions & 3 deletions framework/src/play/mvc/CookieSessionStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public void save(Session session) {
if (session.isEmpty()) {
// The session is empty: delete the cookie
if (Http.Request.current().cookies.containsKey(COOKIE_PREFIX + "_SESSION") || !SESSION_SEND_ONLY_IF_CHANGED) {
Http.Response.current().setCookie(COOKIE_PREFIX + "_SESSION", "", null, "/", 0, COOKIE_SECURE, SESSION_HTTPONLY);
Http.Response.current().setCookie(COOKIE_PREFIX + "_SESSION", "", null, "/", 0, COOKIE_SECURE, SESSION_HTTPONLY, SESSION_SAMESITE);
}
return;
}
Expand All @@ -84,10 +84,10 @@ public void save(Session session) {
String sign = Crypto.sign(sessionData, Play.secretKey.getBytes());
if (COOKIE_EXPIRE == null) {
Http.Response.current().setCookie(COOKIE_PREFIX + "_SESSION", sign + "-" + sessionData, null, "/", null, COOKIE_SECURE,
SESSION_HTTPONLY);
SESSION_HTTPONLY, SESSION_SAMESITE);
} else {
Http.Response.current().setCookie(COOKIE_PREFIX + "_SESSION", sign + "-" + sessionData, null, "/",
Time.parseDuration(COOKIE_EXPIRE), COOKIE_SECURE, SESSION_HTTPONLY);
Time.parseDuration(COOKIE_EXPIRE), COOKIE_SECURE, SESSION_HTTPONLY, SESSION_SAMESITE);
}
} catch (Exception e) {
throw new UnexpectedException("Session serializationProblem", e);
Expand Down
38 changes: 29 additions & 9 deletions framework/src/play/mvc/Http.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import java.io.InputStream;
import java.io.Serializable;
import java.lang.reflect.Method;
import java.text.ParseException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -163,6 +162,10 @@ public static class Cookie implements Serializable {
* See http://www.owasp.org/index.php/HttpOnly
*/
public boolean httpOnly = false;
/**
* See https://owasp.org/www-community/SameSite
*/
public SAMESITE sameSite;
}

/**
Expand Down Expand Up @@ -707,8 +710,8 @@ public void setContentTypeIfNotSet(String contentType) {
* @param value
* Cookie value
*/
public void setCookie(String name, String value) {
setCookie(name, value, null, "/", null, false);
public void setCookie(String name, String value, SAMESITE sameSite) {
setCookie(name, value, null, "/", null, false, sameSite);
}

/**
Expand All @@ -730,7 +733,7 @@ public void removeCookie(String name) {
* cookie path
*/
public void removeCookie(String name, String path) {
setCookie(name, "", null, path, 0, false);
setCookie(name, "", null, path, 0, false, null);
}

/**
Expand All @@ -743,15 +746,15 @@ public void removeCookie(String name, String path) {
* @param duration
* the cookie duration (Ex: 3d)
*/
public void setCookie(String name, String value, String duration) {
setCookie(name, value, null, "/", Time.parseDuration(duration), false);
public void setCookie(String name, String value, String duration, SAMESITE sameSite) {
setCookie(name, value, null, "/", Time.parseDuration(duration), false, sameSite);
}

public void setCookie(String name, String value, String domain, String path, Integer maxAge, boolean secure) {
setCookie(name, value, domain, path, maxAge, secure, false);
public void setCookie(String name, String value, String domain, String path, Integer maxAge, boolean secure, SAMESITE sameSite) {
setCookie(name, value, domain, path, maxAge, secure, false, sameSite);
}

public void setCookie(String name, String value, String domain, String path, Integer maxAge, boolean secure, boolean httpOnly) {
public void setCookie(String name, String value, String domain, String path, Integer maxAge, boolean secure, boolean httpOnly, SAMESITE sameSite) {
path = Play.ctxPath + path;
if (cookies.containsKey(name) && cookies.get(name).path.equals(path)
&& ((cookies.get(name).domain == null && domain == null) || (cookies.get(name).domain.equals(domain)))) {
Expand All @@ -765,6 +768,7 @@ public void setCookie(String name, String value, String domain, String path, Int
cookie.path = path;
cookie.secure = secure;
cookie.httpOnly = httpOnly;
cookie.sameSite = sameSite;
if (domain != null) {
cookie.domain = domain;
} else {
Expand Down Expand Up @@ -1018,4 +1022,20 @@ public WebSocketFrame(byte[] data) {

public static class WebSocketClose extends WebSocketEvent {
}

public enum SAMESITE {
Copy link
Copy Markdown
Contributor

@asolntsev asolntsev Feb 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably a matter of taste, but I would prefer SameSite name. Upper case doesn't make it readable.

Copy link
Copy Markdown
Author

@Alexandermjos Alexandermjos Nov 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated the casing to SameSite

STRICT("Strict"),
LAX("Lax"),
NONE("None");

private final String value;

SAMESITE(String value) {
this.value = value;
}

public String getValue() {
return value;
}
}
}
6 changes: 4 additions & 2 deletions framework/src/play/mvc/Scope.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ public class Scope {
.equals("true");
public static boolean SESSION_SEND_ONLY_IF_CHANGED = Play.configuration
.getProperty("application.session.sendOnlyIfChanged", "false").toLowerCase().equals("true");
public static final Http.SAMESITE SESSION_SAMESITE = Play.configuration.getProperty("application.session.cookie.sameSite") != null ?
Http.SAMESITE.valueOf(Play.configuration.getProperty("application.session.cookie.sameSite").toUpperCase()) : null;

public static SessionStore sessionStore = createSessionStore();

Expand Down Expand Up @@ -78,13 +80,13 @@ void save() {
}
if (out.isEmpty()) {
if (Http.Request.current().cookies.containsKey(COOKIE_PREFIX + "_FLASH") || !SESSION_SEND_ONLY_IF_CHANGED) {
Http.Response.current().setCookie(COOKIE_PREFIX + "_FLASH", "", null, "/", 0, COOKIE_SECURE, SESSION_HTTPONLY);
Http.Response.current().setCookie(COOKIE_PREFIX + "_FLASH", "", null, "/", 0, COOKIE_SECURE, SESSION_HTTPONLY, SESSION_SAMESITE);
}
return;
}
try {
String flashData = CookieDataCodec.encode(out);
Http.Response.current().setCookie(COOKIE_PREFIX + "_FLASH", flashData, null, "/", null, COOKIE_SECURE, SESSION_HTTPONLY);
Http.Response.current().setCookie(COOKIE_PREFIX + "_FLASH", flashData, null, "/", null, COOKIE_SECURE, SESSION_HTTPONLY, SESSION_SAMESITE);
} catch (Exception e) {
throw new UnexpectedException("Flash serializationProblem", e);
}
Expand Down
14 changes: 10 additions & 4 deletions framework/src/play/server/PlayHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;

import java.text.ParseException;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentLinkedQueue;
Expand Down Expand Up @@ -375,7 +374,11 @@ protected static void addToResponse(Response response, HttpResponse nettyRespons
c.setMaxAge(cookie.maxAge);
}
c.setHttpOnly(cookie.httpOnly);
nettyResponse.headers().add(SET_COOKIE, ServerCookieEncoder.STRICT.encode(c));
String encodedCookie = ServerCookieEncoder.STRICT.encode(c);
if (cookie.sameSite != null) {
encodedCookie += "; SameSite=" + cookie.sameSite.getValue();
}
nettyResponse.headers().add(SET_COOKIE, encodedCookie);
}

if (!response.headers.containsKey(CACHE_CONTROL) && !response.headers.containsKey(EXPIRES)
Expand Down Expand Up @@ -792,8 +795,11 @@ public static void serve500(Exception e, ChannelHandlerContext ctx, HttpRequest
c.setMaxAge(cookie.maxAge);
}
c.setHttpOnly(cookie.httpOnly);

nettyResponse.headers().add(SET_COOKIE, ServerCookieEncoder.STRICT.encode(c));
String encodedCookie = ServerCookieEncoder.STRICT.encode(c);
if (cookie.sameSite != null) {
encodedCookie += "; SameSite=" + cookie.sameSite.getValue();
}
nettyResponse.headers().add(SET_COOKIE, encodedCookie);
}

} catch (Exception exx) {
Expand Down
22 changes: 20 additions & 2 deletions framework/test-src/play/mvc/HttpResponseTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,30 @@ public class HttpResponseTest {
public void verifyDefaultCookieDomain() {
Http.Cookie.defaultDomain = null;
Http.Response response = new Http.Response();
response.setCookie("testCookie", "testValue");
response.setCookie("testCookie", "testValue", null);
assertThat(response.cookies.get("testCookie").domain).isNull();

Http.Cookie.defaultDomain = ".abc.com";
response = new Http.Response();
response.setCookie("testCookie", "testValue");
response.setCookie("testCookie", "testValue", null);
assertThat(response.cookies.get("testCookie").domain).isEqualTo(".abc.com");
}

@Test
public void verifySameSiteCookie() {
Http.Cookie.defaultDomain = null;
Http.Response response = new Http.Response();
response.setCookie("testCookie", "testValue", null);
assertThat(response.cookies.get("testCookie").sameSite).isNull();

Http.Cookie.defaultDomain = ".abc.com";
response = new Http.Response();
response.setCookie("testCookie", "testValue", Http.SAMESITE.LAX);
assertThat(response.cookies.get("testCookie").sameSite).isEqualTo(Http.SAMESITE.LAX);

Http.Cookie.defaultDomain = ".abc.com";
response = new Http.Response();
response.setCookie("testCookie", "testValue", Http.SAMESITE.STRICT);
assertThat(response.cookies.get("testCookie").sameSite).isEqualTo(Http.SAMESITE.STRICT);
}
}
1 change: 1 addition & 0 deletions resources/application-skel/conf/application.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ date.format=yyyy-MM-dd
# application.session.cookie=PLAY
# application.session.maxAge=1h
# application.session.secure=false
# application.session.cookie.sameSite=lax

# Session/Cookie sharing between subdomain
# ~~~~~~~~~~~~~~~~~~~~~~
Expand Down