@@ -158,6 +158,19 @@ public class GitlabConfigurationServiceIT { | |||
.withMessage("GitLab configuration doesn't exist."); | |||
} | |||
@Test | |||
public void updateConfiguration_whenAllowedGroupsIsEmptyAndAutoProvisioningIsEnabled_throwsException() { | |||
gitlabConfigurationService.createConfiguration(buildGitlabConfiguration(AUTO_PROVISIONING)); | |||
UpdateGitlabConfigurationRequest updateGitlabConfigurationRequest = builder() | |||
.gitlabConfigurationId(UNIQUE_GITLAB_CONFIGURATION_ID) | |||
.allowedGroups(withValueOrThrow(new LinkedHashSet<>())) | |||
.build(); | |||
assertThatThrownBy(() -> gitlabConfigurationService.updateConfiguration(updateGitlabConfigurationRequest)) | |||
.isInstanceOf(IllegalArgumentException.class) | |||
.hasMessage("allowedGroups cannot be empty when Auto-provisioning is enabled."); | |||
} | |||
@Test | |||
public void updateConfiguration_whenAllUpdateFieldDefined_updatesEverything() { | |||
gitlabConfigurationService.createConfiguration(buildGitlabConfiguration(JIT)); | |||
@@ -306,6 +319,25 @@ public class GitlabConfigurationServiceIT { | |||
.hasMessage("GitLab configuration already exists. Only one Gitlab configuration is supported."); | |||
} | |||
@Test | |||
public void createConfiguration_whenAllowedGroupsIsEmptyAndAutoProvisioningIsEnabled_shouldReturnBadRequest() { | |||
GitlabConfiguration configuration = new GitlabConfiguration( | |||
UNIQUE_GITLAB_CONFIGURATION_ID, | |||
true, | |||
"applicationId", | |||
"url", | |||
"secret", | |||
true, | |||
Set.of(), | |||
true, | |||
AUTO_PROVISIONING, | |||
"provisioningToken"); | |||
assertThatThrownBy(() -> gitlabConfigurationService.createConfiguration(configuration)) | |||
.isInstanceOf(IllegalArgumentException.class) | |||
.hasMessage("allowedGroups cannot be empty when Auto-provisioning is enabled."); | |||
} | |||
@Test | |||
public void createConfiguration_whenAutoProvisioning_shouldCreateCorrectConfigurationAndScheduleSync() { | |||
GitlabConfiguration configuration = buildGitlabConfiguration(AUTO_PROVISIONING); | |||
@@ -332,8 +364,7 @@ public class GitlabConfigurationServiceIT { | |||
Set.of("group1", "group2", "group3"), | |||
true, | |||
AUTO_PROVISIONING, | |||
null | |||
); | |||
null); | |||
assertThatThrownBy(() -> gitlabConfigurationService.createConfiguration(configuration)) | |||
.isInstanceOf(IllegalStateException.class) | |||
@@ -379,8 +410,7 @@ public class GitlabConfigurationServiceIT { | |||
Set.of("group1", "group2", "group3"), | |||
true, | |||
JIT, | |||
null | |||
); | |||
null); | |||
GitlabConfiguration createdConfiguration = gitlabConfigurationService.createConfiguration(configuration); | |||
@@ -526,7 +556,7 @@ public class GitlabConfigurationServiceIT { | |||
Exception exception = new IllegalStateException("Invalid configuration"); | |||
when(gitlabConfigurationService.validate(gitlabConfiguration)).thenThrow(exception); | |||
Optional<String> message = gitlabConfigurationService.validate(gitlabConfiguration); | |||
Optional<String> message = gitlabConfigurationService.validate(gitlabConfiguration); | |||
assertThat(message).contains("Invalid configuration"); | |||
} |
@@ -85,11 +85,15 @@ public class GitlabConfigurationService { | |||
} | |||
public GitlabConfiguration updateConfiguration(UpdateGitlabConfigurationRequest updateRequest) { | |||
UpdatedValue<Boolean> provisioningEnabled = | |||
updateRequest.provisioningType().map(GitlabConfigurationService::shouldEnableAutoProvisioning); | |||
UpdatedValue<Boolean> provisioningEnabled = updateRequest.provisioningType().map(GitlabConfigurationService::shouldEnableAutoProvisioning); | |||
try (DbSession dbSession = dbClient.openSession(true)) { | |||
throwIfConfigurationDoesntExist(dbSession); | |||
GitlabConfiguration currentConfiguration = getConfiguration(updateRequest.gitlabConfigurationId(), dbSession); | |||
ProvisioningType provisioningType = updateRequest.provisioningType().orElse(currentConfiguration.provisioningType()); | |||
Set<String> allowedGroups = updateRequest.allowedGroups().orElse(currentConfiguration.allowedGroups()); | |||
throwIfAllowedGroupsEmptyAndAutoProvisioning(provisioningType, allowedGroups); | |||
setIfDefined(dbSession, GITLAB_AUTH_ENABLED, updateRequest.enabled().map(String::valueOf)); | |||
setIfDefined(dbSession, GITLAB_AUTH_APPLICATION_ID, updateRequest.applicationId()); | |||
setIfDefined(dbSession, GITLAB_AUTH_URL, updateRequest.url()); | |||
@@ -99,8 +103,7 @@ public class GitlabConfigurationService { | |||
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()); | |||
boolean shouldTriggerProvisioning = | |||
provisioningEnabled.orElse(false) && !currentConfiguration.provisioningType().equals(AUTO_PROVISIONING); | |||
boolean shouldTriggerProvisioning = provisioningEnabled.orElse(false) && !currentConfiguration.provisioningType().equals(AUTO_PROVISIONING); | |||
deleteExternalGroupsWhenDisablingAutoProvisioning(dbSession, currentConfiguration, updateRequest.provisioningType()); | |||
GitlabConfiguration updatedConfiguration = getConfiguration(UNIQUE_GITLAB_CONFIGURATION_ID, dbSession); | |||
if (shouldTriggerProvisioning) { | |||
@@ -122,9 +125,8 @@ public class GitlabConfigurationService { | |||
DbSession dbSession, | |||
GitlabConfiguration currentConfiguration, | |||
UpdatedValue<ProvisioningType> provisioningTypeFromUpdate) { | |||
boolean disableAutoProvisioning = | |||
provisioningTypeFromUpdate.map(provisioningType -> provisioningType.equals(JIT)).orElse(false) | |||
&& currentConfiguration.provisioningType().equals(AUTO_PROVISIONING); | |||
boolean disableAutoProvisioning = provisioningTypeFromUpdate.map(provisioningType -> provisioningType.equals(JIT)).orElse(false) | |||
&& currentConfiguration.provisioningType().equals(AUTO_PROVISIONING); | |||
if (disableAutoProvisioning) { | |||
dbClient.externalGroupDao().deleteByExternalIdentityProvider(dbSession, GitLabIdentityProvider.KEY); | |||
} | |||
@@ -188,6 +190,7 @@ public class GitlabConfigurationService { | |||
public GitlabConfiguration createConfiguration(GitlabConfiguration configuration) { | |||
throwIfConfigurationAlreadyExists(); | |||
throwIfAllowedGroupsEmptyAndAutoProvisioning(configuration.provisioningType(), configuration.allowedGroups()); | |||
boolean enableAutoProvisioning = shouldEnableAutoProvisioning(configuration.provisioningType()); | |||
try (DbSession dbSession = dbClient.openSession(false)) { | |||
@@ -216,6 +219,12 @@ public class GitlabConfigurationService { | |||
}); | |||
} | |||
private static void throwIfAllowedGroupsEmptyAndAutoProvisioning(ProvisioningType provisioningType, Set<String> allowedGroups) { | |||
if (provisioningType == AUTO_PROVISIONING && allowedGroups.isEmpty()) { | |||
throw new IllegalArgumentException("allowedGroups cannot be empty when Auto-provisioning is enabled."); | |||
} | |||
} | |||
private static boolean shouldEnableAutoProvisioning(ProvisioningType provisioningType) { | |||
return AUTO_PROVISIONING.equals(provisioningType); | |||
} | |||
@@ -237,16 +246,15 @@ public class GitlabConfigurationService { | |||
getAllowedGroups(dbSession), | |||
getBooleanOrFalse(dbSession, GITLAB_AUTH_ALLOW_USERS_TO_SIGNUP), | |||
toProvisioningType(getBooleanOrFalse(dbSession, GITLAB_AUTH_PROVISIONING_ENABLED)), | |||
getStringPropertyOrNull(dbSession, GITLAB_AUTH_PROVISIONING_TOKEN) | |||
); | |||
getStringPropertyOrNull(dbSession, GITLAB_AUTH_PROVISIONING_TOKEN)); | |||
} | |||
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()) | |||
).orElse(Set.of()); | |||
.collect(Collectors.toSet())) | |||
.orElse(Set.of()); | |||
} | |||
public void triggerRun() { | |||
@@ -289,8 +297,7 @@ public class GitlabConfigurationService { | |||
gitlabGlobalSettingsValidator.validate( | |||
toValidationMode(gitlabConfiguration.provisioningType()), | |||
url, | |||
gitlabConfiguration.provisioningToken() | |||
); | |||
gitlabConfiguration.provisioningToken()); | |||
} catch (Exception e) { | |||
return Optional.of(e.getMessage()); | |||
} |
@@ -23,6 +23,7 @@ import java.util.HashSet; | |||
import java.util.List; | |||
import java.util.Optional; | |||
import java.util.Set; | |||
import javax.annotation.Nullable; | |||
import org.apache.commons.lang.StringUtils; | |||
import org.sonar.server.common.gitlab.config.GitlabConfiguration; | |||
import org.sonar.server.common.gitlab.config.GitlabConfigurationService; | |||
@@ -35,6 +36,7 @@ import org.sonar.server.v2.api.gitlab.config.resource.GitlabConfigurationResourc | |||
import org.sonar.server.v2.api.gitlab.config.response.GitlabConfigurationSearchRestResponse; | |||
import org.sonar.server.v2.api.response.PageRestResponse; | |||
import static org.sonar.api.utils.Preconditions.checkArgument; | |||
import static org.sonar.server.common.gitlab.config.GitlabConfigurationService.UNIQUE_GITLAB_CONFIGURATION_ID; | |||
public class DefaultGitlabConfigurationController implements GitlabConfigurationController { | |||
@@ -73,7 +75,6 @@ public class DefaultGitlabConfigurationController implements GitlabConfiguration | |||
return toGitLabConfigurationResource(createdConfiguration); | |||
} | |||
private static GitlabConfiguration toGitlabConfiguration(GitlabConfigurationCreateRestRequest createRestRequest) { | |||
return new GitlabConfiguration( | |||
UNIQUE_GITLAB_CONFIGURATION_ID, | |||
@@ -85,8 +86,7 @@ public class DefaultGitlabConfigurationController implements GitlabConfiguration | |||
Set.copyOf(createRestRequest.allowedGroups()), | |||
createRestRequest.allowUsersToSignUp() != null && createRestRequest.allowUsersToSignUp(), | |||
toProvisioningType(createRestRequest.provisioningType()), | |||
createRestRequest.provisioningToken() | |||
); | |||
createRestRequest.provisioningToken()); | |||
} | |||
private GitlabConfigurationResource getGitlabConfigurationResource(String id) { | |||
@@ -116,7 +116,8 @@ public class DefaultGitlabConfigurationController implements GitlabConfiguration | |||
.build(); | |||
} | |||
private static Set<String> getGroups(List<String> groups) { | |||
private static Set<String> getGroups(@Nullable List<String> groups) { | |||
checkArgument(groups != null, "allowedGroups must not be null"); | |||
return new HashSet<>(groups); | |||
} | |||
@@ -132,8 +133,7 @@ public class DefaultGitlabConfigurationController implements GitlabConfiguration | |||
configuration.allowUsersToSignUp(), | |||
toRestProvisioningType(configuration), | |||
StringUtils.isNotEmpty(configuration.provisioningToken()), | |||
configurationError.orElse(null) | |||
); | |||
configurationError.orElse(null)); | |||
} | |||
private static org.sonar.server.v2.api.gitlab.config.resource.ProvisioningType toRestProvisioningType(GitlabConfiguration configuration) { |
@@ -49,8 +49,13 @@ public record GitlabConfigurationCreateRestRequest( | |||
@Schema(description = "Set whether to synchronize groups") | |||
Boolean synchronizeGroups, | |||
@NotEmpty | |||
@ArraySchema(arraySchema = @Schema(description = "GitLab groups allowed to authenticate and provisioned (for Auto-Provisioning only). Subgroups will automatically be included")) | |||
@NotNull | |||
@ArraySchema(arraySchema = @Schema(description = """ | |||
GitLab groups allowed to authenticate. | |||
Subgroups will automatically be included. | |||
When Auto-provisioning is enabled, members of these groups will be automatically provisioned in SonarQube. | |||
This field is required to be non-empty for Auto-provisioning. | |||
""")) | |||
List<String> allowedGroups, | |||
@NotNull |
@@ -193,6 +193,30 @@ public class DefaultGitlabConfigurationControllerTest { | |||
content().json("{\"message\":\"Insufficient privileges\"}")); | |||
} | |||
@Test | |||
public void updateConfiguration_whenAllowedGroupsIsNull_shouldReturnBadRequest() throws Exception { | |||
userSession.logIn().setSystemAdministrator(); | |||
mockMvc.perform(patch(GITLAB_CONFIGURATION_ENDPOINT + "/existing-id") | |||
.contentType(JSON_MERGE_PATCH_CONTENT_TYPE) | |||
.content(""" | |||
{ | |||
"enabled": true, | |||
"applicationId": "application-id", | |||
"secret": "123", | |||
"url": "www.url.com", | |||
"synchronizeGroups": true, | |||
"allowedGroups": null, | |||
"provisioningType": "AUTO_PROVISIONING", | |||
"allowUsersToSignUp": true | |||
} | |||
""")) | |||
.andExpectAll( | |||
status().isBadRequest(), | |||
content().json("{\"message\":\"allowedGroups must not be null\"}")); | |||
} | |||
@Test | |||
public void updateConfiguration_whenAllFieldsUpdated_performUpdates() throws Exception { | |||
userSession.logIn().setSystemAdministrator(); | |||
@@ -398,6 +422,30 @@ public class DefaultGitlabConfigurationControllerTest { | |||
} | |||
@Test | |||
public void create_whenAllowedGroupsIsNull_shouldReturnBadRequest() throws Exception { | |||
userSession.logIn().setSystemAdministrator(); | |||
mockMvc.perform( | |||
post(GITLAB_CONFIGURATION_ENDPOINT) | |||
.contentType(MediaType.APPLICATION_JSON_VALUE) | |||
.content(""" | |||
{ | |||
"enabled": true, | |||
"applicationId": "application-id", | |||
"secret": "123", | |||
"url": "www.url.com", | |||
"synchronizeGroups": true, | |||
"allowedGroups": null, | |||
"provisioningType": "JIT" | |||
} | |||
""")) | |||
.andExpectAll( | |||
status().isBadRequest(), | |||
content().json("{\"message\":\"Value {} for field allowedGroups was rejected. Error: must not be null.\"}")); | |||
} | |||
@Test | |||
public void create_whenRequiredParameterIsMissing_shouldReturnBadRequest() throws Exception { | |||
userSession.logIn().setSystemAdministrator(); |