From 0f79e0bc370162ea8f92f6bb9aeb23d674daa8c0 Mon Sep 17 00:00:00 2001
From: Bill Smith <wsmith@unicon.net>
Date: Tue, 29 Jan 2019 15:55:05 -0700
Subject: [PATCH] [SHIBUI-960]

Added a check to see if an EntityRoleWhitelistFilter has any retained
roles or not. If not, we skip generating that xml element entirely.

Includes accompanying unit test.
---
 .../JPAMetadataResolverServiceImpl.groovy     | 16 +++++++-------
 ...JPAMetadataResolverServiceImplTests.groovy | 21 +++++++++++++++++++
 2 files changed, 30 insertions(+), 7 deletions(-)

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 be89653f4..ef50962b0 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
@@ -281,13 +281,15 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService {
     }
 
     void constructXmlNodeForFilter(EntityRoleWhiteListFilter filter, def markupBuilderDelegate) {
-        markupBuilderDelegate.MetadataFilter(
-                'xsi:type': 'EntityRoleWhiteList',
-                'xmlns:md': 'urn:oasis:names:tc:SAML:2.0:metadata'
-        ) {
-            filter.retainedRoles.each {
-                // TODO: fix
-                markupBuilderDelegate.RetainedRole(it.startsWith('md:') ? it : "md:${it}")
+        if (!filter.retainedRoles?.isEmpty()) {
+            markupBuilderDelegate.MetadataFilter(
+                    'xsi:type': 'EntityRoleWhiteList',
+                    'xmlns:md': 'urn:oasis:names:tc:SAML:2.0:metadata'
+            ) {
+                filter.retainedRoles.each {
+                    // TODO: fix
+                    markupBuilderDelegate.RetainedRole(it.startsWith('md:') ? it : "md:${it}")
+                }
             }
         }
     }
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 b46888e14..dd55874a0 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
@@ -8,6 +8,7 @@ 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.MetadataFilter
 import edu.internet2.tier.shibboleth.admin.ui.domain.filters.RequiredValidUntilFilter
 import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.ClasspathMetadataResource
 import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.DynamicHttpMetadataResolver
@@ -239,6 +240,26 @@ class JPAMetadataResolverServiceImplTests extends Specification {
         generatedXmlIsTheSameAsExpectedXml('/conf/532.xml', domBuilder.parseText(writer.toString()))
     }
 
+    def 'SHIBUI-960 test generating FileBackedHttpMetadataResolver with empty EntityRoleWhitelistFilter produces no filter in the xml'() {
+        given:
+        def resolver = testObjectGenerator.fileBackedHttpMetadataResolver()
+        def erwFilter = testObjectGenerator.entityRoleWhitelistFilter()
+        erwFilter.retainedRoles.clear()
+        resolver.metadataFilters.add(erwFilter)
+
+        when:
+        genXmlSnippet(markupBuilder) {
+            JPAMetadataResolverServiceImpl.cast(metadataResolverService).constructXmlNodeForResolver(resolver, markupBuilder) {
+                resolver.metadataFilters.each { MetadataFilter filter ->
+                    JPAMetadataResolverServiceImpl.cast(metadataResolverService).constructXmlNodeForFilter(filter, markupBuilder)
+                }
+            }
+        }
+
+        then:
+        generatedXmlIsTheSameAsExpectedXml('/conf/532.xml', domBuilder.parseText(writer.toString()))
+    }
+
     def 'test generating ResourceBackedMetadataResolver with SVN resource type xml snippet'() {
         given:
         def resolver = new edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.ResourceBackedMetadataResolver().with {