From 3e064e484240c0878a8b99713a105479f9a7a542 Mon Sep 17 00:00:00 2001 From: saquino0827 Date: Fri, 13 Sep 2024 13:46:05 -0500 Subject: [PATCH 1/4] Remove Invalid Tokens from cache when found Co-authored-by: jcrichlake <145698165+jcrichlake@users.noreply.github.com> Co-authored-by: pluckyswan <96704946+pluckyswan@users.noreply.github.com> Co-authored-by: halprin --- .../auth/AuthRequestValidator.java | 9 +++++---- .../auth/AuthRequestValidatorTest.groovy | 9 +++------ .../external/inmemory/KeyCache.java | 5 +++++ .../cdc/trustedintermediary/wrappers/Cache.java | 2 ++ .../external/inmemory/KeyCacheTest.groovy | 14 ++++++++++++++ 5 files changed, 29 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidator.java b/app/src/main/java/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidator.java index f55c44b1c..efd230a8e 100644 --- a/app/src/main/java/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidator.java +++ b/app/src/main/java/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidator.java @@ -21,7 +21,7 @@ public class AuthRequestValidator { private static final AuthRequestValidator INSTANCE = new AuthRequestValidator(); @Inject private AuthEngine jwtEngine; - @Inject private Cache keyCache; + @Inject Cache keyCache; @Inject private Secrets secrets; @Inject private Logger logger; @@ -42,19 +42,20 @@ public boolean isValidAuthenticatedRequest(DomainRequest request) return false; } + var ourPublicKey = "trusted-intermediary-public-key-" + ApplicationContext.getEnvironment(); try { logger.logDebug("Checking if bearer token is valid..."); - jwtEngine.validateToken(token, retrievePublicKey()); + jwtEngine.validateToken(token, retrievePublicKey(ourPublicKey)); logger.logInfo("Bearer token is valid"); return true; } catch (InvalidTokenException e) { logger.logError("Invalid bearer token!", e); + this.keyCache.remove(ourPublicKey); return false; } } - protected String retrievePublicKey() throws SecretRetrievalException { - var ourPublicKey = "trusted-intermediary-public-key-" + ApplicationContext.getEnvironment(); + protected String retrievePublicKey(String ourPublicKey) throws SecretRetrievalException { String key = this.keyCache.get(ourPublicKey); if (key != null) { return key; diff --git a/app/src/test/groovy/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidatorTest.groovy b/app/src/test/groovy/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidatorTest.groovy index 3b20e104d..c64422a4f 100644 --- a/app/src/test/groovy/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidatorTest.groovy +++ b/app/src/test/groovy/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidatorTest.groovy @@ -1,5 +1,6 @@ package gov.hhs.cdc.trustedintermediary.auth +import gov.hhs.cdc.trustedintermediary.context.ApplicationContext import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext import gov.hhs.cdc.trustedintermediary.domainconnector.DomainRequest import gov.hhs.cdc.trustedintermediary.external.inmemory.KeyCache @@ -227,21 +228,17 @@ class AuthRequestValidatorTest extends Specification{ def validator = AuthRequestValidator.getInstance() def token = "fake-token-here" def header = Map.of("Authorization", "Bearer " + token) - def mockEngine = Mock(JjwtEngine) - def mockCache = Mock(KeyCache) def request = new DomainRequest() def expected = false - TestApplicationContext.register(Cache, mockCache) - TestApplicationContext.register(AuthEngine, mockEngine) + TestApplicationContext.register(Cache, KeyCache.getInstance()) TestApplicationContext.injectRegisteredImplementations() when: request.setHeaders(header) - mockCache.get(_ as String) >> {"my-fake-private-key"} - mockEngine.validateToken(_ as String, _ as String) >> { throw new InvalidTokenException(new Throwable("fake exception"))} def actual = validator.isValidAuthenticatedRequest(request) then: actual == expected + validator.keyCache.get("trusted-intermediary-public-key-" + ApplicationContext.getEnvironment()) == null } } diff --git a/shared/src/main/java/gov/hhs/cdc/trustedintermediary/external/inmemory/KeyCache.java b/shared/src/main/java/gov/hhs/cdc/trustedintermediary/external/inmemory/KeyCache.java index 66b929ab4..28ae90b3d 100644 --- a/shared/src/main/java/gov/hhs/cdc/trustedintermediary/external/inmemory/KeyCache.java +++ b/shared/src/main/java/gov/hhs/cdc/trustedintermediary/external/inmemory/KeyCache.java @@ -30,4 +30,9 @@ public void put(String key, String value) { public String get(String key) { return keys.get(key); } + + @Override + public void remove(String key) { + keys.remove(key); + } } diff --git a/shared/src/main/java/gov/hhs/cdc/trustedintermediary/wrappers/Cache.java b/shared/src/main/java/gov/hhs/cdc/trustedintermediary/wrappers/Cache.java index a659f8b5e..946c2dfd1 100644 --- a/shared/src/main/java/gov/hhs/cdc/trustedintermediary/wrappers/Cache.java +++ b/shared/src/main/java/gov/hhs/cdc/trustedintermediary/wrappers/Cache.java @@ -6,4 +6,6 @@ public interface Cache { void put(String key, String value); String get(String key); + + void remove(String key); } diff --git a/shared/src/test/groovy/gov/hhs/cdc/trustedintermediary/external/inmemory/KeyCacheTest.groovy b/shared/src/test/groovy/gov/hhs/cdc/trustedintermediary/external/inmemory/KeyCacheTest.groovy index a45cdbec6..5666c6fed 100644 --- a/shared/src/test/groovy/gov/hhs/cdc/trustedintermediary/external/inmemory/KeyCacheTest.groovy +++ b/shared/src/test/groovy/gov/hhs/cdc/trustedintermediary/external/inmemory/KeyCacheTest.groovy @@ -43,4 +43,18 @@ class KeyCacheTest extends Specification { keys.values().toSet().size() == 1 // all entries have same value, threads had to wait on the lock } + + def "keyCache removal works"() { + given: + def cache = KeyCache.getInstance() + def value = "fake_key" + def key = "report_stream" + def expected = null + when: + cache.put(key, value) + cache.remove(key) + def actual = cache.get(key) + then: + actual == expected + } } From c6ffbb0f9e06aae7a06f4dcb71f8cf479b61028a Mon Sep 17 00:00:00 2001 From: saquino0827 Date: Fri, 13 Sep 2024 16:09:31 -0500 Subject: [PATCH 2/4] Updated the public key into a class variable and fixed unit tests Co-authored-by: Sylvie --- .../cdc/trustedintermediary/auth/AuthRequestValidator.java | 7 ++++--- .../auth/AuthRequestValidatorTest.groovy | 5 ++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidator.java b/app/src/main/java/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidator.java index efd230a8e..c021ba302 100644 --- a/app/src/main/java/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidator.java +++ b/app/src/main/java/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidator.java @@ -25,6 +25,8 @@ public class AuthRequestValidator { @Inject private Secrets secrets; @Inject private Logger logger; + String ourPublicKey = "trusted-intermediary-public-key-" + ApplicationContext.getEnvironment(); + private AuthRequestValidator() {} public static AuthRequestValidator getInstance() { @@ -42,10 +44,9 @@ public boolean isValidAuthenticatedRequest(DomainRequest request) return false; } - var ourPublicKey = "trusted-intermediary-public-key-" + ApplicationContext.getEnvironment(); try { logger.logDebug("Checking if bearer token is valid..."); - jwtEngine.validateToken(token, retrievePublicKey(ourPublicKey)); + jwtEngine.validateToken(token, retrievePublicKey()); logger.logInfo("Bearer token is valid"); return true; } catch (InvalidTokenException e) { @@ -55,7 +56,7 @@ public boolean isValidAuthenticatedRequest(DomainRequest request) } } - protected String retrievePublicKey(String ourPublicKey) throws SecretRetrievalException { + protected String retrievePublicKey() throws SecretRetrievalException { String key = this.keyCache.get(ourPublicKey); if (key != null) { return key; diff --git a/app/src/test/groovy/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidatorTest.groovy b/app/src/test/groovy/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidatorTest.groovy index c64422a4f..9bd0aeb6e 100644 --- a/app/src/test/groovy/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidatorTest.groovy +++ b/app/src/test/groovy/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidatorTest.groovy @@ -1,13 +1,12 @@ package gov.hhs.cdc.trustedintermediary.auth -import gov.hhs.cdc.trustedintermediary.context.ApplicationContext + import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext import gov.hhs.cdc.trustedintermediary.domainconnector.DomainRequest import gov.hhs.cdc.trustedintermediary.external.inmemory.KeyCache import gov.hhs.cdc.trustedintermediary.external.jjwt.JjwtEngine import gov.hhs.cdc.trustedintermediary.wrappers.AuthEngine import gov.hhs.cdc.trustedintermediary.wrappers.Cache -import gov.hhs.cdc.trustedintermediary.wrappers.InvalidTokenException import gov.hhs.cdc.trustedintermediary.wrappers.Secrets import spock.lang.Specification @@ -239,6 +238,6 @@ class AuthRequestValidatorTest extends Specification{ then: actual == expected - validator.keyCache.get("trusted-intermediary-public-key-" + ApplicationContext.getEnvironment()) == null + validator.keyCache.get(validator.ourPublicKey) == null } } From 813353edf8e3496514f668af5597aae7bca7425f Mon Sep 17 00:00:00 2001 From: saquino0827 Date: Fri, 13 Sep 2024 16:12:26 -0500 Subject: [PATCH 3/4] Remove excess blank newline Co-authored-by: Sylvie --- .../cdc/trustedintermediary/auth/AuthRequestValidatorTest.groovy | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/test/groovy/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidatorTest.groovy b/app/src/test/groovy/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidatorTest.groovy index 9bd0aeb6e..54b1470e2 100644 --- a/app/src/test/groovy/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidatorTest.groovy +++ b/app/src/test/groovy/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidatorTest.groovy @@ -1,6 +1,5 @@ package gov.hhs.cdc.trustedintermediary.auth - import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext import gov.hhs.cdc.trustedintermediary.domainconnector.DomainRequest import gov.hhs.cdc.trustedintermediary.external.inmemory.KeyCache From d7abdd9a81bdbdfd9df5513753037a46ff06878e Mon Sep 17 00:00:00 2001 From: saquino0827 Date: Mon, 16 Sep 2024 10:21:46 -0500 Subject: [PATCH 4/4] Add comments for caching --- .../hhs/cdc/trustedintermediary/auth/AuthRequestValidator.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidator.java b/app/src/main/java/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidator.java index c021ba302..6ad006dd6 100644 --- a/app/src/main/java/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidator.java +++ b/app/src/main/java/gov/hhs/cdc/trustedintermediary/auth/AuthRequestValidator.java @@ -14,7 +14,8 @@ /** * This class is used to check the validity of a http request. It has methods that extract the * bearer token, check if the token is empty or null, and if the token is valid. For example, - * expired tokens, empty tokens, or tokens not signed by our private key, will be invalid. + * expired tokens, empty tokens, or tokens not signed by our private key, will be invalid. Tokens + * are cached on first use, and removed if invalid. */ public class AuthRequestValidator {