Skip to content

Commit

Permalink
Merge pull request #214 from okta/ak_okta_755069_jjwt_signingkey_is_d…
Browse files Browse the repository at this point in the history
…eprecated

OKTA-755069: jjwt api setSigningKeyResolver() is deprecated
  • Loading branch information
arvindkrishnakumar-okta authored Aug 29, 2024
2 parents c79b6d8 + 35e516a commit 6d965cc
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 33 deletions.
1 change: 0 additions & 1 deletion impl/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@
<groupId>io.jsonwebtoken</groupId>
<artifactId>jjwt-impl</artifactId>
<version>${jjwt.version}</version>
<scope>runtime</scope>
</dependency>
<dependency>
<groupId>io.jsonwebtoken</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import java.time.Duration;

/**
* Classes in this `impl` implementation package may change in NON backward compatible way, and should ONLY be used as
* Classes in this `impl` implementation package may change in NON-backward compatible way, and should ONLY be used as
* a "runtime" dependency.
*/
public class JjwtIdTokenVerifier extends TokenVerifierSupport
Expand All @@ -47,9 +47,9 @@ public Jwt decode(String idToken, String nonce) throws JwtVerificationException
return decode(idToken, parser(), ClaimsValidator.compositeClaimsValidator(
new ClaimsValidator.ContainsAudienceClaimsValidator(clientId),
jws -> {
String actualNonce = jws.getBody().get("nonce", String.class);
String actualNonce = jws.getPayload().get("nonce", String.class);
if (!Objects.nullSafeEquals(actualNonce, nonce)) {
throw new IncorrectClaimException(jws.getHeader(), jws.getBody(), "nonce", actualNonce, "Claim `nonce` does not match expected value. Note: a `null` nonce is only valid when both the expected and actual values are `null`.");
throw new IncorrectClaimException(jws.getHeader(), jws.getPayload(), "nonce", actualNonce, "Claim `nonce` does not match expected value. Note: a `null` nonce is only valid when both the expected and actual values are `null`.");
}
}));
}
Expand Down
34 changes: 16 additions & 18 deletions impl/src/main/java/com/okta/jwt/impl/jjwt/TokenVerifierSupport.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,12 @@
import com.okta.jwt.Jwt;
import com.okta.jwt.JwtVerificationException;
import com.okta.jwt.impl.DefaultJwt;
import io.jsonwebtoken.Claims;
import io.jsonwebtoken.Jws;
import io.jsonwebtoken.JwtException;
import io.jsonwebtoken.JwtHandlerAdapter;
import io.jsonwebtoken.JwtParser;
import io.jsonwebtoken.Jwts;
import io.jsonwebtoken.SignatureAlgorithm;
import io.jsonwebtoken.SigningKeyResolver;
import io.jsonwebtoken.UnsupportedJwtException;
import io.jsonwebtoken.*;
import io.jsonwebtoken.impl.DefaultClaimsBuilder;

import java.time.Clock;
import java.time.Duration;
import java.util.Date;
import java.util.*;

abstract class TokenVerifierSupport {

Expand Down Expand Up @@ -64,10 +57,15 @@ abstract class TokenVerifierSupport {
protected JwtParser buildJwtParser() {
io.jsonwebtoken.Clock jwtClock = () -> Date.from(clock.instant());
return Jwts.parser()
.setSigningKeyResolver(keyResolver)
.keyLocator(header -> {
Claims claims = new DefaultClaimsBuilder()
.issuer(issuer)
.build();
return keyResolver.resolveSigningKey((JwsHeader) header, claims);
})
.requireIssuer(issuer)
.setAllowedClockSkewSeconds(leeway.getSeconds())
.setClock(jwtClock)
.clockSkewSeconds(leeway.getSeconds())
.clock(jwtClock)
.build();
}

Expand All @@ -80,9 +78,9 @@ protected Jwt decode(String token, JwtParser parser, ClaimsValidator claimsValid
try {
Jws<Claims> jwt = parser.parse(token, new OktaJwtHandler(claimsValidator));
return new DefaultJwt(token,
jwt.getBody().getIssuedAt().toInstant(),
jwt.getBody().getExpiration().toInstant(),
jwt.getBody());
jwt.getPayload().getIssuedAt().toInstant(),
jwt.getPayload().getExpiration().toInstant(),
jwt.getPayload());
} catch (JwtException e) {
throw new JwtVerificationException("Failed to parse token", e);
}
Expand Down Expand Up @@ -121,8 +119,8 @@ public Jws<Claims> onClaimsJws(Jws<Claims> jws) {

// validate alg
String alg = jws.getHeader().getAlgorithm();
if(!SignatureAlgorithm.RS256.getValue().equals(alg)) {
throw new UnsupportedJwtException("JWT Header 'alg' of [" + alg + "] is not supported, only RSA25 signatures are supported");
if(!"RS256".equals(alg)) {
throw new UnsupportedJwtException("JWT Header 'alg' of [" + alg + "] is not supported, only RSA256 signatures are supported");
}

claimsValidator.validateClaims(jws);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,15 @@ abstract class TokenVerifierTestSupport {
.issuer(TEST_ISSUER)
.issuedAt(Date.from(now))
.notBefore(Date.from(now))
.header().add("kid", TEST_PUB_KEY_ID).and()
.expiration(Date.from(now.plus(1L, ChronoUnit.HOURS)))
.setHeaderParam("kid", TEST_PUB_KEY_ID)
}

@Test
void missingIssuerClaim() {
expect JwtVerificationException, {
buildThenDecodeToken(baseJwtBuilder()
.setIssuer(null))
.issuer(null))
}
}

Expand Down Expand Up @@ -160,13 +160,13 @@ abstract class TokenVerifierTestSupport {
void jkuNotUsedTest() {

def signingKeyResolver = mock(SigningKeyResolver)
when(signingKeyResolver.resolveSigningKey(any(Header), any(Claims))).thenReturn(TEST_KEY_PAIR.getPublic())
when(signingKeyResolver.resolveSigningKey(any(Header) as JwsHeader, any(Claims))).thenReturn(TEST_KEY_PAIR.getPublic())

def jwtBuilder = baseJwtBuilder()
.setHeaderParam("jku", "http://example.com")

assertThat buildThenDecodeToken(jwtBuilder, signingKeyResolver), notNullValue()
verify(signingKeyResolver).resolveSigningKey(any(Header), any(Claims))
verify(signingKeyResolver).resolveSigningKey(any(Header) as JwsHeader, any(Claims))
verifyNoMoreInteractions(signingKeyResolver)
}

Expand Down Expand Up @@ -207,15 +207,15 @@ abstract class TokenVerifierTestSupport {
Instant now = Instant.now()
expect JwtVerificationException, {
buildThenDecodeToken(baseJwtBuilder()
.setExpiration(Date.from(now.minus(10L, ChronoUnit.SECONDS))))
.expiration(Date.from(now.minus(10L, ChronoUnit.SECONDS))))
}
}

@Test
void expiredUnderLeeway() {
Instant now = Instant.now()
buildThenDecodeToken(baseJwtBuilder()
.setExpiration(Date.from(now.minus(8L, ChronoUnit.SECONDS))))
.expiration(Date.from(now.minus(8L, ChronoUnit.SECONDS))))
}


Expand All @@ -224,29 +224,29 @@ abstract class TokenVerifierTestSupport {
Instant now = Instant.now()
expect JwtVerificationException, {
buildThenDecodeToken(baseJwtBuilder()
.setNotBefore(Date.from(now.plus(11L, ChronoUnit.SECONDS))))
.notBefore(Date.from(now.plus(11L, ChronoUnit.SECONDS))))
}
}

@Test
void notBeforeUnderLeeway() {
Instant now = Instant.now()
buildThenDecodeToken(baseJwtBuilder()
.setNotBefore(Date.from(now.minus(9L, ChronoUnit.SECONDS))))
.notBefore(Date.from(now.minus(9L, ChronoUnit.SECONDS))))
}

@Test
void oktaOrgIssuerMismatch() {
def orgIssuer = "https://test.example.com"
String token = baseJwtBuilder()
.setIssuer(orgIssuer)
.setHeaderParam("kid", "OKTA_ORG_KEY")
.issuer(orgIssuer)
.header().add("kid", "OKTA_ORG_KEY").and()
.signWith(ORG_KEY_PAIR.getPrivate(), SignatureAlgorithm.RS256)
.compact()

def e = expect JwtVerificationException, {decodeToken(token, signingKeyResolver)}
assertThat e.getMessage(), equalTo("Failed to parse token")
assertThat e.cause.getMessage(), equalTo("Expected iss claim to be: ${TEST_ISSUER}, but was: ${orgIssuer}.".toString())
assertThat e.cause.getMessage(), equalTo("JWT signature does not match locally computed signature. JWT validity cannot be asserted and should not be trusted.".toString())
}

String buildJwtWithFudgedHeader(String headerJson, String body) {
Expand Down

0 comments on commit 6d965cc

Please sign in to comment.