mirror/keycloak
1
0
Fork 0
mirror of https://github.com/keycloak/keycloak.git synced 2025-12-30 03:19:54 -06:00
Code Issues Packages Projects Releases Wiki Activity

Better message when updating users when import is disabled

Browse Source 
Closes #31456

Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
This commit is contained in:
Pedro Igor
2024-11-05 17:16:50 -03:00
committed by Alexander Schwartz
parent b70303f293
commit d3c5082244
7 changed files with 42 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java
View File

@@ -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;

14
model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCacheSession.java vendored
View File

@@ -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) {

14
model/storage-private/src/main/java/org/keycloak/storage/UserStorageManager.java
View File

@@ -115,12 +115,7 @@ public class UserStorageManager extends AbstractStorageManager<UserStorageProvid
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 class UserStorageManager extends AbstractStorageManager<UserStorageProvid
}
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);

16
server-spi-private/src/main/java/org/keycloak/models/utils/ReadOnlyUserModelDelegate.java
View File

@@ -31,11 +31,17 @@ import java.util.function.Function;
public class ReadOnlyUserModelDelegate extends UserModelDelegate {
private final Function<String, RuntimeException> 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<String, RuntimeException> 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);
}
}

2
services/src/main/java/org/keycloak/services/resources/admin/UserResource.java
View File

@@ -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();

3
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperTest.java
View File

@@ -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
}
});

19
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/noimport/LDAPProvidersIntegrationNoImportTest.java
View File

@@ -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