SONAR-23078 Fix SSF-644

diff --git a/server/sonar-auth-gitlab/build.gradle b/server/sonar-auth-gitlab/build.gradle
index 09f78a36d8be358d7196f0bf39061f490a42e3a3..bc6749ecba6b62070f9ac93f1de4c3882bec89e4 100644 (file)
--- a/server/sonar-auth-gitlab/build.gradle
+++ b/server/sonar-auth-gitlab/build.gradle
@@ -20,8 +20,19 @@ dependencies {
 
     testImplementation 'com.squareup.okhttp3:mockwebserver'
     testImplementation 'com.squareup.okhttp3:okhttp'
-    testImplementation 'junit:junit'
     testImplementation 'com.tngtech.java:junit-dataprovider'
+    testImplementation 'junit:junit'
     testImplementation 'org.assertj:assertj-core'
+    testImplementation 'org.junit.jupiter:junit-jupiter-api'
+    testImplementation 'org.junit.jupiter:junit-jupiter-params'
     testImplementation 'org.mockito:mockito-core'
+    testImplementation 'org.mockito:mockito-junit-jupiter'
+
+    testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine'
+    testRuntimeOnly 'org.junit.vintage:junit-vintage-engine'
+}
+
+test {
+    // Enabling the JUnit Platform (see https://github.com/junit-team/junit5-samples/tree/master/junit5-migration-gradle)
+    useJUnitPlatform()
 }

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 4478d48b30fde7d6be0ada30bc5e552d2c52c233..93fbfbe504c3dae9d8493276c64a279e688975c5 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
@@ -148,7 +148,11 @@ public class GitLabIdentityProvider implements OAuth2IdentityProvider {
   }
 
   private static boolean isAllowedGroup(String group, Set<String> allowedGroups) {
-    return allowedGroups.stream().anyMatch(group::startsWith);
+    return allowedGroups.stream().anyMatch(allowedGroup -> isExactGroupOrParentGroup(group, allowedGroup));
+  }
+
+  private static boolean isExactGroupOrParentGroup(String group, String allowedGroup) {
+    return group.equals(allowedGroup) || group.startsWith(allowedGroup + "/");
   }
 
   private Set<String> getGroups(OAuth20Service scribe, OAuth2AccessToken accessToken) {

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 873d8dc89954814dcc1f228fc3abf5749364510b..bbfce499e7aeb4ee300cb07224bf2fa9eeb2589b 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
@@ -22,19 +22,21 @@ package org.sonar.auth.gitlab;
 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 java.util.stream.Stream;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.mockito.Answers;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
 import org.sonar.api.server.authentication.Display;
 import org.sonar.api.server.authentication.OAuth2IdentityProvider;
 import org.sonar.api.server.authentication.UnauthorizedException;
@@ -42,17 +44,17 @@ 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.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.lenient;
 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 {
+@ExtendWith(MockitoExtension.class)
+class GitLabIdentityProviderTest {
 
   private static final String OAUTH_CODE = "code fdsojfsjodfg";
   private static final String AUTHORIZATION_URL = "AUTHORIZATION_URL";
@@ -78,24 +80,23 @@ public class GitLabIdentityProviderTest {
 
   private GitLabIdentityProvider gitLabIdentityProvider;
 
-  @Before
-  public void setup() throws IOException, ExecutionException, InterruptedException {
-    openMocks(this);
+  @BeforeEach
+  void setup() throws IOException, ExecutionException, InterruptedException {
     gitLabIdentityProvider = new GitLabIdentityProvider(gitLabSettings, gitLabRestClient, scribeApi, scribeFactory);
 
-    when(initContext.generateCsrfState()).thenReturn(STATE);
-    when(initContext.getCallbackUrl()).thenReturn(CALLBACK_URL);
+    lenient().when(initContext.generateCsrfState()).thenReturn(STATE);
+    lenient().when(initContext.getCallbackUrl()).thenReturn(CALLBACK_URL);
 
-    when(callbackContext.getCallbackUrl()).thenReturn(CALLBACK_URL);
-    when(callbackContext.getHttpRequest().getParameter(OAuthConstants.CODE)).thenReturn(OAUTH_CODE);
+    lenient().when(callbackContext.getCallbackUrl()).thenReturn(CALLBACK_URL);
+    lenient().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);
+    lenient().when(scribeFactory.newScribe(gitLabSettings, CALLBACK_URL, scribeApi)).thenReturn(scribe);
+    lenient().when(scribe.getAccessToken(OAUTH_CODE)).thenReturn(accessToken);
+    lenient().when(scribe.getAuthorizationUrl(STATE)).thenReturn(AUTHORIZATION_URL);
   }
 
   @Test
-  public void test_identity_provider() {
+  void test_identity_provider() {
     when(gitLabSettings.isEnabled()).thenReturn(true);
     when(gitLabSettings.allowUsersToSignUp()).thenReturn(true);
 
@@ -109,7 +110,7 @@ public class GitLabIdentityProviderTest {
   }
 
   @Test
-  public void init_whenSuccessful_redirectsToUrl() {
+  void init_whenSuccessful_redirectsToUrl() {
     gitLabIdentityProvider.init(initContext);
 
     verify(initContext).generateCsrfState();
@@ -117,7 +118,7 @@ public class GitLabIdentityProviderTest {
   }
 
   @Test
-  public void init_whenErrorWhileBuildingScribe_shouldReThrow() {
+  void init_whenErrorWhileBuildingScribe_shouldReThrow() {
     IllegalStateException exception = new IllegalStateException("GitLab authentication is disabled");
     when(scribeFactory.newScribe(any(), any(), any())).thenThrow(exception);
 
@@ -129,7 +130,7 @@ public class GitLabIdentityProviderTest {
   }
 
   @Test
-  public void onCallback_withGroupSyncDisabledAndNoAllowedGroups_redirectsToRequestedPage() {
+  void onCallback_withGroupSyncDisabledAndNoAllowedGroups_redirectsToRequestedPage() {
     GsonUser gsonUser = mockGsonUser();
 
     gitLabIdentityProvider.callback(callbackContext);
@@ -140,7 +141,7 @@ public class GitLabIdentityProviderTest {
   }
 
   @Test
-  public void onCallback_withGroupSyncDisabledAndAllowedGroups_redirectsToRequestedPage() {
+  void onCallback_withGroupSyncDisabledAndAllowedGroups_redirectsToRequestedPage() {
     when(gitLabSettings.syncUserGroups()).thenReturn(false);
 
     GsonUser gsonUser = mockGsonUser();
@@ -152,9 +153,9 @@ public class GitLabIdentityProviderTest {
     verify(gitLabRestClient, never()).getGroups(any(), any());
   }
 
-  @Test
-  @UseDataProvider("allowedGroups")
-  public void onCallback_withGroupSyncAndAllowedGroupsMatching_redirectsToRequestedPage(Set<String> allowedGroups) {
+  @ParameterizedTest
+  @MethodSource("allowedGroups")
+  void onCallback_withGroupSyncAndAllowedGroupsMatching_redirectsToRequestedPage(Set<String> allowedGroups) {
     when(gitLabSettings.syncUserGroups()).thenReturn(true);
     when(gitLabSettings.allowedGroups()).thenReturn(allowedGroups);
 
@@ -167,18 +168,19 @@ public class GitLabIdentityProviderTest {
     verify(callbackContext).redirectToRequestedPage();
   }
 
-  @DataProvider
-  public static Object[][] allowedGroups() {
-    return new Object[][]{
-      {Set.of()},
-      {Set.of("path")}
-    };
+  static Stream<Arguments> allowedGroups() {
+    return Stream.of(
+      Arguments.of(Set.of()),
+      Arguments.of(Set.of("path")),
+      Arguments.of(Set.of("path/to/group"))
+    );
   }
 
-  @Test
-  public void onCallback_withGroupSyncAndAllowedGroupsNotMatching_shouldThrow() {
+  @ParameterizedTest
+  @MethodSource("notAllowedGroups")
+  void onCallback_withGroupSyncAndAllowedGroupsNotMatching_shouldThrow(Set<String> allowedGroups) {
     when(gitLabSettings.syncUserGroups()).thenReturn(true);
-    when(gitLabSettings.allowedGroups()).thenReturn(Set.of("path2"));
+    when(gitLabSettings.allowedGroups()).thenReturn(allowedGroups);
 
     mockGsonUser();
     mockGitlabGroups();
@@ -188,8 +190,15 @@ public class GitLabIdentityProviderTest {
       .withMessage("You are not allowed to authenticate");
   }
 
+  static Stream<Arguments> notAllowedGroups() {
+    return Stream.of(
+      Arguments.of(Set.of("pat")),
+      Arguments.of(Set.of("path2"))
+    );
+  }
+
   @Test
-  public void onCallback_ifScribeFactoryFails_shouldThrow() {
+  void onCallback_ifScribeFactoryFails_shouldThrow() {
     IllegalStateException exception = new IllegalStateException("message");
     when(scribeFactory.newScribe(any(), any(), any())).thenThrow(exception);
 
@@ -232,7 +241,7 @@ public class GitLabIdentityProviderTest {
   }
 
   @Test
-  public void newScribe_whenGitLabAuthIsDisabled_throws() {
+  void newScribe_whenGitLabAuthIsDisabled_throws() {
     when(gitLabSettings.isEnabled()).thenReturn(false);
 
     assertThatIllegalStateException()
@@ -240,9 +249,9 @@ public class GitLabIdentityProviderTest {
       .withMessage("GitLab authentication is disabled");
   }
 
-  @Test
-  @UseDataProvider("groupsSyncToScope")
-  public void newScribe_whenGitLabSettingsValid_shouldUseCorrectScopeDependingOnGroupSync(boolean groupSyncEnabled, String expectedScope) {
+  @ParameterizedTest
+  @MethodSource("groupsSyncToScope")
+  void newScribe_whenGitLabSettingsValid_shouldUseCorrectScopeDependingOnGroupSync(boolean groupSyncEnabled, String expectedScope) {
     setupGitlabSettingsWithGroupSync(groupSyncEnabled);
 
 
@@ -255,12 +264,11 @@ public class GitLabIdentityProviderTest {
     assertThat(realScribe.getDefaultScope()).isEqualTo(expectedScope);
   }
 
-  @DataProvider
-  public static Object[][] groupsSyncToScope() {
-    return new Object[][]{
-      {false, "read_user"},
-      {true, "api"}
-    };
+  static Stream<Arguments> groupsSyncToScope() {
+    return Stream.of(
+      Arguments.of(false, "read_user"),
+      Arguments.of(true, "api")
+    );
   }
 
   private void setupGitlabSettingsWithGroupSync(boolean enableGroupSync) {