From 9b4134cadac4891daaefa2bdd337e69168ba06b9 Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Thu, 25 Feb 2021 13:54:04 -0800 Subject: [PATCH] Allow for time skew in JWT authentication --- .../authorization/JWTTokenProvider.java | 16 ++++++-- .../authorization/JWTTokenProviderTest.java | 40 +++++++++++++++++++ .../testIssuedAtSkew/mappings/app-2.json | 37 +++++++++++++++++ 3 files changed, 89 insertions(+), 4 deletions(-) create mode 100644 src/test/resources/org/kohsuke/github/extras/authorization/JWTTokenProviderTest/wiremock/testIssuedAtSkew/mappings/app-2.json diff --git a/src/main/java/org/kohsuke/github/extras/authorization/JWTTokenProvider.java b/src/main/java/org/kohsuke/github/extras/authorization/JWTTokenProvider.java index 90013fe09..1643b65d7 100644 --- a/src/main/java/org/kohsuke/github/extras/authorization/JWTTokenProvider.java +++ b/src/main/java/org/kohsuke/github/extras/authorization/JWTTokenProvider.java @@ -103,20 +103,28 @@ public class JWTTokenProvider implements AuthorizationProvider { private String refreshJWT() { Instant now = Instant.now(); - // Token expires in 10 minutes - Instant expiration = Instant.now().plus(Duration.ofMinutes(10)); + // Max token expiration is 10 minutes for GitHub + // We use a smaller window since we likely will not need more than a few seconds + Instant expiration = now.plus(Duration.ofMinutes(8)); + + // Setting the issued at to a time in the past to allow for clock skew + Instant issuedAt = getIssuedAt(now); // Let's set the JWT Claims JwtBuilder builder = Jwts.builder() - .setIssuedAt(Date.from(now)) + .setIssuedAt(Date.from(issuedAt)) .setExpiration(Date.from(expiration)) .setIssuer(this.applicationId) .signWith(privateKey, SignatureAlgorithm.RS256); - // Token will refresh after 8 minutes + // Token will refresh 2 minutes before it expires validUntil = expiration.minus(Duration.ofMinutes(2)); // Builds the JWT and serializes it to a compact, URL-safe string return builder.compact(); } + + Instant getIssuedAt(Instant now) { + return now.minus(Duration.ofMinutes(2)); + } } diff --git a/src/test/java/org/kohsuke/github/extras/authorization/JWTTokenProviderTest.java b/src/test/java/org/kohsuke/github/extras/authorization/JWTTokenProviderTest.java index 8c55558f9..351892c4d 100644 --- a/src/test/java/org/kohsuke/github/extras/authorization/JWTTokenProviderTest.java +++ b/src/test/java/org/kohsuke/github/extras/authorization/JWTTokenProviderTest.java @@ -3,10 +3,16 @@ package org.kohsuke.github.extras.authorization; import org.junit.Test; import org.kohsuke.github.AbstractGitHubWireMockTest; import org.kohsuke.github.GitHub; +import org.kohsuke.github.HttpException; import java.io.File; import java.io.IOException; import java.security.GeneralSecurityException; +import java.time.Duration; +import java.time.Instant; + +import static org.hamcrest.Matchers.*; + /* * This test will request an application ensuring that the header for the "Authorization" matches a valid JWT token. * A JWT token in the Authorization header will always start with "ey" which is always the start of the base64 @@ -33,6 +39,9 @@ public class JWTTokenProviderTest extends AbstractGitHubWireMockTest { @Test public void testAuthorizationHeaderPattern() throws GeneralSecurityException, IOException { + // authorization header check is custom + snapshotNotAllowed(); + JWTTokenProvider jwtTokenProvider = new JWTTokenProvider(TEST_APP_ID_2, new File(this.getClass().getResource(PRIVATE_KEY_FILE_APP_2).getFile())); GitHub gh = getGitHubBuilder().withEndpoint(mockGitHub.apiServer().baseUrl()) @@ -44,4 +53,35 @@ public class JWTTokenProviderTest extends AbstractGitHubWireMockTest { gh.getApp(); } + @Test + public void testIssuedAtSkew() throws GeneralSecurityException, IOException { + // TODO: This isn't a great test as it doesn't really check anything in CI + // This test was accurate when recorded but it doesn't verify that the jwt token is different + // or accurate in anyway. + + JWTTokenProvider jwtTokenProvider = new JWTTokenProvider(TEST_APP_ID_2, + new File(this.getClass().getResource(PRIVATE_KEY_FILE_APP_2).getFile())) { + + @Override + Instant getIssuedAt(Instant now) { + return now.plus(Duration.ofMinutes(2)); + } + }; + GitHub gh = getGitHubBuilder().withEndpoint(mockGitHub.apiServer().baseUrl()) + .withAuthorizationProvider(jwtTokenProvider) + .build(); + + try { + // Request the application, the wiremock matcher will ensure that the header + // for the authorization is present and has a the format of a valid JWT token + gh.getApp(); + fail(); + } catch (HttpException e) { + assertThat(e.getResponseCode(), equalTo(401)); + assertThat(e.getMessage(), + containsString( + "'Issued at' claim ('iat') must be an Integer representing the time that the assertion was issued")); + } + } + } diff --git a/src/test/resources/org/kohsuke/github/extras/authorization/JWTTokenProviderTest/wiremock/testIssuedAtSkew/mappings/app-2.json b/src/test/resources/org/kohsuke/github/extras/authorization/JWTTokenProviderTest/wiremock/testIssuedAtSkew/mappings/app-2.json new file mode 100644 index 000000000..44020edc9 --- /dev/null +++ b/src/test/resources/org/kohsuke/github/extras/authorization/JWTTokenProviderTest/wiremock/testIssuedAtSkew/mappings/app-2.json @@ -0,0 +1,37 @@ +{ + "id": "70456278-27d2-470a-bdfb-0edb4ac61781", + "name": "app", + "request": { + "url": "/app", + "method": "GET", + "headers": { + "Authorization": { + "matches": "^Bearer (?ey\\S*)\\.(?\\S*)\\.(?\\S*)$" + }, + "Accept": { + "equalTo": "application/vnd.github.machine-man-preview+json" + } + } + }, + "response": { + "status": 401, + "body": "{\"message\":\"'Issued at' claim ('iat') must be an Integer representing the time that the assertion was issued\",\"documentation_url\":\"https://docs.github.com/rest\"}", + "headers": { + "Server": "GitHub.com", + "Date": "Thu, 25 Feb 2021 18:29:11 GMT", + "Content-Type": "application/json; charset=utf-8", + "X-GitHub-Media-Type": "github.v3; param=machine-man-preview; format=json", + "Strict-Transport-Security": "max-age=31536000; includeSubdomains; preload", + "X-Frame-Options": "deny", + "X-Content-Type-Options": "nosniff", + "X-XSS-Protection": "1; mode=block", + "Referrer-Policy": "origin-when-cross-origin, strict-origin-when-cross-origin", + "Content-Security-Policy": "default-src 'none'", + "Vary": "Accept-Encoding, Accept, X-Requested-With", + "X-GitHub-Request-Id": "F443:2D43:8AECD5:A2A02D:6037EC76" + } + }, + "uuid": "70456278-27d2-470a-bdfb-0edb4ac61781", + "persistent": true, + "insertionIndex": 2 +} \ No newline at end of file