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 0d968589435..c6b7087c0bf 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 @@ -230,7 +230,7 @@ public class LDAPStorageProvider implements UserStorageProvider, // 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, ReadOnlyException::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 09a93757abf..329502066a6 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 @@ public class UserCacheSession implements UserCache, OnCreateComponent, OnUpdateC 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 class UserCacheSession implements UserCache, OnCreateComponent, OnUpdateC 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 6cfacbbfa74..22910f01720 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 @@ public class UserStorageManager extends AbstractStorageManager 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 class ReadOnlyUserModelDelegate extends UserModelDelegate { 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 class ReadOnlyUserModelDelegate extends UserModelDelegate { } 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/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java index 658c79e7a3b..85918ee56f7 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 @@ -233,7 +233,7 @@ public class UserResource { throw ErrorResponse.exists("User exists with same username or email"); } catch (ReadOnlyException re) { session.getTransactionManager().setRollbackOnly(); - throw ErrorResponse.error("User is read only!", Status.BAD_REQUEST); + throw ErrorResponse.error(re.getMessage() == null ? "User is read only!" : re.getMessage(), Status.BAD_REQUEST); } catch (PasswordPolicyNotMetException e) { logger.warn("Password policy not met for user " + e.getUsername(), e); session.getTransactionManager().setRollbackOnly(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperTest.java index c41cea3d7cf..0c628f4a228 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperTest.java @@ -30,6 +30,7 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.representations.idm.ComponentRepresentation; +import org.keycloak.storage.ReadOnlyException; import org.keycloak.storage.UserStoragePrivateUtil; import org.keycloak.storage.ldap.LDAPConfig; import org.keycloak.storage.ldap.LDAPStorageProvider; @@ -275,7 +276,7 @@ public class LDAPGroupMapperTest extends AbstractLDAPTest { try { mary.joinGroup(group12); Assert.fail("Not expected to successfully add group12 in no-import mode and READ_ONLY mode of the group mapper"); - } catch (ModelException me) { + } catch (ReadOnlyException me) { // Ignore } }); 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 874c5583532..a2db35583ea 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,6 +17,8 @@ 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; @@ -38,9 +40,11 @@ import org.keycloak.component.ComponentModel; 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; @@ -350,6 +354,7 @@ public class LDAPProvidersIntegrationNoImportTest extends LDAPProvidersIntegrati Assert.assertEquals("654321", johnRep.getAttributes().get("postal_code").get(0)); } finally { // Revert + johnRep = john.toRepresentation(); johnRep.setFirstName(firstNameOrig); johnRep.setLastName(lastNameOrig); johnRep.singleAttribute("postal_code", postalCodeOrig); @@ -361,6 +366,20 @@ public class LDAPProvidersIntegrationNoImportTest extends LDAPProvidersIntegrati } } + @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 (BadRequestException bde) { + Assert.assertEquals("The user is read-only. Not possible to write 'required action VERIFY_EMAIL' when updating user 'johnkeycloak'.", bde.getResponse().readEntity(ErrorRepresentation.class).getErrorMessage()); + } + } + // No need to test this in no-import mode. There won't be any users in localStorage. @Test @Ignore