Skip to content

Commit

Permalink
Merged in SHIBUI-1249 (pull request #432)
Browse files Browse the repository at this point in the history 
SHIBUI-1249

Approved-by: Ryan Mathis <rmathis@unicon.net>
  • Loading branch information
@dima767 @rmathis
dima767 authored and rmathis committed Nov 21, 2019
2 parents 1e61381 + edbc2cf commit 74b9046
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 19 deletions.
48 changes: 30 additions & 18 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 @@ -15,6 +15,7 @@
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.transaction.interceptor.TransactionAspectSupport;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.GetMapping;
Expand All @@ -27,6 +28,7 @@
import org.springframework.web.client.HttpClientErrorException;
import org.springframework.web.servlet.support.ServletUriComponentsBuilder;

import javax.script.ScriptException;
import java.net.URI;
import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -57,7 +59,7 @@ public class MetadataFiltersController {

@ExceptionHandler
public ResponseEntity<?> notFoundHandler(HttpClientErrorException ex) {
if(ex.getStatusCode() == NOT_FOUND) {
if (ex.getStatusCode() == NOT_FOUND) {
return ResponseEntity.notFound().build();
}
throw ex;
Expand All @@ -78,13 +80,14 @@ public ResponseEntity<?> getOne(@PathVariable String metadataResolverId, @PathVa
}

@PostMapping("/Filters")
@Transactional
public ResponseEntity<?> create(@PathVariable String metadataResolverId, @RequestBody MetadataFilter createdFilter) {
MetadataResolver metadataResolver = findResolverOrThrowHttp404(metadataResolverId);
metadataResolver.addFilter(createdFilter);
MetadataResolver persistedMr = repository.save(metadataResolver);

// we reload the filters here after save
metadataResolverService.reloadFilters(persistedMr.getResourceId());
reloadFiltersAndHandleScriptException(persistedMr.getResourceId());

MetadataFilter persistedFilter = newlyPersistedFilter(persistedMr.getMetadataFilters().stream(), createdFilter.getResourceId());

Expand All @@ -106,7 +109,7 @@ public ResponseEntity<?> update(@PathVariable String metadataResolverId,
.stream()
.filter(it -> it.getResourceId().equals(resourceId))
.findFirst();
if(!filterTobeUpdatedOptional.isPresent()) {
if (!filterTobeUpdatedOptional.isPresent()) {
return ResponseEntity.notFound().build();
}
MetadataFilter filterTobeUpdated = filterTobeUpdatedOptional.get();
Expand All @@ -129,8 +132,8 @@ public ResponseEntity<?> update(@PathVariable String metadataResolverId,
metadataResolver.markAsModified();
repository.save(metadataResolver);

// TODO: this is wrong
metadataResolverService.reloadFilters(metadataResolver.getResourceId());
// TODO: do we need to reload filters here?
reloadFiltersAndHandleScriptException(metadataResolver.getResourceId());

return ResponseEntity.ok().body(persistedFilter);
}
Expand All @@ -149,7 +152,7 @@ public ResponseEntity<?> delete(@PathVariable String metadataResolverId,
//change that we need to make in the entire code base
List<MetadataFilter> updatedFilters = new ArrayList<>(resolver.getMetadataFilters());
boolean removed = updatedFilters.removeIf(f -> f.getResourceId().equals(resourceId));
if(!removed) {
if (!removed) {
throw HTTP_404_CLIENT_ERROR_EXCEPTION.get();
}
resolver.setMetadataFilters(updatedFilters);
Expand All @@ -164,17 +167,31 @@ public ResponseEntity<?> delete(@PathVariable String metadataResolverId,
return ResponseEntity.noContent().build();
}

private void reloadFiltersAndHandleScriptException(String resolverResourceId) {
try {
metadataResolverService.reloadFilters(resolverResourceId);
} catch (Throwable ex) {
//explicitly mark transaction for rollback when we get ScriptException as we call reloadFilters
//after persistence call. Then re-throw the exception
//to let RestControllerSupport advice return proper 400 error message
if (ex instanceof ScriptException) {
TransactionAspectSupport.currentTransactionStatus().setRollbackOnly();
throw ex;
}
}
}

private MetadataResolver findResolverOrThrowHttp404(String resolverResourceId) {
MetadataResolver resolver = repository.findByResourceId(resolverResourceId);
if(resolver == null) {
if (resolver == null) {
throw HTTP_404_CLIENT_ERROR_EXCEPTION.get();
}
return resolver;
}

private MetadataFilter findFilterOrThrowHttp404(String filterResourceId) {
MetadataFilter filter = filterRepository.findByResourceId(filterResourceId);
if(filter == null) {
if (filter == null) {
throw HTTP_404_CLIENT_ERROR_EXCEPTION.get();
}
return filter;
Expand All @@ -189,28 +206,25 @@ private MetadataFilter newlyPersistedFilter(Stream<MetadataFilter> filters, fina
}

/**
*
* Add else if instanceof block here for each concrete filter types we add in the future
*/
private void updateConcreteFilterTypeData(MetadataFilter filterToBeUpdated, MetadataFilter filterWithUpdatedData) {
//TODO: Could we maybe use Dozer here before things get out of control? https://dozermapper.github.io
// Mapper mapper = new net.sf.dozer.Mapper(); // or autowire one
// mapper.map(fromFilter, toFilter);
if(filterWithUpdatedData instanceof EntityAttributesFilter) {
if (filterWithUpdatedData instanceof EntityAttributesFilter) {
EntityAttributesFilter toFilter = EntityAttributesFilter.class.cast(filterToBeUpdated);
EntityAttributesFilter fromFilter = EntityAttributesFilter.class.cast(filterWithUpdatedData);
toFilter.setEntityAttributesFilterTarget(fromFilter.getEntityAttributesFilterTarget());
toFilter.setRelyingPartyOverrides(fromFilter.getRelyingPartyOverrides());
toFilter.setAttributeRelease(fromFilter.getAttributeRelease());
}
else if(filterWithUpdatedData instanceof EntityRoleWhiteListFilter) {
} else if (filterWithUpdatedData instanceof EntityRoleWhiteListFilter) {
EntityRoleWhiteListFilter toFilter = EntityRoleWhiteListFilter.class.cast(filterToBeUpdated);
EntityRoleWhiteListFilter fromFilter = EntityRoleWhiteListFilter.class.cast(filterWithUpdatedData);
toFilter.setRemoveEmptyEntitiesDescriptors(fromFilter.getRemoveEmptyEntitiesDescriptors());
toFilter.setRemoveRolelessEntityDescriptors(fromFilter.getRemoveRolelessEntityDescriptors());
toFilter.setRetainedRoles(fromFilter.getRetainedRoles());
}
else if (filterWithUpdatedData instanceof SignatureValidationFilter) {
} else if (filterWithUpdatedData instanceof SignatureValidationFilter) {
SignatureValidationFilter toFilter = SignatureValidationFilter.class.cast(filterToBeUpdated);
SignatureValidationFilter fromFilter = SignatureValidationFilter.class.cast(filterWithUpdatedData);
toFilter.setRequireSignedRoot(fromFilter.getRequireSignedRoot());
Expand All @@ -220,13 +234,11 @@ else if (filterWithUpdatedData instanceof SignatureValidationFilter) {
toFilter.setDynamicTrustedNamesStrategyRef(fromFilter.getDynamicTrustedNamesStrategyRef());
toFilter.setTrustEngineRef(fromFilter.getTrustEngineRef());
toFilter.setPublicKey(fromFilter.getPublicKey());
}
else if(filterWithUpdatedData instanceof RequiredValidUntilFilter) {
} else if (filterWithUpdatedData instanceof RequiredValidUntilFilter) {
RequiredValidUntilFilter toFilter = RequiredValidUntilFilter.class.cast(filterToBeUpdated);
RequiredValidUntilFilter fromFilter = RequiredValidUntilFilter.class.cast(filterWithUpdatedData);
toFilter.setMaxValidityInterval(fromFilter.getMaxValidityInterval());
}
else if (filterWithUpdatedData instanceof NameIdFormatFilter) {
} else if (filterWithUpdatedData instanceof NameIdFormatFilter) {
NameIdFormatFilter toFilter = NameIdFormatFilter.class.cast(filterToBeUpdated);
NameIdFormatFilter fromFilter = NameIdFormatFilter.class.cast(filterWithUpdatedData);
toFilter.setRemoveExistingFormats(fromFilter.getRemoveExistingFormats());
Expand Down
37 changes: 36 additions & 1 deletion ...net2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ 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.configuration.CustomPropertiesConfiguration
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.resolvers.opensaml.OpenSamlChainingMetadataResolver
import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository
import edu.internet2.tier.shibboleth.admin.ui.service.MetadataResolverConverterService
Expand All @@ -23,6 +25,7 @@ import org.springframework.http.HttpHeaders
import org.springframework.test.context.ActiveProfiles
import spock.lang.Specification

import static edu.internet2.tier.shibboleth.admin.ui.domain.filters.EntityAttributesFilterTarget.EntityAttributesFilterTargetType.CONDITION_SCRIPT
import static org.springframework.http.HttpMethod.PUT

/**
Expand Down Expand Up @@ -191,7 +194,7 @@ class MetadataFiltersControllerIntegrationTests extends Specification {
def "POST new Filter updates resolver's modifiedDate - SHIBUI-1500"() {
given: 'MetadataResolver with attached entity attributes is available in data store'
def resolver = generator.buildRandomMetadataResolverOfType('FileBacked')
def filter = generator.entityAttributesFilter()
def filter = generator.entityAttributesFilter()
def resolverResourceId = resolver.resourceId
metadataResolverRepository.save(resolver)
MetadataResolver openSamlRepresentation = metadataResolverConverterService.convertToOpenSamlRepresentation(resolver)
Expand All @@ -209,6 +212,38 @@ class MetadataFiltersControllerIntegrationTests extends Specification {
originalModifiedDate < afterFilterAddedModifiedDate
}

def "EntityAttributesFilter with invalid script does not result in persisting that filter"() {
def resolver = generator.buildRandomMetadataResolverOfType('FileBacked')
def resolverResourceId = resolver.resourceId
metadataResolverRepository.save(resolver)
MetadataResolver openSamlRepresentation = metadataResolverConverterService.convertToOpenSamlRepresentation(resolver)
OpenSamlChainingMetadataResolverUtil.updateChainingMetadataResolver((OpenSamlChainingMetadataResolver) chainingMetadataResolver, openSamlRepresentation)
def filter = new EntityAttributesFilter().with {
it.name = 'SHIBUI-1249'
it.resourceId = 'SHIBUI-1249'
it.entityAttributesFilterTarget = new EntityAttributesFilterTarget().with {
it.entityAttributesFilterTargetType = CONDITION_SCRIPT
it.singleValue = """
echo('invalid;
"""
it
}
it
}

when:
def result = restTemplate.postForEntity("$BASE_URI/$resolverResourceId/Filters", filter, String)

then:
result.statusCodeValue == 400

when:
result = this.restTemplate.getForEntity("$BASE_URI/$resolverResourceId", Map)

then:
result.body.metadataFilters.size == 0
}

private HttpEntity<String> createRequestHttpEntityFor(Closure jsonBodySupplier) {
new HttpEntity<String>(jsonBodySupplier(), ['Content-Type': 'application/json'] as HttpHeaders)
}
Expand Down

0 comments on commit 74b9046

Please sign in to comment.