Merge pull request #3848 from stianst/KEYCLOAK-4378-2.5.x

KEYCLOAK-4378 New user attribute is not added after first login from …

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
         }
     }
 

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()
+        );
+    }
 }