Skip to content

Commit fbfa88a

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 b99673b commit fbfa88a

File tree

2 files changed

+125
-6
lines changed

2 files changed

+125
-6
lines changed

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

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -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.getHostName() + '/' + 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: 87 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,27 +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;
29+
import java.io.FileNotFoundException;
30+
import java.net.URI;
1831
import org.eclipse.che.api.factory.server.scm.PersonalAccessToken;
1932
import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager;
33+
import org.eclipse.che.api.factory.server.scm.exception.UnknownScmProviderException;
2034
import org.eclipse.che.api.workspace.server.devfile.FileContentProvider;
2135
import org.eclipse.che.api.workspace.server.devfile.URLFetcher;
36+
import org.eclipse.che.api.workspace.server.devfile.exception.DevfileException;
2237
import org.mockito.Mock;
23-
import org.mockito.Mockito;
2438
import org.mockito.testng.MockitoTestNGListener;
39+
import org.testng.annotations.AfterMethod;
40+
import org.testng.annotations.BeforeMethod;
2541
import org.testng.annotations.Listeners;
2642
import org.testng.annotations.Test;
2743

2844
@Listeners(MockitoTestNGListener.class)
2945
public class GitlabAuthorizingFileContentProviderTest {
3046
@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+
}
3165

3266
@Test
3367
public void shouldExpandRelativePaths() throws Exception {
34-
URLFetcher urlFetcher = Mockito.mock(URLFetcher.class);
3568
GitlabUrl gitlabUrl = new GitlabUrl().withHostName("gitlab.net").withSubGroups("eclipse/che");
3669
FileContentProvider fileContentProvider =
3770
new GitlabAuthorizingFileContentProvider(gitlabUrl, urlFetcher, personalAccessTokenManager);
@@ -47,7 +80,6 @@ public void shouldExpandRelativePaths() throws Exception {
4780

4881
@Test
4982
public void shouldPreserveAbsolutePaths() throws Exception {
50-
URLFetcher urlFetcher = Mockito.mock(URLFetcher.class);
5183
GitlabUrl gitlabUrl = new GitlabUrl().withHostName("gitlab.net").withSubGroups("eclipse/che");
5284
FileContentProvider fileContentProvider =
5385
new GitlabAuthorizingFileContentProvider(gitlabUrl, urlFetcher, personalAccessTokenManager);
@@ -59,4 +91,56 @@ public void shouldPreserveAbsolutePaths() throws Exception {
5991
fileContentProvider.fetchContent(url);
6092
verify(urlFetcher).fetch(eq(url), eq("Bearer my-token"));
6193
}
94+
95+
@Test(expectedExceptions = FileNotFoundException.class)
96+
public void shouldThrowFileNotFoundException() throws Exception {
97+
// given
98+
when(urlFetcher.fetch(
99+
eq(
100+
wireMockServer.url(
101+
"/api/v4/projects/eclipse%2Fche/repository/files/devfile.yaml/raw?ref=HEAD"))))
102+
.thenThrow(new FileNotFoundException());
103+
when(personalAccessTokenManager.getAndStore(anyString()))
104+
.thenThrow(new UnknownScmProviderException("", ""));
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");
111+
FileContentProvider fileContentProvider =
112+
new GitlabAuthorizingFileContentProvider(gitlabUrl, urlFetcher, personalAccessTokenManager);
113+
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+
143+
// when
144+
fileContentProvider.fetchContent("devfile.yaml");
145+
}
62146
}

0 commit comments

Comments
 (0)