diff --git a/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/oauth-client-list.html b/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/oauth-client-list.html index c477f14ce60..cf9f9850d83 100755 --- a/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/oauth-client-list.html +++ b/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/oauth-client-list.html @@ -22,7 +22,7 @@
- Add Application + Add Client
diff --git a/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/user-detail.html b/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/user-detail.html index 892e8a8e921..f0b32d4b7e5 100755 --- a/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/user-detail.html +++ b/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/user-detail.html @@ -26,12 +26,19 @@
* Required fields
+
+ +
+ +
+
+
+ required ng-pattern="/^[^\<\>\\\/]*$/">
diff --git a/model/api/src/main/java/org/keycloak/models/UserModel.java b/model/api/src/main/java/org/keycloak/models/UserModel.java index 762acc8f8ff..b01cf93e1fe 100755 --- a/model/api/src/main/java/org/keycloak/models/UserModel.java +++ b/model/api/src/main/java/org/keycloak/models/UserModel.java @@ -17,6 +17,8 @@ public interface UserModel { String getLoginName(); + void setLoginName(String loginName); + boolean isEnabled(); boolean isTotp(); diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaKeycloakTransaction.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaKeycloakTransaction.java index 6e7f3f38cfe..095b3414a19 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaKeycloakTransaction.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaKeycloakTransaction.java @@ -3,6 +3,7 @@ package org.keycloak.models.jpa; import org.keycloak.models.KeycloakTransaction; import javax.persistence.EntityManager; +import javax.persistence.PersistenceException; /** * @author Bill Burke @@ -23,7 +24,11 @@ public class JpaKeycloakTransaction implements KeycloakTransaction { @Override public void commit() { - em.getTransaction().commit(); + try { + em.getTransaction().commit(); + } catch (PersistenceException e) { + throw PersistenceExceptionConverter.convert(e.getCause() != null ? e.getCause() : e); + } } @Override diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/OAuthClientAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/OAuthClientAdapter.java index 1ae4a3dd316..48d0e4f8ce6 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/OAuthClientAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/OAuthClientAdapter.java @@ -21,5 +21,6 @@ public class OAuthClientAdapter extends ClientAdapter implements OAuthClientMode @Override public void setClientId(String id) { entity.setName(id); + } } diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/PersistenceExceptionConverter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/PersistenceExceptionConverter.java index 712f1e7c0b5..dfa5053822f 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/PersistenceExceptionConverter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/PersistenceExceptionConverter.java @@ -6,6 +6,7 @@ import org.keycloak.models.ModelDuplicateException; import javax.persistence.EntityExistsException; import javax.persistence.EntityManager; +import javax.persistence.PersistenceException; import java.lang.reflect.InvocationHandler; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; @@ -31,14 +32,17 @@ public class PersistenceExceptionConverter implements InvocationHandler { try { return method.invoke(em, args); } catch (InvocationTargetException e) { - Throwable c = e.getCause(); - if (c.getCause() != null && c.getCause() instanceof ConstraintViolationException) { - throw new ModelDuplicateException(c); - } if (c instanceof EntityExistsException) { - throw new ModelDuplicateException(c); - } else { - throw new ModelException(c); - } + throw convert(e.getCause()); + } + } + + public static ModelException convert(Throwable t) { + if (t.getCause() != null && t.getCause() instanceof ConstraintViolationException) { + throw new ModelDuplicateException(t); + } if (t instanceof EntityExistsException) { + throw new ModelDuplicateException(t); + } else { + throw new ModelException(t); } } diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java index 54559ab8817..d76524dedeb 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java @@ -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); diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java index 50daa114494..98570625c9e 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java @@ -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(); diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/ApplicationEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/ApplicationEntity.java index ffafaa50441..adcae871455 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/ApplicationEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/ApplicationEntity.java @@ -7,10 +7,13 @@ import javax.persistence.Entity; import javax.persistence.FetchType; import javax.persistence.GeneratedValue; import javax.persistence.Id; +import javax.persistence.JoinColumn; import javax.persistence.JoinTable; import javax.persistence.ManyToOne; import javax.persistence.OneToMany; import javax.persistence.OneToOne; +import javax.persistence.Table; +import javax.persistence.UniqueConstraint; import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; @@ -30,9 +33,6 @@ public class ApplicationEntity extends ClientEntity { private String managementUrl; private boolean bearerOnly; - @ManyToOne() - private RealmEntity realm; - @OneToMany(fetch = FetchType.EAGER, cascade ={CascadeType.REMOVE}, orphanRemoval = true, mappedBy = "application") Collection roles = new ArrayList(); @@ -80,14 +80,6 @@ public class ApplicationEntity extends ClientEntity { this.defaultRoles = defaultRoles; } - public RealmEntity getRealm() { - return realm; - } - - public void setRealm(RealmEntity realm) { - this.realm = realm; - } - public boolean isBearerOnly() { return bearerOnly; } diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/ClientEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/ClientEntity.java index b365ee0f5da..07295532b85 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/ClientEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/ClientEntity.java @@ -4,6 +4,7 @@ import org.hibernate.annotations.GenericGenerator; import javax.persistence.CascadeType; import javax.persistence.CollectionTable; +import javax.persistence.Column; import javax.persistence.ElementCollection; import javax.persistence.Entity; import javax.persistence.FetchType; @@ -11,8 +12,11 @@ import javax.persistence.GeneratedValue; import javax.persistence.Id; import javax.persistence.Inheritance; import javax.persistence.InheritanceType; +import javax.persistence.JoinColumn; import javax.persistence.ManyToOne; import javax.persistence.OneToMany; +import javax.persistence.Table; +import javax.persistence.UniqueConstraint; import java.util.HashSet; import java.util.Set; @@ -21,12 +25,14 @@ import java.util.Set; * @version $Revision: 1 $ */ @Entity -@Inheritance(strategy = InheritanceType.JOINED) -public class ClientEntity { +@Inheritance(strategy = InheritanceType.TABLE_PER_CLASS) +@Table(uniqueConstraints = {@UniqueConstraint(columnNames = {"realm", "name"})}) +public abstract class ClientEntity { @Id @GenericGenerator(name="keycloak_generator", strategy="org.keycloak.models.jpa.utils.JpaIdGenerator") @GeneratedValue(generator = "keycloak_generator") private String id; + @Column(name = "name") private String name; private boolean enabled; private String secret; @@ -34,6 +40,9 @@ public class ClientEntity { private int notBefore; private boolean publicClient; + @ManyToOne + @JoinColumn(name = "realm") + protected RealmEntity realm; @ElementCollection @CollectionTable @@ -42,6 +51,13 @@ public class ClientEntity { @CollectionTable protected Set redirectUris = new HashSet(); + public RealmEntity getRealm() { + return realm; + } + + public void setRealm(RealmEntity realm) { + this.realm = realm; + } public String getId() { return id; diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OAuthClientEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OAuthClientEntity.java index ff695301707..2a570244a97 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OAuthClientEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OAuthClientEntity.java @@ -1,20 +1,8 @@ package org.keycloak.models.jpa.entities; -import javax.persistence.CollectionTable; -import javax.persistence.ElementCollection; import javax.persistence.Entity; -import javax.persistence.FetchType; -import javax.persistence.GeneratedValue; -import javax.persistence.Id; -import javax.persistence.ManyToOne; import javax.persistence.NamedQueries; import javax.persistence.NamedQuery; -import javax.persistence.OneToOne; - -import org.hibernate.annotations.GenericGenerator; - -import java.util.HashSet; -import java.util.Set; /** * @author Bill Burke @@ -27,17 +15,4 @@ import java.util.Set; }) @Entity public class OAuthClientEntity extends ClientEntity { - - @ManyToOne() - private RealmEntity realm; - - public RealmEntity getRealm() { - return realm; - } - - public void setRealm(RealmEntity realm) { - this.realm = realm; - } - - } diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/RealmEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/RealmEntity.java index 7d4b5a91ce7..ba662f42d18 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/RealmEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/RealmEntity.java @@ -83,7 +83,7 @@ public class RealmEntity { @JoinTable(name="AuthProviders") List authenticationProviders = new ArrayList(); - @OneToMany(fetch = FetchType.LAZY, cascade ={CascadeType.REMOVE}, orphanRemoval = true, mappedBy = "realm") + @OneToMany(fetch = FetchType.LAZY, cascade ={CascadeType.REMOVE}, orphanRemoval = true) Collection applications = new ArrayList(); @OneToMany(fetch = FetchType.LAZY, cascade ={CascadeType.REMOVE}, orphanRemoval = true, mappedBy = "realm") diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserEntity.java index 1ffba3ba806..f7ef212c4ab 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserEntity.java @@ -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 diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/api/MongoIndex.java b/model/mongo/src/main/java/org/keycloak/models/mongo/api/MongoIndex.java index 48b6d09698f..ed44b2b91bf 100644 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/api/MongoIndex.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/api/MongoIndex.java @@ -23,4 +23,6 @@ public @interface MongoIndex { boolean unique() default false; + boolean sparse() default false; + } diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/impl/MongoStoreImpl.java b/model/mongo/src/main/java/org/keycloak/models/mongo/impl/MongoStoreImpl.java index 483128dbb0a..445f77796c6 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/impl/MongoStoreImpl.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/impl/MongoStoreImpl.java @@ -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 diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/impl/types/MongoEntityMapper.java b/model/mongo/src/main/java/org/keycloak/models/mongo/impl/types/MongoEntityMapper.java index d62deac4095..1d3d665af28 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/impl/types/MongoEntityMapper.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/impl/types/MongoEntityMapper.java @@ -39,8 +39,10 @@ public class MongoEntityMapper implements Mapper 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 diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java index 3ae25027174..608082e761e 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java @@ -34,6 +34,12 @@ public class UserAdapter extends AbstractMongoAdapter implements Use return user.getLoginName(); } + @Override + public void setLoginName(String loginName) { + user.setLoginName(loginName); + updateUser(); + } + @Override public boolean isEnabled() { return user.isEnabled(); diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/entities/ApplicationEntity.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/entities/ApplicationEntity.java index be086d99975..c98842e91bd 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/entities/ApplicationEntity.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/entities/ApplicationEntity.java @@ -7,12 +7,14 @@ import com.mongodb.DBObject; import com.mongodb.QueryBuilder; import org.keycloak.models.mongo.api.MongoCollection; import org.keycloak.models.mongo.api.MongoField; +import org.keycloak.models.mongo.api.MongoIndex; import org.keycloak.models.mongo.api.context.MongoStoreInvocationContext; /** * @author Marek Posolda */ @MongoCollection(collectionName = "applications") +@MongoIndex(name = "name-within-realm", fields = { "realmId", "name" }, unique = true) public class ApplicationEntity extends ClientEntity { private boolean surrogateAuthRequired; diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/entities/OAuthClientEntity.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/entities/OAuthClientEntity.java index 7c6ea588f35..0ca8045c3bf 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/entities/OAuthClientEntity.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/entities/OAuthClientEntity.java @@ -1,6 +1,7 @@ package org.keycloak.models.mongo.keycloak.entities; import org.keycloak.models.mongo.api.MongoCollection; +import org.keycloak.models.mongo.api.MongoIndex; import java.util.List; @@ -8,6 +9,7 @@ import java.util.List; * @author Marek Posolda */ @MongoCollection(collectionName = "oauthClients") +@MongoIndex(name = "name-within-realm", fields = { "realmId", "name" }, unique = true) public class OAuthClientEntity extends ClientEntity { } diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/entities/UserEntity.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/entities/UserEntity.java index 5aeaceeec84..752d2e926e9 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/entities/UserEntity.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/entities/UserEntity.java @@ -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 Marek Posolda */ @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; diff --git a/model/tests/src/test/java/org/keycloak/model/test/AbstractModelTest.java b/model/tests/src/test/java/org/keycloak/model/test/AbstractModelTest.java index 8438bb37811..2f7177b215b 100755 --- a/model/tests/src/test/java/org/keycloak/model/test/AbstractModelTest.java +++ b/model/tests/src/test/java/org/keycloak/model/test/AbstractModelTest.java @@ -99,6 +99,10 @@ public class AbstractModelTest { } else { identitySession.getTransaction().commit(); } + resetSession(); + } + + protected void resetSession() { identitySession.close(); identitySession = factory.createSession(); identitySession.getTransaction().begin(); diff --git a/model/tests/src/test/java/org/keycloak/model/test/AdapterTest.java b/model/tests/src/test/java/org/keycloak/model/test/AdapterTest.java index 625e959d113..45ba227ae3f 100755 --- a/model/tests/src/test/java/org/keycloak/model/test/AdapterTest.java +++ b/model/tests/src/test/java/org/keycloak/model/test/AdapterTest.java @@ -218,9 +218,9 @@ public class AdapterTest extends AbstractModelTest { realmModel.addScopeMapping(app, realmRole); - Assert.assertTrue(identitySession.removeRealm(realmModel.getId())); - Assert.assertFalse(identitySession.removeRealm(realmModel.getId())); - Assert.assertNull(identitySession.getRealm(realmModel.getId())); + Assert.assertTrue(realmManager.removeRealm(realmModel)); + Assert.assertFalse(realmManager.removeRealm(realmModel)); + Assert.assertNull(realmManager.getRealm(realmModel.getId())); } @@ -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; @@ -522,17 +522,131 @@ public class AdapterTest extends AbstractModelTest { commit(true); // Ty to rename realm to duplicate name - realmModel = realmManager.createRealm("JUGGLER2"); + realmManager.createRealm("JUGGLER2"); + commit(); + try { + realmManager.getRealmByName("JUGGLER2").setName("JUGGLER"); + commit(); + Assert.fail("Expected exception"); + } catch (ModelDuplicateException e) { + } + + resetSession(); + } + + @Test + public void testAppNameCollisions() throws Exception { + realmManager.createRealm("JUGGLER1").addApplication("app1"); + realmManager.createRealm("JUGGLER2").addApplication("app1"); + commit(); - realmModel = realmManager.getRealmByName("JUGGLER2"); + // Try to create app with duplicate name try { - realmModel.setName("JUGGLER"); + realmManager.getRealmByName("JUGGLER1").addApplication("app1"); commit(); Assert.fail("Expected exception"); } catch (ModelDuplicateException e) { } commit(true); + + // Ty to rename app to duplicate name + realmManager.getRealmByName("JUGGLER1").addApplication("app2"); + commit(); + try { + realmManager.getRealmByName("JUGGLER1").getApplicationByName("app2").setName("app1"); + commit(); + Assert.fail("Expected exception"); + } catch (ModelDuplicateException e) { + } + + resetSession(); + } + + @Test + public void testClientNameCollisions() throws Exception { + realmManager.createRealm("JUGGLER1").addOAuthClient("client1"); + realmManager.createRealm("JUGGLER2").addOAuthClient("client1"); + + commit(); + + // Try to create app with duplicate name + try { + realmManager.getRealmByName("JUGGLER1").addOAuthClient("client1"); + commit(); + Assert.fail("Expected exception"); + } catch (ModelDuplicateException e) { + } + commit(true); + + // Ty to rename app to duplicate name + realmManager.getRealmByName("JUGGLER1").addOAuthClient("client2"); + commit(); + try { + realmManager.getRealmByName("JUGGLER1").getOAuthClient("client2").setClientId("client1"); + commit(); + Assert.fail("Expected exception"); + } catch (ModelDuplicateException e) { + } + + 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(); } } diff --git a/model/tests/src/test/resources/testcomposites.json b/model/tests/src/test/resources/testcomposites.json index 61038ea65b9..9b0878447ec 100755 --- a/model/tests/src/test/resources/testcomposites.json +++ b/model/tests/src/test/resources/testcomposites.json @@ -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" } diff --git a/services/src/main/java/org/keycloak/services/managers/RealmManager.java b/services/src/main/java/org/keycloak/services/managers/RealmManager.java index 4f7b9e076b9..ff76a99da07 100755 --- a/services/src/main/java/org/keycloak/services/managers/RealmManager.java +++ b/services/src/main/java/org/keycloak/services/managers/RealmManager.java @@ -104,9 +104,9 @@ public class RealmManager { public boolean removeRealm(RealmModel realm) { boolean removed = identitySession.removeRealm(realm.getId()); - - getKeycloakAdminstrationRealm().removeApplication(realm.getAdminApp().getId()); - + if (removed) { + getKeycloakAdminstrationRealm().removeApplication(realm.getAdminApp().getId()); + } return removed; } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ApplicationResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ApplicationResource.java index 0fb20a94930..8878c6bfa54 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/ApplicationResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/ApplicationResource.java @@ -5,6 +5,7 @@ import org.jboss.resteasy.logging.Logger; import org.jboss.resteasy.spi.NotFoundException; import org.keycloak.models.ApplicationModel; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.RealmModel; import org.keycloak.models.UserCredentialModel; import org.keycloak.models.UserModel; @@ -17,6 +18,7 @@ import org.keycloak.services.managers.ModelToRepresentation; import org.keycloak.services.managers.RealmManager; import org.keycloak.services.managers.ResourceAdminManager; import org.keycloak.services.resources.KeycloakApplication; +import org.keycloak.services.resources.flows.Flows; import org.keycloak.util.JsonSerialization; import javax.ws.rs.Consumes; @@ -32,6 +34,7 @@ import javax.ws.rs.QueryParam; import javax.ws.rs.core.Application; import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; import java.io.IOException; import java.util.HashMap; @@ -74,11 +77,16 @@ public class ApplicationResource { @PUT @Consumes(MediaType.APPLICATION_JSON) - public void update(final ApplicationRepresentation rep) { + public Response update(final ApplicationRepresentation rep) { auth.requireManage(); ApplicationManager applicationManager = new ApplicationManager(new RealmManager(session)); - applicationManager.updateApplication(rep, application); + try { + applicationManager.updateApplication(rep, application); + return Response.noContent().build(); + } catch (ModelDuplicateException e) { + return Flows.errors().exists("Application " + rep.getName() + " already exists"); + } } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ApplicationsResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ApplicationsResource.java index ec236cf5858..d3e954397a4 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/ApplicationsResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/ApplicationsResource.java @@ -6,6 +6,7 @@ import org.jboss.resteasy.spi.NotFoundException; import org.jboss.resteasy.spi.ResteasyProviderFactory; import org.keycloak.models.ApplicationModel; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.RealmModel; import org.keycloak.representations.idm.ApplicationRepresentation; import org.keycloak.services.managers.ApplicationManager; @@ -72,12 +73,13 @@ public class ApplicationsResource { public Response createApplication(final @Context UriInfo uriInfo, final ApplicationRepresentation rep) { auth.requireManage(); - if (realm.getApplicationNameMap().containsKey(rep.getName())) { + ApplicationManager resourceManager = new ApplicationManager(new RealmManager(session)); + try { + ApplicationModel applicationModel = resourceManager.createApplication(realm, rep); + return Response.created(uriInfo.getAbsolutePathBuilder().path(applicationModel.getName()).build()).build(); + } catch (ModelDuplicateException e) { return Flows.errors().exists("Application " + rep.getName() + " already exists"); } - ApplicationManager resourceManager = new ApplicationManager(new RealmManager(session)); - ApplicationModel applicationModel = resourceManager.createApplication(realm, rep); - return Response.created(uriInfo.getAbsolutePathBuilder().path(applicationModel.getName()).build()).build(); } @Path("{app-name}") diff --git a/services/src/main/java/org/keycloak/services/resources/admin/OAuthClientResource.java b/services/src/main/java/org/keycloak/services/resources/admin/OAuthClientResource.java index eb72e630915..5b07018c26a 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/OAuthClientResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/OAuthClientResource.java @@ -4,6 +4,7 @@ import org.jboss.resteasy.annotations.cache.NoCache; import org.jboss.resteasy.logging.Logger; import org.jboss.resteasy.spi.NotFoundException; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.OAuthClientModel; import org.keycloak.models.RealmModel; import org.keycloak.models.UserCredentialModel; @@ -12,6 +13,7 @@ import org.keycloak.representations.idm.OAuthClientRepresentation; import org.keycloak.services.managers.ModelToRepresentation; import org.keycloak.services.managers.OAuthClientManager; import org.keycloak.services.resources.KeycloakApplication; +import org.keycloak.services.resources.flows.Flows; import org.keycloak.util.JsonSerialization; import javax.ws.rs.Consumes; @@ -24,6 +26,7 @@ import javax.ws.rs.Produces; import javax.ws.rs.core.Application; import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; import java.io.IOException; @@ -64,11 +67,16 @@ public class OAuthClientResource { @PUT @Consumes(MediaType.APPLICATION_JSON) - public void update(final OAuthClientRepresentation rep) { + public Response update(final OAuthClientRepresentation rep) { auth.requireManage(); OAuthClientManager manager = new OAuthClientManager(realm); - manager.update(rep, oauthClient); + try { + manager.update(rep, oauthClient); + return Response.noContent().build(); + } catch (ModelDuplicateException e) { + return Flows.errors().exists("Client " + rep.getName() + " already exists"); + } } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/OAuthClientsResource.java b/services/src/main/java/org/keycloak/services/resources/admin/OAuthClientsResource.java index 3fdcd166913..f940c931c25 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/OAuthClientsResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/OAuthClientsResource.java @@ -5,10 +5,12 @@ import org.jboss.resteasy.logging.Logger; import org.jboss.resteasy.spi.NotFoundException; import org.jboss.resteasy.spi.ResteasyProviderFactory; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.OAuthClientModel; import org.keycloak.models.RealmModel; import org.keycloak.representations.idm.OAuthClientRepresentation; import org.keycloak.services.managers.OAuthClientManager; +import org.keycloak.services.resources.flows.Flows; import javax.ws.rs.Consumes; import javax.ws.rs.GET; @@ -74,8 +76,12 @@ public class OAuthClientsResource { auth.requireManage(); OAuthClientManager resourceManager = new OAuthClientManager(realm); - OAuthClientModel oauth = resourceManager.create(rep); - return Response.created(uriInfo.getAbsolutePathBuilder().path(oauth.getId()).build()).build(); + try { + OAuthClientModel oauth = resourceManager.create(rep); + return Response.created(uriInfo.getAbsolutePathBuilder().path(oauth.getId()).build()).build(); + } catch (ModelDuplicateException e) { + return Flows.errors().exists("Client " + rep.getName() + " already exists"); + } } @Path("{id}") diff --git a/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java b/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java index 2e564371d7f..7ec8c169a93 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java @@ -9,6 +9,7 @@ import org.keycloak.audit.Event; import org.keycloak.audit.EventQuery; import org.keycloak.models.ApplicationModel; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.RealmModel; import org.keycloak.representations.adapters.action.SessionStats; import org.keycloak.representations.idm.RealmAuditRepresentation; @@ -18,10 +19,12 @@ import org.keycloak.services.managers.ModelToRepresentation; import org.keycloak.services.managers.RealmManager; import org.keycloak.services.managers.ResourceAdminManager; import org.keycloak.services.managers.TokenManager; +import org.keycloak.services.resources.flows.Flows; import javax.ws.rs.*; import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -94,11 +97,16 @@ public class RealmAdminResource { @PUT @Consumes("application/json") - public void updateRealm(final RealmRepresentation rep) { + public Response updateRealm(final RealmRepresentation rep) { auth.requireManage(); logger.debug("updating realm: " + realm.getName()); - new RealmManager(session).updateRealm(rep, realm); + try { + new RealmManager(session).updateRealm(rep, realm); + return Response.noContent().build(); + } catch (ModelDuplicateException e) { + return Flows.errors().exists("Realm " + rep.getRealm() + " already exists"); + } } @DELETE diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java index 0ceb0b8600c..8857e449886 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java @@ -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) { diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/RegisterTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/RegisterTest.java index aeea46283bb..429e5f90a9b 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/RegisterTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/RegisterTest.java @@ -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(); } diff --git a/testsuite/integration/src/test/resources/testcomposite.json b/testsuite/integration/src/test/resources/testcomposite.json index 61038ea65b9..9b0878447ec 100755 --- a/testsuite/integration/src/test/resources/testcomposite.json +++ b/testsuite/integration/src/test/resources/testcomposite.json @@ -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" }