libre.sh/keycloak-scim
4
0
Fork 0
Code Issues 15 Pull requests Releases 1 Activity Actions

KEYCLOAK-7014 - Correctly handle null-values in UserAttributes

Browse source
This commit is contained in:
vramik 2020-01-06 15:53:40 +01:00 committed by Hynek Mlnařík
parent 39fff1c538
commit a2b3747d0e
6 changed files with 1774 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
core/src/main/java/org/keycloak/json/StringListMapDeserializer.java
View file

@ -41,12 +41,13 @@ public class StringListMapDeserializer extends JsonDeserializer<Object> {
Map.Entry<String, JsonNode> e = itr.next();
List<String> values = new LinkedList<>();
if (!e.getValue().isArray()) {
values.add(e.getValue().asText());
values.add((e.getValue().isNull()) ? null : e.getValue().asText());
} else {
ArrayNode a = (ArrayNode) e.getValue();
Iterator<JsonNode> vitr = a.elements();
while (vitr.hasNext()) {
values.add(vitr.next().asText());
JsonNode node = vitr.next();
values.add((node.isNull() ? null : node.asText()));
}
}
map.put(e.getKey(), values);

107
core/src/test/java/org/keycloak/json/StringListMapDeserializerTest.java Normal file
View file

@ -0,0 +1,107 @@
/*
* Copyright 2020 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.databind.ObjectMapper;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import java.io.IOException;
import java.util.List;
import java.util.Map;
import static junit.framework.TestCase.assertTrue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
/**
* @author <a href="mailto:urs.honegger@starmind.com">Urs Honegger</a>
*/
public class StringListMapDeserializerTest {
private ObjectMapper mapper;
@Before
public void setUp() {
mapper = new ObjectMapper();
}
@After
public void tearDown() {
mapper = null;
}
@Test
public void nonNullValue() throws IOException {
Map<String, List<String>> attributes = deserialize("\"foo\": \"bar\"");
assertTrue(attributes.containsKey("foo"));
List<String> foo = attributes.get("foo");
assertEquals(1, foo.size());
assertEquals("bar", foo.get(0));
}
@Test
public void nonNullValueArray() throws IOException {
Map<String, List<String>> attributes = deserialize("\"foo\": [ \"bar\", \"baz\" ]");
assertTrue(attributes.containsKey("foo"));
List<String> foo = attributes.get("foo");
assertEquals(2, foo.size());
assertEquals("baz", foo.get(1));
}
@Test
public void nullValue() throws IOException {
// null values must deserialize to null
Map<String, List<String>> attributes = deserialize("\"foo\": null");
assertTrue(attributes.containsKey("foo"));
List<String> foo = attributes.get("foo");
assertEquals(1, foo.size());
assertNull(foo.get(0));
}
@Test
public void nullValueArray() throws IOException {
// null values must deserialize to null
Map<String, List<String>> attributes = deserialize("\"foo\": [ null, \"something\", null ]");
assertTrue(attributes.containsKey("foo"));
List<String> foo = attributes.get("foo");
assertEquals(3, foo.size());
assertEquals("something", foo.get(1));
assertNull(foo.get(2));
}
private Map<String, List<String>> deserialize(String attributeKeyValueString) throws IOException {
TestObject testObject = mapper.readValue("{ \"attributes\": {" + attributeKeyValueString + " } }", TestObject.class);
return testObject.getAttributes();
}
private static class TestObject {
@JsonDeserialize(using = StringListMapDeserializer.class)
private final Map<String, List<String>> attributes = null;
public Map<String, List<String>> getAttributes() {
return attributes;
}
}
}

55
model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java
View file

@ -51,6 +51,7 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.Objects;
import java.util.stream.Stream;
import javax.persistence.LockModeType;
/**
@ -71,6 +72,7 @@ public class UserAdapter implements UserModel, JpaModel<UserEntity> {
this.session = session;
}
@Override
public UserEntity getEntity() {
return user;
}
@ -113,32 +115,35 @@ public class UserAdapter implements UserModel, JpaModel<UserEntity> {
@Override
public void setSingleAttribute(String name, String value) {
String firstExistingAttrId = null;
List<UserAttributeEntity> toRemove = new ArrayList<>();
for (UserAttributeEntity attr : user.getAttributes()) {
if (attr.getName().equals(name)) {
if (firstExistingAttrId == null) {
attr.setValue(value);
firstExistingAttrId = attr.getId();
} else {
toRemove.add(attr);
if (value == null) {
user.getAttributes().removeIf(a -> a.getName().equals(name));
} else {
String firstExistingAttrId = null;
List<UserAttributeEntity> toRemove = new ArrayList<>();
for (UserAttributeEntity attr : user.getAttributes()) {
if (attr.getName().equals(name)) {
if (firstExistingAttrId == null) {
attr.setValue(value);
firstExistingAttrId = attr.getId();
} else {
toRemove.add(attr);
}
}
}
}
if (firstExistingAttrId != null) {
// Remove attributes through HQL to avoid StaleUpdateException
Query query = em.createNamedQuery("deleteUserAttributesByNameAndUserOtherThan");
query.setParameter("name", name);
query.setParameter("userId", user.getId());
query.setParameter("attrId", firstExistingAttrId);
int numUpdated = query.executeUpdate();
if (firstExistingAttrId != null) {
// Remove attributes through HQL to avoid StaleUpdateException
Query query = em.createNamedQuery("deleteUserAttributesByNameAndUserOtherThan");
query.setParameter("name", name);
query.setParameter("userId", user.getId());
query.setParameter("attrId", firstExistingAttrId);
int numUpdated = query.executeUpdate();
// Remove attribute from local entity
user.getAttributes().removeAll(toRemove);
} else {
persistAttributeValue(name, value);
// Remove attribute from local entity
user.getAttributes().removeAll(toRemove);
} else {
persistAttributeValue(name, value);
}
}
}
@ -146,10 +151,8 @@ public class UserAdapter implements UserModel, JpaModel<UserEntity> {
public void setAttribute(String name, List<String> values) {
// Remove all existing
removeAttribute(name);
// Put all new
for (String value : values) {
persistAttributeValue(name, value);
for (Iterator<String> it = values.stream().filter(Objects::nonNull).iterator(); it.hasNext();) {
persistAttributeValue(name, it.next());
}
}

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

@ -35,7 +35,6 @@ import org.keycloak.events.admin.OperationType;
import org.keycloak.events.admin.ResourceType;
import org.keycloak.models.Constants;
import org.keycloak.models.PasswordPolicy;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.credential.PasswordCredentialModel;
import org.keycloak.models.utils.ModelToRepresentation;
@ -85,7 +84,8 @@ import java.util.LinkedList;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicInteger;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
@ -95,6 +95,8 @@ import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.keycloak.testsuite.Assert.assertNames;
import static org.keycloak.testsuite.admin.AbstractAdminTest.loadJson;
import org.keycloak.testsuite.updaters.Creator;
import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude;
import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer;
@ -716,6 +718,19 @@ public class UserTest extends AbstractAdminTest {
assertNull(user1.getAttributes());
}
@Test
public void testImportUserWithNullAttribute() {
RealmRepresentation rep = loadJson(getClass().getResourceAsStream("/import/testrealm-user-null-attr.json"), RealmRepresentation.class);
try (Creator<RealmResource> c = Creator.create(adminClient, rep)) {
List<UserRepresentation> users = c.resource().users().list();
// there should be only one user
assertThat(users, hasSize(1));
// test there are only 2 attributes imported from json file, attribute "key3" : [ null ] shoudn't be imported
assertThat(users.get(0).getAttributes().size(), equalTo(2));
}
}
private void assertAttributeValue(String expectedValue, List<String> attrValues) {
assertEquals(1, attrValues.size());
assertEquals(expectedValue, attrValues.get(0));

1
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/model/UserModelTest.java
View file

@ -323,6 +323,7 @@ public class UserModelTest extends AbstractTestRealmKeycloakTest {
user.setSingleAttribute("key1", "value1");
user.setSingleAttribute("key2", "value2");
user.setSingleAttribute("key3", null); //KEYCLOAK-7014
// Overwrite the first attribute
user.setSingleAttribute("key1", "value3");

1617
testsuite/integration-arquillian/tests/base/src/test/resources/import/testrealm-user-null-attr.json Normal file
View file

File diff suppressed because it is too large Load diff