KEYCLOAK-8141 Fix issue where attribute values are duplicated if updates to user are done in parallell

This commit is contained in:
stianst 2020-03-06 08:28:34 +01:00 committed by Hynek Mlnařík
parent 73aff6d60e
commit 49db2c13a5
2 changed files with 49 additions and 2 deletions

View file

@ -242,8 +242,11 @@ public class UserResource {
if (rep.getAttributes() != null) { if (rep.getAttributes() != null) {
for (Map.Entry<String, List<String>> attr : rep.getAttributes().entrySet()) { for (Map.Entry<String, List<String>> attr : rep.getAttributes().entrySet()) {
List<String> currentValue = user.getAttribute(attr.getKey());
if (currentValue == null || currentValue.size() != attr.getValue().size() || !currentValue.containsAll(attr.getValue())) {
user.setAttribute(attr.getKey(), attr.getValue()); user.setAttribute(attr.getKey(), attr.getValue());
} }
}
for (String attr : attrsToRemove) { for (String attr : attrsToRemove) {
user.removeAttribute(attr); user.removeAttribute(attr);

View file

@ -23,16 +23,27 @@ import org.keycloak.admin.client.resource.ClientResource;
import org.keycloak.admin.client.resource.ClientsResource; import org.keycloak.admin.client.resource.ClientsResource;
import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.admin.client.resource.RealmResource;
import org.keycloak.admin.client.resource.RolesResource; import org.keycloak.admin.client.resource.RolesResource;
import org.keycloak.admin.client.resource.UserResource;
import org.keycloak.admin.client.resource.UsersResource;
import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.representations.idm.GroupRepresentation; import org.keycloak.representations.idm.GroupRepresentation;
import org.keycloak.representations.idm.RoleRepresentation; import org.keycloak.representations.idm.RoleRepresentation;
import javax.ws.rs.NotFoundException; import javax.ws.rs.NotFoundException;
import javax.ws.rs.core.Response; import javax.ws.rs.core.Response;
import org.keycloak.testsuite.admin.ApiUtil;
import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.testsuite.admin.ApiUtil;
import org.keycloak.testsuite.util.UserBuilder;
import java.util.List;
import java.util.Map;
import java.util.Objects; import java.util.Objects;
import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicInteger;
import org.junit.Ignore;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
@ -51,6 +62,39 @@ public class ConcurrencyTest extends AbstractConcurrencyTest {
System.out.println("took " + end + " ms"); System.out.println("took " + end + " ms");
} }
// KEYCLOAK-8141 Verify that no attribute values are duplicated, and there are no locking exceptions when adding attributes in parallell
@Test
@Ignore
public void createUserAttributes() throws Throwable {
AtomicInteger c = new AtomicInteger();
UsersResource users = testRealm().users();
UserRepresentation u = UserBuilder.create().username("attributes").build();
Response response = users.create(u);
String userId = ApiUtil.getCreatedId(response);
response.close();
UserResource user = users.get(userId);
concurrentTest((threadIndex, keycloak, realm) -> {
UserRepresentation rep = user.toRepresentation();
rep.singleAttribute("a-" + c.getAndIncrement(), "value");
user.update(rep);
});
UserRepresentation rep = user.toRepresentation();
// Number of attributes should be equal to created attributes, or less (concurrent requests may drop attributes added by other threads)
assertTrue(rep.getAttributes().size() <= c.get());
// All attributes should have a single value
for (Map.Entry<String, List<String>> e : rep.getAttributes().entrySet()) {
assertEquals(1, e.getValue().size());
}
}
@Test @Test
public void testAllConcurrently() throws Throwable { public void testAllConcurrently() throws Throwable {
AtomicInteger uniqueCounter = new AtomicInteger(100000); AtomicInteger uniqueCounter = new AtomicInteger(100000);