Skip to content

Commit bda3855

Browse files
bitwisemanbitwiseman
authored
Merge pull request hub4j#1039 from bitwiseman/task/jwt
Allow for time skew in JWT authentication
2 parents d2732bc + 9b4134c commit bda3855

3 files changed

Lines changed: 89 additions & 4 deletions

File tree

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,20 +103,28 @@ private static PrivateKey getPrivateKeyFromString(final String key) throws Gener
103103
private String refreshJWT() {
104104
Instant now = Instant.now();
105105

106-
// Token expires in 10 minutes
107-
Instant expiration = Instant.now().plus(Duration.ofMinutes(10));
106+
// Max token expiration is 10 minutes for GitHub
107+
// We use a smaller window since we likely will not need more than a few seconds
108+
Instant expiration = now.plus(Duration.ofMinutes(8));
109+
110+
// Setting the issued at to a time in the past to allow for clock skew
111+
Instant issuedAt = getIssuedAt(now);
108112

109113
// Let's set the JWT Claims
110114
JwtBuilder builder = Jwts.builder()
111-
.setIssuedAt(Date.from(now))
115+
.setIssuedAt(Date.from(issuedAt))
112116
.setExpiration(Date.from(expiration))
113117
.setIssuer(this.applicationId)
114118
.signWith(privateKey, SignatureAlgorithm.RS256);
115119

116-
// Token will refresh after 8 minutes
120+
// Token will refresh 2 minutes before it expires
117121
validUntil = expiration.minus(Duration.ofMinutes(2));
118122

119123
// Builds the JWT and serializes it to a compact, URL-safe string
120124
return builder.compact();
121125
}
126+
127+
Instant getIssuedAt(Instant now) {
128+
return now.minus(Duration.ofMinutes(2));
129+
}
122130
}

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,16 @@
33
import org.junit.Test;
44
import org.kohsuke.github.AbstractGitHubWireMockTest;
55
import org.kohsuke.github.GitHub;
6+
import org.kohsuke.github.HttpException;
67

78
import java.io.File;
89
import java.io.IOException;
910
import java.security.GeneralSecurityException;
11+
import java.time.Duration;
12+
import java.time.Instant;
13+
14+
import static org.hamcrest.Matchers.*;
15+
1016
/*
1117
* This test will request an application ensuring that the header for the "Authorization" matches a valid JWT token.
1218
* 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 {
3339

3440
@Test
3541
public void testAuthorizationHeaderPattern() throws GeneralSecurityException, IOException {
42+
// authorization header check is custom
43+
snapshotNotAllowed();
44+
3645
JWTTokenProvider jwtTokenProvider = new JWTTokenProvider(TEST_APP_ID_2,
3746
new File(this.getClass().getResource(PRIVATE_KEY_FILE_APP_2).getFile()));
3847
GitHub gh = getGitHubBuilder().withEndpoint(mockGitHub.apiServer().baseUrl())
@@ -44,4 +53,35 @@ public void testAuthorizationHeaderPattern() throws GeneralSecurityException, IO
4453
gh.getApp();
4554
}
4655

56+
@Test
57+
public void testIssuedAtSkew() throws GeneralSecurityException, IOException {
58+
// TODO: This isn't a great test as it doesn't really check anything in CI
59+
// This test was accurate when recorded but it doesn't verify that the jwt token is different
60+
// or accurate in anyway.
61+
62+
JWTTokenProvider jwtTokenProvider = new JWTTokenProvider(TEST_APP_ID_2,
63+
new File(this.getClass().getResource(PRIVATE_KEY_FILE_APP_2).getFile())) {
64+
65+
@Override
66+
Instant getIssuedAt(Instant now) {
67+
return now.plus(Duration.ofMinutes(2));
68+
}
69+
};
70+
GitHub gh = getGitHubBuilder().withEndpoint(mockGitHub.apiServer().baseUrl())
71+
.withAuthorizationProvider(jwtTokenProvider)
72+
.build();
73+
74+
try {
75+
// Request the application, the wiremock matcher will ensure that the header
76+
// for the authorization is present and has a the format of a valid JWT token
77+
gh.getApp();
78+
fail();
79+
} catch (HttpException e) {
80+
assertThat(e.getResponseCode(), equalTo(401));
81+
assertThat(e.getMessage(),
82+
containsString(
83+
"'Issued at' claim ('iat') must be an Integer representing the time that the assertion was issued"));
84+
}
85+
}
86+
4787
}

‎src/test/resources/org/kohsuke/github/extras/authorization/JWTTokenProviderTest/wiremock/testIssuedAtSkew/mappings/app-2.json‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
{
2+
"id": "70456278-27d2-470a-bdfb-0edb4ac61781",
3+
"name": "app",
4+
"request": {
5+
"url": "/app",
6+
"method": "GET",
7+
"headers": {
8+
"Authorization": {
9+
"matches": "^Bearer (?<JWTHeader>ey\\S*)\\.(?<JWTPayload>\\S*)\\.(?<JWTSignature>\\S*)$"
10+
},
11+
"Accept": {
12+
"equalTo": "application/vnd.github.machine-man-preview+json"
13+
}
14+
}
15+
},
16+
"response": {
17+
"status": 401,
18+
"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\"}",
19+
"headers": {
20+
"Server": "GitHub.com",
21+
"Date": "Thu, 25 Feb 2021 18:29:11 GMT",
22+
"Content-Type": "application/json; charset=utf-8",
23+
"X-GitHub-Media-Type": "github.v3; param=machine-man-preview; format=json",
24+
"Strict-Transport-Security": "max-age=31536000; includeSubdomains; preload",
25+
"X-Frame-Options": "deny",
26+
"X-Content-Type-Options": "nosniff",
27+
"X-XSS-Protection": "1; mode=block",
28+
"Referrer-Policy": "origin-when-cross-origin, strict-origin-when-cross-origin",
29+
"Content-Security-Policy": "default-src 'none'",
30+
"Vary": "Accept-Encoding, Accept, X-Requested-With",
31+
"X-GitHub-Request-Id": "F443:2D43:8AECD5:A2A02D:6037EC76"
32+
}
33+
},
34+
"uuid": "70456278-27d2-470a-bdfb-0edb4ac61781",
35+
"persistent": true,
36+
"insertionIndex": 2
37+
}

0 commit comments

Comments
 (0)