From 7b297b7bb6287d8b5f6e0d6c66f210c062f7d1a1 Mon Sep 17 00:00:00 2001 From: Belen Pruvost Date: Thu, 29 Apr 2021 17:30:41 +0200 Subject: [PATCH] SONAR-14682 - Make webhook validation configurable --- .../db/migration/version/v84/DbVersion84.java | 2 - .../version/v84/DropLocalWebhooks.java | 72 --------- .../db/migration/version/v89/DbVersion89.java | 1 - .../version/v89/DropLocalWebhooks.java | 82 ---------- .../version/v84/DropLocalWebhooksTest.java | 132 --------------- .../version/v89/DropLocalWebhooksTest.java | 153 ------------------ .../v84/DropLocalWebhooksTest/schema.sql | 29 ---- .../v89/DropLocalWebhooksTest/schema.sql | 29 ---- .../src/pages/setup/upgrade-notes.md | 4 + .../org/sonar/process/ProcessProperties.java | 1 - .../server/webhook/WebhookCustomDns.java | 5 +- .../server/webhook/WebhookCallerImplTest.java | 7 +- .../server/webhook/WebhookCustomDnsTest.java | 22 ++- .../server/webhook/ws/WebhookSupport.java | 6 +- .../sonar/core/config/SecurityProperties.java | 29 +++- .../config/CorePropertyDefinitionsTest.java | 2 +- .../java/org/sonar/api/CoreProperties.java | 2 + 17 files changed, 55 insertions(+), 523 deletions(-) delete mode 100644 server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v84/DropLocalWebhooks.java delete mode 100644 server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v89/DropLocalWebhooks.java delete mode 100644 server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v84/DropLocalWebhooksTest.java delete mode 100644 server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v89/DropLocalWebhooksTest.java delete mode 100644 server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v84/DropLocalWebhooksTest/schema.sql delete mode 100644 server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v89/DropLocalWebhooksTest/schema.sql diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v84/DbVersion84.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v84/DbVersion84.java index 8785f8d7a7b..d08316d59a9 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v84/DbVersion84.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v84/DbVersion84.java @@ -786,8 +786,6 @@ public class DbVersion84 implements DbVersion { .add(3804, "Populate 'need_issue_sync' of 'project_branches'", PopulateProjectBranchesNeedIssueSync.class) .add(3805, "Make 'need_issue_sync' of 'project_branches' not null", MakeProjectBranchesNeedIssueSyncNonNull.class) - .add(3806, "Drop local webhooks", DropLocalWebhooks.class) - // Migration of ALM_SETTINGS table .add(3807, "Add columns 'CLIENT_ID' and 'CLIENT_SECRET' to 'ALM_SETTINGS' table", AddClientIdAndClientSecretColumns.class) diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v84/DropLocalWebhooks.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v84/DropLocalWebhooks.java deleted file mode 100644 index efac912e47e..00000000000 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v84/DropLocalWebhooks.java +++ /dev/null @@ -1,72 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2021 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.platform.db.migration.version.v84; - -import java.net.InetAddress; -import java.net.MalformedURLException; -import java.net.URL; -import java.net.UnknownHostException; -import java.sql.SQLException; -import org.sonar.api.utils.log.Logger; -import org.sonar.api.utils.log.Loggers; -import org.sonar.db.Database; -import org.sonar.server.platform.db.migration.step.DataChange; -import org.sonar.server.platform.db.migration.step.MassUpdate; - -public class DropLocalWebhooks extends DataChange { - private static final Logger LOG = Loggers.get(DropLocalWebhooks.class); - - public DropLocalWebhooks(Database db) { - super(db); - } - - @Override - protected void execute(Context context) throws SQLException { - MassUpdate massUpdate = context.prepareMassUpdate(); - massUpdate.select("select w.uuid, w.name, w.url, w.project_uuid, p.name from webhooks w left join projects p on p.uuid = w.project_uuid"); - massUpdate.update("delete from webhooks where uuid = ?"); - massUpdate.execute((row, update) -> { - try { - String webhookName = row.getString(2); - String webhookUrl = row.getString(3); - URL url = new URL(webhookUrl); - InetAddress address = InetAddress.getByName(url.getHost()); - if (address.isLoopbackAddress() || address.isAnyLocalAddress()) { - boolean projectLevel = row.getString(4) != null; - if (projectLevel) { - String projectName = row.getString(5); - LOG.warn("Webhook '{}' for project '{}' has been removed because it used an invalid, unsafe URL. Please recreate " + - "this webhook with a valid URL or ask a project administrator to do it if it is still needed.", webhookName, projectName); - } else { - LOG.warn("Global webhook '{}' has been removed because it used an invalid, unsafe URL. Please recreate this webhook with a valid URL" + - " if it is still needed.", webhookName); - } - - update.setString(1, row.getString(1)); - return true; - } - } catch (MalformedURLException | UnknownHostException e) { - return false; - } - - return false; - }); - } -} diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v89/DbVersion89.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v89/DbVersion89.java index ca3f5e06407..ee063aa1594 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v89/DbVersion89.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v89/DbVersion89.java @@ -28,7 +28,6 @@ public class DbVersion89 implements DbVersion { public void addSteps(MigrationStepRegistry registry) { registry .add(4400, "Add indices on columns 'type' and 'value' to 'new_code_periods' table", AddIndicesToNewCodePeriodTable.class) - .add(4401, "Drop local webhooks", DropLocalWebhooks.class) .add(4402, "Add Index on column 'main_branch_project_uuid' to 'components' table", AddMainBranchProjectUuidIndexToComponentTable.class) .add(4403, "Drop Github endpoint on project level setting", DropGithubEndpointOnProjectLevelSetting.class) .add(4404, "Increase size of 'value' column in 'new_code_periods' table ", IncreaseSizeOfValueColumnInNewCodePeriodsTable.class) diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v89/DropLocalWebhooks.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v89/DropLocalWebhooks.java deleted file mode 100644 index 41b77b1dc66..00000000000 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v89/DropLocalWebhooks.java +++ /dev/null @@ -1,82 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2021 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.platform.db.migration.version.v89; - -import java.net.InetAddress; -import java.net.MalformedURLException; -import java.net.SocketException; -import java.net.URL; -import java.net.UnknownHostException; -import java.sql.SQLException; -import org.sonar.api.utils.log.Logger; -import org.sonar.api.utils.log.Loggers; -import org.sonar.db.Database; -import org.sonar.server.platform.db.migration.step.DataChange; -import org.sonar.server.platform.db.migration.step.MassUpdate; -import org.sonar.server.platform.db.migration.version.v89.util.NetworkInterfaceProvider; - -public class DropLocalWebhooks extends DataChange { - private static final Logger LOG = Loggers.get(DropLocalWebhooks.class); - - private final NetworkInterfaceProvider networkInterfaceProvider; - - public DropLocalWebhooks(Database db, NetworkInterfaceProvider networkInterfaceProvider) { - super(db); - this.networkInterfaceProvider = networkInterfaceProvider; - } - - @Override - protected void execute(Context context) throws SQLException { - MassUpdate massUpdate = context.prepareMassUpdate(); - massUpdate.select("select w.uuid, w.name, w.url, w.project_uuid, p.name from webhooks w left join projects p on p.uuid = w.project_uuid"); - massUpdate.update("delete from webhooks where uuid = ?"); - massUpdate.execute((row, update) -> { - try { - String webhookName = row.getString(2); - String webhookUrl = row.getString(3); - URL url = new URL(webhookUrl); - InetAddress address = InetAddress.getByName(url.getHost()); - if (isLocalAddress(address)) { - boolean projectLevel = row.getString(4) != null; - if (projectLevel) { - String projectName = row.getString(5); - LOG.warn("Webhook '{}' for project '{}' has been removed because it used an invalid, unsafe URL. Please recreate " + - "this webhook with a valid URL or ask a project administrator to do it if it is still needed.", webhookName, projectName); - } else { - LOG.warn("Global webhook '{}' has been removed because it used an invalid, unsafe URL. Please recreate this webhook with a valid URL" + - " if it is still needed.", webhookName); - } - - update.setString(1, row.getString(1)); - return true; - } - } catch (MalformedURLException | UnknownHostException | SocketException e) { - return false; - } - - return false; - }); - } - - private boolean isLocalAddress(InetAddress address) throws SocketException { - return networkInterfaceProvider.getNetworkInterfaceAddresses().stream() - .anyMatch(a -> a != null && a.equals(address)); - } -} diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v84/DropLocalWebhooksTest.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v84/DropLocalWebhooksTest.java deleted file mode 100644 index adfc1bd35a1..00000000000 --- a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v84/DropLocalWebhooksTest.java +++ /dev/null @@ -1,132 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2021 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.platform.db.migration.version.v84; - -import java.sql.SQLException; -import java.util.List; -import javax.annotation.Nullable; -import org.junit.Rule; -import org.junit.Test; -import org.sonar.api.utils.System2; -import org.sonar.api.utils.log.LogTester; -import org.sonar.api.utils.log.LoggerLevel; -import org.sonar.db.CoreDbTester; -import org.sonar.server.platform.db.migration.step.DataChange; - -import static org.assertj.core.api.Assertions.assertThat; - -public class DropLocalWebhooksTest { - - @Rule - public LogTester logTester = new LogTester(); - - private static final String TABLE_NAME = "webhooks"; - - @Rule - public CoreDbTester dbTester = CoreDbTester.createForSchema(DropLocalWebhooksTest.class, "schema.sql"); - - private final DataChange underTest = new DropLocalWebhooks(dbTester.database()); - - @Test - public void execute() throws SQLException { - prepareWebhooks(); - - underTest.execute(); - - verifyMigrationResult(); - } - - @Test - public void migrationIsReEntrant() throws SQLException { - prepareWebhooks(); - - underTest.execute(); - underTest.execute(); - - verifyMigrationResult(); - } - - @Test - public void migrationIsSuccessfulWhenNoWebhooksDeleted() throws SQLException { - insertProject("p1", "pn1"); - insertWebhook("uuid-1", "https://10.15.15.15:5555/some_webhook", "p1"); - insertWebhook("uuid-5", "https://some.valid.address.com/random_webhook", null); - - underTest.execute(); - - assertThat(dbTester.countRowsOfTable(TABLE_NAME)).isEqualTo(2); - assertThat(logTester.logs(LoggerLevel.WARN)).isEmpty(); - } - - @Test - public void migrationIsSuccessfulWhenNoWebhooksInDb() throws SQLException { - insertProject("p1", "pn1"); - - underTest.execute(); - - assertThat(dbTester.countRowsOfTable(TABLE_NAME)).isZero(); - assertThat(logTester.logs(LoggerLevel.WARN)).isEmpty(); - } - - private void prepareWebhooks() { - insertProject("p1", "pn1"); - insertProject("p2", "pn2"); - insertWebhook("uuid-1", "https://10.15.15.15:5555/some_webhook", "p1"); - insertWebhook("uuid-2", "https://0.0.0.0/some_webhook", "p1"); - insertWebhook("uuid-3", "https://172.16.16.16:6666/some_webhook", "p2"); - insertWebhook("uuid-4", "https://127.0.0.1/some_webhook", "p2"); - insertWebhook("uuid-5", "https://some.valid.address.com/random_webhook", null); - insertWebhook("uuid-6", "https://248.235.76.254:7777/some_webhook", null); - insertWebhook("uuid-7", "https://localhost/some_webhook", null); - } - - private void verifyMigrationResult() { - assertThat(dbTester.countRowsOfTable(TABLE_NAME)).isEqualTo(4); - assertThat(dbTester.select("select uuid from " + TABLE_NAME).stream().map(columns -> columns.get("UUID"))) - .containsOnly("uuid-1", "uuid-3", "uuid-5", "uuid-6"); - - List logs = logTester.logs(LoggerLevel.WARN); - assertThat(logs).hasSize(3); - assertThat(logs).containsExactlyInAnyOrder( - "Global webhook 'webhook-uuid-7' has been removed because it used an invalid, unsafe URL. Please recreate this webhook with a valid URL if it is still needed.", - "Webhook 'webhook-uuid-4' for project 'pn2' has been removed because it used an invalid, unsafe URL. Please recreate this webhook with a valid URL or ask a project administrator to do it if it is still needed.", - "Webhook 'webhook-uuid-2' for project 'pn1' has been removed because it used an invalid, unsafe URL. Please recreate this webhook with a valid URL or ask a project administrator to do it if it is still needed."); - } - - private void insertProject(String uuid, String name) { - dbTester.executeInsert("PROJECTS", - "NAME", name, - "ORGANIZATION_UUID", "default", - "KEE", uuid + "-key", - "UUID", uuid, - "PRIVATE", Boolean.toString(false), - "QUALIFIER", "TRK", - "UPDATED_AT", System2.INSTANCE.now()); - } - - private void insertWebhook(String uuid, String url, @Nullable String projectUuid) { - dbTester.executeInsert(TABLE_NAME, - "UUID", uuid, - "NAME", "webhook-" + uuid, - "PROJECT_UUID", projectUuid, - "URL", url, - "CREATED_AT", System2.INSTANCE.now()); - } -} diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v89/DropLocalWebhooksTest.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v89/DropLocalWebhooksTest.java deleted file mode 100644 index 765f769f3eb..00000000000 --- a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v89/DropLocalWebhooksTest.java +++ /dev/null @@ -1,153 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2021 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.platform.db.migration.version.v89; - -import com.google.common.collect.ImmutableList; -import java.io.IOException; -import java.net.InetAddress; -import java.sql.SQLException; -import java.util.List; -import javax.annotation.Nullable; -import okhttp3.HttpUrl; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.sonar.api.utils.System2; -import org.sonar.api.utils.log.LogTester; -import org.sonar.api.utils.log.LoggerLevel; -import org.sonar.db.CoreDbTester; -import org.sonar.server.platform.db.migration.step.DataChange; -import org.sonar.server.platform.db.migration.version.v89.util.NetworkInterfaceProvider; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -public class DropLocalWebhooksTest { - - @Rule - public LogTester logTester = new LogTester(); - - private static final String TABLE_NAME = "webhooks"; - - @Rule - public CoreDbTester dbTester = CoreDbTester.createForSchema(DropLocalWebhooksTest.class, "schema.sql"); - - private final NetworkInterfaceProvider networkInterfaceProvider = mock(NetworkInterfaceProvider.class); - - private final DataChange underTest = new DropLocalWebhooks(dbTester.database(), networkInterfaceProvider); - - @Before - public void prepare() throws IOException { - InetAddress inetAddress1 = InetAddress.getByName(HttpUrl.parse("https://0.0.0.0/some_webhook").host()); - InetAddress inetAddress2 = InetAddress.getByName(HttpUrl.parse("https://127.0.0.1/some_webhook").host()); - InetAddress inetAddress3 = InetAddress.getByName(HttpUrl.parse("https://localhost/some_webhook").host()); - - when(networkInterfaceProvider.getNetworkInterfaceAddresses()) - .thenReturn(ImmutableList.of(inetAddress1, inetAddress2, inetAddress3)); - } - - @Test - public void execute() throws SQLException { - prepareWebhooks(); - - underTest.execute(); - - verifyMigrationResult(); - } - - @Test - public void migrationIsReEntrant() throws SQLException { - prepareWebhooks(); - - underTest.execute(); - underTest.execute(); - - verifyMigrationResult(); - } - - @Test - public void migrationIsSuccessfulWhenNoWebhooksDeleted() throws SQLException { - insertProject("p1", "pn1"); - insertWebhook("uuid-1", "https://10.15.15.15:5555/some_webhook", "p1"); - insertWebhook("uuid-5", "https://some.valid.address.com/random_webhook", null); - - underTest.execute(); - - assertThat(dbTester.countRowsOfTable(TABLE_NAME)).isEqualTo(2); - assertThat(logTester.logs(LoggerLevel.WARN)).isEmpty(); - } - - @Test - public void migrationIsSuccessfulWhenNoWebhooksInDb() throws SQLException { - insertProject("p1", "pn1"); - - underTest.execute(); - - assertThat(dbTester.countRowsOfTable(TABLE_NAME)).isZero(); - assertThat(logTester.logs(LoggerLevel.WARN)).isEmpty(); - } - - private void prepareWebhooks() { - insertProject("p1", "pn1"); - insertProject("p2", "pn2"); - insertWebhook("uuid-1", "https://10.15.15.15:5555/some_webhook", "p1"); - insertWebhook("uuid-2", "https://0.0.0.0/some_webhook", "p1"); - insertWebhook("uuid-3", "https://172.16.16.16:6666/some_webhook", "p2"); - insertWebhook("uuid-4", "https://127.0.0.1/some_webhook", "p2"); - insertWebhook("uuid-5", "https://some.valid.address.com/random_webhook", null); - insertWebhook("uuid-6", "https://248.235.76.254:7777/some_webhook", null); - insertWebhook("uuid-7", "https://localhost/some_webhook", null); - } - - private void verifyMigrationResult() { - assertThat(dbTester.countRowsOfTable(TABLE_NAME)).isEqualTo(4); - assertThat(dbTester.select("select uuid from " + TABLE_NAME).stream().map(columns -> columns.get("UUID"))) - .containsOnly("uuid-1", "uuid-3", "uuid-5", "uuid-6"); - - List logs = logTester.logs(LoggerLevel.WARN); - assertThat(logs).hasSize(3); - assertThat(logs).containsExactlyInAnyOrder( - "Global webhook 'webhook-uuid-7' has been removed because it used an invalid, unsafe URL. Please recreate this webhook with a valid URL if it is still needed.", - "Webhook 'webhook-uuid-4' for project 'pn2' has been removed because it used an invalid, unsafe URL. Please recreate this webhook with a valid URL or ask a project administrator to do it if it is still needed.", - "Webhook 'webhook-uuid-2' for project 'pn1' has been removed because it used an invalid, unsafe URL. Please recreate this webhook with a valid URL or ask a project administrator to do it if it is still needed."); - } - - private void insertProject(String uuid, String name) { - dbTester.executeInsert("PROJECTS", - "NAME", name, - "ORGANIZATION_UUID", "default", - "KEE", uuid + "-key", - "UUID", uuid, - "PRIVATE", Boolean.toString(false), - "QUALIFIER", "TRK", - "UPDATED_AT", System2.INSTANCE.now()); - } - - private void insertWebhook(String uuid, String url, @Nullable String projectUuid) { - dbTester.executeInsert(TABLE_NAME, - "UUID", uuid, - "NAME", "webhook-" + uuid, - "PROJECT_UUID", projectUuid, - "URL", url, - "CREATED_AT", System2.INSTANCE.now()); - } -} - diff --git a/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v84/DropLocalWebhooksTest/schema.sql b/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v84/DropLocalWebhooksTest/schema.sql deleted file mode 100644 index de11e9e16ed..00000000000 --- a/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v84/DropLocalWebhooksTest/schema.sql +++ /dev/null @@ -1,29 +0,0 @@ -CREATE TABLE "PROJECTS"( - "UUID" VARCHAR(40) NOT NULL, - "KEE" VARCHAR(400) NOT NULL, - "QUALIFIER" VARCHAR(10) NOT NULL, - "ORGANIZATION_UUID" VARCHAR(40) NOT NULL, - "NAME" VARCHAR(2000), - "DESCRIPTION" VARCHAR(2000), - "PRIVATE" BOOLEAN NOT NULL, - "TAGS" VARCHAR(500), - "CREATED_AT" BIGINT, - "UPDATED_AT" BIGINT NOT NULL -); -ALTER TABLE "PROJECTS" ADD CONSTRAINT "PK_NEW_PROJECTS" PRIMARY KEY("UUID"); -CREATE UNIQUE INDEX "UNIQ_PROJECTS_KEE" ON "PROJECTS"("KEE"); -CREATE INDEX "IDX_QUALIFIER" ON "PROJECTS"("QUALIFIER"); - -CREATE TABLE "WEBHOOKS"( - "UUID" VARCHAR(40) NOT NULL, - "ORGANIZATION_UUID" VARCHAR(40), - "PROJECT_UUID" VARCHAR(40), - "NAME" VARCHAR(100) NOT NULL, - "URL" VARCHAR(2000) NOT NULL, - "SECRET" VARCHAR(200), - "CREATED_AT" BIGINT NOT NULL, - "UPDATED_AT" BIGINT -); -ALTER TABLE "WEBHOOKS" ADD CONSTRAINT "PK_WEBHOOKS" PRIMARY KEY("UUID"); -CREATE INDEX "ORGANIZATION_WEBHOOK" ON "WEBHOOKS"("ORGANIZATION_UUID"); -CREATE INDEX "PROJECT_WEBHOOK" ON "WEBHOOKS"("PROJECT_UUID"); diff --git a/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v89/DropLocalWebhooksTest/schema.sql b/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v89/DropLocalWebhooksTest/schema.sql deleted file mode 100644 index de11e9e16ed..00000000000 --- a/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v89/DropLocalWebhooksTest/schema.sql +++ /dev/null @@ -1,29 +0,0 @@ -CREATE TABLE "PROJECTS"( - "UUID" VARCHAR(40) NOT NULL, - "KEE" VARCHAR(400) NOT NULL, - "QUALIFIER" VARCHAR(10) NOT NULL, - "ORGANIZATION_UUID" VARCHAR(40) NOT NULL, - "NAME" VARCHAR(2000), - "DESCRIPTION" VARCHAR(2000), - "PRIVATE" BOOLEAN NOT NULL, - "TAGS" VARCHAR(500), - "CREATED_AT" BIGINT, - "UPDATED_AT" BIGINT NOT NULL -); -ALTER TABLE "PROJECTS" ADD CONSTRAINT "PK_NEW_PROJECTS" PRIMARY KEY("UUID"); -CREATE UNIQUE INDEX "UNIQ_PROJECTS_KEE" ON "PROJECTS"("KEE"); -CREATE INDEX "IDX_QUALIFIER" ON "PROJECTS"("QUALIFIER"); - -CREATE TABLE "WEBHOOKS"( - "UUID" VARCHAR(40) NOT NULL, - "ORGANIZATION_UUID" VARCHAR(40), - "PROJECT_UUID" VARCHAR(40), - "NAME" VARCHAR(100) NOT NULL, - "URL" VARCHAR(2000) NOT NULL, - "SECRET" VARCHAR(200), - "CREATED_AT" BIGINT NOT NULL, - "UPDATED_AT" BIGINT -); -ALTER TABLE "WEBHOOKS" ADD CONSTRAINT "PK_WEBHOOKS" PRIMARY KEY("UUID"); -CREATE INDEX "ORGANIZATION_WEBHOOK" ON "WEBHOOKS"("ORGANIZATION_UUID"); -CREATE INDEX "PROJECT_WEBHOOK" ON "WEBHOOKS"("PROJECT_UUID"); diff --git a/server/sonar-docs/src/pages/setup/upgrade-notes.md b/server/sonar-docs/src/pages/setup/upgrade-notes.md index 590837efd0d..634f4db7d2d 100644 --- a/server/sonar-docs/src/pages/setup/upgrade-notes.md +++ b/server/sonar-docs/src/pages/setup/upgrade-notes.md @@ -20,6 +20,10 @@ To improve security, webhooks, by default, aren't allowed to point to the SonarQ [Full release notes](https://jira.sonarsource.com/secure/ReleaseNote.jspa?projectId=10930&version=16710) +**Enforced security on Webhooks** +By default and to enforce security constraints, webhooks are not allowed to point to the IPs addresses of the local SonarQube instance. +However, that behavior can be changed in [Administration > General Settings > Security](/#sonarqube-admin#/admin/settings?category=security/). + ## Release 8.8 Upgrade Notes **CSS analysis now requires Node.js 10+** In order to analyze CSS code, you now need to have Node.js 10+ installed on the machine running the scan. diff --git a/server/sonar-process/src/main/java/org/sonar/process/ProcessProperties.java b/server/sonar-process/src/main/java/org/sonar/process/ProcessProperties.java index 344e44b3cd9..a7da1d7672d 100644 --- a/server/sonar-process/src/main/java/org/sonar/process/ProcessProperties.java +++ b/server/sonar-process/src/main/java/org/sonar/process/ProcessProperties.java @@ -146,7 +146,6 @@ public class ProcessProperties { SONAR_WEB_SSO_REFRESH_INTERVAL_IN_MINUTES("sonar.web.sso.refreshIntervalInMinutes", "5"), SONAR_SECURITY_REALM("sonar.security.realm"), SONAR_AUTHENTICATOR_IGNORE_STARTUP_FAILURE("sonar.authenticator.ignoreStartupFailure", "false"), - SONAR_VALIDATE_WEBHOOKS("sonar.validateWebhooks", Boolean.TRUE.toString()), LDAP_SERVERS("ldap.servers"), LDAP_URL("ldap.url"), diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookCustomDns.java b/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookCustomDns.java index 71c4448c747..403add74336 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookCustomDns.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookCustomDns.java @@ -30,7 +30,8 @@ import org.sonar.api.ce.ComputeEngineSide; import org.sonar.api.config.Configuration; import org.sonar.api.server.ServerSide; -import static org.sonar.process.ProcessProperties.Property.SONAR_VALIDATE_WEBHOOKS; +import static org.sonar.api.CoreProperties.SONAR_VALIDATE_WEBHOOKS_DEFAULT_VALUE; +import static org.sonar.api.CoreProperties.SONAR_VALIDATE_WEBHOOKS_PROPERTY; @ServerSide @ComputeEngineSide @@ -48,7 +49,7 @@ public class WebhookCustomDns implements Dns { @Override public List lookup(@NotNull String host) throws UnknownHostException { InetAddress address = InetAddress.getByName(host); - if (configuration.getBoolean(SONAR_VALIDATE_WEBHOOKS.getKey()).orElse(true) + if (configuration.getBoolean(SONAR_VALIDATE_WEBHOOKS_PROPERTY).orElse(SONAR_VALIDATE_WEBHOOKS_DEFAULT_VALUE) && (address.isLoopbackAddress() || address.isAnyLocalAddress() || isLocalAddress(address))) { throw new IllegalArgumentException("Invalid URL: loopback and wildcard addresses are not allowed for webhooks."); } diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/webhook/WebhookCallerImplTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/webhook/WebhookCallerImplTest.java index 885ecae43a6..be6dfc9e565 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/webhook/WebhookCallerImplTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/webhook/WebhookCallerImplTest.java @@ -38,8 +38,8 @@ import org.sonar.api.SonarQubeSide; import org.sonar.api.SonarRuntime; import org.sonar.api.config.Configuration; import org.sonar.api.config.internal.MapSettings; -import org.sonar.api.internal.SonarRuntimeImpl; import org.sonar.api.impl.utils.TestSystem2; +import org.sonar.api.internal.SonarRuntimeImpl; import org.sonar.api.utils.System2; import org.sonar.api.utils.Version; import org.sonar.server.util.OkHttpClientProvider; @@ -47,7 +47,7 @@ import org.sonar.server.util.OkHttpClientProvider; import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.when; -import static org.sonar.process.ProcessProperties.Property.SONAR_VALIDATE_WEBHOOKS; +import static org.sonar.api.CoreProperties.SONAR_VALIDATE_WEBHOOKS_PROPERTY; public class WebhookCallerImplTest { @@ -285,7 +285,8 @@ public class WebhookCallerImplTest { private WebhookCaller newSender(boolean validateWebhook) { SonarRuntime runtime = SonarRuntimeImpl.forSonarQube(Version.parse("6.2"), SonarQubeSide.SERVER, SonarEdition.COMMUNITY); - when(configuration.getBoolean(SONAR_VALIDATE_WEBHOOKS.getKey())).thenReturn(Optional.of(validateWebhook)); + when(configuration.getBoolean(SONAR_VALIDATE_WEBHOOKS_PROPERTY)) + .thenReturn(Optional.of(validateWebhook)); WebhookCustomDns webhookCustomDns = new WebhookCustomDns(configuration, networkInterfaceProvider); return new WebhookCallerImpl(system, new OkHttpClientProvider().provide(new MapSettings().asConfig(), runtime), webhookCustomDns); } diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/webhook/WebhookCustomDnsTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/webhook/WebhookCustomDnsTest.java index 8ebee139b63..f3014ef7fda 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/webhook/WebhookCustomDnsTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/webhook/WebhookCustomDnsTest.java @@ -34,7 +34,7 @@ import org.mockito.Mockito; import org.sonar.api.config.Configuration; import static org.mockito.Mockito.when; -import static org.sonar.process.ProcessProperties.Property.SONAR_VALIDATE_WEBHOOKS; +import static org.sonar.api.CoreProperties.SONAR_VALIDATE_WEBHOOKS_PROPERTY; public class WebhookCustomDnsTest { private static final String INVALID_URL = "Invalid URL: loopback and wildcard addresses are not allowed for webhooks."; @@ -46,7 +46,8 @@ public class WebhookCustomDnsTest { @Test public void lookup_fail_on_localhost() { - when(configuration.getBoolean(SONAR_VALIDATE_WEBHOOKS.getKey())).thenReturn(Optional.of(true)); + when(configuration.getBoolean(SONAR_VALIDATE_WEBHOOKS_PROPERTY)) + .thenReturn(Optional.of(true)); Assertions.assertThatThrownBy(() -> underTest.lookup("localhost")) .hasMessageContaining(INVALID_URL) @@ -55,7 +56,8 @@ public class WebhookCustomDnsTest { @Test public void lookup_fail_on_127_0_0_1() { - when(configuration.getBoolean(SONAR_VALIDATE_WEBHOOKS.getKey())).thenReturn(Optional.of(true)); + when(configuration.getBoolean(SONAR_VALIDATE_WEBHOOKS_PROPERTY)) + .thenReturn(Optional.of(true)); Assertions.assertThatThrownBy(() -> underTest.lookup("127.0.0.1")) .hasMessageContaining(INVALID_URL) @@ -66,7 +68,9 @@ public class WebhookCustomDnsTest { public void lookup_fail_on_192_168_1_21() throws UnknownHostException, SocketException { InetAddress inetAddress = InetAddress.getByName(HttpUrl.parse("https://192.168.1.21/").host()); - when(configuration.getBoolean(SONAR_VALIDATE_WEBHOOKS.getKey())).thenReturn(Optional.of(true)); + when(configuration.getBoolean(SONAR_VALIDATE_WEBHOOKS_PROPERTY)) + .thenReturn(Optional.of(true)); + when(networkInterfaceProvider.getNetworkInterfaceAddresses()) .thenReturn(ImmutableList.of(inetAddress)); @@ -88,7 +92,9 @@ public class WebhookCustomDnsTest { String differentCaseAddress = getDifferentCaseInetAddress(inet6Address.get()); - when(configuration.getBoolean(SONAR_VALIDATE_WEBHOOKS.getKey())).thenReturn(Optional.of(true)); + when(configuration.getBoolean(SONAR_VALIDATE_WEBHOOKS_PROPERTY)) + .thenReturn(Optional.of(true)); + when(networkInterfaceProvider.getNetworkInterfaceAddresses()) .thenReturn(ImmutableList.of(inet6Address.get())); @@ -109,7 +115,8 @@ public class WebhookCustomDnsTest { @Test public void lookup_dont_fail_on_localhost_if_validation_disabled() throws UnknownHostException { - when(configuration.getBoolean(SONAR_VALIDATE_WEBHOOKS.getKey())).thenReturn(Optional.of(false)); + when(configuration.getBoolean(SONAR_VALIDATE_WEBHOOKS_PROPERTY)) + .thenReturn(Optional.of(false)); Assertions.assertThat(underTest.lookup("localhost")) .extracting(InetAddress::toString) @@ -118,7 +125,8 @@ public class WebhookCustomDnsTest { @Test public void lookup_dont_fail_on_classic_host_with_validation_enabled() throws UnknownHostException { - when(configuration.getBoolean(SONAR_VALIDATE_WEBHOOKS.getKey())).thenReturn(Optional.of(true)); + when(configuration.getBoolean(SONAR_VALIDATE_WEBHOOKS_PROPERTY)) + .thenReturn(Optional.of(true)); Assertions.assertThat(underTest.lookup("sonarsource.com").toString()).contains("sonarsource.com/"); } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/WebhookSupport.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/WebhookSupport.java index 16a1b382853..bd13da48dab 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/WebhookSupport.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/WebhookSupport.java @@ -27,9 +27,10 @@ import org.sonar.api.config.Configuration; import org.sonar.db.project.ProjectDto; import org.sonar.server.user.UserSession; +import static org.sonar.api.CoreProperties.SONAR_VALIDATE_WEBHOOKS_DEFAULT_VALUE; +import static org.sonar.api.CoreProperties.SONAR_VALIDATE_WEBHOOKS_PROPERTY; import static org.sonar.api.web.UserRole.ADMIN; import static org.sonar.db.permission.GlobalPermission.ADMINISTER; -import static org.sonar.process.ProcessProperties.Property.SONAR_VALIDATE_WEBHOOKS; public class WebhookSupport { @@ -59,7 +60,8 @@ public class WebhookSupport { } InetAddress address = InetAddress.getByName(okUrl.host()); - if (configuration.getBoolean(SONAR_VALIDATE_WEBHOOKS.getKey()).orElse(true) + if (configuration.getBoolean(SONAR_VALIDATE_WEBHOOKS_PROPERTY) + .orElse(SONAR_VALIDATE_WEBHOOKS_DEFAULT_VALUE) && (address.isLoopbackAddress() || address.isAnyLocalAddress() || isLocalAddress(address))) { throw new IllegalArgumentException("Invalid URL: loopback and wildcard addresses are not allowed for webhooks."); } diff --git a/sonar-core/src/main/java/org/sonar/core/config/SecurityProperties.java b/sonar-core/src/main/java/org/sonar/core/config/SecurityProperties.java index e9e20c59335..225731cd74a 100644 --- a/sonar-core/src/main/java/org/sonar/core/config/SecurityProperties.java +++ b/sonar-core/src/main/java/org/sonar/core/config/SecurityProperties.java @@ -20,11 +20,17 @@ package org.sonar.core.config; import java.util.List; -import org.sonar.api.CoreProperties; import org.sonar.api.PropertyType; import org.sonar.api.config.PropertyDefinition; import static java.util.Arrays.asList; +import static org.sonar.api.CoreProperties.CATEGORY_SECURITY; +import static org.sonar.api.CoreProperties.CORE_ALLOW_PERMISSION_MANAGEMENT_FOR_PROJECT_ADMINS_DEFAULT_VALUE; +import static org.sonar.api.CoreProperties.CORE_ALLOW_PERMISSION_MANAGEMENT_FOR_PROJECT_ADMINS_PROPERTY; +import static org.sonar.api.CoreProperties.CORE_FORCE_AUTHENTICATION_DEFAULT_VALUE; +import static org.sonar.api.CoreProperties.CORE_FORCE_AUTHENTICATION_PROPERTY; +import static org.sonar.api.CoreProperties.SONAR_VALIDATE_WEBHOOKS_DEFAULT_VALUE; +import static org.sonar.api.CoreProperties.SONAR_VALIDATE_WEBHOOKS_PROPERTY; class SecurityProperties { @@ -34,24 +40,33 @@ class SecurityProperties { static List all() { return asList( - PropertyDefinition.builder(CoreProperties.CORE_FORCE_AUTHENTICATION_PROPERTY) - .defaultValue(Boolean.toString(CoreProperties.CORE_FORCE_AUTHENTICATION_DEFAULT_VALUE)) + PropertyDefinition.builder(CORE_FORCE_AUTHENTICATION_PROPERTY) + .defaultValue(Boolean.toString(CORE_FORCE_AUTHENTICATION_DEFAULT_VALUE)) .name("Force user authentication") .description( "Forcing user authentication prevents anonymous users from accessing the SonarQube UI, or project data via the Web API. " + "Some specific read-only Web APIs, including those required to prompt authentication, are still available anonymously." + "
Disabling this setting can expose the instance to security risks.") .type(PropertyType.BOOLEAN) - .category(CoreProperties.CATEGORY_SECURITY) + .category(CATEGORY_SECURITY) .build(), - PropertyDefinition.builder(CoreProperties.CORE_ALLOW_PERMISSION_MANAGEMENT_FOR_PROJECT_ADMINS_PROPERTY) - .defaultValue(Boolean.toString(CoreProperties.CORE_ALLOW_PERMISSION_MANAGEMENT_FOR_PROJECT_ADMINS_DEFAULT_VALUE)) + PropertyDefinition.builder(CORE_ALLOW_PERMISSION_MANAGEMENT_FOR_PROJECT_ADMINS_PROPERTY) + .defaultValue(Boolean.toString(CORE_ALLOW_PERMISSION_MANAGEMENT_FOR_PROJECT_ADMINS_DEFAULT_VALUE)) .name("Enable permission management for project administrators") .description( "Set if users with 'Administer' role in a project should be allowed to change project permissions. By default users with 'Administer' " + "role are allowed to change both project configuration and project permissions.") .type(PropertyType.BOOLEAN) - .category(CoreProperties.CATEGORY_SECURITY) + .category(CATEGORY_SECURITY) + .build(), + PropertyDefinition.builder(SONAR_VALIDATE_WEBHOOKS_PROPERTY) + .defaultValue(Boolean.toString(SONAR_VALIDATE_WEBHOOKS_DEFAULT_VALUE)) + .name("Enable local webhooks validation") + .description( + "Forcing local webhooks validation prevents the creation and triggering of local webhooks" + + "
Disabling this setting can expose the instance to security risks.") + .type(PropertyType.BOOLEAN) + .category(CATEGORY_SECURITY) .build() ); diff --git a/sonar-core/src/test/java/org/sonar/core/config/CorePropertyDefinitionsTest.java b/sonar-core/src/test/java/org/sonar/core/config/CorePropertyDefinitionsTest.java index 0079520e18f..b098ef95616 100644 --- a/sonar-core/src/test/java/org/sonar/core/config/CorePropertyDefinitionsTest.java +++ b/sonar-core/src/test/java/org/sonar/core/config/CorePropertyDefinitionsTest.java @@ -30,7 +30,7 @@ public class CorePropertyDefinitionsTest { @Test public void all() { List defs = CorePropertyDefinitions.all(); - assertThat(defs).hasSize(56); + assertThat(defs).hasSize(57); } @Test diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/CoreProperties.java b/sonar-plugin-api/src/main/java/org/sonar/api/CoreProperties.java index 08dfa2c6f78..b3ef7e3050e 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/CoreProperties.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/CoreProperties.java @@ -205,6 +205,8 @@ public interface CoreProperties { boolean CORE_FORCE_AUTHENTICATION_DEFAULT_VALUE = true; String CORE_ALLOW_PERMISSION_MANAGEMENT_FOR_PROJECT_ADMINS_PROPERTY = "sonar.allowPermissionManagementForProjectAdmins"; boolean CORE_ALLOW_PERMISSION_MANAGEMENT_FOR_PROJECT_ADMINS_DEFAULT_VALUE = true; + String SONAR_VALIDATE_WEBHOOKS_PROPERTY = "sonar.validateWebhooks"; + boolean SONAR_VALIDATE_WEBHOOKS_DEFAULT_VALUE = true; /** * @deprecated since 2.14. See http://jira.sonarsource.com/browse/SONAR-3153. Replaced by {@link #CORE_AUTHENTICATOR_REALM}. -- 2.39.5