From b49283da908d801b537d9e771921dbeac06546f6 Mon Sep 17 00:00:00 2001 From: Igor Vinokur Date: Tue, 19 Nov 2024 17:19:47 +0200 Subject: [PATCH] Use HTTP response status code to check gitlab repositories accessibility (#741) Improve the isPublicRepository() function in the GitlabAuthorizingFileContentProvider class to check the http response status code. The previous way with the urlFetcher does not work because gitlab server returns a content of an html page with credentials input instead of an error. --- .../GitlabAuthorizingFileContentProvider.java | 41 +++++++++- ...labAuthorizingFileContentProviderTest.java | 77 +++++++++++++++++-- 2 files changed, 108 insertions(+), 10 deletions(-) diff --git a/wsmaster/che-core-api-factory-gitlab-common/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabAuthorizingFileContentProvider.java b/wsmaster/che-core-api-factory-gitlab-common/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabAuthorizingFileContentProvider.java index b3115c7f64..fd270923a7 100644 --- a/wsmaster/che-core-api-factory-gitlab-common/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabAuthorizingFileContentProvider.java +++ b/wsmaster/che-core-api-factory-gitlab-common/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabAuthorizingFileContentProvider.java @@ -11,27 +11,62 @@ */ package org.eclipse.che.api.factory.server.gitlab; +import static java.net.HttpURLConnection.HTTP_OK; +import static java.time.Duration.ofSeconds; + +import com.google.common.util.concurrent.ThreadFactoryBuilder; import java.io.IOException; +import java.io.InputStream; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.time.Duration; +import java.util.concurrent.Executors; import org.eclipse.che.api.factory.server.scm.AuthorizingFileContentProvider; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; import org.eclipse.che.api.workspace.server.devfile.URLFetcher; +import org.eclipse.che.commons.lang.concurrent.LoggingUncaughtExceptionHandler; /** Gitlab specific authorizing file content provider. */ class GitlabAuthorizingFileContentProvider extends AuthorizingFileContentProvider { + private final HttpClient httpClient; + + private static final Duration DEFAULT_HTTP_TIMEOUT = ofSeconds(10); + GitlabAuthorizingFileContentProvider( GitlabUrl gitlabUrl, URLFetcher urlFetcher, PersonalAccessTokenManager personalAccessTokenManager) { super(gitlabUrl, urlFetcher, personalAccessTokenManager); + this.httpClient = + HttpClient.newBuilder() + .executor( + Executors.newCachedThreadPool( + new ThreadFactoryBuilder() + .setUncaughtExceptionHandler(LoggingUncaughtExceptionHandler.getInstance()) + .setNameFormat(GitlabAuthorizingFileContentProvider.class.getName() + "-%d") + .setDaemon(true) + .build())) + .connectTimeout(DEFAULT_HTTP_TIMEOUT) + .version(HttpClient.Version.HTTP_1_1) + .build(); } @Override protected boolean isPublicRepository(GitlabUrl remoteFactoryUrl) { + HttpRequest request = + HttpRequest.newBuilder( + URI.create( + remoteFactoryUrl.getProviderUrl() + '/' + remoteFactoryUrl.getSubGroups())) + .timeout(DEFAULT_HTTP_TIMEOUT) + .build(); try { - urlFetcher.fetch(remoteFactoryUrl.getProviderUrl() + '/' + remoteFactoryUrl.getSubGroups()); - return true; - } catch (IOException e) { + HttpResponse response = + httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream()); + return response.statusCode() == HTTP_OK; + } catch (IOException | InterruptedException e) { return false; } } diff --git a/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabAuthorizingFileContentProviderTest.java b/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabAuthorizingFileContentProviderTest.java index 796223a47f..8749e03251 100644 --- a/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabAuthorizingFileContentProviderTest.java +++ b/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabAuthorizingFileContentProviderTest.java @@ -11,29 +11,60 @@ */ package org.eclipse.che.api.factory.server.gitlab; +import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; +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.lang.String.format; +import static java.net.HttpURLConnection.HTTP_MOVED_TEMP; +import static java.net.HttpURLConnection.HTTP_OK; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import com.github.tomakehurst.wiremock.WireMockServer; +import com.github.tomakehurst.wiremock.client.WireMock; +import com.github.tomakehurst.wiremock.common.Slf4jNotifier; import java.io.FileNotFoundException; +import java.net.URI; 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.UnknownScmProviderException; import org.eclipse.che.api.workspace.server.devfile.FileContentProvider; import org.eclipse.che.api.workspace.server.devfile.URLFetcher; +import org.eclipse.che.api.workspace.server.devfile.exception.DevfileException; import org.mockito.Mock; -import org.mockito.Mockito; 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; @Listeners(MockitoTestNGListener.class) public class GitlabAuthorizingFileContentProviderTest { @Mock private PersonalAccessTokenManager personalAccessTokenManager; + @Mock private URLFetcher urlFetcher; + + private WireMockServer wireMockServer; + private WireMock wireMock; + + @BeforeMethod + public void start() { + wireMockServer = + new WireMockServer(wireMockConfig().notifier(new Slf4jNotifier(false)).dynamicPort()); + wireMockServer.start(); + WireMock.configureFor("localhost", wireMockServer.port()); + wireMock = new WireMock("localhost", wireMockServer.port()); + } + + @AfterMethod + void stop() { + wireMockServer.stop(); + } @Test public void shouldExpandRelativePaths() throws Exception { - URLFetcher urlFetcher = Mockito.mock(URLFetcher.class); GitlabUrl gitlabUrl = new GitlabUrl().withHostName("gitlab.net").withSubGroups("eclipse/che"); FileContentProvider fileContentProvider = new GitlabAuthorizingFileContentProvider(gitlabUrl, urlFetcher, personalAccessTokenManager); @@ -49,7 +80,6 @@ public void shouldExpandRelativePaths() throws Exception { @Test public void shouldPreserveAbsolutePaths() throws Exception { - URLFetcher urlFetcher = Mockito.mock(URLFetcher.class); GitlabUrl gitlabUrl = new GitlabUrl().withHostName("gitlab.net").withSubGroups("eclipse/che"); FileContentProvider fileContentProvider = new GitlabAuthorizingFileContentProvider(gitlabUrl, urlFetcher, personalAccessTokenManager); @@ -65,18 +95,51 @@ public void shouldPreserveAbsolutePaths() throws Exception { @Test(expectedExceptions = FileNotFoundException.class) public void shouldThrowFileNotFoundException() throws Exception { // given - URLFetcher urlFetcher = Mockito.mock(URLFetcher.class); when(urlFetcher.fetch( eq( - "https://gitlab.com/api/v4/projects/eclipse%2Fche/repository/files/devfile.yaml/raw?ref=HEAD"))) + wireMockServer.url( + "/api/v4/projects/eclipse%2Fche/repository/files/devfile.yaml/raw?ref=HEAD")))) .thenThrow(new FileNotFoundException()); - when(urlFetcher.fetch(eq("https://gitlab.com/eclipse/che"))).thenReturn("content"); when(personalAccessTokenManager.getAndStore(anyString())) .thenThrow(new UnknownScmProviderException("", "")); - GitlabUrl gitlabUrl = new GitlabUrl().withHostName("gitlab.com").withSubGroups("eclipse/che"); + URI uri = URI.create(wireMockServer.url("/")); + GitlabUrl gitlabUrl = + new GitlabUrl() + .withScheme("http") + .withHostName(format("%s:%s", uri.getHost(), uri.getPort())) + .withSubGroups("eclipse/che"); FileContentProvider fileContentProvider = new GitlabAuthorizingFileContentProvider(gitlabUrl, urlFetcher, personalAccessTokenManager); + stubFor(get(urlEqualTo("/eclipse/che")).willReturn(aResponse().withStatus(HTTP_OK))); + + // when + fileContentProvider.fetchContent("devfile.yaml"); + } + + @Test( + expectedExceptions = DevfileException.class, + expectedExceptionsMessageRegExp = "Could not reach devfile at test path") + public void shouldThrowDevfileException() throws Exception { + // given + when(urlFetcher.fetch( + eq( + wireMockServer.url( + "/api/v4/projects/eclipse%2Fche/repository/files/devfile.yaml/raw?ref=HEAD")))) + .thenThrow(new FileNotFoundException("test path")); + when(personalAccessTokenManager.getAndStore(anyString())) + .thenThrow(new UnknownScmProviderException("", "")); + URI uri = URI.create(wireMockServer.url("/")); + GitlabUrl gitlabUrl = + new GitlabUrl() + .withScheme("http") + .withHostName(format("%s:%s", uri.getHost(), uri.getPort())) + .withSubGroups("eclipse/che"); + FileContentProvider fileContentProvider = + new GitlabAuthorizingFileContentProvider(gitlabUrl, urlFetcher, personalAccessTokenManager); + + stubFor(get(urlEqualTo("/eclipse/che")).willReturn(aResponse().withStatus(HTTP_MOVED_TEMP))); + // when fileContentProvider.fetchContent("devfile.yaml"); }