libre.sh/keycloak-scim
2
0
Fork 0
Code Issues 14 Pull requests Releases Packages Activity Actions 6

Sort order of updates for user properties (#32853)

Browse source 
This should reduce deadlocks on the user property table if the users are updated concurrently.

Closes #32852

Signed-off-by: Alexander Schwartz <aschwart@redhat.com>
This commit is contained in:
Alexander Schwartz 2024-09-18 12:37:42 +02:00 committed by GitHub
parent 8ef7007e3c
commit 2a95d0abfa
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 18 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/LDAPMappersComparator.java
View file

@ -18,15 +18,10 @@
package org.keycloak.storage.ldap.mappers;
import org.keycloak.component.ComponentModel;
import org.keycloak.models.AuthenticationExecutionModel;
import org.keycloak.models.UserModel;
import org.keycloak.storage.ldap.LDAPConfig;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
/**
* TODO: Possibly add "priority" instead of hardcoding behaviour
@ -67,7 +62,7 @@ public class LDAPMappersComparator {
if (isO2AttrMapper) {
return 1;
} else {
return 0;
return compareWithStableOrdering(o1, o2);
}
} else if (!isO2AttrMapper) {
return -1;
@ -82,7 +77,7 @@ public class LDAPMappersComparator {
if (isO2UsernameMapper) {
return 1;
} else {
return 0;
return compareWithStableOrdering(o1, o2);
}
} else if (!isO2UsernameMapper) {
return -1;
@ -98,13 +93,21 @@ public class LDAPMappersComparator {
if (isO2LdapAttr) {
return 1;
} else {
return 0;
return compareWithStableOrdering(o1, o2);
}
} else if (!isO2LdapAttr) {
return -1;
}
return 0;
return compareWithStableOrdering(o1, o2);
}
/**
* Ensure a stable ordering, so the mappers are always executed in the same order.
* This can avoid database deadlocks as the mappers will modify attributes always in the same order.
*/
private static int compareWithStableOrdering(ComponentModel o1, ComponentModel o2) {
return o1.getId().compareTo(o2.getId());
}
}

2
model/jpa/src/main/java/org/keycloak/models/jpa/JpaIdentityProviderStorageProvider.java
View file

@ -459,7 +459,7 @@ public class JpaIdentityProviderStorageProvider implements IdentityProviderStora
builder.equal(mapper.get("realmId"), getRealm().getId()),
builder.equal(mapper.get("identityProviderAlias"), identityProviderAlias));
TypedQuery<IdentityProviderMapperEntity> typedQuery = em.createQuery(query.select(mapper).where(predicate));
TypedQuery<IdentityProviderMapperEntity> typedQuery = em.createQuery(query.select(mapper).where(predicate).orderBy(builder.asc(mapper.get("id"))));
return closing(typedQuery.getResultStream().map(this::toModel));
}

2
model/storage-private/src/main/java/org/keycloak/storage/datastore/DefaultExportImportManager.java
View file

@ -905,7 +905,7 @@ public class DefaultExportImportManager implements ExportImportManager {
user.setLastName(userRep.getLastName());
user.setFederationLink(userRep.getFederationLink());
if (userRep.getAttributes() != null) {
for (Map.Entry<String, List<String>> entry : userRep.getAttributes().entrySet()) {
for (Map.Entry<String, List<String>> entry : userRep.getAttributes().entrySet().stream().sorted(Map.Entry.comparingByKey()).toList()) {
List<String> value = entry.getValue();
if (value != null) {
user.setAttribute(entry.getKey(), new ArrayList<>(value));

2
server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java
View file

@ -115,7 +115,7 @@ public final class DefaultUserProfile implements UserProfile {
try {
Map<String, List<String>> writable = new HashMap<>(attributes.getWritable());
for (Map.Entry<String, List<String>> attribute : writable.entrySet()) {
for (Map.Entry<String, List<String>> attribute : writable.entrySet().stream().sorted(Map.Entry.comparingByKey()).toList()) {
String name = attribute.getKey();
List<String> currentValue = user.getAttributeStream(name)
.filter(Objects::nonNull).collect(Collectors.toList());

3
services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpCreateUserIfUniqueAuthenticator.java
View file

@ -35,7 +35,6 @@ import org.keycloak.services.messages.Messages;
import jakarta.ws.rs.core.Response;
import java.util.List;
import java.util.Map;
import java.util.UUID;
/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
@ -87,7 +86,7 @@ public class IdpCreateUserIfUniqueAuthenticator extends AbstractIdpAuthenticator
if (federatedUser != null) {
federatedUser.setEnabled(true);
for (Map.Entry<String, List<String>> attr : serializedCtx.getAttributes().entrySet()) {
for (Map.Entry<String, List<String>> attr : serializedCtx.getAttributes().entrySet().stream().sorted(Map.Entry.comparingByKey()).toList()) {
if (!UserModel.USERNAME.equalsIgnoreCase(attr.getKey())) {
federatedUser.setAttribute(attr.getKey(), attr.getValue());
}

3
services/src/main/java/org/keycloak/protocol/oidc/DefaultTokenExchangeProvider.java
View file

@ -24,7 +24,6 @@ import org.keycloak.broker.provider.BrokeredIdentityContext;
import org.keycloak.broker.provider.ExchangeExternalToken;
import org.keycloak.broker.provider.ExchangeTokenToIdentityProviderToken;
import org.keycloak.broker.provider.IdentityProvider;
import org.keycloak.broker.provider.IdentityProviderFactory;
import org.keycloak.broker.provider.IdentityProviderMapper;
import org.keycloak.broker.provider.IdentityProviderMapperSyncModeDelegate;
import org.keycloak.common.ClientConnection;
@ -677,7 +676,7 @@ public class DefaultTokenExchangeProvider implements TokenExchangeProvider {
}
// make sure user attributes are updated based on attributes set to the context
for (Map.Entry<String, List<String>> attr : context.getAttributes().entrySet()) {
for (Map.Entry<String, List<String>> attr : context.getAttributes().entrySet().stream().sorted(Map.Entry.comparingByKey()).toList()) {
if (!UserModel.USERNAME.equalsIgnoreCase(attr.getKey())) {
user.setAttribute(attr.getKey(), attr.getValue());
}

1
services/src/main/java/org/keycloak/services/resources/admin/UserResource.java
View file

@ -294,6 +294,7 @@ public class UserResource {
.getProviderFactoriesStream(RequiredActionProvider.class)
.map(ProviderFactory::getId)
.distinct()
.sorted()
.forEach(action -> {
if (reqActions.contains(action)) {
user.addRequiredAction(action);