Retry factory authentication when user rejects git service oauth (#405)

Currently when user rejects the oauth page for GitHub factory, Che tries to continue the factory flow without authentication (possible only for public repo). Apply the logic for the other git authentication providers.
pull/406/head
Igor Vinokur 2022-12-21 10:43:36 +02:00 committed by GitHub
parent 7d6f5f5af5
commit ab966cf9fe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 238 additions and 27 deletions

View File

@ -14,6 +14,7 @@ package org.eclipse.che.api.factory.server.bitbucket;
import static org.eclipse.che.api.factory.shared.Constants.CURRENT_VERSION;
import static org.eclipse.che.api.factory.shared.Constants.URL_PARAMETER_NAME;
import static org.eclipse.che.dto.server.DtoFactory.newDto;
import static org.eclipse.che.security.oauth1.OAuthAuthenticationService.ERROR_QUERY_NAME;
import jakarta.validation.constraints.NotNull;
import java.util.Map;
@ -91,13 +92,16 @@ public class BitbucketServerAuthorizingFactoryParametersResolver
new BitbucketServerAuthorizingFileContentProvider(
bitbucketServerUrl, urlFetcher, personalAccessTokenManager);
boolean skipAuthentication =
factoryParameters.get(ERROR_QUERY_NAME) != null
&& factoryParameters.get(ERROR_QUERY_NAME).equals("access_denied");
// create factory from the following location if location exists, else create default factory
return urlFactoryBuilder
.createFactoryFromDevfile(
bitbucketServerUrl,
fileContentProvider,
extractOverrideParams(factoryParameters),
false)
skipAuthentication)
.orElseGet(() -> newDto(FactoryDto.class).withV(CURRENT_VERSION).withSource("repo"))
.acceptVisitor(new BitbucketFactoryVisitor(bitbucketServerUrl));
}

View File

@ -50,13 +50,30 @@ public class BitbucketServerScmFileResolver implements ScmFileResolver {
BitbucketServerUrl bitbucketServerUrl = bitbucketURLParser.parse(repository);
try {
return new BitbucketServerAuthorizingFileContentProvider(
bitbucketServerUrl, urlFetcher, personalAccessTokenManager)
.fetchContent(filePath);
return fetchContent(bitbucketServerUrl, filePath, false);
} catch (DevfileException exception) {
// This catch might mean that the authentication was rejected by user, try to repeat the fetch
// without authentication flow.
try {
return fetchContent(bitbucketServerUrl, filePath, true);
} catch (DevfileException devfileException) {
throw toApiException(devfileException);
}
}
}
private String fetchContent(
BitbucketServerUrl bitbucketServerUrl, String filePath, boolean skipAuthentication)
throws DevfileException, NotFoundException {
try {
BitbucketServerAuthorizingFileContentProvider contentProvider =
new BitbucketServerAuthorizingFileContentProvider(
bitbucketServerUrl, urlFetcher, personalAccessTokenManager);
return skipAuthentication
? contentProvider.fetchContentWithoutAuthentication(filePath)
: contentProvider.fetchContent(filePath);
} catch (IOException e) {
throw new NotFoundException("Unable to retrieve file from given location.");
} catch (DevfileException devfileException) {
throw toApiException(devfileException);
throw new NotFoundException(e.getMessage());
}
}
}

View File

@ -16,6 +16,7 @@ import static org.eclipse.che.api.factory.shared.Constants.CURRENT_VERSION;
import static org.eclipse.che.api.factory.shared.Constants.URL_PARAMETER_NAME;
import static org.eclipse.che.api.workspace.server.devfile.Constants.CURRENT_API_VERSION;
import static org.eclipse.che.dto.server.DtoFactory.newDto;
import static org.eclipse.che.security.oauth1.OAuthAuthenticationService.ERROR_QUERY_NAME;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyMap;
@ -32,6 +33,7 @@ import static org.testng.Assert.assertTrue;
import com.google.common.collect.ImmutableMap;
import java.util.Map;
import java.util.Optional;
import org.eclipse.che.api.core.ApiException;
import org.eclipse.che.api.core.model.factory.ScmInfo;
import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager;
import org.eclipse.che.api.factory.server.urlfactory.DevfileFilenamesProvider;
@ -182,4 +184,26 @@ public class BitbucketServerAuthorizingFactoryParametersResolverTest {
.withSource("repo")
.withDevfile(Map.of("schemaVersion", "2.0.0"));
}
@Test
public void shouldCreateFactoryWithoutAuthentication() throws ApiException {
// given
String bitbucketServerUrl = "http://bitbucket.2mcl.com/scm/~user/repo.git";
Map<String, String> params =
ImmutableMap.of(URL_PARAMETER_NAME, bitbucketServerUrl, ERROR_QUERY_NAME, "access_denied");
when(urlFactoryBuilder.createFactoryFromDevfile(
any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean()))
.thenReturn(Optional.of(generateDevfileFactory()));
// when
bitbucketServerFactoryParametersResolver.createFactory(params);
// then
verify(urlFactoryBuilder)
.createFactoryFromDevfile(
any(BitbucketServerUrl.class),
any(BitbucketServerAuthorizingFileContentProvider.class),
anyMap(),
eq(true));
}
}

View File

@ -14,11 +14,13 @@ package org.eclipse.che.api.factory.server.bitbucket;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.testng.Assert.*;
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.ScmUnauthorizedException;
import org.eclipse.che.api.factory.server.urlfactory.DevfileFilenamesProvider;
import org.eclipse.che.api.workspace.server.devfile.URLFetcher;
import org.mockito.Mock;
@ -80,4 +82,17 @@ public class BitbucketServerScmFileResolverTest {
assertEquals(content, rawContent);
}
@Test
public void shouldFetchContentWithoutAuthentication() throws Exception {
// given
when(personalAccessTokenManager.getAndStore(anyString()))
.thenThrow(new ScmUnauthorizedException("message", "bitbucket-server", "v1", "url"));
// when
serverScmFileResolver.fileContent("https://foo.bar/scm/~username/repo.git", "devfile.yaml");
// then
verify(urlFetcher).fetch(anyString());
}
}

View File

@ -58,7 +58,7 @@ class BitbucketAuthorizingFileContentProvider extends AuthorizingFileContentProv
split[3],
split[4],
split[6],
fileURL.substring(fileURL.indexOf(split[6]) + split[6].length() + 1),
requestURL.substring(requestURL.indexOf(split[6]) + split[6].length() + 1),
token.getToken());
} catch (UnknownScmProviderException e) {
return fetchContentWithoutToken(requestURL, e);

View File

@ -14,6 +14,7 @@ package org.eclipse.che.api.factory.server.bitbucket;
import static org.eclipse.che.api.factory.shared.Constants.CURRENT_VERSION;
import static org.eclipse.che.api.factory.shared.Constants.URL_PARAMETER_NAME;
import static org.eclipse.che.dto.server.DtoFactory.newDto;
import static org.eclipse.che.security.oauth1.OAuthAuthenticationService.ERROR_QUERY_NAME;
import jakarta.validation.constraints.NotNull;
import java.util.Map;
@ -96,7 +97,9 @@ public class BitbucketFactoryParametersResolver extends DefaultFactoryParameterR
// no need to check null value of url parameter as accept() method has performed the check
final BitbucketUrl bitbucketUrl =
bitbucketURLParser.parse(factoryParameters.get(URL_PARAMETER_NAME));
boolean skipAuthentication =
factoryParameters.get(ERROR_QUERY_NAME) != null
&& factoryParameters.get(ERROR_QUERY_NAME).equals("access_denied");
// create factory from the following location if location exists, else create default factory
return urlFactoryBuilder
.createFactoryFromDevfile(
@ -104,7 +107,7 @@ public class BitbucketFactoryParametersResolver extends DefaultFactoryParameterR
new BitbucketAuthorizingFileContentProvider(
bitbucketUrl, urlFetcher, personalAccessTokenManager, bitbucketApiClient),
extractOverrideParams(factoryParameters),
false)
skipAuthentication)
.orElseGet(() -> newDto(FactoryDto.class).withV(CURRENT_VERSION).withSource("repo"))
.acceptVisitor(new BitbucketFactoryVisitor(bitbucketUrl));
}

View File

@ -54,13 +54,30 @@ public class BitbucketScmFileResolver implements ScmFileResolver {
throws ApiException {
final BitbucketUrl bitbucketUrl = bitbucketUrlParser.parse(repository);
try {
return new BitbucketAuthorizingFileContentProvider(
bitbucketUrl, urlFetcher, personalAccessTokenManager, bitbucketApiClient)
.fetchContent(bitbucketUrl.rawFileLocation(filePath));
return fetchContent(bitbucketUrl, filePath, false);
} catch (DevfileException exception) {
// This catch might mean that the authentication was rejected by user, try to repeat the fetch
// without authentication flow.
try {
return fetchContent(bitbucketUrl, filePath, true);
} catch (DevfileException devfileException) {
throw toApiException(devfileException);
}
}
}
private String fetchContent(
BitbucketUrl bitbucketUrl, String filePath, boolean skipAuthentication)
throws DevfileException, NotFoundException {
try {
BitbucketAuthorizingFileContentProvider contentProvider =
new BitbucketAuthorizingFileContentProvider(
bitbucketUrl, urlFetcher, personalAccessTokenManager, bitbucketApiClient);
return skipAuthentication
? contentProvider.fetchContentWithoutAuthentication(filePath)
: contentProvider.fetchContent(filePath);
} catch (IOException e) {
throw new NotFoundException(e.getMessage());
} catch (DevfileException devfileException) {
throw toApiException(devfileException);
}
}
}

View File

@ -16,6 +16,7 @@ import static org.eclipse.che.api.factory.shared.Constants.CURRENT_VERSION;
import static org.eclipse.che.api.factory.shared.Constants.URL_PARAMETER_NAME;
import static org.eclipse.che.api.workspace.server.devfile.Constants.CURRENT_API_VERSION;
import static org.eclipse.che.dto.server.DtoFactory.newDto;
import static org.eclipse.che.security.oauth1.OAuthAuthenticationService.ERROR_QUERY_NAME;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyMap;
@ -33,6 +34,7 @@ import com.google.common.collect.ImmutableMap;
import java.util.Collections;
import java.util.Map;
import java.util.Optional;
import org.eclipse.che.api.core.ApiException;
import org.eclipse.che.api.core.model.factory.ScmInfo;
import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager;
import org.eclipse.che.api.factory.server.urlfactory.DevfileFilenamesProvider;
@ -275,4 +277,26 @@ public class BitbucketFactoryParametersResolverTest {
.withSource("repo")
.withDevfile(Map.of("schemaVersion", "2.0.0"));
}
@Test
public void shouldCreateFactoryWithoutAuthentication() throws ApiException {
// given
String bitbucketUrl = "https://bitbucket.org/user/repo.git";
Map<String, String> params =
ImmutableMap.of(URL_PARAMETER_NAME, bitbucketUrl, ERROR_QUERY_NAME, "access_denied");
when(urlFactoryBuilder.createFactoryFromDevfile(
any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean()))
.thenReturn(Optional.of(generateDevfileFactory()));
// when
bitbucketFactoryParametersResolver.createFactory(params);
// then
verify(urlFactoryBuilder)
.createFactoryFromDevfile(
any(BitbucketUrl.class),
any(BitbucketAuthorizingFileContentProvider.class),
anyMap(),
eq(true));
}
}

View File

@ -13,6 +13,7 @@ package org.eclipse.che.api.factory.server.bitbucket;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.testng.AssertJUnit.assertEquals;
import static org.testng.AssertJUnit.assertFalse;
@ -21,6 +22,7 @@ import static org.testng.AssertJUnit.assertTrue;
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.ScmUnauthorizedException;
import org.eclipse.che.api.factory.server.urlfactory.DevfileFilenamesProvider;
import org.eclipse.che.api.workspace.server.devfile.URLFetcher;
import org.mockito.Mock;
@ -81,4 +83,17 @@ public class BitbucketScmFileResolverTest {
assertEquals(content, rawContent);
}
@Test
public void shouldFetchContentWithoutAuthentication() throws Exception {
// given
when(personalAccessTokenManager.getAndStore(anyString()))
.thenThrow(new ScmUnauthorizedException("message", "bitbucket", "v1", "url"));
// when
bitbucketScmFileResolver.fileContent("https://bitbucket.org/username/repo.git", "devfile.yaml");
// then
verify(urlFetcher).fetch(anyString());
}
}

View File

@ -245,8 +245,7 @@ public class GithubURLParser {
repoName,
branchName,
personalAccessToken != null ? personalAccessToken.getToken() : null);
} catch (UnknownScmProviderException e) {
} catch (UnknownScmProviderException | ScmUnauthorizedException e) {
// get latest commit without authentication
try {
return this.apiClient.getLatestCommit(repoUser, repoName, branchName, null);
@ -256,9 +255,6 @@ public class GithubURLParser {
| URISyntaxException exception) {
LOG.error("Failed to authenticate to GitHub", e);
}
} catch (ScmUnauthorizedException e) {
throw toApiException(e);
} catch (ScmCommunicationException
| UnsatisfiedScmPreconditionException
| ScmConfigurationPersistenceException e) {

View File

@ -36,6 +36,7 @@ import com.google.common.collect.ImmutableMap;
import java.util.Collections;
import java.util.Map;
import java.util.Optional;
import org.eclipse.che.api.core.ApiException;
import org.eclipse.che.api.core.model.factory.ScmInfo;
import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager;
import org.eclipse.che.api.factory.server.urlfactory.DevfileFilenamesProvider;
@ -328,6 +329,28 @@ public class GithubFactoryParametersResolverTest {
assertEquals(scmInfo.getBranch(), "foobar");
}
@Test
public void shouldCreateFactoryWithoutAuthentication() throws ApiException {
// given
String githubUrl = "https://github.com/user/repo.git";
Map<String, String> params =
ImmutableMap.of(URL_PARAMETER_NAME, githubUrl, ERROR_QUERY_NAME, "access_denied");
when(urlFactoryBuilder.createFactoryFromDevfile(
any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean()))
.thenReturn(Optional.of(generateDevfileFactory()));
// when
githubFactoryParametersResolver.createFactory(params);
// then
verify(urlFactoryBuilder)
.createFactoryFromDevfile(
any(GithubUrl.class),
any(GithubAuthorizingFileContentProvider.class),
anyMap(),
eq(true));
}
private FactoryDto generateDevfileFactory() {
return newDto(FactoryDto.class)
.withV(CURRENT_VERSION)

View File

@ -15,11 +15,13 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.testng.Assert.*;
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.ScmUnauthorizedException;
import org.eclipse.che.api.factory.server.urlfactory.DevfileFilenamesProvider;
import org.eclipse.che.api.workspace.server.devfile.URLFetcher;
import org.mockito.Mock;
@ -97,4 +99,17 @@ public class GithubScmFileResolverTest {
assertEquals(content, rawContent);
}
@Test
public void shouldReturnContentWithoutAuthentication() throws Exception {
// given
when(personalAccessTokenManager.getAndStore(anyString()))
.thenThrow(new ScmUnauthorizedException("message", "github", "v1", "url"));
// when
githubScmFileResolver.fileContent("https://github.com/username/repo.git", "devfile.yaml");
// then
verify(urlFetcher).fetch(anyString());
}
}

View File

@ -14,6 +14,7 @@ package org.eclipse.che.api.factory.server.gitlab;
import static org.eclipse.che.api.factory.shared.Constants.CURRENT_VERSION;
import static org.eclipse.che.api.factory.shared.Constants.URL_PARAMETER_NAME;
import static org.eclipse.che.dto.server.DtoFactory.newDto;
import static org.eclipse.che.security.oauth1.OAuthAuthenticationService.ERROR_QUERY_NAME;
import jakarta.validation.constraints.NotNull;
import java.util.Map;
@ -80,7 +81,9 @@ public class GitlabFactoryParametersResolver extends DefaultFactoryParameterReso
throws ApiException {
// no need to check null value of url parameter as accept() method has performed the check
final GitlabUrl gitlabUrl = gitlabURLParser.parse(factoryParameters.get(URL_PARAMETER_NAME));
boolean skipAuthentication =
factoryParameters.get(ERROR_QUERY_NAME) != null
&& factoryParameters.get(ERROR_QUERY_NAME).equals("access_denied");
// create factory from the following location if location exists, else create default factory
return urlFactoryBuilder
.createFactoryFromDevfile(
@ -88,7 +91,7 @@ public class GitlabFactoryParametersResolver extends DefaultFactoryParameterReso
new GitlabAuthorizingFileContentProvider(
gitlabUrl, urlFetcher, personalAccessTokenManager),
extractOverrideParams(factoryParameters),
false)
skipAuthentication)
.orElseGet(() -> newDto(FactoryDto.class).withV(CURRENT_VERSION).withSource("repo"))
.acceptVisitor(new GitlabFactoryVisitor(gitlabUrl));
}

View File

@ -49,13 +49,29 @@ public class GitlabScmFileResolver implements ScmFileResolver {
GitlabUrl gitlabUrl = gitlabUrlParser.parse(repository);
try {
return new GitlabAuthorizingFileContentProvider(
gitlabUrl, urlFetcher, personalAccessTokenManager)
.fetchContent(gitlabUrl.rawFileLocation(filePath));
return fetchContent(gitlabUrl, filePath, false);
} catch (DevfileException exception) {
// This catch might mean that the authentication was rejected by user, try to repeat the fetch
// without authentication flow.
try {
return fetchContent(gitlabUrl, filePath, true);
} catch (DevfileException devfileException) {
throw toApiException(devfileException);
}
}
}
private String fetchContent(GitlabUrl gitlabUrl, String filePath, boolean skipAuthentication)
throws DevfileException, NotFoundException {
try {
GitlabAuthorizingFileContentProvider contentProvider =
new GitlabAuthorizingFileContentProvider(
gitlabUrl, urlFetcher, personalAccessTokenManager);
return skipAuthentication
? contentProvider.fetchContentWithoutAuthentication(filePath)
: contentProvider.fetchContent(filePath);
} catch (IOException e) {
throw new NotFoundException(e.getMessage());
} catch (DevfileException devfileException) {
throw toApiException(devfileException);
}
}
}

View File

@ -16,6 +16,7 @@ import static org.eclipse.che.api.factory.shared.Constants.CURRENT_VERSION;
import static org.eclipse.che.api.factory.shared.Constants.URL_PARAMETER_NAME;
import static org.eclipse.che.api.workspace.server.devfile.Constants.CURRENT_API_VERSION;
import static org.eclipse.che.dto.server.DtoFactory.newDto;
import static org.eclipse.che.security.oauth1.OAuthAuthenticationService.ERROR_QUERY_NAME;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyMap;
@ -32,6 +33,7 @@ import static org.testng.Assert.assertTrue;
import com.google.common.collect.ImmutableMap;
import java.util.Map;
import java.util.Optional;
import org.eclipse.che.api.core.ApiException;
import org.eclipse.che.api.core.model.factory.ScmInfo;
import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager;
import org.eclipse.che.api.factory.server.urlfactory.DevfileFilenamesProvider;
@ -143,6 +145,28 @@ public class GitlabFactoryParametersResolverTest {
assertEquals(source.getBranch(), "foobar");
}
@Test
public void shouldCreateFactoryWithoutAuthentication() throws ApiException {
// given
String gitlabUrl = "https://gitlab.com/user/repo.git";
Map<String, String> params =
ImmutableMap.of(URL_PARAMETER_NAME, gitlabUrl, ERROR_QUERY_NAME, "access_denied");
when(urlFactoryBuilder.createFactoryFromDevfile(
any(RemoteFactoryUrl.class), any(), anyMap(), anyBoolean()))
.thenReturn(Optional.of(generateDevfileFactory()));
// when
gitlabFactoryParametersResolver.createFactory(params);
// then
verify(urlFactoryBuilder)
.createFactoryFromDevfile(
any(GitlabUrl.class),
any(GitlabAuthorizingFileContentProvider.class),
anyMap(),
eq(true));
}
@Test
public void shouldSetScmInfoIntoDevfileV2() throws Exception {

View File

@ -15,11 +15,13 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.testng.Assert.*;
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.ScmUnauthorizedException;
import org.eclipse.che.api.factory.server.urlfactory.DevfileFilenamesProvider;
import org.eclipse.che.api.workspace.server.devfile.URLFetcher;
import org.mockito.Mock;
@ -80,4 +82,17 @@ public class GitlabScmFileResolverTest {
assertEquals(content, rawContent);
}
@Test
public void shouldFetchContentWithoutAuthentication() throws Exception {
// given
when(personalAccessTokenManager.getAndStore(anyString()))
.thenThrow(new ScmUnauthorizedException("message", "gitlab", "v1", "url"));
// when
gitlabScmFileResolver.fileContent("https://gitlab.com/username/repo.git", "devfile.yaml");
// then
verify(urlFetcher).fetch(anyString());
}
}