Skip to content

Commit

Permalink
Resolves playframework#1328: Refactored sameSite value from String to…
Browse files Browse the repository at this point in the history
… Enum. Dont set sameSite for ValidationPlugin
  • Loading branch information
Alexandermjos committed Jan 30, 2022
1 parent 0a8cb43 commit 6d0d0cc
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 18 deletions.
4 changes: 2 additions & 2 deletions framework/src/play/data/validation/ValidationPlugin.java
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, Scope.SESSION_SAMESITE);
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, Scope.SESSION_SAMESITE);
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
27 changes: 21 additions & 6 deletions framework/src/play/mvc/Http.java
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 @@ -166,7 +165,7 @@ public static class Cookie implements Serializable {
/**
* See https://owasp.org/www-community/SameSite
*/
public String sameSite;
public SAMESITE sameSite;
}

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

Expand Down Expand Up @@ -747,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, String sameSite) {
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, String sameSite) {
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, String sameSite) {
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 Down Expand Up @@ -1023,4 +1022,20 @@ public WebSocketFrame(byte[] data) {

public static class WebSocketClose extends WebSocketEvent {
}

public enum SAMESITE {
STRICT("Strict"),
LAX("Lax"),
NONE("None");

private final String value;

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

public String getValue() {
return value;
}
}
}
3 changes: 2 additions & 1 deletion framework/src/play/mvc/Scope.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +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 String SESSION_SAMESITE = Play.configuration.getProperty("application.session.sameSite");
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
8 changes: 4 additions & 4 deletions framework/src/play/server/PlayHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -375,8 +375,8 @@ protected static void addToResponse(Response response, HttpResponse nettyRespons
}
c.setHttpOnly(cookie.httpOnly);
String encodedCookie = ServerCookieEncoder.STRICT.encode(c);
if(cookie.sameSite != null) {
encodedCookie += "; SameSite=" + cookie.sameSite;
if (cookie.sameSite != null) {
encodedCookie += "; SameSite=" + cookie.sameSite.getValue();
}
nettyResponse.headers().add(SET_COOKIE, encodedCookie);
}
Expand Down Expand Up @@ -796,8 +796,8 @@ public static void serve500(Exception e, ChannelHandlerContext ctx, HttpRequest
}
c.setHttpOnly(cookie.httpOnly);
String encodedCookie = ServerCookieEncoder.STRICT.encode(c);
if(cookie.sameSite != null) {
encodedCookie += "; SameSite=" + cookie.sameSite;
if (cookie.sameSite != null) {
encodedCookie += "; SameSite=" + cookie.sameSite.getValue();
}
nettyResponse.headers().add(SET_COOKIE, encodedCookie);
}
Expand Down
8 changes: 4 additions & 4 deletions framework/test-src/play/mvc/HttpResponseTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ public void verifySameSiteCookie() {

Http.Cookie.defaultDomain = ".abc.com";
response = new Http.Response();
response.setCookie("testCookie", "testValue", "lax");
assertThat(response.cookies.get("testCookie").sameSite).isEqualTo("lax");
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", "strict");
assertThat(response.cookies.get("testCookie").sameSite).isEqualTo("strict");
response.setCookie("testCookie", "testValue", Http.SAMESITE.STRICT);
assertThat(response.cookies.get("testCookie").sameSite).isEqualTo(Http.SAMESITE.STRICT);
}
}
2 changes: 1 addition & 1 deletion resources/application-skel/conf/application.conf
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ date.format=yyyy-MM-dd
# application.session.cookie=PLAY
# application.session.maxAge=1h
# application.session.secure=false
# application.session.sameSite=lax
# application.session.cookie.sameSite=lax

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

0 comments on commit 6d0d0cc

Please sign in to comment.