[java-oidc-common] branch main updated: JCOMOIDC-59 - Allow key fetching when a cached keyset document does not contain a given keyID
Phil Smart
philip.smart at jisc.ac.uk
Thu Jan 5 13:54:34 UTC 2023
This is an automated email from the git hooks/post-receive script.
philsmart pushed a commit to branch main
in repository java-oidc-common.
View the commit online:
http://git.shibboleth.net/view/?p=java-oidc-common.git;a=commit;h=68c80e944a7f4e54ab605a5460f57624d3869266
The following commit(s) were added to refs/heads/main by this push:
new 68c80e9 JCOMOIDC-59 - Allow key fetching when a cached keyset document does not contain a given keyID
68c80e9 is described below
commit 68c80e944a7f4e54ab605a5460f57624d3869266
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Thu Jan 5 13:54:31 2023 +0000
JCOMOIDC-59 - Allow key fetching when a cached keyset document does not
contain a given keyID
- Add new API for fetching a keyset considering to the KeyId to find.
If the keyset was from the cache and the keyId was not contained in the
cached version, the keyset document is re-feteched.
https://shibboleth.atlassian.net/browse/JCOMOIDC-59
---
.../net/shibboleth/oidc/jwk/RemoteJwkSetCache.java | 107 +++++++++++++++++++--
.../shibboleth/oidc/jwk/RemoteJwkSetCacheTest.java | 62 +++++++++++-
2 files changed, 155 insertions(+), 14 deletions(-)
diff --git a/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/jwk/RemoteJwkSetCache.java b/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/jwk/RemoteJwkSetCache.java
index 2982a06..88e4372 100644
--- a/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/jwk/RemoteJwkSetCache.java
+++ b/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/jwk/RemoteJwkSetCache.java
@@ -123,6 +123,21 @@ public class RemoteJwkSetCache extends AbstractIdentifiableInitializableComponen
}
}
+ /**
+ * Returns remote JWK set if found from the cache, otherwise fetches and stores it. If the JWK set is from the cache
+ * check it contains the keyId and if not, refresh the JWK set in the cache. Uses the default
+ * {@literal CONTEXT_NAME}. Delegates to {@link #fetch(String, URI, String, Instant)} for the actual implementation.
+ *
+ * @param uri value to check
+ * @param keyId the identifier of the key to check exists in a cached version of the keyset document
+ * @param expires time for disposal of value from cache
+ *
+ * @return JWK set, null if not found from the cache and cannot be fetched.
+ */
+ public JWKSet fetch(@Nonnull final URI uri, @Nonnull final String keyId, @Nonnull final Instant expires) {
+ return fetch(CONTEXT_NAME, uri, keyId, expires);
+ }
+
/**
* Returns remote JWK set if found from the cache, otherwise fetches and stores it.
*
@@ -157,18 +172,11 @@ public class RemoteJwkSetCache extends AbstractIdentifiableInitializableComponen
try {
final StorageRecord<?> entry = storage.read(context, key);
if (entry == null) {
- log.debug("Value '{}' was not in the cache, fetching it", key);
- final JWKSet remoteJwkSet = RemoteJwkUtils.fetchRemoteJwkSet("RemoteJwkSetCache", uri, httpClient,
- httpClientSecurityParameters);
- if (remoteJwkSet != null && remoteJwkSet.getKeys() != null && !remoteJwkSet.getKeys().isEmpty()) {
- storage.create(context, key, remoteJwkSet.toString(), expires.toEpochMilli());
- return remoteJwkSet;
- } else {
- log.warn("Could not find any remote keys from {}", key);
- }
+ log.debug("JWK set '{}' was not in the cache, fetching it", key);
+ return fetchAndStore(context, key, uri, expires);
} else {
final JWKSet cachedSet = JWKSet.parse(entry.getValue());
- log.debug("Cached value found and will be returned, expires at {}", entry.getExpiration());
+ log.debug("Cached JWK set '{}' found and will be returned, expires at {}", key, entry.getExpiration());
return cachedSet;
}
} catch (final IOException | java.text.ParseException e) {
@@ -177,4 +185,83 @@ public class RemoteJwkSetCache extends AbstractIdentifiableInitializableComponen
return null;
}
+
+ /**
+ * Fetches the remote JWK set from the given URI and stores it in the storage service.
+ *
+ * @param context a context label to subdivide the cache
+ * @param cacheKey the key to store the JWK set under in the storage service
+ * @param uri value to fetch the JWK set from
+ * @param expires time (in milliseconds since beginning of epoch) for disposal of value from cache
+ * @return the JWK set document if fetched successfully, {@code null} otherwise.
+ */
+ @Nullable private JWKSet fetchAndStore(@Nonnull @NotEmpty final String context, @Nonnull final String cacheKey,
+ @Nonnull final URI uri, @Nonnull final Instant expires) {
+ try {
+ final JWKSet remoteJwkSet = RemoteJwkUtils.fetchRemoteJwkSet("RemoteJwkSetCache", uri, httpClient,
+ httpClientSecurityParameters);
+ if (remoteJwkSet != null && remoteJwkSet.getKeys() != null && !remoteJwkSet.getKeys().isEmpty()) {
+ storage.create(context, cacheKey, remoteJwkSet.toString(), expires.toEpochMilli());
+ return remoteJwkSet;
+ } else {
+ log.warn("Could not find any remote keys from {}", cacheKey);
+ return null;
+ }
+ } catch (final IOException e) {
+ log.error("Exception reading/writing to storage service", e);
+ return null;
+ }
+ }
+
+ /**
+ * Returns remote JWK set if found from the cache, otherwise fetches and stores it. If the JWK set is retrieved
+ * from the cache, checks it contains the keyId, if not it re-fetches the JWK set even if the set has not expired.
+ * This allows keys to be returned when the JWK Set has been updated but has not yet expired e.g. during key
+ * rotation.
+ *
+ * @param context a context label to subdivide the cache
+ * @param uri value to check
+ * @param keyId the identifier of the key to check exists in a cached version of the keyset document
+ * @param expires time (in milliseconds since beginning of epoch) for disposal of value from cache
+ *
+ * @return JWK set, null if not found from the cache and cannot be fetched.
+ */
+ @Nullable public JWKSet fetch(@Nonnull @NotEmpty final String context, @Nonnull final URI uri,
+ @Nonnull final String keyId, @Nonnull final Instant expires) {
+ final String key = uri.toString();
+
+ final StorageCapabilities caps = storage.getCapabilities();
+ if (context.length() > caps.getContextSize()) {
+ log.error("context {} too long for StorageService (limit {})", context, caps.getContextSize());
+ return null;
+ }
+
+ try {
+ final StorageRecord<?> entry = storage.read(context, key);
+ if (entry == null) {
+ log.debug("JWK set '{}' was not in the cache, fetching it", key);
+ return fetchAndStore(context, key, uri, expires);
+ } else {
+ final JWKSet cachedSet = JWKSet.parse(entry.getValue());
+
+ if (cachedSet.getKeyByKeyId(keyId) == null) {
+ // Cached set does not contain the keyId so re-fetch the document and return (does not re-evaluate
+ // if the key is contained in the fetched document as the document is the latest and nothing else
+ // can be done).
+ log.debug("Cached JWK set does not contain the keyId '{}', ignoring expiry and re-fetching", keyId);
+ storage.delete(context, key);
+ return fetchAndStore(context, key, uri, expires);
+
+ } else {
+ log.debug("Cached JWK set '{}' found and will be returned, expires at {}", key,
+ entry.getExpiration());
+ return cachedSet;
+ }
+ }
+ } catch (final IOException | java.text.ParseException e) {
+ log.error("Exception reading/writing to storage service", e);
+ }
+
+ return null;
+ }
}
diff --git a/oidc-common-crypto-api/src/test/java/net/shibboleth/oidc/jwk/RemoteJwkSetCacheTest.java b/oidc-common-crypto-api/src/test/java/net/shibboleth/oidc/jwk/RemoteJwkSetCacheTest.java
index e50acfd..3b3409a 100644
--- a/oidc-common-crypto-api/src/test/java/net/shibboleth/oidc/jwk/RemoteJwkSetCacheTest.java
+++ b/oidc-common-crypto-api/src/test/java/net/shibboleth/oidc/jwk/RemoteJwkSetCacheTest.java
@@ -48,7 +48,6 @@ public class RemoteJwkSetCacheTest {
RemoteJwkSetCache jwkSetCache;
StorageService storageService;
- HttpClient httpClient;
@BeforeMethod
@@ -58,7 +57,7 @@ public class RemoteJwkSetCacheTest {
}
protected StorageService buildStorageService() throws ComponentInitializationException {
- MemoryStorageService storageService = new MemoryStorageService();
+ final MemoryStorageService storageService = new MemoryStorageService();
storageService.setId("mockId");
storageService.initialize();
return storageService;
@@ -84,7 +83,6 @@ public class RemoteJwkSetCacheTest {
jwkSetCache.setHttpClientSecurityParameters(null);
jwkSetCache.initialize();
final String uri = "http://example.org";
- System.out.println(new URI(uri).toString());
JWKSet jwkSet = jwkSetCache.fetch(new URI(uri), Instant.now().plusSeconds(5));
Assert.assertNotNull(jwkSet);
Assert.assertNotNull(jwkSetCache.getStorage().read(RemoteJwkSetCache.CONTEXT_NAME, uri));
@@ -93,6 +91,47 @@ public class RemoteJwkSetCacheTest {
Thread.sleep(5001);
Assert.assertNull(storageService.read(RemoteJwkSetCache.CONTEXT_NAME, uri));
}
+
+ @Test
+ public void testKeyIdNotInCachedSet() throws Exception{
+ jwkSetCache.setStorage(storageService);
+ jwkSetCache.setHttpClient(createMockHttpClient(validJwkSet()));
+ jwkSetCache.setHttpClientSecurityParameters(null);
+ jwkSetCache.initialize();
+ final String uri = "http://example.org";
+
+ // Initial lookup to cache keyset
+ jwkSetCache.fetch(new URI(uri), Instant.now().plusSeconds(500));
+
+ // Now lookup key that is not in the cached set
+ final JWKSet jwkSet = jwkSetCache.fetch(new URI(uri), "key-not-in-keyset", Instant.now().plusSeconds(500));
+ Assert.assertNotNull(jwkSet);
+ Assert.assertTrue(jwkSet.getKeyByKeyId("key-not-in-keyset") == null);
+ }
+
+ @Test
+ public void testKeyIdNotInCachedSet_Refetch() throws Exception{
+ jwkSetCache.setStorage(storageService);
+ final HttpClient client = createMockHttpClient(validJwkSet());
+ jwkSetCache.setHttpClient(client);
+ jwkSetCache.setHttpClientSecurityParameters(null);
+ jwkSetCache.initialize();
+ final String uri = "http://example.org";
+
+ // Initial lookup to cache keyset
+ jwkSetCache.fetch(new URI(uri), Instant.now().plusSeconds(500));
+
+ // Change the HTTP Client response to the new document
+ final HttpResponse httpResponse = Mockito.mock(HttpResponse.class);
+ Mockito.when(httpResponse.getEntity()).thenReturn(new StringEntity(rotatedValidJwkSet()));
+ Mockito.when(client.execute((HttpUriRequest) Mockito.any(),
+ (HttpContext) Mockito.any())).thenReturn(httpResponse);
+
+ // Now lookup key that is not in the cached set
+ final JWKSet jwkSet = jwkSetCache.fetch(new URI(uri), "key-in-rotated-keyset", Instant.now().plusSeconds(500));
+ Assert.assertNotNull(jwkSet);
+ Assert.assertTrue(jwkSet.getKeyByKeyId("key-in-rotated-keyset") != null);
+ }
@Test
public void testInvalidJwk() throws ClientProtocolException, IOException, ComponentInitializationException,
@@ -104,7 +143,7 @@ public class RemoteJwkSetCacheTest {
Assert.assertNull(jwkSet);
}
- protected HttpClient createMockHttpClient(String output) throws ClientProtocolException, IOException {
+ protected HttpClient createMockHttpClient(final String output) throws ClientProtocolException, IOException {
final HttpClient httpClient = Mockito.mock(HttpClient.class);
final HttpResponse httpResponse = Mockito.mock(HttpResponse.class);
Mockito.when(httpResponse.getEntity()).thenReturn(new StringEntity(output));
@@ -135,5 +174,20 @@ public class RemoteJwkSetCacheTest {
" ]\n" +
"}";
}
+
+ protected String rotatedValidJwkSet() {
+ return "{\n" +
+ " \"keys\": [\n" +
+ " {\n" +
+ " \"kid\": \"key-in-rotated-keyset\",\n" +
+ " \"e\": \"AQAB\",\n" +
+ " \"kty\": \"RSA\",\n" +
+ " \"alg\": \"RS256\",\n" +
+ " \"n\": \"mSLCSG1hK28xrzcSfgbvRinkIRjecBlwsQggynHppHiiT6I80waivIqTJBSFYyVuRCAHXi6apSsL5FUWKd42GOhVUayIyzvuz1CqTuh5a9ACXaJjEVLUFO39QfXxWrxhpSJCTN9aMkdtoV1QJqfAd3IF9MYwfojsoEn3d5XX5TX4RxqZ9-HGbgSLsRuAzFIg9NxxfTYhbECBskhhR4RIcam-1T52FafmK2LMiuIEDPiVg6LvAqWi8gdMRd8WhiP_ZIRJTCH4C0NFKmw1PZyKadVxvwg97vwPTF8qkFdwJ_kjQAMmq77PxankluAkfWjFqbD4JepO4HH3aJvU8Sl_Ow\",\n" +
+ " \"use\": \"sig\"\n" +
+ " }" +
+ " ]\n" +
+ "}";
+ }
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list