Skip to content

Commit bfca617

Browse files
vinokurigvinokurig
committed
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.
1 parent be7243d commit bfca617

File tree

2 files changed

+109
-11
lines changed

2 files changed

+109
-11
lines changed

‎wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabAuthorizingFileContentProvider.java‎

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2012-2023 Red Hat, Inc.
2+
* Copyright (c) 2012-2024 Red Hat, Inc.
33
* This program and the accompanying materials are made
44
* available under the terms of the Eclipse Public License 2.0
55
* which is available at https://www.eclipse.org/legal/epl-2.0/
@@ -11,27 +11,62 @@
1111
*/
1212
package org.eclipse.che.api.factory.server.gitlab;
1313

14+
import static java.net.HttpURLConnection.HTTP_OK;
15+
import static java.time.Duration.ofSeconds;
16+
17+
import com.google.common.util.concurrent.ThreadFactoryBuilder;
1418
import java.io.IOException;
19+
import java.io.InputStream;
20+
import java.net.URI;
21+
import java.net.http.HttpClient;
22+
import java.net.http.HttpRequest;
23+
import java.net.http.HttpResponse;
24+
import java.time.Duration;
25+
import java.util.concurrent.Executors;
1526
import org.eclipse.che.api.factory.server.scm.AuthorizingFileContentProvider;
1627
import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager;
1728
import org.eclipse.che.api.workspace.server.devfile.URLFetcher;
29+
import org.eclipse.che.commons.lang.concurrent.LoggingUncaughtExceptionHandler;
1830

1931
/** Gitlab specific authorizing file content provider. */
2032
class GitlabAuthorizingFileContentProvider extends AuthorizingFileContentProvider<GitlabUrl> {
2133

34+
private final HttpClient httpClient;
35+
36+
private static final Duration DEFAULT_HTTP_TIMEOUT = ofSeconds(10);
37+
2238
GitlabAuthorizingFileContentProvider(
2339
GitlabUrl gitlabUrl,
2440
URLFetcher urlFetcher,
2541
PersonalAccessTokenManager personalAccessTokenManager) {
2642
super(gitlabUrl, urlFetcher, personalAccessTokenManager);
43+
this.httpClient =
44+
HttpClient.newBuilder()
45+
.executor(
46+
Executors.newCachedThreadPool(
47+
new ThreadFactoryBuilder()
48+
.setUncaughtExceptionHandler(LoggingUncaughtExceptionHandler.getInstance())
49+
.setNameFormat(GitlabAuthorizingFileContentProvider.class.getName() + "-%d")
50+
.setDaemon(true)
51+
.build()))
52+
.connectTimeout(DEFAULT_HTTP_TIMEOUT)
53+
.version(HttpClient.Version.HTTP_1_1)
54+
.build();
2755
}
2856

2957
@Override
3058
protected boolean isPublicRepository(GitlabUrl remoteFactoryUrl) {
59+
HttpRequest request =
60+
HttpRequest.newBuilder(
61+
URI.create(
62+
remoteFactoryUrl.getProviderUrl() + '/' + remoteFactoryUrl.getSubGroups()))
63+
.timeout(DEFAULT_HTTP_TIMEOUT)
64+
.build();
3165
try {
32-
urlFetcher.fetch(remoteFactoryUrl.getProviderUrl() + '/' + remoteFactoryUrl.getSubGroups());
33-
return true;
34-
} catch (IOException e) {
66+
HttpResponse<InputStream> response =
67+
httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
68+
return response.statusCode() == HTTP_OK;
69+
} catch (IOException | InterruptedException e) {
3570
return false;
3671
}
3772
}

‎wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabAuthorizingFileContentProviderTest.java‎

Lines changed: 70 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,29 +11,60 @@
1111
*/
1212
package org.eclipse.che.api.factory.server.gitlab;
1313

14+
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
15+
import static com.github.tomakehurst.wiremock.client.WireMock.get;
16+
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
17+
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
18+
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
19+
import static java.lang.String.format;
20+
import static java.net.HttpURLConnection.HTTP_MOVED_TEMP;
21+
import static java.net.HttpURLConnection.HTTP_OK;
1422
import static org.mockito.ArgumentMatchers.*;
1523
import static org.mockito.Mockito.verify;
1624
import static org.mockito.Mockito.when;
1725

26+
import com.github.tomakehurst.wiremock.WireMockServer;
27+
import com.github.tomakehurst.wiremock.client.WireMock;
28+
import com.github.tomakehurst.wiremock.common.Slf4jNotifier;
1829
import java.io.FileNotFoundException;
30+
import java.net.URI;
1931
import org.eclipse.che.api.factory.server.scm.PersonalAccessToken;
2032
import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager;
2133
import org.eclipse.che.api.factory.server.scm.exception.UnknownScmProviderException;
2234
import org.eclipse.che.api.workspace.server.devfile.FileContentProvider;
2335
import org.eclipse.che.api.workspace.server.devfile.URLFetcher;
36+
import org.eclipse.che.api.workspace.server.devfile.exception.DevfileException;
2437
import org.mockito.Mock;
25-
import org.mockito.Mockito;
2638
import org.mockito.testng.MockitoTestNGListener;
39+
import org.testng.annotations.AfterMethod;
40+
import org.testng.annotations.BeforeMethod;
2741
import org.testng.annotations.Listeners;
2842
import org.testng.annotations.Test;
2943

3044
@Listeners(MockitoTestNGListener.class)
3145
public class GitlabAuthorizingFileContentProviderTest {
3246
@Mock private PersonalAccessTokenManager personalAccessTokenManager;
47+
@Mock private URLFetcher urlFetcher;
48+
49+
private WireMockServer wireMockServer;
50+
private WireMock wireMock;
51+
52+
@BeforeMethod
53+
public void start() {
54+
wireMockServer =
55+
new WireMockServer(wireMockConfig().notifier(new Slf4jNotifier(false)).dynamicPort());
56+
wireMockServer.start();
57+
WireMock.configureFor("localhost", wireMockServer.port());
58+
wireMock = new WireMock("localhost", wireMockServer.port());
59+
}
60+
61+
@AfterMethod
62+
void stop() {
63+
wireMockServer.stop();
64+
}
3365

3466
@Test
3567
public void shouldExpandRelativePaths() throws Exception {
36-
URLFetcher urlFetcher = Mockito.mock(URLFetcher.class);
3768
GitlabUrl gitlabUrl = new GitlabUrl().withHostName("gitlab.net").withSubGroups("eclipse/che");
3869
FileContentProvider fileContentProvider =
3970
new GitlabAuthorizingFileContentProvider(gitlabUrl, urlFetcher, personalAccessTokenManager);
@@ -49,7 +80,6 @@ public void shouldExpandRelativePaths() throws Exception {
4980

5081
@Test
5182
public void shouldPreserveAbsolutePaths() throws Exception {
52-
URLFetcher urlFetcher = Mockito.mock(URLFetcher.class);
5383
GitlabUrl gitlabUrl = new GitlabUrl().withHostName("gitlab.net").withSubGroups("eclipse/che");
5484
FileContentProvider fileContentProvider =
5585
new GitlabAuthorizingFileContentProvider(gitlabUrl, urlFetcher, personalAccessTokenManager);
@@ -65,18 +95,51 @@ public void shouldPreserveAbsolutePaths() throws Exception {
6595
@Test(expectedExceptions = FileNotFoundException.class)
6696
public void shouldThrowFileNotFoundException() throws Exception {
6797
// given
68-
URLFetcher urlFetcher = Mockito.mock(URLFetcher.class);
6998
when(urlFetcher.fetch(
7099
eq(
71-
"https://gitlab.com/api/v4/projects/eclipse%2Fche/repository/files/devfile.yaml/raw?ref=HEAD")))
100+
wireMockServer.url(
101+
"/api/v4/projects/eclipse%2Fche/repository/files/devfile.yaml/raw?ref=HEAD"))))
72102
.thenThrow(new FileNotFoundException());
73-
when(urlFetcher.fetch(eq("https://gitlab.com/eclipse/che"))).thenReturn("content");
74103
when(personalAccessTokenManager.getAndStore(anyString()))
75104
.thenThrow(new UnknownScmProviderException("", ""));
76-
GitlabUrl gitlabUrl = new GitlabUrl().withHostName("gitlab.com").withSubGroups("eclipse/che");
105+
URI uri = URI.create(wireMockServer.url("/"));
106+
GitlabUrl gitlabUrl =
107+
new GitlabUrl()
108+
.withScheme("http")
109+
.withHostName(format("%s:%s", uri.getHost(), uri.getPort()))
110+
.withSubGroups("eclipse/che");
77111
FileContentProvider fileContentProvider =
78112
new GitlabAuthorizingFileContentProvider(gitlabUrl, urlFetcher, personalAccessTokenManager);
79113

114+
stubFor(get(urlEqualTo("/eclipse/che")).willReturn(aResponse().withStatus(HTTP_OK)));
115+
116+
// when
117+
fileContentProvider.fetchContent("devfile.yaml");
118+
}
119+
120+
@Test(
121+
expectedExceptions = DevfileException.class,
122+
expectedExceptionsMessageRegExp = "Could not reach devfile at test path")
123+
public void shouldThrowDevfileException() throws Exception {
124+
// given
125+
when(urlFetcher.fetch(
126+
eq(
127+
wireMockServer.url(
128+
"/api/v4/projects/eclipse%2Fche/repository/files/devfile.yaml/raw?ref=HEAD"))))
129+
.thenThrow(new FileNotFoundException("test path"));
130+
when(personalAccessTokenManager.getAndStore(anyString()))
131+
.thenThrow(new UnknownScmProviderException("", ""));
132+
URI uri = URI.create(wireMockServer.url("/"));
133+
GitlabUrl gitlabUrl =
134+
new GitlabUrl()
135+
.withScheme("http")
136+
.withHostName(format("%s:%s", uri.getHost(), uri.getPort()))
137+
.withSubGroups("eclipse/che");
138+
FileContentProvider fileContentProvider =
139+
new GitlabAuthorizingFileContentProvider(gitlabUrl, urlFetcher, personalAccessTokenManager);
140+
141+
stubFor(get(urlEqualTo("/eclipse/che")).willReturn(aResponse().withStatus(HTTP_MOVED_TEMP)));
142+
80143
// when
81144
fileContentProvider.fetchContent("devfile.yaml");
82145
}

0 commit comments

Comments
 (0)