Author: shane.bryzak(a)jboss.com Date: 2008-05-05 09:53:54 -0400 (Mon, 05 May 2008) New Revision: 8115 Modified: trunk/src/main/org/jboss/seam/security/permission/JpaPermissionStore.java trunk/src/main/org/jboss/seam/security/permission/PermissionManager.java trunk/src/main/org/jboss/seam/security/permission/action/PermissionSearch.java Log: fix permission check, optimized queries Modified: trunk/src/main/org/jboss/seam/security/permission/JpaPermissionStore.java =================================================================== --- trunk/src/main/org/jboss/seam/security/permission/JpaPermissionStore.java 2008-05-05 09:02:56 UTC (rev 8114) +++ trunk/src/main/org/jboss/seam/security/permission/JpaPermissionStore.java 2008-05-05 13:53:54 UTC (rev 8115) @@ -45,8 +45,10 @@ @BypassInterceptors public class JpaPermissionStore implements PermissionStore, Serializable { - private static final LogProvider log = Logging.getLogProvider(JpaPermissionStore.class); + private static final LogProvider log = Logging.getLogProvider(JpaPermissionStore.class); + private enum Discrimination { user, role, either } + private ValueExpression<EntityManager> entityManager; private Class userPermissionClass; @@ -68,7 +70,8 @@ @Create public void init() - { + { + // TODO see if we can scan for this automatically if (userPermissionClass == null) { log.debug("No permissionClass set, JpaDynamicPermissionStore will be unavailable."); @@ -147,28 +150,31 @@ } } - protected Query createPermissionQuery(Object target, String action, Principal recipient, boolean isRole) + protected Query createPermissionQuery(Object target, String action, Principal recipient, + Discrimination discrimination) { int queryKey = ((target != null) ? 1 : 0); queryKey |= (action != null ? 2 : 0); queryKey |= (recipient != null ? 4 : 0); - queryKey |= (isRole ? 8 : 0); + queryKey |= (discrimination.equals(Discrimination.user) ? 8 : 0); + queryKey |= (discrimination.equals(Discrimination.role) ? 16 : 0); + queryKey |= (discrimination.equals(Discrimination.either) ? 32 : 0); + boolean isRole = discrimination.equals(Discrimination.role) && rolePermissionClass != null; + if (!queryCache.containsKey(queryKey)) { boolean conditionsAdded = false; StringBuilder q = new StringBuilder(); q.append("select p from "); - q.append(isRole && rolePermissionClass != null ? rolePermissionClass.getName() : - userPermissionClass.getName()); + q.append(isRole ? rolePermissionClass.getName() : userPermissionClass.getName()); q.append(" p"); if (target != null) { q.append(" where "); - q.append(isRole && rolePermissionClass != null ? roleTargetProperty.getName() : - targetProperty.getName()); + q.append(isRole ? roleTargetProperty.getName() : targetProperty.getName()); q.append(" = :target"); conditionsAdded = true; } @@ -176,8 +182,7 @@ if (action != null) { q.append(conditionsAdded ? " and " : " where "); - q.append(isRole && rolePermissionClass != null ? roleActionProperty.getName() : - actionProperty.getName()); + q.append(isRole ? roleActionProperty.getName() : actionProperty.getName()); q.append(" = :action"); conditionsAdded = true; } @@ -185,13 +190,13 @@ if (recipient != null) { q.append(conditionsAdded ? " and " : " where "); - q.append(isRole && rolePermissionClass != null ? roleProperty.getName() : - userProperty.getName()); + q.append(isRole ? roleProperty.getName() : userProperty.getName()); q.append(" = :recipient"); conditionsAdded = true; } - if (discriminatorProperty != null) + // If there is no discrimination, then don't add such a condition to the query + if (!discrimination.equals(Discrimination.either) && discriminatorProperty != null) { q.append(conditionsAdded ? " and " : " where "); q.append(discriminatorProperty.getName()); @@ -207,8 +212,13 @@ 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 (discriminatorProperty != null) query.setParameter("discriminator", getDiscriminatorValue(isRole)); + if (!discrimination.equals(Discrimination.either) && discriminatorProperty != null) + { + query.setParameter("discriminator", getDiscriminatorValue( + discrimination.equals(Discrimination.role))); + } + return query; } @@ -270,7 +280,8 @@ public boolean revokePermission(Permission permission) { Query qry = createPermissionQuery(permission.getTarget(), permission.getAction(), - permission.getRecipient(), permission.getRecipient() instanceof Role); + permission.getRecipient(), permission.getRecipient() instanceof Role ? + Discrimination.role : Discrimination.user); try { @@ -339,11 +350,15 @@ throw new IllegalArgumentException("Cannot resolve principal name for principal " + principal); } + /** + * Returns a list of all user and role permissions for a specific permission target and action. + */ public List<Permission> listPermissions(Object target, String action) { List<Permission> permissions = new ArrayList<Permission>(); - Query permissionQuery = createPermissionQuery(target, action, null, false); + // First query for user permissions + Query permissionQuery = createPermissionQuery(target, action, null, Discrimination.either); List userPermissions = permissionQuery.getResultList(); Map<String,Principal> principalCache = new HashMap<String,Principal>(); @@ -364,7 +379,7 @@ String name = resolvePrincipalName(isUser ? userProperty.getValue(permission) : roleProperty.getValue(permission), isUser); - String key = (isUser ? "user:" : "role:") + name; + String key = (isUser ? "u:" : "r:") + name; if (!principalCache.containsKey(key)) { @@ -380,9 +395,10 @@ principal)); } - if (rolePermissionClass == null) + // If we have a separate class for role permissions, then query them now + if (rolePermissionClass != null) { - permissionQuery = createPermissionQuery(target, action, null, true); + permissionQuery = createPermissionQuery(target, action, null, Discrimination.role); List rolePermissions = permissionQuery.getResultList(); for (Object permission : rolePermissions) @@ -390,7 +406,7 @@ Principal principal; String name = resolvePrincipalName(roleProperty.getValue(permission), false); - String key = "role:" + name; + String key = "r:" + name; if (!principalCache.containsKey(key)) { Modified: trunk/src/main/org/jboss/seam/security/permission/PermissionManager.java =================================================================== --- trunk/src/main/org/jboss/seam/security/permission/PermissionManager.java 2008-05-05 09:02:56 UTC (rev 8114) +++ trunk/src/main/org/jboss/seam/security/permission/PermissionManager.java 2008-05-05 13:53:54 UTC (rev 8115) @@ -85,25 +85,27 @@ public List<Permission> listPermissions(String target, String action) { + if (target == null) return null; Identity.instance().checkPermission(target, PERMISSION_READ); return permissionStore.listPermissions(target, action); } public List<Permission> listPermissions(Object target) { + if (target == null) return null; Identity.instance().checkPermission(target, PERMISSION_READ); return permissionStore.listPermissions(target); } public boolean grantPermission(Permission permission) { - Identity.instance().checkPermission(permission, PERMISSION_GRANT); + Identity.instance().checkPermission(permission.getTarget(), PERMISSION_GRANT); return permissionStore.grantPermission(permission); } public boolean revokePermission(Permission permission) { - Identity.instance().checkPermission(permission, PERMISSION_REVOKE); + Identity.instance().checkPermission(permission.getTarget(), PERMISSION_REVOKE); return permissionStore.revokePermission(permission); } } 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-05 09:02:56 UTC (rev 8114) +++ trunk/src/main/org/jboss/seam/security/permission/action/PermissionSearch.java 2008-05-05 13:53:54 UTC (rev 8115) @@ -32,9 +32,13 @@ private Object target; @Begin - public void loadPermissions(Object target) + public void search(Object target) { this.target = target; + } + + public void refresh() + { permissions = permissionManager.listPermissions(target); }