fix: update the way how Azure OAuth2 token is validated (#462)

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
eclipse-che · Mar 10, 2023 · 1001483 · 1001483
1 parent f81d126
commit 1001483
Show file tree

Hide file tree

Showing 6 changed files with 184 additions and 168 deletions.
diff --git a/...s/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsApiClient.java b/...s/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsApiClient.java
@@ -126,16 +126,6 @@ private AzureDevOpsUser getUser(String url, String authorizationHeader)
         });
   }
 
-  /**
-   * Returns the scopes of the OAuth token. Consider using the REST API:
-   *
-   * <p>https://learn.microsoft.com/en-us/rest/api/azure/devops/tokens/pats/get?view=azure-devops-rest-7.0&tabs=HTTP
-   */
-  public String[] getTokenScopes(String authenticationToken)
-      throws ScmItemNotFoundException, ScmCommunicationException, ScmBadRequestException {
-    return scopes;
-  }
-
   private <T> T executeRequest(
       HttpClient httpClient,
       HttpRequest request,

diff --git a/...rg/eclipse/che/api/factory/server/azure/devops/AzureDevOpsPersonalAccessTokenFetcher.java b/...rg/eclipse/che/api/factory/server/azure/devops/AzureDevOpsPersonalAccessTokenFetcher.java
@@ -14,7 +14,6 @@
 import static org.eclipse.che.api.factory.server.azure.devops.AzureDevOps.getAuthenticateUrlPath;
 import static org.eclipse.che.commons.lang.StringUtils.trimEnd;
 
-import com.google.common.collect.Sets;
 import java.util.Arrays;
 import java.util.Optional;
 import javax.inject.Inject;
@@ -136,20 +135,16 @@ public Optional<Boolean> isValid(PersonalAccessToken personalAccessToken) {
     }
 
     try {
+      AzureDevOpsUser user;
       if (personalAccessToken.getScmTokenName() != null
           && personalAccessToken.getScmTokenName().startsWith(OAUTH_2_PREFIX)) {
-        String[] scopes = azureDevOpsApiClient.getTokenScopes(personalAccessToken.getToken());
-        return Optional.of(Sets.newHashSet(scopes).containsAll(Sets.newHashSet(this.scopes)));
+        user = azureDevOpsApiClient.getUserWithOAuthToken(personalAccessToken.getToken());
       } else {
-        AzureDevOpsUser user =
+        user =
             azureDevOpsApiClient.getUserWithPAT(
                 personalAccessToken.getToken(), personalAccessToken.getScmOrganization());
-        if (personalAccessToken.getScmUserId().equals(user.getId())) {
-          return Optional.of(Boolean.TRUE);
-        } else {
-          return Optional.of(Boolean.FALSE);
-        }
       }
+      return Optional.of(personalAccessToken.getScmUserId().equals(user.getId()));
     } catch (ScmItemNotFoundException | ScmCommunicationException | ScmBadRequestException e) {
       return Optional.of(Boolean.FALSE);
     }

diff --git a/...n/test/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsURLParserTest.java b/...n/test/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsURLParserTest.java
diff --git a/...DevOpsPersonalAccessTokenFetcherTest.java → ...DevOpsPersonalAccessTokenFetcherTest.java b/...DevOpsPersonalAccessTokenFetcherTest.java → ...DevOpsPersonalAccessTokenFetcherTest.java
@@ -11,55 +11,44 @@
  */
 package org.eclipse.che.api.factory.server.azure.devops;
 
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.testng.Assert.*;
+
 import org.eclipse.che.api.auth.shared.dto.OAuthToken;
 import org.eclipse.che.api.factory.server.scm.PersonalAccessToken;
-import org.eclipse.che.api.factory.server.urlfactory.DevfileFilenamesProvider;
-import org.eclipse.che.api.factory.server.urlfactory.URLFactoryBuilder;
 import org.eclipse.che.commons.subject.Subject;
 import org.eclipse.che.security.oauth.OAuthAPI;
 import org.mockito.Mock;
 import org.mockito.testng.MockitoTestNGListener;
 import org.testng.annotations.BeforeMethod;
-import org.testng.annotations.DataProvider;
 import org.testng.annotations.Listeners;
 import org.testng.annotations.Test;
 
-import java.util.Optional;
-
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-import static org.testng.Assert.*;
-
-/**
- * @author Anatalii Bazko
- */
+/** @author Anatalii Bazko */
 @Listeners(MockitoTestNGListener.class)
 public class AzureDevOpsPersonalAccessTokenFetcherTest {
 
-  @Mock
-  private AzureDevOpsApiClient azureDevOpsApiClient;
-  @Mock
-  private OAuthAPI oAuthAPI;
-  @Mock
-  private OAuthToken oAuthToken;
-  @Mock
-  private AzureDevOpsUser azureDevOpsUser;
+  @Mock private AzureDevOpsApiClient azureDevOpsApiClient;
+  @Mock private OAuthAPI oAuthAPI;
+  @Mock private OAuthToken oAuthToken;
+  @Mock private AzureDevOpsUser azureDevOpsUser;
   private AzureDevOpsPersonalAccessTokenFetcher personalAccessTokenFetcher;
 
   @BeforeMethod
   protected void start() {
-    personalAccessTokenFetcher = new AzureDevOpsPersonalAccessTokenFetcher(
-            "localhost",
-            "https://dev.azure.com",
-            new String[]{},
-            azureDevOpsApiClient,
-            oAuthAPI);
+    personalAccessTokenFetcher =
+        new AzureDevOpsPersonalAccessTokenFetcher(
+            "localhost", "https://dev.azure.com", new String[] {}, azureDevOpsApiClient, oAuthAPI);
   }
 
   @Test
-  public void fetchPersonalAccessTokenShouldReturnNullIfScmServerUrlIsNotAzureDevOps() throws Exception {
-    PersonalAccessToken personalAccessToken = personalAccessTokenFetcher.fetchPersonalAccessToken(mock(Subject.class), "https://eclipse.org");
+  public void fetchPersonalAccessTokenShouldReturnNullIfScmServerUrlIsNotAzureDevOps()
+      throws Exception {
+    PersonalAccessToken personalAccessToken =
+        personalAccessTokenFetcher.fetchPersonalAccessToken(
+            mock(Subject.class), "https://eclipse.org");
 
     assertNull(personalAccessToken);
   }
@@ -68,9 +57,11 @@ public void fetchPersonalAccessTokenShouldReturnNullIfScmServerUrlIsNotAzureDevO
   public void fetchPersonalAccessTokenShouldReturnToken() throws Exception {
     when(oAuthAPI.getToken(AzureDevOps.PROVIDER_NAME)).thenReturn(oAuthToken);
     when(azureDevOpsApiClient.getUserWithOAuthToken(any())).thenReturn(azureDevOpsUser);
-    when(azureDevOpsApiClient.getTokenScopes(any())).thenReturn(new String[]{"vso.code_full"});
+    when(azureDevOpsUser.getId()).thenReturn("user-id");
 
-    PersonalAccessToken personalAccessToken = personalAccessTokenFetcher.fetchPersonalAccessToken(mock(Subject.class), "https://dev.azure.com/");
+    PersonalAccessToken personalAccessToken =
+        personalAccessTokenFetcher.fetchPersonalAccessToken(
+            mock(Subject.class), "https://dev.azure.com/");
 
     assertNotNull(personalAccessToken);
   }