Merge pull request #1039 from bitwiseman/task/jwt

Allow for time skew in JWT authentication

src/main/java/org/kohsuke/github/extras/authorization/JWTTokenProvider.java

@@ -103,20 +103,28 @@ private static PrivateKey getPrivateKeyFromString(final String key) throws Gener
     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));
+    }
 }

src/test/java/org/kohsuke/github/extras/authorization/JWTTokenProviderTest.java

@@ -3,10 +3,16 @@
 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 void testAuthorizationHeaderPattern() throws GeneralSecurityException, IO
         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"));
+        }
+    }
+
 }

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 (?<JWTHeader>ey\\S*)\\.(?<JWTPayload>\\S*)\\.(?<JWTSignature>\\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
+}