From 92f76d9ce9a915089dbab02296c22959b469480a Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Fri, 1 Jul 2016 14:26:32 +0200 Subject: [PATCH] SONAR-7732 Do not remove cookies when user is not authenticated --- .../authentication/JwtCsrfVerifier.java | 4 --- .../server/authentication/JwtHttpHandler.java | 6 ---- .../UserSessionInitializer.java | 1 - .../authentication/JwtCsrfVerifierTest.java | 18 ++--------- .../authentication/JwtHttpHandlerTest.java | 32 ++----------------- .../UserSessionInitializerTest.java | 4 +++ 6 files changed, 10 insertions(+), 55 deletions(-) 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 index 01c2adc46b0..f875b418d71 100644 --- 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 @@ -81,10 +81,6 @@ public class JwtCsrfVerifier { 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(), ""); 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 2e8e88e258f..61bf975d51f 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 @@ -94,7 +94,6 @@ public class JwtHttpHandler { if (userDto.isPresent()) { return userDto; } - removeToken(response); return Optional.empty(); } @@ -155,11 +154,6 @@ public class JwtHttpHandler { jwtCsrfVerifier.refreshState(response, (String) token.get(CSRF_JWT_PARAM), sessionTimeoutInSeconds); } - void removeToken(HttpServletResponse response) { - response.addCookie(createCookie(JWT_COOKIE, null, 0)); - jwtCsrfVerifier.removeState(response); - } - private Cookie createCookie(String name, @Nullable String value, int expirationInSeconds) { Cookie cookie = new Cookie(name, value); cookie.setPath("/"); diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/UserSessionInitializer.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/UserSessionInitializer.java index d25063ba7fd..3b2053eee66 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/authentication/UserSessionInitializer.java +++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/UserSessionInitializer.java @@ -85,7 +85,6 @@ public class UserSessionInitializer { setUserSession(request, response); return true; } catch (UnauthorizedException e) { - jwtHttpHandler.removeToken(response); response.setStatus(HTTP_UNAUTHORIZED); if (isWsUrl(path)) { return false; 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 index ab15b2d4027..7e8e770a0ef 100644 --- 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 @@ -183,18 +183,6 @@ public class JwtCsrfVerifierTest { 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(); @@ -204,16 +192,16 @@ public class JwtCsrfVerifierTest { assertThat(cookie.getSecure()).isEqualTo(isSecured); } - private void mockPostJavaWsRequest(){ + private void mockPostJavaWsRequest() { when(request.getRequestURI()).thenReturn(JAVA_WS_URL); when(request.getMethod()).thenReturn("POST"); } - private void mockRequestCsrf(String csrfState){ + private void mockRequestCsrf(String csrfState) { when(request.getHeader("X-XSRF-TOKEN")).thenReturn(csrfState); } - private void executeVerifyStateDoesNotFailOnRequest(String uri, String method){ + private void executeVerifyStateDoesNotFailOnRequest(String uri, String method) { when(request.getRequestURI()).thenReturn(uri); when(request.getMethod()).thenReturn(method); 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 e276fde6007..93ca53231f8 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 @@ -196,7 +196,7 @@ public class JwtHttpHandlerTest { } @Test - public void validate_token_removes_session_when_disconnected_timeout_is_reached() throws Exception { + public void validate_token_does_not_refresh_session_when_disconnected_timeout_is_reached() throws Exception { addJwtCookie(); // Token was created 4 months ago, refreshed 4 minutes ago, and it expired in 5 minutes @@ -206,12 +206,10 @@ public class JwtHttpHandlerTest { when(jwtSerializer.decode(JWT_TOKEN)).thenReturn(Optional.of(claims)); assertThat(underTest.validateToken(request, response).isPresent()).isFalse(); - - verifyCookie(findCookie("JWT-SESSION").get(), null, 0); } @Test - public void validate_token_removes_session_when_user_is_disabled() throws Exception { + public void validate_token_does_not_refresh_session_when_user_is_disabled() throws Exception { addJwtCookie(); UserDto user = addUser(false); @@ -219,19 +217,15 @@ public class JwtHttpHandlerTest { when(jwtSerializer.decode(JWT_TOKEN)).thenReturn(Optional.of(claims)); assertThat(underTest.validateToken(request, response).isPresent()).isFalse(); - - verifyCookie(findCookie("JWT-SESSION").get(), null, 0); } @Test - public void validate_token_removes_session_when_token_is_no_more_valid() throws Exception { + public void validate_token_does_not_refresh_session_when_token_is_no_more_valid() throws Exception { addJwtCookie(); when(jwtSerializer.decode(JWT_TOKEN)).thenReturn(Optional.empty()); assertThat(underTest.validateToken(request, response).isPresent()).isFalse(); - - verifyCookie(findCookie("JWT-SESSION").get(), null, 0); } @Test @@ -279,26 +273,6 @@ public class JwtHttpHandlerTest { verify(jwtCsrfVerifier).refreshState(response, "CSRF_STATE", 3 * 24 * 60 * 60); } - @Test - public void validate_token_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); - } - - @Test - public void remove_token() throws Exception { - underTest.removeToken(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/UserSessionInitializerTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/UserSessionInitializerTest.java index 6ea3fcd2007..dba52e8d4dc 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/authentication/UserSessionInitializerTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/UserSessionInitializerTest.java @@ -137,6 +137,7 @@ public class UserSessionInitializerTest { assertThat(underTest.initUserSession(request, response)).isTrue(); verify(response).setStatus(401); + verifyZeroInteractions(userSession); } @Test @@ -149,6 +150,7 @@ public class UserSessionInitializerTest { assertThat(underTest.initUserSession(request, response)).isTrue(); verify(response).setStatus(401); + verifyZeroInteractions(userSession); } @Test @@ -159,6 +161,7 @@ public class UserSessionInitializerTest { assertThat(underTest.initUserSession(request, response)).isFalse(); verify(response).setStatus(401); + verifyZeroInteractions(userSession); } @Test @@ -169,6 +172,7 @@ public class UserSessionInitializerTest { assertThat(underTest.initUserSession(request, response)).isFalse(); verify(response).setStatus(401); + verifyZeroInteractions(userSession); } @Test -- 2.39.5