Ensure role name unique within realm/app

This commit is contained in:
Stian Thorgersen 2014-04-30 09:42:36 +01:00
parent bb039576d4
commit 316431c4d1
18 changed files with 145 additions and 63 deletions

View file

@ -7,8 +7,6 @@ package org.keycloak.models;
public interface Constants {
String ADMIN_CONSOLE_APPLICATION = "admin-console";
String INTERNAL_ROLE = "KEYCLOAK_";
String ACCOUNT_MANAGEMENT_APP = "account";
String INSTALLED_APP_URN = "urn:ietf:wg:oauth:2.0:oob";

View file

@ -99,8 +99,6 @@ public class ApplicationAdapter extends ClientAdapter implements ApplicationMode
@Override
public RoleModel addRole(String name) {
RoleModel role = getRole(name);
if (role != null) return role;
ApplicationRoleEntity roleEntity = new ApplicationRoleEntity();
roleEntity.setName(name);
roleEntity.setApplication(applicationEntity);

View file

@ -949,8 +949,6 @@ public class RealmAdapter implements RealmModel {
@Override
public RoleModel addRole(String name) {
RoleModel role = getRole(name);
if (role != null) return role;
RealmRoleEntity entity = new RealmRoleEntity();
entity.setName(name);
entity.setRealm(realm);

View file

@ -1,10 +1,15 @@
package org.keycloak.models.jpa.entities;
import javax.persistence.Entity;
import javax.persistence.Inheritance;
import javax.persistence.InheritanceType;
import javax.persistence.JoinColumn;
import javax.persistence.JoinTable;
import javax.persistence.ManyToOne;
import javax.persistence.NamedQueries;
import javax.persistence.NamedQuery;
import javax.persistence.Table;
import javax.persistence.UniqueConstraint;
/**
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
@ -15,10 +20,21 @@ import javax.persistence.NamedQuery;
})
@Entity
public class ApplicationRoleEntity extends RoleEntity {
private String name;
@ManyToOne
@JoinTable(name = "ApplicationRole")
@JoinColumn(name = "application")
private ApplicationEntity application;
public String getName() {
return name;
}
public void setName(String name) {
this.name = name;
}
public ApplicationEntity getApplication() {
return application;
}

View file

@ -25,7 +25,7 @@ import java.util.Set;
* @version $Revision: 1 $
*/
@Entity
@Inheritance(strategy = InheritanceType.TABLE_PER_CLASS)
@Inheritance(strategy = InheritanceType.SINGLE_TABLE)
@Table(uniqueConstraints = {@UniqueConstraint(columnNames = {"realm", "name"})})
public abstract class ClientEntity {
@Id

View file

@ -1,9 +1,12 @@
package org.keycloak.models.jpa.entities;
import javax.persistence.Entity;
import javax.persistence.JoinColumn;
import javax.persistence.ManyToOne;
import javax.persistence.NamedQueries;
import javax.persistence.NamedQuery;
import javax.persistence.Table;
import javax.persistence.UniqueConstraint;
/**
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
@ -14,9 +17,21 @@ import javax.persistence.NamedQuery;
})
@Entity
public class RealmRoleEntity extends RoleEntity {
private String name;
@ManyToOne
@JoinColumn(name = "realm")
private RealmEntity realm;
public String getName() {
return name;
}
public void setName(String name) {
this.name = name;
}
public RealmEntity getRealm() {
return realm;
}

View file

@ -9,6 +9,8 @@ import javax.persistence.InheritanceType;
import javax.persistence.JoinColumn;
import javax.persistence.JoinTable;
import javax.persistence.ManyToMany;
import javax.persistence.Table;
import javax.persistence.UniqueConstraint;
import java.util.ArrayList;
import java.util.Collection;
@ -20,19 +22,21 @@ import org.hibernate.annotations.GenericGenerator;
*/
@Entity
@Inheritance(strategy = InheritanceType.SINGLE_TABLE)
@Table(uniqueConstraints = {
@UniqueConstraint(columnNames = { "name", "application" }),
@UniqueConstraint(columnNames = { "name", "realm" })
})
public abstract class RoleEntity {
@Id
@GenericGenerator(name="keycloak_generator", strategy="org.keycloak.models.jpa.utils.JpaIdGenerator")
@GeneratedValue(generator = "keycloak_generator")
private String id;
private String name;
private String description;
@ManyToMany(fetch = FetchType.LAZY, cascade = {})
@JoinTable(name = "CompositeRole", joinColumns = @JoinColumn(name = "composite"), inverseJoinColumns = @JoinColumn(name = "role"))
private Collection<RoleEntity> compositeRoles = new ArrayList<RoleEntity>();
public String getId() {
return id;
}
@ -41,13 +45,9 @@ public abstract class RoleEntity {
this.id = id;
}
public String getName() {
return name;
}
public abstract String getName();
public void setName(String name) {
this.name = name;
}
public abstract void setName(String name);
public String getDescription() {
return description;

View file

@ -19,8 +19,6 @@ public @interface MongoIndex {
String[] fields();
String name() default "";
boolean unique() default false;
boolean sparse() default false;

View file

@ -142,17 +142,11 @@ public class MongoStoreImpl implements MongoStore {
for (String f : index.fields()) {
fields.put(f, 1);
}
String name = index.name();
if (name.length() == 0) {
name = null;
}
boolean unique = index.unique();
boolean sparse = index.sparse();
BasicDBObject options = new BasicDBObject();
if (name != null) {
options.put("name", name);
}
if (unique) {
options.put("unique", unique);
}

View file

@ -113,11 +113,6 @@ public class ApplicationAdapter extends ClientAdapter<ApplicationEntity> impleme
@Override
public RoleAdapter addRole(String name) {
RoleAdapter existing = getRole(name);
if (existing != null) {
return existing;
}
RoleEntity roleEntity = new RoleEntity();
roleEntity.setName(name);
roleEntity.setApplicationId(getId());

View file

@ -525,13 +525,6 @@ public class RealmAdapter extends AbstractMongoAdapter<RealmEntity> implements R
@Override
public RoleModel addRole(String name) {
RoleAdapter role = getRole(name);
if (role != null) {
// Compatibility with JPA model
return role;
// throw new IllegalArgumentException("Role " + name + " already exists");
}
RoleEntity roleEntity = new RoleEntity();
roleEntity.setName(name);
roleEntity.setRealmId(getId());

View file

@ -14,7 +14,7 @@ import org.keycloak.models.mongo.api.context.MongoStoreInvocationContext;
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
@MongoCollection(collectionName = "applications")
@MongoIndex(name = "name-within-realm", fields = { "realmId", "name" }, unique = true)
@MongoIndex(fields = { "realmId", "name" }, unique = true)
public class ApplicationEntity extends ClientEntity {
private boolean surrogateAuthRequired;

View file

@ -9,7 +9,7 @@ import java.util.List;
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
@MongoCollection(collectionName = "oauthClients")
@MongoIndex(name = "name-within-realm", fields = { "realmId", "name" }, unique = true)
@MongoIndex(fields = { "realmId", "name" }, unique = true)
public class OAuthClientEntity extends ClientEntity {
}

View file

@ -7,6 +7,7 @@ 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.MongoStore;
import org.keycloak.models.mongo.api.context.MongoStoreInvocationContext;
@ -16,6 +17,7 @@ import java.util.List;
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
@MongoCollection(collectionName = "roles")
@MongoIndex(fields = "nameIndex", unique = true)
public class RoleEntity extends AbstractMongoIdentifiableEntity implements MongoEntity {
private static final Logger logger = Logger.getLogger(RoleEntity.class);
@ -28,6 +30,19 @@ public class RoleEntity extends AbstractMongoIdentifiableEntity implements Mongo
private String realmId;
private String applicationId;
@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 getNameIndex() {
if (realmId != null) {
return realmId + "//" + name;
} else {
return applicationId + "//" + name;
}
}
public void setNameIndex(String ignored) {
}
@MongoField
public String getName() {
return name;

View file

@ -17,8 +17,8 @@ import java.util.Map;
*/
@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),
@MongoIndex(fields = { "realmId", "loginName" }, unique = true),
@MongoIndex(fields = { "emailIndex" }, unique = true, sparse = true),
})
public class UserEntity extends AbstractMongoIdentifiableEntity implements MongoEntity {
@ -84,11 +84,11 @@ public class UserEntity extends AbstractMongoIdentifiableEntity implements Mongo
@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() {
public String getEmailIndex() {
return email != null ? realmId + "//" + email : null;
}
public void setEmailRealm(String emailRealm) {
public void setEmailIndex(String ignored) {
}
@MongoField

View file

@ -649,4 +649,64 @@ public class AdapterTest extends AbstractModelTest {
resetSession();
}
@Test
public void testAppRoleCollisions() throws Exception {
realmManager.createRealm("JUGGLER1").addRole("role1");
realmManager.getRealmByName("JUGGLER1").addApplication("app1").addRole("role1");
realmManager.getRealmByName("JUGGLER1").addApplication("app2").addRole("role1");
commit();
// Try to add role with same name
try {
realmManager.getRealmByName("JUGGLER1").getApplicationByName("app1").addRole("role1");
commit();
Assert.fail("Expected exception");
} catch (ModelDuplicateException e) {
}
commit(true);
// Ty to rename role to duplicate name
realmManager.getRealmByName("JUGGLER1").getApplicationByName("app1").addRole("role2");
commit();
try {
realmManager.getRealmByName("JUGGLER1").getApplicationByName("app1").getRole("role2").setName("role1");
commit();
Assert.fail("Expected exception");
} catch (ModelDuplicateException e) {
}
resetSession();
}
@Test
public void testRealmRoleCollisions() throws Exception {
realmManager.createRealm("JUGGLER1").addRole("role1");
realmManager.getRealmByName("JUGGLER1").addApplication("app1").addRole("role1");
realmManager.getRealmByName("JUGGLER1").addApplication("app2").addRole("role1");
commit();
// Try to add role with same name
try {
realmManager.getRealmByName("JUGGLER1").addRole("role1");
commit();
Assert.fail("Expected exception");
} catch (ModelDuplicateException e) {
}
commit(true);
// Ty to rename role to duplicate name
realmManager.getRealmByName("JUGGLER1").addRole("role2");
commit();
try {
realmManager.getRealmByName("JUGGLER1").getRole("role2").setName("role1");
commit();
Assert.fail("Expected exception");
} catch (ModelDuplicateException e) {
}
resetSession();
}
}

View file

@ -52,7 +52,7 @@ public class RoleByIdResource extends RoleResource {
protected RoleModel getRoleModel(String id) {
RoleModel roleModel = realm.getRoleById(id);
if (roleModel == null || roleModel.getName().startsWith(Constants.INTERNAL_ROLE)) {
if (roleModel == null) {
throw new NotFoundException("Could not find role with id: " + id);
}

View file

@ -3,6 +3,7 @@ package org.keycloak.services.resources.admin;
import org.jboss.resteasy.annotations.cache.NoCache;
import org.jboss.resteasy.spi.NotFoundException;
import org.keycloak.models.Constants;
import org.keycloak.models.ModelDuplicateException;
import org.keycloak.models.RealmModel;
import org.keycloak.models.RoleContainerModel;
import org.keycloak.models.RoleModel;
@ -50,9 +51,7 @@ public class RoleContainerResource extends RoleResource {
Set<RoleModel> roleModels = roleContainer.getRoles();
List<RoleRepresentation> roles = new ArrayList<RoleRepresentation>();
for (RoleModel roleModel : roleModels) {
if (!roleModel.getName().startsWith(Constants.INTERNAL_ROLE)) {
roles.add(ModelToRepresentation.toRepresentation(roleModel));
}
roles.add(ModelToRepresentation.toRepresentation(roleModel));
}
return roles;
}
@ -62,15 +61,13 @@ public class RoleContainerResource extends RoleResource {
public Response createRole(final @Context UriInfo uriInfo, final RoleRepresentation rep) {
auth.requireManage();
if (roleContainer.getRole(rep.getName()) != null || rep.getName().startsWith(Constants.INTERNAL_ROLE)) {
try {
RoleModel role = roleContainer.addRole(rep.getName());
role.setDescription(rep.getDescription());
return Response.created(uriInfo.getAbsolutePathBuilder().path(role.getName()).build()).build();
} catch (ModelDuplicateException e) {
return Flows.errors().exists("Role with name " + rep.getName() + " already exists");
}
RoleModel role = roleContainer.addRole(rep.getName());
if (role == null) {
throw new NotFoundException("Role not found");
}
role.setDescription(rep.getDescription());
return Response.created(uriInfo.getAbsolutePathBuilder().path(role.getName()).build()).build();
}
@Path("{role-name}")
@ -81,7 +78,7 @@ public class RoleContainerResource extends RoleResource {
auth.requireView();
RoleModel roleModel = roleContainer.getRole(roleName);
if (roleModel == null || roleModel.getName().startsWith(Constants.INTERNAL_ROLE)) {
if (roleModel == null) {
throw new NotFoundException("Could not find role: " + roleName);
}
return getRole(roleModel);
@ -103,14 +100,19 @@ public class RoleContainerResource extends RoleResource {
@Path("{role-name}")
@PUT
@Consumes("application/json")
public void updateRole(final @PathParam("role-name") String roleName, final RoleRepresentation rep) {
public Response updateRole(final @PathParam("role-name") String roleName, final RoleRepresentation rep) {
auth.requireManage();
RoleModel role = roleContainer.getRole(roleName);
if (role == null || role.getName().startsWith(Constants.INTERNAL_ROLE)) {
if (role == null) {
throw new NotFoundException("Could not find role: " + roleName);
}
updateRole(rep, role);
try {
updateRole(rep, role);
return Response.noContent().build();
} catch (ModelDuplicateException e) {
return Flows.errors().exists("Role with name " + rep.getName() + " already exists");
}
}
@Path("{role-name}/composites")
@ -120,7 +122,7 @@ public class RoleContainerResource extends RoleResource {
auth.requireManage();
RoleModel role = roleContainer.getRole(roleName);
if (role == null || role.getName().startsWith(Constants.INTERNAL_ROLE)) {
if (role == null) {
throw new NotFoundException("Could not find role: " + roleName);
}
addComposites(roles, role);
@ -134,7 +136,7 @@ public class RoleContainerResource extends RoleResource {
auth.requireManage();
RoleModel role = roleContainer.getRole(roleName);
if (role == null || role.getName().startsWith(Constants.INTERNAL_ROLE)) {
if (role == null) {
throw new NotFoundException("Could not find role: " + roleName);
}
return getRoleComposites(role);
@ -148,7 +150,7 @@ public class RoleContainerResource extends RoleResource {
auth.requireManage();
RoleModel role = roleContainer.getRole(roleName);
if (role == null || role.getName().startsWith(Constants.INTERNAL_ROLE)) {
if (role == null) {
throw new NotFoundException("Could not find role: " + roleName);
}
return getRealmRoleComposites(role);
@ -163,7 +165,7 @@ public class RoleContainerResource extends RoleResource {
auth.requireManage();
RoleModel role = roleContainer.getRole(roleName);
if (role == null || role.getName().startsWith(Constants.INTERNAL_ROLE)) {
if (role == null) {
throw new NotFoundException("Could not find role: " + roleName);
}
return getApplicationRoleComposites(appName, role);
@ -177,7 +179,7 @@ public class RoleContainerResource extends RoleResource {
auth.requireManage();
RoleModel role = roleContainer.getRole(roleName);
if (role == null || role.getName().startsWith(Constants.INTERNAL_ROLE)) {
if (role == null) {
throw new NotFoundException("Could not find role: " + roleName);
}
deleteComposites(roles, role);