[jboss-cvs] Picketbox SVN: r65 - in trunk: security-jboss-sx/identity/src/test/java/org/jboss/test/identity/impl and 8 other directories.

jboss-cvs-commits at lists.jboss.org jboss-cvs-commits at lists.jboss.org
Fri Mar 26 21:17:57 EDT 2010


Author: sguilhen at redhat.com
Date: 2010-03-26 21:17:55 -0400 (Fri, 26 Mar 2010)
New Revision: 65

Modified:
   trunk/security-jboss-sx/identity/src/main/java/org/jboss/security/identity/plugins/SimpleRoleGroup.java
   trunk/security-jboss-sx/identity/src/test/java/org/jboss/test/identity/impl/RoleUnitTestCase.java
   trunk/security-jboss-sx/jbosssx/src/main/java/org/jboss/security/jacc/ContextPolicy.java
   trunk/security-jboss-sx/jbosssx/src/main/java/org/jboss/security/mapping/providers/DeploymentRolesMappingProvider.java
   trunk/security-jboss-sx/jbosssx/src/main/java/org/jboss/security/plugins/JBossAuthorizationManager.java
   trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/authorization/StandaloneJBossAMgrUnitTestCase.java
   trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/authorization/ejb/EJBPolicyModuleDelegateUnitTestCase.java
   trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/authorization/xacml/EJBXACMLUnitTestCase.java
   trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/authorization/xacml/WebXACMLUnitTestCase.java
   trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/util/SecurityTestUtil.java
   trunk/security-spi/identity/src/main/java/org/jboss/security/identity/RoleGroup.java
Log:
SECURITY-477: Access to the roles list of SimpleRoleGroup is now synchronized to avoid ConcurrentModificationExceptions.

Modified: trunk/security-jboss-sx/identity/src/main/java/org/jboss/security/identity/plugins/SimpleRoleGroup.java
===================================================================
--- trunk/security-jboss-sx/identity/src/main/java/org/jboss/security/identity/plugins/SimpleRoleGroup.java	2010-03-22 17:10:24 UTC (rev 64)
+++ trunk/security-jboss-sx/identity/src/main/java/org/jboss/security/identity/plugins/SimpleRoleGroup.java	2010-03-27 01:17:55 UTC (rev 65)
@@ -24,6 +24,7 @@
 import java.security.Principal;
 import java.security.acl.Group;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Enumeration;
 import java.util.List;
 import java.util.Set;
@@ -56,8 +57,6 @@
    public SimpleRoleGroup(String roleName, List<Role> roles)
    {
       super(roleName);
-      if (this.roles == null)
-         this.roles = new ArrayList<Role>();
       this.roles.addAll(roles);
    }
 
@@ -80,44 +79,67 @@
       }
    }
 
+   /*
+    * (non-Javadoc)
+    * @see org.jboss.security.identity.plugins.SimpleRole#getType()
+    */
    @Override
    public RoleType getType()
    {
       return RoleType.group;
    }
 
-   /**
-    * @see RoleGroup#addRole(Role)
+   /*
+    * (non-Javadoc)
+    * @see org.jboss.security.identity.RoleGroup#addRole(org.jboss.security.identity.Role)
     */
-   public void addRole(Role role)
+   public synchronized void addRole(Role role)
    {
       this.roles.add(role);
    }
 
-   /**
-    * @see RoleGroup#removeRole(Role)
+   /*
+    * (non-Javadoc)
+    * @see org.jboss.security.identity.RoleGroup#addAll(java.util.List)
     */
-   public void removeRole(Role role)
+   public synchronized void addAll(List<Role> roles)
    {
+      if (roles != null)
+         this.roles.addAll(roles);
+   }
+
+   /*
+    * (non-Javadoc)
+    * @see org.jboss.security.identity.RoleGroup#removeRole(org.jboss.security.identity.Role)
+    */
+   public synchronized void removeRole(Role role)
+   {
       this.roles.remove(role);
    }
 
-   /**
-    * @see RoleGroup#clearRoles()
+   /*
+    * (non-Javadoc)
+    * @see org.jboss.security.identity.RoleGroup#clearRoles()
     */
-   public void clearRoles()
+   public synchronized void clearRoles()
    {
       this.roles.clear();
    }
 
-   /**
-    * @see RoleGroup#getRoles()
+   /*
+    * (non-Javadoc)
+    * @see org.jboss.security.identity.RoleGroup#getRoles()
     */
    public List<Role> getRoles()
    {
-      return roles;
+      // unmodifiable view: clients must update the roles through the addRole and removeRole methods.
+      return Collections.unmodifiableList(roles);
    }
 
+   /*
+    * (non-Javadoc)
+    * @see org.jboss.security.identity.plugins.SimpleRole#clone()
+    */
    @SuppressWarnings("unchecked")
    public synchronized Object clone() throws CloneNotSupportedException
    {
@@ -127,6 +149,10 @@
       return clone;
    }
 
+   /*
+    * (non-Javadoc)
+    * @see org.jboss.security.identity.plugins.SimpleRole#containsAll(org.jboss.security.identity.Role)
+    */
    @Override
    public boolean containsAll(Role anotherRole)
    {
@@ -134,11 +160,15 @@
 
       if (anotherRole.getType() == RoleType.simple)
       {
-         for (Role r : roles)
+         // synchronize iteration to avoid concurrent modification exception.
+         synchronized (this)
          {
-            isContained = r.containsAll(anotherRole);
-            if (isContained)
-               return true;
+            for (Role r : roles)
+            {
+               isContained = r.containsAll(anotherRole);
+               if (isContained)
+                  return true;
+            }
          }
       }
       else
@@ -157,8 +187,9 @@
       return false;
    }
 
-   /**
-    * @see RoleGroup#containsAtleastOneRole(RoleGroup)
+   /*
+    * (non-Javadoc)
+    * @see org.jboss.security.identity.RoleGroup#containsAtleastOneRole(org.jboss.security.identity.RoleGroup)
     */
    public boolean containsAtleastOneRole(RoleGroup anotherRole)
    {
@@ -173,11 +204,13 @@
       return false;
    }
 
-   /**
-    * @see RoleGroup#containsRole(Role)
+   /*
+    * (non-Javadoc)
+    * @see org.jboss.security.identity.RoleGroup#containsRole(org.jboss.security.identity.Role)
     */
-   public boolean containsRole(Role role)
+   public synchronized boolean containsRole(Role role)
    {
+      // synchronize iteration to avoid concurrent modification exception.
       for (Role r : roles)
       {
          if (r.containsAll(role))
@@ -186,6 +219,10 @@
       return false;
    }
 
+   /*
+    * (non-Javadoc)
+    * @see org.jboss.security.identity.plugins.SimpleRole#toString()
+    */
    @Override
    public String toString()
    {

Modified: trunk/security-jboss-sx/identity/src/test/java/org/jboss/test/identity/impl/RoleUnitTestCase.java
===================================================================
--- trunk/security-jboss-sx/identity/src/test/java/org/jboss/test/identity/impl/RoleUnitTestCase.java	2010-03-22 17:10:24 UTC (rev 64)
+++ trunk/security-jboss-sx/identity/src/test/java/org/jboss/test/identity/impl/RoleUnitTestCase.java	2010-03-27 01:17:55 UTC (rev 65)
@@ -57,13 +57,13 @@
    public void testSimpleRoleGroupContains() throws Exception
    {
       SimpleRoleGroup firstRoleGroup = new SimpleRoleGroup("firstrg");
-      firstRoleGroup.getRoles().add(new SimpleRole("A"));
-      firstRoleGroup.getRoles().add(new SimpleRole("B"));
-      firstRoleGroup.getRoles().add(new SimpleRole("C"));
+      firstRoleGroup.addRole(new SimpleRole("A"));
+      firstRoleGroup.addRole(new SimpleRole("B"));
+      firstRoleGroup.addRole(new SimpleRole("C"));
 
       SimpleRoleGroup secondRoleGroup = new SimpleRoleGroup("secondrg");
-      secondRoleGroup.getRoles().add(new SimpleRole("A"));
-      secondRoleGroup.getRoles().add(new SimpleRole("B"));
+      secondRoleGroup.addRole(new SimpleRole("A"));
+      secondRoleGroup.addRole(new SimpleRole("B"));
 
       assertTrue(firstRoleGroup.containsAll(firstRoleGroup));
       assertTrue(secondRoleGroup.containsAll(secondRoleGroup));

Modified: trunk/security-jboss-sx/jbosssx/src/main/java/org/jboss/security/jacc/ContextPolicy.java
===================================================================
--- trunk/security-jboss-sx/jbosssx/src/main/java/org/jboss/security/jacc/ContextPolicy.java	2010-03-22 17:10:24 UTC (rev 64)
+++ trunk/security-jboss-sx/jbosssx/src/main/java/org/jboss/security/jacc/ContextPolicy.java	2010-03-27 01:17:55 UTC (rev 65)
@@ -255,7 +255,11 @@
    void removeRole(String roleName)
       throws PolicyContextException
    {
-      rolePermissions.remove(roleName);
+      // JACC 1.4 spec: if "*" is used as the role name and no role by this name exists in the config, remove all roles.
+      if ("*".equals(roleName) && !this.rolePermissions.containsKey("*"))
+         this.rolePermissions.clear();
+      else
+         this.rolePermissions.remove(roleName);
    }
 
    void removeUncheckedPolicy()

Modified: trunk/security-jboss-sx/jbosssx/src/main/java/org/jboss/security/mapping/providers/DeploymentRolesMappingProvider.java
===================================================================
--- trunk/security-jboss-sx/jbosssx/src/main/java/org/jboss/security/mapping/providers/DeploymentRolesMappingProvider.java	2010-03-22 17:10:24 UTC (rev 64)
+++ trunk/security-jboss-sx/jbosssx/src/main/java/org/jboss/security/mapping/providers/DeploymentRolesMappingProvider.java	2010-03-27 01:17:55 UTC (rev 65)
@@ -132,7 +132,7 @@
          }
          
          mappedObject.clearRoles();
-         mappedObject.getRoles().addAll(newRoles.getRoles()); 
+         mappedObject.addAll(newRoles.getRoles()); 
       } 
       return mappedObject;
    } 

Modified: trunk/security-jboss-sx/jbosssx/src/main/java/org/jboss/security/plugins/JBossAuthorizationManager.java
===================================================================
--- trunk/security-jboss-sx/jbosssx/src/main/java/org/jboss/security/plugins/JBossAuthorizationManager.java	2010-03-22 17:10:24 UTC (rev 64)
+++ trunk/security-jboss-sx/jbosssx/src/main/java/org/jboss/security/plugins/JBossAuthorizationManager.java	2010-03-27 01:17:55 UTC (rev 65)
@@ -509,7 +509,7 @@
       Enumeration<? extends Principal> principals = roleGroup.members();
       while(principals.hasMoreElements())
       {
-         srg.getRoles().add(new SimpleRole(principals.nextElement().getName()));
+         srg.addRole(new SimpleRole(principals.nextElement().getName()));
       }
       return srg;  
    }

Modified: trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/authorization/StandaloneJBossAMgrUnitTestCase.java
===================================================================
--- trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/authorization/StandaloneJBossAMgrUnitTestCase.java	2010-03-22 17:10:24 UTC (rev 64)
+++ trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/authorization/StandaloneJBossAMgrUnitTestCase.java	2010-03-27 01:17:55 UTC (rev 65)
@@ -100,7 +100,7 @@
    private RoleGroup getRoleGroup()
    {
       RoleGroup rg = new SimpleRoleGroup(SecurityConstants.ROLES_IDENTIFIER);
-      rg.getRoles().add(new SimpleRole("ServletUserRole")); 
+      rg.addRole(new SimpleRole("ServletUserRole")); 
       return rg;
    }
 }
\ No newline at end of file

Modified: trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/authorization/ejb/EJBPolicyModuleDelegateUnitTestCase.java
===================================================================
--- trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/authorization/ejb/EJBPolicyModuleDelegateUnitTestCase.java	2010-03-22 17:10:24 UTC (rev 64)
+++ trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/authorization/ejb/EJBPolicyModuleDelegateUnitTestCase.java	2010-03-27 01:17:55 UTC (rev 65)
@@ -23,7 +23,6 @@
 
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -38,7 +37,6 @@
 import org.jboss.security.authorization.ResourceKeys;
 import org.jboss.security.authorization.modules.ejb.EJBPolicyModuleDelegate;
 import org.jboss.security.authorization.resources.EJBResource;
-import org.jboss.security.identity.Role;
 import org.jboss.security.identity.RoleGroup;
 import org.jboss.security.identity.plugins.SimpleRole;
 import org.jboss.security.identity.plugins.SimpleRoleGroup;
@@ -272,12 +270,9 @@
    private RoleGroup getRoleGroup(String[] roles)
    {
       SimpleRoleGroup srg = new SimpleRoleGroup(SecurityConstants.ROLES_IDENTIFIER);
-
-      List<Role> roleList = srg.getRoles(); 
-      
       for(String role:roles)
       {
-         roleList.add(new SimpleRole(role));   
+         srg.addRole(new SimpleRole(role));   
       }
       return srg;
    }

Modified: trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/authorization/xacml/EJBXACMLUnitTestCase.java
===================================================================
--- trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/authorization/xacml/EJBXACMLUnitTestCase.java	2010-03-22 17:10:24 UTC (rev 64)
+++ trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/authorization/xacml/EJBXACMLUnitTestCase.java	2010-03-27 01:17:55 UTC (rev 65)
@@ -158,7 +158,7 @@
    private RoleGroup getRoleGroup()
    {
       SimpleRoleGroup srg = new SimpleRoleGroup(SecurityConstants.ROLES_IDENTIFIER);
-      srg.getRoles().add(new SimpleRole("ProjectUser")); 
+      srg.addRole(new SimpleRole("ProjectUser")); 
       return srg;
    } 
    

Modified: trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/authorization/xacml/WebXACMLUnitTestCase.java
===================================================================
--- trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/authorization/xacml/WebXACMLUnitTestCase.java	2010-03-22 17:10:24 UTC (rev 64)
+++ trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/authorization/xacml/WebXACMLUnitTestCase.java	2010-03-27 01:17:55 UTC (rev 65)
@@ -128,7 +128,7 @@
    private RoleGroup getRoleGroup()
    { 
       SimpleRoleGroup srg = new SimpleRoleGroup(SecurityConstants.ROLES_IDENTIFIER);
-      srg.getRoles().add(new SimpleRole("ServletUserRole"));
+      srg.addRole(new SimpleRole("ServletUserRole"));
       return srg;
    } 
    

Modified: trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/util/SecurityTestUtil.java
===================================================================
--- trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/util/SecurityTestUtil.java	2010-03-22 17:10:24 UTC (rev 64)
+++ trunk/security-jboss-sx/jbosssx/src/test/java/org/jboss/test/util/SecurityTestUtil.java	2010-03-27 01:17:55 UTC (rev 65)
@@ -22,7 +22,6 @@
 package org.jboss.test.util;
 
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 
 import org.jboss.security.SecurityConstants;
@@ -32,7 +31,6 @@
 import org.jboss.security.config.ApplicationPolicy;
 import org.jboss.security.config.AuthorizationInfo;
 import org.jboss.security.config.SecurityConfiguration;
-import org.jboss.security.identity.Role;
 import org.jboss.security.identity.RoleGroup;
 import org.jboss.security.identity.plugins.SimpleRole;
 import org.jboss.security.identity.plugins.SimpleRoleGroup;
@@ -50,11 +48,9 @@
    {
       SimpleRoleGroup srg = new SimpleRoleGroup(SecurityConstants.ROLES_IDENTIFIER);
 
-      List<Role> roleList = srg.getRoles(); 
-      
       for(String role:roles)
       {
-         roleList.add(new SimpleRole(role));   
+         srg.addRole(new SimpleRole(role));   
       }
       return srg;
    }
@@ -62,7 +58,7 @@
    public static RoleGroup getRoleGroup(String rolename)
    {
       SimpleRoleGroup srg = new SimpleRoleGroup(SecurityConstants.ROLES_IDENTIFIER);
-      srg.getRoles().add(new SimpleRole(rolename));
+      srg.addRole(new SimpleRole(rolename));
       return srg;
    }
    

Modified: trunk/security-spi/identity/src/main/java/org/jboss/security/identity/RoleGroup.java
===================================================================
--- trunk/security-spi/identity/src/main/java/org/jboss/security/identity/RoleGroup.java	2010-03-22 17:10:24 UTC (rev 64)
+++ trunk/security-spi/identity/src/main/java/org/jboss/security/identity/RoleGroup.java	2010-03-27 01:17:55 UTC (rev 65)
@@ -34,8 +34,12 @@
 public interface RoleGroup extends Role
 {  
    /**
-    * Get the roles contained
-    * @return
+    * <p>
+    * Get the roles contained in the {@code RoleGroup}. The returned {@code List} should be unmodifiable as the
+    * {@code RoleGroup} interface provides methods to add and remove roles.
+    * </p>
+    * 
+    * @return a unmodifiable {@code List} containing the {@code RoleGroup}'s roles.
     */
    public List<Role> getRoles(); 
    
@@ -46,6 +50,15 @@
    public void addRole(Role aRole);
    
    /**
+    * <p>
+    * Adds all specified roles to the role group.
+    * </p>
+    * 
+    * @param roles the list of roles to be added.
+    */
+   public void addAll(List<Role> roles);
+   
+   /**
     * Clear all the roles
     */
    public void clearRoles();




More information about the jboss-cvs-commits mailing list