From 292089cee8893e3b007e69b6d3ac522a1e6bffa7 Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Fri, 25 Apr 2014 17:01:54 +0100 Subject: [PATCH] Ensure Realm names are unique --- .../models/ModelDuplicateException.java | 27 +++++++++ .../org/keycloak/models/ModelException.java | 27 +++++++++ .../models/jpa/JpaKeycloakSession.java | 3 +- .../jpa/PersistenceExceptionConverter.java | 42 ++++++++++++++ .../models/jpa/entities/RealmEntity.java | 4 +- .../keycloak/models/mongo/api/MongoIndex.java | 26 +++++++++ .../models/mongo/api/MongoIndexes.java | 22 +++++++ .../models/mongo/impl/MongoStoreImpl.java | 57 ++++++++++++++++++- .../mongo/keycloak/adapters/RealmAdapter.java | 5 +- .../mongo/keycloak/entities/RealmEntity.java | 13 +++-- .../resources/admin/RealmsAdminResource.java | 32 +++++++---- 11 files changed, 236 insertions(+), 22 deletions(-) create mode 100644 model/api/src/main/java/org/keycloak/models/ModelDuplicateException.java create mode 100644 model/api/src/main/java/org/keycloak/models/ModelException.java create mode 100644 model/jpa/src/main/java/org/keycloak/models/jpa/PersistenceExceptionConverter.java create mode 100644 model/mongo/src/main/java/org/keycloak/models/mongo/api/MongoIndex.java create mode 100644 model/mongo/src/main/java/org/keycloak/models/mongo/api/MongoIndexes.java diff --git a/model/api/src/main/java/org/keycloak/models/ModelDuplicateException.java b/model/api/src/main/java/org/keycloak/models/ModelDuplicateException.java new file mode 100644 index 0000000000..be0176b09c --- /dev/null +++ b/model/api/src/main/java/org/keycloak/models/ModelDuplicateException.java @@ -0,0 +1,27 @@ +package org.keycloak.models; + +/** + * @author Stian Thorgersen + */ +public class ModelDuplicateException extends ModelException { + + public ModelDuplicateException() { + } + + public ModelDuplicateException(String message) { + super(message); + } + + public ModelDuplicateException(String message, Throwable cause) { + super(message, cause); + } + + public ModelDuplicateException(Throwable cause) { + super(cause); + } + + public ModelDuplicateException(String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace) { + super(message, cause, enableSuppression, writableStackTrace); + } + +} diff --git a/model/api/src/main/java/org/keycloak/models/ModelException.java b/model/api/src/main/java/org/keycloak/models/ModelException.java new file mode 100644 index 0000000000..bbaa40a723 --- /dev/null +++ b/model/api/src/main/java/org/keycloak/models/ModelException.java @@ -0,0 +1,27 @@ +package org.keycloak.models; + +/** + * @author Stian Thorgersen + */ +public class ModelException extends RuntimeException { + + public ModelException() { + } + + public ModelException(String message) { + super(message); + } + + public ModelException(String message, Throwable cause) { + super(message, cause); + } + + public ModelException(Throwable cause) { + super(cause); + } + + public ModelException(String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace) { + super(message, cause, enableSuppression, writableStackTrace); + } + +} diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaKeycloakSession.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaKeycloakSession.java index 057679e15a..9bcca9f49a 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaKeycloakSession.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaKeycloakSession.java @@ -6,6 +6,7 @@ import org.keycloak.models.utils.KeycloakModelUtils; import javax.persistence.EntityManager; import javax.persistence.TypedQuery; +import java.lang.reflect.Proxy; import java.util.ArrayList; import java.util.LinkedList; import java.util.List; @@ -18,7 +19,7 @@ public class JpaKeycloakSession implements KeycloakSession { protected EntityManager em; public JpaKeycloakSession(EntityManager em) { - this.em = em; + this.em = PersistenceExceptionConverter.create(em); } @Override 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 new file mode 100644 index 0000000000..c8a0523410 --- /dev/null +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/PersistenceExceptionConverter.java @@ -0,0 +1,42 @@ +package org.keycloak.models.jpa; + +import org.hibernate.exception.ConstraintViolationException; +import org.keycloak.models.ModelException; +import org.keycloak.models.ModelDuplicateException; + +import javax.persistence.EntityManager; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; + +/** + * @author Stian Thorgersen + */ +public class PersistenceExceptionConverter implements InvocationHandler { + + private EntityManager em; + + public static EntityManager create(EntityManager em) { + return (EntityManager) Proxy.newProxyInstance(EntityManager.class.getClassLoader(), new Class[]{EntityManager.class}, new PersistenceExceptionConverter(em)); + } + + private PersistenceExceptionConverter(EntityManager em) { + this.em = em; + } + + @Override + public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { + 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); + } else { + throw new ModelException(c); + } + } + } + +} 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 904a3e48a1..7d4b5a91ce 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 @@ -1,8 +1,6 @@ package org.keycloak.models.jpa.entities; -import org.keycloak.models.ApplicationModel; - import javax.persistence.CascadeType; import javax.persistence.CollectionTable; import javax.persistence.Column; @@ -37,7 +35,9 @@ public class RealmEntity { @Id protected String id; + @Column(unique = true) protected String name; + protected boolean enabled; protected boolean sslNotRequired; protected boolean registrationAllowed; 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 new file mode 100644 index 0000000000..48b6d09698 --- /dev/null +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/api/MongoIndex.java @@ -0,0 +1,26 @@ +package org.keycloak.models.mongo.api; + +import java.lang.annotation.Documented; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +/** + * @author Stian Thorgersen + */ +@Target({TYPE}) +@Documented +@Retention(RUNTIME) +@Inherited +public @interface MongoIndex { + + String[] fields(); + + String name() default ""; + + boolean unique() default false; + +} diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/api/MongoIndexes.java b/model/mongo/src/main/java/org/keycloak/models/mongo/api/MongoIndexes.java new file mode 100644 index 0000000000..da5020daca --- /dev/null +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/api/MongoIndexes.java @@ -0,0 +1,22 @@ +package org.keycloak.models.mongo.api; + +import java.lang.annotation.Documented; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +/** + * @author Stian Thorgersen + */ +@Target({TYPE}) +@Documented +@Retention(RUNTIME) +@Inherited +public @interface MongoIndexes { + + MongoIndex[] value(); + +} 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 654b7238ac..5afcc7d1c9 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 @@ -6,11 +6,16 @@ import com.mongodb.DB; import com.mongodb.DBCollection; import com.mongodb.DBCursor; import com.mongodb.DBObject; +import com.mongodb.MongoException; import org.jboss.logging.Logger; +import org.keycloak.models.ModelException; +import org.keycloak.models.ModelDuplicateException; 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.MongoIdentifiableEntity; +import org.keycloak.models.mongo.api.MongoIndex; +import org.keycloak.models.mongo.api.MongoIndexes; import org.keycloak.models.mongo.api.MongoStore; import org.keycloak.models.mongo.api.context.MongoStoreInvocationContext; import org.keycloak.models.mongo.api.context.MongoTask; @@ -88,6 +93,8 @@ public class MongoStoreImpl implements MongoStore { // dropDatabase(); clearManagedCollections(managedEntityTypes); } + + initManagedCollections(managedEntityTypes); } protected void dropDatabase() { @@ -106,6 +113,46 @@ public class MongoStoreImpl implements MongoStore { } } + protected void initManagedCollections(Class[] managedEntityTypes) { + for (Class clazz : managedEntityTypes) { + EntityInfo entityInfo = getEntityInfo(clazz); + String dbCollectionName = entityInfo.getDbCollectionName(); + if (dbCollectionName != null && !database.collectionExists(dbCollectionName)) { + DBCollection dbCollection = database.getCollection(dbCollectionName); + + logger.debug("Created collection " + dbCollection.getName() + " in " + this.database.getName()); + + MongoIndex index = clazz.getAnnotation(MongoIndex.class); + if (index != null) { + createIndex(dbCollection, index); + } + + MongoIndexes indexes = clazz.getAnnotation(MongoIndexes.class); + if (indexes != null) { + for (MongoIndex i : indexes.value()) { + createIndex(dbCollection, i); + } + } + } + } + } + + protected void createIndex(DBCollection dbCollection, MongoIndex index) { + BasicDBObject fields = new BasicDBObject(); + for (String f : index.fields()) { + fields.put(f, 1); + } + String name = index.name(); + if (name.length() == 0) { + name = null; + } + boolean unique = index.unique(); + + dbCollection.ensureIndex(fields, name, unique); + + logger.debug("Created index " + fields + (unique ? " (unique)" : "") + " on " + dbCollection.getName() + " in " + this.database.getName()); + } + @Override public void insertEntity(MongoIdentifiableEntity entity, MongoStoreInvocationContext context) { Class clazz = entity.getClass(); @@ -129,7 +176,15 @@ public class MongoStoreImpl implements MongoStore { // Adding "_id" dbObject.put("_id", currentId); - dbCollection.insert(dbObject); + try { + dbCollection.insert(dbObject); + } catch (MongoException e) { + if (e instanceof MongoException.DuplicateKey) { + throw new ModelDuplicateException(e); + } else { + throw new ModelException(e); + } + } // Treat object as created in this transaction (It is already submited to transaction) context.addCreatedEntity(entity); diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java index 8da896fa18..4d4452af1a 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java @@ -1242,12 +1242,13 @@ public class RealmAdapter extends AbstractMongoAdapter implements R @Override public ApplicationModel getAdminApp() { - return new ApplicationAdapter(this, realm.getAdminApp(), invocationContext); + ApplicationEntity appData = getMongoStore().loadEntity(ApplicationEntity.class, realm.getAdminAppId(), invocationContext); + return new ApplicationAdapter(this, appData, invocationContext); } @Override public void setAdminApp(ApplicationModel app) { - realm.setAdminApp(((ApplicationAdapter) app).getMongoEntity()); + realm.setAdminAppId(app.getId()); } @Override diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/entities/RealmEntity.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/entities/RealmEntity.java index 0fed4a7945..ffab7d759e 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/entities/RealmEntity.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/entities/RealmEntity.java @@ -7,6 +7,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 org.keycloak.models.mongo.api.context.MongoStoreInvocationContext; import java.util.ArrayList; @@ -21,6 +23,7 @@ import java.util.Set; * @author Marek Posolda */ @MongoCollection(collectionName = "realms") +@MongoIndex(fields = { "name" }, unique = true) public class RealmEntity extends AbstractMongoIdentifiableEntity implements MongoEntity { private String name; @@ -70,7 +73,7 @@ public class RealmEntity extends AbstractMongoIdentifiableEntity implements Mong private long auditExpiration; private List auditListeners = new ArrayList(); - private ApplicationEntity adminApp; + private String adminAppId; @MongoField public String getName() { @@ -397,12 +400,12 @@ public class RealmEntity extends AbstractMongoIdentifiableEntity implements Mong } @MongoField - public ApplicationEntity getAdminApp() { - return adminApp; + public String getAdminAppId() { + return adminAppId; } - public void setAdminApp(ApplicationEntity adminApp) { - this.adminApp = adminApp; + public void setAdminAppId(String adminAppId) { + this.adminAppId = adminAppId; } @Override diff --git a/services/src/main/java/org/keycloak/services/resources/admin/RealmsAdminResource.java b/services/src/main/java/org/keycloak/services/resources/admin/RealmsAdminResource.java index 33cd49b39b..81377d36e9 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/RealmsAdminResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/RealmsAdminResource.java @@ -10,6 +10,7 @@ import org.jboss.resteasy.util.GenericType; import org.keycloak.models.AdminRoles; import org.keycloak.models.ApplicationModel; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.representations.idm.RealmRepresentation; @@ -49,6 +50,7 @@ public class RealmsAdminResource { } public static final CacheControl noCache = new CacheControl(); + static { noCache.setNoCache(true); } @@ -97,21 +99,22 @@ public class RealmsAdminResource { logger.debug("importRealm: {0}", rep.getRealm()); RealmManager realmManager = new RealmManager(session); - if (realmManager.getRealmByName(rep.getRealm()) != null) { + + try { + RealmModel realm = realmManager.importRealm(rep); + grantPermissionsToRealmCreator(realm); + + URI location = realmUrl(uriInfo).build(realm.getName()); + logger.debug("imported realm success, sending back: {0}", location.toString()); + return Response.created(location).build(); + } catch (ModelDuplicateException e) { return Flows.errors().exists("Realm " + rep.getRealm() + " already exists"); } - - RealmModel realm = realmManager.importRealm(rep); - grantPermissionsToRealmCreator(realm); - - URI location = realmUrl(uriInfo).build(realm.getName()); - logger.debug("imported realm success, sending back: {0}", location.toString()); - return Response.created(location).build(); } @POST @Consumes(MediaType.MULTIPART_FORM_DATA) - public Response uploadRealm(@Context final UriInfo uriInfo, MultipartFormDataInput input) throws IOException { + public Response uploadRealm(@Context final UriInfo uriInfo, MultipartFormDataInput input) throws IOException { if (!auth.hasRealmRole(AdminRoles.CREATE_REALM)) { throw new ForbiddenException(); } @@ -122,9 +125,16 @@ public class RealmsAdminResource { RealmManager realmManager = new RealmManager(session); for (InputPart inputPart : inputParts) { inputPart.setMediaType(MediaType.APPLICATION_JSON_TYPE); - RealmRepresentation rep = inputPart.getBody(new GenericType(){}); + RealmRepresentation rep = inputPart.getBody(new GenericType() { + }); + + RealmModel realm; + try { + realm = realmManager.importRealm(rep); + } catch (ModelDuplicateException e) { + return Flows.errors().exists("Realm " + rep.getRealm() + " already exists"); + } - RealmModel realm = realmManager.importRealm(rep); grantPermissionsToRealmCreator(realm); if (inputParts.size() == 1) {