]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-9919 obfuscate webhook URL in api/webhooks/list response
authorSébastien Lesaint <sebastien.lesaint@sonarsource.com>
Mon, 17 Dec 2018 15:32:56 +0000 (16:32 +0100)
committersonartech <sonartech@sonarsource.com>
Thu, 20 Dec 2018 10:41:48 +0000 (11:41 +0100)
server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookTesting.java
server/sonar-server-common/src/main/java/org/sonar/server/webhook/HttpUrlHelper.java
server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookCallerImpl.java
server/sonar-server-common/src/test/java/org/sonar/server/webhook/HttpUrlHelperTest.java
server/sonar-server/src/main/java/org/sonar/server/webhook/ws/ListAction.java
server/sonar-server/src/test/java/org/sonar/server/webhook/ws/ListActionTest.java

index 58ba1e46606504823b9fb909eeab7fa36a57a447..886962b15be7a2789cb44bb7fd5d960b03928b88 100644 (file)
@@ -19,6 +19,8 @@
  */
 package org.sonar.db.webhook;
 
+import java.util.Arrays;
+import java.util.function.Consumer;
 import org.sonar.db.component.ComponentDto;
 import org.sonar.db.organization.OrganizationDto;
 
@@ -47,17 +49,21 @@ public class WebhookTesting {
       .setOrganizationUuid(organizationDto.getUuid());
   }
 
-  public static WebhookDto newOrganizationWebhook(String name, String organizationUuid) {
-    return getWebhookDto()
+  @SafeVarargs
+  public static WebhookDto newOrganizationWebhook(String name, String organizationUuid, Consumer<WebhookDto>... consumers) {
+    return getWebhookDto(consumers)
             .setName(name)
             .setOrganizationUuid(organizationUuid);
   }
 
-  private static WebhookDto getWebhookDto() {
-    return new WebhookDto()
+  @SafeVarargs
+  private static WebhookDto getWebhookDto(Consumer<WebhookDto>... consumers) {
+    WebhookDto res = new WebhookDto()
       .setUuid(randomAlphanumeric(40))
       .setName(randomAlphanumeric(64))
       .setUrl("https://www.random-site/" + randomAlphanumeric(256))
       .setCreatedAt(Calendar.getInstance().getTimeInMillis());
+    Arrays.stream(consumers).forEach(consumer -> consumer.accept(res));
+    return res;
   }
 }
index 20f38dcd5a2da192c3790e8303d58229e1c8f67a..fde1eb6ff4d7f49666ccbbb470b5595cb07d4956 100644 (file)
@@ -29,11 +29,19 @@ import okhttp3.HttpUrl;
 import static com.google.common.base.Preconditions.checkState;
 import static org.apache.commons.lang.StringUtils.repeat;
 
-final class HttpUrlHelper {
+public final class HttpUrlHelper {
   private HttpUrlHelper() {
     // prevents instantiation
   }
 
+  public static String obfuscateCredentials(String originalUrl) {
+    HttpUrl parsedUrl = HttpUrl.parse(originalUrl);
+    if (parsedUrl != null) {
+      return obfuscateCredentials(originalUrl, parsedUrl);
+    }
+    return originalUrl;
+  }
+
   /**
    * According to inline comment in {@link okhttp3.HttpUrl.Builder#parse(HttpUrl base, String input)}:
    * <blockquote>
@@ -44,7 +52,7 @@ final class HttpUrlHelper {
    * This function replaces the chars of the username and the password from the {@code originalUrl} by '*' chars
    * based on username and password parsed in {@code parsedUrl}.
    */
-  public static String toEffectiveUrl(String originalUrl, HttpUrl parsedUrl) {
+  static String obfuscateCredentials(String originalUrl, HttpUrl parsedUrl) {
     String username = parsedUrl.username();
     String password = parsedUrl.password();
     if (username.isEmpty() && password.isEmpty()) {
@@ -93,6 +101,7 @@ final class HttpUrlHelper {
     return authentStringOf(repeat("*", userName.length()), password == null ? null : repeat("*", password.length()));
   }
 
+  @CheckForNull
   private static String replaceOrDieImpl(String original, String target, String replacement) {
     String res = original.replace(target, replacement);
     if (!res.equals(original)) {
index d6c76864c5fa7116ef7de92adf6d0db50c4a3cd3..2346d7c98c13e7d9930b1c03c3e49f4278d0dde9 100644 (file)
@@ -68,7 +68,7 @@ public class WebhookCallerImpl implements WebhookCaller {
       if (url == null) {
         throw new IllegalArgumentException("Webhook URL is not valid: " + webhook.getUrl());
       }
-      builder.setEffectiveUrl(HttpUrlHelper.toEffectiveUrl(webhook.getUrl(), url));
+      builder.setEffectiveUrl(HttpUrlHelper.obfuscateCredentials(webhook.getUrl(), url));
       Request request = buildHttpRequest(url, payload);
       try (Response response = execute(request)) {
         builder.setHttpStatus(response.code());
index 16ba1a36b938d1238f61b0259e09e4dc3ab82336..e08ff126b98e9cae02f4e841b81af7675be5d086 100644 (file)
@@ -31,18 +31,21 @@ import org.junit.runner.RunWith;
 
 import static org.apache.commons.lang.StringUtils.repeat;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.sonar.server.webhook.HttpUrlHelper.obfuscateCredentials;
 
 @RunWith(DataProviderRunner.class)
 public class HttpUrlHelperTest {
 
   @Test
-  @UseDataProvider("toEffectiveUrlUseCases")
-  public void verify_toEffectiveUrl(String originalUrl, String expectedUrl) {
-    assertThat(HttpUrlHelper.toEffectiveUrl(originalUrl, HttpUrl.parse(originalUrl))).isEqualTo(expectedUrl);
+  @UseDataProvider("obfuscateCredentialsUseCases")
+  public void verify_obfuscateCredentials(String originalUrl, String expectedUrl) {
+    assertThat(obfuscateCredentials(originalUrl, HttpUrl.parse(originalUrl)))
+      .isEqualTo(obfuscateCredentials(originalUrl))
+      .isEqualTo(expectedUrl);
   }
 
   @DataProvider
-  public static Object[][] toEffectiveUrlUseCases() {
+  public static Object[][] obfuscateCredentialsUseCases() {
     List<Object[]> rows = new ArrayList<>();
     for (String before : Arrays.asList("http://", "https://")) {
       for (String host : Arrays.asList("foo", "127.0.0.1", "[2001:db8:85a3:0:0:8a2e:370:7334]", "[2001:0db8:85a3:0000:0000:8a2e:0370:7334]")) {
index 42d2542f952446204981de4863cd64e5db338512..6e70d62769c89c9d32cd6ff62577365badb236ff 100644 (file)
@@ -42,6 +42,7 @@ import org.sonarqube.ws.Webhooks.ListResponseElement;
 import static java.util.Optional.ofNullable;
 import static org.apache.commons.lang.StringUtils.isNotBlank;
 import static org.sonar.api.utils.DateUtils.formatDateTime;
+import static org.sonar.server.webhook.HttpUrlHelper.obfuscateCredentials;
 import static org.sonar.server.webhook.ws.WebhooksWsParameters.LIST_ACTION;
 import static org.sonar.server.webhook.ws.WebhooksWsParameters.ORGANIZATION_KEY_PARAM;
 import static org.sonar.server.webhook.ws.WebhooksWsParameters.PROJECT_KEY_PARAM;
@@ -138,13 +139,12 @@ public class ListAction implements WebhooksWsAction {
   private static void writeResponse(Request request, Response response, List<WebhookDto> webhookDtos, Map<String, WebhookDeliveryLiteDto> lastDeliveries) {
     ListResponse.Builder responseBuilder = ListResponse.newBuilder();
     webhookDtos
-      .stream()
       .forEach(webhook -> {
         ListResponseElement.Builder responseElementBuilder = responseBuilder.addWebhooksBuilder();
         responseElementBuilder
           .setKey(webhook.getUuid())
           .setName(webhook.getName())
-          .setUrl(webhook.getUrl());
+          .setUrl(obfuscateCredentials(webhook.getUrl()));
         addLastDelivery(responseElementBuilder, webhook, lastDeliveries);
       });
     writeProtobuf(responseBuilder.build(), request, response);
index d9553ebe47a41929b9698e8b682f559c114125a1..7af6120595daef5c74a58760c26d346e2413f3c0 100644 (file)
@@ -147,6 +147,26 @@ public class ListActionTest {
     assertThat(elements.get(1).hasLatestDelivery()).isFalse();
   }
 
+  @Test
+  public void obfuscate_credentials_in_webhook_URLs() {
+    String url = "http://foo:barouf@toto/bop";
+    String expectedUrl = "http://***:******@toto/bop";
+    WebhookDto webhook1 = webhookDbTester.insert(newOrganizationWebhook("aaa", defaultOrganizationProvider.get().getUuid(), t -> t.setUrl(url)));
+    webhookDeliveryDbTester.insert(newDto("WH1-DELIVERY-1-UUID", webhook1.getUuid(), "COMPONENT_1", "TASK_1").setCreatedAt(BEFORE));
+    webhookDeliveryDbTester.insert(newDto("WH1-DELIVERY-2-UUID", webhook1.getUuid(), "COMPONENT_1", "TASK_2").setCreatedAt(NOW));
+    WebhookDto webhook2 = webhookDbTester.insert(newOrganizationWebhook("bbb", db.getDefaultOrganization().getUuid(), t -> t.setUrl(url)));
+
+    userSession.logIn().addPermission(ADMINISTER, db.getDefaultOrganization().getUuid());
+
+    ListResponse response = wsActionTester.newRequest().executeProtobuf(ListResponse.class);
+
+    List<Webhooks.ListResponseElement> elements = response.getWebhooksList();
+    assertThat(elements)
+      .hasSize(2)
+      .extracting(Webhooks.ListResponseElement::getUrl)
+      .containsOnly(expectedUrl);
+  }
+
   @Test
   public void list_global_webhooks() {
     WebhookDto dto1 = webhookDbTester.insertWebhook(db.getDefaultOrganization());