KEYCLOAK-3745 Change attributes in user rep

core/src/main/java/org/keycloak/json/StringListMapDeserializer.java

@@ -0,0 +1,57 @@
+/*
+ * Copyright 2016 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.keycloak.json;
+
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.databind.DeserializationContext;
+import com.fasterxml.jackson.databind.JsonDeserializer;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ArrayNode;
+
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+
+public class StringListMapDeserializer extends JsonDeserializer<Object> {
+
+    @Override
+    public Object deserialize(JsonParser jsonParser, DeserializationContext deserializationContext) throws IOException {
+        JsonNode jsonNode = jsonParser.readValueAsTree();
+        Iterator<Map.Entry<String, JsonNode>> itr = jsonNode.fields();
+        Map<String, List<String>> map = new HashMap<>();
+        while (itr.hasNext()) {
+            Map.Entry<String, JsonNode> e = itr.next();
+            List<String> values = new LinkedList<>();
+            if (!e.getValue().isArray()) {
+                values.add(e.getValue().asText());
+            } else {
+                ArrayNode a = (ArrayNode) e.getValue();
+                Iterator<JsonNode> vitr = a.elements();
+                while (vitr.hasNext()) {
+                    values.add(vitr.next().asText());
+                }
+            }
+            map.put(e.getKey(), values);
+        }
+        return map;
+    }
+
+}

core/src/main/java/org/keycloak/representations/idm/UserRepresentation.java

@@ -17,7 +17,8 @@
 
 package org.keycloak.representations.idm;
 
-import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import org.keycloak.json.StringListMapDeserializer;
 
 import java.util.Arrays;
 import java.util.HashMap;
@@ -43,8 +44,8 @@ public class UserRepresentation {
     protected String federationLink;
     protected String serviceAccountClientId; // For rep, it points to clientId (not DB ID)
 
-    // Currently there is Map<String, List<String>> but for backwards compatibility, we also need to support Map<String, String>
-    protected Map<String, Object> attributes;
+    @JsonDeserialize(using = StringListMapDeserializer.class)
+    protected Map<String, List<String>> attributes;
     protected List<CredentialRepresentation> credentials;
     protected List<String> requiredActions;
     protected List<FederatedIdentityRepresentation> federatedIdentities;
@@ -141,17 +142,11 @@ public void setEmailVerified(Boolean emailVerified) {
         this.emailVerified = emailVerified;
     }
 
-    public Map<String, Object> getAttributes() {
+    public Map<String, List<String>> getAttributes() {
         return attributes;
     }
 
-    // This method can be removed once we can remove backwards compatibility with Keycloak 1.3 (then getAttributes() can be changed to return Map<String, List<String>> )
-    @JsonIgnore
-    public Map<String, List<String>> getAttributesAsListValues() {
-        return (Map) attributes;
-    }
-
-    public void setAttributes(Map<String, Object> attributes) {
+    public void setAttributes(Map<String, List<String>> attributes) {
         this.attributes = attributes;
     }
 

server-spi/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java

@@ -197,7 +197,7 @@ public static UserRepresentation toRepresentation(KeycloakSession session, Realm
         rep.setRequiredActions(reqActions);
 
         if (user.getAttributes() != null && !user.getAttributes().isEmpty()) {
-            Map<String, Object> attrs = new HashMap<>();
+            Map<String, List<String>> attrs = new HashMap<>();
             attrs.putAll(user.getAttributes());
             rep.setAttributes(attrs);
         }

server-spi/src/main/java/org/keycloak/models/utils/RepresentationToModel.java

@@ -1342,16 +1342,10 @@ public static UserModel createUser(KeycloakSession session, RealmModel newRealm,
         user.setLastName(userRep.getLastName());
         user.setFederationLink(userRep.getFederationLink());
         if (userRep.getAttributes() != null) {
-            for (Map.Entry<String, Object> entry : userRep.getAttributes().entrySet()) {
-                Object value = entry.getValue();
-
-                if (value instanceof Collection) {
-                    Collection<String> colVal = (Collection<String>) value;
-                    user.setAttribute(entry.getKey(), new ArrayList<>(colVal));
-                } else if (value instanceof String) {
-                    // TODO: This is here just for backwards compatibility with KC 1.3 and earlier
-                    String stringVal = (String) value;
-                    user.setSingleAttribute(entry.getKey(), stringVal);
+            for (Map.Entry<String, List<String>> entry : userRep.getAttributes().entrySet()) {
+                List<String> value = entry.getValue();
+                if (value != null) {
+                    user.setAttribute(entry.getKey(), new ArrayList<>(value));
                 }
             }
         }
@@ -2226,20 +2220,11 @@ public static Scope toModel(ScopeRepresentation scope, ResourceServer resourceSe
     public static void importFederatedUser(KeycloakSession session, RealmModel newRealm, UserRepresentation userRep) {
         UserFederatedStorageProvider federatedStorage = session.userFederatedStorage();
         if (userRep.getAttributes() != null) {
-            for (Map.Entry<String, Object> entry : userRep.getAttributes().entrySet()) {
+            for (Map.Entry<String, List<String>> entry : userRep.getAttributes().entrySet()) {
                 String key = entry.getKey();
-                Object value = entry.getValue();
-                if (value == null) continue;
-
-                if (value instanceof Collection) {
-                    Collection<String> colVal = (Collection<String>) value;
-                    List<String> list = new LinkedList<>();
-                    list.addAll(colVal);
-                    federatedStorage.setAttribute(newRealm, userRep.getId(), key, list);
-                } else if (value instanceof String) {
-                    // TODO: This is here just for backwards compatibility with KC 1.3 and earlier
-                    String stringVal = (String) value;
-                    federatedStorage.setSingleAttribute(newRealm, userRep.getId(), key, stringVal);
+                List<String> value = entry.getValue();
+                if (value != null) {
+                    federatedStorage.setAttribute(newRealm, userRep.getId(), key, new LinkedList<>(value));
                 }
             }
         }

services/src/main/java/org/keycloak/exportimport/util/ExportUtils.java

@@ -635,7 +635,7 @@ public static UserRepresentation exportFederatedUser(KeycloakSession session, Re
         userRep.setId(id);
         MultivaluedHashMap<String, String> attributes = session.userFederatedStorage().getAttributes(realm, id);
         if (attributes.size() > 0) {
-            Map<String, Object> attrs = new HashMap<>();
+            Map<String, List<String>> attrs = new HashMap<>();
             attrs.putAll(attributes);
             userRep.setAttributes(attrs);
         }

services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java

@@ -262,8 +262,8 @@ public static void updateUserFromRep(UserModel user, UserRepresentation rep, Set
             }
         }
 
-        if (rep.getAttributesAsListValues() != null) {
-            for (Map.Entry<String, List<String>> attr : rep.getAttributesAsListValues().entrySet()) {
+        if (rep.getAttributes() != null) {
+            for (Map.Entry<String, List<String>> attr : rep.getAttributes().entrySet()) {
                 user.setAttribute(attr.getKey(), attr.getValue());
             }
 

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/ProfileTest.java

@@ -72,13 +72,8 @@ public void configureTestRealm(RealmRepresentation testRealm) {
         UserRepresentation user = RealmRepUtil.findUser(testRealm, "test-user@localhost");
         user.setFirstName("First");
         user.setLastName("Last");
-        Map<String, Object> attributes = user.getAttributes();
-        if (attributes == null) {
-            attributes = new HashMap<>();
-            user.setAttributes(attributes);
-        }
-        attributes.put("key1", "value1");
-        attributes.put("key2", "value2");
+        user.singleAttribute("key1", "value1");
+        user.singleAttribute("key2", "value2");
 
         UserRepresentation user2 = UserBuilder.create()
                                               .enabled(true)

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/custom/CustomAuthFlowOTPTest.java

@@ -159,11 +159,7 @@ public void conditionalOTPUserAttributeSkip() {
         setConditionalOTPForm(config);
 
         //add skip user attribute to user
-        Map<String, Object> userAttributes = new HashMap<>();
-        List<String> attributeValues = new ArrayList<>();
-        attributeValues.add("skip");
-        userAttributes.put("userSkipAttribute", attributeValues);
-        testUser.setAttributes(userAttributes);
+        testUser.singleAttribute("userSkipAttribute", "skip");
         testRealmResource().users().get(testUser.getId()).update(testUser);
 
         //test OTP is skipped
@@ -182,11 +178,7 @@ public void conditionalOTPUserAttributeForce() {
         setConditionalOTPForm(config);
 
         //add force user attribute to user
-        Map<String, Object> userAttributes = new HashMap<>();
-        List<String> attributeValues = new ArrayList<>();
-        attributeValues.add("force");
-        userAttributes.put("userSkipAttribute", attributeValues);
-        testUser.setAttributes(userAttributes);
+        testUser.singleAttribute("userSkipAttribute", "force");
         testRealmResource().users().get(testUser.getId()).update(testUser);
 
         //test OTP is required

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/TermsAndConditionsTest.java

@@ -94,7 +94,7 @@ public void termsAccepted() {
 
         // assert user attribute is properly set
         UserRepresentation user = ActionUtil.findUserWithAdminClient(adminClient, "test-user@localhost");
-        Map<String,List<String>> attributes = user.getAttributesAsListValues();
+        Map<String,List<String>> attributes = user.getAttributes();
         assertNotNull("timestamp for terms acceptance was not stored in user attributes", attributes);
         List<String> termsAndConditions = attributes.get(TermsAndConditions.USER_ATTRIBUTE);
         assertTrue("timestamp for terms acceptance was not stored in user attributes as "
@@ -128,7 +128,7 @@ public void termsDeclined() {
 
         // assert user attribute is properly removed
         UserRepresentation user = ActionUtil.findUserWithAdminClient(adminClient, "test-user@localhost");
-        Map<String,List<String>> attributes = user.getAttributesAsListValues();
+        Map<String,List<String>> attributes = user.getAttributes();
         if (attributes != null) {
             assertNull("expected null for terms acceptance user attribute " + TermsAndConditions.USER_ATTRIBUTE,
                     attributes.get(TermsAndConditions.USER_ATTRIBUTE));

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java

@@ -417,19 +417,19 @@ public void attributes() {
         List<String> vals = new ArrayList<>();
         vals.add("value2user2");
         vals.add("value2user2_2");
-        user2.getAttributesAsListValues().put("attr2", vals);
+        user2.getAttributes().put("attr2", vals);
 
         String user2Id = createUser(user2);
 
         user1 = realm.users().get(user1Id).toRepresentation();
-        assertEquals(2, user1.getAttributesAsListValues().size());
-        assertAttributeValue("value1user1", user1.getAttributesAsListValues().get("attr1"));
-        assertAttributeValue("value2user1", user1.getAttributesAsListValues().get("attr2"));
+        assertEquals(2, user1.getAttributes().size());
+        assertAttributeValue("value1user1", user1.getAttributes().get("attr1"));
+        assertAttributeValue("value2user1", user1.getAttributes().get("attr2"));
 
         user2 = realm.users().get(user2Id).toRepresentation();
-        assertEquals(2, user2.getAttributesAsListValues().size());
-        assertAttributeValue("value1user2", user2.getAttributesAsListValues().get("attr1"));
-        vals = user2.getAttributesAsListValues().get("attr2");
+        assertEquals(2, user2.getAttributes().size());
+        assertAttributeValue("value1user2", user2.getAttributes().get("attr1"));
+        vals = user2.getAttributes().get("attr2");
         assertEquals(2, vals.size());
         assertTrue(vals.contains("value2user2") && vals.contains("value2user2_2"));
 
@@ -439,18 +439,18 @@ public void attributes() {
         updateUser(realm.users().get(user1Id), user1);
 
         user1 = realm.users().get(user1Id).toRepresentation();
-        assertEquals(3, user1.getAttributesAsListValues().size());
-        assertAttributeValue("value3user1", user1.getAttributesAsListValues().get("attr1"));
-        assertAttributeValue("value2user1", user1.getAttributesAsListValues().get("attr2"));
-        assertAttributeValue("value4user1", user1.getAttributesAsListValues().get("attr3"));
+        assertEquals(3, user1.getAttributes().size());
+        assertAttributeValue("value3user1", user1.getAttributes().get("attr1"));
+        assertAttributeValue("value2user1", user1.getAttributes().get("attr2"));
+        assertAttributeValue("value4user1", user1.getAttributes().get("attr3"));
 
         user1.getAttributes().remove("attr1");
         updateUser(realm.users().get(user1Id), user1);
 
         user1 = realm.users().get(user1Id).toRepresentation();
-        assertEquals(2, user1.getAttributesAsListValues().size());
-        assertAttributeValue("value2user1", user1.getAttributesAsListValues().get("attr2"));
-        assertAttributeValue("value4user1", user1.getAttributesAsListValues().get("attr3"));
+        assertEquals(2, user1.getAttributes().size());
+        assertAttributeValue("value2user1", user1.getAttributes().get("attr2"));
+        assertAttributeValue("value4user1", user1.getAttributes().get("attr3"));
 
         user1.getAttributes().clear();
         updateUser(realm.users().get(user1Id), user1);

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/exportimport/ExportImportUtil.java

@@ -164,13 +164,13 @@ public static void assertDataImportedInRealm(Keycloak adminClient, KeycloakTesti
         Assert.assertEquals("app-admin", appRoles.iterator().next().getName());
 
         // Test attributes
-        Map<String, List<String>> attrs = wburke.getAttributesAsListValues();
+        Map<String, List<String>> attrs = wburke.getAttributes();
         Assert.assertEquals(1, attrs.size());
         List<String> attrVals = attrs.get("email");
         Assert.assertEquals(1, attrVals.size());
         Assert.assertEquals("bburke@redhat.com", attrVals.get(0));
 
-        attrs = admin.getAttributesAsListValues();
+        attrs = admin.getAttributes();
         Assert.assertEquals(2, attrs.size());
         attrVals = attrs.get("key1");
         Assert.assertEquals(1, attrVals.size());

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/i18n/EmailTest.java

@@ -52,8 +52,7 @@ public class EmailTest extends AbstractI18NTest {
 
     private void changeUserLocale(String locale) {
         UserRepresentation user = findUser("login-test");
-        if (user.getAttributes() == null) user.setAttributes(new HashMap<String, Object>());
-        user.getAttributes().put(UserModel.LOCALE, Collections.singletonList(locale));
+        user.singleAttribute(UserModel.LOCALE, locale);
         ApiUtil.findUserByUsernameId(testRealm(), "login-test").update(user);
     }
 

testsuite/integration/src/test/java/org/keycloak/testsuite/authorization/ScopeManagementTest.java

@@ -18,16 +18,22 @@
 
 package org.keycloak.testsuite.authorization;
 
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
 import org.junit.Test;
 import org.keycloak.authorization.model.Scope;
+import org.keycloak.representations.idm.UserRepresentation;
 import org.keycloak.representations.idm.authorization.ScopeRepresentation;
+import org.keycloak.util.JsonSerialization;
 
 import javax.ws.rs.client.Entity;
 import javax.ws.rs.client.Invocation.Builder;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.Response.Status;
 
+import java.io.IOException;
+import java.util.LinkedList;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;