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 7d873afa6..a6491fa86 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 @@ -6,10 +6,10 @@ import edu.internet2.tier.shibboleth.admin.ui.domain.filters.RequiredValidUntilFilter; import edu.internet2.tier.shibboleth.admin.ui.domain.filters.SignatureValidationFilter; import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver; +import edu.internet2.tier.shibboleth.admin.ui.repository.FilterRepository; import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository; import edu.internet2.tier.shibboleth.admin.ui.service.MetadataResolverService; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; + import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; @@ -24,7 +24,6 @@ import org.springframework.web.servlet.support.ServletUriComponentsBuilder; import java.net.URI; -import java.util.List; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -32,14 +31,15 @@ @RequestMapping("/api/MetadataResolvers/{metadataResolverId}") public class MetadataFiltersController { - private static Logger LOGGER = LoggerFactory.getLogger(MetadataFiltersController.class); - @Autowired private MetadataResolverRepository repository; @Autowired private MetadataResolverService metadataResolverService; + @Autowired + private FilterRepository filterRepository; + @GetMapping("/Filters") @Transactional(readOnly = true) public ResponseEntity getAll(@PathVariable String metadataResolverId) { @@ -78,8 +78,7 @@ public ResponseEntity create(@PathVariable String metadataResolverId, @Reques // we reload the filters here after save metadataResolverService.reloadFilters(persistedMr.getName()); - MetadataFilter persistedFilter = - convertIntoTransientRepresentationIfNecessary(persistedMr.getMetadataFilters().stream(), createdFilter.getResourceId()); + MetadataFilter persistedFilter = newlyPersistedFilter(persistedMr.getMetadataFilters().stream(), createdFilter.getResourceId()); return ResponseEntity .created(getResourceUriFor(persistedMr, createdFilter.getResourceId())) @@ -91,30 +90,28 @@ public ResponseEntity create(@PathVariable String metadataResolverId, @Reques public ResponseEntity update(@PathVariable String metadataResolverId, @PathVariable String resourceId, @RequestBody MetadataFilter updatedFilter) { + MetadataFilter filterTobeUpdated = filterRepository.findByResourceId(resourceId); + if (filterTobeUpdated == null) { + return ResponseEntity.notFound().build(); + } MetadataResolver metadataResolver = repository.findByResourceId(metadataResolverId); if(metadataResolver == null) { return ResponseEntity.notFound().build(); } - if (!resourceId.equals(updatedFilter.getResourceId())) { - return new ResponseEntity(HttpStatus.CONFLICT); + // check to make sure that the relationship exists + if (!metadataResolver.getMetadataFilters().contains(filterTobeUpdated)) { + // TODO: find a better response + return new ResponseEntity<>(HttpStatus.BAD_REQUEST); } - List filters = - metadataResolver.getMetadataFilters().stream() - .filter(f -> f.getResourceId().equals(updatedFilter.getResourceId())) - .collect(Collectors.toList()); - if (filters.size() > 1) { - // TODO: I don't think this should ever happen, but... if it does... - // do something? throw exception, return error? - LOGGER.warn("More than one filter was found for id {}! This is probably a bad thing.\n" + - "We're going to go ahead and use the first one, but .. look in to this!", updatedFilter.getResourceId()); + if (!resourceId.equals(updatedFilter.getResourceId())) { + return new ResponseEntity(HttpStatus.CONFLICT); } - MetadataFilter filterTobeUpdated = filters.get(0); // Verify we're the only one attempting to update the filter - if (updatedFilter.getVersion() != filterTobeUpdated.hashCode()) { + if (updatedFilter.getVersion() != filterTobeUpdated.getVersion()) { return new ResponseEntity(HttpStatus.CONFLICT); } @@ -122,32 +119,19 @@ public ResponseEntity update(@PathVariable String metadataResolverId, filterTobeUpdated.setFilterEnabled(updatedFilter.isFilterEnabled()); updateConcreteFilterTypeData(filterTobeUpdated, updatedFilter); - //convert before saving into database - if(filterTobeUpdated instanceof EntityAttributesFilter) { - EntityAttributesFilter.class.cast(filterTobeUpdated).fromTransientRepresentation(); - } + MetadataFilter persistedFilter = filterRepository.save(filterTobeUpdated); - MetadataResolver persistedMr = repository.save(metadataResolver); - - metadataResolverService.reloadFilters(persistedMr.getName()); - - MetadataFilter persistedFilter = - convertIntoTransientRepresentationIfNecessary(persistedMr.getMetadataFilters().stream(), updatedFilter.getResourceId()); - - persistedFilter.setVersion(persistedFilter.hashCode()); + // TODO: this is wrong + metadataResolverService.reloadFilters(metadataResolver.getName()); return ResponseEntity.ok().body(persistedFilter); } - private MetadataFilter convertIntoTransientRepresentationIfNecessary(Stream filters, final String filterResourceId) { + private MetadataFilter newlyPersistedFilter(Stream filters, final String filterResourceId) { MetadataFilter persistedFilter = filters .filter(f -> f.getResourceId().equals(filterResourceId)) .collect(Collectors.toList()).get(0); - //convert before saving into database - if(persistedFilter instanceof EntityAttributesFilter) { - EntityAttributesFilter.class.cast(persistedFilter).intoTransientRepresentation(); - } return persistedFilter; } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java index a62ed9a24..e5e923918 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java @@ -51,19 +51,22 @@ public EntityAttributesFilter() { @Transient private RelyingPartyOverridesRepresentation relyingPartyOverrides; + @PostLoad public void intoTransientRepresentation() { this.attributeRelease = getAttributeReleaseListFromAttributeList(this.attributes); this.relyingPartyOverrides = getRelyingPartyOverridesRepresentationFromAttributeList(attributes); - updateVersion(); } + @PrePersist + @PreUpdate public void fromTransientRepresentation() { List attributeList = new ArrayList<>(); attributeList.addAll(getAttributeListFromAttributeReleaseList(this.attributeRelease)); attributeList.addAll(getAttributeListFromRelyingPartyOverridesRepresentation(this.relyingPartyOverrides)); if(!attributeList.isEmpty()) { - this.attributes = (List) (List) attributeList; + this.attributes.clear(); + this.attributes.addAll((List) (List) attributeList); } } -} +} \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java index 0347de1f7..a80d538c9 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java @@ -1,5 +1,6 @@ package edu.internet2.tier.shibboleth.admin.ui.domain.filters; +import com.fasterxml.jackson.annotation.JsonGetter; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; @@ -46,9 +47,13 @@ public class MetadataFilter extends AbstractAuditable { private boolean filterEnabled; @Transient - private int version; - - public void updateVersion() { - this.version = hashCode(); + private transient Integer version; + + @JsonGetter("version") + public int getVersion() { + if (version != null && version != 0) { + return this.version; + } + return this.hashCode(); } -} +} \ No newline at end of file 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 new file mode 100644 index 000000000..1804ff6f1 --- /dev/null +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy @@ -0,0 +1,97 @@ +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.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 +import org.springframework.boot.test.context.TestConfiguration +import org.springframework.boot.test.web.client.TestRestTemplate +import org.springframework.context.annotation.Bean +import org.springframework.http.HttpEntity +import org.springframework.http.HttpHeaders +import org.springframework.test.context.ActiveProfiles +import spock.lang.Specification + +import static org.springframework.http.HttpMethod.PUT + +/** + * @author Dmitriy Kopylenko + */ +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) +@ActiveProfiles("no-auth") +class MetadataFiltersControllerIntegrationTests extends Specification { + + @Autowired + private TestRestTemplate restTemplate + + @Autowired + MetadataResolverRepository metadataResolverRepository + + @Autowired + AttributeUtility attributeUtility + + ObjectMapper mapper + TestObjectGenerator generator + + JsonSlurper jsonSlurper = new JsonSlurper() + + static BASE_URI = '/api/MetadataResolvers' + + def setup() { + generator = new TestObjectGenerator(attributeUtility) + mapper = new ObjectMapper() + mapper.enable(SerializationFeature.INDENT_OUTPUT) + mapper.registerModule(new JavaTimeModule()) + } + + def cleanup() { + metadataResolverRepository.deleteAll() + } + + def "PUT EntityAttributesFilter"() { + given: 'MetadataResolver with attached entity attributes is available in data store' + def resolver = generator.buildRandomMetadataResolverOfType('FileBacked') + resolver.metadataFilters << generator.entityAttributesFilter() + def filterResourceId = resolver.metadataFilters[0].resourceId + def resolverResourceId = resolver.resourceId + metadataResolverRepository.save(resolver) + + + when: 'GET request is made with resource Id matching the existing filter' + def result = this.restTemplate.getForEntity("$BASE_URI/$resolverResourceId/Filters/$filterResourceId", String) + def existingFilterMap = jsonSlurper.parseText(result.body) + + and: 'PUT call is made with unmodified filter state' + def updatedResultFromPUT = this.restTemplate.exchange( + "$BASE_URI/$resolverResourceId/Filters/$filterResourceId", + PUT, + createRequestHttpEntityFor { JsonOutput.toJson(existingFilterMap) }, String) + + then: + updatedResultFromPUT.statusCode.value() == 200 + } + + private HttpEntity createRequestHttpEntityFor(Closure jsonBodySupplier) { + new HttpEntity(jsonBodySupplier(), ['Content-Type': 'application/json'] as HttpHeaders) + } + + @TestConfiguration + static class Config { + @Bean + MetadataResolver metadataResolver() { + new ChainingMetadataResolver().with { + it.id = 'tester' + it.initialize() + return it + } + } + } +} \ No newline at end of file