SONAR-15513 Fix SSF-153

5 files changed, 53 insertions, 11 deletions

diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/Cookies.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/Cookies.java
index 022d015973c..7fbdfc30112 100644
--- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/Cookies.java
+++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/Cookies.java
@@ -34,6 +34,8 @@ import static java.util.Objects.requireNonNull;
  * The {@link javax.servlet.http.Cookie#setSecure(boolean)} will automatically be set to true.
  */
 public class Cookies {
+  public static final String SET_COOKIE = "Set-Cookie";
+  public static final String SAMESITE_LAX = "Lax";
 
   private static final String HTTPS_HEADER = "X-Forwarded-Proto";
   private static final String HTTPS_VALUE = "https";
@@ -65,6 +67,8 @@ public class Cookies {
     private boolean httpOnly;
     private int expiry;
 
+    private String sameSite;
+
     CookieBuilder(HttpServletRequest request) {
       this.request = request;
     }
@@ -86,6 +90,14 @@ public class Cookies {
     }
 
     /**
+     * SameSite attribute, only work for toString()
+     */
+    public CookieBuilder setSameSite(@Nullable String sameSite) {
+      this.sameSite = sameSite;
+      return this;
+    }
+
+    /**
      * Sets the flag that controls if this cookie will be hidden from scripts on the client side.
      */
     public CookieBuilder setHttpOnly(boolean httpOnly) {
@@ -110,6 +122,17 @@ public class Cookies {
       return cookie;
     }
 
+    public String toValueString() {
+      String output = String.format("%s=%s; Path=%s; SameSite=%s; Max-Age=%d", name, value, getContextPath(request), sameSite, expiry);
+      if (httpOnly) {
+        output += "; HttpOnly";
+      }
+      if (isHttps(request)) {
+        output += "; Secure";
+      }
+      return output;
+    }
+
     private static boolean isHttps(HttpServletRequest request) {
       return HTTPS_VALUE.equalsIgnoreCase(request.getHeader(HTTPS_HEADER));
     }
diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/JwtCsrfVerifier.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/JwtCsrfVerifier.java
index c42acda4728..dddba6c2beb 100644
--- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/JwtCsrfVerifier.java
+++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/JwtCsrfVerifier.java
@@ -31,6 +31,8 @@ import org.apache.commons.lang.StringUtils;
 import org.sonar.server.authentication.event.AuthenticationException;
 
 import static org.apache.commons.lang.StringUtils.isBlank;
+import static org.sonar.server.authentication.Cookies.SAMESITE_LAX;
+import static org.sonar.server.authentication.Cookies.SET_COOKIE;
 import static org.sonar.server.authentication.Cookies.newCookieBuilder;
 import static org.sonar.server.authentication.event.AuthenticationEvent.Method;
 import static org.sonar.server.authentication.event.AuthenticationEvent.Source;
@@ -47,7 +49,13 @@ public class JwtCsrfVerifier {
     // Create a state token to prevent request forgery.
     // Store it in the cookie for later validation.
     String state = new BigInteger(130, new SecureRandom()).toString(32);
-    response.addCookie(newCookieBuilder(request).setName(CSRF_STATE_COOKIE).setValue(state).setHttpOnly(false).setExpiry(timeoutInSeconds).build());
+    response.addHeader(SET_COOKIE, newCookieBuilder(request)
+      .setName(CSRF_STATE_COOKIE)
+      .setValue(state)
+      .setHttpOnly(false)
+      .setExpiry(timeoutInSeconds)
+      .setSameSite(SAMESITE_LAX)
+      .toValueString());
     return state;
   }
 
@@ -78,7 +86,14 @@ public class JwtCsrfVerifier {
   }
 
   public void refreshState(HttpServletRequest request, HttpServletResponse response, String csrfState, int timeoutInSeconds) {
-    response.addCookie(newCookieBuilder(request).setName(CSRF_STATE_COOKIE).setValue(csrfState).setHttpOnly(false).setExpiry(timeoutInSeconds).build());
+    response.addHeader(SET_COOKIE,
+      newCookieBuilder(request)
+        .setName(CSRF_STATE_COOKIE)
+        .setValue(csrfState)
+        .setHttpOnly(false)
+        .setExpiry(timeoutInSeconds)
+        .setSameSite(SAMESITE_LAX)
+        .toValueString());
   }
 
   public void removeState(HttpServletRequest request, HttpServletResponse response) {
diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/JwtHttpHandler.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/JwtHttpHandler.java
index 3ed502fe0f9..5e42ecca1c9 100644
--- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/JwtHttpHandler.java
+++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/JwtHttpHandler.java
@@ -42,6 +42,8 @@ import static java.util.Objects.requireNonNull;
 import static org.apache.commons.lang.StringUtils.isEmpty;
 import static org.apache.commons.lang.time.DateUtils.addSeconds;
 import static org.sonar.process.ProcessProperties.Property.WEB_SESSION_TIMEOUT_IN_MIN;
+import static org.sonar.server.authentication.Cookies.SAMESITE_LAX;
+import static org.sonar.server.authentication.Cookies.SET_COOKIE;
 import static org.sonar.server.authentication.Cookies.findCookie;
 import static org.sonar.server.authentication.Cookies.newCookieBuilder;
 import static org.sonar.server.authentication.JwtSerializer.LAST_REFRESH_TIME_PARAM;
@@ -92,7 +94,7 @@ public class JwtHttpHandler {
         .put(LAST_REFRESH_TIME_PARAM, system2.now())
         .put(CSRF_JWT_PARAM, csrfState)
         .build()));
-    response.addCookie(createCookie(request, JWT_COOKIE, token, sessionTimeoutInSeconds));
+    response.addHeader(SET_COOKIE, createJwtSession(request, JWT_COOKIE, token, sessionTimeoutInSeconds));
   }
 
   private SessionTokenDto createSessionToken(UserDto user, long expirationTime) {
@@ -176,7 +178,7 @@ public class JwtHttpHandler {
   private void refreshToken(DbSession dbSession, SessionTokenDto tokenFromDb, Claims tokenFromCookie, HttpServletRequest request, HttpServletResponse response) {
     long expirationTime = system2.now() + sessionTimeoutInSeconds * 1000L;
     String refreshToken = jwtSerializer.refresh(tokenFromCookie, expirationTime);
-    response.addCookie(createCookie(request, JWT_COOKIE, refreshToken, sessionTimeoutInSeconds));
+    response.addHeader(SET_COOKIE, createJwtSession(request, JWT_COOKIE, refreshToken, sessionTimeoutInSeconds));
     jwtCsrfVerifier.refreshState(request, response, (String) tokenFromCookie.get(CSRF_JWT_PARAM), sessionTimeoutInSeconds);
 
     dbClient.sessionTokensDao().update(dbSession, tokenFromDb.setExpirationDate(expirationTime));
@@ -208,6 +210,10 @@ public class JwtHttpHandler {
     return newCookieBuilder(request).setName(name).setValue(value).setHttpOnly(true).setExpiry(expirationInSeconds).build();
   }
 
+  private static String createJwtSession(HttpServletRequest request, String name, @Nullable String value, int expirationInSeconds) {
+    return newCookieBuilder(request).setName(name).setValue(value).setHttpOnly(true).setExpiry(expirationInSeconds).setSameSite(SAMESITE_LAX).toValueString();
+  }
+
   private Optional<UserDto> selectUserFromUuid(DbSession dbSession, String userUuid) {
     UserDto user = dbClient.userDao().selectByUuid(dbSession, userUuid);
     return Optional.ofNullable(user != null && user.isActive() ? user : null);
diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/JwtCsrfVerifierTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/JwtCsrfVerifierTest.java
index 5b2e689479d..f1582453ffd 100644
--- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/JwtCsrfVerifierTest.java
+++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/JwtCsrfVerifierTest.java
@@ -33,6 +33,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
+import static org.sonar.server.authentication.Cookies.SET_COOKIE;
 import static org.sonar.server.authentication.event.AuthenticationEvent.Method;
 
 public class JwtCsrfVerifierTest {
@@ -59,8 +60,7 @@ public class JwtCsrfVerifierTest {
     String state = underTest.generateState(request, response, TIMEOUT);
     assertThat(state).isNotEmpty();
 
-    verify(response).addCookie(cookieArgumentCaptor.capture());
-    verifyCookie(cookieArgumentCaptor.getValue());
+    verify(response).addHeader(SET_COOKIE, String.format("XSRF-TOKEN=%s; Path=/; SameSite=Lax; Max-Age=30", state));
   }
 
   @Test
@@ -164,8 +164,7 @@ public class JwtCsrfVerifierTest {
   public void refresh_state() {
     underTest.refreshState(request, response, CSRF_STATE, 30);
 
-    verify(response).addCookie(cookieArgumentCaptor.capture());
-    verifyCookie(cookieArgumentCaptor.getValue());
+    verify(response).addHeader(SET_COOKIE, String.format("XSRF-TOKEN=%s; Path=/; SameSite=Lax; Max-Age=30", CSRF_STATE));
   }
 
   @Test
diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/JwtHttpHandlerTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/JwtHttpHandlerTest.java
index 1abce885127..29a3fe9e876 100644
--- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/JwtHttpHandlerTest.java
+++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/JwtHttpHandlerTest.java
@@ -56,6 +56,7 @@ import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.when;
 import static org.sonar.db.user.UserTesting.newUserDto;
+import static org.sonar.server.authentication.Cookies.SET_COOKIE;
 
 public class JwtHttpHandlerTest {
 
@@ -99,9 +100,7 @@ public class JwtHttpHandlerTest {
     UserDto user = db.users().insertUser();
     underTest.generateToken(user, request, response);
 
-    Optional<Cookie> jwtCookie = findCookie("JWT-SESSION");
-    assertThat(jwtCookie).isPresent();
-    verifyCookie(jwtCookie.get(), JWT_TOKEN, 3 * 24 * 60 * 60);
+    verify(response).addHeader(SET_COOKIE, "JWT-SESSION=TOKEN; Path=/; SameSite=Lax; Max-Age=259200; HttpOnly");
 
     verify(jwtSerializer).encode(jwtArgumentCaptor.capture());
     verifyToken(jwtArgumentCaptor.getValue(), user, 3 * 24 * 60 * 60, NOW);