From 128e7e4204f5f0e52e7e9b9c3da7bf6c59c41979 Mon Sep 17 00:00:00 2001 From: Igor Vinokur Date: Thu, 17 Aug 2023 16:29:24 +0300 Subject: [PATCH] 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(). --- .../che/security/oauth/EmbeddedOAuthAPI.java | 22 +------------------ ...ucketServerPersonalAccessTokenFetcher.java | 7 ++++++ ...tServerPersonalAccessTokenFetcherTest.java | 17 ++++++++++++++ 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPI.java b/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPI.java index 352d7bbc20..35e0b4a874 100644 --- a/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth/EmbeddedOAuthAPI.java +++ b/wsmaster/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.Link; 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 class EmbeddedOAuthAPI implements OAuthAPI, OAuthTokenFetcher { if (token != null) { return token; } - Optional 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/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcher.java b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcher.java index 5d22669cca..b22878ee63 100644 --- a/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcher.java +++ b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/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 class BitbucketServerPersonalAccessTokenFetcher implements PersonalAccess } } 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/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcherTest.java b/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcherTest.java index bff6ca6f30..acdfa4d90c 100644 --- a/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcherTest.java +++ b/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcherTest.java @@ -231,6 +231,23 @@ public class BitbucketServerPersonalAccessTokenFetcherTest { // 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> result = fetcher.isValid(personalAccessTokenParams); + // then + assertFalse(result.isEmpty()); + assertTrue(result.get().first); + assertEquals(result.get().second, bitbucketUser.getName()); } @DataProvider