KEYCLOAK-11511 Improve exception handling of REST user creation

This commit is contained in:
k-tamura 2020-01-14 18:23:13 +09:00 committed by Hynek Mlnařík
parent b6a3fba6e3
commit 221aad9877
2 changed files with 60 additions and 3 deletions

View file

@ -21,6 +21,7 @@ import org.jboss.resteasy.annotations.cache.NoCache;
import javax.ws.rs.NotFoundException; import javax.ws.rs.NotFoundException;
import org.jboss.resteasy.spi.ResteasyProviderFactory; import org.jboss.resteasy.spi.ResteasyProviderFactory;
import org.keycloak.common.ClientConnection; import org.keycloak.common.ClientConnection;
import org.keycloak.common.util.ObjectUtil;
import org.keycloak.events.admin.OperationType; import org.keycloak.events.admin.OperationType;
import org.keycloak.events.admin.ResourceType; import org.keycloak.events.admin.ResourceType;
import org.keycloak.models.Constants; import org.keycloak.models.Constants;
@ -31,6 +32,7 @@ import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel; import org.keycloak.models.UserModel;
import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.models.utils.ModelToRepresentation;
import org.keycloak.models.utils.RepresentationToModel; import org.keycloak.models.utils.RepresentationToModel;
import org.keycloak.policy.PasswordPolicyNotMetException;
import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.services.ErrorResponse; import org.keycloak.services.ErrorResponse;
import org.keycloak.services.ForbiddenException; import org.keycloak.services.ForbiddenException;
@ -105,8 +107,13 @@ public class UsersResource {
public Response createUser(final UserRepresentation rep) { public Response createUser(final UserRepresentation rep) {
auth.users().requireManage(); auth.users().requireManage();
String username = rep.getUsername();
if (ObjectUtil.isBlank(username)) {
return ErrorResponse.error("User name is missing", Response.Status.BAD_REQUEST);
}
// Double-check duplicated username and email here due to federation // Double-check duplicated username and email here due to federation
if (session.users().getUserByUsername(rep.getUsername(), realm) != null) { if (session.users().getUserByUsername(username, realm) != null) {
return ErrorResponse.exists("User exists with same username"); return ErrorResponse.exists("User exists with same username");
} }
if (rep.getEmail() != null && !realm.isDuplicateEmailsAllowed() && session.users().getUserByEmail(rep.getEmail(), realm) != null) { if (rep.getEmail() != null && !realm.isDuplicateEmailsAllowed() && session.users().getUserByEmail(rep.getEmail(), realm) != null) {
@ -114,7 +121,7 @@ public class UsersResource {
} }
try { try {
UserModel user = session.users().addUser(realm, rep.getUsername()); UserModel user = session.users().addUser(realm, username);
Set<String> emptySet = Collections.emptySet(); Set<String> emptySet = Collections.emptySet();
UserResource.updateUserFromRep(user, rep, emptySet, realm, session, false); UserResource.updateUserFromRep(user, rep, emptySet, realm, session, false);
@ -131,12 +138,17 @@ public class UsersResource {
session.getTransactionManager().setRollbackOnly(); session.getTransactionManager().setRollbackOnly();
} }
return ErrorResponse.exists("User exists with same username or email"); return ErrorResponse.exists("User exists with same username or email");
} catch (PasswordPolicyNotMetException e) {
if (session.getTransactionManager().isActive()) {
session.getTransactionManager().setRollbackOnly();
}
return ErrorResponse.error("Password policy not met", Response.Status.BAD_REQUEST);
} catch (ModelException me){ } catch (ModelException me){
if (session.getTransactionManager().isActive()) { if (session.getTransactionManager().isActive()) {
session.getTransactionManager().setRollbackOnly(); session.getTransactionManager().setRollbackOnly();
} }
logger.warn("Could not create user", me); logger.warn("Could not create user", me);
return ErrorResponse.exists("Could not create user"); return ErrorResponse.error("Could not create user", Response.Status.BAD_REQUEST);
} }
} }
/** /**

View file

@ -438,6 +438,51 @@ public class UserTest extends AbstractAdminTest {
assertEquals(user.getFederationLink(), createdUser.getFederationLink()); assertEquals(user.getFederationLink(), createdUser.getFederationLink());
} }
@Test
public void createUserWithoutUsername() {
UserRepresentation user = new UserRepresentation();
user.setEmail("user1@localhost");
Response response = realm.users().create(user);
assertEquals(400, response.getStatus());
ErrorRepresentation error = response.readEntity(ErrorRepresentation.class);
Assert.assertEquals("User name is missing", error.getErrorMessage());
response.close();
}
@Test
public void createUserWithEmptyUsername() {
UserRepresentation user = new UserRepresentation();
user.setUsername("");
user.setEmail("user2@localhost");
Response response = realm.users().create(user);
assertEquals(400, response.getStatus());
ErrorRepresentation error = response.readEntity(ErrorRepresentation.class);
Assert.assertEquals("User name is missing", error.getErrorMessage());
response.close();
}
@Test
public void createUserWithInvalidPolicyPassword() {
RealmRepresentation rep = realm.toRepresentation();
String passwordPolicy = rep.getPasswordPolicy();
rep.setPasswordPolicy("length(8)");
realm.update(rep);
UserRepresentation user = new UserRepresentation();
user.setUsername("user4");
user.setEmail("user4@localhost");
CredentialRepresentation rawPassword = new CredentialRepresentation();
rawPassword.setValue("ABCD");
rawPassword.setType(CredentialRepresentation.PASSWORD);
user.setCredentials(Arrays.asList(rawPassword));
Response response = realm.users().create(user);
assertEquals(400, response.getStatus());
ErrorRepresentation error = response.readEntity(ErrorRepresentation.class);
Assert.assertEquals("Password policy not met", error.getErrorMessage());
rep.setPasswordPolicy(passwordPolicy);
realm.update(rep);
response.close();
}
private List<String> createUsers() { private List<String> createUsers() {
List<String> ids = new ArrayList<>(); List<String> ids = new ArrayList<>();