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 c444bd214..48a74d793 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 @@ -1,6 +1,7 @@ package edu.internet2.tier.shibboleth.admin.ui.service import com.google.common.base.Predicate +import edu.internet2.tier.shibboleth.admin.ui.configuration.ShibUIConfiguration 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.filters.EntityRoleWhiteListFilter @@ -52,6 +53,9 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService { @Autowired private MetadataResolversPositionOrderContainerService resolversPositionOrderContainerService + @Autowired + private ShibUIConfiguration shibUIConfiguration + // TODO: enhance @Override void reloadFilters(String metadataResolverResourceId) { @@ -64,6 +68,13 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService { List metadataFilters = new ArrayList<>() + // set up namespace protection + if (shibUIConfiguration.protectedAttributeNamespaces && shibUIConfiguration.protectedAttributeNamespaces.size() > 0) { + def target = new org.opensaml.saml.metadata.resolver.filter.impl.EntityAttributesFilter() + target.attributeFilter = new ScriptedPredicate(new EvaluableScript(protectedNamespaceScript())) + metadataFilters.add(target) + } + for (edu.internet2.tier.shibboleth.admin.ui.domain.filters.MetadataFilter metadataFilter : jpaMetadataResolver.getMetadataFilters()) { if (metadataFilter instanceof EntityAttributesFilter) { EntityAttributesFilter entityAttributesFilter = (EntityAttributesFilter) metadataFilter @@ -104,6 +115,21 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService { } } + private String protectedNamespaceScript() { + return """(function (attribute) { + "use strict"; + var namespaces = [${shibUIConfiguration.protectedAttributeNamespaces.collect({"\"${it}\""}).join(', ')}]; + // check the parameter + if (attribute === null) { return true; } + for (var i in namespaces) { + if (attribute.getName().startsWith(namespaces[i])) { + return false; + } + } + return true; + }(input));""" + } + private class ScriptedPredicate extends net.shibboleth.utilities.java.support.logic.ScriptedPredicate { protected ScriptedPredicate(@Nonnull EvaluableScript theScript) { super(theScript) @@ -133,9 +159,18 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService { if ((mr.type != 'BaseMetadataResolver') && (mr.enabled)) { constructXmlNodeForResolver(mr, delegate) { //TODO: enhance + def didNamespaceProtectionFilter = !(shibUIConfiguration.protectedAttributeNamespaces && shibUIConfiguration.protectedAttributeNamespaces.size() > 0) mr.metadataFilters.each { edu.internet2.tier.shibboleth.admin.ui.domain.filters.MetadataFilter filter -> + if (filter instanceof EntityAttributesFilter && !didNamespaceProtectionFilter) { + constructXmlNodeForEntityAttributeNamespaceProtection(delegate) + didNamespaceProtectionFilter = true + } constructXmlNodeForFilter(filter, delegate) } + if (!didNamespaceProtectionFilter) { + constructXmlNodeForEntityAttributeNamespaceProtection(delegate) + didNamespaceProtectionFilter = true + } } } } @@ -144,6 +179,16 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService { } } + void constructXmlNodeForEntityAttributeNamespaceProtection(def markupBuilderDelegate) { + markupBuilderDelegate.MetadataFilter('xsi:type': 'EntityAttributes') { + AttributeFilterScript() { + Script() { + mkp.yieldUnescaped("\n\n") + } + } + } + } + void constructXmlNodeForFilter(SignatureValidationFilter filter, def markupBuilderDelegate) { if(filter.xmlShouldBeGenerated()) { markupBuilderDelegate.MetadataFilter(id: filter.name, diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/CoreShibUiConfiguration.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/CoreShibUiConfiguration.java index fc1e0e8b4..fb60112f2 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/CoreShibUiConfiguration.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/CoreShibUiConfiguration.java @@ -47,7 +47,7 @@ import javax.servlet.http.HttpServletRequest; @Configuration -@EnableConfigurationProperties(CustomPropertiesConfiguration.class) +@EnableConfigurationProperties({CustomPropertiesConfiguration.class, ShibUIConfiguration.class}) public class CoreShibUiConfiguration { private static final Logger logger = LoggerFactory.getLogger(CoreShibUiConfiguration.class); diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/ShibUIConfiguration.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/ShibUIConfiguration.java new file mode 100644 index 000000000..953ed2119 --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/ShibUIConfiguration.java @@ -0,0 +1,20 @@ +package edu.internet2.tier.shibboleth.admin.ui.configuration; + +import lombok.Getter; +import lombok.Setter; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.context.annotation.Configuration; + +import java.util.List; + +@Configuration +@ConfigurationProperties(prefix = "shibui") +@Getter +@Setter +public class ShibUIConfiguration { + /** + * A list of namespaces that should be excluded from incoming metadata. This is used to prevent third party metadata + * sources from using attributes that they might not have the rights to use. + */ + private List protectedAttributeNamespaces; +} diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImplTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImplTests.groovy index 1e2e61b3c..7184270c3 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImplTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImplTests.groovy @@ -5,6 +5,7 @@ import edu.internet2.tier.shibboleth.admin.ui.configuration.Internationalization import edu.internet2.tier.shibboleth.admin.ui.configuration.MetadataResolverConverterConfiguration import edu.internet2.tier.shibboleth.admin.ui.configuration.PlaceholderResolverComponentsConfiguration import edu.internet2.tier.shibboleth.admin.ui.configuration.SearchConfiguration +import edu.internet2.tier.shibboleth.admin.ui.configuration.ShibUIConfiguration 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.filters.RequiredValidUntilFilter @@ -42,7 +43,14 @@ import org.springframework.test.annotation.DirtiesContext import org.springframework.test.context.ContextConfiguration import org.xmlunit.builder.DiffBuilder import org.xmlunit.builder.Input +import spock.lang.Ignore import spock.lang.Specification +import spock.lang.Unroll + +import javax.xml.transform.OutputKeys +import javax.xml.transform.TransformerFactory +import javax.xml.transform.dom.DOMSource +import javax.xml.transform.stream.StreamResult import static edu.internet2.tier.shibboleth.admin.ui.util.TestHelpers.generatedXmlIsTheSameAsExpectedXml @@ -74,6 +82,9 @@ class JPAMetadataResolverServiceImplTests extends Specification { @Autowired MetadataResolverConverterService mdrConverterService + @Autowired + ShibUIConfiguration shibUIConfiguration + TestObjectGenerator testObjectGenerator DOMBuilder domBuilder @@ -349,6 +360,52 @@ class JPAMetadataResolverServiceImplTests extends Specification { generatedXmlIsTheSameAsExpectedXml('/conf/704.3.xml', domBuilder.parseText(writer.toString())) } + @Unroll + @DirtiesContext(methodMode = DirtiesContext.MethodMode.AFTER_METHOD) + def 'test namespace protection [#namespaces]'() { + setup: + shibUIConfiguration.protectedAttributeNamespaces = namespaces + def resolver = new DynamicHttpMetadataResolver().with { + it.xmlId = 'DynamicHttpMetadataResolver' + it.metadataRequestURLConstructionScheme = new MetadataQueryProtocolScheme().with { + it.content = 'http://mdq-beta.incommon.org/global' + it + } + it + } + metadataResolverRepository.save(resolver) + + expect: + generatedXmlIsTheSameAsExpectedXml(filename, metadataResolverService.generateConfiguration()) + + where: + namespaces | filename + ['http://shibboleth.net/ns/profiles'] | '/conf/984.xml' + ['http://shibboleth.net/ns/profiles', 'http://scaldingspoon.com/iam'] | '/conf/984-2.xml' + } + + @Ignore('there is a bug in org.opensaml.saml.metadata.resolver.filter.impl.EntityAttributesFilter.applyFilter') + def 'test namespace protection internal filtering'() { + setup: + shibUIConfiguration.protectedAttributeNamespaces = ['http://shibboleth.net/ns/profiles'] + def resolver = new edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.ResourceBackedMetadataResolver().with { + it.resourceId = 'testme984' + it.name = 'testme984' + it.classpathMetadataResource = new ClasspathMetadataResource('metadata/984-3.xml') + it + } + def x = metadataResolverRepository.save(resolver) + ((OpenSamlChainingMetadataResolver)metadataResolver).resolvers.add(mdrConverterService.convertToOpenSamlRepresentation(x)) + + when: + metadataResolverService.reloadFilters('testme984') + def ed = metadataResolver.resolveSingle(new CriteriaSet(new EntityIdCriterion('http://test.scaldingspoon.org/test1'))) + def x2 = openSamlObjects.marshalToXmlString(ed) + + then: + !DiffBuilder.compare(Input.fromStream(this.class.getResourceAsStream('/metadata/984-3-expected.xml'))).withTest(Input.fromString(openSamlObjects.marshalToXmlString(ed))).ignoreComments().ignoreWhitespace().build().hasDifferences() + } + static genXmlSnippet(MarkupBuilder xml, Closure xmlNodeGenerator) { xml.MetadataProvider('id': 'ShibbolethMetadata', 'xmlns': 'urn:mace:shibboleth:2.0:metadata', diff --git a/backend/src/test/resources/conf/984-2.xml b/backend/src/test/resources/conf/984-2.xml new file mode 100644 index 000000000..0c5749f10 --- /dev/null +++ b/backend/src/test/resources/conf/984-2.xml @@ -0,0 +1,33 @@ + + + + http://mdq-beta.incommon.org/global + + + + + + + diff --git a/backend/src/test/resources/conf/984.xml b/backend/src/test/resources/conf/984.xml new file mode 100644 index 000000000..5974aa861 --- /dev/null +++ b/backend/src/test/resources/conf/984.xml @@ -0,0 +1,33 @@ + + + + http://mdq-beta.incommon.org/global + + + + + + + diff --git a/backend/src/test/resources/metadata/984-3-expected.xml b/backend/src/test/resources/metadata/984-3-expected.xml new file mode 100644 index 000000000..b15921f7d --- /dev/null +++ b/backend/src/test/resources/metadata/984-3-expected.xml @@ -0,0 +1,20 @@ + + + + + + internal + + + + + urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified + + + \ No newline at end of file diff --git a/backend/src/test/resources/metadata/984-3.xml b/backend/src/test/resources/metadata/984-3.xml new file mode 100644 index 000000000..87ab7d428 --- /dev/null +++ b/backend/src/test/resources/metadata/984-3.xml @@ -0,0 +1,23 @@ + + + + + + internal + + + givenName + employeeNumber + + + + + urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified + + + \ No newline at end of file