]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-7733 Add CSRF protection with JWT
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Mon, 13 Jun 2016 12:40:10 +0000 (14:40 +0200)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Wed, 15 Jun 2016 09:08:36 +0000 (11:08 +0200)
12 files changed:
server/sonar-server/src/main/java/org/sonar/server/authentication/AuthenticationModule.java
server/sonar-server/src/main/java/org/sonar/server/authentication/CsrfVerifier.java [deleted file]
server/sonar-server/src/main/java/org/sonar/server/authentication/JwtCsrfVerifier.java [new file with mode: 0644]
server/sonar-server/src/main/java/org/sonar/server/authentication/JwtHttpHandler.java
server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java
server/sonar-server/src/main/java/org/sonar/server/authentication/OAuthCsrfVerifier.java [new file with mode: 0644]
server/sonar-server/src/test/java/org/sonar/server/authentication/CsrfVerifierTest.java [deleted file]
server/sonar-server/src/test/java/org/sonar/server/authentication/JwtCsrfVerifierTest.java [new file with mode: 0644]
server/sonar-server/src/test/java/org/sonar/server/authentication/JwtHttpHandlerTest.java
server/sonar-server/src/test/java/org/sonar/server/authentication/OAuth2ContextFactoryTest.java
server/sonar-server/src/test/java/org/sonar/server/authentication/OAuthCsrfVerifierTest.java [new file with mode: 0644]
server/sonar-web/src/main/webapp/WEB-INF/app/controllers/sessions_controller.rb

index 2ad26f657efc860a3546ae61e4c398231ac9e41c..b2740cc6b9b9236cb14273339c8a72de68c2216d 100644 (file)
@@ -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/CsrfVerifier.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/CsrfVerifier.java
deleted file mode 100644 (file)
index 60ccd96..0000000
+++ /dev/null
@@ -1,78 +0,0 @@
-/*
- * 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.codec.digest.DigestUtils.sha256Hex;
-import static org.apache.commons.lang.StringUtils.isBlank;
-
-import java.math.BigInteger;
-import java.security.SecureRandom;
-import java.util.Optional;
-import javax.servlet.http.Cookie;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-import org.sonar.api.platform.Server;
-import org.sonar.server.exceptions.UnauthorizedException;
-
-public class CsrfVerifier {
-
-  private static final String CSRF_STATE_COOKIE = "OAUTHSTATE";
-
-  private final Server server;
-
-  public CsrfVerifier(Server server) {
-    this.server = server;
-  }
-
-  public String generateState(HttpServletResponse response) {
-    // Create a state token to prevent request forgery.
-    // Store it in the session for later validation.
-    String state = new BigInteger(130, new SecureRandom()).toString(32);
-    Cookie cookie = new Cookie(CSRF_STATE_COOKIE, sha256Hex(state));
-    cookie.setPath(server.getContextPath() + "/");
-    cookie.setHttpOnly(true);
-    cookie.setMaxAge(-1);
-    cookie.setSecure(server.isSecured());
-    response.addCookie(cookie);
-    return state;
-  }
-
-  public void verifyState(HttpServletRequest request, HttpServletResponse response) {
-    Optional<Cookie> stateCookie = CookieUtils.findCookie(CSRF_STATE_COOKIE, request);
-    if (!stateCookie.isPresent()) {
-      throw new UnauthorizedException();
-    }
-    Cookie cookie = stateCookie.get();
-
-    String hashInCookie = cookie.getValue();
-
-    // remove cookie
-    cookie.setValue(null);
-    cookie.setMaxAge(0);
-    cookie.setPath(server.getContextPath() + "/");
-    response.addCookie(cookie);
-
-    String stateInRequest = request.getParameter("state");
-    if (isBlank(stateInRequest) || !sha256Hex(stateInRequest).equals(hashInCookie)) {
-      throw new UnauthorizedException();
-    }
-  }
-
-}
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 (file)
index 0000000..0364310
--- /dev/null
@@ -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<String> UPDATE_METHODS = ImmutableSet.of("POST", "PUT", "DELETE");
+  private static final String API_URL = "/api";
+  private static final Set<String> 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;
+  }
+
+}
index 30c1567afcb7433532bad99b6e83f5a1845b1c5e..12e61f09321c037835d9cf7dbafd46008fada0c4 100644 (file)
@@ -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) {
index 1652724910275322c0533e3c9d5ff045714d75b9..6135d6aff491977259fbbb2c5f599e28278b0a4e 100644 (file)
@@ -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/OAuthCsrfVerifier.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuthCsrfVerifier.java
new file mode 100644 (file)
index 0000000..9870ada
--- /dev/null
@@ -0,0 +1,78 @@
+/*
+ * 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.codec.digest.DigestUtils.sha256Hex;
+import static org.apache.commons.lang.StringUtils.isBlank;
+
+import java.math.BigInteger;
+import java.security.SecureRandom;
+import java.util.Optional;
+import javax.servlet.http.Cookie;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import org.sonar.api.platform.Server;
+import org.sonar.server.exceptions.UnauthorizedException;
+
+public class OAuthCsrfVerifier {
+
+  private static final String CSRF_STATE_COOKIE = "OAUTHSTATE";
+
+  private final Server server;
+
+  public OAuthCsrfVerifier(Server server) {
+    this.server = server;
+  }
+
+  public String generateState(HttpServletResponse response) {
+    // Create a state token to prevent request forgery.
+    // Store it in the session for later validation.
+    String state = new BigInteger(130, new SecureRandom()).toString(32);
+    Cookie cookie = new Cookie(CSRF_STATE_COOKIE, sha256Hex(state));
+    cookie.setPath(server.getContextPath() + "/");
+    cookie.setHttpOnly(true);
+    cookie.setMaxAge(-1);
+    cookie.setSecure(server.isSecured());
+    response.addCookie(cookie);
+    return state;
+  }
+
+  public void verifyState(HttpServletRequest request, HttpServletResponse response) {
+    Optional<Cookie> stateCookie = CookieUtils.findCookie(CSRF_STATE_COOKIE, request);
+    if (!stateCookie.isPresent()) {
+      throw new UnauthorizedException();
+    }
+    Cookie cookie = stateCookie.get();
+
+    String hashInCookie = cookie.getValue();
+
+    // remove cookie
+    cookie.setValue(null);
+    cookie.setMaxAge(0);
+    cookie.setPath(server.getContextPath() + "/");
+    response.addCookie(cookie);
+
+    String stateInRequest = request.getParameter("state");
+    if (isBlank(stateInRequest) || !sha256Hex(stateInRequest).equals(hashInCookie)) {
+      throw new UnauthorizedException();
+    }
+  }
+
+}
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/CsrfVerifierTest.java
deleted file mode 100644 (file)
index 5b77cd4..0000000
+++ /dev/null
@@ -1,147 +0,0 @@
-/*
- * 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 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;
-
-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 {
-
-  @Rule
-  public ExpectedException thrown = ExpectedException.none();
-
-  ArgumentCaptor<Cookie> cookieArgumentCaptor = ArgumentCaptor.forClass(Cookie.class);
-
-  Server server = mock(Server.class);
-  HttpServletResponse response = mock(HttpServletResponse.class);
-  HttpServletRequest request = mock(HttpServletRequest.class);
-
-  CsrfVerifier underTest = new CsrfVerifier(server);
-
-  @Before
-  public void setUp() throws Exception {
-    when(server.getContextPath()).thenReturn("");
-  }
-
-  @Test
-  public void generate_state_on_secured_server() throws Exception {
-    when(server.isSecured()).thenReturn(true);
-
-    String state = underTest.generateState(response);
-    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);
-    assertThat(state).isNotEmpty();
-
-    verify(response).addCookie(cookieArgumentCaptor.capture());
-
-    verifyCookie(cookieArgumentCaptor.getValue(), false);
-  }
-
-  @Test
-  public void verify_state() throws Exception {
-    String state = "state";
-    when(request.getCookies()).thenReturn(new Cookie[] {new Cookie("OAUTHSTATE", sha256Hex(state))});
-    when(request.getParameter("state")).thenReturn(state);
-
-    underTest.verifyState(request, response);
-
-    verify(response).addCookie(cookieArgumentCaptor.capture());
-    Cookie updatedCookie = cookieArgumentCaptor.getValue();
-    assertThat(updatedCookie.getName()).isEqualTo("OAUTHSTATE");
-    assertThat(updatedCookie.getValue()).isNull();
-    assertThat(updatedCookie.getPath()).isEqualTo("/");
-    assertThat(updatedCookie.getMaxAge()).isEqualTo(0);
-  }
-
-  @Test
-  public void verify_state_when_context() throws Exception {
-    String state = "state";
-    when(request.getCookies()).thenReturn(new Cookie[] {new Cookie("OAUTHSTATE", sha256Hex(state))});
-    when(request.getParameter("state")).thenReturn(state);
-    when(server.getContextPath()).thenReturn("/sonarqube");
-
-    underTest.verifyState(request, response);
-
-    verify(response).addCookie(cookieArgumentCaptor.capture());
-    Cookie updatedCookie = cookieArgumentCaptor.getValue();
-    assertThat(updatedCookie.getPath()).isEqualTo("/sonarqube/");
-  }
-
-  @Test
-  public void fail_with_unauthorized_when_state_cookie_is_not_the_same_as_state_parameter() throws Exception {
-    when(request.getCookies()).thenReturn(new Cookie[] {new Cookie("OAUTHSTATE", sha1Hex("state"))});
-    when(request.getParameter("state")).thenReturn("other value");
-
-    thrown.expect(UnauthorizedException.class);
-    underTest.verifyState(request, response);
-  }
-
-  @Test
-  public void fail_to_verify_state_when_state_cookie_is_null() throws Exception {
-    when(request.getCookies()).thenReturn(new Cookie[] {new Cookie("OAUTHSTATE", null)});
-    when(request.getParameter("state")).thenReturn("state");
-
-    thrown.expect(UnauthorizedException.class);
-    underTest.verifyState(request, response);
-  }
-
-  @Test
-  public void fail_with_unauthorized_when_state_parameter_is_empty() throws Exception {
-    when(request.getCookies()).thenReturn(new Cookie[] {new Cookie("OAUTHSTATE", sha1Hex("state"))});
-    when(request.getParameter("state")).thenReturn("");
-
-    thrown.expect(UnauthorizedException.class);
-    underTest.verifyState(request, response);
-  }
-
-  private void verifyCookie(Cookie cookie, boolean isSecured) {
-    assertThat(cookie.getName()).isEqualTo("OAUTHSTATE");
-    assertThat(cookie.getValue()).isNotEmpty();
-    assertThat(cookie.getPath()).isEqualTo("/");
-    assertThat(cookie.isHttpOnly()).isTrue();
-    assertThat(cookie.getMaxAge()).isEqualTo(-1);
-    assertThat(cookie.getSecure()).isEqualTo(isSecured);
-  }
-}
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 (file)
index 0000000..7cc73d5
--- /dev/null
@@ -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<Cookie> 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);
+  }
+}
index fc3efd8b65ad2b096629480726ce99d6aaf1e411..8ec20f37ebff9834681a3e523497d973632af9f0 100644 (file)
@@ -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);
index c687b55ae823bec29bde2a3cec9039aca689419d..677bfaf7dfbc3345fec685e3265dff9fbc59cbcb 100644 (file)
@@ -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/OAuthCsrfVerifierTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuthCsrfVerifierTest.java
new file mode 100644 (file)
index 0000000..120f8d1
--- /dev/null
@@ -0,0 +1,147 @@
+/*
+ * 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.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;
+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 OAuthCsrfVerifierTest {
+
+  @Rule
+  public ExpectedException thrown = ExpectedException.none();
+
+  ArgumentCaptor<Cookie> cookieArgumentCaptor = ArgumentCaptor.forClass(Cookie.class);
+
+  Server server = mock(Server.class);
+  HttpServletResponse response = mock(HttpServletResponse.class);
+  HttpServletRequest request = mock(HttpServletRequest.class);
+
+  OAuthCsrfVerifier underTest = new OAuthCsrfVerifier(server);
+
+  @Before
+  public void setUp() throws Exception {
+    when(server.getContextPath()).thenReturn("");
+  }
+
+  @Test
+  public void generate_state_on_secured_server() throws Exception {
+    when(server.isSecured()).thenReturn(true);
+
+    String state = underTest.generateState(response);
+    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);
+    assertThat(state).isNotEmpty();
+
+    verify(response).addCookie(cookieArgumentCaptor.capture());
+
+    verifyCookie(cookieArgumentCaptor.getValue(), false);
+  }
+
+  @Test
+  public void verify_state() throws Exception {
+    String state = "state";
+    when(request.getCookies()).thenReturn(new Cookie[] {new Cookie("OAUTHSTATE", sha256Hex(state))});
+    when(request.getParameter("state")).thenReturn(state);
+
+    underTest.verifyState(request, response);
+
+    verify(response).addCookie(cookieArgumentCaptor.capture());
+    Cookie updatedCookie = cookieArgumentCaptor.getValue();
+    assertThat(updatedCookie.getName()).isEqualTo("OAUTHSTATE");
+    assertThat(updatedCookie.getValue()).isNull();
+    assertThat(updatedCookie.getPath()).isEqualTo("/");
+    assertThat(updatedCookie.getMaxAge()).isEqualTo(0);
+  }
+
+  @Test
+  public void verify_state_when_context() throws Exception {
+    String state = "state";
+    when(request.getCookies()).thenReturn(new Cookie[] {new Cookie("OAUTHSTATE", sha256Hex(state))});
+    when(request.getParameter("state")).thenReturn(state);
+    when(server.getContextPath()).thenReturn("/sonarqube");
+
+    underTest.verifyState(request, response);
+
+    verify(response).addCookie(cookieArgumentCaptor.capture());
+    Cookie updatedCookie = cookieArgumentCaptor.getValue();
+    assertThat(updatedCookie.getPath()).isEqualTo("/sonarqube/");
+  }
+
+  @Test
+  public void fail_with_unauthorized_when_state_cookie_is_not_the_same_as_state_parameter() throws Exception {
+    when(request.getCookies()).thenReturn(new Cookie[] {new Cookie("OAUTHSTATE", sha1Hex("state"))});
+    when(request.getParameter("state")).thenReturn("other value");
+
+    thrown.expect(UnauthorizedException.class);
+    underTest.verifyState(request, response);
+  }
+
+  @Test
+  public void fail_to_verify_state_when_state_cookie_is_null() throws Exception {
+    when(request.getCookies()).thenReturn(new Cookie[] {new Cookie("OAUTHSTATE", null)});
+    when(request.getParameter("state")).thenReturn("state");
+
+    thrown.expect(UnauthorizedException.class);
+    underTest.verifyState(request, response);
+  }
+
+  @Test
+  public void fail_with_unauthorized_when_state_parameter_is_empty() throws Exception {
+    when(request.getCookies()).thenReturn(new Cookie[] {new Cookie("OAUTHSTATE", sha1Hex("state"))});
+    when(request.getParameter("state")).thenReturn("");
+
+    thrown.expect(UnauthorizedException.class);
+    underTest.verifyState(request, response);
+  }
+
+  private void verifyCookie(Cookie cookie, boolean isSecured) {
+    assertThat(cookie.getName()).isEqualTo("OAUTHSTATE");
+    assertThat(cookie.getValue()).isNotEmpty();
+    assertThat(cookie.getPath()).isEqualTo("/");
+    assertThat(cookie.isHttpOnly()).isTrue();
+    assertThat(cookie.getMaxAge()).isEqualTo(-1);
+    assertThat(cookie.getSecure()).isEqualTo(isSecured);
+  }
+}
index 4ca15d13a7199b685778cb21765c1a7539d3a5c4..83285fc1e0d0637815d3bb3b40f732666366a1d3 100644 (file)
@@ -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