From 3e8d197ded3bf62af793bb3dff99d8cfbed557eb Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Wed, 12 Sep 2018 11:36:54 -0700 Subject: [PATCH] [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 }