SONAR-15171 filter groups sync on min_access_level 10

diff --git a/server/sonar-auth-common/src/main/java/org/sonar/auth/OAuthRestClient.java b/server/sonar-auth-common/src/main/java/org/sonar/auth/OAuthRestClient.java
index c21795d5aefc834e7d42171e7d5b86e676633317..95e0f9c4f1ed85fc3d7d42641b5c704cee871b53 100644 (file)
--- a/server/sonar-auth-common/src/main/java/org/sonar/auth/OAuthRestClient.java
+++ b/server/sonar-auth-common/src/main/java/org/sonar/auth/OAuthRestClient.java
@@ -64,10 +64,15 @@ public class OAuthRestClient {
 
   public static <E> List<E> executePaginatedRequest(String request, OAuth20Service scribe, OAuth2AccessToken accessToken, Function<String, List<E>> function) {
     List<E> result = new ArrayList<>();
-    readPage(result, scribe, accessToken, request + "?per_page=" + DEFAULT_PAGE_SIZE, function);
+    readPage(result, scribe, accessToken, addPerPageQueryParameter(request, DEFAULT_PAGE_SIZE), function);
     return result;
   }
 
+  public static String addPerPageQueryParameter(String request, int pageSize) {
+    String separator = request.contains("?") ? "&" : "?";
+    return request + separator + "per_page=" + pageSize;
+  }
+
   private static <E> void readPage(List<E> result, OAuth20Service scribe, OAuth2AccessToken accessToken, String endPoint, Function<String, List<E>> function) {
     try (Response nextResponse = executeRequest(endPoint, scribe, accessToken)) {
       String content = nextResponse.getBody();

diff --git a/server/sonar-auth-common/src/test/java/org/sonar/auth/OAuthRestClientTest.java b/server/sonar-auth-common/src/test/java/org/sonar/auth/OAuthRestClientTest.java
index 3f29fd2597d9106d9b585953edf3e1ac926df074..02730651fc5c461308b90515ce2f2009b2651141 100644 (file)
--- a/server/sonar-auth-common/src/test/java/org/sonar/auth/OAuthRestClientTest.java
+++ b/server/sonar-auth-common/src/test/java/org/sonar/auth/OAuthRestClientTest.java
@@ -97,6 +97,23 @@ public class OAuthRestClientTest {
     assertThat(response).contains("A", "B");
   }
 
+  @Test
+  public void execute_paginated_request_with_query_parameter() throws InterruptedException {
+    mockWebServer.enqueue(new MockResponse()
+      .setHeader("Link", "<" + serverUrl + "/test?param=value&per_page=100&page=2>; rel=\"next\", <" + serverUrl + "/test?param=value&per_page=100&page=2>; rel=\"last\"")
+      .setBody("A"));
+    mockWebServer.enqueue(new MockResponse()
+      .setHeader("Link", "<" + serverUrl + "/test?param=value&per_page=100&page=1>; rel=\"prev\", <" + serverUrl + "/test?param=value&per_page=100&page=1>; rel=\"first\"")
+      .setBody("B"));
+
+    List<String> response = executePaginatedRequest(serverUrl + "/test?param=value", oAuth20Service, auth2AccessToken, Arrays::asList);
+
+    assertThat(response).contains("A", "B");
+
+    assertThat(mockWebServer.takeRequest().getPath()).isEqualTo("/test?param=value&per_page=100");
+    assertThat(mockWebServer.takeRequest().getPath()).isEqualTo("/test?param=value&per_page=100&page=2");
+  }
+
   @Test
   public void execute_paginated_request_case_insensitive_headers() {
     mockWebServer.enqueue(new MockResponse()

diff --git a/server/sonar-auth-gitlab/src/main/java/org/sonar/auth/gitlab/GitLabRestClient.java b/server/sonar-auth-gitlab/src/main/java/org/sonar/auth/gitlab/GitLabRestClient.java
index 22334164639cf272d9274854a5675d13ed117162..5c19193a4ab15c9791fcd95c678b2ad1ffea9c1c 100644 (file)
--- a/server/sonar-auth-gitlab/src/main/java/org/sonar/auth/gitlab/GitLabRestClient.java
+++ b/server/sonar-auth-gitlab/src/main/java/org/sonar/auth/gitlab/GitLabRestClient.java
@@ -46,6 +46,6 @@ public class GitLabRestClient {
   }
 
   List<GsonGroup> getGroups(OAuth20Service scribe, OAuth2AccessToken accessToken) {
-    return OAuthRestClient.executePaginatedRequest(settings.url() + API_SUFFIX + "/groups", scribe, accessToken, GsonGroup::parse);
+    return OAuthRestClient.executePaginatedRequest(settings.url() + API_SUFFIX + "/groups?min_access_level=10", scribe, accessToken, GsonGroup::parse);
   }
 }

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 778de10eef299afdbc924aaf6dc0ebb1f52f4dff..5d2e086a03c716bf40ab9c6c5988bfbe31975174 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,6 +19,7 @@
  */
 package org.sonar.auth.gitlab;
 
+import org.assertj.core.api.Assertions;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -32,9 +33,6 @@ import static org.mockito.Mockito.when;
 
 public class GitLabIdentityProviderTest {
 
-  @Rule
-  public ExpectedException expectedException = ExpectedException.none();
-
   @Test
   public void test_identity_provider() {
     GitLabSettings gitLabSettings = mock(GitLabSettings.class);
@@ -106,9 +104,8 @@ public class GitLabIdentityProviderTest {
     OAuth2IdentityProvider.InitContext initContext = mock(OAuth2IdentityProvider.InitContext.class);
     when(initContext.getCallbackUrl()).thenReturn("http://server/callback");
 
-    expectedException.expect(IllegalStateException.class);
-    expectedException.expectMessage("GitLab authentication is disabled");
-
-    gitLabIdentityProvider.init(initContext);
+    Assertions.assertThatThrownBy(() -> gitLabIdentityProvider.init(initContext))
+      .hasMessage("GitLab authentication is disabled")
+      .isInstanceOf(IllegalStateException.class);
   }
 }

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 d5e656106c30aeaf4c7eb2f0e1003d6b6b2d499e..7d84c561d3b3344eb40258fae05b6b78b274f656 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
@@ -22,10 +22,10 @@ package org.sonar.auth.gitlab;
 import javax.servlet.http.HttpServletRequest;
 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;
-import org.junit.rules.ExpectedException;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mockito;
 import org.sonar.api.config.internal.MapSettings;
@@ -47,19 +47,16 @@ public class IntegrationTest {
 
   private static final String ANY_CODE_VALUE = "ANY_CODE";
 
-  @Rule
-  public ExpectedException expectedException = ExpectedException.none();
-
   @Rule
   public MockWebServer gitlab = new MockWebServer();
 
-  private MapSettings mapSettings = new MapSettings();
+  private final MapSettings mapSettings = new MapSettings();
 
-  private GitLabSettings gitLabSettings = new GitLabSettings(mapSettings.asConfig());
+  private final GitLabSettings gitLabSettings = new GitLabSettings(mapSettings.asConfig());
 
   private String gitLabUrl;
 
-  private GitLabIdentityProvider gitLabIdentityProvider = new GitLabIdentityProvider(gitLabSettings,
+  private final GitLabIdentityProvider gitLabIdentityProvider = new GitLabIdentityProvider(gitLabSettings,
     new GitLabRestClient(gitLabSettings),
     new ScribeGitLabOauth2Api(gitLabSettings));
 
@@ -102,7 +99,7 @@ public class IntegrationTest {
   }
 
   @Test
-  public void synchronize_groups() {
+  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");
@@ -125,6 +122,9 @@ public class IntegrationTest {
     verify(callbackContext).authenticate(captor.capture());
     UserIdentity value = captor.getValue();
     assertThat(value.getGroups()).contains("group1", "group2");
+    assertThat(gitlab.takeRequest().getPath()).isEqualTo("/oauth/token");
+    assertThat(gitlab.takeRequest().getPath()).isEqualTo("/api/v4/user");
+    assertThat(gitlab.takeRequest().getPath()).isEqualTo("/api/v4/groups?min_access_level=10&per_page=100");
   }
 
   @Test
@@ -182,10 +182,9 @@ public class IntegrationTest {
         + " \"refresh_token\": \"8257e65c97202ed1726cf9571600918f3bffb2544b26e00a61df9897668c33a1\"\n" + "}"));
     gitlab.enqueue(new MockResponse().setResponseCode(404).setBody("empty"));
 
-    expectedException.expect(IllegalStateException.class);
-    expectedException.expectMessage("Fail to execute request '" + gitLabSettings.url() + "/api/v4/user'. HTTP code: 404, response: empty");
-
-    gitLabIdentityProvider.callback(callbackContext);
+    Assertions.assertThatThrownBy(() -> gitLabIdentityProvider.callback(callbackContext))
+      .hasMessage("Fail to execute request '" + gitLabSettings.url() + "/api/v4/user'. HTTP code: 404, response: empty")
+      .isInstanceOf((IllegalStateException.class));
   }
 
 }