SONAR-22088 Fix GitLab auth when group sync is disabled

diff --git a/server/sonar-auth-gitlab/build.gradle b/server/sonar-auth-gitlab/build.gradle
index 26b522bafa89589eeb61792b75383c9376e7037f..9a0574fd46ef61cc64c5bf705b5c47b2a3fe9e45 100644 (file)
--- a/server/sonar-auth-gitlab/build.gradle
+++ b/server/sonar-auth-gitlab/build.gradle
@@ -21,6 +21,7 @@ dependencies {
     testImplementation 'com.squareup.okhttp3:mockwebserver'
     testImplementation 'com.squareup.okhttp3:okhttp'
     testImplementation 'junit:junit'
+    testImplementation 'com.tngtech.java:junit-dataprovider'
     testImplementation 'org.assertj:assertj-core'
     testImplementation 'org.mockito:mockito-core'
 }

diff --git a/server/sonar-auth-gitlab/src/main/java/org/sonar/auth/gitlab/GitLabIdentityProvider.java b/server/sonar-auth-gitlab/src/main/java/org/sonar/auth/gitlab/GitLabIdentityProvider.java
index 1ed3db4beca0d51475722e94476cf83439213f44..4478d48b30fde7d6be0ada30bc5e552d2c52c233 100644 (file)
--- a/server/sonar-auth-gitlab/src/main/java/org/sonar/auth/gitlab/GitLabIdentityProvider.java
+++ b/server/sonar-auth-gitlab/src/main/java/org/sonar/auth/gitlab/GitLabIdentityProvider.java
@@ -20,16 +20,17 @@
 package org.sonar.auth.gitlab;
 
 import com.github.scribejava.core.builder.ServiceBuilder;
-import com.github.scribejava.core.builder.ServiceBuilderOAuth20;
 import com.github.scribejava.core.model.OAuth2AccessToken;
 import com.github.scribejava.core.model.OAuthConstants;
 import com.github.scribejava.core.oauth.OAuth20Service;
+import com.google.common.annotations.VisibleForTesting;
 import java.io.IOException;
 import java.util.Collection;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import java.util.stream.Stream;
+import javax.inject.Inject;
 import org.sonar.api.server.authentication.Display;
 import org.sonar.api.server.authentication.OAuth2IdentityProvider;
 import org.sonar.api.server.authentication.UnauthorizedException;
@@ -41,17 +42,24 @@ import static java.util.stream.Collectors.toSet;
 
 public class GitLabIdentityProvider implements OAuth2IdentityProvider {
 
-  public static final String API_SCOPE = "api";
-  public static final String READ_USER_SCOPE = "read_user";
   public static final String KEY = "gitlab";
   private final GitLabSettings gitLabSettings;
   private final ScribeGitLabOauth2Api scribeApi;
   private final GitLabRestClient gitLabRestClient;
+  private final ScribeFactory scribeFactory;
 
+  @Inject
   public GitLabIdentityProvider(GitLabSettings gitLabSettings, GitLabRestClient gitLabRestClient, ScribeGitLabOauth2Api scribeApi) {
+    this(gitLabSettings, gitLabRestClient, scribeApi, new ScribeFactory());
+  }
+
+  @VisibleForTesting
+  GitLabIdentityProvider(GitLabSettings gitLabSettings, GitLabRestClient gitLabRestClient, ScribeGitLabOauth2Api scribeApi,
+    ScribeFactory scribeFactory) {
     this.gitLabSettings = gitLabSettings;
     this.scribeApi = scribeApi;
     this.gitLabRestClient = gitLabRestClient;
+    this.scribeFactory = scribeFactory;
   }
 
   @Override
@@ -85,23 +93,18 @@ public class GitLabIdentityProvider implements OAuth2IdentityProvider {
   @Override
   public void init(InitContext context) {
     String state = context.generateCsrfState();
-    OAuth20Service scribe = newScribeBuilder(context).build(scribeApi);
-    String url = scribe.getAuthorizationUrl(state);
-    context.redirectTo(url);
-  }
-
-  private ServiceBuilderOAuth20 newScribeBuilder(OAuth2Context context) {
-    checkState(isEnabled(), "GitLab authentication is disabled");
-    return new ServiceBuilder(gitLabSettings.applicationId())
-      .apiSecret(gitLabSettings.secret())
-      .defaultScope(gitLabSettings.syncUserGroups() ? API_SCOPE : READ_USER_SCOPE)
-      .callback(context.getCallbackUrl());
+    try (OAuth20Service scribe = scribeFactory.newScribe(gitLabSettings, context.getCallbackUrl(), scribeApi)) {
+      String url = scribe.getAuthorizationUrl(state);
+      context.redirectTo(url);
+    } catch (IOException e) {
+      throw new IllegalStateException(e);
+    }
   }
 
   @Override
   public void callback(CallbackContext context) {
-    try {
-      onCallback(context);
+    try (OAuth20Service scribe = scribeFactory.newScribe(gitLabSettings, context.getCallbackUrl(), scribeApi)) {
+      onCallback(context, scribe);
     } catch (IOException | ExecutionException e) {
       throw new IllegalStateException(e);
     } catch (InterruptedException e) {
@@ -110,12 +113,10 @@ public class GitLabIdentityProvider implements OAuth2IdentityProvider {
     }
   }
 
-  private void onCallback(CallbackContext context) throws InterruptedException, ExecutionException, IOException {
+  private void onCallback(CallbackContext context, OAuth20Service scribe) throws InterruptedException, ExecutionException, IOException {
     HttpRequest request = context.getHttpRequest();
-    OAuth20Service scribe = newScribeBuilder(context).build(scribeApi);
     String code = request.getParameter(OAuthConstants.CODE);
     OAuth2AccessToken accessToken = scribe.getAccessToken(code);
-
     GsonUser user = gitLabRestClient.getUser(scribe, accessToken);
 
     UserIdentity.Builder builder = UserIdentity.builder()
@@ -124,22 +125,20 @@ public class GitLabIdentityProvider implements OAuth2IdentityProvider {
       .setName(user.getName())
       .setEmail(user.getEmail());
 
-
-    Set<String> userGroups = getGroups(scribe, accessToken);
-
-    if (!gitLabSettings.allowedGroups().isEmpty()) {
-      validateUserInAllowedGroups(userGroups, gitLabSettings.allowedGroups());
-    }
-
     if (gitLabSettings.syncUserGroups()) {
+      Set<String> userGroups = getGroups(scribe, accessToken);
+      validateUserInAllowedGroups(userGroups, gitLabSettings.allowedGroups());
       builder.setGroups(userGroups);
     }
-
     context.authenticate(builder.build());
     context.redirectToRequestedPage();
   }
 
-  private static void validateUserInAllowedGroups(Set<String> userGroups, Set<String> allowedGroups) {
+  private void validateUserInAllowedGroups(Set<String> userGroups, Set<String> allowedGroups) {
+    if (gitLabSettings.allowedGroups().isEmpty()) {
+      return;
+    }
+
     boolean allowedUser = userGroups.stream()
       .anyMatch(userGroup -> isAllowedGroup(userGroup, allowedGroups));
 
@@ -160,4 +159,19 @@ public class GitLabIdentityProvider implements OAuth2IdentityProvider {
       .collect(toSet());
   }
 
+  static class ScribeFactory {
+
+    private static final String API_SCOPE = "api";
+    private static final String READ_USER_SCOPE = "read_user";
+
+    OAuth20Service newScribe(GitLabSettings gitLabSettings, String callbackUrl, ScribeGitLabOauth2Api scribeApi) {
+      checkState(gitLabSettings.isEnabled(), "GitLab authentication is disabled");
+      return new ServiceBuilder(gitLabSettings.applicationId())
+        .apiSecret(gitLabSettings.secret())
+        .defaultScope(gitLabSettings.syncUserGroups() ? API_SCOPE : READ_USER_SCOPE)
+        .callback(callbackUrl)
+        .build(scribeApi);
+    }
+  }
+
 }

diff --git a/server/sonar-auth-gitlab/src/test/java/org/sonar/auth/gitlab/GitLabIdentityProviderTest.java b/server/sonar-auth-gitlab/src/test/java/org/sonar/auth/gitlab/GitLabIdentityProviderTest.java
index 49399eb64e75d7fcee49d78145c751f8b3a8a510..e8316171d97a3d700873ab9e2ea73f3192e1cb99 100644 (file)
--- a/server/sonar-auth-gitlab/src/test/java/org/sonar/auth/gitlab/GitLabIdentityProviderTest.java
+++ b/server/sonar-auth-gitlab/src/test/java/org/sonar/auth/gitlab/GitLabIdentityProviderTest.java
@@ -19,25 +19,85 @@
  */
 package org.sonar.auth.gitlab;
 
-import org.assertj.core.api.Assertions;
+import com.github.scribejava.core.model.OAuth2AccessToken;
+import com.github.scribejava.core.model.OAuthConstants;
+import com.github.scribejava.core.oauth.OAuth20Service;
+import com.tngtech.java.junit.dataprovider.DataProvider;
+import com.tngtech.java.junit.dataprovider.DataProviderRunner;
+import com.tngtech.java.junit.dataprovider.UseDataProvider;
+import java.io.IOException;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import org.junit.Before;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Answers;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
 import org.sonar.api.server.authentication.Display;
 import org.sonar.api.server.authentication.OAuth2IdentityProvider;
+import org.sonar.api.server.authentication.UnauthorizedException;
+import org.sonar.api.server.authentication.UserIdentity;
 
+import static java.util.stream.Collectors.toSet;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
+import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
+import static org.mockito.MockitoAnnotations.openMocks;
 
+@RunWith(DataProviderRunner.class)
 public class GitLabIdentityProviderTest {
 
+  private static final String OAUTH_CODE = "code fdsojfsjodfg";
+  private static final String AUTHORIZATION_URL = "AUTHORIZATION_URL";
+  private static final String CALLBACK_URL = "CALLBACK_URL";
+  private static final String STATE = "State request";
+
+  @Mock
+  private GitLabRestClient gitLabRestClient;
+  @Mock
+  private GitLabSettings gitLabSettings;
+  @Mock(answer = Answers.RETURNS_DEEP_STUBS)
+  private GitLabIdentityProvider.ScribeFactory scribeFactory;
+  @Mock
+  private OAuth2IdentityProvider.InitContext initContext;
+  @Mock(answer = Answers.RETURNS_DEEP_STUBS)
+  private OAuth2IdentityProvider.CallbackContext callbackContext;
+  @Mock
+  private OAuth20Service scribe;
+  @Mock
+  private ScribeGitLabOauth2Api scribeApi;
+  @Mock
+  private OAuth2AccessToken accessToken;
+
+  private GitLabIdentityProvider gitLabIdentityProvider;
+
+  @Before
+  public void setup() throws IOException, ExecutionException, InterruptedException {
+    openMocks(this);
+    gitLabIdentityProvider = new GitLabIdentityProvider(gitLabSettings, gitLabRestClient, scribeApi, scribeFactory);
+
+    when(initContext.generateCsrfState()).thenReturn(STATE);
+    when(initContext.getCallbackUrl()).thenReturn(CALLBACK_URL);
+
+    when(callbackContext.getCallbackUrl()).thenReturn(CALLBACK_URL);
+    when(callbackContext.getHttpRequest().getParameter(OAuthConstants.CODE)).thenReturn(OAUTH_CODE);
+
+    when(scribeFactory.newScribe(gitLabSettings, CALLBACK_URL, scribeApi)).thenReturn(scribe);
+    when(scribe.getAccessToken(OAUTH_CODE)).thenReturn(accessToken);
+    when(scribe.getAuthorizationUrl(STATE)).thenReturn(AUTHORIZATION_URL);
+  }
+
   @Test
   public void test_identity_provider() {
-    GitLabSettings gitLabSettings = mock(GitLabSettings.class);
     when(gitLabSettings.isEnabled()).thenReturn(true);
     when(gitLabSettings.allowUsersToSignUp()).thenReturn(true);
-    GitLabIdentityProvider gitLabIdentityProvider = new GitLabIdentityProvider(gitLabSettings, new GitLabRestClient(gitLabSettings),
-      new ScribeGitLabOauth2Api(gitLabSettings));
 
     assertThat(gitLabIdentityProvider.getKey()).isEqualTo("gitlab");
     assertThat(gitLabIdentityProvider.getName()).isEqualTo("GitLab");
@@ -49,61 +109,165 @@ public class GitLabIdentityProviderTest {
   }
 
   @Test
-  public void test_init() {
-    GitLabSettings gitLabSettings = mock(GitLabSettings.class);
-    when(gitLabSettings.isEnabled()).thenReturn(true);
-    when(gitLabSettings.allowUsersToSignUp()).thenReturn(true);
-    when(gitLabSettings.applicationId()).thenReturn("123");
-    when(gitLabSettings.secret()).thenReturn("456");
-    when(gitLabSettings.url()).thenReturn("http://server");
-    when(gitLabSettings.syncUserGroups()).thenReturn(true);
-    GitLabIdentityProvider gitLabIdentityProvider = new GitLabIdentityProvider(gitLabSettings, new GitLabRestClient(gitLabSettings),
-      new ScribeGitLabOauth2Api(gitLabSettings));
+  public void init_whenSuccessful_redirectsToUrl() {
+    gitLabIdentityProvider.init(initContext);
+
+    verify(initContext).generateCsrfState();
+    verify(initContext).redirectTo(AUTHORIZATION_URL);
+  }
+
+  @Test
+  public void init_whenErrorWhileBuildingScribe_shouldReThrow() {
+    IllegalStateException exception = new IllegalStateException("GitLab authentication is disabled");
+    when(scribeFactory.newScribe(any(), any(), any())).thenThrow(exception);
 
     OAuth2IdentityProvider.InitContext initContext = mock(OAuth2IdentityProvider.InitContext.class);
     when(initContext.getCallbackUrl()).thenReturn("http://server/callback");
 
-    gitLabIdentityProvider.init(initContext);
+    assertThatIllegalStateException()
+      .isThrownBy(() -> gitLabIdentityProvider.init(initContext))
+      .isEqualTo(exception);
+  }
+
+  @Test
+  public void onCallback_withGroupSyncDisabledAndNoAllowedGroups_redirectsToRequestedPage() {
+    GsonUser gsonUser = mockGsonUser();
+
+    gitLabIdentityProvider.callback(callbackContext);
 
-    verify(initContext).redirectTo("http://server/oauth/authorize?response_type=code&client_id=123&redirect_uri=http%3A%2F%2Fserver%2Fcallback&scope=api");
+    verifyAuthenticateIsCalledWithExpectedIdentity(callbackContext, gsonUser, Set.of());
+    verify(callbackContext).redirectToRequestedPage();
+    verify(gitLabRestClient, never()).getGroups(any(), any());
   }
 
   @Test
-  public void test_init_without_sync() {
-    GitLabSettings gitLabSettings = mock(GitLabSettings.class);
-    when(gitLabSettings.isEnabled()).thenReturn(true);
-    when(gitLabSettings.allowUsersToSignUp()).thenReturn(true);
-    when(gitLabSettings.applicationId()).thenReturn("123");
-    when(gitLabSettings.secret()).thenReturn("456");
-    when(gitLabSettings.url()).thenReturn("http://server");
+  public void onCallback_withGroupSyncDisabledAndAllowedGroups_redirectsToRequestedPage() {
     when(gitLabSettings.syncUserGroups()).thenReturn(false);
-    GitLabIdentityProvider gitLabIdentityProvider = new GitLabIdentityProvider(gitLabSettings, new GitLabRestClient(gitLabSettings),
-      new ScribeGitLabOauth2Api(gitLabSettings));
 
-    OAuth2IdentityProvider.InitContext initContext = mock(OAuth2IdentityProvider.InitContext.class);
-    when(initContext.getCallbackUrl()).thenReturn("http://server/callback");
+    GsonUser gsonUser = mockGsonUser();
 
-    gitLabIdentityProvider.init(initContext);
+    gitLabIdentityProvider.callback(callbackContext);
 
-    verify(initContext).redirectTo("http://server/oauth/authorize?response_type=code&client_id=123&redirect_uri=http%3A%2F%2Fserver%2Fcallback&scope=read_user");
+    verifyAuthenticateIsCalledWithExpectedIdentity(callbackContext, gsonUser, Set.of());
+    verify(callbackContext).redirectToRequestedPage();
+    verify(gitLabRestClient, never()).getGroups(any(), any());
   }
 
   @Test
-  public void fail_to_init() {
-    GitLabSettings gitLabSettings = mock(GitLabSettings.class);
+  @UseDataProvider("allowedGroups")
+  public void onCallback_withGroupSyncAndAllowedGroupsMatching_redirectsToRequestedPage(Set<String> allowedGroups) {
+    when(gitLabSettings.syncUserGroups()).thenReturn(true);
+    when(gitLabSettings.allowedGroups()).thenReturn(allowedGroups);
+
+    GsonUser gsonUser = mockGsonUser();
+    Set<GsonGroup> gsonGroups = mockGitlabGroups();
+
+    gitLabIdentityProvider.callback(callbackContext);
+
+    verifyAuthenticateIsCalledWithExpectedIdentity(callbackContext, gsonUser, gsonGroups);
+    verify(callbackContext).redirectToRequestedPage();
+  }
+
+  @DataProvider
+  public static Object[][] allowedGroups() {
+    return new Object[][]{
+      {Set.of()},
+      {Set.of("path")}
+    };
+  }
+
+  @Test
+  public void onCallback_withGroupSyncAndAllowedGroupsNotMatching_shouldThrow() {
+    when(gitLabSettings.syncUserGroups()).thenReturn(true);
+    when(gitLabSettings.allowedGroups()).thenReturn(Set.of("path2"));
+
+    mockGsonUser();
+    mockGitlabGroups();
+
+    assertThatExceptionOfType(UnauthorizedException.class)
+      .isThrownBy(() -> gitLabIdentityProvider.callback(callbackContext))
+      .withMessage("You are not allowed to authenticate");
+  }
+
+  @Test
+  public void onCallback_ifScribeFactoryFails_shouldThrow() {
+    IllegalStateException exception = new IllegalStateException("message");
+    when(scribeFactory.newScribe(any(), any(), any())).thenThrow(exception);
+
+    assertThatIllegalStateException()
+      .isThrownBy(() -> gitLabIdentityProvider.callback(callbackContext))
+      .isEqualTo(exception);
+  }
+
+  private Set<GsonGroup> mockGitlabGroups() {
+    GsonGroup gsonGroup = mock(GsonGroup.class);
+    when(gsonGroup.getFullPath()).thenReturn("path/to/group");
+    GsonGroup gsonGroup2 = mock(GsonGroup.class);
+    when(gsonGroup2.getFullPath()).thenReturn("path/to/group2");
+    when(gitLabRestClient.getGroups(scribe, accessToken)).thenReturn(List.of(gsonGroup, gsonGroup2));
+    return Set.of(gsonGroup, gsonGroup2);
+  }
+
+  private static void verifyAuthenticateIsCalledWithExpectedIdentity(OAuth2IdentityProvider.CallbackContext callbackContext,
+    GsonUser gsonUser, Set<GsonGroup> gsonGroups) {
+    ArgumentCaptor<UserIdentity> userIdentityCaptor = ArgumentCaptor.forClass(UserIdentity.class);
+    verify(callbackContext).authenticate(userIdentityCaptor.capture());
+
+    UserIdentity actualIdentity = userIdentityCaptor.getValue();
+
+    assertThat(actualIdentity.getProviderId()).asLong().isEqualTo(gsonUser.getId());
+    assertThat(actualIdentity.getProviderLogin()).isEqualTo(gsonUser.getUsername());
+    assertThat(actualIdentity.getName()).isEqualTo(gsonUser.getName());
+    assertThat(actualIdentity.getEmail()).isEqualTo(gsonUser.getEmail());
+    assertThat(actualIdentity.getGroups()).isEqualTo(gsonGroups.stream().map(GsonGroup::getFullPath).collect(toSet()));
+  }
+
+  private GsonUser mockGsonUser() {
+    GsonUser gsonUser = mock();
+    when(gsonUser.getId()).thenReturn(432423L);
+    when(gsonUser.getUsername()).thenReturn("userName");
+    when(gsonUser.getName()).thenReturn("name");
+    when(gsonUser.getEmail()).thenReturn("toto@gitlab.com");
+    when(gitLabRestClient.getUser(scribe, accessToken)).thenReturn(gsonUser);
+    return gsonUser;
+  }
+
+  @Test
+  public void newScribe_whenGitLabAuthIsDisabled_throws() {
     when(gitLabSettings.isEnabled()).thenReturn(false);
-    when(gitLabSettings.allowUsersToSignUp()).thenReturn(true);
-    when(gitLabSettings.applicationId()).thenReturn("123");
-    when(gitLabSettings.secret()).thenReturn("456");
-    when(gitLabSettings.url()).thenReturn("http://server");
-    GitLabIdentityProvider gitLabIdentityProvider = new GitLabIdentityProvider(gitLabSettings, new GitLabRestClient(gitLabSettings),
+
+    assertThatIllegalStateException()
+      .isThrownBy(() -> new GitLabIdentityProvider.ScribeFactory().newScribe(gitLabSettings, CALLBACK_URL, new ScribeGitLabOauth2Api(gitLabSettings)))
+      .withMessage("GitLab authentication is disabled");
+  }
+
+  @Test
+  @UseDataProvider("groupsSyncToScope")
+  public void newScribe_whenGitLabSettingsValid_shouldUseCorrectScopeDependingOnGroupSync(boolean groupSyncEnabled, String expectedScope) {
+    setupGitlabSettingsWithGroupSync(groupSyncEnabled);
+
+
+    OAuth20Service realScribe = new GitLabIdentityProvider.ScribeFactory().newScribe(gitLabSettings, CALLBACK_URL,
       new ScribeGitLabOauth2Api(gitLabSettings));
 
-    OAuth2IdentityProvider.InitContext initContext = mock(OAuth2IdentityProvider.InitContext.class);
-    when(initContext.getCallbackUrl()).thenReturn("http://server/callback");
+    assertThat(realScribe).isNotNull();
+    assertThat(realScribe.getCallback()).isEqualTo(CALLBACK_URL);
+    assertThat(realScribe.getApiSecret()).isEqualTo(gitLabSettings.secret());
+    assertThat(realScribe.getDefaultScope()).isEqualTo(expectedScope);
+  }
 
-    Assertions.assertThatThrownBy(() -> gitLabIdentityProvider.init(initContext))
-      .hasMessage("GitLab authentication is disabled")
-      .isInstanceOf(IllegalStateException.class);
+  @DataProvider
+  public static Object[][] groupsSyncToScope() {
+    return new Object[][]{
+      {false, "read_user"},
+      {true, "api"}
+    };
+  }
+
+  private void setupGitlabSettingsWithGroupSync(boolean enableGroupSync) {
+    when(gitLabSettings.isEnabled()).thenReturn(true);
+    when(gitLabSettings.applicationId()).thenReturn("123");
+    when(gitLabSettings.secret()).thenReturn("456");
+    when(gitLabSettings.syncUserGroups()).thenReturn(enableGroupSync);
   }
 }

diff --git a/server/sonar-auth-gitlab/src/test/java/org/sonar/auth/gitlab/IntegrationTest.java b/server/sonar-auth-gitlab/src/test/java/org/sonar/auth/gitlab/IntegrationTest.java
index 8f933d881ef456ab20e7fb9b13a31e4f38ca6d87..7446ea83523eeba3c1a34a37aaf00cfb0168ca5b 100644 (file)
--- a/server/sonar-auth-gitlab/src/test/java/org/sonar/auth/gitlab/IntegrationTest.java
+++ b/server/sonar-auth-gitlab/src/test/java/org/sonar/auth/gitlab/IntegrationTest.java
@@ -72,7 +72,8 @@ public class IntegrationTest {
       .setProperty(GITLAB_AUTH_URL, gitLabUrl)
       .setProperty(GITLAB_AUTH_APPLICATION_ID, "123")
       .setProperty(GITLAB_AUTH_SECRET, "456")
-      .setProperty(GITLAB_AUTH_ALLOWED_GROUPS, "group1,group2");
+      .setProperty(GITLAB_AUTH_ALLOWED_GROUPS, "group1,group2")
+      .setProperty(GITLAB_AUTH_SYNC_USER_GROUPS, "true");
   }
 
   @Test
@@ -96,7 +97,7 @@ public class IntegrationTest {
   }
 
   @Test
-  public void callback_whenNotAllowedUser_shouldThrow() {
+  public void callback_whenGroupNotAllowedAndGroupSyncEnabled_shouldThrow() {
     OAuth2IdentityProvider.CallbackContext callbackContext = mockCallbackContext();
 
     mockAccessTokenResponse();
@@ -108,6 +109,21 @@ public class IntegrationTest {
       .hasMessage("You are not allowed to authenticate");
   }
 
+  @Test
+  public void callback_whenGroupNotAllowedAndGroupSyncDisabled_shouldThrow() {
+    mapSettings.setProperty(GITLAB_AUTH_SYNC_USER_GROUPS, "false");
+    OAuth2IdentityProvider.CallbackContext callbackContext = mockCallbackContext();
+
+    mockAccessTokenResponse();
+    mockUserResponse();
+    mockSingleGroupReponse("wrong-group");
+
+    gitLabIdentityProvider.callback(callbackContext);
+
+    verify(callbackContext).authenticate(any());
+    verify(callbackContext).redirectToRequestedPage();
+  }
+
   @Test
   public void callback_whenAllowedUserBySubgroupMembership_shouldAuthenticate() {
     OAuth2IdentityProvider.CallbackContext callbackContext = mockCallbackContext();

diff --git a/sonar-core/src/main/resources/org/sonar/l10n/core.properties b/sonar-core/src/main/resources/org/sonar/l10n/core.properties
index 0cedde212a9caf369cc393db23d3a5c653a5d10a..a09e67795bc2cfcf0224356fb19e6794d2865907 100644 (file)
--- a/sonar-core/src/main/resources/org/sonar/l10n/core.properties
+++ b/sonar-core/src/main/resources/org/sonar/l10n/core.properties
@@ -1598,7 +1598,7 @@ settings.authentication.gitlab.form.secret.description=Secret provided by GitLab
 settings.authentication.gitlab.form.synchronizeGroups.name=Synchronize user groups
 settings.authentication.gitlab.form.synchronizeGroups.description=For each GitLab group they belong to, the user will be associated to a group with the same name (if it exists) in SonarQube. If enabled, the GitLab OAuth 2 application will need to provide the api scope.
 settings.authentication.gitlab.form.allowedGroups.name=Allowed groups
-settings.authentication.gitlab.form.allowedGroups.description.JIT=Only members of these groups (and sub-groups) will be allowed to authenticate. Please enter the group slug as it appears in the GitLab URL, for instance `my-gitlab-group`. ⚠︎ if not set and `Allow users to sign up` is enabled, any user from GitLab will be able to login to this SonarQube instance.
+settings.authentication.gitlab.form.allowedGroups.description.JIT=Only members of these groups (and sub-groups) will be allowed to authenticate. Enter the group slug as it appears in the GitLab URL, for instance `my-gitlab-group`. ⚠︎ When you turn on `Allow users to sign up`, make sure to also turn on group synchronization and provide a list of allowed groups. Otherwise, any GitLab user will be able to log in to this SonarQube instance.
 settings.authentication.gitlab.form.allowedGroups.description.AUTO_PROVISIONING=Only members of these groups (and sub-groups) will be provisioned. Please enter the group slug as it appears in the GitLab URL, for instance `my-gitlab-group`.
 settings.authentication.gitlab.form.allowUsersToSignUp.name=Allow users to sign up
 settings.authentication.gitlab.form.allowUsersToSignUp.description=Allow new users to authenticate. When set to disabled, only existing users will be able to authenticate to the server.