From 646dc7e336124912d663c43fb0e5410bb502f884 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Mon, 13 Jun 2016 14:40:10 +0200 Subject: [PATCH] SONAR-7733 Add CSRF protection with JWT --- .../authentication/AuthenticationModule.java | 5 +- .../authentication/JwtCsrfVerifier.java | 115 +++++++++ .../server/authentication/JwtHttpHandler.java | 15 +- .../authentication/OAuth2ContextFactory.java | 4 +- ...rfVerifier.java => OAuthCsrfVerifier.java} | 4 +- .../authentication/JwtCsrfVerifierTest.java | 223 ++++++++++++++++++ .../authentication/JwtHttpHandlerTest.java | 61 ++++- .../OAuth2ContextFactoryTest.java | 2 +- ...erTest.java => OAuthCsrfVerifierTest.java} | 18 +- .../app/controllers/sessions_controller.rb | 1 + 10 files changed, 427 insertions(+), 21 deletions(-) create mode 100644 server/sonar-server/src/main/java/org/sonar/server/authentication/JwtCsrfVerifier.java rename server/sonar-server/src/main/java/org/sonar/server/authentication/{CsrfVerifier.java => OAuthCsrfVerifier.java} (97%) create mode 100644 server/sonar-server/src/test/java/org/sonar/server/authentication/JwtCsrfVerifierTest.java rename server/sonar-server/src/test/java/org/sonar/server/authentication/{CsrfVerifierTest.java => OAuthCsrfVerifierTest.java} (98%) diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/AuthenticationModule.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/AuthenticationModule.java index 2ad26f657ef..b2740cc6b9b 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/authentication/AuthenticationModule.java +++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/AuthenticationModule.java @@ -33,10 +33,11 @@ public class AuthenticationModule extends Module { BaseContextFactory.class, OAuth2ContextFactory.class, UserIdentityAuthenticator.class, - CsrfVerifier.class, + OAuthCsrfVerifier.class, GenerateJwtTokenFilter.class, ValidateJwtTokenFilter.class, JwtSerializer.class, - JwtHttpHandler.class); + JwtHttpHandler.class, + JwtCsrfVerifier.class); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/JwtCsrfVerifier.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/JwtCsrfVerifier.java new file mode 100644 index 00000000000..03643101f19 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/JwtCsrfVerifier.java @@ -0,0 +1,115 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.authentication; + +import static org.apache.commons.lang.StringUtils.isBlank; + +import com.google.common.collect.ImmutableSet; +import java.math.BigInteger; +import java.security.SecureRandom; +import java.util.Set; +import javax.annotation.Nullable; +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.apache.commons.lang.StringUtils; +import org.sonar.api.platform.Server; +import org.sonar.server.exceptions.UnauthorizedException; + +public class JwtCsrfVerifier { + + private static final String CSRF_STATE_COOKIE = "XSRF-TOKEN"; + private static final String CSRF_HEADER = "X-XSRF-TOKEN"; + + private static final Set UPDATE_METHODS = ImmutableSet.of("POST", "PUT", "DELETE"); + private static final String API_URL = "/api"; + private static final Set RAILS_UPDATE_API_URLS = ImmutableSet.of( + "/api/events", + "/api/favourites", + "/api/issues/add_comment", + "/api/issues/delete_comment", + "/api/issues/edit_comment", + "/api/issues/bulk_change", + "/api/projects/create", + "/api/properties/create", + "/api/server", + "/api/user_properties" + ); + + private final Server server; + + public JwtCsrfVerifier(Server server) { + this.server = server; + } + + public String generateState(HttpServletResponse response, int timeoutInSeconds) { + // 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(createCookie(state, timeoutInSeconds)); + return state; + } + + public void verifyState(HttpServletRequest request, @Nullable String csrfState) { + if (!shouldRequestBeChecked(request)) { + return; + } + String stateInHeader = request.getHeader(CSRF_HEADER); + if (isBlank(csrfState) || !StringUtils.equals(csrfState, stateInHeader)) { + throw new UnauthorizedException(); + } + } + + public void refreshState(HttpServletResponse response, String csrfState, int timeoutInSeconds){ + response.addCookie(createCookie(csrfState, timeoutInSeconds)); + } + + public void removeState(HttpServletResponse response){ + response.addCookie(createCookie(null, 0)); + } + + private static boolean shouldRequestBeChecked(HttpServletRequest request) { + if (UPDATE_METHODS.contains(request.getMethod())) { + String path = request.getRequestURI().replaceFirst(request.getContextPath(), ""); + return path.startsWith(API_URL) + && !isRailsWsUrl(path); + } + return false; + } + + private static boolean isRailsWsUrl(String uri){ + for (String railsUrl : RAILS_UPDATE_API_URLS) { + if (uri.startsWith(railsUrl)) { + return true; + } + } + return false; + } + + private Cookie createCookie(@Nullable String csrfState, int timeoutInSeconds){ + Cookie cookie = new Cookie(CSRF_STATE_COOKIE, csrfState); + cookie.setPath(server.getContextPath() + "/"); + cookie.setSecure(server.isSecured()); + cookie.setHttpOnly(false); + cookie.setMaxAge(timeoutInSeconds); + return cookie; + } + +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/JwtHttpHandler.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/JwtHttpHandler.java index 30c1567afcb..12e61f09321 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/authentication/JwtHttpHandler.java +++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/JwtHttpHandler.java @@ -50,6 +50,8 @@ public class JwtHttpHandler { private static final String JWT_COOKIE = "JWT-SESSION"; private static final String LAST_REFRESH_TIME_PARAM = "lastRefreshTime"; + private static final String CSRF_JWT_PARAM = "xsrfToken"; + // Time after which a user will be disconnected private static final int SESSION_DISCONNECT_IN_SECONDS = 3 * 30 * 24 * 60 * 60; @@ -66,20 +68,26 @@ public class JwtHttpHandler { // This timeout is used to disconnect the user we he has not browse any page for a while private final int sessionTimeoutInSeconds; + private final JwtCsrfVerifier jwtCsrfVerifier; - public JwtHttpHandler(System2 system2, DbClient dbClient, Server server, Settings settings, JwtSerializer jwtSerializer) { + public JwtHttpHandler(System2 system2, DbClient dbClient, Server server, Settings settings, JwtSerializer jwtSerializer, JwtCsrfVerifier jwtCsrfVerifier) { this.jwtSerializer = jwtSerializer; this.server = server; this.dbClient = dbClient; this.system2 = system2; this.sessionTimeoutInSeconds = getSessionTimeoutInSeconds(settings); + this.jwtCsrfVerifier = jwtCsrfVerifier; } void generateToken(String userLogin, HttpServletResponse response) { + String csrfState = jwtCsrfVerifier.generateState(response, sessionTimeoutInSeconds); + String token = jwtSerializer.encode(new JwtSerializer.JwtSession( userLogin, sessionTimeoutInSeconds, - ImmutableMap.of(LAST_REFRESH_TIME_PARAM, system2.now()))); + ImmutableMap.of( + LAST_REFRESH_TIME_PARAM, system2.now(), + CSRF_JWT_PARAM, csrfState))); response.addCookie(createCookie(JWT_COOKIE, token, sessionTimeoutInSeconds)); } @@ -115,6 +123,7 @@ public class JwtHttpHandler { return; } + jwtCsrfVerifier.verifyState(request, (String) token.get(CSRF_JWT_PARAM)); request.getSession().setAttribute(RAILS_USER_ID_SESSION, user.get().getId()); if (now.after(DateUtils.addSeconds(getLastRefreshDate(token), SESSION_REFRESH_IN_SECONDS))) { refreshToken(token, response); @@ -130,11 +139,13 @@ public class JwtHttpHandler { private void refreshToken(Claims token, HttpServletResponse response) { String refreshToken = jwtSerializer.refresh(token, sessionTimeoutInSeconds); response.addCookie(createCookie(JWT_COOKIE, refreshToken, sessionTimeoutInSeconds)); + jwtCsrfVerifier.refreshState(response, (String) token.get(CSRF_JWT_PARAM), sessionTimeoutInSeconds); } private void removeSession(HttpServletRequest request, HttpServletResponse response) { request.getSession().removeAttribute(RAILS_USER_ID_SESSION); response.addCookie(createCookie(JWT_COOKIE, null, 0)); + jwtCsrfVerifier.removeState(response); } private Cookie createCookie(String name, @Nullable String value, int expirationInSeconds) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java index 16527249102..6135d6aff49 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java +++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java @@ -35,9 +35,9 @@ public class OAuth2ContextFactory { private final UserIdentityAuthenticator userIdentityAuthenticator; private final Server server; - private final CsrfVerifier csrfVerifier; + private final OAuthCsrfVerifier csrfVerifier; - public OAuth2ContextFactory(UserIdentityAuthenticator userIdentityAuthenticator, Server server, CsrfVerifier csrfVerifier) { + public OAuth2ContextFactory(UserIdentityAuthenticator userIdentityAuthenticator, Server server, OAuthCsrfVerifier csrfVerifier) { this.userIdentityAuthenticator = userIdentityAuthenticator; this.server = server; this.csrfVerifier = csrfVerifier; diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/CsrfVerifier.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuthCsrfVerifier.java similarity index 97% rename from server/sonar-server/src/main/java/org/sonar/server/authentication/CsrfVerifier.java rename to server/sonar-server/src/main/java/org/sonar/server/authentication/OAuthCsrfVerifier.java index 60ccd965184..9870ada7c82 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/authentication/CsrfVerifier.java +++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuthCsrfVerifier.java @@ -31,13 +31,13 @@ import javax.servlet.http.HttpServletResponse; import org.sonar.api.platform.Server; import org.sonar.server.exceptions.UnauthorizedException; -public class CsrfVerifier { +public class OAuthCsrfVerifier { private static final String CSRF_STATE_COOKIE = "OAUTHSTATE"; private final Server server; - public CsrfVerifier(Server server) { + public OAuthCsrfVerifier(Server server) { this.server = server; } diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/JwtCsrfVerifierTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/JwtCsrfVerifierTest.java new file mode 100644 index 00000000000..7cc73d57e67 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/JwtCsrfVerifierTest.java @@ -0,0 +1,223 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.authentication; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.mockito.ArgumentCaptor; +import org.sonar.api.platform.Server; +import org.sonar.server.exceptions.UnauthorizedException; + +public class JwtCsrfVerifierTest { + + @Rule + public ExpectedException thrown = ExpectedException.none(); + + static final int TIMEOUT = 30; + static final String CSRF_STATE = "STATE"; + static final String JAVA_WS_URL = "/api/metrics/create"; + + ArgumentCaptor cookieArgumentCaptor = ArgumentCaptor.forClass(Cookie.class); + + Server server = mock(Server.class); + HttpServletResponse response = mock(HttpServletResponse.class); + HttpServletRequest request = mock(HttpServletRequest.class); + + JwtCsrfVerifier underTest = new JwtCsrfVerifier(server); + + @Before + public void setUp() throws Exception { + when(server.getContextPath()).thenReturn(""); + when(request.getContextPath()).thenReturn(""); + } + + @Test + public void generate_state_on_secured_server() throws Exception { + when(server.isSecured()).thenReturn(true); + + String state = underTest.generateState(response, TIMEOUT); + assertThat(state).isNotEmpty(); + + verify(response).addCookie(cookieArgumentCaptor.capture()); + verifyCookie(cookieArgumentCaptor.getValue(), true); + } + + @Test + public void generate_state_on_not_secured_server() throws Exception { + when(server.isSecured()).thenReturn(false); + + String state = underTest.generateState(response, TIMEOUT); + assertThat(state).isNotEmpty(); + + verify(response).addCookie(cookieArgumentCaptor.capture()); + + verifyCookie(cookieArgumentCaptor.getValue(), false); + } + + @Test + public void verify_state() throws Exception { + mockRequestCsrf(CSRF_STATE); + mockPostJavaWsRequest(); + + underTest.verifyState(request, CSRF_STATE); + } + + @Test + public void fail_with_unauthorized_when_state_header_is_not_the_same_as_state_parameter() throws Exception { + mockRequestCsrf("other value"); + mockPostJavaWsRequest(); + + thrown.expect(UnauthorizedException.class); + underTest.verifyState(request, CSRF_STATE); + } + + @Test + public void fail_with_unauthorized_when_state_is_null() throws Exception { + mockRequestCsrf(CSRF_STATE); + mockPostJavaWsRequest(); + + thrown.expect(UnauthorizedException.class); + underTest.verifyState(request, null); + } + + @Test + public void fail_with_unauthorized_when_state_parameter_is_empty() throws Exception { + mockRequestCsrf(CSRF_STATE); + mockPostJavaWsRequest(); + + thrown.expect(UnauthorizedException.class); + underTest.verifyState(request, ""); + } + + @Test + public void verify_POST_request() throws Exception { + mockRequestCsrf("other value"); + when(request.getRequestURI()).thenReturn(JAVA_WS_URL); + when(request.getMethod()).thenReturn("POST"); + + thrown.expect(UnauthorizedException.class); + underTest.verifyState(request, CSRF_STATE); + } + + @Test + public void verify_PUT_request() throws Exception { + mockRequestCsrf("other value"); + when(request.getRequestURI()).thenReturn(JAVA_WS_URL); + when(request.getMethod()).thenReturn("PUT"); + + thrown.expect(UnauthorizedException.class); + underTest.verifyState(request, CSRF_STATE); + } + + @Test + public void verify_DELETE_request() throws Exception { + mockRequestCsrf("other value"); + when(request.getRequestURI()).thenReturn(JAVA_WS_URL); + when(request.getMethod()).thenReturn("DELETE"); + + thrown.expect(UnauthorizedException.class); + underTest.verifyState(request, CSRF_STATE); + } + + @Test + public void ignore_GET_request() throws Exception { + when(request.getRequestURI()).thenReturn(JAVA_WS_URL); + when(request.getMethod()).thenReturn("GET"); + + underTest.verifyState(request, null); + } + + @Test + public void ignore_rails_ws_requests() throws Exception { + executeVerifyStateDoesNotFailOnRequest("/api/events", "POST"); + executeVerifyStateDoesNotFailOnRequest("/api/favourites", "POST"); + executeVerifyStateDoesNotFailOnRequest("/api/issues/add_comment?key=ABCD", "POST"); + executeVerifyStateDoesNotFailOnRequest("/api/issues/delete_comment?key=ABCD", "POST"); + executeVerifyStateDoesNotFailOnRequest("/api/issues/edit_comment?key=ABCD", "POST"); + executeVerifyStateDoesNotFailOnRequest("/api/issues/bulk_change?key=ABCD", "POST"); + executeVerifyStateDoesNotFailOnRequest("/api/projects/create?key=ABCD", "POST"); + executeVerifyStateDoesNotFailOnRequest("/api/properties/create?key=ABCD", "POST"); + executeVerifyStateDoesNotFailOnRequest("/api/server", "POST"); + executeVerifyStateDoesNotFailOnRequest("/api/user_properties", "POST"); + } + + @Test + public void ignore_not_api_requests() throws Exception { + executeVerifyStateDoesNotFailOnRequest("/events", "POST"); + executeVerifyStateDoesNotFailOnRequest("/favorites", "POST"); + } + + @Test + public void refresh_state() throws Exception { + when(server.isSecured()).thenReturn(true); + + underTest.refreshState(response, CSRF_STATE, 30); + + verify(response).addCookie(cookieArgumentCaptor.capture()); + verifyCookie(cookieArgumentCaptor.getValue(), true); + } + + @Test + public void remove_state() throws Exception { + when(server.isSecured()).thenReturn(true); + + underTest.removeState(response); + + verify(response).addCookie(cookieArgumentCaptor.capture()); + Cookie cookie = cookieArgumentCaptor.getValue(); + assertThat(cookie.getValue()).isNull(); + assertThat(cookie.getMaxAge()).isEqualTo(0); + } + + private void verifyCookie(Cookie cookie, boolean isSecured) { + assertThat(cookie.getName()).isEqualTo("XSRF-TOKEN"); + assertThat(cookie.getValue()).isNotEmpty(); + assertThat(cookie.getPath()).isEqualTo("/"); + assertThat(cookie.isHttpOnly()).isFalse(); + assertThat(cookie.getMaxAge()).isEqualTo(TIMEOUT); + assertThat(cookie.getSecure()).isEqualTo(isSecured); + } + + private void mockPostJavaWsRequest(){ + when(request.getRequestURI()).thenReturn(JAVA_WS_URL); + when(request.getMethod()).thenReturn("POST"); + } + + private void mockRequestCsrf(String csrfState){ + when(request.getHeader("X-XSRF-TOKEN")).thenReturn(csrfState); + } + + private void executeVerifyStateDoesNotFailOnRequest(String uri, String method){ + when(request.getRequestURI()).thenReturn(uri); + when(request.getMethod()).thenReturn(method); + + underTest.verifyState(request, null); + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/JwtHttpHandlerTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/JwtHttpHandlerTest.java index fc3efd8b65a..8ec20f37ebf 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/authentication/JwtHttpHandlerTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/JwtHttpHandlerTest.java @@ -60,6 +60,7 @@ public class JwtHttpHandlerTest { static final String JWT_TOKEN = "TOKEN"; static final String USER_LOGIN = "john"; + static final String CSRF_STATE = "CSRF_STATE"; static final long NOW = 10_000_000_000L; static final long FOUR_MINUTES_AGO = NOW - 4 * 60 * 1000L; @@ -90,8 +91,9 @@ public class JwtHttpHandlerTest { Server server = mock(Server.class); Settings settings = new Settings(); JwtSerializer jwtSerializer = mock(JwtSerializer.class); + JwtCsrfVerifier jwtCsrfVerifier = mock(JwtCsrfVerifier.class); - JwtHttpHandler underTest = new JwtHttpHandler(system2, dbClient, server, settings, jwtSerializer); + JwtHttpHandler underTest = new JwtHttpHandler(system2, dbClient, server, settings, jwtSerializer, jwtCsrfVerifier); @Before public void setUp() throws Exception { @@ -100,6 +102,7 @@ public class JwtHttpHandlerTest { when(server.getContextPath()).thenReturn(""); when(request.getSession()).thenReturn(httpSession); when(jwtSerializer.encode(any(JwtSerializer.JwtSession.class))).thenReturn(JWT_TOKEN); + when(jwtCsrfVerifier.generateState(eq(response), anyInt())).thenReturn(CSRF_STATE); } @Test @@ -114,6 +117,17 @@ public class JwtHttpHandlerTest { verifyToken(jwtArgumentCaptor.getValue(), 3 * 24 * 60 * 60, NOW); } + @Test + public void generate_csrf_state() throws Exception { + underTest.generateToken(USER_LOGIN, response); + + verify(jwtCsrfVerifier).generateState(response, 3 * 24 * 60 * 60); + + verify(jwtSerializer).encode(jwtArgumentCaptor.capture()); + JwtSerializer.JwtSession token = jwtArgumentCaptor.getValue(); + assertThat(token.getProperties().get("xsrfToken")).isEqualTo(CSRF_STATE); + } + @Test public void validate_session() throws Exception { addJwtCookie(); @@ -133,7 +147,7 @@ public class JwtHttpHandlerTest { int sessionTimeoutInHours = 10; settings.setProperty("sonar.auth.sessionTimeoutInHours", sessionTimeoutInHours); - underTest = new JwtHttpHandler(system2, dbClient, server, settings, jwtSerializer); + underTest = new JwtHttpHandler(system2, dbClient, server, settings, jwtSerializer, jwtCsrfVerifier); underTest.generateToken(USER_LOGIN, response); verify(jwtSerializer).encode(jwtArgumentCaptor.capture()); @@ -145,7 +159,7 @@ public class JwtHttpHandlerTest { int firstSessionTimeoutInHours = 10; settings.setProperty("sonar.auth.sessionTimeoutInHours", firstSessionTimeoutInHours); - underTest = new JwtHttpHandler(system2, dbClient, server, settings, jwtSerializer); + underTest = new JwtHttpHandler(system2, dbClient, server, settings, jwtSerializer, jwtCsrfVerifier); underTest.generateToken(USER_LOGIN, response); // The property is updated, but it won't be taking into account @@ -247,6 +261,47 @@ public class JwtHttpHandlerTest { verifyZeroInteractions(httpSession, jwtSerializer); } + @Test + public void verify_csrf_state() throws Exception { + addJwtCookie(); + addUser(); + Claims claims = createToken(NOW); + claims.put("xsrfToken", CSRF_STATE); + when(jwtSerializer.decode(JWT_TOKEN)).thenReturn(Optional.of(claims)); + + underTest.validateToken(request, response); + + verify(jwtCsrfVerifier).verifyState(request, CSRF_STATE); + } + + @Test + public void refresh_state_when_refreshing_token() throws Exception { + addJwtCookie(); + addUser(); + + // Token was created 10 days ago and refreshed 6 minutes ago + Claims claims = createToken(TEN_DAYS_AGO); + claims.put("xsrfToken", "CSRF_STATE"); + when(jwtSerializer.decode(JWT_TOKEN)).thenReturn(Optional.of(claims)); + + underTest.validateToken(request, response); + + verify(jwtSerializer).refresh(any(Claims.class), anyInt()); + verify(jwtCsrfVerifier).refreshState(response, "CSRF_STATE", 3 * 24 * 60 * 60); + } + + @Test + public void remove_state_when_removing_token() throws Exception { + addJwtCookie(); + // Token is invalid => it will be removed + when(jwtSerializer.decode(JWT_TOKEN)).thenReturn(Optional.empty()); + + underTest.validateToken(request, response); + + verifyCookie(findCookie("JWT-SESSION").get(), null, 0); + verify(jwtCsrfVerifier).removeState(response); + } + private void verifyToken(JwtSerializer.JwtSession token, int expectedExpirationTime, long expectedRefreshTime) { assertThat(token.getExpirationTimeInSeconds()).isEqualTo(expectedExpirationTime); assertThat(token.getUserLogin()).isEqualTo(USER_LOGIN); diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuth2ContextFactoryTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuth2ContextFactoryTest.java index c687b55ae82..677bfaf7dfb 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuth2ContextFactoryTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuth2ContextFactoryTest.java @@ -55,7 +55,7 @@ public class OAuth2ContextFactoryTest { UserIdentityAuthenticator userIdentityAuthenticator = mock(UserIdentityAuthenticator.class); Server server = mock(Server.class); - CsrfVerifier csrfVerifier = mock(CsrfVerifier.class); + OAuthCsrfVerifier csrfVerifier = mock(OAuthCsrfVerifier.class); HttpServletRequest request = mock(HttpServletRequest.class); HttpServletResponse response = mock(HttpServletResponse.class); diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/CsrfVerifierTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuthCsrfVerifierTest.java similarity index 98% rename from server/sonar-server/src/test/java/org/sonar/server/authentication/CsrfVerifierTest.java rename to server/sonar-server/src/test/java/org/sonar/server/authentication/OAuthCsrfVerifierTest.java index 5b77cd434e8..120f8d1a298 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/authentication/CsrfVerifierTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuthCsrfVerifierTest.java @@ -19,6 +19,13 @@ */ package org.sonar.server.authentication; +import static org.apache.commons.codec.digest.DigestUtils.sha1Hex; +import static org.apache.commons.codec.digest.DigestUtils.sha256Hex; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -30,14 +37,7 @@ import org.mockito.ArgumentCaptor; import org.sonar.api.platform.Server; import org.sonar.server.exceptions.UnauthorizedException; -import static org.apache.commons.codec.digest.DigestUtils.sha1Hex; -import static org.apache.commons.codec.digest.DigestUtils.sha256Hex; -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -public class CsrfVerifierTest { +public class OAuthCsrfVerifierTest { @Rule public ExpectedException thrown = ExpectedException.none(); @@ -48,7 +48,7 @@ public class CsrfVerifierTest { HttpServletResponse response = mock(HttpServletResponse.class); HttpServletRequest request = mock(HttpServletRequest.class); - CsrfVerifier underTest = new CsrfVerifier(server); + OAuthCsrfVerifier underTest = new OAuthCsrfVerifier(server); @Before public void setUp() throws Exception { diff --git a/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/sessions_controller.rb b/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/sessions_controller.rb index 4ca15d13a71..83285fc1e0d 100644 --- a/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/sessions_controller.rb +++ b/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/sessions_controller.rb @@ -56,6 +56,7 @@ class SessionsController < ApplicationController self.current_user.on_logout end cookies.delete 'JWT-SESSION' + cookies.delete 'XSRF-TOKEN' flash[:notice]=message('session.flash_notice.logged_out') redirect_to(home_path) reset_session -- 2.39.5