Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
<guava.version>28.0-jre</guava.version>
<jsr305.version>3.0.2</jsr305.version>
<jsr311-api.version>1.1.1</jsr311-api.version>
<junit.version>4.13</junit.version>
<junit.version>4.13.1</junit.version>
<slf4j-api.version>1.7.12</slf4j-api.version>
<mockito-core.version>3.2.4</mockito-core.version>
<commons-io.version>2.6</commons-io.version>
Expand Down
14 changes: 12 additions & 2 deletions src/main/java/com/spotify/github/v3/clients/GitHubClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,9 @@ private Request.Builder requestBuilder(final String path) {
(3) Installation Token, generated from the JWT token. Also used in Github Apps.
*/
private String getAuthorizationHeader(final String path) {
if (isJwtRequest(path) && getPrivateKey().isEmpty()) {
throw new IllegalStateException("This endpoint needs a client with a private key for an App");
}
if (getAccessToken().isPresent()) {
return String.format("token %s", token);
} else if (getPrivateKey().isPresent()) {
Expand Down Expand Up @@ -603,12 +606,19 @@ private AccessToken generateInstallationToken(final String jwtToken, final int i

final Response response = client.newCall(request).execute();

if (response.body() == null) {
if (!response.isSuccessful()) {
throw new Exception(
String.format(
"Got status %s when getting an access token from GitHub: %s",
"Got non-2xx status %s when getting an access token from GitHub: %s",
response.code(), response.message()));
}

if (response.body() == null) {
throw new Exception(
String.format(
"Got empty response body when getting an access token from GitHub, HTTP status was: %s",
response.message()));
}
final String text = response.body().string();
response.body().close();
return Json.create().fromJson(text, AccessToken.class);
Expand Down
44 changes: 36 additions & 8 deletions src/test/java/com/spotify/github/v3/clients/GitHubAuthTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,12 @@

import static com.spotify.github.v3.clients.ChecksClientTest.loadResource;
import static org.hamcrest.CoreMatchers.hasItem;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.CoreMatchers.startsWith;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertThrows;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.spotify.github.jackson.Json;
Expand All @@ -33,8 +36,10 @@
import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.time.Duration;
import java.time.ZonedDateTime;
import java.util.Objects;
import java.util.concurrent.TimeUnit;
import okhttp3.OkHttpClient;
import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
Expand Down Expand Up @@ -72,7 +77,12 @@ public GitHubAuthTest() throws JsonProcessingException {}

@Before
public void setUp() throws IOException {
client = new OkHttpClient();
client =
new OkHttpClient.Builder()
.connectTimeout(Duration.ofSeconds(1))
.readTimeout(Duration.ofSeconds(1))
.build();

mockServer.start();
url = mockServer.url("/").uri();
checksClient =
Expand Down Expand Up @@ -139,10 +149,20 @@ public void fetchesANewInstallationTokenIfExpired() throws Exception {
assertThat(mockServer.takeRequest().getPath(), is("/repos/foo/bar/check-runs/123"));
}

@Test(expected = Exception.class)
public void throwsIfFetchingInstallationTokenRequestIsUnsuccessful() {
@Test
public void throwsIfFetchingInstallationTokenRequestIsUnsuccessful() throws Exception {
mockServer.enqueue(new MockResponse().setResponseCode(500));
checksClient.getCheckRun(123).join();
RuntimeException ex =
assertThrows(RuntimeException.class, () -> checksClient.getCheckRun(123).join());

assertThat(ex.getMessage(), is("Could not generate access token for github app"));

assertThat(ex.getCause(), is(notNullValue()));
assertThat(ex.getCause().getMessage(), startsWith("Got non-2xx status 500 when getting an access token from GitHub"));

RecordedRequest recordedRequest = mockServer.takeRequest(1, TimeUnit.MILLISECONDS);
// make sure it was the expected request that threw
assertThat(recordedRequest.getRequestUrl().encodedPath(), is("/app/installations/1/access_tokens"));
}

@Test
Expand Down Expand Up @@ -178,16 +198,24 @@ public void assertChecksApiContainsCorrectHeader() throws Exception {
assertThat(request2.getMethod(), is("PATCH"));
}

@Test(expected = Exception.class)
@Test
public void assertInstallationEndpointWithoutInstallationThrows() {
final GitHubClient github = GitHubClient.create(client, url, key, 123);
github.createRepositoryClient("foo", "bar").createChecksApiClient().getCheckRun(123).join();
final RuntimeException ex = assertThrows(RuntimeException.class,
() -> github.createRepositoryClient("foo", "bar").createChecksApiClient().getCheckRun(123)
.join());
assertThat(ex.getMessage(), is("This endpoint needs a client with an installation ID"));
}

@Test(expected = Exception.class)
@Test
public void assertJwtEndpointWithNoKeyThrows() {
final GitHubClient github = GitHubClient.create(client, url, "a-token");
github.createRepositoryClient("foo", "bar").createGithubAppClient().getInstallations().join();

final IllegalStateException ex = assertThrows(IllegalStateException.class,
() -> github.createRepositoryClient("foo", "bar").createGithubAppClient().getInstallations()
.join());

assertThat(ex.getMessage(), is("This endpoint needs a client with a private key for an App"));
}

@Test
Expand Down