diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java index 0d9685894356..11cd3618bb4d 100755 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java @@ -55,6 +55,7 @@ import org.keycloak.models.LDAPConstants; import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.ModelException; +import org.keycloak.models.ModelIllegalStateException; import org.keycloak.models.RealmModel; import org.keycloak.models.RequiredActionProviderModel; import org.keycloak.models.RoleModel; @@ -230,7 +231,7 @@ protected UserModel proxy(RealmModel realm, UserModel local, LDAPObject ldapObje // Any attempt to write data, which are not supported by the LDAP schema, should fail // This check is skipped when register new user as there are many "generic" attributes always written (EG. enabled, emailVerified) and those are usually unsupported by LDAP schema if (!model.isImportEnabled() && !newUser) { - UserModel readOnlyDelegate = new ReadOnlyUserModelDelegate(local, ModelException::new); + UserModel readOnlyDelegate = new ReadOnlyUserModelDelegate(local, ModelIllegalStateException::new); proxied = new LDAPWritesOnlyUserModelDelegate(readOnlyDelegate, this); } break; diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java index 09a93757abfa..329502066a64 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java @@ -340,12 +340,7 @@ protected UserModel cacheUser(RealmModel realm, UserModel delegate, Long revisio int notBefore = getDelegate().getNotBeforeOfUser(realm, delegate); if (isReadOnlyOrganizationMember(delegate)) { - return new ReadOnlyUserModelDelegate(delegate) { - @Override - public boolean isEnabled() { - return false; - } - }; + return new ReadOnlyUserModelDelegate(delegate, false); } CachedUser cached; @@ -355,12 +350,7 @@ public boolean isEnabled() { ComponentModel component = realm.getComponent(delegate.getFederationLink()); UserStorageProviderModel model = new UserStorageProviderModel(component); if (!model.isEnabled()) { - return new ReadOnlyUserModelDelegate(delegate) { - @Override - public boolean isEnabled() { - return false; - } - }; + return new ReadOnlyUserModelDelegate(delegate, false); } UserStorageProviderModel.CachePolicy policy = model.getCachePolicy(); if (policy != null && policy == UserStorageProviderModel.CachePolicy.NO_CACHE) { diff --git a/model/storage-private/src/main/java/org/keycloak/storage/UserStorageManager.java b/model/storage-private/src/main/java/org/keycloak/storage/UserStorageManager.java index 6cfacbbfa742..22910f017202 100755 --- a/model/storage-private/src/main/java/org/keycloak/storage/UserStorageManager.java +++ b/model/storage-private/src/main/java/org/keycloak/storage/UserStorageManager.java @@ -115,12 +115,7 @@ private UserFederatedStorageProvider getFederatedStorage() { protected UserModel importValidation(RealmModel realm, UserModel user) { if (isReadOnlyOrganizationMember(user)) { - return new ReadOnlyUserModelDelegate(user) { - @Override - public boolean isEnabled() { - return false; - } - }; + return new ReadOnlyUserModelDelegate(user, false); } if (user == null || user.getFederationLink() == null) return user; @@ -134,12 +129,7 @@ public boolean isEnabled() { } if (!model.isEnabled()) { - return new ReadOnlyUserModelDelegate(user) { - @Override - public boolean isEnabled() { - return false; - } - }; + return new ReadOnlyUserModelDelegate(user, false); } ImportedUserValidation importedUserValidation = getStorageProviderInstance(model, ImportedUserValidation.class, true); diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/ReadOnlyUserModelDelegate.java b/server-spi-private/src/main/java/org/keycloak/models/utils/ReadOnlyUserModelDelegate.java index 8ad38ee3f45b..df1b6c86a215 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/ReadOnlyUserModelDelegate.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/ReadOnlyUserModelDelegate.java @@ -31,11 +31,17 @@ public class ReadOnlyUserModelDelegate extends UserModelDelegate { private final Function exceptionCreator; + private Boolean enabled; public ReadOnlyUserModelDelegate(UserModel delegate) { this(delegate, ReadOnlyException::new); } + public ReadOnlyUserModelDelegate(UserModel delegate, boolean enabled) { + this(delegate, ReadOnlyException::new); + this.enabled = enabled; + } + public ReadOnlyUserModelDelegate(UserModel delegate, Function exceptionCreator) { super(delegate); this.exceptionCreator = exceptionCreator; @@ -51,6 +57,14 @@ public void setEnabled(boolean enabled) { throw readOnlyException("enabled"); } + @Override + public boolean isEnabled() { + if (enabled == null) { + return super.isEnabled(); + } + return enabled; + } + @Override public void setSingleAttribute(String name, String value) { throw readOnlyException("attribute(" + name + ")"); @@ -142,7 +156,7 @@ public void grantRole(RoleModel role) { } private RuntimeException readOnlyException(String detail) { - String message = String.format("Not possible to write '%s' when updating user '%s'", detail, getUsername()); + String message = String.format("The user is read-only. Not possible to write '%s' when updating user '%s'.", detail, getUsername()); return exceptionCreator.apply(message); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/noimport/LDAPProvidersIntegrationNoImportTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/noimport/LDAPProvidersIntegrationNoImportTest.java index 874c5583532b..ba7e77ddf1ec 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/noimport/LDAPProvidersIntegrationNoImportTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/noimport/LDAPProvidersIntegrationNoImportTest.java @@ -17,11 +17,14 @@ package org.keycloak.testsuite.federation.ldap.noimport; +import static org.junit.Assert.fail; + import java.util.Collections; import java.util.List; import java.util.stream.Collectors; import jakarta.ws.rs.BadRequestException; +import jakarta.ws.rs.InternalServerErrorException; import jakarta.ws.rs.core.Response; import java.util.Map; @@ -38,9 +41,11 @@ import org.keycloak.models.LDAPConstants; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; +import org.keycloak.models.UserModel.RequiredAction; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.representations.idm.ComponentRepresentation; +import org.keycloak.representations.idm.ErrorRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.storage.StorageId; import org.keycloak.storage.UserStorageProvider; @@ -361,6 +366,20 @@ public void testImpossibleToChangeNonLDAPMappedAttributes() { } } + @Test + public void testCannotUpdateReadOnlyUserImportDisabled() { + UserResource user = ApiUtil.findUserByUsernameId(testRealm(), "johnkeycloak"); + + try { + UserRepresentation rep = user.toRepresentation(); + rep.setRequiredActions(List.of(RequiredAction.VERIFY_EMAIL.name())); + user.update(rep); + fail("Should fail as the user is read-only"); + } catch (InternalServerErrorException isee) { + Assert.assertEquals("The user is read-only. Not possible to write 'required action VERIFY_EMAIL' when updating user 'johnkeycloak'.", isee.getResponse().readEntity(ErrorRepresentation.class).getErrorMessage()); + } + } + // No need to test this in no-import mode. There won't be any users in localStorage. @Test @Ignore