Skip to content

Commit

Permalink
[SHIBUI-570]
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
Bill Smith committed Sep 11, 2018
1 parent 75013d0 commit fdbe478
Show file tree
Hide file tree
Showing 9 changed files with 117 additions and 76 deletions.
22 changes: 14 additions & 8 deletions ...oovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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?")
}
}
}
Expand Down
17 changes: 17 additions & 0 deletions ...ernet2/tier/shibboleth/admin/ui/configuration/MetadataResolverConverterConfiguration.java
View file
Original file line number Diff line number Diff line change
@@ -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();
}
}
29 changes: 0 additions & 29 deletions ...ain/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand All @@ -98,33 +97,6 @@ public ResponseEntity<?> create(@PathVariable String metadataResolverId, @Reques
.body(persistedFilter);
}

private void refreshOrInitResolver(MetadataResolver resolver) {
List<org.opensaml.saml.metadata.resolver.MetadataResolver> 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,
Expand Down Expand Up @@ -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);
}
Expand Down
30 changes: 27 additions & 3 deletions .../tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlChainingMetadataResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -25,7 +29,7 @@ public class OpenSamlChainingMetadataResolver extends ChainingMetadataResolver {
@Nonnull @NonnullElements private List<MetadataResolver> mutableResolvers;

public OpenSamlChainingMetadataResolver() {
this.mutableResolvers = Collections.emptyList();
this.mutableResolvers = new ArrayList<>();
}

public OpenSamlChainingMetadataResolver(@Nonnull List<MetadataResolver> mutableResolvers) {
Expand All @@ -35,7 +39,7 @@ public OpenSamlChainingMetadataResolver(@Nonnull List<MetadataResolver> mutableR
@Override
public void setResolvers(@Nonnull @NonnullElements final List<? extends MetadataResolver> newResolvers) {
if (newResolvers == null || newResolvers.isEmpty()) {
mutableResolvers = Collections.emptyList();
mutableResolvers = new ArrayList<>();
return;
}

Expand All @@ -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<>();
}
}

Expand All @@ -66,4 +70,24 @@ public void refresh() throws ResolverException {
}
}
}

@Override
@Nonnull public Iterable<EntityDescriptor> resolve(@Nullable final CriteriaSet criteria) throws ResolverException {
ComponentSupport.ifNotInitializedThrowUninitializedComponentException(this);

for (final MetadataResolver resolver : mutableResolvers) {
try {
final Iterable<EntityDescriptor> 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();
}
}
41 changes: 12 additions & 29 deletions ...shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFileBackedHTTPMetadataResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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());

Expand Down Expand Up @@ -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;
}
}
15 changes: 15 additions & 0 deletions ...ier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFilesystemMetadataResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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());

Expand All @@ -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);
}
}
}
15 changes: 15 additions & 0 deletions ...shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlResourceBackedMetadataResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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());

Expand All @@ -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);
}
}
}
1 change: 0 additions & 1 deletion .../edu/internet2/tier/shibboleth/admin/ui/service/MetadataResolverConverterServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
/**
* @author Bill Smith (wsmith@unicon.net)
*/
@Service
public class MetadataResolverConverterServiceImpl implements MetadataResolverConverterService {

@Autowired
Expand Down
Loading

0 comments on commit fdbe478

Please sign in to comment.