From 8cdbef594e717639653ee2a995a5fded05436f75 Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Fri, 7 Sep 2018 13:14:59 -0700 Subject: [PATCH 01/13] [SHIBUI-570] Added simple caching of fetchMetdata and unmarshallMetadata. --- .../MetadataResolversController.java | 2 + ...penSamlFileBackedHTTPMetadataResolver.java | 41 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java index 4d7d7aaac..2b4b90d65 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java @@ -118,6 +118,8 @@ public ResponseEntity create(@RequestBody MetadataResolver newResolver) throw MetadataResolver persistedResolver = resolverRepository.save(newResolver); positionOrderContainerService.appendPositionOrderForNew(persistedResolver); + //TODO: currently, the update call might explode, but the save works.. in which case, the UI never gets + // an valid response. This operation is not atomic. Should we return an error here? updateChainingMetadataResolver(persistedResolver); return ResponseEntity.created(getResourceUriFor(persistedResolver)).body(persistedResolver); diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFileBackedHTTPMetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFileBackedHTTPMetadataResolver.java index 5f4c10905..e74bff535 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFileBackedHTTPMetadataResolver.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFileBackedHTTPMetadataResolver.java @@ -8,21 +8,35 @@ import org.apache.http.impl.client.HttpClients; import org.apache.lucene.index.IndexWriter; import org.joda.time.DateTime; +import org.opensaml.core.xml.XMLObject; +import org.opensaml.core.xml.io.UnmarshallingException; import org.opensaml.saml.metadata.resolver.impl.FileBackedHTTPMetadataResolver; +import javax.annotation.Nonnull; import javax.annotation.Nullable; +import java.io.InputStream; +import java.time.Instant; + import static edu.internet2.tier.shibboleth.admin.util.DurationUtility.toMillis; /** * @author Bill Smith (wsmith@unicon.net) */ public class OpenSamlFileBackedHTTPMetadataResolver extends FileBackedHTTPMetadataResolver { + + private static final long MILLISECONDS_IN_ONE_SECOND = 1000; + private IndexWriter indexWriter; private FileBackedHttpMetadataResolver sourceResolver; private OpenSamlMetadataResolverDelegate delegate; + private byte[] cachedMetadataBytes; + private Instant metadataLastFetchedAt; + boolean shouldRefreshMetadata; + XMLObject cachedMetadata; + public OpenSamlFileBackedHTTPMetadataResolver(ParserPool parserPool, IndexWriter indexWriter, FileBackedHttpMetadataResolver sourceResolver) throws ResolverException { @@ -67,4 +81,31 @@ protected void initMetadataResolver() throws ComponentInitializationException { this.sourceResolver.getResourceId(), indexWriter); } + + @Override + protected byte[] fetchMetadata() throws ResolverException { + if (metadataLastFetchedAt == null || shouldRefreshMetadata()) { + this.cachedMetadataBytes = super.fetchMetadata(); + this.metadataLastFetchedAt = Instant.now(); + } + return cachedMetadataBytes; + } + + private boolean shouldRefreshMetadata() { + if ((Instant.now().getEpochSecond() - metadataLastFetchedAt.getEpochSecond()) > (this.getMinRefreshDelay() / MILLISECONDS_IN_ONE_SECOND)) { + shouldRefreshMetadata = true; + } + return shouldRefreshMetadata; + } + + @Override + protected XMLObject unmarshallMetadata(@Nonnull final InputStream metadataInput) + throws UnmarshallingException { + //TODO: This should probably be based on something other than minRefreshDelay + if (cachedMetadata == null || shouldRefreshMetadata) { + this.cachedMetadata = super.unmarshallMetadata(metadataInput); + this.shouldRefreshMetadata = false; + } + return this.cachedMetadata; + } } From 459df7185baa2bda1a299c421798081b2da51e82 Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Fri, 7 Sep 2018 17:36:03 -0700 Subject: [PATCH 02/13] [SHIBUI-570] GRAMAAARRRRR --- .../admin/ui/controller/MetadataResolversController.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java index 2b4b90d65..7388679da 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java @@ -119,7 +119,7 @@ public ResponseEntity create(@RequestBody MetadataResolver newResolver) throw positionOrderContainerService.appendPositionOrderForNew(persistedResolver); //TODO: currently, the update call might explode, but the save works.. in which case, the UI never gets - // an valid response. This operation is not atomic. Should we return an error here? + // n valid response. This operation is not atomic. Should we return an error here? updateChainingMetadataResolver(persistedResolver); return ResponseEntity.created(getResourceUriFor(persistedResolver)).body(persistedResolver); From 75013d037ebad0a581bc22c1dd954db4b1ff164a Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Mon, 10 Sep 2018 16:56:33 -0700 Subject: [PATCH 03/13] [SHIBUI-570] Swapped MetadataResolverConverterServiceImpl's @Service annotation out for a new Configuration class with a new Bean declaration. How was this even working? Updated OpenSamlChainingMetadataResolver to initialize mutableResolvers with a new ArrayList instead of Collections.emptyList. Big mistake. Also, overrode resolve() to use the mutableResolvers list instead of the (empty) original resolvers list. Added new OpenSamlBatchMetadataResolverDelegate to handle refilter(). Added refilter() to all Batch-ish OpenSaml resolvers. Removed refreshOrInitResolver from filter controller. Not needed. Added refilter() calls in JPAMetadataResolverServiceImpl.reloadFilters. --- ...OpenSamlBatchMetadataResolverDelegate.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlBatchMetadataResolverDelegate.java diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlBatchMetadataResolverDelegate.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlBatchMetadataResolverDelegate.java new file mode 100644 index 000000000..855de838f --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlBatchMetadataResolverDelegate.java @@ -0,0 +1,19 @@ +package edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml; + +import org.opensaml.core.xml.XMLObject; +import org.opensaml.saml.metadata.resolver.impl.AbstractBatchMetadataResolver; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * @author Bill Smith (wsmith@unicon.net) + */ +public class OpenSamlBatchMetadataResolverDelegate extends AbstractBatchMetadataResolver { + private static final Logger logger = LoggerFactory.getLogger(OpenSamlBatchMetadataResolverDelegate.class); + + //TODO: Not sure this delegate is really buying us anything.. other than to get this one line in to a shared class. + //Maybe we'll do more in here later? + public void refilter(AbstractBatchMetadataResolver.BatchEntityBackingStore backingStore, XMLObject filteredMetadata) { + backingStore.setCachedFilteredMetadata(filteredMetadata); + } +} From fdbe478994c9c411942d61fb12dce56970e85634 Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Tue, 11 Sep 2018 08:11:16 -0700 Subject: [PATCH 04/13] [SHIBUI-570] Swapped MetadataResolverConverterServiceImpl's @Service annotation out for a new Configuration class with a new Bean declaration. How was this even working? Updated OpenSamlChainingMetadataResolver to initialize mutableResolvers with a new ArrayList instead of Collections.emptyList. Big mistake. Also, overrode resolve() to use the mutableResolvers list instead of the (empty) original resolvers list. Added new OpenSamlBatchMetadataResolverDelegate to handle refilter(). Added refilter() to all Batch-ish OpenSaml resolvers. Removed refreshOrInitResolver from filter controller. Not needed. Added refilter() calls in JPAMetadataResolverServiceImpl.reloadFilters. --- .../JPAMetadataResolverServiceImpl.groovy | 22 ++++++---- ...etadataResolverConverterConfiguration.java | 17 ++++++++ .../controller/MetadataFiltersController.java | 29 ------------- .../OpenSamlChainingMetadataResolver.java | 30 ++++++++++++-- ...penSamlFileBackedHTTPMetadataResolver.java | 41 ++++++------------- .../OpenSamlFilesystemMetadataResolver.java | 15 +++++++ ...penSamlResourceBackedMetadataResolver.java | 15 +++++++ .../MetadataResolverConverterServiceImpl.java | 1 - ...JPAMetadataResolverServiceImplTests.groovy | 23 ++++++++--- 9 files changed, 117 insertions(+), 76 deletions(-) create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/MetadataResolverConverterConfiguration.java diff --git a/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy b/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy index a4a8451fd..df37471c2 100644 --- a/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy +++ b/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy @@ -11,20 +11,21 @@ import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.FileBackedHttpMet import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.FilesystemMetadataResolver import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.LocalDynamicMetadataResolver import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.ResourceBackedMetadataResolver +import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml.OpenSamlFileBackedHTTPMetadataResolver +import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml.OpenSamlFilesystemMetadataResolver +import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml.OpenSamlResourceBackedMetadataResolver import edu.internet2.tier.shibboleth.admin.ui.opensaml.OpenSamlObjects import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository import groovy.util.logging.Slf4j import groovy.xml.DOMBuilder import groovy.xml.MarkupBuilder -import net.shibboleth.utilities.java.support.logic.ScriptedPredicate -import net.shibboleth.utilities.java.support.resolver.ResolverException import net.shibboleth.utilities.java.support.scripting.EvaluableScript import org.opensaml.saml.common.profile.logic.EntityIdPredicate import org.opensaml.saml.metadata.resolver.ChainingMetadataResolver import org.opensaml.saml.metadata.resolver.MetadataResolver -import org.opensaml.saml.metadata.resolver.RefreshableMetadataResolver import org.opensaml.saml.metadata.resolver.filter.MetadataFilter import org.opensaml.saml.metadata.resolver.filter.MetadataFilterChain +import org.opensaml.saml.metadata.resolver.impl.AbstractBatchMetadataResolver import org.opensaml.saml.saml2.core.Attribute import org.opensaml.saml.saml2.metadata.EntityDescriptor import org.springframework.beans.factory.annotation.Autowired @@ -94,11 +95,16 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService { metadataFilterChain.setFilters(metadataFilters) } - if (metadataResolver instanceof RefreshableMetadataResolver) { - try { - ((RefreshableMetadataResolver) metadataResolver).refresh() - } catch (ResolverException e) { - log.warn("error refreshing metadataResolver " + metadataResolverName, e) + if (targetMetadataResolver != null && targetMetadataResolver instanceof AbstractBatchMetadataResolver) { + if (targetMetadataResolver instanceof OpenSamlFileBackedHTTPMetadataResolver) { + (OpenSamlFileBackedHTTPMetadataResolver) targetMetadataResolver.refilter() + } else if (targetMetadataResolver instanceof OpenSamlFilesystemMetadataResolver) { + (OpenSamlFilesystemMetadataResolver) targetMetadataResolver.refilter() + } else if (targetMetadataResolver instanceof OpenSamlResourceBackedMetadataResolver) { + (OpenSamlResourceBackedMetadataResolver) targetMetadataResolver.refilter() + } else { + //TODO: Do something here if we need to refilter other non-Batch resolvers + println("We shouldn't be here. But we are. Why?") } } } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/MetadataResolverConverterConfiguration.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/MetadataResolverConverterConfiguration.java new file mode 100644 index 000000000..6380e0018 --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/MetadataResolverConverterConfiguration.java @@ -0,0 +1,17 @@ +package edu.internet2.tier.shibboleth.admin.ui.configuration; + +import edu.internet2.tier.shibboleth.admin.ui.service.MetadataResolverConverterService; +import edu.internet2.tier.shibboleth.admin.ui.service.MetadataResolverConverterServiceImpl; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +/** + * @author Bill Smith (wsmith@unicon.net) + */ +@Configuration +public class MetadataResolverConverterConfiguration { + @Bean + public MetadataResolverConverterService metadataResolverConverterService() { + return new MetadataResolverConverterServiceImpl(); + } +} diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java index 632809bea..ce50be62f 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java @@ -89,7 +89,6 @@ public ResponseEntity create(@PathVariable String metadataResolverId, @Reques // we reload the filters here after save metadataResolverService.reloadFilters(persistedMr.getName()); - refreshOrInitResolver(metadataResolver); MetadataFilter persistedFilter = newlyPersistedFilter(persistedMr.getMetadataFilters().stream(), createdFilter.getResourceId()); @@ -98,33 +97,6 @@ public ResponseEntity create(@PathVariable String metadataResolverId, @Reques .body(persistedFilter); } - private void refreshOrInitResolver(MetadataResolver resolver) { - List resolvers = ((ChainingMetadataResolver) chainingMetadataResolver).getResolvers(); - resolvers.stream().filter(it -> it.getId().equals(resolver.getResourceId())).forEach(it -> { - if (it instanceof RefreshableMetadataResolver) { - try { - ((RefreshableMetadataResolver) it).refresh(); - } catch (ResolverException e) { - //TODO what should we do if we can't refresh? - } - } else if (it instanceof OpenSamlFunctionDrivenDynamicHTTPMetadataResolver) { - try { - ((OpenSamlFunctionDrivenDynamicHTTPMetadataResolver) it).refresh(); - } catch (ComponentInitializationException e) { - //TODO what should we do if we can't refresh? - } - } else if (it instanceof OpenSamlLocalDynamicMetadataResolver) { - try { - ((OpenSamlLocalDynamicMetadataResolver) it).refresh(); - } catch (ComponentInitializationException e) { - //TODO what should we do if we can't refresh? - } - } else { - //TODO we shouldn't get here, but if we do... throw exception? - } - }); - } - @PutMapping("/Filters/{resourceId}") public ResponseEntity update(@PathVariable String metadataResolverId, @PathVariable String resourceId, @@ -159,7 +131,6 @@ public ResponseEntity update(@PathVariable String metadataResolverId, // TODO: this is wrong metadataResolverService.reloadFilters(metadataResolver.getName()); - refreshOrInitResolver(metadataResolver); return ResponseEntity.ok().body(persistedFilter); } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlChainingMetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlChainingMetadataResolver.java index aa5f45745..b798e347e 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlChainingMetadataResolver.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlChainingMetadataResolver.java @@ -4,14 +4,18 @@ import com.google.common.collect.Collections2; import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements; import net.shibboleth.utilities.java.support.component.ComponentInitializationException; +import net.shibboleth.utilities.java.support.component.ComponentSupport; +import net.shibboleth.utilities.java.support.resolver.CriteriaSet; import net.shibboleth.utilities.java.support.resolver.ResolverException; import org.opensaml.saml.metadata.resolver.ChainingMetadataResolver; import org.opensaml.saml.metadata.resolver.MetadataResolver; import org.opensaml.saml.metadata.resolver.RefreshableMetadataResolver; +import org.opensaml.saml.saml2.metadata.EntityDescriptor; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -25,7 +29,7 @@ public class OpenSamlChainingMetadataResolver extends ChainingMetadataResolver { @Nonnull @NonnullElements private List mutableResolvers; public OpenSamlChainingMetadataResolver() { - this.mutableResolvers = Collections.emptyList(); + this.mutableResolvers = new ArrayList<>(); } public OpenSamlChainingMetadataResolver(@Nonnull List mutableResolvers) { @@ -35,7 +39,7 @@ public OpenSamlChainingMetadataResolver(@Nonnull List mutableR @Override public void setResolvers(@Nonnull @NonnullElements final List newResolvers) { if (newResolvers == null || newResolvers.isEmpty()) { - mutableResolvers = Collections.emptyList(); + mutableResolvers = new ArrayList<>(); return; } @@ -54,7 +58,7 @@ protected void doInitialize() throws ComponentInitializationException { super.doInitialize(); if (mutableResolvers == null) { log.warn("OpenSamlChainingMetadataResolver was not configured with any member MetadataResolvers"); - mutableResolvers = Collections.emptyList(); + mutableResolvers = new ArrayList<>(); } } @@ -66,4 +70,24 @@ public void refresh() throws ResolverException { } } } + + @Override + @Nonnull public Iterable resolve(@Nullable final CriteriaSet criteria) throws ResolverException { + ComponentSupport.ifNotInitializedThrowUninitializedComponentException(this); + + for (final MetadataResolver resolver : mutableResolvers) { + try { + final Iterable descriptors = resolver.resolve(criteria); + if (descriptors != null && descriptors.iterator().hasNext()) { + return descriptors; + } + } catch (final ResolverException e) { + log.warn("Error retrieving metadata from resolver of type {}, proceeding to next resolver", + resolver.getClass().getName(), e); + continue; + } + } + + return Collections.emptyList(); + } } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFileBackedHTTPMetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFileBackedHTTPMetadataResolver.java index e74bff535..eac7debe7 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFileBackedHTTPMetadataResolver.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFileBackedHTTPMetadataResolver.java @@ -10,7 +10,10 @@ import org.joda.time.DateTime; import org.opensaml.core.xml.XMLObject; import org.opensaml.core.xml.io.UnmarshallingException; +import org.opensaml.saml.metadata.resolver.filter.FilterException; import org.opensaml.saml.metadata.resolver.impl.FileBackedHTTPMetadataResolver; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -25,17 +28,15 @@ */ public class OpenSamlFileBackedHTTPMetadataResolver extends FileBackedHTTPMetadataResolver { + private static final Logger logger = LoggerFactory.getLogger(OpenSamlFileBackedHTTPMetadataResolver.class); + private static final long MILLISECONDS_IN_ONE_SECOND = 1000; private IndexWriter indexWriter; private FileBackedHttpMetadataResolver sourceResolver; private OpenSamlMetadataResolverDelegate delegate; - - private byte[] cachedMetadataBytes; - private Instant metadataLastFetchedAt; - boolean shouldRefreshMetadata; - XMLObject cachedMetadata; + private OpenSamlBatchMetadataResolverDelegate batchDelegate; public OpenSamlFileBackedHTTPMetadataResolver(ParserPool parserPool, IndexWriter indexWriter, @@ -44,6 +45,7 @@ public OpenSamlFileBackedHTTPMetadataResolver(ParserPool parserPool, this.indexWriter = indexWriter; this.sourceResolver = sourceResolver; this.delegate = new OpenSamlMetadataResolverDelegate(); + this.batchDelegate = new OpenSamlBatchMetadataResolverDelegate(); this.setId(sourceResolver.getResourceId()); @@ -82,30 +84,11 @@ protected void initMetadataResolver() throws ComponentInitializationException { indexWriter); } - @Override - protected byte[] fetchMetadata() throws ResolverException { - if (metadataLastFetchedAt == null || shouldRefreshMetadata()) { - this.cachedMetadataBytes = super.fetchMetadata(); - this.metadataLastFetchedAt = Instant.now(); - } - return cachedMetadataBytes; - } - - private boolean shouldRefreshMetadata() { - if ((Instant.now().getEpochSecond() - metadataLastFetchedAt.getEpochSecond()) > (this.getMinRefreshDelay() / MILLISECONDS_IN_ONE_SECOND)) { - shouldRefreshMetadata = true; - } - return shouldRefreshMetadata; - } - - @Override - protected XMLObject unmarshallMetadata(@Nonnull final InputStream metadataInput) - throws UnmarshallingException { - //TODO: This should probably be based on something other than minRefreshDelay - if (cachedMetadata == null || shouldRefreshMetadata) { - this.cachedMetadata = super.unmarshallMetadata(metadataInput); - this.shouldRefreshMetadata = false; + public void refilter() { + try { + batchDelegate.refilter(this.getBackingStore(), filterMetadata(getCachedOriginalMetadata())); + } catch (FilterException e) { + logger.error("An error occurred while attempting to filter metadata!", e); } - return this.cachedMetadata; } } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFilesystemMetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFilesystemMetadataResolver.java index ad3ee65d9..e20fe632b 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFilesystemMetadataResolver.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFilesystemMetadataResolver.java @@ -5,7 +5,10 @@ import net.shibboleth.utilities.java.support.xml.ParserPool; import org.apache.lucene.index.IndexWriter; import org.joda.time.DateTime; +import org.opensaml.saml.metadata.resolver.filter.FilterException; import org.opensaml.saml.metadata.resolver.impl.FilesystemMetadataResolver; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import javax.annotation.Nullable; import java.io.File; @@ -14,9 +17,12 @@ * @author Bill Smith (wsmith@unicon.net) */ public class OpenSamlFilesystemMetadataResolver extends FilesystemMetadataResolver { + private static final Logger logger = LoggerFactory.getLogger(OpenSamlFilesystemMetadataResolver.class); + private IndexWriter indexWriter; private edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.FilesystemMetadataResolver sourceResolver; private OpenSamlMetadataResolverDelegate delegate; + private OpenSamlBatchMetadataResolverDelegate batchDelegate; public OpenSamlFilesystemMetadataResolver(ParserPool parserPool, IndexWriter indexWriter, @@ -26,6 +32,7 @@ public OpenSamlFilesystemMetadataResolver(ParserPool parserPool, this.indexWriter = indexWriter; this.sourceResolver = sourceResolver; this.delegate = new OpenSamlMetadataResolverDelegate(); + this.batchDelegate = new OpenSamlBatchMetadataResolverDelegate(); this.setId(sourceResolver.getResourceId()); @@ -48,4 +55,12 @@ protected void initMetadataResolver() throws ComponentInitializationException { this.sourceResolver.getResourceId(), indexWriter); } + + public void refilter() { + try { + batchDelegate.refilter(this.getBackingStore(), filterMetadata(getCachedOriginalMetadata())); + } catch (FilterException e) { + logger.error("An error occurred while attempting to filter metadata!", e); + } + } } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlResourceBackedMetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlResourceBackedMetadataResolver.java index 67cde7971..00bf91c3b 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlResourceBackedMetadataResolver.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlResourceBackedMetadataResolver.java @@ -5,7 +5,10 @@ import net.shibboleth.utilities.java.support.xml.ParserPool; import org.apache.lucene.index.IndexWriter; import org.joda.time.DateTime; +import org.opensaml.saml.metadata.resolver.filter.FilterException; import org.opensaml.saml.metadata.resolver.impl.ResourceBackedMetadataResolver; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import javax.annotation.Nullable; import java.io.IOException; @@ -14,9 +17,12 @@ * @author Bill Smith (wsmith@unicon.net) */ public class OpenSamlResourceBackedMetadataResolver extends ResourceBackedMetadataResolver { + private static final Logger logger = LoggerFactory.getLogger(OpenSamlResourceBackedMetadataResolver.class); + private IndexWriter indexWriter; private edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.ResourceBackedMetadataResolver sourceResolver; private OpenSamlMetadataResolverDelegate delegate; + private OpenSamlBatchMetadataResolverDelegate batchDelegate; public OpenSamlResourceBackedMetadataResolver(ParserPool parserPool, IndexWriter indexWriter, @@ -26,6 +32,7 @@ public OpenSamlResourceBackedMetadataResolver(ParserPool parserPool, this.indexWriter = indexWriter; this.sourceResolver = sourceResolver; this.delegate = new OpenSamlMetadataResolverDelegate(); + this.batchDelegate = new OpenSamlBatchMetadataResolverDelegate(); this.setId(sourceResolver.getResourceId()); @@ -48,4 +55,12 @@ protected void initMetadataResolver() throws ComponentInitializationException { this.sourceResolver.getResourceId(), indexWriter); } + + public void refilter() { + try { + batchDelegate.refilter(this.getBackingStore(), filterMetadata(getCachedOriginalMetadata())); + } catch (FilterException e) { + logger.error("An error occurred while attempting to filter metadata!", e); + } + } } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/MetadataResolverConverterServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/MetadataResolverConverterServiceImpl.java index 4ae0d07d2..e1ed54f82 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/MetadataResolverConverterServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/MetadataResolverConverterServiceImpl.java @@ -31,7 +31,6 @@ /** * @author Bill Smith (wsmith@unicon.net) */ -@Service public class MetadataResolverConverterServiceImpl implements MetadataResolverConverterService { @Autowired diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImplTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImplTests.groovy index 381ab3a18..316bfebbd 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImplTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImplTests.groovy @@ -2,15 +2,16 @@ package edu.internet2.tier.shibboleth.admin.ui.service import edu.internet2.tier.shibboleth.admin.ui.configuration.CoreShibUiConfiguration import edu.internet2.tier.shibboleth.admin.ui.configuration.InternationalizationConfiguration +import edu.internet2.tier.shibboleth.admin.ui.configuration.MetadataResolverConverterConfiguration import edu.internet2.tier.shibboleth.admin.ui.configuration.SearchConfiguration import edu.internet2.tier.shibboleth.admin.ui.domain.filters.EntityAttributesFilter import edu.internet2.tier.shibboleth.admin.ui.domain.filters.EntityAttributesFilterTarget import edu.internet2.tier.shibboleth.admin.ui.domain.filters.RequiredValidUntilFilter import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.ClasspathMetadataResource import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.SvnMetadataResource +import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml.OpenSamlChainingMetadataResolver import edu.internet2.tier.shibboleth.admin.ui.opensaml.OpenSamlObjects import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository - import edu.internet2.tier.shibboleth.admin.ui.util.TestObjectGenerator import edu.internet2.tier.shibboleth.admin.util.AttributeUtility import groovy.xml.DOMBuilder @@ -39,10 +40,9 @@ import spock.lang.Specification import static edu.internet2.tier.shibboleth.admin.ui.util.TestHelpers.generatedXmlIsTheSameAsExpectedXml - @SpringBootTest @DataJpaTest -@ContextConfiguration(classes=[CoreShibUiConfiguration, SearchConfiguration, InternationalizationConfiguration]) +@ContextConfiguration(classes=[CoreShibUiConfiguration, MetadataResolverConverterConfiguration, SearchConfiguration, InternationalizationConfiguration]) @EnableJpaRepositories(basePackages = ["edu.internet2.tier.shibboleth.admin.ui"]) @EntityScan("edu.internet2.tier.shibboleth.admin.ui") @DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_CLASS) @@ -65,6 +65,9 @@ class JPAMetadataResolverServiceImplTests extends Specification { @Autowired AttributeUtility attributeUtility + @Autowired + MetadataResolverConverterService mdrConverterService + TestObjectGenerator testObjectGenerator DOMBuilder domBuilder @@ -111,8 +114,13 @@ class JPAMetadataResolverServiceImplTests extends Specification { ''' when: - def mdr = new edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver().with { + def mdr = new edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.ResourceBackedMetadataResolver().with { + it.resourceId = "testme" it.name = "testme" + it.classpathMetadataResource = new ClasspathMetadataResource().with { + it.file = "metadata/aggregate.xml" + it + } it.metadataFilters.add(new EntityAttributesFilter().with { it.entityAttributesFilterTarget = new EntityAttributesFilterTarget().with { it.entityAttributesFilterTargetType = EntityAttributesFilterTarget.EntityAttributesFilterTargetType.ENTITY @@ -125,12 +133,14 @@ class JPAMetadataResolverServiceImplTests extends Specification { return it } metadataResolverRepository.save(mdr) + ((OpenSamlChainingMetadataResolver) metadataResolver).getResolvers().add(mdrConverterService.convertToOpenSamlRepresentation(mdr)) metadataResolverService.reloadFilters("testme") then: assert metadataResolverRepository.findAll().size() > 0 def ed = metadataResolver.resolveSingle(new CriteriaSet(new EntityIdCriterion('http://test.scaldingspoon.org/test1'))) def resultString = openSamlObjects.marshalToXmlString(ed) + println(resultString) def diff = DiffBuilder.compare(Input.fromString(expectedXML)).withTest(Input.fromString(resultString)).ignoreComments().ignoreWhitespace().build() !diff.hasDifferences() } @@ -302,9 +312,10 @@ class JPAMetadataResolverServiceImplTests extends Specification { it } - return new ChainingMetadataResolver().with { + return new OpenSamlChainingMetadataResolver().with { it.id = 'chain' - it.resolvers = [aggregate] + //it.resolvers = [aggregate] +// it.resolvers = [] it.initialize() it } From c37f8906e392792fc4c64a41d97e173787b97aff Mon Sep 17 00:00:00 2001 From: Jj! Date: Tue, 11 Sep 2018 11:08:07 -0500 Subject: [PATCH 05/13] [AS-570] quick fix in initialization --- .../opensaml/OpenSamlResourceBackedMetadataResolver.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlResourceBackedMetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlResourceBackedMetadataResolver.java index 00bf91c3b..fd2e945fc 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlResourceBackedMetadataResolver.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlResourceBackedMetadataResolver.java @@ -6,6 +6,7 @@ import org.apache.lucene.index.IndexWriter; import org.joda.time.DateTime; import org.opensaml.saml.metadata.resolver.filter.FilterException; +import org.opensaml.saml.metadata.resolver.filter.MetadataFilterChain; import org.opensaml.saml.metadata.resolver.impl.ResourceBackedMetadataResolver; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -38,6 +39,9 @@ public OpenSamlResourceBackedMetadataResolver(ParserPool parserPool, OpenSamlMetadataResolverConstructorHelper.updateOpenSamlMetadataResolverFromReloadableMetadataResolverAttributes( this, sourceResolver.getReloadableMetadataResolverAttributes(), parserPool); + + //TODO: check if this is the right thing to do + this.setMetadataFilter(new MetadataFilterChain()); } // TODO: this is still probably not the best way to do this? From 1350872b12f3a677fb5bda9fb8a85cfeb3d21992 Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Tue, 11 Sep 2018 15:13:19 -0700 Subject: [PATCH 06/13] [SHIBUI-570] Removed Batch resolver delegate. Added Refilterable interface. Implemented interface where appropriate. Added initialization for metadataFilterChain. Saving filters is much speedier now for these resolver types. --- ...OpenSamlBatchMetadataResolverDelegate.java | 19 ------------------ ...penSamlFileBackedHTTPMetadataResolver.java | 20 ++++++++----------- .../OpenSamlFilesystemMetadataResolver.java | 13 ++++++++---- ...penSamlResourceBackedMetadataResolver.java | 10 ++++++---- .../resolvers/opensaml/Refilterable.java | 16 +++++++++++++++ 5 files changed, 39 insertions(+), 39 deletions(-) delete mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlBatchMetadataResolverDelegate.java create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/Refilterable.java diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlBatchMetadataResolverDelegate.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlBatchMetadataResolverDelegate.java deleted file mode 100644 index 855de838f..000000000 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlBatchMetadataResolverDelegate.java +++ /dev/null @@ -1,19 +0,0 @@ -package edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml; - -import org.opensaml.core.xml.XMLObject; -import org.opensaml.saml.metadata.resolver.impl.AbstractBatchMetadataResolver; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * @author Bill Smith (wsmith@unicon.net) - */ -public class OpenSamlBatchMetadataResolverDelegate extends AbstractBatchMetadataResolver { - private static final Logger logger = LoggerFactory.getLogger(OpenSamlBatchMetadataResolverDelegate.class); - - //TODO: Not sure this delegate is really buying us anything.. other than to get this one line in to a shared class. - //Maybe we'll do more in here later? - public void refilter(AbstractBatchMetadataResolver.BatchEntityBackingStore backingStore, XMLObject filteredMetadata) { - backingStore.setCachedFilteredMetadata(filteredMetadata); - } -} diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFileBackedHTTPMetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFileBackedHTTPMetadataResolver.java index eac7debe7..d0a9b6ff4 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFileBackedHTTPMetadataResolver.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFileBackedHTTPMetadataResolver.java @@ -8,35 +8,27 @@ import org.apache.http.impl.client.HttpClients; import org.apache.lucene.index.IndexWriter; import org.joda.time.DateTime; -import org.opensaml.core.xml.XMLObject; -import org.opensaml.core.xml.io.UnmarshallingException; import org.opensaml.saml.metadata.resolver.filter.FilterException; +import org.opensaml.saml.metadata.resolver.filter.MetadataFilterChain; import org.opensaml.saml.metadata.resolver.impl.FileBackedHTTPMetadataResolver; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import javax.annotation.Nonnull; import javax.annotation.Nullable; -import java.io.InputStream; -import java.time.Instant; - import static edu.internet2.tier.shibboleth.admin.util.DurationUtility.toMillis; /** * @author Bill Smith (wsmith@unicon.net) */ -public class OpenSamlFileBackedHTTPMetadataResolver extends FileBackedHTTPMetadataResolver { +public class OpenSamlFileBackedHTTPMetadataResolver extends FileBackedHTTPMetadataResolver implements Refilterable { private static final Logger logger = LoggerFactory.getLogger(OpenSamlFileBackedHTTPMetadataResolver.class); - private static final long MILLISECONDS_IN_ONE_SECOND = 1000; - private IndexWriter indexWriter; private FileBackedHttpMetadataResolver sourceResolver; private OpenSamlMetadataResolverDelegate delegate; - private OpenSamlBatchMetadataResolverDelegate batchDelegate; public OpenSamlFileBackedHTTPMetadataResolver(ParserPool parserPool, IndexWriter indexWriter, @@ -45,7 +37,6 @@ public OpenSamlFileBackedHTTPMetadataResolver(ParserPool parserPool, this.indexWriter = indexWriter; this.sourceResolver = sourceResolver; this.delegate = new OpenSamlMetadataResolverDelegate(); - this.batchDelegate = new OpenSamlBatchMetadataResolverDelegate(); this.setId(sourceResolver.getResourceId()); @@ -58,6 +49,8 @@ public OpenSamlFileBackedHTTPMetadataResolver(ParserPool parserPool, this.setBackupFileInitNextRefreshDelay(toMillis(sourceResolver.getBackupFileInitNextRefreshDelay())); this.setInitializeFromBackupFile(sourceResolver.getInitializeFromBackupFile()); + this.setMetadataFilter(new MetadataFilterChain()); + //TODO: Where does this get set in OpenSAML land? // sourceResolver.getMetadataURL(); } @@ -84,9 +77,12 @@ protected void initMetadataResolver() throws ComponentInitializationException { indexWriter); } + /** + * {@inheritDoc} + */ public void refilter() { try { - batchDelegate.refilter(this.getBackingStore(), filterMetadata(getCachedOriginalMetadata())); + this.getBackingStore().setCachedFilteredMetadata(filterMetadata(getCachedOriginalMetadata())); } catch (FilterException e) { logger.error("An error occurred while attempting to filter metadata!", e); } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFilesystemMetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFilesystemMetadataResolver.java index e20fe632b..b4fb6d578 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFilesystemMetadataResolver.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFilesystemMetadataResolver.java @@ -6,6 +6,7 @@ import org.apache.lucene.index.IndexWriter; import org.joda.time.DateTime; import org.opensaml.saml.metadata.resolver.filter.FilterException; +import org.opensaml.saml.metadata.resolver.filter.MetadataFilterChain; import org.opensaml.saml.metadata.resolver.impl.FilesystemMetadataResolver; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -16,13 +17,13 @@ /** * @author Bill Smith (wsmith@unicon.net) */ -public class OpenSamlFilesystemMetadataResolver extends FilesystemMetadataResolver { +public class OpenSamlFilesystemMetadataResolver extends FilesystemMetadataResolver implements Refilterable { + private static final Logger logger = LoggerFactory.getLogger(OpenSamlFilesystemMetadataResolver.class); private IndexWriter indexWriter; private edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.FilesystemMetadataResolver sourceResolver; private OpenSamlMetadataResolverDelegate delegate; - private OpenSamlBatchMetadataResolverDelegate batchDelegate; public OpenSamlFilesystemMetadataResolver(ParserPool parserPool, IndexWriter indexWriter, @@ -32,12 +33,13 @@ public OpenSamlFilesystemMetadataResolver(ParserPool parserPool, this.indexWriter = indexWriter; this.sourceResolver = sourceResolver; this.delegate = new OpenSamlMetadataResolverDelegate(); - this.batchDelegate = new OpenSamlBatchMetadataResolverDelegate(); this.setId(sourceResolver.getResourceId()); OpenSamlMetadataResolverConstructorHelper.updateOpenSamlMetadataResolverFromReloadableMetadataResolverAttributes( this, sourceResolver.getReloadableMetadataResolverAttributes(), parserPool); + + this.setMetadataFilter(new MetadataFilterChain()); } // TODO: this is still probably not the best way to do this? @@ -56,9 +58,12 @@ protected void initMetadataResolver() throws ComponentInitializationException { indexWriter); } + /** + * {@inheritDoc} + */ public void refilter() { try { - batchDelegate.refilter(this.getBackingStore(), filterMetadata(getCachedOriginalMetadata())); + this.getBackingStore().setCachedFilteredMetadata(filterMetadata(getCachedOriginalMetadata())); } catch (FilterException e) { logger.error("An error occurred while attempting to filter metadata!", e); } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlResourceBackedMetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlResourceBackedMetadataResolver.java index fd2e945fc..0a4b2a7f2 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlResourceBackedMetadataResolver.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlResourceBackedMetadataResolver.java @@ -17,13 +17,13 @@ /** * @author Bill Smith (wsmith@unicon.net) */ -public class OpenSamlResourceBackedMetadataResolver extends ResourceBackedMetadataResolver { +public class OpenSamlResourceBackedMetadataResolver extends ResourceBackedMetadataResolver implements Refilterable { + private static final Logger logger = LoggerFactory.getLogger(OpenSamlResourceBackedMetadataResolver.class); private IndexWriter indexWriter; private edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.ResourceBackedMetadataResolver sourceResolver; private OpenSamlMetadataResolverDelegate delegate; - private OpenSamlBatchMetadataResolverDelegate batchDelegate; public OpenSamlResourceBackedMetadataResolver(ParserPool parserPool, IndexWriter indexWriter, @@ -33,7 +33,6 @@ public OpenSamlResourceBackedMetadataResolver(ParserPool parserPool, this.indexWriter = indexWriter; this.sourceResolver = sourceResolver; this.delegate = new OpenSamlMetadataResolverDelegate(); - this.batchDelegate = new OpenSamlBatchMetadataResolverDelegate(); this.setId(sourceResolver.getResourceId()); @@ -60,9 +59,12 @@ protected void initMetadataResolver() throws ComponentInitializationException { indexWriter); } + /** + * {@inheritDoc} + */ public void refilter() { try { - batchDelegate.refilter(this.getBackingStore(), filterMetadata(getCachedOriginalMetadata())); + this.getBackingStore().setCachedFilteredMetadata(filterMetadata(getCachedOriginalMetadata())); } catch (FilterException e) { logger.error("An error occurred while attempting to filter metadata!", e); } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/Refilterable.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/Refilterable.java new file mode 100644 index 000000000..339a3b8e3 --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/Refilterable.java @@ -0,0 +1,16 @@ +package edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml; + +/** + * Indicates that the resolver implementing this interface is a resolver that allows for its metadata to be + * filtered multiple times. + * + * @author Bill Smith (wsmith@unicon.net) + */ +public interface Refilterable { + + /** + * Reapply this resolver's filters to its cached, unfiltered metadata, and set the result back to its cached, + * filtered metadata. + */ + void refilter(); +} \ No newline at end of file From 670fb5ca9103494ea14775ce17fd964df21720d5 Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Tue, 11 Sep 2018 15:24:20 -0700 Subject: [PATCH 07/13] [SHIBUI-570] Removed duplicate method. --- .../OpenSamlChainingMetadataResolver.java | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlChainingMetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlChainingMetadataResolver.java index 0cc2102c1..3213736a6 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlChainingMetadataResolver.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlChainingMetadataResolver.java @@ -90,24 +90,4 @@ public void refresh() throws ResolverException { } } } - - @Override - @Nonnull public Iterable resolve(@Nullable final CriteriaSet criteria) throws ResolverException { - ComponentSupport.ifNotInitializedThrowUninitializedComponentException(this); - - for (final MetadataResolver resolver : mutableResolvers) { - try { - final Iterable descriptors = resolver.resolve(criteria); - if (descriptors != null && descriptors.iterator().hasNext()) { - return descriptors; - } - } catch (final ResolverException e) { - log.warn("Error retrieving metadata from resolver of type {}, proceeding to next resolver", - resolver.getClass().getName(), e); - continue; - } - } - - return Collections.emptyList(); - } } From 640423aad9c2ad6e56e796bf43af9e74c29132a5 Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Wed, 12 Sep 2018 09:16:49 -0700 Subject: [PATCH 08/13] [SHIBUI-570] Dropped min refresh down to 30S. --- .../tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy index 7510f9929..7c3eef273 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy @@ -460,7 +460,7 @@ class TestObjectGenerator { it.metadataURL = 'https://idp.unicon.net/idp/shibboleth' it.reloadableMetadataResolverAttributes = new ReloadableMetadataResolverAttributes().with { - it.minRefreshDelay = 'PT5M' + it.minRefreshDelay = 'PT30S' it.maxRefreshDelay = 'PT1H' it.refreshDelayFactor = 0.75 it From 0721845e81fe808af2b82bb3a607a3b57ecac085 Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Wed, 12 Sep 2018 09:38:20 -0700 Subject: [PATCH 09/13] [SHIBUI-570] Removed specifying delay, opting for defaults instead. --- .../tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy | 3 --- backend/src/test/resources/conf/278.2.xml | 5 +---- backend/src/test/resources/conf/278.xml | 5 +---- backend/src/test/resources/conf/532.xml | 5 +---- 4 files changed, 3 insertions(+), 15 deletions(-) diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy index 7c3eef273..6a7d0e1cf 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy @@ -460,9 +460,6 @@ class TestObjectGenerator { it.metadataURL = 'https://idp.unicon.net/idp/shibboleth' it.reloadableMetadataResolverAttributes = new ReloadableMetadataResolverAttributes().with { - it.minRefreshDelay = 'PT30S' - it.maxRefreshDelay = 'PT1H' - it.refreshDelayFactor = 0.75 it } it diff --git a/backend/src/test/resources/conf/278.2.xml b/backend/src/test/resources/conf/278.2.xml index 2b71b5ed1..5a34f756b 100644 --- a/backend/src/test/resources/conf/278.2.xml +++ b/backend/src/test/resources/conf/278.2.xml @@ -39,10 +39,7 @@ + metadataURL="https://idp.unicon.net/idp/shibboleth"> diff --git a/backend/src/test/resources/conf/278.xml b/backend/src/test/resources/conf/278.xml index b244bf6b6..a337f72d7 100644 --- a/backend/src/test/resources/conf/278.xml +++ b/backend/src/test/resources/conf/278.xml @@ -32,10 +32,7 @@ + metadataURL="https://idp.unicon.net/idp/shibboleth"> diff --git a/backend/src/test/resources/conf/532.xml b/backend/src/test/resources/conf/532.xml index bf4ce340b..85fead908 100644 --- a/backend/src/test/resources/conf/532.xml +++ b/backend/src/test/resources/conf/532.xml @@ -8,8 +8,5 @@ + metadataURL="https://idp.unicon.net/idp/shibboleth" /> From 8ac52b8e9c04c52fba1d2c6c467bc4dbdd4056c9 Mon Sep 17 00:00:00 2001 From: Jj! Date: Wed, 12 Sep 2018 11:48:37 -0500 Subject: [PATCH 10/13] [AS-570] WIP --- .../admin/ui/service/JPAMetadataResolverServiceImpl.groovy | 2 +- .../admin/ui/controller/MetadataFiltersController.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy b/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy index df37471c2..186dd0116 100644 --- a/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy +++ b/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy @@ -56,7 +56,7 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService { void reloadFilters(String metadataResolverName) { ChainingMetadataResolver chainingMetadataResolver = (ChainingMetadataResolver) metadataResolver MetadataResolver targetMetadataResolver = chainingMetadataResolver.getResolvers().find { it.id == metadataResolverName } - edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver jpaMetadataResolver = metadataResolverRepository.findByName(metadataResolverName) + edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver jpaMetadataResolver = metadataResolverRepository.findByResourceId(metadataResolverName) if (targetMetadataResolver && targetMetadataResolver.getMetadataFilter() instanceof MetadataFilterChain) { MetadataFilterChain metadataFilterChain = (MetadataFilterChain) targetMetadataResolver.getMetadataFilter() diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java index ce50be62f..431632023 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java @@ -88,7 +88,7 @@ public ResponseEntity create(@PathVariable String metadataResolverId, @Reques MetadataResolver persistedMr = repository.save(metadataResolver); // we reload the filters here after save - metadataResolverService.reloadFilters(persistedMr.getName()); + metadataResolverService.reloadFilters(persistedMr.getResourceId()); MetadataFilter persistedFilter = newlyPersistedFilter(persistedMr.getMetadataFilters().stream(), createdFilter.getResourceId()); @@ -130,7 +130,7 @@ public ResponseEntity update(@PathVariable String metadataResolverId, MetadataFilter persistedFilter = filterRepository.save(filterTobeUpdated); // TODO: this is wrong - metadataResolverService.reloadFilters(metadataResolver.getName()); + metadataResolverService.reloadFilters(metadataResolver.getResourceId()); return ResponseEntity.ok().body(persistedFilter); } From 3e8d197ded3bf62af793bb3dff99d8cfbed557eb Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Wed, 12 Sep 2018 11:36:54 -0700 Subject: [PATCH 11/13] [SHIBUI-570] Replaced references to metadataResolverName with ResourceId. Replaced ChainingMetadataResolver with OpenSaml. Replaced instanceof with smaller one based on Refilterable. Moved updateChainingMetadataResolver to Util class. Updated unit tests that create EntityAttributesFilters to create Targets with valid values for regex and condition script. Unit tests WIP. --- .../JPAMetadataResolverServiceImpl.groovy | 31 +++++++------------ .../MetadataResolversController.java | 18 ++++------- .../OpenSamlChainingMetadataResolverUtil.java | 20 ++++++++++++ ...taFiltersControllerIntegrationTests.groovy | 18 ++++++++--- .../admin/ui/util/TestObjectGenerator.groovy | 19 ++++++++++-- 5 files changed, 68 insertions(+), 38 deletions(-) create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/util/OpenSamlChainingMetadataResolverUtil.java diff --git a/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy b/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy index 186dd0116..ec201d00b 100644 --- a/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy +++ b/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy @@ -11,9 +11,8 @@ import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.FileBackedHttpMet import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.FilesystemMetadataResolver import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.LocalDynamicMetadataResolver import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.ResourceBackedMetadataResolver -import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml.OpenSamlFileBackedHTTPMetadataResolver -import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml.OpenSamlFilesystemMetadataResolver -import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml.OpenSamlResourceBackedMetadataResolver +import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml.OpenSamlChainingMetadataResolver +import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml.Refilterable import edu.internet2.tier.shibboleth.admin.ui.opensaml.OpenSamlObjects import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository import groovy.util.logging.Slf4j @@ -21,11 +20,9 @@ import groovy.xml.DOMBuilder import groovy.xml.MarkupBuilder import net.shibboleth.utilities.java.support.scripting.EvaluableScript import org.opensaml.saml.common.profile.logic.EntityIdPredicate -import org.opensaml.saml.metadata.resolver.ChainingMetadataResolver import org.opensaml.saml.metadata.resolver.MetadataResolver import org.opensaml.saml.metadata.resolver.filter.MetadataFilter import org.opensaml.saml.metadata.resolver.filter.MetadataFilterChain -import org.opensaml.saml.metadata.resolver.impl.AbstractBatchMetadataResolver import org.opensaml.saml.saml2.core.Attribute import org.opensaml.saml.saml2.metadata.EntityDescriptor import org.springframework.beans.factory.annotation.Autowired @@ -53,10 +50,10 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService { // TODO: enhance @Override - void reloadFilters(String metadataResolverName) { - ChainingMetadataResolver chainingMetadataResolver = (ChainingMetadataResolver) metadataResolver - MetadataResolver targetMetadataResolver = chainingMetadataResolver.getResolvers().find { it.id == metadataResolverName } - edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver jpaMetadataResolver = metadataResolverRepository.findByResourceId(metadataResolverName) + void reloadFilters(String metadataResolverResourceId) { + OpenSamlChainingMetadataResolver chainingMetadataResolver = (OpenSamlChainingMetadataResolver) metadataResolver + MetadataResolver targetMetadataResolver = chainingMetadataResolver.getResolvers().find { it.id == metadataResolverResourceId } + edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver jpaMetadataResolver = metadataResolverRepository.findByResourceId(metadataResolverResourceId) if (targetMetadataResolver && targetMetadataResolver.getMetadataFilter() instanceof MetadataFilterChain) { MetadataFilterChain metadataFilterChain = (MetadataFilterChain) targetMetadataResolver.getMetadataFilter() @@ -95,17 +92,11 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService { metadataFilterChain.setFilters(metadataFilters) } - if (targetMetadataResolver != null && targetMetadataResolver instanceof AbstractBatchMetadataResolver) { - if (targetMetadataResolver instanceof OpenSamlFileBackedHTTPMetadataResolver) { - (OpenSamlFileBackedHTTPMetadataResolver) targetMetadataResolver.refilter() - } else if (targetMetadataResolver instanceof OpenSamlFilesystemMetadataResolver) { - (OpenSamlFilesystemMetadataResolver) targetMetadataResolver.refilter() - } else if (targetMetadataResolver instanceof OpenSamlResourceBackedMetadataResolver) { - (OpenSamlResourceBackedMetadataResolver) targetMetadataResolver.refilter() - } else { - //TODO: Do something here if we need to refilter other non-Batch resolvers - println("We shouldn't be here. But we are. Why?") - } + if (targetMetadataResolver != null && targetMetadataResolver instanceof Refilterable) { + (Refilterable) targetMetadataResolver.refilter() + } else { + //TODO: Do something here if we need to refilter other non-Batch resolvers + println("We shouldn't be here. But we are. Why?") } } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java index 7388679da..47458273d 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java @@ -3,15 +3,16 @@ import com.fasterxml.jackson.databind.exc.InvalidTypeIdException; import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver; import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolverValidationService; +import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml.OpenSamlChainingMetadataResolver; import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository; import edu.internet2.tier.shibboleth.admin.ui.service.IndexWriterService; import edu.internet2.tier.shibboleth.admin.ui.service.MetadataResolverConverterService; import edu.internet2.tier.shibboleth.admin.ui.service.MetadataResolverService; import edu.internet2.tier.shibboleth.admin.ui.service.MetadataResolversPositionOrderContainerService; +import edu.internet2.tier.shibboleth.admin.util.OpenSamlChainingMetadataResolverUtil; import lombok.extern.slf4j.Slf4j; import net.shibboleth.utilities.java.support.component.ComponentInitializationException; import net.shibboleth.utilities.java.support.resolver.ResolverException; -import org.opensaml.saml.metadata.resolver.ChainingMetadataResolver; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; @@ -36,7 +37,6 @@ import java.io.IOException; import java.io.StringWriter; import java.net.URI; -import java.util.ArrayList; import java.util.List; import static edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolverValidator.ValidationResult; @@ -120,19 +120,12 @@ public ResponseEntity create(@RequestBody MetadataResolver newResolver) throw //TODO: currently, the update call might explode, but the save works.. in which case, the UI never gets // n valid response. This operation is not atomic. Should we return an error here? - updateChainingMetadataResolver(persistedResolver); + org.opensaml.saml.metadata.resolver.MetadataResolver openSamlRepresentation = metadataResolverConverterService.convertToOpenSamlRepresentation(persistedResolver); + OpenSamlChainingMetadataResolverUtil.updateChainingMetadataResolver((OpenSamlChainingMetadataResolver) chainingMetadataResolver, openSamlRepresentation); return ResponseEntity.created(getResourceUriFor(persistedResolver)).body(persistedResolver); } - private void updateChainingMetadataResolver(MetadataResolver persistedResolver) throws IOException, ResolverException, ComponentInitializationException { - org.opensaml.saml.metadata.resolver.MetadataResolver openSamlResolver = metadataResolverConverterService.convertToOpenSamlRepresentation(persistedResolver); - List resolverList = new ArrayList<>(((ChainingMetadataResolver) chainingMetadataResolver).getResolvers()); - resolverList.removeIf(resolver -> resolver.getId().equals(persistedResolver.getResourceId())); - resolverList.add(openSamlResolver); - ((ChainingMetadataResolver) chainingMetadataResolver).setResolvers(resolverList); - } - @PutMapping("/MetadataResolvers/{resourceId}") @Transactional public ResponseEntity update(@PathVariable String resourceId, @RequestBody MetadataResolver updatedResolver) throws IOException, ResolverException, ComponentInitializationException { @@ -155,7 +148,8 @@ public ResponseEntity update(@PathVariable String resourceId, @RequestBody Me MetadataResolver persistedResolver = resolverRepository.save(updatedResolver); - updateChainingMetadataResolver(persistedResolver); + org.opensaml.saml.metadata.resolver.MetadataResolver openSamlRepresentation = metadataResolverConverterService.convertToOpenSamlRepresentation(persistedResolver); + OpenSamlChainingMetadataResolverUtil.updateChainingMetadataResolver((OpenSamlChainingMetadataResolver) chainingMetadataResolver, openSamlRepresentation); return ResponseEntity.ok(persistedResolver); } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/util/OpenSamlChainingMetadataResolverUtil.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/util/OpenSamlChainingMetadataResolverUtil.java new file mode 100644 index 000000000..81b8f3675 --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/util/OpenSamlChainingMetadataResolverUtil.java @@ -0,0 +1,20 @@ +package edu.internet2.tier.shibboleth.admin.util; + +import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml.OpenSamlChainingMetadataResolver; +import org.opensaml.saml.metadata.resolver.MetadataResolver; + +import java.util.ArrayList; +import java.util.List; + +/** + * @author Bill Smith (wsmith@unicon.net) + */ +public class OpenSamlChainingMetadataResolverUtil { + + public static void updateChainingMetadataResolver(OpenSamlChainingMetadataResolver chainingMetadataResolver, MetadataResolver openSamlResolver) { + List resolverList = new ArrayList<>(chainingMetadataResolver.getResolvers()); + resolverList.removeIf(resolver -> resolver.getId().equals(openSamlResolver.getId())); + resolverList.add(openSamlResolver); + chainingMetadataResolver.setResolvers(resolverList); + } +} diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy index 06f9d0ca0..7f7beefe7 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy @@ -3,12 +3,14 @@ package edu.internet2.tier.shibboleth.admin.ui.controller import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.databind.SerializationFeature import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule +import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml.OpenSamlChainingMetadataResolver import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository +import edu.internet2.tier.shibboleth.admin.ui.service.MetadataResolverConverterService import edu.internet2.tier.shibboleth.admin.ui.util.TestObjectGenerator import edu.internet2.tier.shibboleth.admin.util.AttributeUtility +import edu.internet2.tier.shibboleth.admin.util.OpenSamlChainingMetadataResolverUtil import groovy.json.JsonOutput import groovy.json.JsonSlurper -import org.opensaml.saml.metadata.resolver.ChainingMetadataResolver import org.opensaml.saml.metadata.resolver.MetadataResolver import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.test.context.SpringBootTest @@ -38,6 +40,12 @@ class MetadataFiltersControllerIntegrationTests extends Specification { @Autowired AttributeUtility attributeUtility + @Autowired + MetadataResolverConverterService metadataResolverConverterService + + @Autowired + MetadataResolver chainingMetadataResolver + ObjectMapper mapper TestObjectGenerator generator @@ -63,7 +71,8 @@ class MetadataFiltersControllerIntegrationTests extends Specification { def filterResourceId = resolver.metadataFilters[0].resourceId def resolverResourceId = resolver.resourceId metadataResolverRepository.save(resolver) - + MetadataResolver openSamlRepresentation = metadataResolverConverterService.convertToOpenSamlRepresentation(resolver) + OpenSamlChainingMetadataResolverUtil.updateChainingMetadataResolver((OpenSamlChainingMetadataResolver) chainingMetadataResolver, openSamlRepresentation) when: 'GET request is made with resource Id matching the existing filter' def result = this.restTemplate.getForEntity("$BASE_URI/$resolverResourceId/Filters/$filterResourceId", String) @@ -86,7 +95,8 @@ class MetadataFiltersControllerIntegrationTests extends Specification { def filterResourceId = resolver.metadataFilters[0].resourceId def resolverResourceId = resolver.resourceId metadataResolverRepository.save(resolver) - + MetadataResolver openSamlRepresentation = metadataResolverConverterService.convertToOpenSamlRepresentation(resolver) + OpenSamlChainingMetadataResolverUtil.updateChainingMetadataResolver((OpenSamlChainingMetadataResolver) chainingMetadataResolver, openSamlRepresentation) when: 'GET request is made with resource Id matching the existing filter' def result = this.restTemplate.getForEntity("$BASE_URI/$resolverResourceId/Filters/$filterResourceId", String) @@ -182,7 +192,7 @@ class MetadataFiltersControllerIntegrationTests extends Specification { static class Config { @Bean MetadataResolver metadataResolver() { - new ChainingMetadataResolver().with { + new OpenSamlChainingMetadataResolver().with { it.id = 'tester' it.initialize() return it diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy index 6a7d0e1cf..64bb8a407 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy @@ -2,6 +2,7 @@ package edu.internet2.tier.shibboleth.admin.ui.util import edu.internet2.tier.shibboleth.admin.ui.domain.* import edu.internet2.tier.shibboleth.admin.ui.domain.filters.* +import edu.internet2.tier.shibboleth.admin.ui.domain.filters.EntityAttributesFilterTarget.EntityAttributesFilterTargetType import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.FilterRepresentation import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.FilterTargetRepresentation import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.RelyingPartyOverridesRepresentation @@ -321,11 +322,23 @@ class TestObjectGenerator { return representation } + String buildEntityAttributesFilterTargetValueByType(EntityAttributesFilterTargetType type) { + switch (type) { + case EntityAttributesFilterTargetType.ENTITY: + return generator.randomString() + case EntityAttributesFilterTargetType.CONDITION_SCRIPT: + return "eval(true);" + case EntityAttributesFilterTargetType.REGEX: + return "/foo.*/" + } + } + EntityAttributesFilterTarget buildEntityAttributesFilterTarget() { EntityAttributesFilterTarget entityAttributesFilterTarget = new EntityAttributesFilterTarget() - entityAttributesFilterTarget.setSingleValue(generator.randomString()) entityAttributesFilterTarget.setEntityAttributesFilterTargetType(randomFilterTargetType()) + entityAttributesFilterTarget.setSingleValue( + buildEntityAttributesFilterTargetValueByType(entityAttributesFilterTarget.getEntityAttributesFilterTargetType())) return entityAttributesFilterTarget } @@ -353,8 +366,10 @@ class TestObjectGenerator { FilterTargetRepresentation buildFilterTargetRepresentation() { FilterTargetRepresentation representation = new FilterTargetRepresentation() - representation.setValue(generator.randomStringList()) representation.setType(randomFilterTargetType().toString()) + representation.setValue([ + buildEntityAttributesFilterTargetValueByType(representation.getType()) + ]) return representation } From dc3a20507e0574ab593753ea8a1f664ef185a38e Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Wed, 12 Sep 2018 12:54:39 -0700 Subject: [PATCH 12/13] [SHIBUI-570] Various test fixes. --- .../MetadataResolversControllerIntegrationTests.groovy | 4 ++-- .../tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy index 221c5010e..69efa8133 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy @@ -7,12 +7,12 @@ import edu.internet2.tier.shibboleth.admin.ui.domain.filters.EntityAttributesFil import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.DynamicHttpMetadataResolver import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.FileBackedHttpMetadataResolver import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.LocalDynamicMetadataResolver +import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.opensaml.OpenSamlChainingMetadataResolver import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository import edu.internet2.tier.shibboleth.admin.ui.util.TestObjectGenerator import edu.internet2.tier.shibboleth.admin.util.AttributeUtility import groovy.json.JsonOutput import groovy.json.JsonSlurper -import org.opensaml.saml.metadata.resolver.ChainingMetadataResolver import org.opensaml.saml.metadata.resolver.MetadataResolver import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.test.context.SpringBootTest @@ -315,7 +315,7 @@ class MetadataResolversControllerIntegrationTests extends Specification { static class Config { @Bean MetadataResolver metadataResolver() { - new ChainingMetadataResolver() + new OpenSamlChainingMetadataResolver() } } } diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy index 64bb8a407..13dc8a554 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy @@ -368,7 +368,7 @@ class TestObjectGenerator { representation.setType(randomFilterTargetType().toString()) representation.setValue([ - buildEntityAttributesFilterTargetValueByType(representation.getType()) + buildEntityAttributesFilterTargetValueByType(EntityAttributesFilterTargetType.valueOf(representation.getType())) ]) return representation From 6d84f57778f732461c44f737854148905e5239b3 Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Wed, 12 Sep 2018 13:14:03 -0700 Subject: [PATCH 13/13] [SHIBUI-570] Replaced a println. --- .../admin/ui/service/JPAMetadataResolverServiceImpl.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy b/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy index ec201d00b..f8eb32a38 100644 --- a/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy +++ b/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy @@ -96,7 +96,7 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService { (Refilterable) targetMetadataResolver.refilter() } else { //TODO: Do something here if we need to refilter other non-Batch resolvers - println("We shouldn't be here. But we are. Why?") + log.warn("Target resolver is not a Refilterable resolver. Skipping refilter()") } }