From ab966cf9fe3536bfa357d9c6c024f5fc0685950b Mon Sep 17 00:00:00 2001 From: Igor Vinokur Date: Wed, 21 Dec 2022 10:43:36 +0200 Subject: [PATCH] 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. --- ...rAuthorizingFactoryParametersResolver.java | 6 +++- .../BitbucketServerScmFileResolver.java | 29 +++++++++++++++---- ...horizingFactoryParametersResolverTest.java | 24 +++++++++++++++ .../BitbucketServerScmFileResolverTest.java | 15 ++++++++++ ...tbucketAuthorizingFileContentProvider.java | 2 +- .../BitbucketFactoryParametersResolver.java | 7 +++-- .../bitbucket/BitbucketScmFileResolver.java | 27 +++++++++++++---- ...itbucketFactoryParametersResolverTest.java | 24 +++++++++++++++ .../BitbucketScmFileResolverTest.java | 15 ++++++++++ .../server/github/GithubURLParser.java | 6 +--- .../GithubFactoryParametersResolverTest.java | 23 +++++++++++++++ .../github/GithubScmFileResolverTest.java | 15 ++++++++++ .../GitlabFactoryParametersResolver.java | 7 +++-- .../server/gitlab/GitlabScmFileResolver.java | 26 +++++++++++++---- .../GitlabFactoryParametersResolverTest.java | 24 +++++++++++++++ .../gitlab/GitlabScmFileResolverTest.java | 15 ++++++++++ 16 files changed, 238 insertions(+), 27 deletions(-) diff --git a/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolver.java b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolver.java index 0c249780ee..7ddfefa360 100644 --- a/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolver.java +++ b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolver.java @@ -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)); } diff --git a/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerScmFileResolver.java b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerScmFileResolver.java index 26f4e9732f..726f90e3c3 100644 --- a/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerScmFileResolver.java +++ b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerScmFileResolver.java @@ -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()); } } } diff --git a/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolverTest.java b/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolverTest.java index 191e34630d..5316bb4323 100644 --- a/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolverTest.java +++ b/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolverTest.java @@ -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 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)); + } } diff --git a/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerScmFileResolverTest.java b/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerScmFileResolverTest.java index 44ececf4ce..555db5a744 100644 --- a/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerScmFileResolverTest.java +++ b/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerScmFileResolverTest.java @@ -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()); + } } diff --git a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProvider.java b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProvider.java index d2d1ec25cf..549b8b3664 100644 --- a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProvider.java +++ b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProvider.java @@ -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); diff --git a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolver.java b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolver.java index 405177e74c..e9ff3015f7 100644 --- a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolver.java +++ b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolver.java @@ -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)); } diff --git a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketScmFileResolver.java b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketScmFileResolver.java index 9dc46e2967..2505ab4b27 100644 --- a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketScmFileResolver.java +++ b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketScmFileResolver.java @@ -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); } } } diff --git a/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolverTest.java b/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolverTest.java index 20002c69cf..bf52a7a468 100644 --- a/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolverTest.java +++ b/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolverTest.java @@ -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 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)); + } } diff --git a/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketScmFileResolverTest.java b/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketScmFileResolverTest.java index 3fba62a7f6..a2164a129d 100644 --- a/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketScmFileResolverTest.java +++ b/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketScmFileResolverTest.java @@ -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()); + } } diff --git a/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubURLParser.java b/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubURLParser.java index ffcd08904a..ec278c54cc 100644 --- a/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubURLParser.java +++ b/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubURLParser.java @@ -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) { diff --git a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubFactoryParametersResolverTest.java b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubFactoryParametersResolverTest.java index 0ed026fc78..34045a4969 100644 --- a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubFactoryParametersResolverTest.java +++ b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubFactoryParametersResolverTest.java @@ -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 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) diff --git a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubScmFileResolverTest.java b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubScmFileResolverTest.java index df9cb7abf0..2b7b57c344 100644 --- a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubScmFileResolverTest.java +++ b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubScmFileResolverTest.java @@ -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()); + } } diff --git a/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolver.java b/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolver.java index e18979640e..0ece723a38 100644 --- a/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolver.java +++ b/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolver.java @@ -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)); } diff --git a/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabScmFileResolver.java b/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabScmFileResolver.java index cf2e4b7e9d..a926a256c2 100644 --- a/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabScmFileResolver.java +++ b/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabScmFileResolver.java @@ -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); } } } diff --git a/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolverTest.java b/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolverTest.java index 59dec0a897..1b76bf9c54 100644 --- a/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolverTest.java +++ b/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolverTest.java @@ -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 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 { diff --git a/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabScmFileResolverTest.java b/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabScmFileResolverTest.java index 74d682296c..0ff7cdf66b 100644 --- a/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabScmFileResolverTest.java +++ b/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabScmFileResolverTest.java @@ -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()); + } }