Do not return secret token in the Oauth API (#538)

Remove the personalAccessTokenManager.get() call from the OAuth API getToken() method. The OAuth API must not know anything about PAT secrets. It should get tokens only by requesting an SCM provider OAuth API. Fix validating the Bitbucket-Server PAT method by requesting user instead of requesting. This prevents the code execution going to a recursive loop: bitbucketServerApiClient.getPersonalAccessToken() calls oauthApi.getToken() which referred to personalAccessTokenManager.getToken() which validated the token by calling scmPersonalAccessTokenFetcher.getScmUsername() -> bitbucketServerApiClient.getPersonalAccessToken().
eclipse-che · Aug 17, 2023 · f5a70d0 · f5a70d0
1 parent d96e672
commit f5a70d0
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 21 deletions.
diff --git a/...ster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPI.java b/...ster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPI.java
@@ -38,11 +38,6 @@
 import org.eclipse.che.api.core.rest.shared.dto.LinkParameter;
 import org.eclipse.che.api.core.util.LinksHelper;
 import org.eclipse.che.api.factory.server.scm.OAuthTokenFetcher;
-import org.eclipse.che.api.factory.server.scm.PersonalAccessToken;
-import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager;
-import org.eclipse.che.api.factory.server.scm.exception.ScmCommunicationException;
-import org.eclipse.che.api.factory.server.scm.exception.ScmConfigurationPersistenceException;
-import org.eclipse.che.api.factory.server.scm.exception.ScmUnauthorizedException;
 import org.eclipse.che.commons.env.EnvironmentContext;
 import org.eclipse.che.commons.subject.Subject;
 import org.eclipse.che.security.oauth.shared.dto.OAuthAuthenticatorDescriptor;
@@ -64,7 +59,6 @@ public class EmbeddedOAuthAPI implements OAuthAPI, OAuthTokenFetcher {
   protected String errorPage;
 
   @Inject protected OAuthAuthenticatorProvider providers;
-  @Inject protected PersonalAccessTokenManager personalAccessTokenManager;
   private String redirectAfterLogin;
 
   @Override
@@ -159,25 +153,11 @@ public OAuthToken getToken(String oauthProvider)
       if (token != null) {
         return token;
       }
-      Optional<PersonalAccessToken> tokenOptional =
-          personalAccessTokenManager.get(subject, provider.getEndpointUrl());
-      if (tokenOptional.isPresent()) {
-        PersonalAccessToken accessToken = tokenOptional.get();
-        return newDto(OAuthToken.class).withToken(accessToken.getToken());
-      }
       throw new UnauthorizedException(
           "OAuth token for user " + subject.getUserId() + " was not found");
-    } catch (IOException | ScmConfigurationPersistenceException | ScmCommunicationException e) {
+    } catch (IOException e) {
       throw new ServerException(e.getLocalizedMessage(), e);
-    } catch (ScmUnauthorizedException e) {
-      throwUnauthorizedException(subject);
     }
-    return null;
-  }
-
-  private void throwUnauthorizedException(Subject subject) throws UnauthorizedException {
-    throw new UnauthorizedException(
-        "OAuth token for user " + subject.getUserId() + " was not found");
   }
 
   @Override

diff --git a/...g/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcher.java b/...g/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcher.java
@@ -11,6 +11,7 @@
  */
 package org.eclipse.che.api.factory.server.bitbucket;
 
+import static com.google.common.base.Strings.isNullOrEmpty;
 import static java.lang.String.format;
 import static java.lang.String.valueOf;
 
@@ -159,6 +160,12 @@ public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
       }
     }
     try {
+      // Token is added manually by a user without token id. Validate only by requesting user info.
+      if (isNullOrEmpty(params.getScmTokenId())) {
+        BitbucketUser user = bitbucketServerApiClient.getUser(params.getToken());
+        return Optional.of(Pair.of(Boolean.TRUE, user.getName()));
+      }
+      // Token is added by OAuth. Token id is available.
       BitbucketPersonalAccessToken bitbucketPersonalAccessToken =
           bitbucketServerApiClient.getPersonalAccessToken(Long.valueOf(params.getScmTokenId()));
       return Optional.of(

diff --git a/...lipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcherTest.java b/...lipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcherTest.java
@@ -231,6 +231,23 @@ public void shouldBeAbleToValidateToken()
     // then
     assertFalse(result.isEmpty());
     assertTrue(result.get().first);
+    assertEquals(result.get().second, bitbucketUser.getName());
+  }
+
+  @Test
+  public void shouldValidateTokenWithoutId()
+      throws ScmUnauthorizedException, ScmCommunicationException, ScmItemNotFoundException {
+    // given
+    when(personalAccessTokenParams.getScmProviderUrl()).thenReturn(someBitbucketURL);
+    when(personalAccessTokenParams.getToken()).thenReturn("token");
+    when(bitbucketServerApiClient.isConnected(eq(someBitbucketURL))).thenReturn(true);
+    when(bitbucketServerApiClient.getUser(eq("token"))).thenReturn(bitbucketUser);
+    // when
+    Optional<Pair<Boolean, String>> result = fetcher.isValid(personalAccessTokenParams);
+    // then
+    assertFalse(result.isEmpty());
+    assertTrue(result.get().first);
+    assertEquals(result.get().second, bitbucketUser.getName());
   }
 
   @DataProvider