From 2c93d7e5ed4bdafa8054cbb7a139d8dfc149365b Mon Sep 17 00:00:00 2001 From: Pierre Date: Wed, 15 Sep 2021 12:10:05 +0200 Subject: [PATCH] SONAR-15171 filter groups sync on min_access_level 10 --- .../java/org/sonar/auth/OAuthRestClient.java | 7 +++++- .../org/sonar/auth/OAuthRestClientTest.java | 17 ++++++++++++++ .../sonar/auth/gitlab/GitLabRestClient.java | 2 +- .../gitlab/GitLabIdentityProviderTest.java | 11 ++++----- .../sonar/auth/gitlab/IntegrationTest.java | 23 +++++++++---------- 5 files changed, 39 insertions(+), 21 deletions(-) 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 c21795d5aef..95e0f9c4f1e 100644 --- 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 List executePaginatedRequest(String request, OAuth20Service scribe, OAuth2AccessToken accessToken, Function> function) { List 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 void readPage(List result, OAuth20Service scribe, OAuth2AccessToken accessToken, String endPoint, Function> 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 3f29fd2597d..02730651fc5 100644 --- 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 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 22334164639..5c19193a4ab 100644 --- 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 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 778de10eef2..5d2e086a03c 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 @@ -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 d5e656106c3..7d84c561d3b 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 @@ -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)); } } -- 2.39.5