diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/LDAPMappersComparator.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/LDAPMappersComparator.java index dc9e3a66ed..fc9d1cbfbe 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/LDAPMappersComparator.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/LDAPMappersComparator.java @@ -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()); } } diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaIdentityProviderStorageProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaIdentityProviderStorageProvider.java index 7548a60768..2d6aef5366 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaIdentityProviderStorageProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaIdentityProviderStorageProvider.java @@ -459,7 +459,7 @@ public class JpaIdentityProviderStorageProvider implements IdentityProviderStora builder.equal(mapper.get("realmId"), getRealm().getId()), builder.equal(mapper.get("identityProviderAlias"), identityProviderAlias)); - TypedQuery typedQuery = em.createQuery(query.select(mapper).where(predicate)); + TypedQuery typedQuery = em.createQuery(query.select(mapper).where(predicate).orderBy(builder.asc(mapper.get("id")))); return closing(typedQuery.getResultStream().map(this::toModel)); } diff --git a/model/storage-private/src/main/java/org/keycloak/storage/datastore/DefaultExportImportManager.java b/model/storage-private/src/main/java/org/keycloak/storage/datastore/DefaultExportImportManager.java index 87a35ee1b3..3ce750759c 100644 --- a/model/storage-private/src/main/java/org/keycloak/storage/datastore/DefaultExportImportManager.java +++ b/model/storage-private/src/main/java/org/keycloak/storage/datastore/DefaultExportImportManager.java @@ -905,7 +905,7 @@ public class DefaultExportImportManager implements ExportImportManager { user.setLastName(userRep.getLastName()); user.setFederationLink(userRep.getFederationLink()); if (userRep.getAttributes() != null) { - for (Map.Entry> entry : userRep.getAttributes().entrySet()) { + for (Map.Entry> entry : userRep.getAttributes().entrySet().stream().sorted(Map.Entry.comparingByKey()).toList()) { List value = entry.getValue(); if (value != null) { user.setAttribute(entry.getKey(), new ArrayList<>(value)); diff --git a/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java b/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java index 420f817a10..329397ee0e 100644 --- a/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java +++ b/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java @@ -115,7 +115,7 @@ public final class DefaultUserProfile implements UserProfile { try { Map> writable = new HashMap<>(attributes.getWritable()); - for (Map.Entry> attribute : writable.entrySet()) { + for (Map.Entry> attribute : writable.entrySet().stream().sorted(Map.Entry.comparingByKey()).toList()) { String name = attribute.getKey(); List currentValue = user.getAttributeStream(name) .filter(Objects::nonNull).collect(Collectors.toList()); diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpCreateUserIfUniqueAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpCreateUserIfUniqueAuthenticator.java index a81beb4f99..34f6c6750c 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpCreateUserIfUniqueAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpCreateUserIfUniqueAuthenticator.java @@ -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 Marek Posolda @@ -87,7 +86,7 @@ public class IdpCreateUserIfUniqueAuthenticator extends AbstractIdpAuthenticator if (federatedUser != null) { federatedUser.setEnabled(true); - for (Map.Entry> attr : serializedCtx.getAttributes().entrySet()) { + for (Map.Entry> attr : serializedCtx.getAttributes().entrySet().stream().sorted(Map.Entry.comparingByKey()).toList()) { if (!UserModel.USERNAME.equalsIgnoreCase(attr.getKey())) { federatedUser.setAttribute(attr.getKey(), attr.getValue()); } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/DefaultTokenExchangeProvider.java b/services/src/main/java/org/keycloak/protocol/oidc/DefaultTokenExchangeProvider.java index e418d572fa..aab7dbd2d5 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/DefaultTokenExchangeProvider.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/DefaultTokenExchangeProvider.java @@ -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> attr : context.getAttributes().entrySet()) { + for (Map.Entry> attr : context.getAttributes().entrySet().stream().sorted(Map.Entry.comparingByKey()).toList()) { if (!UserModel.USERNAME.equalsIgnoreCase(attr.getKey())) { user.setAttribute(attr.getKey(), attr.getValue()); } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java index 8cc6f0f0a2..16921dac7f 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java @@ -294,6 +294,7 @@ public class UserResource { .getProviderFactoriesStream(RequiredActionProvider.class) .map(ProviderFactory::getId) .distinct() + .sorted() .forEach(action -> { if (reqActions.contains(action)) { user.addRequiredAction(action);