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().pull/546/head
parent
d96e67252a
commit
f5a70d0f9a
|
|
@ -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<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
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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<Pair<Boolean, String>> result = fetcher.isValid(personalAccessTokenParams);
|
||||
// then
|
||||
assertFalse(result.isEmpty());
|
||||
assertTrue(result.get().first);
|
||||
assertEquals(result.get().second, bitbucketUser.getName());
|
||||
}
|
||||
|
||||
@DataProvider
|
||||
|
|
|
|||
Loading…
Reference in New Issue