Pass status code to Exception on Unauthorised to GitHub Error (#618)
Pass status code to Exception on Unauthorised to GitHub Error in order to be able to recognise GitHub Server url if oAuth is not configured and no PAT is present.pull/638/head
parent
d077bdd93d
commit
06e14c854d
|
|
@ -1,5 +1,5 @@
|
|||
/*
|
||||
* Copyright (c) 2012-2023 Red Hat, Inc.
|
||||
* Copyright (c) 2012-2024 Red Hat, Inc.
|
||||
* This program and the accompanying materials are made
|
||||
* available under the terms of the Eclipse Public License 2.0
|
||||
* which is available at https://www.eclipse.org/legal/epl-2.0/
|
||||
|
|
@ -120,13 +120,18 @@ public abstract class AbstractGithubURLParser {
|
|||
private boolean isApiRequestRelevant(String repositoryUrl) {
|
||||
Optional<String> serverUrlOptional = getServerUrl(repositoryUrl);
|
||||
if (serverUrlOptional.isPresent()) {
|
||||
GithubApiClient GithubApiClient = new GithubApiClient(serverUrlOptional.get());
|
||||
GithubApiClient githubApiClient = new GithubApiClient(serverUrlOptional.get());
|
||||
try {
|
||||
// If the user request catches the unauthorised error, it means that the provided url
|
||||
// belongs to GitHub.
|
||||
GithubApiClient.getUser("");
|
||||
githubApiClient.getUser("");
|
||||
} catch (ScmCommunicationException e) {
|
||||
return e.getStatusCode() == HTTP_UNAUTHORIZED;
|
||||
return e.getStatusCode() == HTTP_UNAUTHORIZED
|
||||
// Check the error message as well, because other providers might also return 401
|
||||
// for such requests.
|
||||
&& e.getMessage().contains("Requires authentication")
|
||||
|| // for older GitHub Enterprise versions
|
||||
e.getMessage().contains("Must authenticate to access this API.");
|
||||
} catch (ScmItemNotFoundException | ScmBadRequestException | IllegalArgumentException e) {
|
||||
return false;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -269,24 +269,25 @@ public class GithubApiClient {
|
|||
try {
|
||||
HttpResponse<InputStream> response =
|
||||
httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
|
||||
LOG.trace("executeRequest={} response {}", request, response.statusCode());
|
||||
if (response.statusCode() == HTTP_OK) {
|
||||
int statusCode = response.statusCode();
|
||||
LOG.trace("executeRequest={} response {}", request, statusCode);
|
||||
if (statusCode == HTTP_OK) {
|
||||
return responseConverter.apply(response);
|
||||
} else if (response.statusCode() == HTTP_NO_CONTENT) {
|
||||
} else if (statusCode == HTTP_NO_CONTENT) {
|
||||
return null;
|
||||
} else {
|
||||
String body =
|
||||
response.body() == null
|
||||
? "Unrecognised error"
|
||||
: CharStreams.toString(new InputStreamReader(response.body(), Charsets.UTF_8));
|
||||
switch (response.statusCode()) {
|
||||
switch (statusCode) {
|
||||
case HTTP_BAD_REQUEST:
|
||||
throw new ScmBadRequestException(body);
|
||||
case HTTP_NOT_FOUND:
|
||||
throw new ScmItemNotFoundException(body);
|
||||
default:
|
||||
throw new ScmCommunicationException(
|
||||
"Unexpected status code " + response.statusCode() + " " + response.toString());
|
||||
"Unexpected status code " + statusCode + " " + body, statusCode);
|
||||
}
|
||||
}
|
||||
} catch (IOException | InterruptedException | UncheckedIOException e) {
|
||||
|
|
|
|||
|
|
@ -285,8 +285,7 @@ public class GithubApiClientTest {
|
|||
|
||||
@Test(
|
||||
expectedExceptions = ScmCommunicationException.class,
|
||||
expectedExceptionsMessageRegExp =
|
||||
"Unexpected status code 502 \\(GET http://localhost:\\d*/api/v3/user\\) 502")
|
||||
expectedExceptionsMessageRegExp = "Unexpected status code 502 item not found")
|
||||
public void shouldThrowExceptionOnOtherError() throws Exception {
|
||||
// given
|
||||
stubFor(
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
/*
|
||||
* Copyright (c) 2012-2023 Red Hat, Inc.
|
||||
* Copyright (c) 2012-2024 Red Hat, Inc.
|
||||
* This program and the accompanying materials are made
|
||||
* available under the terms of the Eclipse Public License 2.0
|
||||
* which is available at https://www.eclipse.org/legal/epl-2.0/
|
||||
|
|
@ -11,6 +11,12 @@
|
|||
*/
|
||||
package org.eclipse.che.api.factory.server.github;
|
||||
|
||||
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.net.HttpURLConnection.HTTP_UNAUTHORIZED;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.ArgumentMatchers.anyString;
|
||||
import static org.mockito.ArgumentMatchers.eq;
|
||||
|
|
@ -22,6 +28,9 @@ import static org.mockito.Mockito.when;
|
|||
import static org.testng.Assert.assertEquals;
|
||||
import static org.testng.Assert.assertTrue;
|
||||
|
||||
import com.github.tomakehurst.wiremock.WireMockServer;
|
||||
import com.github.tomakehurst.wiremock.client.WireMock;
|
||||
import com.github.tomakehurst.wiremock.common.Slf4jNotifier;
|
||||
import java.util.Optional;
|
||||
import org.eclipse.che.api.core.ApiException;
|
||||
import org.eclipse.che.api.factory.server.scm.PersonalAccessToken;
|
||||
|
|
@ -49,9 +58,17 @@ public class GithubURLParserTest {
|
|||
/** Instance of component that will be tested. */
|
||||
private GithubURLParser githubUrlParser;
|
||||
|
||||
WireMockServer wireMockServer;
|
||||
WireMock wireMock;
|
||||
|
||||
/** Setup objects/ */
|
||||
@BeforeMethod
|
||||
protected void start() throws ApiException {
|
||||
wireMockServer =
|
||||
new WireMockServer(wireMockConfig().notifier(new Slf4jNotifier(false)).dynamicPort());
|
||||
wireMockServer.start();
|
||||
WireMock.configureFor("localhost", wireMockServer.port());
|
||||
wireMock = new WireMock("localhost", wireMockServer.port());
|
||||
this.personalAccessTokenManager = mock(PersonalAccessTokenManager.class);
|
||||
this.githubApiClient = mock(GithubApiClient.class);
|
||||
|
||||
|
|
@ -315,4 +332,40 @@ public class GithubURLParserTest {
|
|||
verify(personalAccessTokenManager, times(2))
|
||||
.get(any(Subject.class), eq("https://github-server.com"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldValidateOldVersionGitHubServerUrl() throws Exception {
|
||||
// given
|
||||
String url = wireMockServer.url("/user/repo");
|
||||
stubFor(
|
||||
get(urlEqualTo("/api/v3/user"))
|
||||
.willReturn(
|
||||
aResponse()
|
||||
.withStatus(HTTP_UNAUTHORIZED)
|
||||
.withBody("{\"message\": \"Must authenticate to access this API.\",\n}")));
|
||||
|
||||
// when
|
||||
boolean valid = githubUrlParser.isValid(url);
|
||||
|
||||
// then
|
||||
assertTrue(valid);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldValidateGitHubServerUrl() throws Exception {
|
||||
// given
|
||||
String url = wireMockServer.url("/user/repo");
|
||||
stubFor(
|
||||
get(urlEqualTo("/api/v3/user"))
|
||||
.willReturn(
|
||||
aResponse()
|
||||
.withStatus(HTTP_UNAUTHORIZED)
|
||||
.withBody("{\"message\": \"Requires authentication\",\n}")));
|
||||
|
||||
// when
|
||||
boolean valid = githubUrlParser.isValid(url);
|
||||
|
||||
// then
|
||||
assertTrue(valid);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue