Fix update token on workspace start (#597)

Change the getHostName() function to getProviderUrl() in order to fix an error while updating an oauth token on workspace start.
Throw ScmUnauthorizedException if an oAuth token is not valid, for the dashboard to open the authorisation page and update the token.
7.74.x
Igor Vinokur 2023-11-04 19:12:12 +02:00 committed by ivinokur
parent 5b15aceef9
commit f156246fa8
12 changed files with 177 additions and 58 deletions

View File

@ -156,6 +156,7 @@
<org.reflections.version>0.9.9</org.reflections.version>
<org.slf4j.version>1.7.36</org.slf4j.version>
<org.testng.version>7.4.0</org.testng.version>
<org.wiremock.version>3.0.1</org.wiremock.version>
</properties>
<dependencyManagement>
<dependencies>
@ -1409,6 +1410,11 @@
<artifactId>testng</artifactId>
<version>${org.testng.version}</version>
</dependency>
<dependency>
<groupId>org.wiremock</groupId>
<artifactId>wiremock-standalone</artifactId>
<version>${org.wiremock.version}</version>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson</groupId>
<artifactId>jackson-bom</artifactId>

View File

@ -140,5 +140,10 @@
<artifactId>testng</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.wiremock</groupId>
<artifactId>wiremock-standalone</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
</project>

View File

@ -89,12 +89,7 @@ public class AzureDevOpsPersonalAccessTokenFetcher implements PersonalAccessToke
new PersonalAccessTokenParams(
scmServerUrl, tokenName, tokenId, oAuthToken.getToken(), null));
if (valid.isEmpty()) {
throw new ScmCommunicationException(
"Unable to verify if current token is a valid Azure DevOps token. Token's scm-url needs to be '"
+ azureDevOpsScmApiEndpoint
+ "' and was '"
+ scmServerUrl
+ "'");
throw buildScmUnauthorizedException(cheSubject);
} else if (!valid.get().first) {
throw new ScmCommunicationException(
"Current token doesn't have the necessary privileges. Please make sure Che app scopes are correct and containing at least: "
@ -108,14 +103,7 @@ public class AzureDevOpsPersonalAccessTokenFetcher implements PersonalAccessToke
tokenId,
oAuthToken.getToken());
} catch (UnauthorizedException e) {
throw new ScmUnauthorizedException(
cheSubject.getUserName()
+ " is not authorized in "
+ AzureDevOps.PROVIDER_NAME
+ " OAuth provider.",
AzureDevOps.PROVIDER_NAME,
"2.0",
getLocalAuthenticateUrl());
throw buildScmUnauthorizedException(cheSubject);
} catch (NotFoundException nfe) {
throw new UnknownScmProviderException(nfe.getMessage(), scmServerUrl);
} catch (ServerException | ForbiddenException | BadRequestException | ConflictException e) {
@ -124,6 +112,17 @@ public class AzureDevOpsPersonalAccessTokenFetcher implements PersonalAccessToke
}
}
private ScmUnauthorizedException buildScmUnauthorizedException(Subject cheSubject) {
return new ScmUnauthorizedException(
cheSubject.getUserName()
+ " is not authorized in "
+ AzureDevOps.PROVIDER_NAME
+ " OAuth provider.",
AzureDevOps.PROVIDER_NAME,
"2.0",
getLocalAuthenticateUrl());
}
@Override
public Optional<Boolean> isValid(PersonalAccessToken personalAccessToken) {
if (!isValidScmServerUrl(personalAccessToken.getScmProviderUrl())) {

View File

@ -11,17 +11,33 @@
*/
package org.eclipse.che.api.factory.server.azure.devops;
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
import static com.github.tomakehurst.wiremock.client.WireMock.get;
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
import static java.net.HttpURLConnection.HTTP_FORBIDDEN;
import static org.eclipse.che.dto.server.DtoFactory.newDto;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.testng.Assert.*;
import com.github.tomakehurst.wiremock.WireMockServer;
import com.github.tomakehurst.wiremock.client.WireMock;
import com.github.tomakehurst.wiremock.common.Slf4jNotifier;
import com.google.common.net.HttpHeaders;
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.scm.exception.ScmUnauthorizedException;
import org.eclipse.che.commons.subject.Subject;
import org.eclipse.che.commons.subject.SubjectImpl;
import org.eclipse.che.security.oauth.OAuthAPI;
import org.mockito.Mock;
import org.mockito.testng.MockitoTestNGListener;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Listeners;
import org.testng.annotations.Test;
@ -34,13 +50,33 @@ public class AzureDevOpsPersonalAccessTokenFetcherTest {
@Mock private OAuthAPI oAuthAPI;
@Mock private OAuthToken oAuthToken;
@Mock private AzureDevOpsUser azureDevOpsUser;
final int httpPort = 3301;
WireMockServer wireMockServer;
WireMock wireMock;
final String azureOauthToken = "token";
private AzureDevOpsPersonalAccessTokenFetcher personalAccessTokenFetcher;
@BeforeMethod
protected void start() {
wireMockServer =
new WireMockServer(wireMockConfig().notifier(new Slf4jNotifier(false)).port(httpPort));
wireMockServer.start();
WireMock.configureFor("localhost", httpPort);
wireMock = new WireMock("localhost", httpPort);
personalAccessTokenFetcher =
new AzureDevOpsPersonalAccessTokenFetcher(
"localhost", "https://dev.azure.com", new String[] {}, azureDevOpsApiClient, oAuthAPI);
"localhost",
"http://localhost:3301",
new String[] {},
new AzureDevOpsApiClient(wireMockServer.url("/")),
oAuthAPI);
}
@AfterMethod
void stop() {
wireMockServer.stop();
}
@Test
@ -55,6 +91,9 @@ public class AzureDevOpsPersonalAccessTokenFetcherTest {
@Test
public void fetchPersonalAccessTokenShouldReturnToken() throws Exception {
personalAccessTokenFetcher =
new AzureDevOpsPersonalAccessTokenFetcher(
"localhost", "https://dev.azure.com", new String[] {}, azureDevOpsApiClient, oAuthAPI);
when(oAuthAPI.getToken(AzureDevOps.PROVIDER_NAME)).thenReturn(oAuthToken);
when(azureDevOpsApiClient.getUserWithOAuthToken(any())).thenReturn(azureDevOpsUser);
when(azureDevOpsUser.getEmailAddress()).thenReturn("user-email");
@ -65,4 +104,20 @@ public class AzureDevOpsPersonalAccessTokenFetcherTest {
assertNotNull(personalAccessToken);
}
@Test(
expectedExceptions = ScmUnauthorizedException.class,
expectedExceptionsMessageRegExp =
"Username is not authorized in azure-devops OAuth provider.")
public void shouldThrowUnauthorizedExceptionIfTokenIsNotValid() throws Exception {
Subject subject = new SubjectImpl("Username", "id1", "token", false);
OAuthToken oAuthToken = newDto(OAuthToken.class).withToken(azureOauthToken).withScope("");
when(oAuthAPI.getToken(anyString())).thenReturn(oAuthToken);
stubFor(
get(urlEqualTo("/_apis/profile/profiles/me?api-version=7.0"))
.withHeader(HttpHeaders.AUTHORIZATION, equalTo("token " + azureOauthToken))
.willReturn(aResponse().withStatus(HTTP_FORBIDDEN)));
personalAccessTokenFetcher.fetchPersonalAccessToken(subject, wireMockServer.url("/"));
}
}

View File

@ -99,12 +99,7 @@ public class BitbucketPersonalAccessTokenFetcher implements PersonalAccessTokenF
new PersonalAccessTokenParams(
scmServerUrl, tokenName, tokenId, oAuthToken.getToken(), null));
if (valid.isEmpty()) {
throw new ScmCommunicationException(
"Unable to verify if current token is a valid Bitbucket token. Token's scm-url needs to be '"
+ BitbucketApiClient.BITBUCKET_SERVER
+ "' and was '"
+ scmServerUrl
+ "'");
throw buildScmUnauthorizedException(cheSubject);
} else if (!valid.get().first) {
throw new ScmCommunicationException(
"Current token doesn't have the necessary privileges. Please make sure Che app scopes are correct and containing at least: "
@ -120,14 +115,7 @@ public class BitbucketPersonalAccessTokenFetcher implements PersonalAccessTokenF
tokenId,
oAuthToken.getToken());
} catch (UnauthorizedException e) {
throw new ScmUnauthorizedException(
cheSubject.getUserName()
+ " is not authorized in "
+ OAUTH_PROVIDER_NAME
+ " OAuth provider.",
OAUTH_PROVIDER_NAME,
"2.0",
getLocalAuthenticateUrl());
throw buildScmUnauthorizedException(cheSubject);
} catch (NotFoundException nfe) {
throw new UnknownScmProviderException(nfe.getMessage(), scmServerUrl);
} catch (ServerException | ForbiddenException | BadRequestException | ConflictException e) {
@ -136,6 +124,17 @@ public class BitbucketPersonalAccessTokenFetcher implements PersonalAccessTokenF
}
}
private ScmUnauthorizedException buildScmUnauthorizedException(Subject cheSubject) {
return new ScmUnauthorizedException(
cheSubject.getUserName()
+ " is not authorized in "
+ OAUTH_PROVIDER_NAME
+ " OAuth provider.",
OAUTH_PROVIDER_NAME,
"2.0",
getLocalAuthenticateUrl());
}
@Override
public Optional<Boolean> isValid(PersonalAccessToken personalAccessToken) {
if (!bitbucketApiClient.isConnected(personalAccessToken.getScmProviderUrl())) {

View File

@ -212,4 +212,20 @@ public class BitbucketPersonalAccessTokenFetcherTest {
assertFalse(bitbucketPersonalAccessTokenFetcher.isValid(params).isPresent());
}
@Test(
expectedExceptions = ScmUnauthorizedException.class,
expectedExceptionsMessageRegExp = "Username is not authorized in bitbucket OAuth provider.")
public void shouldThrowUnauthorizedExceptionIfTokenIsNotValid() throws Exception {
Subject subject = new SubjectImpl("Username", "id1", "token", false);
OAuthToken oAuthToken = newDto(OAuthToken.class).withToken(bitbucketOauthToken).withScope("");
when(oAuthAPI.getToken(anyString())).thenReturn(oAuthToken);
stubFor(
get(urlEqualTo("/user"))
.withHeader(HttpHeaders.AUTHORIZATION, equalTo("token " + bitbucketOauthToken))
.willReturn(aResponse().withStatus(HTTP_FORBIDDEN)));
bitbucketPersonalAccessTokenFetcher.fetchPersonalAccessToken(
subject, BitbucketApiClient.BITBUCKET_SERVER);
}
}

View File

@ -159,12 +159,7 @@ public class GithubPersonalAccessTokenFetcher implements PersonalAccessTokenFetc
new PersonalAccessTokenParams(
scmServerUrl, tokenName, tokenId, oAuthToken.getToken(), null));
if (valid.isEmpty()) {
throw new ScmCommunicationException(
"Unable to verify if current token is a valid GitHub token. Token's scm-url needs to be '"
+ GithubApiClient.GITHUB_SAAS_ENDPOINT
+ "' and was '"
+ scmServerUrl
+ "'");
throw buildScmUnauthorizedException(cheSubject);
} else if (!valid.get().first) {
throw new ScmCommunicationException(
"Current token doesn't have the necessary privileges. Please make sure Che app scopes are correct and containing at least: "
@ -178,14 +173,7 @@ public class GithubPersonalAccessTokenFetcher implements PersonalAccessTokenFetc
tokenId,
oAuthToken.getToken());
} catch (UnauthorizedException e) {
throw new ScmUnauthorizedException(
cheSubject.getUserName()
+ " is not authorized in "
+ OAUTH_PROVIDER_NAME
+ " OAuth provider.",
OAUTH_PROVIDER_NAME,
"2.0",
getLocalAuthenticateUrl());
throw buildScmUnauthorizedException(cheSubject);
} catch (NotFoundException nfe) {
throw new UnknownScmProviderException(nfe.getMessage(), scmServerUrl);
} catch (ServerException | ForbiddenException | BadRequestException | ConflictException e) {
@ -194,6 +182,17 @@ public class GithubPersonalAccessTokenFetcher implements PersonalAccessTokenFetc
}
}
private ScmUnauthorizedException buildScmUnauthorizedException(Subject cheSubject) {
return new ScmUnauthorizedException(
cheSubject.getUserName()
+ " is not authorized in "
+ OAUTH_PROVIDER_NAME
+ " OAuth provider.",
OAUTH_PROVIDER_NAME,
"2.0",
getLocalAuthenticateUrl());
}
@Override
@Deprecated
public Optional<Boolean> isValid(PersonalAccessToken personalAccessToken) {

View File

@ -161,6 +161,21 @@ public class GithubPersonalAccessTokenFetcherTest {
githubPATFetcher.fetchPersonalAccessToken(subject, wireMockServer.url("/"));
}
@Test(
expectedExceptions = ScmUnauthorizedException.class,
expectedExceptionsMessageRegExp = "Username is not authorized in github OAuth provider.")
public void shouldThrowUnauthorizedExceptionIfTokenIsNotValid() throws Exception {
Subject subject = new SubjectImpl("Username", "id1", "token", false);
OAuthToken oAuthToken = newDto(OAuthToken.class).withToken(githubOauthToken).withScope("");
when(oAuthAPI.getToken(anyString())).thenReturn(oAuthToken);
stubFor(
get(urlEqualTo("/api/v3/user"))
.withHeader(HttpHeaders.AUTHORIZATION, equalTo("token " + githubOauthToken))
.willReturn(aResponse().withStatus(HTTP_FORBIDDEN)));
githubPATFetcher.fetchPersonalAccessToken(subject, wireMockServer.url("/"));
}
@Test
public void shouldReturnToken() throws Exception {
Subject subject = new SubjectImpl("Username", "id1", "token", false);

View File

@ -112,10 +112,12 @@ public class GitlabOAuthTokenFetcher implements PersonalAccessTokenFetcher {
isValid(
new PersonalAccessTokenParams(
scmServerUrl, tokenName, tokenId, oAuthToken.getToken(), null));
if (valid.isEmpty() || !valid.get().first) {
if (valid.isEmpty()) {
throw buildScmUnauthorizedException(cheSubject);
} else if (!valid.get().first) {
throw new ScmCommunicationException(
"Current token doesn't have the necessary privileges. Please make sure Che app scopes are correct and containing at least: "
+ DEFAULT_TOKEN_SCOPES.toString());
"Current token doesn't have the necessary privileges. Please make sure Che app scopes are correct and containing at least: "
+ DEFAULT_TOKEN_SCOPES);
}
return new PersonalAccessToken(
scmServerUrl,
@ -125,14 +127,7 @@ public class GitlabOAuthTokenFetcher implements PersonalAccessTokenFetcher {
tokenId,
oAuthToken.getToken());
} catch (UnauthorizedException e) {
throw new ScmUnauthorizedException(
cheSubject.getUserName()
+ " is not authorized in "
+ OAUTH_PROVIDER_NAME
+ " OAuth provider.",
OAUTH_PROVIDER_NAME,
"2.0",
getLocalAuthenticateUrl());
throw buildScmUnauthorizedException(cheSubject);
} catch (NotFoundException nfe) {
throw new UnknownScmProviderException(nfe.getMessage(), scmServerUrl);
} catch (ServerException | ForbiddenException | BadRequestException | ConflictException e) {
@ -141,6 +136,17 @@ public class GitlabOAuthTokenFetcher implements PersonalAccessTokenFetcher {
}
}
private ScmUnauthorizedException buildScmUnauthorizedException(Subject cheSubject) {
return new ScmUnauthorizedException(
cheSubject.getUserName()
+ " is not authorized in "
+ OAUTH_PROVIDER_NAME
+ " OAuth provider.",
OAUTH_PROVIDER_NAME,
"2.0",
getLocalAuthenticateUrl());
}
@Override
public Optional<Boolean> isValid(PersonalAccessToken personalAccessToken) {
GitlabApiClient gitlabApiClient = getApiClient(personalAccessToken.getScmProviderUrl());

View File

@ -17,6 +17,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.get;
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
import static java.net.HttpURLConnection.HTTP_FORBIDDEN;
import static org.eclipse.che.dto.server.DtoFactory.newDto;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.when;
@ -75,7 +76,7 @@ public class GitlabOAuthTokenFetcherTest {
@Test(
expectedExceptions = ScmCommunicationException.class,
expectedExceptionsMessageRegExp =
"Current token doesn't have the necessary privileges. Please make sure Che app scopes are correct and containing at least: \\[api, write_repository\\]")
"Current token doesn't have the necessary privileges. Please make sure Che app scopes are correct and containing at least: \\[api, write_repository]")
public void shouldThrowExceptionOnInsufficientTokenScopes() throws Exception {
Subject subject = new SubjectImpl("Username", "id1", "token", false);
OAuthToken oAuthToken = newDto(OAuthToken.class).withToken("oauthtoken").withScope("api repo");
@ -185,4 +186,19 @@ public class GitlabOAuthTokenFetcherTest {
assertTrue(valid.isPresent());
assertTrue(valid.get().first);
}
@Test(
expectedExceptions = ScmUnauthorizedException.class,
expectedExceptionsMessageRegExp = "Username is not authorized in gitlab OAuth provider.")
public void shouldThrowUnauthorizedExceptionIfTokenIsNotValid() throws Exception {
Subject subject = new SubjectImpl("Username", "id1", "token", false);
OAuthToken oAuthToken = newDto(OAuthToken.class).withToken("token").withScope("");
when(oAuthAPI.getToken(anyString())).thenReturn(oAuthToken);
stubFor(
get(urlEqualTo("/api/v4/user"))
.withHeader(HttpHeaders.AUTHORIZATION, equalTo("token token"))
.willReturn(aResponse().withStatus(HTTP_FORBIDDEN)));
oAuthTokenFetcher.fetchPersonalAccessToken(subject, wireMockServer.url("/"));
}
}

View File

@ -148,7 +148,8 @@ public class FactoryService extends Service {
factoryParametersResolverHolder.getFactoryParametersResolver(
singletonMap(URL_PARAMETER_NAME, url));
personalAccessTokenManager.getAndStore(
factoryParametersResolver.parseFactoryUrl(url).getHostName());
// get the provider URL from the factory URL
factoryParametersResolver.parseFactoryUrl(url).getProviderUrl());
} catch (ScmCommunicationException
| ScmConfigurationPersistenceException
| UnknownScmProviderException

View File

@ -93,6 +93,8 @@ public class FactoryServiceTest {
private static final DtoFactory DTO = DtoFactory.getInstance();
private final String scmServerUrl = "https://hostName.com";
@Mock private FactoryAcceptValidator acceptValidator;
@Mock private PreferenceManager preferenceManager;
@Mock private UserManager userManager;
@ -228,7 +230,7 @@ public class FactoryServiceTest {
FactoryParametersResolver factoryParametersResolver = mock(FactoryParametersResolver.class);
RemoteFactoryUrl remoteFactoryUrl = mock(RemoteFactoryUrl.class);
when(factoryParametersResolver.parseFactoryUrl(eq("someUrl"))).thenReturn(remoteFactoryUrl);
when(remoteFactoryUrl.getHostName()).thenReturn("hostName");
when(remoteFactoryUrl.getProviderUrl()).thenReturn(scmServerUrl);
doReturn(factoryParametersResolver).when(dummyHolder).getFactoryParametersResolver(anyMap());
service =
new FactoryService(
@ -242,7 +244,7 @@ public class FactoryServiceTest {
.post(SERVICE_PATH + "/token/refresh");
// then
verify(personalAccessTokenManager).getAndStore(eq("hostName"));
verify(personalAccessTokenManager).getAndStore(eq(scmServerUrl));
}
@Test