[seam-commits] Seam SVN: r8115 - in trunk/src/main/org/jboss/seam/security/permission: action and 1 other directory.

seam-commits at lists.jboss.org seam-commits at lists.jboss.org
Mon May 5 09:53:54 EDT 2008


Author: shane.bryzak at 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);
    }
    




More information about the seam-commits mailing list