Skip to content

Commit

Permalink
[DPC-4434] Bump junit.jupiter to 5.11.3 (#2354)
Browse files Browse the repository at this point in the history
## 🎫 Ticket

https://jira.cms.gov/browse/DPC-4434

## 🛠 Changes

Bump junit-jupiter to 5.11.3 to get our integration tests running again.

## ℹ️ Context

When running `make ci-app`, both locally and as part of the Github build
process our tests haven't been running. This was because the most recent
bump of Drop Wizard brought in a transitive dependency on junit-jupiter
5.11.3, and we were running 5.10.2. This conflict doesn't cause a
failure, but it does prevent Junit from finding any of our tests. This
PR bumps our version to match the one pulled in from Drop Wizard and
gets our tests running again.

### Note

This change was merged on 11/26 and our tests haven't been running ever
since. We're going to have to go back and fix any potential failing
tests we've introduced since then.

## 🧪 Validation

- Ran make ci-app locally and tests are running.  
- Submitted this PR and can see the tests failing.

---------

Co-authored-by: Ashley Weaver <ashleyweaver@navapbc.com>
  • Loading branch information
MEspositoE14s and ashley-weaver authored Dec 16, 2024
1 parent f1b23a2 commit 3184fba
Show file tree
Hide file tree
Showing 13 changed files with 148 additions and 156 deletions.
2 changes: 1 addition & 1 deletion dpc-api/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

<properties>
<mainClass>gov.cms.dpc.api.DPCAPIService</mainClass>
<jjwt.version>0.12.6</jjwt.version>
<jjwt.version>0.11.4</jjwt.version>
<mockserverVersion>5.15.0</mockserverVersion>
</properties>

Expand Down
6 changes: 3 additions & 3 deletions dpc-api/src/main/java/gov/cms/dpc/api/DPCAPIModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import com.google.inject.Provides;
import com.google.inject.name.Named;
import gov.cms.dpc.api.auth.jwt.IJTICache;
import gov.cms.dpc.api.auth.jwt.KeyResolverAdapter;
import gov.cms.dpc.api.converters.ChecksumConverterProvider;
import gov.cms.dpc.api.converters.HttpRangeHeaderParamConverterProvider;
import gov.cms.dpc.api.core.FileManager;
Expand Down Expand Up @@ -37,6 +36,7 @@
import gov.cms.dpc.macaroons.thirdparty.MemoryThirdPartyKeyStore;
import gov.cms.dpc.queue.service.DataService;
import io.dropwizard.hibernate.UnitOfWorkAwareProxyFactory;
import io.jsonwebtoken.SigningKeyResolverAdapter;
import org.hibernate.SessionFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -103,13 +103,13 @@ public KeyResource provideKeyResource(PublicKeyDAO dao) {
}

@Provides
public TokenResource provideTokenResource(TokenDAO dao, MacaroonBakery bakery, KeyResolverAdapter resolver, IJTICache cache, @APIV1 String publicURL) {
public TokenResource provideTokenResource(TokenDAO dao, MacaroonBakery bakery, SigningKeyResolverAdapter resolver, IJTICache cache, @APIV1 String publicURL) {
return new UnitOfWorkAwareProxyFactory(authHibernateBundle)
.create(TokenResource.class,
new Class<?>[]{TokenDAO.class,
MacaroonBakery.class,
TokenPolicy.class,
KeyResolverAdapter.class,
SigningKeyResolverAdapter.class,
IJTICache.class,
String.class},
new Object[]{dao,
Expand Down
4 changes: 2 additions & 2 deletions dpc-api/src/main/java/gov/cms/dpc/api/auth/AuthModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
import gov.cms.dpc.api.auth.jwt.CaffeineJTICache;
import gov.cms.dpc.api.auth.jwt.IJTICache;
import gov.cms.dpc.api.auth.jwt.JwtKeyResolver;
import gov.cms.dpc.api.auth.jwt.KeyResolverAdapter;
import gov.cms.dpc.api.auth.macaroonauth.MacaroonsAuthenticator;
import gov.cms.dpc.api.auth.staticauth.StaticAuthFactory;
import gov.cms.dpc.api.auth.staticauth.StaticAuthFilter;
import gov.cms.dpc.api.auth.staticauth.StaticAuthenticator;
import gov.cms.dpc.macaroons.thirdparty.BakeryKeyPair;
import io.dropwizard.auth.Authenticator;
import io.dropwizard.auth.UnauthorizedHandler;
import io.jsonwebtoken.SigningKeyResolverAdapter;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import ru.vyarus.dropwizard.guice.module.support.DropwizardAwareModule;
Expand Down Expand Up @@ -49,7 +49,7 @@ public void configure() {
binder.bind(authenticatorTypeLiteral).to(MacaroonsAuthenticator.class);
}
binder.bind(DPCAuthDynamicFeature.class);
binder.bind(KeyResolverAdapter.class).to(JwtKeyResolver.class);
binder.bind(SigningKeyResolverAdapter.class).to(JwtKeyResolver.class);
binder.bind(IJTICache.class).to(CaffeineJTICache.class);
binder.bind(BakeryKeyPair.class).toProvider(new BakeryKeyPairProvider(this.configuration()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import gov.cms.dpc.macaroons.MacaroonCaveat;
import io.jsonwebtoken.Claims;
import io.jsonwebtoken.JwsHeader;
import io.jsonwebtoken.SigningKeyResolverAdapter;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.MDC;
Expand All @@ -22,7 +23,7 @@

import static gov.cms.dpc.api.auth.MacaroonHelpers.ORGANIZATION_CAVEAT_KEY;

public class JwtKeyResolver extends KeyResolverAdapter {
public class JwtKeyResolver extends SigningKeyResolverAdapter {

private static final Logger logger = LoggerFactory.getLogger(JwtKeyResolver.class);

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,21 @@
import java.time.Instant;
import java.time.OffsetDateTime;
import java.time.ZoneOffset;
import java.util.Set;
import java.util.UUID;

/**
* Implementation of {@link LocatorAdapter} that simply verifies whether the required claims and values are present.
* Implementation of {@link SigningKeyResolverAdapter} that simply verifies whether the required claims and values are present.
* As far as I can tell, this is the only way to get access to the JWS claims without actually verifying the signature.
* See: <a href="https://github.com/jwtk/jjwt/issues/205"></a>
* See: https://github.com/jwtk/jjwt/issues/205
* <p>
* The downside is that this method will always return a null {@link Key}, which means the {@link Jwts#parser()} method will always throw an {@link IllegalArgumentException}, which we need to catch.
*/
public class ValidatingKeyResolver extends KeyResolverAdapter {
public class ValidatingKeyResolver extends SigningKeyResolverAdapter {

private final IJTICache cache;
private final Set<String> audClaim;
private final String audClaim;

public ValidatingKeyResolver(IJTICache cache, Set<String> audClaim) {
public ValidatingKeyResolver(IJTICache cache, String audClaim) {
this.cache = cache;
this.audClaim = audClaim;
}
Expand Down Expand Up @@ -107,7 +106,7 @@ void validateClaims(String issuer, Claims claims) {
}

// Test correct aud claim
final Set<String> audience = getClaimIfPresent("audience", claims.getAudience());
final String audience = getClaimIfPresent("audience", claims.getAudience());
if (!audience.equals(this.audClaim)) {
throw new WebApplicationException("Audience claim value is incorrect", Response.Status.BAD_REQUEST);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,17 @@
import gov.cms.dpc.api.auth.MacaroonHelpers;
import gov.cms.dpc.api.auth.OrganizationPrincipal;
import gov.cms.dpc.api.auth.annotations.Authorizer;
import gov.cms.dpc.common.annotations.Public;
import gov.cms.dpc.api.auth.jwt.IJTICache;
import gov.cms.dpc.api.auth.jwt.KeyResolverAdapter;
import gov.cms.dpc.api.auth.jwt.ValidatingKeyResolver;
import gov.cms.dpc.api.entities.TokenEntity;
import gov.cms.dpc.api.jdbi.TokenDAO;
import gov.cms.dpc.api.models.CollectionResponse;
import gov.cms.dpc.api.models.CreateTokenRequest;
import gov.cms.dpc.api.models.JWTAuthResponse;
import gov.cms.dpc.api.models.CreateTokenRequest;
import gov.cms.dpc.api.resources.AbstractTokenResource;
import gov.cms.dpc.common.annotations.APIV1;
import gov.cms.dpc.common.annotations.NoHtml;
import gov.cms.dpc.common.annotations.Public;
import gov.cms.dpc.macaroons.CaveatSupplier;
import gov.cms.dpc.macaroons.MacaroonBakery;
import gov.cms.dpc.macaroons.MacaroonCaveat;
Expand Down Expand Up @@ -68,15 +67,15 @@ public class TokenResource extends AbstractTokenResource {
private final TokenDAO dao;
private final MacaroonBakery bakery;
private final TokenPolicy policy;
private final KeyResolverAdapter resolver;
private final SigningKeyResolverAdapter resolver;
private final IJTICache cache;
private final String authURL;

@Inject
public TokenResource(TokenDAO dao,
MacaroonBakery bakery,
TokenPolicy policy,
KeyResolverAdapter resolver,
SigningKeyResolverAdapter resolver,
IJTICache cache,
@APIV1 String publicURL) {
this.dao = dao;
Expand Down Expand Up @@ -244,11 +243,11 @@ public JWTAuthResponse authorizeJWT(
public Response validateJWT(@NoHtml @NotEmpty(message = "Must submit JWT") String jwt) {

try {
Jwts.parser()
Jwts.parserBuilder()
.requireAudience(this.authURL)
.keyLocator(new ValidatingKeyResolver(this.cache, Set.of(this.authURL)))
.setSigningKeyResolver(new ValidatingKeyResolver(this.cache, this.authURL))
.build()
.parseSignedClaims(jwt);
.parseClaimsJws(jwt);
} catch (IllegalArgumentException e) {
// This is fine, we just want the body
} catch (MalformedJwtException e) {
Expand Down Expand Up @@ -277,14 +276,14 @@ private void validateJWTQueryParams(String grantType, String clientAssertionType
}

private JWTAuthResponse handleJWT(String jwtBody, String requestedScope) {
final Jws<Claims> claims = Jwts.parser()
.keyLocator(this.resolver)
final Jws<Claims> claims = Jwts.parserBuilder()
.setSigningKeyResolver(this.resolver)
.requireAudience(this.authURL)
.build()
.parseSignedClaims(jwtBody);
.parseClaimsJws(jwtBody);

// Extract the Client Macaroon from the subject field (which is the same as the issuer)
final String clientMacaroon = claims.getPayload().getSubject();
final String clientMacaroon = claims.getBody().getSubject();
final List<Macaroon> macaroons = MacaroonBakery.deserializeMacaroon(clientMacaroon);

// Get org id from macaroon caveats
Expand Down Expand Up @@ -355,20 +354,20 @@ private OffsetDateTime handleExpirationTime(Optional<OffsetDateTimeParam> expire
@SuppressWarnings("JdkObsolete") // Date class is used by Jwt
private void handleJWTClaims(UUID organizationID, Jws<Claims> claims) {
// Issuer and Sub must be present and identical
final String issuer = getClaimIfPresent("issuer", claims.getPayload().getIssuer());
final String subject = getClaimIfPresent("subject", claims.getPayload().getSubject());
final String issuer = getClaimIfPresent("issuer", claims.getBody().getIssuer());
final String subject = getClaimIfPresent("subject", claims.getBody().getSubject());
if (!issuer.equals(subject)) {
throw new WebApplicationException("Issuer and Subject must be identical", Response.Status.BAD_REQUEST);
}

// JTI must be present and have not been used in the past 5 minutes.
if (!this.cache.isJTIOk(getClaimIfPresent("id", claims.getPayload().getId()), true)) {
if (!this.cache.isJTIOk(getClaimIfPresent("id", claims.getBody().getId()), true)) {
logger.warn("JWT being replayed for organization {}", organizationID);
throw new WebApplicationException(INVALID_JWT_MSG, Response.Status.UNAUTHORIZED);
}

// Ensure the expiration time for the token is not more than 5 minutes in the future
final Date expiration = getClaimIfPresent("expiration", claims.getPayload().getExpiration());
final Date expiration = getClaimIfPresent("expiration", claims.getBody().getExpiration());
if (OffsetDateTime.now(ZoneOffset.UTC).plusMinutes(5).isBefore(expiration.toInstant().atOffset(ZoneOffset.UTC))) {
throw new WebApplicationException("Not authorized", Response.Status.UNAUTHORIZED);
}
Expand Down
Loading

0 comments on commit 3184fba

Please sign in to comment.