[java-oidc-common] branch main updated: JCOMOIDC-23 - Add OpenID Provider Configuration Document Resolver
Phil Smart
philip.smart at jisc.ac.uk
Tue Mar 8 10:50:31 UTC 2022
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=c225ee0e01717a0086c13250554bb8d08f991f36
The following commit(s) were added to refs/heads/main by this push:
new c225ee0 JCOMOIDC-23 - Add OpenID Provider Configuration Document Resolver
c225ee0 is described below
commit c225ee0e01717a0086c13250554bb8d08f991f36
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Tue Mar 8 10:50:21 2022 +0000
JCOMOIDC-23 - Add OpenID Provider Configuration Document Resolver
- Prevent expired metadata from being returned if new metadata not
resolved
- Allow backing store to be nullable
- Cleanup
https://shibboleth.atlassian.net/browse/JCOMOIDC-23
---
oidc-common-crypto-impl/BackingStore.java | 20 ++++++
.../metadata/cache/impl/AbstractMetadataCache.java | 28 ++++++--
.../metadata/cache/impl/DynamicMetadataCache.java | 12 +++-
.../cache/impl/BatchMetadataCacheTest.java | 5 +-
.../cache/impl/DynamicMetadataCacheTest.java | 83 +++++++++++++++-------
...PProviderConfigurationFetchingStrategyTest.java | 2 +-
6 files changed, 108 insertions(+), 42 deletions(-)
diff --git a/oidc-common-crypto-impl/BackingStore.java b/oidc-common-crypto-impl/BackingStore.java
index ea8e9f9..465086a 100644
--- a/oidc-common-crypto-impl/BackingStore.java
+++ b/oidc-common-crypto-impl/BackingStore.java
@@ -1,5 +1,25 @@
+/*
+ * Licensed to the University Corporation for Advanced Internet Development,
+ * Inc. (UCAID) under one or more contributor license agreements. See the
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID licenses this file to You under the Apache
+ * License, Version 2.0 (the "License"); you may not use this file except in
+ * compliance with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
package net.shibboleth.oidc.metadata.impl;
+/**
+ * A backing store marker interface.
+ */
public interface BackingStore {
}
diff --git a/oidc-common-metadata-impl/src/main/java/net/shibboleth/oidc/metadata/cache/impl/AbstractMetadataCache.java b/oidc-common-metadata-impl/src/main/java/net/shibboleth/oidc/metadata/cache/impl/AbstractMetadataCache.java
index 4c6abee..7a270ab 100644
--- a/oidc-common-metadata-impl/src/main/java/net/shibboleth/oidc/metadata/cache/impl/AbstractMetadataCache.java
+++ b/oidc-common-metadata-impl/src/main/java/net/shibboleth/oidc/metadata/cache/impl/AbstractMetadataCache.java
@@ -81,7 +81,7 @@ public abstract class AbstractMetadataCache<IdentifierType, MetadataType>
@NonnullAfterInit @Positive private Float refreshDelayFactor;
/** Backing store for runtime metadata.*/
- @Nonnull private final BackingStore<IdentifierType, MetadataType> backingStore;
+ @Nullable private final BackingStore<IdentifierType, MetadataType> backingStore;
/**
* A hook that is executed just before a cache entry will been removed/invalidated/evicted.
@@ -112,7 +112,7 @@ public abstract class AbstractMetadataCache<IdentifierType, MetadataType>
*
* @param store the metadata backing store.
*/
- AbstractMetadataCache(@Nonnull final BackingStore<IdentifierType, MetadataType> store) {
+ AbstractMetadataCache(@Nullable final BackingStore<IdentifierType, MetadataType> store) {
this(store, null);
}
@@ -126,9 +126,9 @@ public abstract class AbstractMetadataCache<IdentifierType, MetadataType>
* @param store the backing store.
* @param executor the scheduled executor
*/
- AbstractMetadataCache(@Nonnull final BackingStore<IdentifierType, MetadataType> store,
+ AbstractMetadataCache(@Nullable final BackingStore<IdentifierType, MetadataType> store,
@Nullable final ScheduledExecutorService executor) {
- backingStore = Constraint.isNotNull(store, "A backingstore must be set");
+ backingStore = store;
if (executor != null) {
executorService = executor;
createOwnSchedular = false;
@@ -165,7 +165,7 @@ public abstract class AbstractMetadataCache<IdentifierType, MetadataType>
if (metadataFilterStrategy == null) {
throw new ComponentInitializationException("Metadata filter strategy can not be null");
}
- if (refreshDelayFactor == null || backingStore == null) {
+ if (refreshDelayFactor == null) {
throw new ComponentInitializationException("Metadata cache not property initialized");
}
if (metadataValidPredicate == null) {
@@ -359,6 +359,9 @@ public abstract class AbstractMetadataCache<IdentifierType, MetadataType>
metadataIter.remove();
}
}
+ if (!metadata.isEmpty()) {
+ log.trace("{} Metadata cache has found '{}' entry(s) for '{}'", getLogPrefix(), metadata.size(), identifier);
+ }
return metadata;
}
@@ -382,9 +385,9 @@ public abstract class AbstractMetadataCache<IdentifierType, MetadataType>
/**
* Get the backing store.
*
- * @return the backing store.
+ * @return the backing store. Can be {@literal null}.
*/
- @Nonnull protected BackingStore<IdentifierType, MetadataType> getBackingStore() {
+ @Nullable protected BackingStore<IdentifierType, MetadataType> getBackingStore() {
return backingStore;
}
@@ -467,6 +470,17 @@ public abstract class AbstractMetadataCache<IdentifierType, MetadataType>
}
+ /**
+ * Determine if the metadata has expired based on the expiration time set in the managment metadata.
+ *
+ * @param mgmtData
+ * @return
+ */
+ protected boolean hasExpired(@Nonnull final MetadataManagementData<IdentifierType> mgmtData) {
+ return Instant.now().isAfter(mgmtData.getExpirationTime());
+ }
+
+
/**
* Remove/discard from the backing store all metadata for the entity with the given identifier.
*
diff --git a/oidc-common-metadata-impl/src/main/java/net/shibboleth/oidc/metadata/cache/impl/DynamicMetadataCache.java b/oidc-common-metadata-impl/src/main/java/net/shibboleth/oidc/metadata/cache/impl/DynamicMetadataCache.java
index e4628e9..bf5aae0 100644
--- a/oidc-common-metadata-impl/src/main/java/net/shibboleth/oidc/metadata/cache/impl/DynamicMetadataCache.java
+++ b/oidc-common-metadata-impl/src/main/java/net/shibboleth/oidc/metadata/cache/impl/DynamicMetadataCache.java
@@ -200,7 +200,7 @@ public class DynamicMetadataCache<IdentifierType, MetadataType>
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
- this.fetchStrategy = Constraint.isNotNull(strategy, "Dynamic Metadata fetch strategy can not be null");
+ fetchStrategy = Constraint.isNotNull(strategy, "Dynamic Metadata fetch strategy can not be null");
}
/**
@@ -463,7 +463,7 @@ public class DynamicMetadataCache<IdentifierType, MetadataType>
return;
}
} else {
- log.debug("{} Metadata for '{}' is stale and requires refreshing", getLogPrefix(), identifier);
+ log.trace("{} Metadata for '{}' is stale and requires refreshing", getLogPrefix(), identifier);
}
MetadataType resolvedMetadata = null;
@@ -601,10 +601,15 @@ public class DynamicMetadataCache<IdentifierType, MetadataType>
*
* @throws MetadataCacheException on error.
*/
- //TODO: Support isValid checks on returned metadata? see AbstractMetadataResolver#lookupEntityID
@Nonnull private List<MetadataType> read(@Nonnull final MetadataManagementData<IdentifierType> mgmtData,
@Nonnull final IdentifierType identifier) throws MetadataCacheException {
+ // Do not read if expired.
+ if (hasExpired(mgmtData)) {
+ log.trace("{} Metadata has expired for '{}'", getLogPrefix(), identifier);
+ return Collections.emptyList();
+ }
+
// record access attempt.
mgmtData.recordEntityAccess();
// get optimistic lock
@@ -629,6 +634,7 @@ public class DynamicMetadataCache<IdentifierType, MetadataType>
}
}
+
/**
* Cleanup task that removes expired and idle metadata from the backing store.
diff --git a/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/cache/impl/BatchMetadataCacheTest.java b/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/cache/impl/BatchMetadataCacheTest.java
index 44a57ca..bb941f6 100644
--- a/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/cache/impl/BatchMetadataCacheTest.java
+++ b/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/cache/impl/BatchMetadataCacheTest.java
@@ -56,22 +56,19 @@ import net.shibboleth.oidc.metadata.impl.DefaultBatchBackingStore;
import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
import net.shibboleth.utilities.java.support.resolver.CriteriaSet;
+/** Tests for the {@link BatchMetadataCache}.*/
public class BatchMetadataCacheTest {
- // OIDC provider metadata cache
private BatchMetadataCache<Issuer, OIDCProviderMetadata> cache;
private ManuallyTriggeredScheduledExecutorService scheduler;
- @Nonnull
private LoadingStrategy defaultLoadingStrategy;
- @Nonnull
private Function<byte[], List<OIDCProviderMetadata>> defaultParsingStrategy;
@BeforeMethod
void setup() throws Exception {
-
defaultLoadingStrategy = new LoadingStrategy() {
diff --git a/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/cache/impl/DynamicMetadataCacheTest.java b/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/cache/impl/DynamicMetadataCacheTest.java
index 3c13ca4..baad887 100644
--- a/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/cache/impl/DynamicMetadataCacheTest.java
+++ b/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/cache/impl/DynamicMetadataCacheTest.java
@@ -17,6 +17,7 @@
package net.shibboleth.oidc.metadata.cache.impl;
+import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertSame;
import static org.testng.Assert.assertTrue;
@@ -83,7 +84,7 @@ public class DynamicMetadataCacheTest {
cache = new DynamicMetadataCache<Issuer, OIDCProviderMetadata>(
new DefaultDynamicBackingStore<>(), scheduler);
cache.setFetchStrategy(defaultFetchStrategy);
- cache.setIdentifierExtractionStrategy(m -> m.getIssuer());
+ cache.setIdentifierExtractionStrategy(OIDCProviderMetadata::getIssuer);
cache.setMetadataExpirationTimeStrategy(ctx -> ctx.getNow().plus(Duration.ofMinutes(5)));
cache.setCriteriaToIdentifierStrategy(crit -> {
final IssuerIDCriterion issuerId = crit.get(IssuerIDCriterion.class);
@@ -119,12 +120,12 @@ public class DynamicMetadataCacheTest {
throws ComponentInitializationException, URISyntaxException, InterruptedException {
// Give our own executor, so we do not need to wait.
- ManuallyTriggeredScheduledExecutorService scheduler = new ManuallyTriggeredScheduledExecutorService();
- DynamicMetadataCache<Issuer, OIDCProviderMetadata> cacheLocal =
+ final ManuallyTriggeredScheduledExecutorService scheduler = new ManuallyTriggeredScheduledExecutorService();
+ final DynamicMetadataCache<Issuer, OIDCProviderMetadata> cacheLocal =
new DynamicMetadataCache<Issuer, OIDCProviderMetadata>(new DefaultDynamicBackingStore<>(),scheduler);
cacheLocal.setFetchStrategy(defaultFetchStrategy);
// use a cache local to this method
- cacheLocal.setIdentifierExtractionStrategy(m -> m.getIssuer());
+ cacheLocal.setIdentifierExtractionStrategy(OIDCProviderMetadata::getIssuer);
cacheLocal.setMetadataExpirationTimeStrategy(ctx -> ctx.getNow().plus(Duration.ofMinutes(10)));
cacheLocal.setCriteriaToIdentifierStrategy(crit -> crit.get(IssuerIDCriterion.class).getIssuerID());
//test a simple logging hook
@@ -156,7 +157,7 @@ public class DynamicMetadataCacheTest {
// create some metadata to add - probably ignored as needs refereshing
- OIDCProviderMetadata metadata =
+ final OIDCProviderMetadata metadata =
new OIDCProviderMetadata(iss, List.of(SubjectType.PUBLIC), new URI("http://example.oidc.op.org/metadata"));
cacheLocal.getBackingStore().getOrderedValues().add(metadata);
cacheLocal.getBackingStore().getIndexedValues().put(iss, List.of(metadata));
@@ -183,6 +184,40 @@ public class DynamicMetadataCacheTest {
final Issuer iss = new Issuer("https://example.oidc.op.org");
cacheLocal.get(new CriteriaSet(new IssuerIDCriterion(iss)));
}
+
+ @Test
+ public void testPreExpiredMetadata() throws Exception {
+
+ cache.setMetadataExpirationTimeStrategy(ctx -> ctx.getNow().minus(Duration.ofMinutes(5)));
+ cache.setMinCacheDuration(Duration.ofSeconds(0));
+ cache.initialize();
+ final Issuer iss = new Issuer("https://example.oidc.op.org");
+ final var metadata = cache.get(new CriteriaSet(new IssuerIDCriterion(iss)));
+ assertEquals(metadata.size(), 0);
+
+ }
+
+ @Test
+ public void testFetchFailsAndCachedIsExpired() throws Exception {
+ // Add expired record to the cache
+ final Issuer iss = new Issuer("https://example.oidc.op.org");
+
+ final MetadataManagementData<Issuer> mgmtData = cache.getBackingStore()
+ .computeManagementDataIfAbsent(iss, MetadataManagementData::new);
+ mgmtData.setExpirationTime(Instant.now().minus(Duration.ofSeconds(1)));
+ mgmtData.setRefreshTriggerTime(Instant.now());
+ cache.getBackingStore().getIndexedValues().put(iss,
+ List.of(new OIDCProviderMetadata(iss, List.of(SubjectType.PUBLIC), new URI("http://example.oidc.op.org/metadata"))));
+
+ cache.setMetadataExpirationTimeStrategy(ctx -> ctx.getNow().minus(Duration.ofMinutes(5)));
+ cache.setMinCacheDuration(Duration.ofSeconds(0));
+ cache.setFetchStrategy(c -> null);
+ cache.initialize();
+
+ final var metadata = cache.get(new CriteriaSet(new IssuerIDCriterion(iss)));
+ assertEquals(metadata.size(), 0);
+
+ }
/* FIXME disabled as not working on Jenkins, will fix.*/
@Test(enabled = false)
@@ -196,7 +231,7 @@ public class DynamicMetadataCacheTest {
// use a cache local to this method
cacheLocal.setFetchStrategy(defaultFetchStrategy);
- cacheLocal.setIdentifierExtractionStrategy(m -> m.getIssuer());
+ cacheLocal.setIdentifierExtractionStrategy(OIDCProviderMetadata::getIssuer);
cacheLocal.setMetadataExpirationTimeStrategy(ctx -> ctx.getNow().plus(Duration.ofMinutes(10)));
cacheLocal.setCriteriaToIdentifierStrategy(crit -> crit.get(IssuerIDCriterion.class).getIssuerID());
@@ -230,7 +265,7 @@ public class DynamicMetadataCacheTest {
mgmtData.recordEntityAccess();
// create some metadata to add - probably ignored as needs refereshing
- OIDCProviderMetadata metadata =
+ final OIDCProviderMetadata metadata =
new OIDCProviderMetadata(iss, List.of(SubjectType.PUBLIC), new URI("http://example.oidc.op.org/metadata"));
cacheLocal.getBackingStore().getOrderedValues().add(metadata);
cacheLocal.getBackingStore().getIndexedValues().put(iss, List.of(metadata));
@@ -266,7 +301,7 @@ public class DynamicMetadataCacheTest {
mgmtData.setLastUpdateTime(firstUpdateTime);
// create some metadata to add - probably ignored as needs refreshing
- OIDCProviderMetadata metadata =
+ final OIDCProviderMetadata metadata =
new OIDCProviderMetadata(iss, List.of(SubjectType.PUBLIC), new URI("http://example.oidc.op.org/metadata"));
cache.getBackingStore().getOrderedValues().add(metadata);
cache.getBackingStore().getIndexedValues().put(iss, List.of(metadata));
@@ -286,7 +321,7 @@ public class DynamicMetadataCacheTest {
@Test
public void testGetNotCached_Success() throws MetadataCacheException, ComponentInitializationException {
cache.initialize();
- List<OIDCProviderMetadata> metadata =
+ final List<OIDCProviderMetadata> metadata =
cache.get(new CriteriaSet(new IssuerIDCriterion(new Issuer("the-issuer"))));
assertTrue(metadata.isEmpty() == false);
@@ -305,7 +340,7 @@ public class DynamicMetadataCacheTest {
} catch (final URISyntaxException e) {
return null;
}});
- localCache.setIdentifierExtractionStrategy(m -> m.getIssuer());
+ localCache.setIdentifierExtractionStrategy(OIDCProviderMetadata::getIssuer);
localCache.setMetadataExpirationTimeStrategy(ctx -> ctx.getNow().plus(Duration.ofMinutes(5)));
localCache.setCriteriaToIdentifierStrategy(crit -> {
final IssuerIDCriterion issuerId = crit.get(IssuerIDCriterion.class);
@@ -326,7 +361,7 @@ public class DynamicMetadataCacheTest {
localCache.setId("MockCache");
localCache.initialize();
- List<OIDCProviderMetadata> metadata =
+ final List<OIDCProviderMetadata> metadata =
localCache.get(new CriteriaSet(new IssuerIDCriterion(new Issuer("the-issuer"))));
assertTrue(metadata.isEmpty());
@@ -335,7 +370,7 @@ public class DynamicMetadataCacheTest {
@Test
public void testGetNotCached_WrongCriteria_Fail() throws MetadataCacheException, ComponentInitializationException {
cache.initialize();
- List<OIDCProviderMetadata> metadata =
+ final List<OIDCProviderMetadata> metadata =
cache.get(new CriteriaSet(new EntityIdCriterion("wrong-criteria")));
assertTrue(metadata.isEmpty());
@@ -346,22 +381,16 @@ public class DynamicMetadataCacheTest {
MetadataCacheException, ComponentInitializationException {
cache.initialize();
final ExecutorService service = Executors.newFixedThreadPool(3);
- Future<?> futureOne = service.submit(() -> {
- return cache.get(new CriteriaSet(new IssuerIDCriterion(new Issuer("the-issuer"))));
- });
- Future<?> futureTwo = service.submit(() -> {
- return cache.get(new CriteriaSet(new IssuerIDCriterion(new Issuer("the-issuer"))));
- });
+ final Future<?> futureOne = service.submit(() -> cache.get(new CriteriaSet(new IssuerIDCriterion(new Issuer("the-issuer")))));
+ final Future<?> futureTwo = service.submit(() -> cache.get(new CriteriaSet(new IssuerIDCriterion(new Issuer("the-issuer")))));
// different entity
- Future<?> futureThree = service.submit(() -> {
- return cache.get(new CriteriaSet(new IssuerIDCriterion(new Issuer("the-issuer"))));
- });
- List<?> firstMetadata = (List<?>) futureOne.get();
- List<?> secondMetadata = (List<?>) futureThree.get();
- List<?> thirdMetadata = (List<?>) futureTwo.get();
+ final Future<?> futureThree = service.submit(() -> cache.get(new CriteriaSet(new IssuerIDCriterion(new Issuer("the-issuer")))));
+ final List<?> firstMetadata = (List<?>) futureOne.get();
+ final List<?> secondMetadata = (List<?>) futureThree.get();
+ final List<?> thirdMetadata = (List<?>) futureTwo.get();
// non-interleaved request
- List<OIDCProviderMetadata> provider =
+ final List<OIDCProviderMetadata> provider =
cache.get(new CriteriaSet(new IssuerIDCriterion(new Issuer("the-issuer"))));
assertTrue(firstMetadata.size() == 1);
assertTrue(secondMetadata.size() == 1);
@@ -378,11 +407,11 @@ public class DynamicMetadataCacheTest {
@Test
public void testGetCached_Success() throws MetadataCacheException, ComponentInitializationException {
cache.initialize();
- List<OIDCProviderMetadata> metadata =
+ final List<OIDCProviderMetadata> metadata =
cache.get(new CriteriaSet(new IssuerIDCriterion(new Issuer("the-issuer"))));
assertTrue(metadata.size() == 1);
- List<OIDCProviderMetadata> metadataCached =
+ final List<OIDCProviderMetadata> metadataCached =
cache.get(new CriteriaSet(new IssuerIDCriterion(new Issuer("the-issuer"))));
assertTrue(metadata.size() == 1);
// first was cached, so this should be the same
diff --git a/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/impl/HTTPProviderConfigurationFetchingStrategyTest.java b/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/impl/HTTPProviderConfigurationFetchingStrategyTest.java
index c61532c..3851205 100644
--- a/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/impl/HTTPProviderConfigurationFetchingStrategyTest.java
+++ b/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/impl/HTTPProviderConfigurationFetchingStrategyTest.java
@@ -63,7 +63,7 @@ public class HTTPProviderConfigurationFetchingStrategyTest {
final var metadata =
new ClassPathResource("/net/shibboleth/oidc/metadata/impl/openid-configuration.json");
final var metadataAsString = CharStreams.toString(new InputStreamReader(
- metadata.getInputStream(), StandardCharsets.UTF_8));;
+ metadata.getInputStream(), StandardCharsets.UTF_8));
Mockito.when(httpResponse.getEntity()).thenReturn(new StringEntity(metadataAsString));
Mockito.when(httpClient.execute((HttpUriRequest) Mockito.any(), (ResponseHandler) Mockito.any(),
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list