Ensure user loginName and email unique within realm

This commit is contained in:
Stian Thorgersen 2014-04-29 17:15:28 +01:00
parent 1d94649b96
commit 646e762cbe
18 changed files with 174 additions and 63 deletions

View file

@ -26,12 +26,19 @@
<form class="form-horizontal" name="userForm" novalidate kc-read-only="!access.manageUsers">
<span class="fieldset-notice"><span class="required">*</span> Required fields</span>
<fieldset class="border-top">
<div class="form-group">
<label class="col-sm-2 control-label"for="id">ID</label>
<div class="col-sm-4">
<input class="form-control" type="text" id="id" name="id" data-ng-model="user.id" autofocus data-ng-readonly="true">
</div>
</div>
<div class="form-group">
<label class="col-sm-2 control-label"for="username">Username <span class="required" data-ng-show="create">*</span></label>
<div class="col-sm-4">
<!-- Characters >,<,/,\ are forbidden in username -->
<input class="form-control" type="text" id="username" name="username" data-ng-model="user.username" autofocus
required ng-pattern="/^[^\<\>\\\/]*$/" data-ng-readonly="!create">
required ng-pattern="/^[^\<\>\\\/]*$/">
</div>
</div>

View file

@ -17,6 +17,8 @@ public interface UserModel {
String getLoginName();
void setLoginName(String loginName);
boolean isEnabled();
boolean isTotp();

View file

@ -27,7 +27,7 @@ public class JpaKeycloakTransaction implements KeycloakTransaction {
try {
em.getTransaction().commit();
} catch (PersistenceException e) {
throw PersistenceExceptionConverter.convert(e.getCause());
throw PersistenceExceptionConverter.convert(e.getCause() != null ? e.getCause() : e);
}
}

View file

@ -454,9 +454,6 @@ public class RealmAdapter implements RealmModel {
@Override
public UserModel addUser(String username) {
if (getUser(username) != null) {
throw new RuntimeException("Username already exists: " + username);
}
UserEntity entity = new UserEntity();
entity.setLoginName(username);
entity.setRealm(realm);

View file

@ -34,6 +34,11 @@ public class UserAdapter implements UserModel {
return user.getLoginName();
}
@Override
public void setLoginName(String loginName) {
user.setLoginName(loginName);
}
@Override
public boolean isEnabled() {
return user.isEnabled();

View file

@ -10,12 +10,15 @@ import javax.persistence.ElementCollection;
import javax.persistence.Entity;
import javax.persistence.GeneratedValue;
import javax.persistence.Id;
import javax.persistence.JoinColumn;
import javax.persistence.ManyToOne;
import javax.persistence.MapKeyColumn;
import javax.persistence.NamedQueries;
import javax.persistence.NamedQuery;
import javax.persistence.OneToMany;
import javax.persistence.OneToOne;
import javax.persistence.Table;
import javax.persistence.UniqueConstraint;
import java.util.ArrayList;
import java.util.Collection;
@ -35,6 +38,10 @@ import java.util.Set;
@NamedQuery(name="getRealmUserByFirstLastName", query="select u from UserEntity u where u.firstName = :first and u.lastName = :last and u.realm = :realm")
})
@Entity
@Table(uniqueConstraints = {
@UniqueConstraint(columnNames = { "realm", "loginName" }),
@UniqueConstraint(columnNames = { "realm", "email" })
})
public class UserEntity {
@Id
@GenericGenerator(name="uuid_generator", strategy="org.keycloak.models.jpa.utils.JpaIdGenerator")
@ -52,6 +59,7 @@ public class UserEntity {
@ManyToOne
@JoinColumn(name = "realm")
protected RealmEntity realm;
@ElementCollection

View file

@ -23,4 +23,6 @@ public @interface MongoIndex {
boolean unique() default false;
boolean sparse() default false;
}

View file

@ -139,7 +139,7 @@ public class MongoStoreImpl implements MongoStore {
protected void createIndex(DBCollection dbCollection, MongoIndex index) {
BasicDBObject fields = new BasicDBObject();
for (String f : index.fields()) {
for (String f : index.fields()) {
fields.put(f, 1);
}
String name = index.name();
@ -147,10 +147,22 @@ public class MongoStoreImpl implements MongoStore {
name = null;
}
boolean unique = index.unique();
boolean sparse = index.sparse();
dbCollection.ensureIndex(fields, name, unique);
BasicDBObject options = new BasicDBObject();
if (name != null) {
options.put("name", name);
}
if (unique) {
options.put("unique", unique);
}
if (sparse) {
options.put("sparse", sparse);
}
logger.debug("Created index " + fields + (unique ? " (unique)" : "") + " on " + dbCollection.getName() + " in " + this.database.getName());
dbCollection.ensureIndex(fields, options);
logger.debug("Created index " + fields + "(options: " + options + ") on " + dbCollection.getName() + " in " + this.database.getName());
}
@Override

View file

@ -39,8 +39,10 @@ public class MongoEntityMapper<T extends MongoEntity> implements Mapper<T, Basic
String propName = property.getName();
Object propValue = property.getValue(applicationObject);
Object dbValue = propValue == null ? null : mapperRegistry.convertApplicationObjectToDBObject(propValue, Object.class);
dbObject.put(propName, dbValue);
if (propValue != null) {
Object dbValue = mapperRegistry.convertApplicationObjectToDBObject(propValue, Object.class);
dbObject.put(propName, dbValue);
}
}
return dbObject;

View file

@ -490,10 +490,6 @@ public class RealmAdapter extends AbstractMongoAdapter<RealmEntity> implements R
// Add just user entity without defaultRoles
protected UserAdapter addUserEntity(String username) {
if (getUser(username) != null) {
throw new IllegalArgumentException("User " + username + " already exists");
}
UserEntity userEntity = new UserEntity();
userEntity.setLoginName(username);
// Compatibility with JPA model, which has user disabled by default

View file

@ -34,6 +34,12 @@ public class UserAdapter extends AbstractMongoAdapter<UserEntity> implements Use
return user.getLoginName();
}
@Override
public void setLoginName(String loginName) {
user.setLoginName(loginName);
updateUser();
}
@Override
public boolean isEnabled() {
return user.isEnabled();

View file

@ -5,6 +5,8 @@ import org.keycloak.models.mongo.api.AbstractMongoIdentifiableEntity;
import org.keycloak.models.mongo.api.MongoCollection;
import org.keycloak.models.mongo.api.MongoEntity;
import org.keycloak.models.mongo.api.MongoField;
import org.keycloak.models.mongo.api.MongoIndex;
import org.keycloak.models.mongo.api.MongoIndexes;
import java.util.ArrayList;
import java.util.List;
@ -14,6 +16,10 @@ import java.util.Map;
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
@MongoCollection(collectionName = "users")
@MongoIndexes({
@MongoIndex(name = "loginName-within-realm", fields = { "realmId", "loginName" }, unique = true),
@MongoIndex(name = "email-within-realm", fields = { "emailRealm" }, unique = true, sparse = true),
})
public class UserEntity extends AbstractMongoIdentifiableEntity implements MongoEntity {
private String loginName;
@ -76,6 +82,15 @@ public class UserEntity extends AbstractMongoIdentifiableEntity implements Mongo
this.email = email;
}
@MongoField
// TODO This is required as Mongo doesn't support sparse indexes with compound keys (see https://jira.mongodb.org/browse/SERVER-2193)
public String getEmailRealm() {
return email != null ? realmId + "//" + email : null;
}
public void setEmailRealm(String emailRealm) {
}
@MongoField
public boolean isEmailVerified() {
return emailVerified;

View file

@ -99,6 +99,10 @@ public class AbstractModelTest {
} else {
identitySession.getTransaction().commit();
}
resetSession();
}
protected void resetSession() {
identitySession.close();
identitySession = factory.createSession();
identitySession.getTransaction().begin();

View file

@ -267,7 +267,7 @@ public class AdapterTest extends AbstractModelTest {
UserModel user3 = realmModel.addUser("doublelast");
user3.setFirstName("Ole");
user3.setLastName("Alver Veland");
user3.setEmail("knut@redhat.com");
user3.setEmail("knut2@redhat.com");
}
RealmManager adapter = realmManager;
@ -531,9 +531,7 @@ public class AdapterTest extends AbstractModelTest {
} catch (ModelDuplicateException e) {
}
identitySession.close();
identitySession = factory.createSession();
identitySession.getTransaction().begin();
resetSession();
}
@Test
@ -562,9 +560,7 @@ public class AdapterTest extends AbstractModelTest {
} catch (ModelDuplicateException e) {
}
identitySession.close();
identitySession = factory.createSession();
identitySession.getTransaction().begin();
resetSession();
}
@Test
@ -593,9 +589,64 @@ public class AdapterTest extends AbstractModelTest {
} catch (ModelDuplicateException e) {
}
identitySession.close();
identitySession = factory.createSession();
identitySession.getTransaction().begin();
resetSession();
}
@Test
public void testUsernameCollisions() throws Exception {
realmManager.createRealm("JUGGLER1").addUser("user1");
realmManager.createRealm("JUGGLER2").addUser("user1");
commit();
// Try to create user with duplicate login name
try {
realmManager.getRealmByName("JUGGLER1").addUser("user1");
commit();
Assert.fail("Expected exception");
} catch (ModelDuplicateException e) {
}
commit(true);
// Ty to rename user to duplicate login name
realmManager.getRealmByName("JUGGLER1").addUser("user2");
commit();
try {
realmManager.getRealmByName("JUGGLER1").getUser("user2").setLoginName("user1");
commit();
Assert.fail("Expected exception");
} catch (ModelDuplicateException e) {
}
resetSession();
}
@Test
public void testEmailCollisions() throws Exception {
realmManager.createRealm("JUGGLER1").addUser("user1").setEmail("email@example.com");
realmManager.createRealm("JUGGLER2").addUser("user1").setEmail("email@example.com");
commit();
// Try to create user with duplicate email
try {
realmManager.getRealmByName("JUGGLER1").addUser("user2").setEmail("email@example.com");
commit();
Assert.fail("Expected exception");
} catch (ModelDuplicateException e) {
}
resetSession();
// Ty to rename user to duplicate email
realmManager.getRealmByName("JUGGLER1").addUser("user3").setEmail("email2@example.com");
commit();
try {
realmManager.getRealmByName("JUGGLER1").getUser("user3").setEmail("email@example.com");
commit();
Assert.fail("Expected exception");
} catch (ModelDuplicateException e) {
}
resetSession();
}
}

View file

@ -18,7 +18,7 @@
{
"username" : "REALM_COMPOSITE_1_USER",
"enabled": true,
"email" : "test-user@localhost",
"email" : "test-user1@localhost",
"credentials" : [
{ "type" : "password",
"value" : "password" }
@ -27,7 +27,7 @@
{
"username" : "REALM_ROLE_1_USER",
"enabled": true,
"email" : "test-user@localhost",
"email" : "test-user2@localhost",
"credentials" : [
{ "type" : "password",
"value" : "password" }
@ -36,7 +36,7 @@
{
"username" : "REALM_APP_COMPOSITE_USER",
"enabled": true,
"email" : "test-user@localhost",
"email" : "test-user3@localhost",
"credentials" : [
{ "type" : "password",
"value" : "password" }
@ -45,7 +45,7 @@
{
"username" : "REALM_APP_ROLE_USER",
"enabled": true,
"email" : "test-user@localhost",
"email" : "test-user4@localhost",
"credentials" : [
{ "type" : "password",
"value" : "password" }
@ -54,7 +54,7 @@
{
"username" : "APP_COMPOSITE_USER",
"enabled": true,
"email" : "test-user@localhost",
"email" : "test-user5@localhost",
"credentials" : [
{ "type" : "password",
"value" : "password" }

View file

@ -8,6 +8,7 @@ import org.keycloak.models.ApplicationModel;
import org.keycloak.models.ClientModel;
import org.keycloak.models.Constants;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.ModelDuplicateException;
import org.keycloak.models.RealmModel;
import org.keycloak.models.RoleModel;
import org.keycloak.models.UserCredentialModel;
@ -84,14 +85,20 @@ public class UsersResource {
@Path("{username}")
@PUT
@Consumes("application/json")
public void updateUser(final @PathParam("username") String username, final UserRepresentation rep) {
public Response updateUser(final @PathParam("username") String username, final UserRepresentation rep) {
auth.requireManage();
UserModel user = realm.getUser(username);
if (user == null) {
throw new NotFoundException("User not found");
try {
UserModel user = realm.getUser(username);
if (user == null) {
throw new NotFoundException("User not found");
}
updateUserFromRep(user, rep);
return Response.noContent().build();
} catch (ModelDuplicateException e) {
return Flows.errors().exists("User exists with same username or email");
}
updateUserFromRep(user, rep);
}
@POST
@ -99,17 +106,14 @@ public class UsersResource {
public Response createUser(final @Context UriInfo uriInfo, final UserRepresentation rep) {
auth.requireManage();
if (realm.getUser(rep.getUsername()) != null) {
return Flows.errors().exists("User with username " + rep.getUsername() + " already exists");
}
UserModel user = realm.addUser(rep.getUsername());
if (user == null) {
throw new NotFoundException("User not found");
}
try {
UserModel user = realm.addUser(rep.getUsername());
updateUserFromRep(user, rep);
updateUserFromRep(user, rep);
return Response.created(uriInfo.getAbsolutePathBuilder().path(user.getLoginName()).build()).build();
return Response.created(uriInfo.getAbsolutePathBuilder().path(user.getLoginName()).build()).build();
} catch (ModelDuplicateException e) {
return Flows.errors().exists("User exists with same username or email");
}
}
private void updateUserFromRep(UserModel user, UserRepresentation rep) {

View file

@ -75,12 +75,12 @@ public class RegisterTest {
loginPage.clickRegister();
registerPage.assertCurrent();
registerPage.register("firstName", "lastName", "email", "test-user@localhost", "password", "password");
registerPage.register("firstName", "lastName", "registerExistingUseremail", "test-user@localhost", "password", "password");
registerPage.assertCurrent();
Assert.assertEquals("Username already exists", registerPage.getError());
events.expectRegister("test-user@localhost", "email").user((String) null).error("username_in_use").assertEvent();
events.expectRegister("test-user@localhost", "registerExistingUseremail").user((String) null).error("username_in_use").assertEvent();
}
@Test
@ -89,12 +89,12 @@ public class RegisterTest {
loginPage.clickRegister();
registerPage.assertCurrent();
registerPage.register("firstName", "lastName", "email", "registerUserInvalidPasswordConfirm", "password", "invalid");
registerPage.register("firstName", "lastName", "registerUserInvalidPasswordConfirmemail", "registerUserInvalidPasswordConfirm", "password", "invalid");
registerPage.assertCurrent();
Assert.assertEquals("Password confirmation doesn't match", registerPage.getError());
events.expectRegister("registerUserInvalidPasswordConfirm", "email").user((String) null).error("invalid_registration").assertEvent();
events.expectRegister("registerUserInvalidPasswordConfirm", "registerUserInvalidPasswordConfirmemail").user((String) null).error("invalid_registration").assertEvent();
}
@Test
@ -103,12 +103,12 @@ public class RegisterTest {
loginPage.clickRegister();
registerPage.assertCurrent();
registerPage.register("firstName", "lastName", "email", "registerUserMissingPassword", null, null);
registerPage.register("firstName", "lastName", "registerUserMissingPasswordemail", "registerUserMissingPassword", null, null);
registerPage.assertCurrent();
Assert.assertEquals("Please specify password.", registerPage.getError());
events.expectRegister("registerUserMissingPassword", "email").user((String) null).error("invalid_registration").assertEvent();
events.expectRegister("registerUserMissingPassword", "registerUserMissingPasswordemail").user((String) null).error("invalid_registration").assertEvent();
}
@Test
@ -125,17 +125,17 @@ public class RegisterTest {
loginPage.clickRegister();
registerPage.assertCurrent();
registerPage.register("firstName", "lastName", "email", "registerPasswordPolicy", "pass", "pass");
registerPage.register("firstName", "lastName", "registerPasswordPolicyemail", "registerPasswordPolicy", "pass", "pass");
registerPage.assertCurrent();
Assert.assertEquals("Invalid password: minimum length 8", registerPage.getError());
events.expectRegister("registerPasswordPolicy", "email").user((String) null).error("invalid_registration").assertEvent();
events.expectRegister("registerPasswordPolicy", "registerPasswordPolicyemail").user((String) null).error("invalid_registration").assertEvent();
registerPage.register("firstName", "lastName", "email", "registerPasswordPolicy", "password", "password");
registerPage.register("firstName", "lastName", "registerPasswordPolicyemail", "registerPasswordPolicy", "password", "password");
Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType());
String userId = events.expectRegister("registerPasswordPolicy", "email").assertEvent().getUserId();
String userId = events.expectRegister("registerPasswordPolicy", "registerPasswordPolicyemail").assertEvent().getUserId();
events.expectLogin().user(userId).detail(Details.USERNAME, "registerPasswordPolicy").assertEvent();
} finally {
@ -154,12 +154,12 @@ public class RegisterTest {
loginPage.clickRegister();
registerPage.assertCurrent();
registerPage.register("firstName", "lastName", "email", null, "password", "password");
registerPage.register("firstName", "lastName", "registerUserMissingUsernameemail", null, "password", "password");
registerPage.assertCurrent();
Assert.assertEquals("Please specify username", registerPage.getError());
events.expectRegister(null, "email").removeDetail("username").error("invalid_registration").assertEvent();
events.expectRegister(null, "registerUserMissingUsernameemail").removeDetail("username").error("invalid_registration").assertEvent();
}
@Test
@ -168,11 +168,11 @@ public class RegisterTest {
loginPage.clickRegister();
registerPage.assertCurrent();
registerPage.register("firstName", "lastName", "email", "registerUserSuccess", "password", "password");
registerPage.register("firstName", "lastName", "registerUserSuccessemail", "registerUserSuccess", "password", "password");
Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType());
String userId = events.expectRegister("registerUserSuccess", "email").assertEvent().getUserId();
String userId = events.expectRegister("registerUserSuccess", "registerUserSuccessemail").assertEvent().getUserId();
events.expectLogin().detail("username", "registerUserSuccess").user(userId).assertEvent();
}

View file

@ -18,7 +18,7 @@
{
"username" : "REALM_COMPOSITE_1_USER",
"enabled": true,
"email" : "test-user@localhost",
"email" : "test-user1@localhost",
"credentials" : [
{ "type" : "password",
"value" : "password" }
@ -27,7 +27,7 @@
{
"username" : "REALM_ROLE_1_USER",
"enabled": true,
"email" : "test-user@localhost",
"email" : "test-user2@localhost",
"credentials" : [
{ "type" : "password",
"value" : "password" }
@ -36,7 +36,7 @@
{
"username" : "REALM_APP_COMPOSITE_USER",
"enabled": true,
"email" : "test-user@localhost",
"email" : "test-user3@localhost",
"credentials" : [
{ "type" : "password",
"value" : "password" }
@ -45,7 +45,7 @@
{
"username" : "REALM_APP_ROLE_USER",
"enabled": true,
"email" : "test-user@localhost",
"email" : "test-user4@localhost",
"credentials" : [
{ "type" : "password",
"value" : "password" }
@ -54,7 +54,7 @@
{
"username" : "APP_COMPOSITE_USER",
"enabled": true,
"email" : "test-user@localhost",
"email" : "test-user5@localhost",
"credentials" : [
{ "type" : "password",
"value" : "password" }