From 9f614fdbf0fe66eec11ece07a98e7ad056476be1 Mon Sep 17 00:00:00 2001 From: Nolwenn Cadic <98824442+Nolwenn-cadic-sonarsource@users.noreply.github.com> Date: Thu, 25 Jul 2024 09:00:10 +0200 Subject: [PATCH] SONAR-22596 Fail Gitlab sync task if user consent is needed --- .../sonar/auth/DevOpsPlatformSettings.java | 2 + .../org/sonar/auth/github/GitHubSettings.java | 1 + .../org/sonar/auth/gitlab/GitLabSettings.java | 6 + .../sonar/auth/gitlab/GitLabSettingsTest.java | 12 ++ ...iredIfGitlabAutoProvisioningEnabledIT.java | 104 ++++++++++++++++++ ...quiredIfGitlabAutoProvisioningEnabled.java | 91 +++++++++++++++ .../migration/version/v107/DbVersion107.java | 6 +- .../version/v107/DbVersion107Test.java | 40 +++++++ 8 files changed, 260 insertions(+), 2 deletions(-) create mode 100644 server/sonar-db-migration/src/it/java/org/sonar/server/platform/db/migration/version/v107/AddUserConsentRequiredIfGitlabAutoProvisioningEnabledIT.java create mode 100644 server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v107/AddUserConsentRequiredIfGitlabAutoProvisioningEnabled.java create mode 100644 server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v107/DbVersion107Test.java diff --git a/server/sonar-auth-common/src/main/java/org/sonar/auth/DevOpsPlatformSettings.java b/server/sonar-auth-common/src/main/java/org/sonar/auth/DevOpsPlatformSettings.java index 23486ab9418..3c5b74fcb60 100644 --- a/server/sonar-auth-common/src/main/java/org/sonar/auth/DevOpsPlatformSettings.java +++ b/server/sonar-auth-common/src/main/java/org/sonar/auth/DevOpsPlatformSettings.java @@ -27,4 +27,6 @@ public interface DevOpsPlatformSettings { boolean isProjectVisibilitySynchronizationActivated(); + boolean isUserConsentRequiredAfterUpgrade(); + } diff --git a/server/sonar-auth-github/src/main/java/org/sonar/auth/github/GitHubSettings.java b/server/sonar-auth-github/src/main/java/org/sonar/auth/github/GitHubSettings.java index 57696283ba3..e98af18b583 100644 --- a/server/sonar-auth-github/src/main/java/org/sonar/auth/github/GitHubSettings.java +++ b/server/sonar-auth-github/src/main/java/org/sonar/auth/github/GitHubSettings.java @@ -168,6 +168,7 @@ public class GitHubSettings implements DevOpsPlatformSettings { return isEnabled() && internalProperties.read(GITHUB_PROVISIONING).map(Boolean::parseBoolean).orElse(false); } + @Override public boolean isUserConsentRequiredAfterUpgrade() { return configuration.get(GITHUB_USER_CONSENT_FOR_PERMISSIONS_REQUIRED_AFTER_UPGRADE).isPresent(); } diff --git a/server/sonar-auth-gitlab/src/main/java/org/sonar/auth/gitlab/GitLabSettings.java b/server/sonar-auth-gitlab/src/main/java/org/sonar/auth/gitlab/GitLabSettings.java index feef4ed823b..5a14cb14fde 100644 --- a/server/sonar-auth-gitlab/src/main/java/org/sonar/auth/gitlab/GitLabSettings.java +++ b/server/sonar-auth-gitlab/src/main/java/org/sonar/auth/gitlab/GitLabSettings.java @@ -46,6 +46,7 @@ public class GitLabSettings implements DevOpsPlatformSettings { public static final String GITLAB_AUTH_SYNC_USER_GROUPS = "sonar.auth.gitlab.groupsSync"; public static final String GITLAB_AUTH_PROVISIONING_TOKEN = "provisioning.gitlab.token.secured"; public static final String GITLAB_AUTH_PROVISIONING_ENABLED = "provisioning.gitlab.enabled"; + public static final String GITLAB_USER_CONSENT_FOR_PERMISSION_PROVISIONING_REQUIRED = "sonar.auth.gitlab.userConsentForPermissionProvisioningRequired"; private static final String CATEGORY = "authentication"; private static final String SUBCATEGORY = "gitlab"; @@ -111,6 +112,11 @@ public class GitLabSettings implements DevOpsPlatformSettings { return true; } + @Override + public boolean isUserConsentRequiredAfterUpgrade() { + return configuration.getBoolean(GITLAB_USER_CONSENT_FOR_PERMISSION_PROVISIONING_REQUIRED).isPresent(); + } + static List definitions() { return Arrays.asList( PropertyDefinition.builder(GITLAB_AUTH_ENABLED) diff --git a/server/sonar-auth-gitlab/src/test/java/org/sonar/auth/gitlab/GitLabSettingsTest.java b/server/sonar-auth-gitlab/src/test/java/org/sonar/auth/gitlab/GitLabSettingsTest.java index 9a732f4d04c..2c21df6171a 100644 --- a/server/sonar-auth-gitlab/src/test/java/org/sonar/auth/gitlab/GitLabSettingsTest.java +++ b/server/sonar-auth-gitlab/src/test/java/org/sonar/auth/gitlab/GitLabSettingsTest.java @@ -35,6 +35,7 @@ import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_PROVISIONING_TOKE import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_SECRET; import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_SYNC_USER_GROUPS; import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_URL; +import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_USER_CONSENT_FOR_PERMISSION_PROVISIONING_REQUIRED; public class GitLabSettingsTest { @@ -122,6 +123,17 @@ public class GitLabSettingsTest { assertThat(config.isProjectVisibilitySynchronizationActivated()).isTrue(); } + @Test + public void isUserConsentRequiredForPermissionProvisioning_returnsFalseByDefault() { + assertThat(config.isUserConsentRequiredAfterUpgrade()).isFalse(); + } + + @Test + public void isUserConsentRequiredForPermissionProvisioning_returnsTrueWhenPropertyPresent() { + settings.setProperty(GITLAB_USER_CONSENT_FOR_PERMISSION_PROVISIONING_REQUIRED, ""); + assertThat(config.isUserConsentRequiredAfterUpgrade()).isTrue(); + } + private void enableGitlabAuthentication() { settings.setProperty(GITLAB_AUTH_ENABLED, true); settings.setProperty(GITLAB_AUTH_APPLICATION_ID, "on"); diff --git a/server/sonar-db-migration/src/it/java/org/sonar/server/platform/db/migration/version/v107/AddUserConsentRequiredIfGitlabAutoProvisioningEnabledIT.java b/server/sonar-db-migration/src/it/java/org/sonar/server/platform/db/migration/version/v107/AddUserConsentRequiredIfGitlabAutoProvisioningEnabledIT.java new file mode 100644 index 00000000000..a2ee6205550 --- /dev/null +++ b/server/sonar-db-migration/src/it/java/org/sonar/server/platform/db/migration/version/v107/AddUserConsentRequiredIfGitlabAutoProvisioningEnabledIT.java @@ -0,0 +1,104 @@ +/* + * SonarQube + * Copyright (C) 2009-2024 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.v107; + +import java.sql.SQLException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.slf4j.event.Level; +import org.sonar.api.testfixtures.log.LogTesterJUnit5; +import org.sonar.api.utils.System2; +import org.sonar.core.util.UuidFactoryImpl; +import org.sonar.db.MigrationDbTester; +import org.sonar.server.platform.db.migration.step.DataChange; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.server.platform.db.migration.version.v107.AddUserConsentRequiredIfGitlabAutoProvisioningEnabled.PROP_KEY; +import static org.sonar.server.platform.db.migration.version.v107.AddUserConsentRequiredIfGitlabAutoProvisioningEnabled.PROVISIONING_GITLAB_ENABLED_PROP_KEY; + +class AddUserConsentRequiredIfGitlabAutoProvisioningEnabledIT { + @RegisterExtension + public final LogTesterJUnit5 logger = new LogTesterJUnit5(); + + @RegisterExtension + public final MigrationDbTester db = MigrationDbTester.createForMigrationStep(AddUserConsentRequiredIfGitlabAutoProvisioningEnabled.class); + private final DataChange underTest = new AddUserConsentRequiredIfGitlabAutoProvisioningEnabled(db.database(), new System2(), UuidFactoryImpl.INSTANCE); + + @BeforeEach + public void before() { + logger.clear(); + } + + @Test + void migration_whenGitlabAutoProvisioningPropertyNotPresent_shouldNotRequireConsent() throws SQLException { + underTest.execute(); + + assertThat(logger.logs(Level.WARN)).isEmpty(); + assertThat(isConsentRequired()).isFalse(); + } + + @Test + void migration_whenGitlabAutoProvisioningDisabled_shouldNotRequireConsent() throws SQLException { + disableGitlabProvisioning(); + underTest.execute(); + + assertThat(logger.logs(Level.WARN)).isEmpty(); + assertThat(isConsentRequired()).isFalse(); + } + + @Test + void migration_whenGitlabAutoProvisioningEnabled_shouldRequireConsent() throws SQLException { + enableGitlabProvisioning(); + + underTest.execute(); + + assertThat(logger.logs(Level.WARN)).containsExactly("Automatic synchronization was previously activated for Gitlab. It requires user consent to continue working as new" + + " features were added with the synchronization. Please read the upgrade notes."); + assertThat(isConsentRequired()).isTrue(); + } + + @Test + void migration_is_reentrant() throws SQLException { + enableGitlabProvisioning(); + + underTest.execute(); + underTest.execute(); + + assertThat(logger.logs(Level.WARN)).containsExactly("Automatic synchronization was previously activated for Gitlab. It requires user consent to continue working as new" + + " features were added with the synchronization. Please read the upgrade notes."); + assertThat(isConsentRequired()).isTrue(); + } + + private void disableGitlabProvisioning() { + toggleGitlabProvisioning(false); + } + private void enableGitlabProvisioning() { + toggleGitlabProvisioning(true); + } + + private boolean isConsentRequired() { + return db.countSql("select count(*) from properties where prop_key = '" + PROP_KEY + "'") >= 1; + } + + private void toggleGitlabProvisioning(boolean enabled) { + db.executeInsert("properties", "prop_key", PROVISIONING_GITLAB_ENABLED_PROP_KEY, "text_value", String.valueOf(enabled), "is_empty", true, "created_at", 0, "uuid", "uuid"); + } +} \ No newline at end of file diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v107/AddUserConsentRequiredIfGitlabAutoProvisioningEnabled.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v107/AddUserConsentRequiredIfGitlabAutoProvisioningEnabled.java new file mode 100644 index 00000000000..47fc1cb80e3 --- /dev/null +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v107/AddUserConsentRequiredIfGitlabAutoProvisioningEnabled.java @@ -0,0 +1,91 @@ +/* + * SonarQube + * Copyright (C) 2009-2024 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.v107; + +import com.google.common.annotations.VisibleForTesting; +import java.sql.SQLException; +import java.util.Optional; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.sonar.api.utils.System2; +import org.sonar.core.util.UuidFactory; +import org.sonar.db.Database; +import org.sonar.server.platform.db.migration.step.DataChange; +import org.sonar.server.platform.db.migration.step.Upsert; + +public class AddUserConsentRequiredIfGitlabAutoProvisioningEnabled extends DataChange { + + private static final Logger LOGGER = LoggerFactory.getLogger(AddUserConsentRequiredIfGitlabAutoProvisioningEnabled.class.getName()); + + @VisibleForTesting + static final String PROVISIONING_GITLAB_ENABLED_PROP_KEY = "provisioning.gitlab.enabled"; + + @VisibleForTesting + static final String PROP_KEY = "sonar.auth.gitlab.userConsentForPermissionProvisioningRequired"; + + private static final String INSERT_QUERY = """ + insert into properties (uuid, prop_key, is_empty, created_at) + values (?, ?, ?, ?) + """; + + private final System2 system2; + private final UuidFactory uuidFactory; + + public AddUserConsentRequiredIfGitlabAutoProvisioningEnabled(Database db, System2 system2, UuidFactory uuidFactory) { + super(db); + this.system2 = system2; + this.uuidFactory = uuidFactory; + } + + @Override + protected void execute(DataChange.Context context) throws SQLException { + if (!isGitlabAutoProvisioningEnabled(context)) { + return; + } + if (isUserConsentAlreadyRequired(context)) { + return; + } + LOGGER.warn("Automatic synchronization was previously activated for Gitlab. It requires user consent to continue working as new" + + " features were added with the synchronization. Please read the upgrade notes."); + Upsert upsert = context.prepareUpsert(INSERT_QUERY); + upsert + .setString(1, uuidFactory.create()) + .setString(2, PROP_KEY) + .setBoolean(3, true) + .setLong(4, system2.now()) + .execute() + .commit(); + } + + private static boolean isUserConsentAlreadyRequired(Context context) throws SQLException { + return Optional.ofNullable(context.prepareSelect("select count(*) from properties where prop_key = ?") + .setString(1, PROP_KEY) + .get(t -> 1 == t.getInt(1))) + .orElseThrow(); + } + + private static boolean isGitlabAutoProvisioningEnabled(Context context) throws SQLException { + return Optional.ofNullable(context.prepareSelect("select count(*) from properties where prop_key = ? and text_value = ?") + .setString(1, PROVISIONING_GITLAB_ENABLED_PROP_KEY) + .setString(2, "true") + .get(t -> 1 == t.getInt(1))) + .orElseThrow(); + } +} diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v107/DbVersion107.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v107/DbVersion107.java index 3c752d9b6d8..2e42aed367a 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v107/DbVersion107.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v107/DbVersion107.java @@ -22,6 +22,8 @@ package org.sonar.server.platform.db.migration.version.v107; import org.sonar.server.platform.db.migration.step.MigrationStepRegistry; import org.sonar.server.platform.db.migration.version.DbVersion; +// ignoring bad number formatting, as it's indented that we align the migration numbers to SQ versions +@SuppressWarnings("java:S3937") public class DbVersion107 implements DbVersion { /** @@ -35,11 +37,11 @@ public class DbVersion107 implements DbVersion { * 10_1_002 * 10_2_000 */ - @Override public void addSteps(MigrationStepRegistry registry) { registry - .add(10_7_000, "Create 'telemetry_metrics_sent' table", CreateTelemetryMetricsSentTable.class); + .add(10_7_000, "Create 'telemetry_metrics_sent' table", CreateTelemetryMetricsSentTable.class) + .add(10_7_001, "sonar.auth.gitlab.userConsentForPermissionProvisioningRequired", AddUserConsentRequiredIfGitlabAutoProvisioningEnabled.class); } } diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v107/DbVersion107Test.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v107/DbVersion107Test.java new file mode 100644 index 00000000000..ecece10f206 --- /dev/null +++ b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v107/DbVersion107Test.java @@ -0,0 +1,40 @@ +/* + * SonarQube + * Copyright (C) 2009-2024 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.v107; + +import org.junit.jupiter.api.Test; + +import static org.sonar.server.platform.db.migration.version.DbVersionTestUtils.verifyMigrationNotEmpty; +import static org.sonar.server.platform.db.migration.version.DbVersionTestUtils.verifyMinimumMigrationNumber; + +class DbVersion107Test { + + private final DbVersion107 underTest = new DbVersion107(); + + @Test + void migrationNumber_starts_at_107_000() { + verifyMinimumMigrationNumber(underTest, 107_000); + } + + @Test + void verify_migration_is_not_empty() { + verifyMigrationNotEmpty(underTest); + } +} -- 2.39.5