From f78ab5a3b3249017b3cb7b81322f76dca685d7fa Mon Sep 17 00:00:00 2001 From: Dmitriy Kopylenko Date: Fri, 7 Dec 2018 08:54:06 -0500 Subject: [PATCH] SHIBUI-799 - refactor model --- .../JPAMetadataResolverServiceImpl.groovy | 78 ++++++++++++------- .../controller/MetadataFiltersController.java | 1 + .../ui/domain/filters/NameIdFormatFilter.java | 24 ++---- .../filters/NameIdFormatFilterTarget.java | 51 ++++++++++++ .../MetadataFiltersControllerTests.groovy | 1 + ...ymorphicFiltersJacksonHandlingTests.groovy | 43 +++++----- .../repository/FilterRepositoryTests.groovy | 2 +- .../admin/ui/util/TestObjectGenerator.groovy | 26 ++----- backend/src/test/resources/conf/799.xml | 12 --- 9 files changed, 138 insertions(+), 100 deletions(-) create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/NameIdFormatFilterTarget.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 64c5c9098..cf7c44d8f 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 @@ -37,9 +37,9 @@ import org.w3c.dom.Document import javax.annotation.Nonnull -import static edu.internet2.tier.shibboleth.admin.ui.domain.filters.NameIdFormatFilter.FormatAndTarget.Type.CONDITION_REF -import static edu.internet2.tier.shibboleth.admin.ui.domain.filters.NameIdFormatFilter.FormatAndTarget.Type.CONDITION_SCRIPT -import static edu.internet2.tier.shibboleth.admin.ui.domain.filters.NameIdFormatFilter.FormatAndTarget.Type.ENTITY +import static edu.internet2.tier.shibboleth.admin.ui.domain.filters.NameIdFormatFilterTarget.NameIdFormatFilterTargetType.ENTITY +import static edu.internet2.tier.shibboleth.admin.ui.domain.filters.NameIdFormatFilterTarget.NameIdFormatFilterTargetType.CONDITION_SCRIPT +import static edu.internet2.tier.shibboleth.admin.ui.domain.filters.NameIdFormatFilterTarget.NameIdFormatFilterTargetType.REGEX import static edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.ResourceBackedMetadataResolver.ResourceType.CLASSPATH import static edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.ResourceBackedMetadataResolver.ResourceType.SVN @@ -62,7 +62,9 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService { @Override void reloadFilters(String metadataResolverResourceId) { OpenSamlChainingMetadataResolver chainingMetadataResolver = (OpenSamlChainingMetadataResolver) metadataResolver - MetadataResolver targetMetadataResolver = chainingMetadataResolver.getResolvers().find { it.id == metadataResolverResourceId } + 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) { @@ -98,22 +100,25 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService { target.setRules(rules) metadataFilters.add(target) } - if(metadataFilter instanceof NameIdFormatFilter) { + if (metadataFilter instanceof NameIdFormatFilter) { NameIdFormatFilter nameIdFormatFilter = NameIdFormatFilter.cast(metadataFilter) NameIDFormatFilter openSamlTargetFilter = new OpenSamlNameIdFormatFilter() Map, Collection> predicateRules = [:] - nameIdFormatFilter.formats.each { - switch (it.type) { - case ENTITY: - predicateRules.put(new EntityIdPredicate([it.value]), [it.format]) - break - case CONDITION_SCRIPT: - predicateRules.put(new ScriptedPredicate(new EvaluableScript(it.value)), [it.format]) - break - default: - // do nothing, we'd have exploded elsewhere previously. - break - } + def type = nameIdFormatFilter.nameIdFormatFilterTarget.nameIdFormatFilterTargetType + def values = nameIdFormatFilter.nameIdFormatFilterTarget.value + switch (type) { + case ENTITY: + predicateRules[new EntityIdPredicate(values)] = nameIdFormatFilter.formats + break + case CONDITION_SCRIPT: + predicateRules[new ScriptedPredicate(new EvaluableScript(values[0]))] = nameIdFormatFilter.formats + break + case REGEX: + predicateRules[new ScriptedPredicate(new EvaluableScript(generateJavaScriptRegexScript(values[0])))] = nameIdFormatFilter.formats + break + default: + // do nothing, we'd have exploded elsewhere previously. + break } openSamlTargetFilter.rules = predicateRules metadataFilters << openSamlTargetFilter @@ -171,7 +176,7 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService { } void constructXmlNodeForFilter(SignatureValidationFilter filter, def markupBuilderDelegate) { - if(filter.xmlShouldBeGenerated()) { + if (filter.xmlShouldBeGenerated()) { markupBuilderDelegate.MetadataFilter(id: filter.name, 'xsi:type': 'SignatureValidation', 'xmlns:md': 'urn:oasis:names:tc:SAML:2.0:metadata', @@ -242,7 +247,7 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService { } void constructXmlNodeForFilter(RequiredValidUntilFilter filter, def markupBuilderDelegate) { - if(filter.xmlShouldBeGenerated()) { + if (filter.xmlShouldBeGenerated()) { markupBuilderDelegate.MetadataFilter( 'xsi:type': 'RequiredValidUntil', maxValidityInterval: filter.maxValidityInterval @@ -251,27 +256,39 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService { } void constructXmlNodeForFilter(NameIdFormatFilter filter, def markupBuilderDelegate) { + def type = filter.nameIdFormatFilterTarget.nameIdFormatFilterTargetType markupBuilderDelegate.MetadataFilter( 'xsi:type': 'NameIDFormat', 'xmlns:md': 'urn:oasis:names:tc:SAML:2.0:metadata', 'removeExistingFormats': filter.removeExistingFormats ?: null ) { filter.formats.each { - Format(it.format) - if(it.type == ENTITY) { - Entity(it.value) - } - else if(it.type == CONDITION_REF) { - ConditionRef(it.value) - } - else if(it.type == CONDITION_SCRIPT) { - def scriptText = it.value + Format(it) + } + switch (type) { + case ENTITY: + filter.nameIdFormatFilterTarget.value.each { + Entity(it) + } + break + case CONDITION_SCRIPT: + case REGEX: ConditionScript() { Script() { - mkp.yieldUnescaped("\n\n") + def script + def scriptValue = filter.nameIdFormatFilterTarget.value[0] + if (type == CONDITION_SCRIPT) { + script = scriptValue + } else if (type == REGEX) { + script = generateJavaScriptRegexScript(scriptValue) + } + mkp.yieldUnescaped("\n\n") } } - } + break + default: + // do nothing, we'd have exploded elsewhere previously. + break } } } @@ -493,4 +510,5 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService { } } + } \ No newline at end of file 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 0ae56b781..57d734f77 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 @@ -230,6 +230,7 @@ else if (filterWithUpdatedData instanceof NameIdFormatFilter) { NameIdFormatFilter fromFilter = NameIdFormatFilter.class.cast(filterWithUpdatedData); toFilter.setRemoveExistingFormats(fromFilter.getRemoveExistingFormats()); toFilter.setFormats(fromFilter.getFormats()); + toFilter.setNameIdFormatFilterTarget(fromFilter.getNameIdFormatFilterTarget()); } //TODO: add other types of concrete filters update here } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/NameIdFormatFilter.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/NameIdFormatFilter.java index a2fb316f3..6c67252b0 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/NameIdFormatFilter.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/NameIdFormatFilter.java @@ -7,9 +7,11 @@ import lombok.Setter; import lombok.ToString; +import javax.persistence.CascadeType; import javax.persistence.ElementCollection; import javax.persistence.Embeddable; import javax.persistence.Entity; +import javax.persistence.OneToOne; import javax.persistence.OrderColumn; import java.util.List; @@ -28,21 +30,9 @@ public NameIdFormatFilter() { @ElementCollection @OrderColumn - private List formats; - - @Embeddable - @NoArgsConstructor - @AllArgsConstructor - @Getter - @Setter - @EqualsAndHashCode - public static class FormatAndTarget { - private String format; - private String value; - private Type type; - - public enum Type { - ENTITY, CONDITION_REF, CONDITION_SCRIPT - } - } + private List formats; + + @OneToOne(cascade = CascadeType.ALL) + private NameIdFormatFilterTarget nameIdFormatFilterTarget; + } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/NameIdFormatFilterTarget.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/NameIdFormatFilterTarget.java new file mode 100644 index 000000000..a346d983f --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/NameIdFormatFilterTarget.java @@ -0,0 +1,51 @@ +package edu.internet2.tier.shibboleth.admin.ui.domain.filters; + +import edu.internet2.tier.shibboleth.admin.ui.domain.AbstractAuditable; +import lombok.EqualsAndHashCode; +import lombok.ToString; + +import javax.persistence.ElementCollection; +import javax.persistence.Entity; +import javax.persistence.OrderColumn; +import java.util.ArrayList; +import java.util.List; + +@Entity +@EqualsAndHashCode(callSuper = true) +@ToString +public class NameIdFormatFilterTarget extends AbstractAuditable { + + public enum NameIdFormatFilterTargetType { + ENTITY, CONDITION_SCRIPT, REGEX + } + + private NameIdFormatFilterTargetType nameIdFormatFilterTargetType; + + public NameIdFormatFilterTargetType getNameIdFormatFilterTargetType() { + return nameIdFormatFilterTargetType; + } + + public void setNameIdFormatFilterTargetType(NameIdFormatFilterTargetType nameIdFormatFilterTargetType) { + this.nameIdFormatFilterTargetType = nameIdFormatFilterTargetType; + } + + @ElementCollection + @OrderColumn + private List value; + + public List getValue() { + return value; + } + + public void setSingleValue(String value) { + List values = new ArrayList<>(); + values.add(value); + this.value = values; + } + + public void setValue(List value) { + this.value = value; + } + + +} diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerTests.groovy index 15717ca61..802afdb81 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerTests.groovy @@ -167,6 +167,7 @@ class MetadataFiltersControllerTests extends Specification { .content(postedJsonBody)) then: + println postedJsonBody result.andExpect(status().isCreated()) .andExpect(content().json(expectedJsonBody, true)) .andExpect(header().string(expectedResponseHeader, containsString(expectedResponseHeaderValue))) diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/filters/PolymorphicFiltersJacksonHandlingTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/filters/PolymorphicFiltersJacksonHandlingTests.groovy index 383632119..9c1ba5342 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/filters/PolymorphicFiltersJacksonHandlingTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/filters/PolymorphicFiltersJacksonHandlingTests.groovy @@ -180,28 +180,27 @@ class PolymorphicFiltersJacksonHandlingTests extends Specification { given: def givenFilterJson = """ { - "@type" : "NameIDFormat", - "name" : "NameIDFormat", - "resourceId" : "20147f53-e368-4911-921a-2e24598e37f8", - "filterEnabled" : true, - "removeExistingFormats" : false, - "formats" : [ { - "format" : "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent", - "value" : "https://sp1.example.org", - "type" : "ENTITY" - }, { - "format" : "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress", - "value" : "https://sp2.example.org", - "type" : "ENTITY" - }, { - "format" : "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent", - "value" : "conditionRefBeanId", - "type" : "CONDITION_REF" - }, { - "format" : "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress", - "value" : "input.getEntityID().equals(\\"https://sp1.example.org\\");", - "type" : "CONDITION_SCRIPT" - } ] + "createdDate" : null, + "modifiedDate" : null, + "createdBy" : null, + "modifiedBy" : null, + "name" : "NameIDFormat", + "resourceId" : "ab95b80f-102b-494c-a3b8-27b625553977", + "filterEnabled" : false, + "removeExistingFormats" : false, + "formats" : [ "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent" ], + "nameIdFormatFilterTarget" : { + "createdDate" : null, + "modifiedDate" : null, + "createdBy" : null, + "modifiedBy" : null, + "nameIdFormatFilterTargetType" : "ENTITY", + "value" : [ "https://sp1.example.org" ], + "audId" : null + }, + "audId" : null, + "@type" : "NameIDFormat", + "version" : 1896953777 }""" when: diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/FilterRepositoryTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/FilterRepositoryTests.groovy index 7a733d011..d39cddc93 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/FilterRepositoryTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/FilterRepositoryTests.groovy @@ -83,6 +83,6 @@ class FilterRepositoryTests extends Specification { then: persistedFilter.audId > 0L - persistedFilter.formats.size() == 4 + persistedFilter.formats.size() == 1 } } 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 482e31007..8eb8426f4 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 @@ -15,10 +15,7 @@ import org.opensaml.saml.saml2.metadata.Organization import java.nio.file.Files import java.util.function.Supplier -import static edu.internet2.tier.shibboleth.admin.ui.domain.filters.NameIdFormatFilter.* -import static edu.internet2.tier.shibboleth.admin.ui.domain.filters.NameIdFormatFilter.FormatAndTarget.* -import static edu.internet2.tier.shibboleth.admin.ui.domain.filters.NameIdFormatFilter.FormatAndTarget.Type.* -import static edu.internet2.tier.shibboleth.admin.ui.domain.filters.NameIdFormatFilter.FormatAndTarget.Type.* +import static edu.internet2.tier.shibboleth.admin.ui.domain.filters.NameIdFormatFilterTarget.NameIdFormatFilterTargetType.ENTITY /** * @author Bill Smith (wsmith@unicon.net) @@ -225,20 +222,13 @@ class TestObjectGenerator { static NameIdFormatFilter nameIdFormatFilter() { return new NameIdFormatFilter().with { it.name = "NameIDFormat" - it.formats = [ - new FormatAndTarget( - format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent', - type: Type.ENTITY, value: 'https://sp1.example.org'), - new FormatAndTarget( - format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:emailAddress', - type: Type.ENTITY, value: 'https://sp2.example.org'), - new FormatAndTarget( - format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent', - type: Type.CONDITION_REF, value: 'conditionRefBeanId'), - new FormatAndTarget( - format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent', - type: Type.CONDITION_SCRIPT, value: 'input.getEntityID().equals("https://sp1.example.org");') - ] + it.formats = ['urn:oasis:names:tc:SAML:2.0:nameid-format:persistent'] + it.nameIdFormatFilterTarget = new NameIdFormatFilterTarget(nameIdFormatFilterTargetType: ENTITY, singleValue: 'https://sp1.example.org') + + /*it.name = "NameIDFormat" + it.formats = ['urn:oasis:names:tc:SAML:2.0:nameid-format:persistent', 'urn:oasis:names:tc:SAML:2.0:nameid-format:email'] + it.nameIdFormatFilterTarget = new NameIdFormatFilterTarget(nameIdFormatFilterTargetType: CONDITION_SCRIPT, singleValue: 'eval(true);')*/ + it } } diff --git a/backend/src/test/resources/conf/799.xml b/backend/src/test/resources/conf/799.xml index b7e32082a..90f43f50d 100644 --- a/backend/src/test/resources/conf/799.xml +++ b/backend/src/test/resources/conf/799.xml @@ -8,17 +8,5 @@ urn:oasis:names:tc:SAML:2.0:nameid-format:persistent https://sp1.example.org - urn:oasis:names:tc:SAML:2.0:nameid-format:emailAddress - https://sp2.example.org - urn:oasis:names:tc:SAML:2.0:nameid-format:persistent - conditionRefBeanId - urn:oasis:names:tc:SAML:2.0:nameid-format:persistent - - -