SONAR-21413 Use GitLab allowed groups to filter users authenticating

3 files changed, 174 insertions, 61 deletions

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 93d81dc3dff..d2a9b0fb980 100644
--- 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
@@ -32,6 +32,7 @@ import java.util.concurrent.ExecutionException;
 import java.util.stream.Stream;
 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 org.sonar.api.server.http.HttpRequest;
 
@@ -41,7 +42,6 @@ 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;
@@ -84,16 +84,16 @@ public class GitLabIdentityProvider implements OAuth2IdentityProvider {
   @Override
   public void init(InitContext context) {
     String state = context.generateCsrfState();
-    OAuth20Service scribe = newScribeBuilder(context, gitLabSettings.syncUserGroups()).build(scribeApi);
+    OAuth20Service scribe = newScribeBuilder(context).build(scribeApi);
     String url = scribe.getAuthorizationUrl(state);
     context.redirectTo(url);
   }
 
-  private ServiceBuilderOAuth20 newScribeBuilder(OAuth2Context context, boolean syncUserGroups) {
+  private ServiceBuilderOAuth20 newScribeBuilder(OAuth2Context context) {
     checkState(isEnabled(), "GitLab authentication is disabled");
     return new ServiceBuilder(gitLabSettings.applicationId())
       .apiSecret(gitLabSettings.secret())
-      .defaultScope(syncUserGroups ? API_SCOPE : READ_USER_SCOPE)
+      .defaultScope(API_SCOPE)
       .callback(context.getCallbackUrl());
   }
 
@@ -111,7 +111,7 @@ public class GitLabIdentityProvider implements OAuth2IdentityProvider {
 
   private void onCallback(CallbackContext context) throws InterruptedException, ExecutionException, IOException {
     HttpRequest request = context.getHttpRequest();
-    OAuth20Service scribe = newScribeBuilder(context, gitLabSettings.syncUserGroups()).build(scribeApi);
+    OAuth20Service scribe = newScribeBuilder(context).build(scribeApi);
     String code = request.getParameter(OAuthConstants.CODE);
     OAuth2AccessToken accessToken = scribe.getAccessToken(code);
 
@@ -123,14 +123,34 @@ 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()) {
-      builder.setGroups(getGroups(scribe, accessToken));
+      builder.setGroups(userGroups);
     }
 
     context.authenticate(builder.build());
     context.redirectToRequestedPage();
   }
 
+  private static void validateUserInAllowedGroups(Set<String> userGroups, Set<String> allowedGroups) {
+    boolean allowedUser = userGroups.stream()
+      .anyMatch(userGroup -> isAllowedGroup(userGroup, allowedGroups));
+
+    if (!allowedUser) {
+      throw new UnauthorizedException("You are not allowed to authenticate");
+    }
+  }
+
+  private static boolean isAllowedGroup(String group, Set<String> allowedGroups) {
+    return allowedGroups.stream().anyMatch(group::startsWith);
+  }
+
   private Set<String> getGroups(OAuth20Service scribe, OAuth2AccessToken accessToken) {
     List<GsonGroup> groups = gitLabRestClient.getGroups(scribe, accessToken);
     return Stream.of(groups)
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 49399eb64e7..3371b3188a1 100644
--- 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
@@ -85,7 +85,7 @@ public class GitLabIdentityProviderTest {
 
     gitLabIdentityProvider.init(initContext);
 
-    verify(initContext).redirectTo("http://server/oauth/authorize?response_type=code&client_id=123&redirect_uri=http%3A%2F%2Fserver%2Fcallback&scope=read_user");
+    verify(initContext).redirectTo("http://server/oauth/authorize?response_type=code&client_id=123&redirect_uri=http%3A%2F%2Fserver%2Fcallback&scope=api");
   }
 
   @Test
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 65da50c2d0f..8f933d881ef 100644
--- 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
@@ -21,7 +21,6 @@ package org.sonar.auth.gitlab;
 
 import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.MockWebServer;
-import org.assertj.core.api.Assertions;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -29,13 +28,17 @@ import org.mockito.ArgumentCaptor;
 import org.mockito.Mockito;
 import org.sonar.api.config.internal.MapSettings;
 import org.sonar.api.server.authentication.OAuth2IdentityProvider;
+import org.sonar.api.server.authentication.UnauthorizedException;
 import org.sonar.api.server.authentication.UserIdentity;
 import org.sonar.api.server.http.HttpRequest;
 
 import static java.lang.String.format;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
+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;
@@ -68,23 +71,17 @@ public class IntegrationTest {
       .setProperty(GITLAB_AUTH_ALLOW_USERS_TO_SIGNUP, "true")
       .setProperty(GITLAB_AUTH_URL, gitLabUrl)
       .setProperty(GITLAB_AUTH_APPLICATION_ID, "123")
-      .setProperty(GITLAB_AUTH_SECRET, "456");
+      .setProperty(GITLAB_AUTH_SECRET, "456")
+      .setProperty(GITLAB_AUTH_ALLOWED_GROUPS, "group1,group2");
   }
 
   @Test
-  public void authenticate_user() {
-    OAuth2IdentityProvider.CallbackContext callbackContext = Mockito.mock(OAuth2IdentityProvider.CallbackContext.class);
-    when(callbackContext.getCallbackUrl()).thenReturn("http://server/callback");
-
-    HttpRequest httpRequest = Mockito.mock(HttpRequest.class);
-    when(httpRequest.getParameter("code")).thenReturn(ANY_CODE_VALUE);
-    when(callbackContext.getHttpRequest()).thenReturn(httpRequest);
+  public void callback_whenAllowedUser_shouldAuthenticate() {
+    OAuth2IdentityProvider.CallbackContext callbackContext = mockCallbackContext();
 
-    gitlab.enqueue(new MockResponse().setBody(
-      "{\n" + " \"access_token\": \"de6780bc506a0446309bd9362820ba8aed28aa506c71eedbe1c5c4f9dd350e54\",\n" + " \"token_type\": \"bearer\",\n" + " \"expires_in\": 7200,\n"
-        + " \"refresh_token\": \"8257e65c97202ed1726cf9571600918f3bffb2544b26e00a61df9897668c33a1\"\n" + "}"));
-    // response of /user
-    gitlab.enqueue(new MockResponse().setBody("{\"id\": 123, \"username\":\"toto\", \"name\":\"Toto Toto\",\"email\":\"toto@toto.com\"}"));
+    mockAccessTokenResponse();
+    mockUserResponse();
+    mockSingleGroupReponse("group1");
 
     gitLabIdentityProvider.callback(callbackContext);
 
@@ -99,22 +96,53 @@ public class IntegrationTest {
   }
 
   @Test
-  public void synchronize_groups() throws InterruptedException {
-    mapSettings.setProperty(GITLAB_AUTH_SYNC_USER_GROUPS, "true");
-    OAuth2IdentityProvider.CallbackContext callbackContext = Mockito.mock(OAuth2IdentityProvider.CallbackContext.class);
-    when(callbackContext.getCallbackUrl()).thenReturn("http://server/callback");
+  public void callback_whenNotAllowedUser_shouldThrow() {
+    OAuth2IdentityProvider.CallbackContext callbackContext = mockCallbackContext();
+
+    mockAccessTokenResponse();
+    mockUserResponse();
+    mockSingleGroupReponse("wrong-group");
+
+    assertThatThrownBy(() -> gitLabIdentityProvider.callback(callbackContext))
+      .isInstanceOf((UnauthorizedException.class))
+      .hasMessage("You are not allowed to authenticate");
+  }
+
+  @Test
+  public void callback_whenAllowedUserBySubgroupMembership_shouldAuthenticate() {
+    OAuth2IdentityProvider.CallbackContext callbackContext = mockCallbackContext();
+
+    mockAccessTokenResponse();
+    mockUserResponse();
+    mockSingleGroupReponse("group1/subgroup");
+
+    gitLabIdentityProvider.callback(callbackContext);
+
+    verify(callbackContext).authenticate(any());
+    verify(callbackContext).redirectToRequestedPage();
+  }
 
-    HttpRequest httpRequest = Mockito.mock(HttpRequest.class);
-    when(httpRequest.getParameter("code")).thenReturn(ANY_CODE_VALUE);
-    when(callbackContext.getHttpRequest()).thenReturn(httpRequest);
 
-    gitlab.enqueue(new MockResponse().setBody(
-      "{\n" + " \"access_token\": \"de6780bc506a0446309bd9362820ba8aed28aa506c71eedbe1c5c4f9dd350e54\",\n" + " \"token_type\": \"bearer\",\n" + " \"expires_in\": 7200,\n"
-        + " \"refresh_token\": \"8257e65c97202ed1726cf9571600918f3bffb2544b26e00a61df9897668c33a1\"\n" + "}"));
-    // response of /user
-    gitlab.enqueue(new MockResponse().setBody("{\"id\": 123, \"username\": \"username\", \"name\": \"name\"}"));
-    // response of /groups
-    gitlab.enqueue(new MockResponse().setBody("[{\"full_path\": \"group1\"}, {\"full_path\": \"group2\"}]"));
+  @Test
+  public void callback_shouldSynchronizeGroups() throws InterruptedException {
+    mapSettings.setProperty(GITLAB_AUTH_SYNC_USER_GROUPS, "true");
+    OAuth2IdentityProvider.CallbackContext callbackContext = mockCallbackContext();
+
+    mockAccessTokenResponse();
+    mockUserResponse();
+    // Response for /groups
+    gitlab.enqueue(new MockResponse().setBody("""
+      [
+        {
+          "id": 1,
+          "full_path": "group1"
+        },
+        {
+          "id": 2,
+          "full_path": "group2"
+        }
+      ]
+      """));
 
     gitLabIdentityProvider.callback(callbackContext);
 
@@ -128,35 +156,60 @@ public class IntegrationTest {
   }
 
   @Test
-  public void synchronize_groups_on_many_pages() {
+  public void callback_whenMultiplePagesOfGroups_shouldSynchronizeAllGroups() {
     mapSettings.setProperty(GITLAB_AUTH_SYNC_USER_GROUPS, "true");
-    OAuth2IdentityProvider.CallbackContext callbackContext = Mockito.mock(OAuth2IdentityProvider.CallbackContext.class);
-    when(callbackContext.getCallbackUrl()).thenReturn("http://server/callback");
-
-    HttpRequest httpRequest = Mockito.mock(HttpRequest.class);
-    when(httpRequest.getParameter("code")).thenReturn(ANY_CODE_VALUE);
-    when(callbackContext.getHttpRequest()).thenReturn(httpRequest);
+    OAuth2IdentityProvider.CallbackContext callbackContext = mockCallbackContext();
 
-    gitlab.enqueue(new MockResponse().setBody(
-      "{\n" + " \"access_token\": \"de6780bc506a0446309bd9362820ba8aed28aa506c71eedbe1c5c4f9dd350e54\",\n" + " \"token_type\": \"bearer\",\n" + " \"expires_in\": 7200,\n"
-        + " \"refresh_token\": \"8257e65c97202ed1726cf9571600918f3bffb2544b26e00a61df9897668c33a1\"\n" + "}"));
-    // response of /user
-    gitlab.enqueue(new MockResponse().setBody("{\"id\": 123, \"username\": \"username\", \"name\": \"name\"}"));
-    // response of /groups, first page
+    mockAccessTokenResponse();
+    mockUserResponse();
+    // Response for /groups, first page
     gitlab.enqueue(new MockResponse()
-      .setBody("[{\"full_path\": \"group1\"}, {\"full_path\": \"group2\"}]")
+      .setBody("""
+        [
+          {
+            "id": 1,
+            "full_path": "group1"
+          },
+          {
+            "id": 2,
+            "full_path": "group2"
+          }
+        ]
+        """)
       .setHeader("Link", format(" <%s/groups?per_page=100&page=2>; rel=\"next\"," +
         "  <%s/groups?per_page=100&&page=3>; rel=\"last\"," +
         "  <%s/groups?per_page=100&&page=1>; rel=\"first\"", gitLabUrl, gitLabUrl, gitLabUrl)));
-    // response of /groups, page 2
+    // Response for /groups, page 2
     gitlab.enqueue(new MockResponse()
-      .setBody("[{\"full_path\": \"group3\"}, {\"full_path\": \"group4\"}]")
+      .setBody("""
+        [
+          {
+            "id": 3,
+            "full_path": "group3"
+          },
+          {
+            "id": 4,
+            "full_path": "group4"
+          }
+        ]
+        """)
       .setHeader("Link", format("<%s/groups?per_page=100&page=3>; rel=\"next\"," +
         "  <%s/groups?per_page=100&&page=3>; rel=\"last\"," +
         "  <%s/groups?per_page=100&&page=1>; rel=\"first\"", gitLabUrl, gitLabUrl, gitLabUrl)));
-    // response of /groups, page 3
+    // Response for /groups, page 3
     gitlab.enqueue(new MockResponse()
-      .setBody("[{\"full_path\": \"group5\"}, {\"full_path\": \"group6\"}]")
+      .setBody("""
+        [
+          {
+            "id": 5,
+            "full_path": "group5"
+          },
+          {
+            "id": 6,
+            "full_path": "group6"
+          }
+        ]
+        """)
       .setHeader("Link", format("<%s/groups?per_page=100&&page=3>; rel=\"last\"," +
         "  <%s/groups?per_page=100&&page=1>; rel=\"first\"", gitLabUrl, gitLabUrl)));
 
@@ -169,22 +222,62 @@ public class IntegrationTest {
   }
 
   @Test
-  public void fail_to_authenticate() {
+  public void callback_whenNoUser_shouldThrow() {
+    OAuth2IdentityProvider.CallbackContext callbackContext = mockCallbackContext();
+
+    mockAccessTokenResponse();
+    // Response for /user
+    gitlab.enqueue(new MockResponse().setResponseCode(404).setBody("empty"));
+
+    assertThatThrownBy(() -> gitLabIdentityProvider.callback(callbackContext))
+      .hasMessage("Fail to execute request '" + gitLabSettings.url() + "/api/v4/user'. HTTP code: 404, response: empty")
+      .isInstanceOf((IllegalStateException.class));
+  }
+
+  private static OAuth2IdentityProvider.CallbackContext mockCallbackContext() {
     OAuth2IdentityProvider.CallbackContext callbackContext = Mockito.mock(OAuth2IdentityProvider.CallbackContext.class);
     when(callbackContext.getCallbackUrl()).thenReturn("http://server/callback");
 
     HttpRequest httpRequest = Mockito.mock(HttpRequest.class);
     when(httpRequest.getParameter("code")).thenReturn(ANY_CODE_VALUE);
     when(callbackContext.getHttpRequest()).thenReturn(httpRequest);
+    return callbackContext;
+  }
 
-    gitlab.enqueue(new MockResponse().setBody(
-      "{\n" + " \"access_token\": \"de6780bc506a0446309bd9362820ba8aed28aa506c71eedbe1c5c4f9dd350e54\",\n" + " \"token_type\": \"bearer\",\n" + " \"expires_in\": 7200,\n"
-        + " \"refresh_token\": \"8257e65c97202ed1726cf9571600918f3bffb2544b26e00a61df9897668c33a1\"\n" + "}"));
-    gitlab.enqueue(new MockResponse().setResponseCode(404).setBody("empty"));
+  private void mockAccessTokenResponse() {
+    // Response for OAuth access token
+    gitlab.enqueue(new MockResponse().setBody("""
+      {
+        "access_token": "de6780bc506a0446309bd9362820ba8aed28aa506c71eedbe1c5c4f9dd350e54",
+        "token_type": "bearer",
+        "expires_in": 7200,
+        "refresh_token": "8257e65c97202ed1726cf9571600918f3bffb2544b26e00a61df9897668c33a1"
+      }
+      """));
+  }
 
-    Assertions.assertThatThrownBy(() -> gitLabIdentityProvider.callback(callbackContext))
-      .hasMessage("Fail to execute request '" + gitLabSettings.url() + "/api/v4/user'. HTTP code: 404, response: empty")
-      .isInstanceOf((IllegalStateException.class));
+  private void mockUserResponse() {
+    // Response for /user
+    gitlab.enqueue(new MockResponse().setBody("""
+      {
+        "id": 123,
+        "username": "toto",
+        "name": "Toto Toto",
+        "email": "toto@toto.com"
+      }
+      """));
+  }
+
+  private void mockSingleGroupReponse(String group) {
+    // Response for /groups
+    gitlab.enqueue(new MockResponse().setBody("""
+      [
+        {
+          "id": 1,
+          "full_path": "%s"
+        }
+      ]
+      """.formatted(group)));
   }
 
 }