Skip to content

Commit

Permalink
SONAR-9919 obfuscate credentials in webhook delivery logs
Browse files Browse the repository at this point in the history
  • Loading branch information
sns-seb authored and sonartech committed Dec 20, 2018
1 parent 9c605f1 commit 5abdde6
Show file tree
Hide file tree
Showing 6 changed files with 212 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* SonarQube
* Copyright (C) 2009-2018 SonarSource SA
* mailto:info 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.webhook;

import com.google.common.base.Supplier;
import java.util.Objects;
import java.util.stream.Stream;
import javax.annotation.CheckForNull;
import javax.annotation.Nullable;
import okhttp3.HttpUrl;

import static com.google.common.base.Preconditions.checkState;
import static org.apache.commons.lang.StringUtils.repeat;

final class HttpUrlHelper {
private HttpUrlHelper() {
// prevents instantiation
}

/**
* According to inline comment in {@link okhttp3.HttpUrl.Builder#parse(HttpUrl base, String input)}:
* <blockquote>
* Username, password and port are optional.
* [username[:password]@]host[:port]
* </blockquote>
* <p>
* 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) {
String username = parsedUrl.username();
String password = parsedUrl.password();
if (username.isEmpty() && password.isEmpty()) {
return originalUrl;
}

if (!username.isEmpty() && !password.isEmpty()) {
String encodedUsername = parsedUrl.encodedUsername();
String encodedPassword = parsedUrl.encodedPassword();
return Stream.<Supplier<String>>of(
() -> replaceOrDie(originalUrl, username, password),
() -> replaceOrDie(originalUrl, encodedUsername, encodedPassword),
() -> replaceOrDie(originalUrl, encodedUsername, password),
() -> replaceOrDie(originalUrl, username, encodedPassword))
.map(Supplier::get)
.filter(Objects::nonNull)
.findFirst()
.orElse(originalUrl);
}
if (!username.isEmpty()) {
return Stream.<Supplier<String>>of(
() -> replaceOrDie(originalUrl, username, null),
() -> replaceOrDie(originalUrl, parsedUrl.encodedUsername(), null))
.map(Supplier::get)
.filter(Objects::nonNull)
.findFirst()
.orElse(originalUrl);
}
checkState(password.isEmpty(), "having a password without a username should never occur");
return originalUrl;
}

@CheckForNull
private static String replaceOrDie(String original, String username, @Nullable String password) {
return replaceOrDieImpl(original, authentStringOf(username, password), obfuscatedAuthentStringOf(username, password));
}

private static String authentStringOf(String username, @Nullable String password) {
if (password == null) {
return username + "@";
}
return username + ":" + password + "@";
}

private static String obfuscatedAuthentStringOf(String userName, @Nullable String password) {
return authentStringOf(repeat("*", userName.length()), password == null ? null : repeat("*", password.length()));
}

private static String replaceOrDieImpl(String original, String target, String replacement) {
String res = original.replace(target, replacement);
if (!res.equals(original)) {
return res;
}
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,12 @@ public WebhookDelivery call(Webhook webhook, WebhookPayload payload) {
.setWebhook(webhook);

try {
Request request = buildHttpRequest(webhook, payload);
HttpUrl url = HttpUrl.parse(webhook.getUrl());
if (url == null) {
throw new IllegalArgumentException("Webhook URL is not valid: " + webhook.getUrl());
}
builder.setEffectiveUrl(HttpUrlHelper.toEffectiveUrl(webhook.getUrl(), url));
Request request = buildHttpRequest(url, payload);
try (Response response = execute(request)) {
builder.setHttpStatus(response.code());
}
Expand All @@ -77,11 +82,7 @@ public WebhookDelivery call(Webhook webhook, WebhookPayload payload) {
.build();
}

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());
}
private static Request buildHttpRequest(HttpUrl url, WebhookPayload payload) {
Request.Builder request = new Request.Builder();
request.url(url);
request.header(PROJECT_KEY_HEADER, payload.getProjectKey());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public class WebhookDelivery {

private final Webhook webhook;
private final WebhookPayload payload;
private final String effectiveUrl;
private final Integer httpStatus;
private final Integer durationInMs;
private final long at;
Expand All @@ -42,6 +43,7 @@ public class WebhookDelivery {
private WebhookDelivery(Builder builder) {
this.webhook = requireNonNull(builder.webhook);
this.payload = requireNonNull(builder.payload);
this.effectiveUrl = builder.effectiveUrl;
this.httpStatus = builder.httpStatus;
this.durationInMs = builder.durationInMs;
this.at = builder.at;
Expand All @@ -56,6 +58,10 @@ public WebhookPayload getPayload() {
return payload;
}

public Optional<String> getEffectiveUrl() {
return Optional.ofNullable(effectiveUrl);
}

/**
* @return the HTTP status if {@link #getError()} is empty, else returns
* {@link Optional#empty()}
Expand Down Expand Up @@ -101,6 +107,7 @@ public boolean isSuccess() {
public static class Builder {
private Webhook webhook;
private WebhookPayload payload;
private String effectiveUrl;
private Integer httpStatus;
private Integer durationInMs;
private long at;
Expand All @@ -116,6 +123,11 @@ public Builder setPayload(WebhookPayload payload) {
return this;
}

public Builder setEffectiveUrl(@Nullable String effectiveUrl) {
this.effectiveUrl = effectiveUrl;
return this;
}

public Builder setHttpStatus(@Nullable Integer httpStatus) {
this.httpStatus = httpStatus;
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ private WebhookDeliveryDto toDto(WebhookDelivery delivery) {
delivery.getWebhook().getCeTaskUuid().ifPresent(dto::setCeTaskUuid);
delivery.getWebhook().getAnalysisUuid().ifPresent(dto::setAnalysisUuid);
dto.setName(delivery.getWebhook().getName());
dto.setUrl(delivery.getWebhook().getUrl());
dto.setUrl(delivery.getEffectiveUrl().orElse(delivery.getWebhook().getUrl()));
dto.setSuccess(delivery.isSuccess());
dto.setHttpStatus(delivery.getHttpStatus().orElse(null));
dto.setDurationMs(delivery.getDurationInMs().orElse(null));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* SonarQube
* Copyright (C) 2009-2018 SonarSource SA
* mailto:info 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.webhook;

import com.tngtech.java.junit.dataprovider.DataProvider;
import com.tngtech.java.junit.dataprovider.DataProviderRunner;
import com.tngtech.java.junit.dataprovider.UseDataProvider;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import okhttp3.HttpUrl;
import org.junit.Test;
import org.junit.runner.RunWith;

import static org.apache.commons.lang.StringUtils.repeat;
import static org.assertj.core.api.Assertions.assertThat;

@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);
}

@DataProvider
public static Object[][] toEffectiveUrlUseCases() {
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]")) {
for (String port : Arrays.asList("", ":123")) {
for (String after : Arrays.asList("", "/", "/bar", "/bar/", "?", "?a=b", "?a=b&c=d")) {
for (String username : Arrays.asList("", "us", "a b", "a%20b")) {
for (String password : Arrays.asList("", "pwd", "pwd%20k", "pwd k", "c:d")) {
if (username.isEmpty()) {
String url = before + host + port + after;
rows.add(new Object[] {url, url});
} else if (password.isEmpty()) {
String url = before + username + '@' + host + port + after;
String expected = before + repeat("*", username.length()) + '@' + host + port + after;
rows.add(new Object[] {url, expected});
} else {
String url = before + username + ':' + password + '@' + host + port + after;
String expected = before + repeat("*", username.length()) + ':' + repeat("*", password.length()) + '@' + host + port + after;
rows.add(new Object[] {url, expected});
}
}
}
}
}
}
}
return rows.toArray(new Object[0][]);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.sonar.db.webhook.WebhookDeliveryDto;
import org.sonar.db.webhook.WebhookDeliveryTesting;

import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -101,6 +102,20 @@ public void purge_deletes_records_older_than_one_month_on_the_project() {
assertThat(selectAllDeliveryUuids(dbTester, dbSession)).containsOnly("D2", "D3");
}

@Test
public void persist_effective_url_if_present() {
when(uuidFactory.create()).thenReturn(DELIVERY_UUID);
String effectiveUrl = randomAlphabetic(15);
WebhookDelivery delivery = newBuilderTemplate()
.setEffectiveUrl(effectiveUrl)
.build();

underTest.persist(delivery);

WebhookDeliveryDto dto = dbClient.webhookDeliveryDao().selectByUuid(dbSession, DELIVERY_UUID).get();
assertThat(dto.getUrl()).isEqualTo(effectiveUrl);
}

private static WebhookDelivery.Builder newBuilderTemplate() {
return new WebhookDelivery.Builder()
.setWebhook(new Webhook("WEBHOOK_UUID_1", "COMPONENT1", "TASK1", RandomStringUtils.randomAlphanumeric(40),"Jenkins", "http://jenkins"))
Expand Down

0 comments on commit 5abdde6

Please sign in to comment.