SONAR-9896 Webhook must support basic authentication

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/webhook/WebhookCallerImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/webhook/WebhookCallerImpl.java
index 40cb2b128d8dcc1e181aebb0f53b56b92238a2db..3c14782e5868c0ded2591c7f58e03d58349612d3 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/webhook/WebhookCallerImpl.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/webhook/WebhookCallerImpl.java
@@ -20,6 +20,7 @@
 package org.sonar.server.computation.task.projectanalysis.webhook;
 
 import java.io.IOException;
+import okhttp3.Credentials;
 import okhttp3.HttpUrl;
 import okhttp3.MediaType;
 import okhttp3.OkHttpClient;
@@ -32,8 +33,10 @@ import org.sonar.api.utils.System2;
 import static java.lang.String.format;
 import static java.net.HttpURLConnection.HTTP_MOVED_PERM;
 import static java.net.HttpURLConnection.HTTP_MOVED_TEMP;
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static okhttp3.internal.http.StatusLine.HTTP_PERM_REDIRECT;
 import static okhttp3.internal.http.StatusLine.HTTP_TEMP_REDIRECT;
+import static org.apache.commons.lang.StringUtils.isNotEmpty;
 
 @ComputeEngineSide
 public class WebhookCallerImpl implements WebhookCaller {
@@ -71,9 +74,17 @@ public class WebhookCallerImpl implements WebhookCaller {
   }
 
   private static Request buildHttpRequest(Webhook webhook, WebhookPayload payload) {
+    HttpUrl url = HttpUrl.parse(webhook.getUrl());
+    if (url == null) {
+      throw new IllegalArgumentException("Webhook URL is not valid: " + webhook.getUrl());
+    }
     Request.Builder request = new Request.Builder();
-    request.url(webhook.getUrl());
+    request.url(url);
     request.header(PROJECT_KEY_HEADER, payload.getProjectKey());
+    if (isNotEmpty(url.username())) {
+      request.header("Authorization", Credentials.basic(url.username(), url.password(), UTF_8));
+    }
+
     RequestBody body = RequestBody.create(JSON, payload.getJson());
     request.post(body);
     return request.build();

diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/webhook/WebhookCallerImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/webhook/WebhookCallerImplTest.java
index aff151adb4892075353f2e35be9351049a61bb5d..155ec647963d7ec492b791e8aea9369ffe4ea376 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/webhook/WebhookCallerImplTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/webhook/WebhookCallerImplTest.java
@@ -19,6 +19,7 @@
  */
 package org.sonar.server.computation.task.projectanalysis.webhook;
 
+import okhttp3.Credentials;
 import okhttp3.HttpUrl;
 import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.MockWebServer;
@@ -103,7 +104,7 @@ public class WebhookCallerImplTest {
     assertThat(delivery.getHttpStatus()).isEmpty();
     assertThat(delivery.getDurationInMs()).isEmpty();
     assertThat(delivery.getError().get()).isInstanceOf(IllegalArgumentException.class);
-    assertThat(delivery.getErrorMessage().get()).isEqualTo("unexpected url: this_is_not_an_url");
+    assertThat(delivery.getErrorMessage().get()).isEqualTo("Webhook URL is not valid: this_is_not_an_url");
     assertThat(delivery.getAt()).isEqualTo(NOW);
     assertThat(delivery.getWebhook()).isSameAs(webhook);
     assertThat(delivery.getPayload()).isSameAs(PAYLOAD);
@@ -133,6 +134,26 @@ public class WebhookCallerImplTest {
     takeAndVerifyPostRequest("/target");
   }
 
+  @Test
+  public void credentials_are_propagated_to_POST_redirects() throws Exception {
+    HttpUrl url = server.url("/redirect").newBuilder().username("theLogin").password("thePassword").build();
+    Webhook webhook = new Webhook(PROJECT_UUID, CE_TASK_UUID, "my-webhook", url.toString());
+
+    // /redirect redirects to /target
+    server.enqueue(new MockResponse().setResponseCode(307).setHeader("Location", server.url("target")));
+    server.enqueue(new MockResponse().setResponseCode(200));
+
+    WebhookDelivery delivery = newSender().call(webhook, PAYLOAD);
+
+    assertThat(delivery.getHttpStatus().get()).isEqualTo(200);
+
+    RecordedRequest redirectedRequest = takeAndVerifyPostRequest("/redirect");
+    assertThat(redirectedRequest.getHeader("Authorization")).isEqualTo(Credentials.basic(url.username(), url.password()));
+
+    RecordedRequest targetRequest = takeAndVerifyPostRequest("/target");
+    assertThat(targetRequest.getHeader("Authorization")).isEqualTo(Credentials.basic(url.username(), url.password()));
+  }
+
   @Test
   public void redirects_throws_ISE_if_header_Location_is_missing() throws Exception {
     HttpUrl url = server.url("/redirect");
@@ -163,11 +184,28 @@ public class WebhookCallerImplTest {
       .hasMessage("Unsupported protocol in redirect of " + url + " to ftp://foo");
   }
 
-  private void takeAndVerifyPostRequest(String expectedPath) throws Exception {
-    RecordedRequest redirectedRequest = server.takeRequest();
+  @Test
+  public void send_basic_authentication_header_if_url_contains_credentials() throws Exception {
+    HttpUrl url = server.url("/ping").newBuilder().username("theLogin").password("thePassword").build();
+    Webhook webhook = new Webhook(PROJECT_UUID, CE_TASK_UUID, "my-webhook", url.toString());
+    server.enqueue(new MockResponse().setBody("pong"));
+
+    WebhookDelivery delivery = newSender().call(webhook, PAYLOAD);
+
+    assertThat(delivery.getWebhook().getUrl())
+      .isEqualTo(url.toString())
+      .contains("://theLogin:thePassword@");
+    RecordedRequest recordedRequest = takeAndVerifyPostRequest("/ping");
+    assertThat(recordedRequest.getHeader("Authorization")).isEqualTo(Credentials.basic(url.username(), url.password()));
+  }
+
+  private RecordedRequest takeAndVerifyPostRequest(String expectedPath) throws Exception {
+    RecordedRequest request = server.takeRequest();
 
-    assertThat(redirectedRequest.getMethod()).isEqualTo("POST");
-    assertThat(redirectedRequest.getPath()).isEqualTo(expectedPath);
+    assertThat(request.getMethod()).isEqualTo("POST");
+    assertThat(request.getPath()).isEqualTo(expectedPath);
+    assertThat(request.getHeader("User-Agent")).isEqualTo("SonarQube/6.2");
+    return request;
   }
 
   private WebhookCaller newSender() {

diff --git a/sonar-core/src/main/java/org/sonar/core/config/WebhookProperties.java b/sonar-core/src/main/java/org/sonar/core/config/WebhookProperties.java
index 91fb4c141e8ce6ec19788651982e0c2d0e494f42..f3e3b98742d4f8b58c9a602eb36894798213406a 100644 (file)
--- a/sonar-core/src/main/java/org/sonar/core/config/WebhookProperties.java
+++ b/sonar-core/src/main/java/org/sonar/core/config/WebhookProperties.java
@@ -51,6 +51,9 @@ public class WebhookProperties {
   private static final String DESCRIPTION = "Webhooks are used to notify external services when a project analysis is done. " +
     "An HTTP POST request including a JSON payload is sent to each of the first ten provided URLs. <br/>" +
     "Learn more in the <a href=\"https://redirect.sonarsource.com/doc/webhooks.html\">Webhooks documentation</a>.";
+  private static final String URL_DESCRIPTION = "Server endpoint that will receive the webhook payload, for example 'http://my_server/foo'. " +
+    "If HTTP Basic authentication is used, HTTPS is recommended to avoid man in the middle attacks. " +
+    "Example: 'https://myLogin:myPassword@my_server/foo'";
 
   private WebhookProperties() {
     // only static stuff
@@ -70,6 +73,7 @@ public class WebhookProperties {
           PropertyFieldDefinition.build(URL_FIELD)
             .name("URL")
             .type(PropertyType.STRING)
+            .description(URL_DESCRIPTION)
             .build())
         .build(),
 
@@ -86,7 +90,9 @@ public class WebhookProperties {
           PropertyFieldDefinition.build(URL_FIELD)
             .name("URL")
             .type(PropertyType.STRING)
+            .description(URL_DESCRIPTION)
             .build())
         .build());
   }
+
 }

diff --git a/tests/src/test/java/org/sonarqube/tests/user/LocalAuthenticationTest.java b/tests/src/test/java/org/sonarqube/tests/user/LocalAuthenticationTest.java
index d5b8a50240c7a94a404c3d3e33938145d69b66bb..7c17b06599a5e7f19932ad65b6805074313003e3 100644 (file)
--- a/tests/src/test/java/org/sonarqube/tests/user/LocalAuthenticationTest.java
+++ b/tests/src/test/java/org/sonarqube/tests/user/LocalAuthenticationTest.java
@@ -139,7 +139,7 @@ public class LocalAuthenticationTest {
   }
 
   @Test
-  public void basic_authentication_does_not_support_utf8_passwords() {
+  public void basic_authentication_supports_utf8_passwords() {
     String login = "user_with_utf8_password";
     // see http://www.cl.cam.ac.uk/~mgk25/ucs/examples/UTF-8-test.txt
     String password = "κόσμε";
@@ -148,7 +148,7 @@ public class LocalAuthenticationTest {
     tester.users().generate(u -> u.setLogin(login).setPassword(password));
 
     // authenticate
-    assertThat(checkAuthenticationWithAuthenticateWebService(login, password)).isFalse();
+    assertThat(checkAuthenticationWithAuthenticateWebService(login, password)).isTrue();
   }
 
   @Test

diff --git a/tests/src/test/java/org/sonarqube/tests/webhook/WebhooksTest.java b/tests/src/test/java/org/sonarqube/tests/webhook/WebhooksTest.java
index 975a5dc93664e148a3fa714f34f1836bff84d08d..2539ab6bb6dfb19e94d06153ffef58ec3aade15b 100644 (file)
--- a/tests/src/test/java/org/sonarqube/tests/webhook/WebhooksTest.java
+++ b/tests/src/test/java/org/sonarqube/tests/webhook/WebhooksTest.java
@@ -195,8 +195,7 @@ public class WebhooksTest {
     assertThat(detail.getPayload()).isNotEmpty();
     assertThat(detail.getErrorStacktrace())
       .contains("java.lang.IllegalArgumentException")
-      .contains("unexpected url")
-      .contains("this_is_not_an_url");
+      .contains("Webhook URL is not valid: this_is_not_an_url");
   }
 
   @Test