Merge pull request #3367 from stianst/KEYCLOAK-3745

KEYCLOAK-3745 Change attributes in user rep
This commit is contained in:
Stian Thorgersen 2016-10-19 14:01:39 +02:00 committed by GitHub
commit bbc1d26b72
13 changed files with 104 additions and 75 deletions

View file

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

View file

@ -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 class UserRepresentation {
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;
}

View file

@ -197,7 +197,7 @@ public class ModelToRepresentation {
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);
}

View file

@ -1342,16 +1342,10 @@ public class RepresentationToModel {
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 class RepresentationToModel {
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));
}
}
}

View file

@ -635,7 +635,7 @@ public class ExportUtils {
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);
}

View file

@ -262,8 +262,8 @@ public class UsersResource {
}
}
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());
}

View file

@ -72,13 +72,8 @@ public class ProfileTest extends TestRealmKeycloakTest {
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)

View file

@ -159,11 +159,7 @@ public class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
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 class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
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

View file

@ -94,7 +94,7 @@ public class TermsAndConditionsTest extends TestRealmKeycloakTest {
// 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 class TermsAndConditionsTest extends TestRealmKeycloakTest {
// 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));

View file

@ -417,19 +417,19 @@ public class UserTest extends AbstractAdminTest {
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 class UserTest extends AbstractAdminTest {
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);

View file

@ -164,13 +164,13 @@ public class ExportImportUtil {
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());

View file

@ -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);
}

View file

@ -18,9 +18,12 @@
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;
@ -28,6 +31,9 @@ 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;