Author: shane.bryzak(a)jboss.com Date: 2008-05-10 09:33:34 -0400 (Sat, 10 May 2008) New Revision: 8150 Modified: trunk/src/main/org/jboss/seam/annotations/security/permission/Permission.java trunk/src/main/org/jboss/seam/security/permission/JpaPermissionStore.java trunk/src/main/org/jboss/seam/security/permission/PermissionMetadata.java trunk/src/main/org/jboss/seam/security/permission/action/PermissionSearch.java Log: made bulk permission updates way more efficient Modified: trunk/src/main/org/jboss/seam/annotations/security/permission/Permission.java =================================================================== --- trunk/src/main/org/jboss/seam/annotations/security/permission/Permission.java 2008-05-10 02:39:00 UTC (rev 8149) +++ trunk/src/main/org/jboss/seam/annotations/security/permission/Permission.java 2008-05-10 13:33:34 UTC (rev 8150) @@ -21,5 +21,5 @@ public @interface Permission { String action(); - long mask() default 0; + long mask() default 0L; } Modified: trunk/src/main/org/jboss/seam/security/permission/JpaPermissionStore.java =================================================================== --- trunk/src/main/org/jboss/seam/security/permission/JpaPermissionStore.java 2008-05-10 02:39:00 UTC (rev 8149) +++ trunk/src/main/org/jboss/seam/security/permission/JpaPermissionStore.java 2008-05-10 13:33:34 UTC (rev 8150) @@ -9,6 +9,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import javax.persistence.EntityManager; import javax.persistence.NoResultException; @@ -33,6 +34,7 @@ import org.jboss.seam.security.SimplePrincipal; import org.jboss.seam.security.management.JpaIdentityStore; import org.jboss.seam.util.AnnotatedBeanProperty; +import org.jboss.seam.security.permission.PermissionMetadata.ActionSet; /** * A permission store implementation that uses JPA as its persistence mechanism. @@ -154,15 +156,13 @@ } } - protected Query createPermissionQuery(Object target, String action, Principal recipient, - Discrimination discrimination) + protected Query createPermissionQuery(Object target, Principal recipient, Discrimination discrimination) { int queryKey = ((target != null) ? 1 : 0); - queryKey |= (action != null ? 2 : 0); - queryKey |= (recipient != null ? 4 : 0); - queryKey |= (discrimination.equals(Discrimination.user) ? 8 : 0); - queryKey |= (discrimination.equals(Discrimination.role) ? 16 : 0); - queryKey |= (discrimination.equals(Discrimination.either) ? 32 : 0); + queryKey |= (recipient != null ? 2 : 0); + queryKey |= (discrimination.equals(Discrimination.user) ? 4 : 0); + queryKey |= (discrimination.equals(Discrimination.role) ? 8 : 0); + queryKey |= (discrimination.equals(Discrimination.either) ? 16 : 0); boolean isRole = discrimination.equals(Discrimination.role) && rolePermissionClass != null; @@ -183,14 +183,6 @@ conditionsAdded = true; } - if (action != null) - { - q.append(conditionsAdded ? " and " : " where "); - q.append(isRole ? roleActionProperty.getName() : actionProperty.getName()); - q.append(" = :action"); - conditionsAdded = true; - } - if (recipient != null) { q.append(conditionsAdded ? " and " : " where "); @@ -214,7 +206,6 @@ Query query = lookupEntityManager().createQuery(queryCache.get(queryKey)); if (target != null) query.setParameter("target", identifierPolicy.getIdentifier(target)); - if (action != null) query.setParameter("action", action); if (recipient != null) query.setParameter("recipient", resolvePrincipal(recipient)); if (!discrimination.equals(Discrimination.either) && discriminatorProperty != null) @@ -228,19 +219,98 @@ public boolean grantPermission(Permission permission) { - boolean recipientIsRole = permission.getRecipient() instanceof Role; + return updatePermissionActions(permission.getTarget(), permission.getRecipient(), + new String[] {permission.getAction()}, true); + } + + public boolean revokePermission(Permission permission) + { + return updatePermissionActions(permission.getTarget(), permission.getRecipient(), + new String[] { permission.getAction() }, false); + } + /** + * This is where the bulk of the actual work happens. + * + * @param target The target object to update permissions for + * @param recipient The recipient to update permissions for + * @param actions The actions that will be updated + * @param set true if the specified actions are to be granted, false if they are to be revoked + * @return true if the operation is successful + */ + protected boolean updatePermissionActions(Object target, Principal recipient, String[] actions, + boolean set) + { + boolean recipientIsRole = recipient instanceof Role; + try { if (recipientIsRole) { if (rolePermissionClass != null) { - Object instance = rolePermissionClass.newInstance(); - roleTargetProperty.setValue(instance, identifierPolicy.getIdentifier(permission.getTarget())); - roleActionProperty.setValue(instance, permission.getAction()); - roleProperty.setValue(instance, permission.getRecipient().getName()); - lookupEntityManager().persist(instance); + List permissions = createPermissionQuery(target, recipient, Discrimination.role).getResultList(); + + if (permissions.isEmpty()) + { + if (!set) return true; + + ActionSet actionSet = metadata.createActionSet(target.getClass(), null); + for (String action : actions) + { + actionSet.add(action); + } + + Object instance = rolePermissionClass.newInstance(); + roleTargetProperty.setValue(instance, identifierPolicy.getIdentifier(target)); + roleActionProperty.setValue(instance, actionSet.toString()); + roleProperty.setValue(instance, resolvePrincipal(recipient)); + lookupEntityManager().persist(instance); + return true; + } + + Object instance = permissions.get(0); + + ActionSet actionSet = metadata.createActionSet(target.getClass(), + roleActionProperty.getValue(instance).toString()); + + for (String action : actions) + { + if (set) + { + actionSet.add(action); + } + else + { + actionSet.remove(action); + } + } + + if (permissions.size() > 1) + { + // This is where it gets a little messy.. if there is more than one permission + // record, then we need to consolidate them all into just the first one + for (Object p : permissions) + { + actionSet.addMembers(roleActionProperty.getValue(p).toString()); + if (!p.equals(instance)) + { + lookupEntityManager().remove(p); + } + } + } + + if (!actionSet.isEmpty()) + { + roleActionProperty.setValue(instance, actionSet.toString()); + lookupEntityManager().merge(instance); + } + else + { + // No actions remaining in set, so just remove the record + lookupEntityManager().remove(instance); + } + return true; } @@ -254,64 +324,167 @@ { throw new RuntimeException("Could not grant permission, userPermissionClass not set"); } - - Object instance = userPermissionClass.newInstance(); - targetProperty.setValue(instance, identifierPolicy.getIdentifier(permission.getTarget())); - actionProperty.setValue(instance, permission.getAction()); - userProperty.setValue(instance, resolvePrincipal(permission.getRecipient())); + + List permissions = createPermissionQuery(target, recipient, recipientIsRole ? + Discrimination.role : Discrimination.user).getResultList(); + + if (permissions.isEmpty()) + { + if (!set) return true; + + ActionSet actionSet = metadata.createActionSet(target.getClass(), null); + for (String action : actions) + { + actionSet.add(action); + } + + Object instance = userPermissionClass.newInstance(); + targetProperty.setValue(instance, identifierPolicy.getIdentifier(target)); + actionProperty.setValue(instance, actionSet.toString()); + userProperty.setValue(instance, resolvePrincipal(recipient)); + + if (discriminatorProperty != null) + { + PermissionDiscriminator discriminator = discriminatorProperty.getAnnotation(); + discriminatorProperty.setValue(instance, recipientIsRole ? discriminator.roleValue() : + discriminator.userValue()); + } + + lookupEntityManager().persist(instance); + return true; + } + + Object instance = permissions.get(0); - if (discriminatorProperty != null) + ActionSet actionSet = metadata.createActionSet(target.getClass(), + actionProperty.getValue(instance).toString()); + + for (String action : actions) { - discriminatorProperty.setValue(instance, getDiscriminatorValue(recipientIsRole)); + if (set) + { + actionSet.add(action); + } + else + { + actionSet.remove(action); + } } - lookupEntityManager().persist(instance); + if (permissions.size() > 1) + { + // Same as with roles, consolidate the records if there is more than one + for (Object p : permissions) + { + actionSet.addMembers(actionProperty.getValue(p).toString()); + if (!p.equals(instance)) + { + lookupEntityManager().remove(p); + } + } + } + + if (!actionSet.isEmpty()) + { + actionProperty.setValue(instance, actionSet.toString()); + lookupEntityManager().merge(instance); + } + else + { + // No actions remaining in set, so just remove the record + lookupEntityManager().remove(instance); + } - return true; + return true; } catch (Exception ex) { throw new RuntimeException("Could not grant permission", ex); - } + } } public boolean grantPermissions(List<Permission> permissions) { - // TODO implement + // Target/Recipient/Action map + Map<Object,Map<Principal,List<Permission>>> groupedPermissions = groupPermissions(permissions); - return false; + for (Object target : groupedPermissions.keySet()) + { + Map<Principal,List<Permission>> recipientPermissions = groupedPermissions.get(target); + + for (Principal recipient : recipientPermissions.keySet()) + { + List<Permission> ps = recipientPermissions.get(recipient); + String[] actions = new String[ps.size()]; + for (int i = 0; i < ps.size(); i++) actions[i] = ps.get(i).getAction(); + updatePermissionActions(target, recipient, actions, true); + } + } + + return true; } - private String getDiscriminatorValue(boolean isRole) + public boolean revokePermissions(List<Permission> permissions) { - PermissionDiscriminator discriminator = discriminatorProperty.getAnnotation(); - return isRole ? discriminator.roleValue() : discriminator.userValue(); - } + // Target/Recipient/Action map + Map<Object,Map<Principal,List<Permission>>> groupedPermissions = groupPermissions(permissions); + + for (Object target : groupedPermissions.keySet()) + { + Map<Principal,List<Permission>> recipientPermissions = groupedPermissions.get(target); + + for (Principal recipient : recipientPermissions.keySet()) + { + List<Permission> ps = recipientPermissions.get(recipient); + String[] actions = new String[ps.size()]; + for (int i = 0; i < ps.size(); i++) actions[i] = ps.get(i).getAction(); + updatePermissionActions(target, recipient, actions, false); + } + } + + return true; + } - public boolean revokePermission(Permission permission) + /** + * Groups a list of arbitrary permissions into a more easily-consumed structure + * + * @param permissions The list of permissions to group + * @return + */ + private Map<Object,Map<Principal,List<Permission>>> groupPermissions(List<Permission> permissions) { - Query qry = createPermissionQuery(permission.getTarget(), permission.getAction(), - permission.getRecipient(), permission.getRecipient() instanceof Role ? - Discrimination.role : Discrimination.user); - - try + // Target/Recipient/Action map + Map<Object,Map<Principal,List<Permission>>> groupedPermissions = new HashMap<Object,Map<Principal,List<Permission>>>(); + + for (Permission permission : permissions) { - Object instance = qry.getSingleResult(); - lookupEntityManager().remove(instance); - return true; + if (!groupedPermissions.containsKey(permission.getTarget())) + { + groupedPermissions.put(permission.getTarget(), new HashMap<Principal,List<Permission>>()); + } + + Map<Principal,List<Permission>> recipientPermissions = groupedPermissions.get(permission.getTarget()); + if (!recipientPermissions.containsKey(permission.getRecipient())) + { + List<Permission> perms = new ArrayList<Permission>(); + perms.add(permission); + recipientPermissions.put(permission.getRecipient(), perms); + } + else + { + recipientPermissions.get(permission.getRecipient()).add(permission); + } } - catch (NoResultException ex) - { - return false; - } + + return groupedPermissions; } - public boolean revokePermissions(List<Permission> permissions) + private String getDiscriminatorValue(boolean isRole) { - // TODO implement - return false; + PermissionDiscriminator discriminator = discriminatorProperty.getAnnotation(); + return isRole ? discriminator.roleValue() : discriminator.userValue(); } - + /** * If the user or role properties in the entity class refer to other entities, then this method * uses the JpaIdentityStore (if available) to lookup that user or role entity. Otherwise it @@ -375,7 +548,7 @@ List<Permission> permissions = new ArrayList<Permission>(); // First query for user permissions - Query permissionQuery = createPermissionQuery(target, action, null, Discrimination.either); + Query permissionQuery = createPermissionQuery(target, null, Discrimination.either); List userPermissions = permissionQuery.getResultList(); Map<String,Principal> principalCache = new HashMap<String,Principal>(); @@ -384,50 +557,28 @@ for (Object permission : userPermissions) { - Principal principal; - boolean isUser = true; + ActionSet actionSet = metadata.createActionSet(target.getClass(), + actionProperty.getValue(permission).toString()); - if (useDiscriminator && - discriminatorProperty.getAnnotation().roleValue().equals(discriminatorProperty.getValue(permission))) - { - isUser = false; - } - - String name = resolvePrincipalName(isUser ? userProperty.getValue(permission) : - roleProperty.getValue(permission), isUser); - - String key = (isUser ? "u:" : "r:") + name; - - if (!principalCache.containsKey(key)) - { - principal = isUser ? new SimplePrincipal(name) : new Role(name); - principalCache.put(key, principal); - } - else - { - principal = principalCache.get(key); - } - - permissions.add(new Permission(target, (String) (action != null ? action : actionProperty.getValue(permission)), - principal)); - } - - // If we have a separate class for role permissions, then query them now - if (rolePermissionClass != null) - { - permissionQuery = createPermissionQuery(target, action, null, Discrimination.role); - List rolePermissions = permissionQuery.getResultList(); - - for (Object permission : rolePermissions) - { + if (action == null || actionSet.contains(action)) + { Principal principal; + boolean isUser = true; - String name = resolvePrincipalName(roleProperty.getValue(permission), false); - String key = "r:" + name; + if (useDiscriminator && + discriminatorProperty.getAnnotation().roleValue().equals(discriminatorProperty.getValue(permission))) + { + isUser = false; + } + + String name = resolvePrincipalName(isUser ? userProperty.getValue(permission) : + roleProperty.getValue(permission), isUser); + String key = (isUser ? "u:" : "r:") + name; + if (!principalCache.containsKey(key)) { - principal = new Role(name); + principal = isUser ? new SimplePrincipal(name) : new Role(name); principalCache.put(key, principal); } else @@ -435,11 +586,63 @@ principal = principalCache.get(key); } - permissions.add(new Permission(target, (String) (action != null ? action : - roleActionProperty.getValue(permission)), principal)); + if (action != null) + { + permissions.add(new Permission(target, action, principal)); + } + else + { + for (String a : actionSet.members()) + { + permissions.add(new Permission(target, a, principal)); + } + } } } + // If we have a separate class for role permissions, then query them now + if (rolePermissionClass != null) + { + permissionQuery = createPermissionQuery(target, null, Discrimination.role); + List rolePermissions = permissionQuery.getResultList(); + + for (Object permission : rolePermissions) + { + ActionSet actionSet = metadata.createActionSet(target.getClass(), + roleActionProperty.getValue(permission).toString()); + + if (action == null || actionSet.contains(action)) + { + Principal principal; + + String name = resolvePrincipalName(roleProperty.getValue(permission), false); + String key = "r:" + name; + + if (!principalCache.containsKey(key)) + { + principal = new Role(name); + principalCache.put(key, principal); + } + else + { + principal = principalCache.get(key); + } + + if (action != null) + { + permissions.add(new Permission(target, action, principal)); + } + else + { + for (String a : actionSet.members()) + { + permissions.add(new Permission(target, a, principal)); + } + } + } + } + } + return permissions; } Modified: trunk/src/main/org/jboss/seam/security/permission/PermissionMetadata.java =================================================================== --- trunk/src/main/org/jboss/seam/security/permission/PermissionMetadata.java 2008-05-10 02:39:00 UTC (rev 8149) +++ trunk/src/main/org/jboss/seam/security/permission/PermissionMetadata.java 2008-05-10 13:33:34 UTC (rev 8150) @@ -3,6 +3,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; @@ -83,7 +84,7 @@ } } - private class ActionSet + protected class ActionSet { private Set<String> members = new HashSet<String>(); private Class targetClass; @@ -91,6 +92,12 @@ public ActionSet(Class targetClass, String members) { this.targetClass = targetClass; + addMembers(members); + } + + public void addMembers(String members) + { + if (members == null) return; if (usesActionMask.get(targetClass)) { @@ -115,9 +122,14 @@ { this.members.add(action); } - } + } } + public boolean contains(String action) + { + return members.contains(action); + } + public ActionSet add(String action) { members.add(action); @@ -130,6 +142,16 @@ return this; } + public Set<String> members() + { + return members; + } + + public boolean isEmpty() + { + return members.isEmpty(); + } + @Override public String toString() { Modified: trunk/src/main/org/jboss/seam/security/permission/action/PermissionSearch.java =================================================================== --- trunk/src/main/org/jboss/seam/security/permission/action/PermissionSearch.java 2008-05-10 02:39:00 UTC (rev 8149) +++ trunk/src/main/org/jboss/seam/security/permission/action/PermissionSearch.java 2008-05-10 13:33:34 UTC (rev 8150) @@ -3,7 +3,11 @@ import static org.jboss.seam.ScopeType.CONVERSATION; import java.io.Serializable; +import java.security.Principal; +import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.jboss.seam.annotations.Begin; import org.jboss.seam.annotations.In; @@ -19,11 +23,13 @@ @Name("org.jboss.seam.security.permission.permissionSearch") public class PermissionSearch implements Serializable { + private Map<Principal,List<Permission>> groupedPermissions = new HashMap<Principal,List<Permission>>(); + @DataModel - List<Permission> permissions; + List<Principal> recipients; @DataModelSelection - Permission selectedPermission; + Principal selectedRecipient; @In IdentityManager identityManager; @@ -39,16 +45,60 @@ public void refresh() { - permissions = permissionManager.listPermissions(target); + List<Permission> permissions = permissionManager.listPermissions(target); + groupedPermissions.clear(); + + for (Permission permission : permissions) + { + List<Permission> recipientPermissions = null; + + if (!groupedPermissions.containsKey(permission.getRecipient())) + { + recipientPermissions = new ArrayList<Permission>(); + groupedPermissions.put(permission.getRecipient(), recipientPermissions); + } + else + { + recipientPermissions = groupedPermissions.get(permission.getRecipient()); + } + + recipientPermissions.add(permission); + } + + recipients = new ArrayList<Principal>(groupedPermissions.keySet()); } + public String getActions(Principal recipient) + { + StringBuilder sb = new StringBuilder(); + + for (Permission permission : groupedPermissions.get(recipient)) + { + if (sb.length() > 0) sb.append(", "); + sb.append(permission.getAction()); + } + + return sb.toString(); + } + public Object getTarget() { return target; } - public Permission getSelectedPermission() + public void revokeSelected() { - return selectedPermission; + permissionManager.revokePermissions(getSelectedPermissions()); + refresh(); } + + public Principal getSelectedRecipient() + { + return selectedRecipient; + } + + public List<Permission> getSelectedPermissions() + { + return groupedPermissions.get(selectedRecipient); + } }