keycloak-aplcache

Merge pull request #3845 from frelibert/KEYCLOAK-4378 KEYCLOAK-4378

2/9/2017 7:02:09 AM

Details

diff --git a/services/src/main/java/org/keycloak/broker/saml/mappers/UserAttributeMapper.java b/services/src/main/java/org/keycloak/broker/saml/mappers/UserAttributeMapper.java
index 8a71493..cf550ed 100755
--- a/services/src/main/java/org/keycloak/broker/saml/mappers/UserAttributeMapper.java
+++ b/services/src/main/java/org/keycloak/broker/saml/mappers/UserAttributeMapper.java
@@ -173,13 +173,17 @@ public class UserAttributeMapper extends AbstractIdentityProviderMapper {
             setIfNotEmpty(user::setLastName, attributeValuesInContext);
         } else {
             List<String> currentAttributeValues = user.getAttributes().get(attribute);
-            if (attributeValuesInContext != null
-                    && currentAttributeValues != null
-                    && !CollectionUtil.collectionEquals(attributeValuesInContext, currentAttributeValues)) {
-                user.setAttribute(attribute, attributeValuesInContext);
-            } else if (attributeValuesInContext == null) {
+            if (attributeValuesInContext == null) {
+                // attribute no longer sent by brokered idp, remove it
                 user.removeAttribute(attribute);
+            } else if (currentAttributeValues == null) {
+                // new attribute sent by brokered idp, add it
+                user.setAttribute(attribute, attributeValuesInContext);
+            } else if (!CollectionUtil.collectionEquals(attributeValuesInContext, currentAttributeValues)) {
+                // attribute sent by brokered idp has different values as before, update it
+                user.setAttribute(attribute, attributeValuesInContext);
             }
+            // attribute allready set
         }
     }
 
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractUserAttributeMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractUserAttributeMapperTest.java
index b2af5ff..2e15ad1 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractUserAttributeMapperTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractUserAttributeMapperTest.java
@@ -233,4 +233,24 @@ public abstract class AbstractUserAttributeMapperTest extends AbstractBaseBroker
           .build()
         );
     }
+
+    @Test
+    public void testAddBasicMappingMultipleValues() {
+        testValueMapping(ImmutableMap.<String, List<String>>builder()
+          .build(),
+          ImmutableMap.<String, List<String>>builder()
+          .put(ATTRIBUTE_TO_MAP_NAME, ImmutableList.<String>builder().add("second value").add("second value 2").build())
+          .build()
+        );
+    }
+
+    @Test
+    public void testDeleteBasicMappingMultipleValues() {
+        testValueMapping(ImmutableMap.<String, List<String>>builder()
+          .put(ATTRIBUTE_TO_MAP_NAME, ImmutableList.<String>builder().add("second value").add("second value 2").build())
+          .build(),
+          ImmutableMap.<String, List<String>>builder()
+          .build()
+        );
+    }
 }