From b45880c057dafd84c9e47be48a061374e316d3d1 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Mon, 17 Dec 2018 16:32:56 +0100 Subject: [PATCH] SONAR-9919 obfuscate webhook URL in api/webhooks/list response --- .../org/sonar/db/webhook/WebhookTesting.java | 14 +++++++++---- .../sonar/server/webhook/HttpUrlHelper.java | 13 ++++++++++-- .../server/webhook/WebhookCallerImpl.java | 2 +- .../server/webhook/HttpUrlHelperTest.java | 11 ++++++---- .../sonar/server/webhook/ws/ListAction.java | 4 ++-- .../server/webhook/ws/ListActionTest.java | 20 +++++++++++++++++++ 6 files changed, 51 insertions(+), 13 deletions(-) diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookTesting.java b/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookTesting.java index 58ba1e46606..886962b15be 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookTesting.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookTesting.java @@ -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... consumers) { + return getWebhookDto(consumers) .setName(name) .setOrganizationUuid(organizationUuid); } - private static WebhookDto getWebhookDto() { - return new WebhookDto() + @SafeVarargs + private static WebhookDto getWebhookDto(Consumer... 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; } } diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/webhook/HttpUrlHelper.java b/server/sonar-server-common/src/main/java/org/sonar/server/webhook/HttpUrlHelper.java index 20f38dcd5a2..fde1eb6ff4d 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/webhook/HttpUrlHelper.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/webhook/HttpUrlHelper.java @@ -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)}: *
@@ -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)) { diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookCallerImpl.java b/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookCallerImpl.java index d6c76864c5f..2346d7c98c1 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookCallerImpl.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookCallerImpl.java @@ -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()); diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/webhook/HttpUrlHelperTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/webhook/HttpUrlHelperTest.java index 16ba1a36b93..e08ff126b98 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/webhook/HttpUrlHelperTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/webhook/HttpUrlHelperTest.java @@ -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 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]")) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/webhook/ws/ListAction.java b/server/sonar-server/src/main/java/org/sonar/server/webhook/ws/ListAction.java index 42d2542f952..6e70d62769c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/webhook/ws/ListAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/webhook/ws/ListAction.java @@ -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 webhookDtos, Map 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); diff --git a/server/sonar-server/src/test/java/org/sonar/server/webhook/ws/ListActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/webhook/ws/ListActionTest.java index d9553ebe47a..7af6120595d 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/webhook/ws/ListActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/webhook/ws/ListActionTest.java @@ -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 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()); -- 2.39.5