SONAR-21413 Make provisioning groups ready to handle allowed groups

4 files changed, 44 insertions, 41 deletions

diff --git a/server/sonar-webserver-common/src/it/java/org/sonar/server/common/gitlab/config/GitlabConfigurationServiceIT.java b/server/sonar-webserver-common/src/it/java/org/sonar/server/common/gitlab/config/GitlabConfigurationServiceIT.java
index e50fffe47e6..cb7e8cd8889 100644
--- a/server/sonar-webserver-common/src/it/java/org/sonar/server/common/gitlab/config/GitlabConfigurationServiceIT.java
+++ b/server/sonar-webserver-common/src/it/java/org/sonar/server/common/gitlab/config/GitlabConfigurationServiceIT.java
@@ -55,11 +55,11 @@ import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 import static org.sonar.alm.client.gitlab.GitlabGlobalSettingsValidator.ValidationMode.AUTH_ONLY;
 import static org.sonar.alm.client.gitlab.GitlabGlobalSettingsValidator.ValidationMode.COMPLETE;
+import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_ALLOWED_GROUPS;
 import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_ALLOW_USERS_TO_SIGNUP;
 import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_APPLICATION_ID;
 import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_ENABLED;
 import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_PROVISIONING_ENABLED;
-import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_PROVISIONING_GROUPS;
 import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_PROVISIONING_TOKEN;
 import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_SECRET;
 import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_SYNC_USER_GROUPS;
@@ -125,7 +125,7 @@ public class GitlabConfigurationServiceIT {
   @Test
   public void getConfiguration_whenConfigurationSetAndEmpty_returnsConfig() {
     dbTester.properties().insertProperty(GITLAB_AUTH_ENABLED, "true", null);
-    dbTester.properties().insertProperty(GITLAB_AUTH_PROVISIONING_GROUPS, "", null);
+    dbTester.properties().insertProperty(GITLAB_AUTH_ALLOWED_GROUPS, "", null);
 
     GitlabConfiguration configuration = gitlabConfigurationService.getConfiguration("gitlab-configuration");
 
@@ -135,10 +135,10 @@ public class GitlabConfigurationServiceIT {
     assertThat(configuration.url()).isEmpty();
     assertThat(configuration.secret()).isEmpty();
     assertThat(configuration.synchronizeGroups()).isFalse();
+    assertThat(configuration.allowedGroups()).isEmpty();
     assertThat(configuration.provisioningType()).isEqualTo(JIT);
     assertThat(configuration.allowUsersToSignUp()).isFalse();
     assertThat(configuration.provisioningToken()).isNull();
-    assertThat(configuration.provisioningGroups()).isEmpty();
   }
 
   @Test
@@ -169,10 +169,10 @@ public class GitlabConfigurationServiceIT {
       .url(withValueOrThrow("url"))
       .secret(withValueOrThrow("secret"))
       .synchronizeGroups(withValueOrThrow(true))
+      .allowedGroups(withValueOrThrow(new LinkedHashSet<>(List.of("group1", "group2", "group3"))))
       .provisioningType(withValueOrThrow(AUTO_PROVISIONING))
       .allowUserToSignUp(withValueOrThrow(true))
       .provisioningToken(withValueOrThrow("provisioningToken"))
-      .provisioningGroups(withValueOrThrow(new LinkedHashSet<>(List.of("group1", "group2", "group3"))))
       .build();
 
     GitlabConfiguration gitlabConfiguration = gitlabConfigurationService.updateConfiguration(updateRequest);
@@ -182,10 +182,10 @@ public class GitlabConfigurationServiceIT {
     verifySettingWasSet(GITLAB_AUTH_URL, "url");
     verifySettingWasSet(GITLAB_AUTH_SECRET, "secret");
     verifySettingWasSet(GITLAB_AUTH_SYNC_USER_GROUPS, "true");
+    verifySettingWasSet(GITLAB_AUTH_ALLOWED_GROUPS, "group1,group2,group3");
     verifySettingWasSet(GITLAB_AUTH_PROVISIONING_ENABLED, "true");
     verifySettingWasSet(GITLAB_AUTH_ALLOW_USERS_TO_SIGNUP, "true");
     verifySettingWasSet(GITLAB_AUTH_PROVISIONING_TOKEN, "provisioningToken");
-    verifySettingWasSet(GITLAB_AUTH_PROVISIONING_GROUPS, "group1,group2,group3");
     verify(managedInstanceService).queueSynchronisationTask();
 
     assertConfigurationFields(gitlabConfiguration);
@@ -290,10 +290,10 @@ public class GitlabConfigurationServiceIT {
     assertThat(configuration.url()).isEqualTo("url");
     assertThat(configuration.secret()).isEqualTo("secret");
     assertThat(configuration.synchronizeGroups()).isTrue();
+    assertThat(configuration.allowedGroups()).containsExactlyInAnyOrder("group1", "group2", "group3");
     assertThat(configuration.provisioningType()).isEqualTo(AUTO_PROVISIONING);
     assertThat(configuration.allowUsersToSignUp()).isTrue();
     assertThat(configuration.provisioningToken()).isEqualTo("provisioningToken");
-    assertThat(configuration.provisioningGroups()).containsExactlyInAnyOrder("group1", "group2", "group3");
   }
 
   @Test
@@ -329,10 +329,11 @@ public class GitlabConfigurationServiceIT {
       "url",
       "secret",
       true,
-      AUTO_PROVISIONING,
+      Set.of("group1", "group2", "group3"),
       true,
-      null,
-      Set.of("group1", "group2", "group3"));
+      AUTO_PROVISIONING,
+      null
+    );
 
     assertThatThrownBy(() -> gitlabConfigurationService.createConfiguration(configuration))
       .isInstanceOf(IllegalStateException.class)
@@ -375,10 +376,11 @@ public class GitlabConfigurationServiceIT {
       "url",
       "secret",
       true,
-      JIT,
+      Set.of("group1", "group2", "group3"),
       true,
-      null,
-      Set.of("group1", "group2", "group3"));
+      JIT,
+      null
+    );
 
     GitlabConfiguration createdConfiguration = gitlabConfigurationService.createConfiguration(configuration);
 
@@ -395,11 +397,10 @@ public class GitlabConfigurationServiceIT {
     verifySettingWasSet(GITLAB_AUTH_URL, configuration.url());
     verifySettingWasSet(GITLAB_AUTH_SECRET, configuration.secret());
     verifySettingWasSet(GITLAB_AUTH_SYNC_USER_GROUPS, String.valueOf(configuration.synchronizeGroups()));
+    verifySettingWasSet(GITLAB_AUTH_ALLOWED_GROUPS, String.join(",", configuration.allowedGroups()));
     verifySettingWasSet(GITLAB_AUTH_ALLOW_USERS_TO_SIGNUP, String.valueOf(configuration.allowUsersToSignUp()));
     verifySettingWasSet(GITLAB_AUTH_PROVISIONING_TOKEN, Strings.nullToEmpty(configuration.provisioningToken()));
-    verifySettingWasSet(GITLAB_AUTH_PROVISIONING_GROUPS, String.join(",", configuration.provisioningGroups()));
-    verifySettingWasSet(GITLAB_AUTH_PROVISIONING_ENABLED,
-      String.valueOf(configuration.provisioningType().equals(AUTO_PROVISIONING)));
+    verifySettingWasSet(GITLAB_AUTH_PROVISIONING_ENABLED, String.valueOf(configuration.provisioningType().equals(AUTO_PROVISIONING)));
   }
 
   private void verifySettingWasSet(String setting, @Nullable String value) {
@@ -434,10 +435,10 @@ public class GitlabConfigurationServiceIT {
     assertPropertyIsDeleted(GITLAB_AUTH_URL);
     assertPropertyIsDeleted(GITLAB_AUTH_SECRET);
     assertPropertyIsDeleted(GITLAB_AUTH_SYNC_USER_GROUPS);
+    assertPropertyIsDeleted(GITLAB_AUTH_ALLOWED_GROUPS);
     assertPropertyIsDeleted(GITLAB_AUTH_PROVISIONING_ENABLED);
     assertPropertyIsDeleted(GITLAB_AUTH_ALLOW_USERS_TO_SIGNUP);
     assertPropertyIsDeleted(GITLAB_AUTH_PROVISIONING_TOKEN);
-    assertPropertyIsDeleted(GITLAB_AUTH_PROVISIONING_GROUPS);
 
     assertThat(dbTester.getDbClient().externalGroupDao().selectByIdentityProvider(dbTester.getSession(), GitLabIdentityProvider.KEY)).isEmpty();
   }
@@ -538,10 +539,10 @@ public class GitlabConfigurationServiceIT {
     when(gitlabConfiguration.url()).thenReturn("url");
     when(gitlabConfiguration.secret()).thenReturn("secret");
     when(gitlabConfiguration.synchronizeGroups()).thenReturn(true);
+    when(gitlabConfiguration.allowedGroups()).thenReturn(new LinkedHashSet<>(Set.of("group1", "group2", "group3")));
     when(gitlabConfiguration.provisioningType()).thenReturn(provisioningType);
     when(gitlabConfiguration.allowUsersToSignUp()).thenReturn(true);
     when(gitlabConfiguration.provisioningToken()).thenReturn("provisioningToken");
-    when(gitlabConfiguration.provisioningGroups()).thenReturn(new LinkedHashSet<>(Set.of("group1", "group2", "group3")));
     return gitlabConfiguration;
   }
 
@@ -552,9 +553,9 @@ public class GitlabConfigurationServiceIT {
     assertThat(actualConfiguration.url()).isEqualTo(expectedConfiguration.url());
     assertThat(actualConfiguration.secret()).isEqualTo(expectedConfiguration.secret());
     assertThat(actualConfiguration.synchronizeGroups()).isEqualTo(expectedConfiguration.synchronizeGroups());
+    assertThat(actualConfiguration.allowedGroups()).containsExactlyInAnyOrderElementsOf(expectedConfiguration.allowedGroups());
     assertThat(actualConfiguration.provisioningType()).isEqualTo(expectedConfiguration.provisioningType());
     assertThat(actualConfiguration.allowUsersToSignUp()).isEqualTo(expectedConfiguration.allowUsersToSignUp());
     assertThat(actualConfiguration.provisioningToken()).isEqualTo(expectedConfiguration.provisioningToken());
-    assertThat(actualConfiguration.provisioningGroups()).containsExactlyInAnyOrderElementsOf(expectedConfiguration.provisioningGroups());
   }
 }
diff --git a/server/sonar-webserver-common/src/main/java/org/sonar/server/common/gitlab/config/GitlabConfiguration.java b/server/sonar-webserver-common/src/main/java/org/sonar/server/common/gitlab/config/GitlabConfiguration.java
index b6c71aac7c1..32d39b04985 100644
--- a/server/sonar-webserver-common/src/main/java/org/sonar/server/common/gitlab/config/GitlabConfiguration.java
+++ b/server/sonar-webserver-common/src/main/java/org/sonar/server/common/gitlab/config/GitlabConfiguration.java
@@ -35,13 +35,14 @@ public record GitlabConfiguration(
 
   boolean synchronizeGroups,
 
-  ProvisioningType provisioningType,
+  Set<String> allowedGroups,
 
   boolean allowUsersToSignUp,
 
+  ProvisioningType provisioningType,
+
   @Nullable
-  String provisioningToken,
+  String provisioningToken
 
-  Set<String> provisioningGroups
 ) {
 }
diff --git a/server/sonar-webserver-common/src/main/java/org/sonar/server/common/gitlab/config/GitlabConfigurationService.java b/server/sonar-webserver-common/src/main/java/org/sonar/server/common/gitlab/config/GitlabConfigurationService.java
index 5df75318455..214c196133d 100644
--- a/server/sonar-webserver-common/src/main/java/org/sonar/server/common/gitlab/config/GitlabConfigurationService.java
+++ b/server/sonar-webserver-common/src/main/java/org/sonar/server/common/gitlab/config/GitlabConfigurationService.java
@@ -43,11 +43,11 @@ import static org.apache.commons.lang.StringUtils.isNotBlank;
 import static org.sonar.alm.client.gitlab.GitlabGlobalSettingsValidator.ValidationMode.AUTH_ONLY;
 import static org.sonar.alm.client.gitlab.GitlabGlobalSettingsValidator.ValidationMode.COMPLETE;
 import static org.sonar.api.utils.Preconditions.checkState;
+import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_ALLOWED_GROUPS;
 import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_ALLOW_USERS_TO_SIGNUP;
 import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_APPLICATION_ID;
 import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_ENABLED;
 import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_PROVISIONING_ENABLED;
-import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_PROVISIONING_GROUPS;
 import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_PROVISIONING_TOKEN;
 import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_SECRET;
 import static org.sonar.auth.gitlab.GitLabSettings.GITLAB_AUTH_SYNC_USER_GROUPS;
@@ -65,10 +65,10 @@ public class GitlabConfigurationService {
     GITLAB_AUTH_URL,
     GITLAB_AUTH_SECRET,
     GITLAB_AUTH_SYNC_USER_GROUPS,
+    GITLAB_AUTH_ALLOWED_GROUPS,
     GITLAB_AUTH_PROVISIONING_ENABLED,
     GITLAB_AUTH_ALLOW_USERS_TO_SIGNUP,
-    GITLAB_AUTH_PROVISIONING_TOKEN,
-    GITLAB_AUTH_PROVISIONING_GROUPS);
+    GITLAB_AUTH_PROVISIONING_TOKEN);
 
   public static final String UNIQUE_GITLAB_CONFIGURATION_ID = "gitlab-configuration";
   private final DbClient dbClient;
@@ -95,10 +95,10 @@ public class GitlabConfigurationService {
       setIfDefined(dbSession, GITLAB_AUTH_URL, updateRequest.url());
       setIfDefined(dbSession, GITLAB_AUTH_SECRET, updateRequest.secret());
       setIfDefined(dbSession, GITLAB_AUTH_SYNC_USER_GROUPS, updateRequest.synchronizeGroups().map(String::valueOf));
+      setIfDefined(dbSession, GITLAB_AUTH_ALLOWED_GROUPS, updateRequest.allowedGroups().map(groups -> String.join(",", groups)));
       setIfDefined(dbSession, GITLAB_AUTH_PROVISIONING_ENABLED, provisioningEnabled.map(String::valueOf));
       setIfDefined(dbSession, GITLAB_AUTH_ALLOW_USERS_TO_SIGNUP, updateRequest.allowUsersToSignUp().map(String::valueOf));
       setIfDefined(dbSession, GITLAB_AUTH_PROVISIONING_TOKEN, updateRequest.provisioningToken());
-      setIfDefined(dbSession, GITLAB_AUTH_PROVISIONING_GROUPS, updateRequest.provisioningGroups().map(groups -> String.join(",", groups)));
       boolean shouldTriggerProvisioning =
         provisioningEnabled.orElse(false) && !currentConfiguration.provisioningType().equals(AUTO_PROVISIONING);
       deleteExternalGroupsWhenDisablingAutoProvisioning(dbSession, currentConfiguration, updateRequest.provisioningType());
@@ -196,10 +196,10 @@ public class GitlabConfigurationService {
       setProperty(dbSession, GITLAB_AUTH_URL, configuration.url());
       setProperty(dbSession, GITLAB_AUTH_SECRET, configuration.secret());
       setProperty(dbSession, GITLAB_AUTH_SYNC_USER_GROUPS, String.valueOf(configuration.synchronizeGroups()));
+      setProperty(dbSession, GITLAB_AUTH_ALLOWED_GROUPS, String.join(",", configuration.allowedGroups()));
       setProperty(dbSession, GITLAB_AUTH_PROVISIONING_ENABLED, String.valueOf(enableAutoProvisioning));
       setProperty(dbSession, GITLAB_AUTH_ALLOW_USERS_TO_SIGNUP, String.valueOf(configuration.allowUsersToSignUp()));
       setProperty(dbSession, GITLAB_AUTH_PROVISIONING_TOKEN, configuration.provisioningToken());
-      setProperty(dbSession, GITLAB_AUTH_PROVISIONING_GROUPS, String.join(",", configuration.provisioningGroups()));
       if (enableAutoProvisioning) {
         triggerRun(configuration);
       }
@@ -234,15 +234,15 @@ public class GitlabConfigurationService {
       getStringPropertyOrEmpty(dbSession, GITLAB_AUTH_URL),
       getStringPropertyOrEmpty(dbSession, GITLAB_AUTH_SECRET),
       getBooleanOrFalse(dbSession, GITLAB_AUTH_SYNC_USER_GROUPS),
-      toProvisioningType(getBooleanOrFalse(dbSession, GITLAB_AUTH_PROVISIONING_ENABLED)),
+      getAllowedGroups(dbSession),
       getBooleanOrFalse(dbSession, GITLAB_AUTH_ALLOW_USERS_TO_SIGNUP),
-      getStringPropertyOrNull(dbSession, GITLAB_AUTH_PROVISIONING_TOKEN),
-      getProvisioningGroups(dbSession)
+      toProvisioningType(getBooleanOrFalse(dbSession, GITLAB_AUTH_PROVISIONING_ENABLED)),
+      getStringPropertyOrNull(dbSession, GITLAB_AUTH_PROVISIONING_TOKEN)
     );
   }
 
-  private Set<String> getProvisioningGroups(DbSession dbSession) {
-    return Optional.ofNullable(dbClient.propertiesDao().selectGlobalProperty(dbSession, GITLAB_AUTH_PROVISIONING_GROUPS))
+  private Set<String> getAllowedGroups(DbSession dbSession) {
+    return Optional.ofNullable(dbClient.propertiesDao().selectGlobalProperty(dbSession, GITLAB_AUTH_ALLOWED_GROUPS))
       .map(dto -> Arrays.stream(dto.getValue().split(","))
         .filter(s -> !s.isEmpty())
         .collect(Collectors.toSet())
diff --git a/server/sonar-webserver-common/src/main/java/org/sonar/server/common/gitlab/config/UpdateGitlabConfigurationRequest.java b/server/sonar-webserver-common/src/main/java/org/sonar/server/common/gitlab/config/UpdateGitlabConfigurationRequest.java
index dd2f17ad125..23fd0ef2d56 100644
--- a/server/sonar-webserver-common/src/main/java/org/sonar/server/common/gitlab/config/UpdateGitlabConfigurationRequest.java
+++ b/server/sonar-webserver-common/src/main/java/org/sonar/server/common/gitlab/config/UpdateGitlabConfigurationRequest.java
@@ -30,10 +30,10 @@ public record UpdateGitlabConfigurationRequest(
   NonNullUpdatedValue<String> url,
   NonNullUpdatedValue<String> secret,
   NonNullUpdatedValue<Boolean> synchronizeGroups,
-  NonNullUpdatedValue<ProvisioningType> provisioningType,
+  NonNullUpdatedValue<Set<String>> allowedGroups,
   NonNullUpdatedValue<Boolean> allowUsersToSignUp,
   UpdatedValue<String> provisioningToken,
-  NonNullUpdatedValue<Set<String>> provisioningGroups
+  NonNullUpdatedValue<ProvisioningType> provisioningType
 ) {
 
   public static Builder builder() {
@@ -47,10 +47,10 @@ public record UpdateGitlabConfigurationRequest(
     private NonNullUpdatedValue<String> url = NonNullUpdatedValue.undefined();
     private NonNullUpdatedValue<String> secret = NonNullUpdatedValue.undefined();
     private NonNullUpdatedValue<Boolean> synchronizeGroups = NonNullUpdatedValue.undefined();
+    private NonNullUpdatedValue<Set<String>> allowedGroups = NonNullUpdatedValue.undefined();
     private NonNullUpdatedValue<ProvisioningType> provisioningType = NonNullUpdatedValue.undefined();
     private NonNullUpdatedValue<Boolean> allowUserToSignUp = NonNullUpdatedValue.undefined();
     private UpdatedValue<String> provisioningToken = UpdatedValue.undefined();
-    private NonNullUpdatedValue<Set<String>> provisioningGroups = NonNullUpdatedValue.undefined();
 
     private Builder() {
     }
@@ -85,6 +85,11 @@ public record UpdateGitlabConfigurationRequest(
       return this;
     }
 
+    public Builder allowedGroups(NonNullUpdatedValue<Set<String>> allowedGroups) {
+      this.allowedGroups = allowedGroups;
+      return this;
+    }
+
     public Builder provisioningType(NonNullUpdatedValue<ProvisioningType> provisioningType) {
       this.provisioningType = provisioningType;
       return this;
@@ -100,14 +105,10 @@ public record UpdateGitlabConfigurationRequest(
       return this;
     }
 
-    public Builder provisioningGroups(NonNullUpdatedValue<Set<String>> provisioningGroups) {
-      this.provisioningGroups = provisioningGroups;
-      return this;
-    }
-
     public UpdateGitlabConfigurationRequest build() {
-      return new UpdateGitlabConfigurationRequest(gitlabConfigurationId, enabled, applicationId, url, secret, synchronizeGroups, provisioningType, allowUserToSignUp,
-        provisioningToken, provisioningGroups);
+      return new UpdateGitlabConfigurationRequest(gitlabConfigurationId, enabled, applicationId, url, secret, synchronizeGroups, allowedGroups, allowUserToSignUp,
+        provisioningToken, provisioningType
+      );
     }
   }
 }