Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to newer version of jjwt library #1727

Merged
merged 11 commits into from
Nov 9, 2023
53 changes: 49 additions & 4 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
<jacoco.coverage.target.class.method>0.50</jacoco.coverage.target.class.method>
<!-- For non-ci builds we'd like the build to still complete if jacoco metrics aren't met. -->
<jacoco.haltOnFailure>false</jacoco.haltOnFailure>
<jjwt.suite.version>0.11.5</jjwt.suite.version>
<jjwt.suite.version>0.12.3</jjwt.suite.version>

<jacoco.surefire.argLine />
<surefire.argLine />
Expand Down Expand Up @@ -83,7 +83,7 @@
<plugins>
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<version>2.22.2</version>
<version>3.2.2</version>
<configuration>
<!-- SUREFIRE-1226 workaround -->
<trimStackTrace>false</trimStackTrace>
Expand Down Expand Up @@ -458,6 +458,18 @@
</plugins>
</build>

<dependencyManagement>
<dependencies>
<dependency>
<groupId>com.fasterxml.jackson</groupId>
<artifactId>jackson-bom</artifactId>
<version>2.15.3</version>
<scope>import</scope>
<type>pom</type>
</dependency>
</dependencies>
</dependencyManagement>

<dependencies>
<dependency>
<groupId>org.apache.commons</groupId>
Expand Down Expand Up @@ -510,7 +522,6 @@
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<version>2.15.2</version>
</dependency>
<dependency>
<groupId>commons-io</groupId>
Expand Down Expand Up @@ -652,7 +663,7 @@
<profiles>
<!-- only enable slow-or-flaky-test if -Dtest= is not present -->
<profile>
<id>test-slow-multireleasejar-flaky</id>
<id>test-jwt-slow-multireleasejar-flaky</id>
<activation>
<property>
<name>!test</name>
Expand Down Expand Up @@ -715,6 +726,40 @@
<includesFile>src/test/resources/slow-or-flaky-tests.txt</includesFile>
</configuration>
</execution>
<execution>
<!-- Verify that JWT suite 0.11.x still works -->
<id>jwt0.11.x-test</id>
<phase>integration-test</phase>
<goals>
<goal>test</goal>
</goals>
<configuration>
<classesDirectory>${project.basedir}/target/github-api-${project.version}.jar</classesDirectory>
<useSystemClassLoader>false</useSystemClassLoader>
<excludesFile>src/test/resources/slow-or-flaky-tests.txt</excludesFile>
<argLine>@{jacoco.surefire.argLine} ${surefire.argLine} -Dtest.github.connector=okhttp</argLine>
<classpathDependencyExcludes>
<classpathDependencyExclude>io.jsonwebtoken:*</classpathDependencyExclude>
</classpathDependencyExcludes>
<additionalClasspathDependencies>
<additionalClasspathDependency>
<groupId>io.jsonwebtoken</groupId>
<artifactId>jjwt-api</artifactId>
<version>0.11.5</version>
</additionalClasspathDependency>
<additionalClasspathDependency>
<groupId>io.jsonwebtoken</groupId>
<artifactId>jjwt-impl</artifactId>
<version>0.11.5</version>
</additionalClasspathDependency>
<additionalClasspathDependency>
<groupId>io.jsonwebtoken</groupId>
<artifactId>jjwt-jackson</artifactId>
<version>0.11.5</version>
</additionalClasspathDependency>
</additionalClasspathDependencies>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
package org.kohsuke.github.extras.authorization;

import io.jsonwebtoken.JwtBuilder;
import io.jsonwebtoken.Jwts;
import io.jsonwebtoken.SignatureAlgorithm;
import io.jsonwebtoken.jackson.io.JacksonSerializer;
import org.kohsuke.github.authorization.AuthorizationProvider;

import java.io.File;
Expand All @@ -19,7 +15,6 @@
import java.time.Duration;
import java.time.Instant;
import java.util.Base64;
import java.util.Date;

import javax.annotation.Nonnull;

Expand Down Expand Up @@ -171,18 +166,10 @@ private String refreshJWT() {
// 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(issuedAt))
.setExpiration(Date.from(expiration))
.setIssuer(this.applicationId)
.signWith(privateKey, SignatureAlgorithm.RS256);

// 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.serializeToJsonWith(new JacksonSerializer<>()).compact();
return JwtBuilderUtil.buildJwt(issuedAt, expiration, applicationId, privateKey);
}

Instant getIssuedAt(Instant now) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
package org.kohsuke.github.extras.authorization;

import io.jsonwebtoken.JwtBuilder;
import io.jsonwebtoken.Jwts;
import io.jsonwebtoken.io.Serializer;
import io.jsonwebtoken.jackson.io.JacksonSerializer;
import io.jsonwebtoken.security.SignatureAlgorithm;
import org.kohsuke.github.GHException;

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.security.Key;
import java.security.PrivateKey;
import java.time.Instant;
import java.util.Date;
import java.util.logging.Logger;

/**
* This is a util to build a JWT.
*
* <p>
* This class is used to build a JWT using the jjwt library. It uses reflection to support older versions of jjwt. The
* class may be removed once we are sure we no longer need to support pre-0.12.x versions of jjwt.
* </p>
*/
final class JwtBuilderUtil {

Check warning on line 26 in src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java#L26

Added line #L26 was not covered by tests

private static final Logger LOGGER = Logger.getLogger(JwtBuilderUtil.class.getName());

private static IJwtBuilder builder;

/**
* Build a JWT.
*
* @param issuedAt
* issued at
* @param expiration
* expiration
* @param applicationId
* application id
* @param privateKey
* private key
* @return JWT
*/
static String buildJwt(Instant issuedAt, Instant expiration, String applicationId, PrivateKey privateKey) {
if (builder == null) {
createBuilderImpl(issuedAt, expiration, applicationId, privateKey);
}
return builder.buildJwt(issuedAt, expiration, applicationId, privateKey);
}

private static void createBuilderImpl(Instant issuedAt,
Instant expiration,
String applicationId,
PrivateKey privateKey) {
// Figure out which builder to use and cache it. We don't worry about thread safety here because we're fine if
// the builder is assigned multiple times. The end result will be the same.
try {
builder = new DefaultBuilderImpl();
} catch (NoSuchMethodError | NoClassDefFoundError e) {
LOGGER.warning(
"You are using an outdated version of the io.jsonwebtoken:jjwt-* suite. v0.12.x or later is recommended.");

try {
ReflectionBuilderImpl reflectionBuider = new ReflectionBuilderImpl();
// Build a JWT to eagerly check for any reflection errors.
reflectionBuider.buildJwtWithReflection(issuedAt, expiration, applicationId, privateKey);

builder = reflectionBuider;
} catch (ReflectiveOperationException re) {
throw new GHException(

Check warning on line 71 in src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java#L70-L71

Added lines #L70 - L71 were not covered by tests
"Could not build JWT using reflection on io.jsonwebtoken:jjwt-* suite."
+ "The minimum supported version is v0.11.x, v0.12.x or later is recommended.",
re);
}
}
}

/**
* IJwtBuilder interface to isolate loading of JWT classes allowing us to catch and handle linkage errors.
*/
interface IJwtBuilder {
/**
* Build a JWT.
*
* @param issuedAt
* issued at
* @param expiration
* expiration
* @param applicationId
* application id
* @param privateKey
* private key
* @return JWT
*/
String buildJwt(Instant issuedAt, Instant expiration, String applicationId, PrivateKey privateKey);
}

/**
* A class to isolate loading of JWT classes allowing us to catch and handle linkage errors.
*
* Without this class, JwtBuilderUtil.buildJwt() immediately throws NoClassDefFoundError when called. With this
* class the error is thrown when DefaultBuilder.build() is called allowing us to catch and handle it.
*/
private static class DefaultBuilderImpl implements IJwtBuilder {
/**
* This method builds a JWT using 0.12.x or later versions of jjwt library
*
* @param issuedAt
* issued at
* @param expiration
* expiration
* @param applicationId
* application id
* @param privateKey
* private key
* @return JWT
*/
public String buildJwt(Instant issuedAt, Instant expiration, String applicationId, PrivateKey privateKey) {

// io.jsonwebtoken.security.SignatureAlgorithm is not present in v0.11.x and below.
// Trying to call a method that uses it causes "NoClassDefFoundError" if v0.11.x is being used.
SignatureAlgorithm rs256 = Jwts.SIG.RS256;

JwtBuilder jwtBuilder = Jwts.builder();
jwtBuilder = jwtBuilder.issuedAt(Date.from(issuedAt))
.expiration(Date.from(expiration))
.issuer(applicationId)
.signWith(privateKey, rs256)
.json(new JacksonSerializer<>());
return jwtBuilder.compact();
}
}

/**
* A class to encapsulate building a JWT using reflection.
*/
private static class ReflectionBuilderImpl implements IJwtBuilder {

private Method setIssuedAtMethod;
private Method setExpirationMethod;
private Method setIssuerMethod;
private Enum<?> rs256SignatureAlgorithm;
private Method signWithMethod;
private Method serializeToJsonMethod;

ReflectionBuilderImpl() throws ReflectiveOperationException {
JwtBuilder jwtBuilder = Jwts.builder();
Class<?> jwtReflectionClass = jwtBuilder.getClass();

setIssuedAtMethod = jwtReflectionClass.getMethod("setIssuedAt", Date.class);
setIssuerMethod = jwtReflectionClass.getMethod("setIssuer", String.class);
setExpirationMethod = jwtReflectionClass.getMethod("setExpiration", Date.class);
Class<?> signatureAlgorithmClass = Class.forName("io.jsonwebtoken.SignatureAlgorithm");
rs256SignatureAlgorithm = createEnumInstance(signatureAlgorithmClass, "RS256");
signWithMethod = jwtReflectionClass.getMethod("signWith", Key.class, signatureAlgorithmClass);
serializeToJsonMethod = jwtReflectionClass.getMethod("serializeToJsonWith", Serializer.class);
}

/**
* This method builds a JWT using older (pre 0.12.x) versions of jjwt library by leveraging reflection.
*
* @param issuedAt
* issued at
* @param expiration
* expiration
* @param applicationId
* application id
* @param privateKey
* private key
* @return JWTBuilder
*/
public String buildJwt(Instant issuedAt, Instant expiration, String applicationId, PrivateKey privateKey) {

try {
return buildJwtWithReflection(issuedAt, expiration, applicationId, privateKey);
} catch (ReflectiveOperationException e) {

Check warning on line 177 in src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java#L177

Added line #L177 was not covered by tests
// This should never happen. Reflection errors should have been caught during initialization.
throw new GHException("Reflection errors during JWT creation should have been checked already.", e);

Check warning on line 179 in src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java#L179

Added line #L179 was not covered by tests
}
}

private String buildJwtWithReflection(Instant issuedAt,
Instant expiration,
String applicationId,
PrivateKey privateKey) throws IllegalAccessException, InvocationTargetException {
JwtBuilder jwtBuilder = Jwts.builder();
Object builderObj = jwtBuilder;
builderObj = setIssuedAtMethod.invoke(builderObj, Date.from(issuedAt));
builderObj = setExpirationMethod.invoke(builderObj, Date.from(expiration));
builderObj = setIssuerMethod.invoke(builderObj, applicationId);
builderObj = signWithMethod.invoke(builderObj, privateKey, rs256SignatureAlgorithm);
builderObj = serializeToJsonMethod.invoke(builderObj, new JacksonSerializer<>());
return ((JwtBuilder) builderObj).compact();
}

@SuppressWarnings("unchecked")
private static <T extends Enum<T>> T createEnumInstance(Class<?> type, String name) {
return Enum.valueOf((Class<T>) type, name);
}
}
}
1 change: 1 addition & 0 deletions src/test/resources/slow-or-flaky-tests.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
**/extras/**
**/GHRateLimitTest
**/GHPullRequestTest
**/RequesterRetryTest
**/RateLimitCheckerTest
**/RateLimitHandlerTest