[jboss-cvs] JBossAS SVN: r103202 - in projects/security/security-jboss-sx/branches/Branch_2_0: identity/src/test/java/org/jboss/test/identity/impl and 7 other directories.

jboss-cvs-commits at lists.jboss.org jboss-cvs-commits at lists.jboss.org
Mon Mar 29 15:22:02 EDT 2010


Author: sguilhen at redhat.com
Date: 2010-03-29 15:22:01 -0400 (Mon, 29 Mar 2010)
New Revision: 103202

Modified:
   projects/security/security-jboss-sx/branches/Branch_2_0/identity/src/main/java/org/jboss/security/identity/plugins/SimpleRoleGroup.java
   projects/security/security-jboss-sx/branches/Branch_2_0/identity/src/test/java/org/jboss/test/identity/impl/RoleUnitTestCase.java
   projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/main/java/org/jboss/security/mapping/providers/DeploymentRolesMappingProvider.java
   projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/main/java/org/jboss/security/plugins/JBossAuthorizationManager.java
   projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/authorization/StandaloneJBossAMgrUnitTestCase.java
   projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/authorization/ejb/EJBPolicyModuleDelegateUnitTestCase.java
   projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/authorization/xacml/EJBXACMLUnitTestCase.java
   projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/authorization/xacml/WebXACMLUnitTestCase.java
   projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/util/SecurityTestUtil.java
   projects/security/security-jboss-sx/branches/Branch_2_0/parent/pom.xml
Log:
SECURITY-483: Synchronized access to SimpleRoleGroup's list of roles to avoid ConcurrentModificationException

Modified: projects/security/security-jboss-sx/branches/Branch_2_0/identity/src/main/java/org/jboss/security/identity/plugins/SimpleRoleGroup.java
===================================================================
--- projects/security/security-jboss-sx/branches/Branch_2_0/identity/src/main/java/org/jboss/security/identity/plugins/SimpleRoleGroup.java	2010-03-29 19:21:21 UTC (rev 103201)
+++ projects/security/security-jboss-sx/branches/Branch_2_0/identity/src/main/java/org/jboss/security/identity/plugins/SimpleRoleGroup.java	2010-03-29 19:22:01 UTC (rev 103202)
@@ -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;
@@ -80,42 +81,61 @@
       }
    }
 
+   /*
+    * (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(this.roles);
    }
 
    @SuppressWarnings("unchecked")
@@ -127,6 +147,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 +158,15 @@
 
       if (anotherRole.getType() == RoleType.simple)
       {
-         for (Role r : roles)
+         // synchronize iteration to avoid concurrent modification exceptions.
+         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 +185,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 +202,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)
    {
+      // synchronized method to avoid concurrent modification exception.
       for (Role r : roles)
       {
          if (r.containsAll(role))
@@ -186,15 +217,23 @@
       return false;
    }
 
+   /*
+    * (non-Javadoc)
+    * @see org.jboss.security.identity.plugins.SimpleRole#toString()
+    */
    @Override
    public String toString()
    {
       StringBuilder builder = new StringBuilder();
       builder.append(this.getRoleName());
       builder.append("(");
-      for (Role role : roles)
+      // synchronize iteration to avoid concurrent modification exception.
+      synchronized (this)
       {
-         builder.append(role.toString()).append(",");
+         for (Role role : roles)
+         {
+            builder.append(role.toString()).append(",");
+         }
       }
       builder.append(")");
       return builder.toString();

Modified: projects/security/security-jboss-sx/branches/Branch_2_0/identity/src/test/java/org/jboss/test/identity/impl/RoleUnitTestCase.java
===================================================================
--- projects/security/security-jboss-sx/branches/Branch_2_0/identity/src/test/java/org/jboss/test/identity/impl/RoleUnitTestCase.java	2010-03-29 19:21:21 UTC (rev 103201)
+++ projects/security/security-jboss-sx/branches/Branch_2_0/identity/src/test/java/org/jboss/test/identity/impl/RoleUnitTestCase.java	2010-03-29 19:22:01 UTC (rev 103202)
@@ -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: projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/main/java/org/jboss/security/mapping/providers/DeploymentRolesMappingProvider.java
===================================================================
--- projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/main/java/org/jboss/security/mapping/providers/DeploymentRolesMappingProvider.java	2010-03-29 19:21:21 UTC (rev 103201)
+++ projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/main/java/org/jboss/security/mapping/providers/DeploymentRolesMappingProvider.java	2010-03-29 19:22:01 UTC (rev 103202)
@@ -132,7 +132,7 @@
          }
          
          mappedObject.clearRoles();
-         mappedObject.getRoles().addAll(newRoles.getRoles()); 
+         mappedObject.addAll(newRoles.getRoles()); 
       } 
       return mappedObject;
    } 

Modified: projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/main/java/org/jboss/security/plugins/JBossAuthorizationManager.java
===================================================================
--- projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/main/java/org/jboss/security/plugins/JBossAuthorizationManager.java	2010-03-29 19:21:21 UTC (rev 103201)
+++ projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/main/java/org/jboss/security/plugins/JBossAuthorizationManager.java	2010-03-29 19:22:01 UTC (rev 103202)
@@ -510,7 +510,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: projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/authorization/StandaloneJBossAMgrUnitTestCase.java
===================================================================
--- projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/authorization/StandaloneJBossAMgrUnitTestCase.java	2010-03-29 19:21:21 UTC (rev 103201)
+++ projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/authorization/StandaloneJBossAMgrUnitTestCase.java	2010-03-29 19:22:01 UTC (rev 103202)
@@ -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: projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/authorization/ejb/EJBPolicyModuleDelegateUnitTestCase.java
===================================================================
--- projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/authorization/ejb/EJBPolicyModuleDelegateUnitTestCase.java	2010-03-29 19:21:21 UTC (rev 103201)
+++ projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/authorization/ejb/EJBPolicyModuleDelegateUnitTestCase.java	2010-03-29 19:22:01 UTC (rev 103202)
@@ -273,11 +273,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;
    }

Modified: projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/authorization/xacml/EJBXACMLUnitTestCase.java
===================================================================
--- projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/authorization/xacml/EJBXACMLUnitTestCase.java	2010-03-29 19:21:21 UTC (rev 103201)
+++ projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/authorization/xacml/EJBXACMLUnitTestCase.java	2010-03-29 19:22:01 UTC (rev 103202)
@@ -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: projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/authorization/xacml/WebXACMLUnitTestCase.java
===================================================================
--- projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/authorization/xacml/WebXACMLUnitTestCase.java	2010-03-29 19:21:21 UTC (rev 103201)
+++ projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/authorization/xacml/WebXACMLUnitTestCase.java	2010-03-29 19:22:01 UTC (rev 103202)
@@ -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: projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/util/SecurityTestUtil.java
===================================================================
--- projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/util/SecurityTestUtil.java	2010-03-29 19:21:21 UTC (rev 103201)
+++ projects/security/security-jboss-sx/branches/Branch_2_0/jbosssx/src/test/java/org/jboss/test/util/SecurityTestUtil.java	2010-03-29 19:22:01 UTC (rev 103202)
@@ -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: projects/security/security-jboss-sx/branches/Branch_2_0/parent/pom.xml
===================================================================
--- projects/security/security-jboss-sx/branches/Branch_2_0/parent/pom.xml	2010-03-29 19:21:21 UTC (rev 103201)
+++ projects/security/security-jboss-sx/branches/Branch_2_0/parent/pom.xml	2010-03-29 19:22:01 UTC (rev 103202)
@@ -150,6 +150,6 @@
 
   <properties>
     <org.jboss.javaee.version>GA</org.jboss.javaee.version>
-    <org.jboss.security.spi.version>2.0.4.SP3</org.jboss.security.spi.version>
+    <org.jboss.security.spi.version>2.0.5-SNAPSHOT</org.jboss.security.spi.version>
   </properties>
 </project>




More information about the jboss-cvs-commits mailing list