diff options
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); |