[380629] Design a default Security model

https://bugs.eclipse.org/bugs/show_bug.cgi?id=380629

11 files changed, 107 insertions, 46 deletions

diff --git a/plugins/org.eclipse.emf.cdo.security/model/security.ecore b/plugins/org.eclipse.emf.cdo.security/model/security.ecore
index 683316931a..7b95e9e7a7 100644
--- a/plugins/org.eclipse.emf.cdo.security/model/security.ecore
+++ b/plugins/org.eclipse.emf.cdo.security/model/security.ecore
@@ -27,9 +27,9 @@
   <eClassifiers xsi:type="ecore:EClass" name="Role" eSuperTypes="#//SecurityItem">
     <eStructuralFeatures xsi:type="ecore:EReference" name="assignees" upperBound="-1"
         eType="#//Assignee" eOpposite="#//Assignee/roles"/>
-    <eStructuralFeatures xsi:type="ecore:EAttribute" name="id" eType="ecore:EDataType platform:/plugin/org.eclipse.emf.ecore/model/Ecore.ecore#//EString"/>
     <eStructuralFeatures xsi:type="ecore:EReference" name="checks" upperBound="-1"
         eType="#//Check" containment="true" eOpposite="#//Check/role"/>
+    <eStructuralFeatures xsi:type="ecore:EAttribute" name="id" eType="ecore:EDataType platform:/plugin/org.eclipse.emf.ecore/model/Ecore.ecore#//EString"/>
   </eClassifiers>
   <eClassifiers xsi:type="ecore:EClass" name="Assignee" abstract="true" eSuperTypes="#//SecurityItem">
     <eStructuralFeatures xsi:type="ecore:EReference" name="roles" upperBound="-1"
diff --git a/plugins/org.eclipse.emf.cdo.security/model/security.ecorediag b/plugins/org.eclipse.emf.cdo.security/model/security.ecorediag
index 5963a3890d..7720b96be6 100644
--- a/plugins/org.eclipse.emf.cdo.security/model/security.ecorediag
+++ b/plugins/org.eclipse.emf.cdo.security/model/security.ecorediag
@@ -367,7 +367,7 @@
     <styles xmi:type="notation:ConnectorStyle" xmi:id="_Bl6FgawpEeGqBf0LMO47dg" routing="Rectilinear" lineColor="4210752"/>
     <styles xmi:type="notation:FontStyle" xmi:id="_Bl6FgqwpEeGqBf0LMO47dg" fontName="Segoe UI"/>
     <element xsi:nil="true"/>
-    <bendpoints xmi:type="notation:RelativeBendpoints" xmi:id="_Bl6Fg6wpEeGqBf0LMO47dg" points="[0, 0, -98, 68]$[0, -35, -98, 33]$[114, -35, 16, 33]$[114, -68, 16, 0]"/>
+    <bendpoints xmi:type="notation:RelativeBendpoints" xmi:id="_Bl6Fg6wpEeGqBf0LMO47dg" points="[-18, 0, -116, 68]$[-18, -35, -116, 33]$[114, -35, 16, 33]$[114, -68, 16, 0]"/>
     <sourceAnchor xmi:type="notation:IdentityAnchor" xmi:id="_Bm488qwpEeGqBf0LMO47dg" id="(0.6608695652173913,0.0)"/>
     <targetAnchor xmi:type="notation:IdentityAnchor" xmi:id="_Bm4886wpEeGqBf0LMO47dg" id="(0.3333333333333333,1.0)"/>
   </edges>
@@ -497,7 +497,7 @@
     <styles xmi:type="notation:ConnectorStyle" xmi:id="_Bl-W_awpEeGqBf0LMO47dg" routing="Rectilinear" lineColor="4210752"/>
     <styles xmi:type="notation:FontStyle" xmi:id="_Bl-W_qwpEeGqBf0LMO47dg" fontName="Segoe UI"/>
     <element xsi:nil="true"/>
-    <bendpoints xmi:type="notation:RelativeBendpoints" xmi:id="_Bl-W_6wpEeGqBf0LMO47dg" points="[-33, 0, -156, 74]$[-33, -35, -156, 39]$[97, -35, -26, 39]$[97, -74, -26, 0]"/>
+    <bendpoints xmi:type="notation:RelativeBendpoints" xmi:id="_Bl-W_6wpEeGqBf0LMO47dg" points="[-35, 0, -158, 74]$[-35, -35, -158, 39]$[97, -35, -26, 39]$[97, -74, -26, 0]"/>
     <sourceAnchor xmi:type="notation:IdentityAnchor" xmi:id="_Bm9OYqwpEeGqBf0LMO47dg" id="(0.75,0.0)"/>
     <targetAnchor xmi:type="notation:IdentityAnchor" xmi:id="_Bm9OY6wpEeGqBf0LMO47dg" id="(0.7549019607843137,1.0)"/>
   </edges>
@@ -512,8 +512,8 @@
     <styles xmi:type="notation:FontStyle" xmi:id="_pKXTErIPEeGyraMqKGwiUw" fontColor="4210752" fontName="Segoe UI" fontHeight="10"/>
     <element xmi:type="ecore:EReference" href="security.ecore#//Role/checks"/>
     <bendpoints xmi:type="notation:RelativeBendpoints" xmi:id="_pKXTE7IPEeGyraMqKGwiUw" points="[90, 20, -244, 6]$[244, 20, -90, 6]"/>
-    <sourceAnchor xmi:type="notation:IdentityAnchor" xmi:id="_rYr54LIREeGyraMqKGwiUw" id="(0.10784313725490197,0.21568627450980393)"/>
-    <targetAnchor xmi:type="notation:IdentityAnchor" xmi:id="_rYr54bIREeGyraMqKGwiUw" id="(0.5,0.5)"/>
+    <sourceAnchor xmi:type="notation:IdentityAnchor" xmi:id="_JnbBMLJFEeGyraMqKGwiUw" id="(0.10784313725490197,0.21568627450980393)"/>
+    <targetAnchor xmi:type="notation:IdentityAnchor" xmi:id="_JnboQLJFEeGyraMqKGwiUw" id="(0.5,0.5)"/>
   </edges>
   <edges xmi:type="notation:Edge" xmi:id="_s4d8gLIPEeGyraMqKGwiUw" type="3002" source="_PWGq0LIPEeGyraMqKGwiUw" target="_BlzX26wpEeGqBf0LMO47dg">
     <children xmi:type="notation:Node" xmi:id="_s4fxsLIPEeGyraMqKGwiUw" type="4011">
@@ -526,8 +526,8 @@
     <styles xmi:type="notation:FontStyle" xmi:id="_s4ejkLIPEeGyraMqKGwiUw" fontColor="4210752" fontName="Segoe UI" fontHeight="10"/>
     <element xmi:type="ecore:EReference" href="security.ecore#//Check/role"/>
     <bendpoints xmi:type="notation:RelativeBendpoints" xmi:id="_s4ejkbIPEeGyraMqKGwiUw" points="[-90, 6, 244, 20]$[-244, 6, 90, 20]"/>
-    <sourceAnchor xmi:type="notation:IdentityAnchor" xmi:id="_A7j3YLIQEeGyraMqKGwiUw" id="(0.5,0.5)"/>
-    <targetAnchor xmi:type="notation:IdentityAnchor" xmi:id="_A7kecLIQEeGyraMqKGwiUw" id="(0.10784313725490197,0.21568627450980393)"/>
+    <sourceAnchor xmi:type="notation:IdentityAnchor" xmi:id="_Jnc2YLJFEeGyraMqKGwiUw" id="(0.5,0.5)"/>
+    <targetAnchor xmi:type="notation:IdentityAnchor" xmi:id="_JnddcLJFEeGyraMqKGwiUw" id="(0.10784313725490197,0.21568627450980393)"/>
   </edges>
   <edges xmi:type="notation:Edge" xmi:id="_9vQeALIQEeGyraMqKGwiUw" type="3003" source="_tvbkQLIQEeGyraMqKGwiUw" target="_PWGq0LIPEeGyraMqKGwiUw">
     <styles xmi:type="notation:ConnectorStyle" xmi:id="_9vQeAbIQEeGyraMqKGwiUw" routing="Rectilinear" lineColor="4210752"/>
diff --git a/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/Check.java b/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/Check.java
index a4586c4c88..8f7687077c 100644
--- a/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/Check.java
+++ b/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/Check.java
@@ -3,6 +3,9 @@
 package org.eclipse.emf.cdo.security;
 
 import org.eclipse.emf.cdo.CDOObject;
+import org.eclipse.emf.cdo.common.branch.CDOBranchPoint;
+import org.eclipse.emf.cdo.common.revision.CDORevision;
+import org.eclipse.emf.cdo.common.revision.CDORevisionProvider;
 
 /**
  * <!-- begin-user-doc -->
@@ -82,4 +85,6 @@ public interface Check extends CDOObject
    */
   void setPermission(Permission value);
 
+  boolean isApplicable(CDORevision revision, CDORevisionProvider revisionProvider, CDOBranchPoint securityContext);
+
 } // Check
diff --git a/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/Role.java b/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/Role.java
index 1db7b742db..d0dd96d0e3 100644
--- a/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/Role.java
+++ b/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/Role.java
@@ -21,8 +21,8 @@ import org.eclipse.emf.common.util.EList;
  * The following features are supported:
  * <ul>
  *   <li>{@link org.eclipse.emf.cdo.security.Role#getAssignees <em>Assignees</em>}</li>
- *   <li>{@link org.eclipse.emf.cdo.security.Role#getId <em>Id</em>}</li>
  *   <li>{@link org.eclipse.emf.cdo.security.Role#getChecks <em>Checks</em>}</li>
+ *   <li>{@link org.eclipse.emf.cdo.security.Role#getId <em>Id</em>}</li>
  * </ul>
  * </p>
  *
diff --git a/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/SecurityPackage.java b/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/SecurityPackage.java
index d19b37af36..65f44c2cb1 100644
--- a/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/SecurityPackage.java
+++ b/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/SecurityPackage.java
@@ -280,22 +280,22 @@ public interface SecurityPackage extends EPackage
   int ROLE__ASSIGNEES = SECURITY_ITEM_FEATURE_COUNT + 0;
 
   /**
-   * The feature id for the '<em><b>Id</b></em>' attribute.
+   * The feature id for the '<em><b>Checks</b></em>' containment reference list.
    * <!-- begin-user-doc -->
    * <!-- end-user-doc -->
    * @generated
    * @ordered
    */
-  int ROLE__ID = SECURITY_ITEM_FEATURE_COUNT + 1;
+  int ROLE__CHECKS = SECURITY_ITEM_FEATURE_COUNT + 1;
 
   /**
-   * The feature id for the '<em><b>Checks</b></em>' containment reference list.
+   * The feature id for the '<em><b>Id</b></em>' attribute.
    * <!-- begin-user-doc -->
    * <!-- end-user-doc -->
    * @generated
    * @ordered
    */
-  int ROLE__CHECKS = SECURITY_ITEM_FEATURE_COUNT + 2;
+  int ROLE__ID = SECURITY_ITEM_FEATURE_COUNT + 2;
 
   /**
    * The number of structural features of the '<em>Role</em>' class.
diff --git a/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/ClassCheckImpl.java b/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/ClassCheckImpl.java
index b978875aa1..202357989c 100644
--- a/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/ClassCheckImpl.java
+++ b/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/ClassCheckImpl.java
@@ -2,6 +2,9 @@
  */
 package org.eclipse.emf.cdo.security.impl;
 
+import org.eclipse.emf.cdo.common.branch.CDOBranchPoint;
+import org.eclipse.emf.cdo.common.revision.CDORevision;
+import org.eclipse.emf.cdo.common.revision.CDORevisionProvider;
 import org.eclipse.emf.cdo.security.ClassCheck;
 import org.eclipse.emf.cdo.security.SecurityPackage;
 
@@ -55,4 +58,9 @@ public class ClassCheckImpl extends CheckImpl implements ClassCheck
     return (EList<EClass>)eGet(SecurityPackage.Literals.CLASS_CHECK__CLASSES, true);
   }
 
+  public boolean isApplicable(CDORevision revision, CDORevisionProvider revisionProvider, CDOBranchPoint securityContext)
+  {
+    return false;
+  }
+
 } // ClassCheckImpl
diff --git a/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/PackageCheckImpl.java b/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/PackageCheckImpl.java
index c2f4bea543..c9e92a09f6 100644
--- a/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/PackageCheckImpl.java
+++ b/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/PackageCheckImpl.java
@@ -2,6 +2,9 @@
  */
 package org.eclipse.emf.cdo.security.impl;
 
+import org.eclipse.emf.cdo.common.branch.CDOBranchPoint;
+import org.eclipse.emf.cdo.common.revision.CDORevision;
+import org.eclipse.emf.cdo.common.revision.CDORevisionProvider;
 import org.eclipse.emf.cdo.security.PackageCheck;
 import org.eclipse.emf.cdo.security.SecurityPackage;
 
@@ -56,4 +59,9 @@ public class PackageCheckImpl extends CheckImpl implements PackageCheck
     return (EList<EPackage>)eGet(SecurityPackage.Literals.PACKAGE_CHECK__PACKAGES, true);
   }
 
+  public boolean isApplicable(CDORevision revision, CDORevisionProvider revisionProvider, CDOBranchPoint securityContext)
+  {
+    return false;
+  }
+
 } // PackageCheckImpl
diff --git a/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/ResourceCheckImpl.java b/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/ResourceCheckImpl.java
index 16b64de451..820caf6f25 100644
--- a/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/ResourceCheckImpl.java
+++ b/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/ResourceCheckImpl.java
@@ -2,6 +2,9 @@
  */
 package org.eclipse.emf.cdo.security.impl;
 
+import org.eclipse.emf.cdo.common.branch.CDOBranchPoint;
+import org.eclipse.emf.cdo.common.revision.CDORevision;
+import org.eclipse.emf.cdo.common.revision.CDORevisionProvider;
 import org.eclipse.emf.cdo.security.ResourceCheck;
 import org.eclipse.emf.cdo.security.SecurityPackage;
 
@@ -63,4 +66,9 @@ public class ResourceCheckImpl extends CheckImpl implements ResourceCheck
     eSet(SecurityPackage.Literals.RESOURCE_CHECK__PATTERN, newPattern);
   }
 
+  public boolean isApplicable(CDORevision revision, CDORevisionProvider revisionProvider, CDOBranchPoint securityContext)
+  {
+    return false;
+  }
+
 } // ResourceCheckImpl
diff --git a/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/RoleImpl.java b/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/RoleImpl.java
index c660d8dfff..0959a2d222 100644
--- a/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/RoleImpl.java
+++ b/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/RoleImpl.java
@@ -26,8 +26,8 @@ import org.eclipse.emf.ecore.EClass;
  * The following features are implemented:
  * <ul>
  *   <li>{@link org.eclipse.emf.cdo.security.impl.RoleImpl#getAssignees <em>Assignees</em>}</li>
- *   <li>{@link org.eclipse.emf.cdo.security.impl.RoleImpl#getId <em>Id</em>}</li>
  *   <li>{@link org.eclipse.emf.cdo.security.impl.RoleImpl#getChecks <em>Checks</em>}</li>
+ *   <li>{@link org.eclipse.emf.cdo.security.impl.RoleImpl#getId <em>Id</em>}</li>
  * </ul>
  * </p>
  *
diff --git a/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/SecurityPackageImpl.java b/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/SecurityPackageImpl.java
index a2016d7ca5..57b914d158 100644
--- a/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/SecurityPackageImpl.java
+++ b/plugins/org.eclipse.emf.cdo.security/src/org/eclipse/emf/cdo/security/impl/SecurityPackageImpl.java
@@ -356,7 +356,7 @@ public class SecurityPackageImpl extends EPackageImpl implements SecurityPackage
    */
   public EAttribute getRole_Id()
   {
-    return (EAttribute)roleEClass.getEStructuralFeatures().get(1);
+    return (EAttribute)roleEClass.getEStructuralFeatures().get(2);
   }
 
   /**
@@ -366,7 +366,7 @@ public class SecurityPackageImpl extends EPackageImpl implements SecurityPackage
    */
   public EReference getRole_Checks()
   {
-    return (EReference)roleEClass.getEStructuralFeatures().get(2);
+    return (EReference)roleEClass.getEStructuralFeatures().get(1);
   }
 
   /**
@@ -748,8 +748,8 @@ public class SecurityPackageImpl extends EPackageImpl implements SecurityPackage
 
     roleEClass = createEClass(ROLE);
     createEReference(roleEClass, ROLE__ASSIGNEES);
-    createEAttribute(roleEClass, ROLE__ID);
     createEReference(roleEClass, ROLE__CHECKS);
+    createEAttribute(roleEClass, ROLE__ID);
 
     assigneeEClass = createEClass(ASSIGNEE);
     createEReference(assigneeEClass, ASSIGNEE__ROLES);
@@ -898,15 +898,15 @@ public class SecurityPackageImpl extends EPackageImpl implements SecurityPackage
         this.getAssignee(),
         this.getAssignee_Roles(),
         "assignees", null, 0, -1, Role.class, !IS_TRANSIENT, !IS_VOLATILE, IS_CHANGEABLE, !IS_COMPOSITE, IS_RESOLVE_PROXIES, !IS_UNSETTABLE, IS_UNIQUE, !IS_DERIVED, IS_ORDERED); //$NON-NLS-1$
-    initEAttribute(
-        getRole_Id(),
-        theEcorePackage.getEString(),
-        "id", null, 0, 1, Role.class, !IS_TRANSIENT, !IS_VOLATILE, IS_CHANGEABLE, !IS_UNSETTABLE, !IS_ID, IS_UNIQUE, !IS_DERIVED, IS_ORDERED); //$NON-NLS-1$
     initEReference(
         getRole_Checks(),
         this.getCheck(),
         this.getCheck_Role(),
         "checks", null, 0, -1, Role.class, !IS_TRANSIENT, !IS_VOLATILE, IS_CHANGEABLE, IS_COMPOSITE, !IS_RESOLVE_PROXIES, !IS_UNSETTABLE, IS_UNIQUE, !IS_DERIVED, IS_ORDERED); //$NON-NLS-1$
+    initEAttribute(
+        getRole_Id(),
+        theEcorePackage.getEString(),
+        "id", null, 0, 1, Role.class, !IS_TRANSIENT, !IS_VOLATILE, IS_CHANGEABLE, !IS_UNSETTABLE, !IS_ID, IS_UNIQUE, !IS_DERIVED, IS_ORDERED); //$NON-NLS-1$
 
     initEClass(assigneeEClass, Assignee.class, "Assignee", IS_ABSTRACT, !IS_INTERFACE, IS_GENERATED_INSTANCE_CLASS); //$NON-NLS-1$
     initEReference(
diff --git a/plugins/org.eclipse.emf.cdo.server.security/src/org/eclipse/emf/cdo/server/internal/security/SecurityManager.java b/plugins/org.eclipse.emf.cdo.server.security/src/org/eclipse/emf/cdo/server/internal/security/SecurityManager.java
index 61e8361b00..b56b0e18e8 100644
--- a/plugins/org.eclipse.emf.cdo.server.security/src/org/eclipse/emf/cdo/server/internal/security/SecurityManager.java
+++ b/plugins/org.eclipse.emf.cdo.server.security/src/org/eclipse/emf/cdo/server/internal/security/SecurityManager.java
@@ -18,7 +18,9 @@ import org.eclipse.emf.cdo.eresource.CDOResource;
 import org.eclipse.emf.cdo.net4j.CDONet4jSession;
 import org.eclipse.emf.cdo.net4j.CDONet4jSessionConfiguration;
 import org.eclipse.emf.cdo.net4j.CDONet4jUtil;
+import org.eclipse.emf.cdo.security.Check;
 import org.eclipse.emf.cdo.security.Group;
+import org.eclipse.emf.cdo.security.Permission;
 import org.eclipse.emf.cdo.security.Realm;
 import org.eclipse.emf.cdo.security.RealmUtil;
 import org.eclipse.emf.cdo.security.Role;
@@ -248,40 +250,70 @@ public class SecurityManager implements ISecurityManager
   protected CDOPermission getPermission(CDORevision revision, CDORevisionProvider revisionProvider,
       CDOBranchPoint securityContext, User user)
   {
-    EList<Role> userRoles = null;
+    CDOPermission result = CDOPermission.WRITE;
 
-    Set<Role> readRoles = getNeededRoles(revision, revisionProvider, securityContext, CDOPermission.READ);
-    if (readRoles == null || !readRoles.isEmpty())
+    for (Role role : user.getUnassignedRoles())
     {
-      userRoles = user.getAllRoles();
-
-      for (Role readRole : readRoles)
+      for (Check check : role.getChecks())
       {
-        if (!userRoles.contains(readRole))
+        if (result == CDOPermission.WRITE)
         {
-          return CDOPermission.NONE;
+          if (check.isApplicable(revision, revisionProvider, securityContext))
+          {
+            if (check.getPermission() == Permission.READ)
+            {
+              result = CDOPermission.READ;
+            }
+          }
         }
-      }
-    }
-
-    Set<Role> writeRoles = getNeededRoles(revision, revisionProvider, securityContext, CDOPermission.WRITE);
-    if (writeRoles == null || !writeRoles.isEmpty())
-    {
-      if (userRoles == null)
-      {
-        userRoles = user.getAllRoles();
-      }
-
-      for (Role writeRole : writeRoles)
-      {
-        if (!userRoles.contains(writeRole))
+        else
         {
-          return CDOPermission.READ;
+          // --> result == CDOPermission.READ
+          if (check.isApplicable(revision, revisionProvider, securityContext))
+          {
+            if (check.getPermission() == Permission.READ)
+            {
+              result = CDOPermission.READ;
+            }
+          }
         }
       }
     }
 
-    return CDOPermission.WRITE;
+    // EList<Role> userRoles = null;
+    //
+    // Set<Role> readRoles = getNeededRoles(revision, revisionProvider, securityContext, CDOPermission.READ);
+    // if (readRoles == null || !readRoles.isEmpty())
+    // {
+    // userRoles = user.getAllRoles();
+    //
+    // for (Role readRole : readRoles)
+    // {
+    // if (!userRoles.contains(readRole))
+    // {
+    // return CDOPermission.NONE;
+    // }
+    // }
+    // }
+    //
+    // Set<Role> writeRoles = getNeededRoles(revision, revisionProvider, securityContext, CDOPermission.WRITE);
+    // if (writeRoles == null || !writeRoles.isEmpty())
+    // {
+    // if (userRoles == null)
+    // {
+    // userRoles = user.getAllRoles();
+    // }
+    //
+    // for (Role writeRole : writeRoles)
+    // {
+    // if (!userRoles.contains(writeRole))
+    // {
+    // return CDOPermission.READ;
+    // }
+    // }
+    // }
+    //
+    return result;
   }
 
   protected Set<Role> getNeededRoles(CDORevision revision, CDORevisionProvider revisionProvider,
@@ -437,8 +469,8 @@ public class SecurityManager implements ISecurityManager
       checkRevisionsBeforeCommitting(commitContext, securityContext, user, commitContext.getDirtyObjects());
     }
 
-    private void checkRevisionsBeforeCommitting(CommitContext commitContext, CDOBranchPoint securityContext,
-        User user, InternalCDORevision[] revisions)
+    private void checkRevisionsBeforeCommitting(CommitContext commitContext, CDOBranchPoint securityContext, User user,
+        InternalCDORevision[] revisions)
     {
       for (InternalCDORevision revision : revisions)
       {