From 4eec65913bfde5fe4eda1b9758f17157d1ef4ff1 Mon Sep 17 00:00:00 2001 From: D061587 Date: Mon, 16 Oct 2023 13:19:15 +0200 Subject: [PATCH 01/10] Adopt code to newer version of jjwt. Fixes: #1724 --- pom.xml | 2 +- .../github/extras/authorization/JWTTokenProvider.java | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/pom.xml b/pom.xml index 7a8b952a87..b7b412e40e 100644 --- a/pom.xml +++ b/pom.xml @@ -44,7 +44,7 @@ 0.50 false - 0.11.5 + 0.12.3 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 85ed1f69ed..71c05eb789 100644 --- a/src/main/java/org/kohsuke/github/extras/authorization/JWTTokenProvider.java +++ b/src/main/java/org/kohsuke/github/extras/authorization/JWTTokenProvider.java @@ -2,7 +2,6 @@ import io.jsonwebtoken.JwtBuilder; import io.jsonwebtoken.Jwts; -import io.jsonwebtoken.SignatureAlgorithm; import io.jsonwebtoken.jackson.io.JacksonSerializer; import org.kohsuke.github.authorization.AuthorizationProvider; @@ -173,16 +172,16 @@ private String refreshJWT() { // 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); + .issuedAt(Date.from(issuedAt)) + .expiration(Date.from(expiration)) + .issuer(this.applicationId) + .signWith(privateKey, Jwts.SIG.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 builder.json(new JacksonSerializer<>()).compact(); } Instant getIssuedAt(Instant now) { From c49b667d1edb4871e400c1cca1ef31eab176b3f4 Mon Sep 17 00:00:00 2001 From: D061587 Date: Mon, 6 Nov 2023 09:16:05 +0100 Subject: [PATCH 02/10] Use reflective access to be compatible with older versions of jjwt --- .../authorization/JWTTokenProvider.java | 18 +---- .../extras/authorization/JwtBuilderUtil.java | 73 +++++++++++++++++++ .../JwtReflectiveBuilderException.java | 7 ++ .../extras/authorization/RefreshResult.java | 21 ++++++ 4 files changed, 104 insertions(+), 15 deletions(-) create mode 100644 src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java create mode 100644 src/main/java/org/kohsuke/github/extras/authorization/JwtReflectiveBuilderException.java create mode 100644 src/main/java/org/kohsuke/github/extras/authorization/RefreshResult.java 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 71c05eb789..0bc8fe3df6 100644 --- a/src/main/java/org/kohsuke/github/extras/authorization/JWTTokenProvider.java +++ b/src/main/java/org/kohsuke/github/extras/authorization/JWTTokenProvider.java @@ -1,10 +1,5 @@ package org.kohsuke.github.extras.authorization; -import io.jsonwebtoken.JwtBuilder; -import io.jsonwebtoken.Jwts; -import io.jsonwebtoken.jackson.io.JacksonSerializer; -import org.kohsuke.github.authorization.AuthorizationProvider; - import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -18,10 +13,11 @@ import java.time.Duration; import java.time.Instant; import java.util.Base64; -import java.util.Date; import javax.annotation.Nonnull; +import org.kohsuke.github.authorization.AuthorizationProvider; + /** * A authorization provider that gives valid JWT tokens. These tokens are then used to create a time-based token to * authenticate as an application. This token provider does not provide any kind of caching, and will always request a @@ -170,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() - .issuedAt(Date.from(issuedAt)) - .expiration(Date.from(expiration)) - .issuer(this.applicationId) - .signWith(privateKey, Jwts.SIG.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.json(new JacksonSerializer<>()).compact(); + return JwtBuilderUtil.buildJwt(issuedAt, expiration, applicationId, privateKey); } Instant getIssuedAt(Instant now) { diff --git a/src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java b/src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java new file mode 100644 index 0000000000..5c88dc87b9 --- /dev/null +++ b/src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java @@ -0,0 +1,73 @@ +package org.kohsuke.github.extras.authorization; + +import java.lang.reflect.Method; +import java.security.PrivateKey; +import java.time.Instant; +import java.util.Date; + +import io.jsonwebtoken.JwtBuilder; +import io.jsonwebtoken.Jwts; +import io.jsonwebtoken.SignatureAlgorithm; +import io.jsonwebtoken.jackson.io.JacksonSerializer; + +final class JwtBuilderUtil { + private static Method getMethod(Object obj, String method, Class... params) throws NoSuchMethodException { + Class type = obj.getClass(); + return type.getMethod(method, params); + } + + private static boolean hasMethod(Object obj, String method, Class... params) { + try { + return JwtBuilderUtil.getMethod(obj, method, params) != null; + } catch (NoSuchMethodException e) { + return false; + } + } + + static String buildJwt(Instant issuedAt, Instant expiration, String applicationId, PrivateKey privateKey) { + JwtBuilder jwtBuilder = Jwts.builder(); + if (JwtBuilderUtil.hasMethod(jwtBuilder, "issuedAt", Date.class)) { + jwtBuilder = jwtBuilder.issuedAt(Date.from(issuedAt)) + .expiration(Date.from(expiration)) + .issuer(applicationId) + .signWith(privateKey, Jwts.SIG.RS256); + return jwtBuilder.json(new JacksonSerializer<>()).compact(); + } + + // older jjwt library versions + try { + return JwtBuilderUtil.buildByReflection(jwtBuilder, issuedAt, expiration, applicationId, privateKey); + } catch (ReflectiveOperationException e) { + throw new JwtReflectiveBuilderException( + "Exception building a JWT with reflective access to outdated versions of jjwt. Please consider an update.", + e); + } + } + + private static String buildByReflection(JwtBuilder jwtBuilder, Instant issuedAt, Instant expiration, + String applicationId, + PrivateKey privateKey) throws ReflectiveOperationException { + + Object builderObj = jwtBuilder; + + Method setIssuedAtMethod = JwtBuilderUtil.getMethod(builderObj, "setIssuedAt", Date.class); + builderObj = setIssuedAtMethod.invoke(builderObj, Date.from(issuedAt)); + + Method setExpirationMethod = JwtBuilderUtil.getMethod(builderObj, "setExpiration", Date.class); + builderObj = setExpirationMethod.invoke(builderObj, Date.from(expiration)); + + Method setIssuerMethod = JwtBuilderUtil.getMethod(builderObj, "setIssuer", String.class); + builderObj = setIssuerMethod.invoke(builderObj, applicationId); + + Method signWithMethod = JwtBuilderUtil.getMethod(builderObj, "signWith", PrivateKey.class, + SignatureAlgorithm.class); + builderObj = signWithMethod.invoke(builderObj, privateKey, SignatureAlgorithm.RS256); + + Method serializeToJsonMethod = JwtBuilderUtil.getMethod(builderObj, "serializeToJsonWith", + JacksonSerializer.class); + builderObj = serializeToJsonMethod.invoke(builderObj, new JacksonSerializer<>()); + + JwtBuilder resultBuilder = (JwtBuilder) builderObj; + return resultBuilder.compact(); + } +} diff --git a/src/main/java/org/kohsuke/github/extras/authorization/JwtReflectiveBuilderException.java b/src/main/java/org/kohsuke/github/extras/authorization/JwtReflectiveBuilderException.java new file mode 100644 index 0000000000..2e1c457215 --- /dev/null +++ b/src/main/java/org/kohsuke/github/extras/authorization/JwtReflectiveBuilderException.java @@ -0,0 +1,7 @@ +package org.kohsuke.github.extras.authorization; + +public class JwtReflectiveBuilderException extends RuntimeException { + JwtReflectiveBuilderException(String msg, Throwable cause) { + super(msg, cause); + } +} diff --git a/src/main/java/org/kohsuke/github/extras/authorization/RefreshResult.java b/src/main/java/org/kohsuke/github/extras/authorization/RefreshResult.java new file mode 100644 index 0000000000..48f5a0e11c --- /dev/null +++ b/src/main/java/org/kohsuke/github/extras/authorization/RefreshResult.java @@ -0,0 +1,21 @@ +package org.kohsuke.github.extras.authorization; + +import java.time.Instant; + +class RefreshResult { + private final Instant validUntil; + private final String jwt; + + RefreshResult(Instant validUntil, String jwt) { + this.validUntil = validUntil; + this.jwt = jwt; + } + + Instant getValidUntil() { + return this.validUntil; + } + + String getJwt() { + return this.jwt; + } +} From 462c4dae3e9429166b431cce99d8af0b2debed20 Mon Sep 17 00:00:00 2001 From: D061587 Date: Mon, 6 Nov 2023 09:20:49 +0100 Subject: [PATCH 03/10] Remove unused class --- .../extras/authorization/RefreshResult.java | 21 ------------------- 1 file changed, 21 deletions(-) delete mode 100644 src/main/java/org/kohsuke/github/extras/authorization/RefreshResult.java diff --git a/src/main/java/org/kohsuke/github/extras/authorization/RefreshResult.java b/src/main/java/org/kohsuke/github/extras/authorization/RefreshResult.java deleted file mode 100644 index 48f5a0e11c..0000000000 --- a/src/main/java/org/kohsuke/github/extras/authorization/RefreshResult.java +++ /dev/null @@ -1,21 +0,0 @@ -package org.kohsuke.github.extras.authorization; - -import java.time.Instant; - -class RefreshResult { - private final Instant validUntil; - private final String jwt; - - RefreshResult(Instant validUntil, String jwt) { - this.validUntil = validUntil; - this.jwt = jwt; - } - - Instant getValidUntil() { - return this.validUntil; - } - - String getJwt() { - return this.jwt; - } -} From 3a2db896a538c7ccc898e12bae88207e99d35b9b Mon Sep 17 00:00:00 2001 From: D061587 Date: Tue, 7 Nov 2023 10:44:54 +0100 Subject: [PATCH 04/10] Enhance documentation and add a logging statement for outdated jjwt libraries --- .../extras/authorization/JwtBuilderUtil.java | 56 ++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java b/src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java index 5c88dc87b9..608e8db9b5 100644 --- a/src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java +++ b/src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java @@ -4,18 +4,48 @@ import java.security.PrivateKey; import java.time.Instant; import java.util.Date; +import java.util.logging.Logger; import io.jsonwebtoken.JwtBuilder; import io.jsonwebtoken.Jwts; import io.jsonwebtoken.SignatureAlgorithm; import io.jsonwebtoken.jackson.io.JacksonSerializer; +/** + * This is a util to build a JWT. + * + *

+ * 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 again, once we + * are sure, we do no longer need to support pre-0.12.x versions of jjwt. + *

+ */ final class JwtBuilderUtil { + + private static final Logger LOGGER = Logger.getLogger(JwtBuilderUtil.class.getName()); + + /** + * Get a method from an object. + * + * @param obj object + * @param method method name + * @param params parameters of the method + * @return method + * @throws NoSuchMethodException if the method does not exist + */ private static Method getMethod(Object obj, String method, Class... params) throws NoSuchMethodException { Class type = obj.getClass(); return type.getMethod(method, params); } + /** + * Check if an object has a method. + * + * @param obj object + * @param method method name + * @param params parameters of the method + * @return true if the method exists + */ private static boolean hasMethod(Object obj, String method, Class... params) { try { return JwtBuilderUtil.getMethod(obj, method, params) != null; @@ -24,6 +54,15 @@ private static boolean hasMethod(Object obj, String method, Class... params) } } + /** + * 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) { JwtBuilder jwtBuilder = Jwts.builder(); if (JwtBuilderUtil.hasMethod(jwtBuilder, "issuedAt", Date.class)) { @@ -34,16 +73,31 @@ static String buildJwt(Instant issuedAt, Instant expiration, String applicationI return jwtBuilder.json(new JacksonSerializer<>()).compact(); } + LOGGER.warning( + "You are using an outdated version of the io.jsonwebtoken:jjwt-* suite. Please consider an update."); + // older jjwt library versions try { return JwtBuilderUtil.buildByReflection(jwtBuilder, issuedAt, expiration, applicationId, privateKey); } catch (ReflectiveOperationException e) { throw new JwtReflectiveBuilderException( - "Exception building a JWT with reflective access to outdated versions of jjwt. Please consider an update.", + "Exception building a JWT with reflective access to outdated versions of the io.jsonwebtoken:jjwt-* suite. Please consider an update.", e); } } + /** + * This method builds a JWT using older (pre 0.12.x) versions of jjwt library by + * leveraging reflection. + * + * @param jwtBuilder builder object + * @param issuedAt issued at + * @param expiration expiration + * @param applicationId application id + * @param privateKey private key + * @return JWT + * @throws ReflectiveOperationException if reflection fails + */ private static String buildByReflection(JwtBuilder jwtBuilder, Instant issuedAt, Instant expiration, String applicationId, PrivateKey privateKey) throws ReflectiveOperationException { From de6827263cf205e9eb41da0b0064e69094eb05fb Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Wed, 8 Nov 2023 01:52:39 -0800 Subject: [PATCH 05/10] Add testing for JWT 0.11.x to pom.xml --- pom.xml | 51 ++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index 5f072e86c4..bd9bb94e04 100644 --- a/pom.xml +++ b/pom.xml @@ -83,7 +83,7 @@ maven-surefire-plugin - 2.22.2 + 3.2.2 false @@ -458,6 +458,18 @@ + + + + com.fasterxml.jackson + jackson-bom + 2.15.3 + import + pom + + + + org.apache.commons @@ -510,7 +522,6 @@ com.fasterxml.jackson.core jackson-databind - 2.15.2 commons-io @@ -652,7 +663,7 @@ - test-slow-multireleasejar-flaky + test-jwt-slow-multireleasejar-flaky !test @@ -715,6 +726,40 @@ src/test/resources/slow-or-flaky-tests.txt + + + jwt0.11.x-test + integration-test + + test + + + ${project.basedir}/target/github-api-${project.version}.jar + false + src/test/resources/slow-or-flaky-tests.txt + @{jacoco.surefire.argLine} ${surefire.argLine} -Dtest.github.connector=okhttp + + io.jsonwebtoken:* + + + + io.jsonwebtoken + jjwt-api + 0.11.5 + + + io.jsonwebtoken + jjwt-impl + 0.11.5 + + + io.jsonwebtoken + jjwt-jackson + 0.11.5 + + + + From 9b0234890fc3ec1f8cd33178623b78ef9a96f754 Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Wed, 8 Nov 2023 02:12:55 -0800 Subject: [PATCH 06/10] Restructured JwtBuilderUtil based on testing When I went to run tests on the reflection code I ran into a number of errors. A few were straighfoward fixes but not all. The issue requiring the biggest change as that some JWT suite classes were being loaded as the JwtBuilderUtil class was being loaded, or at least before the buildJwt() method started executing. I had to move the JWT calls into a nested class to delay the loading of JWT suite classes until inside a try-catch where I could handle it as expected. --- .../authorization/JWTTokenProvider.java | 4 +- .../extras/authorization/JwtBuilderUtil.java | 198 ++++++++++-------- 2 files changed, 113 insertions(+), 89 deletions(-) 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 0bc8fe3df6..cc3f3c4df5 100644 --- a/src/main/java/org/kohsuke/github/extras/authorization/JWTTokenProvider.java +++ b/src/main/java/org/kohsuke/github/extras/authorization/JWTTokenProvider.java @@ -1,5 +1,7 @@ package org.kohsuke.github.extras.authorization; +import org.kohsuke.github.authorization.AuthorizationProvider; + import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -16,8 +18,6 @@ import javax.annotation.Nonnull; -import org.kohsuke.github.authorization.AuthorizationProvider; - /** * A authorization provider that gives valid JWT tokens. These tokens are then used to create a time-based token to * authenticate as an application. This token provider does not provide any kind of caching, and will always request a diff --git a/src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java b/src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java index 608e8db9b5..665935a74b 100644 --- a/src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java +++ b/src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java @@ -1,23 +1,25 @@ 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.Method; +import java.security.Key; import java.security.PrivateKey; import java.time.Instant; import java.util.Date; import java.util.logging.Logger; -import io.jsonwebtoken.JwtBuilder; -import io.jsonwebtoken.Jwts; -import io.jsonwebtoken.SignatureAlgorithm; -import io.jsonwebtoken.jackson.io.JacksonSerializer; - /** * This is a util to build a JWT. * *

- * 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 again, once we - * are sure, we do no longer need to support pre-0.12.x versions of jjwt. + * 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. *

*/ final class JwtBuilderUtil { @@ -25,103 +27,125 @@ final class JwtBuilderUtil { private static final Logger LOGGER = Logger.getLogger(JwtBuilderUtil.class.getName()); /** - * Get a method from an object. + * Build a JWT. * - * @param obj object - * @param method method name - * @param params parameters of the method - * @return method - * @throws NoSuchMethodException if the method does not exist + * @param issuedAt + * issued at + * @param expiration + * expiration + * @param applicationId + * application id + * @param privateKey + * private key + * @return JWT */ - private static Method getMethod(Object obj, String method, Class... params) throws NoSuchMethodException { - Class type = obj.getClass(); - return type.getMethod(method, params); - } + static String buildJwt(Instant issuedAt, Instant expiration, String applicationId, PrivateKey privateKey) { - /** - * Check if an object has a method. - * - * @param obj object - * @param method method name - * @param params parameters of the method - * @return true if the method exists - */ - private static boolean hasMethod(Object obj, String method, Class... params) { try { - return JwtBuilderUtil.getMethod(obj, method, params) != null; - } catch (NoSuchMethodException e) { - return false; + return DefaultBuilderImpl.buildJwt(issuedAt, expiration, applicationId, privateKey); + } catch (NoSuchMethodError | NoClassDefFoundError e) { + LOGGER.info( + "You are using an outdated version of the io.jsonwebtoken:jjwt-* suite. v0.12.x or later is recommended."); + } + + // older jjwt library versions + try { + return ReflectionBuilderImpl.buildJwt(issuedAt, expiration, applicationId, privateKey); + } catch (SecurityException | ReflectiveOperationException re) { + throw new GHException( + "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); } } /** - * Build a JWT. + * A class to isolate loading of JWT classes allowing us to catch and handle linkage errors. * - * @param issuedAt issued at - * @param expiration expiration - * @param applicationId application id - * @param privateKey private key - * @return JWT + * 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. */ - static String buildJwt(Instant issuedAt, Instant expiration, String applicationId, PrivateKey privateKey) { - JwtBuilder jwtBuilder = Jwts.builder(); - if (JwtBuilderUtil.hasMethod(jwtBuilder, "issuedAt", Date.class)) { + private static class DefaultBuilderImpl { + + /** + * 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 + */ + private static 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, Jwts.SIG.RS256); - return jwtBuilder.json(new JacksonSerializer<>()).compact(); - } - - LOGGER.warning( - "You are using an outdated version of the io.jsonwebtoken:jjwt-* suite. Please consider an update."); - - // older jjwt library versions - try { - return JwtBuilderUtil.buildByReflection(jwtBuilder, issuedAt, expiration, applicationId, privateKey); - } catch (ReflectiveOperationException e) { - throw new JwtReflectiveBuilderException( - "Exception building a JWT with reflective access to outdated versions of the io.jsonwebtoken:jjwt-* suite. Please consider an update.", - e); + .signWith(privateKey, rs256) + .json(new JacksonSerializer<>()); + return jwtBuilder.compact(); } } /** - * This method builds a JWT using older (pre 0.12.x) versions of jjwt library by - * leveraging reflection. - * - * @param jwtBuilder builder object - * @param issuedAt issued at - * @param expiration expiration - * @param applicationId application id - * @param privateKey private key - * @return JWT - * @throws ReflectiveOperationException if reflection fails + * A class to encapsulate building a JWT using reflection. */ - private static String buildByReflection(JwtBuilder jwtBuilder, Instant issuedAt, Instant expiration, - String applicationId, - PrivateKey privateKey) throws ReflectiveOperationException { - - Object builderObj = jwtBuilder; - - Method setIssuedAtMethod = JwtBuilderUtil.getMethod(builderObj, "setIssuedAt", Date.class); - builderObj = setIssuedAtMethod.invoke(builderObj, Date.from(issuedAt)); - - Method setExpirationMethod = JwtBuilderUtil.getMethod(builderObj, "setExpiration", Date.class); - builderObj = setExpirationMethod.invoke(builderObj, Date.from(expiration)); - - Method setIssuerMethod = JwtBuilderUtil.getMethod(builderObj, "setIssuer", String.class); - builderObj = setIssuerMethod.invoke(builderObj, applicationId); - - Method signWithMethod = JwtBuilderUtil.getMethod(builderObj, "signWith", PrivateKey.class, - SignatureAlgorithm.class); - builderObj = signWithMethod.invoke(builderObj, privateKey, SignatureAlgorithm.RS256); - - Method serializeToJsonMethod = JwtBuilderUtil.getMethod(builderObj, "serializeToJsonWith", - JacksonSerializer.class); - builderObj = serializeToJsonMethod.invoke(builderObj, new JacksonSerializer<>()); + private static class ReflectionBuilderImpl { + /** + * 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 + * @throws ReflectiveOperationException + * if reflection fails + */ + private static String buildJwt(Instant issuedAt, + Instant expiration, + String applicationId, + PrivateKey privateKey) throws ReflectiveOperationException { + + JwtBuilder jwtBuilder = Jwts.builder(); + Class jwtReflectionClass = jwtBuilder.getClass(); + + Method setIssuedAtMethod = jwtReflectionClass.getMethod("setIssuedAt", Date.class); + Method setIssuerMethod = jwtReflectionClass.getMethod("setIssuer", String.class); + Method setExpirationMethod = jwtReflectionClass.getMethod("setExpiration", Date.class); + Class signatureAlgorithmClass = Class.forName("io.jsonwebtoken.SignatureAlgorithm"); + Enum rs256SignatureAlgorithm = createEnumInstance(signatureAlgorithmClass, "RS256"); + Method signWithMethod = jwtReflectionClass.getMethod("signWith", Key.class, signatureAlgorithmClass); + Method serializeToJsonMethod = jwtReflectionClass.getMethod("serializeToJsonWith", Serializer.class); + + 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(); + } - JwtBuilder resultBuilder = (JwtBuilder) builderObj; - return resultBuilder.compact(); + @SuppressWarnings("unchecked") + private static > T createEnumInstance(Class type, String name) { + return Enum.valueOf((Class) type, name); + } } } From 7dff4b45f26b34f6d5ee3dfbba7056f1dca94888 Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Wed, 8 Nov 2023 02:18:20 -0800 Subject: [PATCH 07/10] Move some test to the slow test list --- src/test/resources/slow-or-flaky-tests.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/resources/slow-or-flaky-tests.txt b/src/test/resources/slow-or-flaky-tests.txt index e0aa93a72e..5e36cd96d4 100644 --- a/src/test/resources/slow-or-flaky-tests.txt +++ b/src/test/resources/slow-or-flaky-tests.txt @@ -1,5 +1,8 @@ **/extras/** +**/AbuseLimitHandlerTest **/GHRateLimitTest +**/GHPullRequestTest +**/GitHubStaticTest **/RequesterRetryTest **/RateLimitCheckerTest **/RateLimitHandlerTest From 8c51f6b227cf338d9955ccc6d022a8f5ccb2b991 Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Wed, 8 Nov 2023 02:26:20 -0800 Subject: [PATCH 08/10] Remove JwtReflectiveBuilderException --- .../authorization/JwtReflectiveBuilderException.java | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 src/main/java/org/kohsuke/github/extras/authorization/JwtReflectiveBuilderException.java diff --git a/src/main/java/org/kohsuke/github/extras/authorization/JwtReflectiveBuilderException.java b/src/main/java/org/kohsuke/github/extras/authorization/JwtReflectiveBuilderException.java deleted file mode 100644 index 2e1c457215..0000000000 --- a/src/main/java/org/kohsuke/github/extras/authorization/JwtReflectiveBuilderException.java +++ /dev/null @@ -1,7 +0,0 @@ -package org.kohsuke.github.extras.authorization; - -public class JwtReflectiveBuilderException extends RuntimeException { - JwtReflectiveBuilderException(String msg, Throwable cause) { - super(msg, cause); - } -} From dbef88278e855f6114cc3c76d053ac72acf18d74 Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Wed, 8 Nov 2023 02:36:40 -0800 Subject: [PATCH 09/10] Slow test list adjustment for coverage --- src/test/resources/slow-or-flaky-tests.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/resources/slow-or-flaky-tests.txt b/src/test/resources/slow-or-flaky-tests.txt index 5e36cd96d4..6b1b48fe7b 100644 --- a/src/test/resources/slow-or-flaky-tests.txt +++ b/src/test/resources/slow-or-flaky-tests.txt @@ -1,8 +1,6 @@ **/extras/** -**/AbuseLimitHandlerTest **/GHRateLimitTest **/GHPullRequestTest -**/GitHubStaticTest **/RequesterRetryTest **/RateLimitCheckerTest **/RateLimitHandlerTest From ef87bb77f55ca9ae2f70a53d626872fa64189fe8 Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Thu, 9 Nov 2023 13:22:06 -0800 Subject: [PATCH 10/10] Convert JWTBuildImpl to instances This makes it so we only need to check once for which implementation to use. --- .../extras/authorization/JwtBuilderUtil.java | 117 +++++++++++++----- 1 file changed, 84 insertions(+), 33 deletions(-) diff --git a/src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java b/src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java index 665935a74b..754b2f315c 100644 --- a/src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java +++ b/src/main/java/org/kohsuke/github/extras/authorization/JwtBuilderUtil.java @@ -7,6 +7,7 @@ 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; @@ -26,6 +27,8 @@ final class JwtBuilderUtil { private static final Logger LOGGER = Logger.getLogger(JwtBuilderUtil.class.getName()); + private static IJwtBuilder builder; + /** * Build a JWT. * @@ -40,33 +43,66 @@ final class JwtBuilderUtil { * @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 { - return DefaultBuilderImpl.buildJwt(issuedAt, expiration, applicationId, privateKey); + builder = new DefaultBuilderImpl(); } catch (NoSuchMethodError | NoClassDefFoundError e) { - LOGGER.info( + LOGGER.warning( "You are using an outdated version of the io.jsonwebtoken:jjwt-* suite. v0.12.x or later is recommended."); - } - // older jjwt library versions - try { - return ReflectionBuilderImpl.buildJwt(issuedAt, expiration, applicationId, privateKey); - } catch (SecurityException | ReflectiveOperationException re) { - throw new GHException( - "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); + 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( + "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 { - + private static class DefaultBuilderImpl implements IJwtBuilder { /** * This method builds a JWT using 0.12.x or later versions of jjwt library * @@ -80,10 +116,7 @@ private static class DefaultBuilderImpl { * private key * @return JWT */ - private static String buildJwt(Instant issuedAt, - Instant expiration, - String applicationId, - PrivateKey privateKey) { + 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. @@ -102,7 +135,28 @@ private static String buildJwt(Instant issuedAt, /** * A class to encapsulate building a JWT using reflection. */ - private static class ReflectionBuilderImpl { + 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. * @@ -115,25 +169,22 @@ private static class ReflectionBuilderImpl { * @param privateKey * private key * @return JWTBuilder - * @throws ReflectiveOperationException - * if reflection fails */ - private static String buildJwt(Instant issuedAt, + public String buildJwt(Instant issuedAt, Instant expiration, String applicationId, PrivateKey privateKey) { + + try { + return buildJwtWithReflection(issuedAt, expiration, applicationId, privateKey); + } catch (ReflectiveOperationException e) { + // 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); + } + } + + private String buildJwtWithReflection(Instant issuedAt, Instant expiration, String applicationId, - PrivateKey privateKey) throws ReflectiveOperationException { - + PrivateKey privateKey) throws IllegalAccessException, InvocationTargetException { JwtBuilder jwtBuilder = Jwts.builder(); - Class jwtReflectionClass = jwtBuilder.getClass(); - - Method setIssuedAtMethod = jwtReflectionClass.getMethod("setIssuedAt", Date.class); - Method setIssuerMethod = jwtReflectionClass.getMethod("setIssuer", String.class); - Method setExpirationMethod = jwtReflectionClass.getMethod("setExpiration", Date.class); - Class signatureAlgorithmClass = Class.forName("io.jsonwebtoken.SignatureAlgorithm"); - Enum rs256SignatureAlgorithm = createEnumInstance(signatureAlgorithmClass, "RS256"); - Method signWithMethod = jwtReflectionClass.getMethod("signWith", Key.class, signatureAlgorithmClass); - Method serializeToJsonMethod = jwtReflectionClass.getMethod("serializeToJsonWith", Serializer.class); - Object builderObj = jwtBuilder; builderObj = setIssuedAtMethod.invoke(builderObj, Date.from(issuedAt)); builderObj = setExpirationMethod.invoke(builderObj, Date.from(expiration));