[SHIBUI-1058][SHIBUI-1231]

Fixed a bug where we were persisting boolean relying party overrides improperly. We now only persist when the value from the UI is true. Note that we still persist based on how the override is defined in the yaml. In other words, the UI controls whether or not to persist and the yaml controls what is actually persisted.
TIER · Feb 14, 2019 · 9cc0050 · 9cc0050
1 parent 2fd8f8d
commit 9cc0050
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 22 deletions.
diff --git a/...nd/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java b/...nd/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityServiceImpl.java
@@ -104,22 +104,23 @@ public List<Attribute> getAttributeListFromRelyingPartyOverridesRepresentation(M
             RelyingPartyOverrideProperty overrideProperty = overridePropertyList.stream().filter(op -> op.getName().equals(key)).findFirst().get();
             switch (ModelRepresentationConversions.AttributeTypes.valueOf(overrideProperty.getDisplayType().toUpperCase())) {
                 case BOOLEAN:
-                    if (overrideProperty.getPersistType() != null &&
-                        !overrideProperty.getPersistType().equalsIgnoreCase("boolean") &&
-                            ((entry.getValue() instanceof Boolean && (Boolean)entry.getValue()) ||
-                            ((entry.getValue() instanceof String) && Boolean.valueOf((String)entry.getValue())))) {
-                        list.add(attributeUtility.createAttributeWithStringValues(overrideProperty.getAttributeName(),
-                                                                                   overrideProperty.getAttributeFriendlyName(),
-                                                                                   overrideProperty.getPersistValue()));
-                    } else {
-                        if (entry.getValue() instanceof String) {
-                            list.add(attributeUtility.createAttributeWithBooleanValue(overrideProperty.getAttributeName(),
+                    if ((entry.getValue() instanceof Boolean && (Boolean)entry.getValue()) ||
+                        ((entry.getValue() instanceof String) && Boolean.valueOf((String)entry.getValue()))) {
+                        if (overrideProperty.getPersistType() != null &&
+                            !overrideProperty.getPersistType().equalsIgnoreCase("boolean")) {
+                            list.add(attributeUtility.createAttributeWithStringValues(overrideProperty.getAttributeName(),
                                     overrideProperty.getAttributeFriendlyName(),
-                                    Boolean.valueOf((String) entry.getValue())));
+                                    overrideProperty.getPersistValue()));
                         } else {
-                            list.add(attributeUtility.createAttributeWithBooleanValue(overrideProperty.getAttributeName(),
-                                    overrideProperty.getAttributeFriendlyName(),
-                                    (Boolean) entry.getValue()));
+                            if (entry.getValue() instanceof String) {
+                                list.add(attributeUtility.createAttributeWithBooleanValue(overrideProperty.getAttributeName(),
+                                        overrideProperty.getAttributeFriendlyName(),
+                                        Boolean.valueOf((String) entry.getValue())));
+                            } else {
+                                list.add(attributeUtility.createAttributeWithBooleanValue(overrideProperty.getAttributeName(),
+                                        overrideProperty.getAttributeFriendlyName(),
+                                        (Boolean) entry.getValue()));
+                            }
                         }
                     }
                     break;

diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestHelpers.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestHelpers.groovy
@@ -20,8 +20,8 @@ class TestHelpers {
                 count += ((Collection)entry.value).size() != 0 ? 1 : 0
             } else if (entry.value instanceof String) {
                 count += StringUtils.isNotBlank((String)entry.value) ? 1 : 0
-            } else {
-                count++
+            } else if (entry.value instanceof Boolean) {
+                count += Boolean.valueOf((Boolean)entry.value) ? 1 : 0
             }
         }
 

diff --git a/...nd/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy b/...nd/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy
@@ -348,12 +348,7 @@ class TestObjectGenerator {
         customPropertiesConfiguration.getOverrides().each { override ->
             switch (ModelRepresentationConversions.AttributeTypes.valueOf(override.getDisplayType().toUpperCase())) {
                 case ModelRepresentationConversions.AttributeTypes.BOOLEAN:
-                    if (override.getPersistType() != null &&
-                            override.getPersistType() != override.getDisplayType()) {
-                        representation.put(override.getName(), generator.randomString(30))
-                    } else {
-                        representation.put(override.getName(), generator.randomBoolean())
-                    }
+                    representation.put(override.getName(), generator.randomBoolean())
                     break
                 case ModelRepresentationConversions.AttributeTypes.INTEGER:
                     representation.put(override.getName(), generator.randomInt(0, 999999))