]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-11072 Allow customization of request parameter used to check CSRF state
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Thu, 26 Jul 2018 12:47:51 +0000 (14:47 +0200)
committerSonarTech <sonartech@sonarsource.com>
Tue, 31 Jul 2018 18:21:25 +0000 (20:21 +0200)
server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2CallbackFilter.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
server/sonar-server/src/test/java/org/sonar/server/authentication/OAuthCsrfVerifierTest.java
sonar-plugin-api/src/main/java/org/sonar/api/server/authentication/OAuth2IdentityProvider.java

index 0712cc0b52d6378b5eeedd8c24422c1ad408350e..1b2944cc9f03a7e188bfef5490f61fba29b10ce2 100644 (file)
@@ -152,6 +152,11 @@ public class OAuth2CallbackFilter extends AuthenticationFilter {
       delegate.verifyCsrfState();
     }
 
+    @Override
+    public void verifyCsrfState(String parameterName) {
+      delegate.verifyCsrfState(parameterName);
+    }
+
     @Override
     public void redirectToRequestedPage() {
       delegate.redirectToRequestedPage();
index 674bc6a562965355c81411dc4f89d017d14d4c4f..26549396c6d129021c93ac001fd5f0216f0d9ec9 100644 (file)
@@ -113,6 +113,11 @@ public class OAuth2ContextFactory {
       csrfVerifier.verifyState(request, response, identityProvider);
     }
 
+    @Override
+    public void verifyCsrfState(String parameterName) {
+      csrfVerifier.verifyState(request, response, identityProvider, parameterName);
+    }
+
     @Override
     public void redirectToRequestedPage() {
       try {
index 297f52dea50965d3c42163ccfd7c976594760330..953a2ff89f63aba0f92fb13f89963eb5fc816106 100644 (file)
@@ -37,6 +37,7 @@ import static org.sonar.server.authentication.event.AuthenticationEvent.Source;
 public class OAuthCsrfVerifier {
 
   private static final String CSRF_STATE_COOKIE = "OAUTHSTATE";
+  private static final String DEFAULT_STATE_PARAMETER_NAME = "state";
 
   public String generateState(HttpServletRequest request, HttpServletResponse response) {
     // Create a state token to prevent request forgery.
@@ -47,6 +48,10 @@ public class OAuthCsrfVerifier {
   }
 
   public void verifyState(HttpServletRequest request, HttpServletResponse response, OAuth2IdentityProvider provider) {
+    verifyState(request, response, provider, DEFAULT_STATE_PARAMETER_NAME);
+  }
+
+  public void verifyState(HttpServletRequest request, HttpServletResponse response, OAuth2IdentityProvider provider, String parameterName) {
     Cookie cookie = findCookie(CSRF_STATE_COOKIE, request)
       .orElseThrow(AuthenticationException.newBuilder()
         .setSource(Source.oauth2(provider))
@@ -56,7 +61,7 @@ public class OAuthCsrfVerifier {
     // remove cookie
     response.addCookie(newCookieBuilder(request).setName(CSRF_STATE_COOKIE).setValue(null).setHttpOnly(true).setExpiry(0).build());
 
-    String stateInRequest = request.getParameter("state");
+    String stateInRequest = request.getParameter(parameterName);
     if (isBlank(stateInRequest) || !sha256Hex(stateInRequest).equals(hashInCookie)) {
       throw AuthenticationException.newBuilder()
         .setSource(Source.oauth2(provider))
index 0ae647000605610b88aa7b7c8cf28efb10fae442..7dc94f061c2164d3acd644e7bf7acfa7d116594e 100644 (file)
@@ -72,6 +72,22 @@ public class OAuthCsrfVerifierTest {
 
   @Test
   public void verify_state() {
+    String state = "state";
+    when(request.getCookies()).thenReturn(new Cookie[] {new Cookie("OAUTHSTATE", sha256Hex(state))});
+    when(request.getParameter("aStateParameter")).thenReturn(state);
+
+    underTest.verifyState(request, response, identityProvider, "aStateParameter");
+
+    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_using_default_state_parameter() {
     String state = "state";
     when(request.getCookies()).thenReturn(new Cookie[] {new Cookie("OAUTHSTATE", sha256Hex(state))});
     when(request.getParameter("state")).thenReturn(state);
index ac9395efa24bf6e645914ded13a1899ddacd67cb..6ef11a059fdcb1f9ca76f169b853dd8204bb6f59 100644 (file)
@@ -76,10 +76,20 @@ public interface OAuth2IdentityProvider extends IdentityProvider {
 
     /**
      * Check that the state is valid.
+     * The state will be read from the 'state' parameter of the HTTP request
+     *
      * It should only be called If {@link InitContext#generateCsrfState()} was used in the init
      */
     void verifyCsrfState();
 
+    /**
+     * Check that the state is valid
+     * The state will be read from the given parameter name of the HTTP request
+     *
+     * It should only be called If {@link InitContext#generateCsrfState()} was used in the init
+     */
+    void verifyCsrfState(String parameterName);
+
     /**
      * Redirect the request to the requested page.
      * Must be called at the end of {@link OAuth2IdentityProvider#callback(CallbackContext)}