You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by st...@apache.org on 2017/06/27 14:11:33 UTC

svn commit: r1800063 [2/2] - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/composite/ oak-core/src/main/java/org/apache/jackrabbit/oak/security/internal/ oak-core/src/test/java/org/apache/jackrabbit/o...

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeProviderEmptyTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeProviderEmptyTest.java?rev=1800063&r1=1800062&r2=1800063&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeProviderEmptyTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeProviderEmptyTest.java Tue Jun 27 14:11:33 2017
@@ -40,7 +40,7 @@ import static org.junit.Assert.assertFal
 import static org.junit.Assert.assertTrue;
 
 /**
- * Test the effect of the combination of
+ * Test the effect of the 'AND' combination of
  *
  * - default permission provider, which always grants full access to an administrative session.
  * - custom provider that prevents all access but supports all permissions and
@@ -51,15 +51,28 @@ import static org.junit.Assert.assertTru
  *
  * The expected outcome is that despite the default provider granting full access,
  * the combination effectively prevents any access.
+ * <p>
+ * Test the effect of the 'OR' combination of
+ *
+ * - default permission provider, which always grants full access to an administrative session.
+ * - custom provider that prevents all access but supports all permissions and
+ *   thus is always called during composite evaluation.
+ *
+ * The tests are executed with the set of principals associated with the admin session,
+ * which in the default permission provider is granted full access.
+ *
+ * The expected outcome is that the combination effectively allows any access.
  */
 public class CompositeProviderEmptyTest extends AbstractCompositeProviderTest {
 
     private CompositePermissionProvider cpp;
+    private CompositePermissionProvider cppO;
 
     @Override
     public void before() throws Exception {
         super.before();
         cpp = createPermissionProvider(root.getContentSession().getAuthInfo().getPrincipals());
+        cppO = createPermissionProviderOR(root.getContentSession().getAuthInfo().getPrincipals());
     }
 
     @Override
@@ -71,12 +84,14 @@ public class CompositeProviderEmptyTest
     public void testGetPrivileges() throws Exception {
         for (String p : NODE_PATHS) {
             assertTrue(cpp.getPrivileges(readOnlyRoot.getTree(p)).isEmpty());
+            assertTrue(cppO.getPrivileges(readOnlyRoot.getTree(p)).isEmpty());
         }
     }
 
     @Test
     public void testGetPrivilegesOnRepo() throws Exception {
         assertTrue(cpp.getPrivileges(null).isEmpty());
+        assertTrue(cppO.getPrivileges(null).isEmpty());
     }
 
     @Test
@@ -92,10 +107,10 @@ public class CompositeProviderEmptyTest
 
     @Test
     public void testHasPrivilegesOnRepo() throws Exception {
-        assertFalse(cpp.hasPrivileges(null, JCR_NAMESPACE_MANAGEMENT));
+        assertFalse(cpp.hasPrivileges(null, JCR_NAMESPACE_MANAGEMENT, JCR_NODE_TYPE_DEFINITION_MANAGEMENT));
+        assertTrue(cppO.hasPrivileges(null, JCR_NAMESPACE_MANAGEMENT, JCR_NODE_TYPE_DEFINITION_MANAGEMENT));
     }
 
-
     @Test
     public void testIsGranted() throws Exception {
         for (String p : NODE_PATHS) {
@@ -104,6 +119,9 @@ public class CompositeProviderEmptyTest
             assertFalse(cpp.isGranted(tree, null, Permissions.READ_NODE));
             assertFalse(cpp.isGranted(tree, null, Permissions.READ_NODE | Permissions.MODIFY_CHILD_NODE_COLLECTION));
             assertFalse(cpp.isGranted(tree, null, Permissions.READ_ACCESS_CONTROL | Permissions.MODIFY_ACCESS_CONTROL));
+            assertTrue(cppO.isGranted(tree, null, Permissions.READ_NODE));
+            assertTrue(cppO.isGranted(tree, null, Permissions.READ_NODE | Permissions.MODIFY_CHILD_NODE_COLLECTION));
+            assertTrue(cppO.isGranted(tree, null, Permissions.READ_ACCESS_CONTROL | Permissions.MODIFY_ACCESS_CONTROL));
         }
     }
 
@@ -117,6 +135,11 @@ public class CompositeProviderEmptyTest
             assertFalse(cpp.isGranted(tree, PROPERTY_STATE, Permissions.ADD_PROPERTY));
             assertFalse(cpp.isGranted(tree, PROPERTY_STATE, Permissions.REMOVE_PROPERTY));
             assertFalse(cpp.isGranted(tree, PROPERTY_STATE, Permissions.READ_ACCESS_CONTROL | Permissions.MODIFY_ACCESS_CONTROL));
+            assertTrue(cppO.isGranted(tree, PROPERTY_STATE, Permissions.READ_PROPERTY));
+            assertTrue(cppO.isGranted(tree, PROPERTY_STATE, Permissions.MODIFY_PROPERTY));
+            assertTrue(cppO.isGranted(tree, PROPERTY_STATE, Permissions.ADD_PROPERTY));
+            assertTrue(cppO.isGranted(tree, PROPERTY_STATE, Permissions.REMOVE_PROPERTY));
+            assertTrue(cppO.isGranted(tree, PROPERTY_STATE, Permissions.READ_ACCESS_CONTROL | Permissions.MODIFY_ACCESS_CONTROL));
         }
     }
 
@@ -128,10 +151,14 @@ public class CompositeProviderEmptyTest
 
             assertFalse(cpp.isGranted(nodePath, Session.ACTION_REMOVE));
             assertFalse(cpp.isGranted(propPath, JackrabbitSession.ACTION_MODIFY_PROPERTY));
-
             assertFalse(cpp.isGranted(nodePath, getActionString(JackrabbitSession.ACTION_MODIFY_ACCESS_CONTROL, JackrabbitSession.ACTION_READ_ACCESS_CONTROL)));
             assertFalse(cpp.isGranted(nonExisting, JackrabbitSession.ACTION_ADD_PROPERTY));
             assertFalse(cpp.isGranted(nonExisting, Session.ACTION_ADD_NODE));
+            assertTrue(cppO.isGranted(nodePath, Session.ACTION_REMOVE));
+            assertTrue(cppO.isGranted(propPath, JackrabbitSession.ACTION_MODIFY_PROPERTY));
+            assertTrue(cppO.isGranted(nodePath, getActionString(JackrabbitSession.ACTION_MODIFY_ACCESS_CONTROL, JackrabbitSession.ACTION_READ_ACCESS_CONTROL)));
+            assertTrue(cppO.isGranted(nonExisting, JackrabbitSession.ACTION_ADD_PROPERTY));
+            assertTrue(cppO.isGranted(nonExisting, Session.ACTION_ADD_NODE));
         }
     }
 
@@ -140,20 +167,31 @@ public class CompositeProviderEmptyTest
         RepositoryPermission rp = cpp.getRepositoryPermission();
         assertFalse(rp.isGranted(Permissions.NAMESPACE_MANAGEMENT));
         assertFalse(rp.isGranted(Permissions.NODE_TYPE_DEFINITION_MANAGEMENT));
+        RepositoryPermission rpO = cppO.getRepositoryPermission();
+        assertTrue(rpO.isGranted(Permissions.NAMESPACE_MANAGEMENT));
+        assertTrue(rpO.isGranted(Permissions.NODE_TYPE_DEFINITION_MANAGEMENT));
     }
 
     @Test
     public void testTreePermissionIsGranted() throws Exception {
         TreePermission parentPermission = TreePermission.EMPTY;
-
         for (String path : TP_PATHS) {
             TreePermission tp = cpp.getTreePermission(readOnlyRoot.getTree(path), parentPermission);
-
             assertFalse(tp.isGranted(Permissions.READ_NODE));
             assertFalse(tp.isGranted(Permissions.REMOVE_NODE));
             assertFalse(tp.isGranted(Permissions.ALL));
+            parentPermission = tp;
+        }
+    }
 
-
+    @Test
+    public void testTreePermissionIsGrantedOR() throws Exception {
+        TreePermission parentPermission = TreePermission.EMPTY;
+        for (String path : TP_PATHS) {
+            TreePermission tp = cppO.getTreePermission(readOnlyRoot.getTree(path), parentPermission);
+            assertTrue(tp.isGranted(Permissions.READ_NODE));
+            assertTrue(tp.isGranted(Permissions.REMOVE_NODE));
+            assertTrue(tp.isGranted(Permissions.ALL));
             parentPermission = tp;
         }
     }
@@ -161,13 +199,21 @@ public class CompositeProviderEmptyTest
     @Test
     public void testTreePermissionIsGrantedProperty() throws Exception {
         TreePermission parentPermission = TreePermission.EMPTY;
-
         for (String path : TP_PATHS) {
             TreePermission tp = cpp.getTreePermission(readOnlyRoot.getTree(path), parentPermission);
-
             assertFalse(tp.isGranted(Permissions.READ_PROPERTY, PROPERTY_STATE));
             assertFalse(tp.isGranted(Permissions.REMOVE_PROPERTY, PROPERTY_STATE));
+            parentPermission = tp;
+        }
+    }
 
+    @Test
+    public void testTreePermissionIsGrantedPropertyOR() throws Exception {
+        TreePermission parentPermission = TreePermission.EMPTY;
+        for (String path : TP_PATHS) {
+            TreePermission tp = cppO.getTreePermission(readOnlyRoot.getTree(path), parentPermission);
+            assertTrue(tp.isGranted(Permissions.READ_PROPERTY, PROPERTY_STATE));
+            assertTrue(tp.isGranted(Permissions.REMOVE_PROPERTY, PROPERTY_STATE));
             parentPermission = tp;
         }
     }
@@ -175,12 +221,21 @@ public class CompositeProviderEmptyTest
     @Test
     public void testTreePermissionCanRead() throws Exception {
         TreePermission parentPermission = TreePermission.EMPTY;
-
         for (String path : TP_PATHS) {
             Tree t = readOnlyRoot.getTree(path);
             TreePermission tp = cpp.getTreePermission(t, parentPermission);
             assertFalse(tp.canRead());
+            parentPermission = tp;
+        }
+    }
 
+    @Test
+    public void testTreePermissionCanReadOR() throws Exception {
+        TreePermission parentPermission = TreePermission.EMPTY;
+        for (String path : TP_PATHS) {
+            Tree t = readOnlyRoot.getTree(path);
+            TreePermission tp = cppO.getTreePermission(t, parentPermission);
+            assertTrue(tp.canRead());
             parentPermission = tp;
         }
     }
@@ -188,12 +243,21 @@ public class CompositeProviderEmptyTest
     @Test
     public void testTreePermissionCanReadProperty() throws Exception {
         TreePermission parentPermission = TreePermission.EMPTY;
-
         for (String path : TP_PATHS) {
             Tree t = readOnlyRoot.getTree(path);
             TreePermission tp = cpp.getTreePermission(t, parentPermission);
             assertFalse(tp.canRead(PROPERTY_STATE));
+            parentPermission = tp;
+        }
+    }
 
+    @Test
+    public void testTreePermissionCanReadPropertyOR() throws Exception {
+        TreePermission parentPermission = TreePermission.EMPTY;
+        for (String path : TP_PATHS) {
+            Tree t = readOnlyRoot.getTree(path);
+            TreePermission tp = cppO.getTreePermission(t, parentPermission);
+            assertTrue(tp.canRead(PROPERTY_STATE));
             parentPermission = tp;
         }
     }
@@ -201,11 +265,11 @@ public class CompositeProviderEmptyTest
     /**
      * {@code AggregatedPermissionProvider} that doesn't grant any access.
      */
-    private static final class EmptyAggregatedProvider extends AbstractAggrProvider {
+    static class EmptyAggregatedProvider extends AbstractAggrProvider {
 
         private static final PermissionProvider BASE = EmptyPermissionProvider.getInstance();
 
-        private EmptyAggregatedProvider(@Nonnull Root root) {
+        public EmptyAggregatedProvider(@Nonnull Root root) {
             super(root);
         }
 

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeProviderNoScopeTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeProviderNoScopeTest.java?rev=1800063&r1=1800062&r2=1800063&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeProviderNoScopeTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeProviderNoScopeTest.java Tue Jun 27 14:11:33 2017
@@ -93,6 +93,20 @@ public class CompositeProviderNoScopeTes
     @Override
     @Test
     public void testGetTreePermissionInstance() throws Exception {
+        PermissionProvider pp = createPermissionProviderOR();
+        TreePermission parentPermission = TreePermission.EMPTY;
+
+        for (String path : TP_PATHS) {
+            Tree t = readOnlyRoot.getTree(path);
+            TreePermission tp = pp.getTreePermission(t, parentPermission);
+            assertCompositeTreePermission(t.isRoot(), tp);
+            parentPermission = tp;
+        }
+    }
+
+    @Override
+    @Test
+    public void testGetTreePermissionInstanceOR() throws Exception {
         PermissionProvider pp = createPermissionProvider();
         TreePermission parentPermission = TreePermission.EMPTY;
 
@@ -115,6 +129,23 @@ public class CompositeProviderNoScopeTes
         assertCompositeTreePermission(tp);
 
         for (String cName : childNames) {
+            ns = ns.getChildNode(cName);
+            tp = tp.getChildPermission(cName, ns);
+            assertCompositeTreePermission(false, tp);
+        }
+    }
+
+    @Override
+    @Test
+    public void testTreePermissionGetChildOR() throws Exception {
+        List<String> childNames = ImmutableList.of("test", "a", "b", "c", "nonexisting");
+
+        Tree rootTree = readOnlyRoot.getTree(ROOT_PATH);
+        NodeState ns = ((ImmutableTree) rootTree).getNodeState();
+        TreePermission tp = createPermissionProviderOR().getTreePermission(rootTree, TreePermission.EMPTY);
+        assertCompositeTreePermission(tp);
+
+        for (String cName : childNames) {
             ns = ns.getChildNode(cName);
             tp = tp.getChildPermission(cName, ns);
             assertCompositeTreePermission(false, tp);

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeTreePermissionTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeTreePermissionTest.java?rev=1800063&r1=1800062&r2=1800063&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeTreePermissionTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeTreePermissionTest.java Tue Jun 27 14:11:33 2017
@@ -23,14 +23,16 @@ import javax.annotation.Nonnull;
 
 import org.apache.jackrabbit.oak.AbstractSecurityTest;
 import org.apache.jackrabbit.oak.api.Root;
+import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants;
 import org.apache.jackrabbit.oak.plugins.tree.RootFactory;
+import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
 import org.apache.jackrabbit.oak.plugins.tree.impl.ImmutableTree;
+import org.apache.jackrabbit.oak.security.authorization.composite.CompositeAuthorizationConfiguration.CompositionType;
 import org.apache.jackrabbit.oak.spi.security.Context;
 import org.apache.jackrabbit.oak.spi.security.authorization.permission.AggregatedPermissionProvider;
 import org.apache.jackrabbit.oak.spi.security.authorization.permission.TreePermission;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
-import org.apache.jackrabbit.oak.util.NodeUtil;
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
@@ -50,8 +52,8 @@ public class CompositeTreePermissionTest
     public void before() throws Exception {
         super.before();
 
-        NodeUtil rootNode = new NodeUtil(root.getTree("/"));
-        rootNode.addChild("test", NodeTypeConstants.NT_OAK_UNSTRUCTURED);
+        Tree rootNode = root.getTree("/");
+        TreeUtil.addChild(rootNode, "test", NodeTypeConstants.NT_OAK_UNSTRUCTURED);
         root.commit();
 
         readOnlyRoot = RootFactory.createReadOnlyRoot(root);
@@ -72,7 +74,8 @@ public class CompositeTreePermissionTest
     }
 
     private TreePermission createRootTreePermission(AggregatedPermissionProvider... providers) {
-        return new CompositePermissionProvider(readOnlyRoot, Arrays.asList(providers), Context.DEFAULT).getTreePermission(rootTree, TreePermission.EMPTY);
+        return new CompositePermissionProvider(readOnlyRoot, Arrays.asList(providers), Context.DEFAULT, CompositionType.AND)
+                .getTreePermission(rootTree, TreePermission.EMPTY);
     }
 
     private static void assertCompositeTreePermission(boolean expected, @Nonnull TreePermission tp) {

Modified: jackrabbit/oak/trunk/oak-doc/src/site/markdown/security/authorization/composite.md
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-doc/src/site/markdown/security/authorization/composite.md?rev=1800063&r1=1800062&r2=1800063&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-doc/src/site/markdown/security/authorization/composite.md (original)
+++ jackrabbit/oak/trunk/oak-doc/src/site/markdown/security/authorization/composite.md Tue Jun 27 14:11:33 2017
@@ -87,7 +87,7 @@ Once multiple modules are deployed a [Co
 characteristics will be returned:
 
 - API calls reading information will return the combined result of the wrapped implementations. 
-- Methods defined solely by `JackrabbitAccessControlManager` additionally test for the delegatees to implement that extentions.
+- Methods defined solely by `JackrabbitAccessControlManager` additionally test for the delegatees to implement that extension.
 - API calls writing back policies will look for the responsible `PolicyOwner` and specifically delegate the call. If no owner can be found an `AccessControlException` is thrown. 
 
 Hence, a given authorization model is free to implement JCR `AccessControlManager` 
@@ -115,7 +115,7 @@ effective permissions:
   permissions have been successfully processed and none of the delegatees involved 
   denied access.
   
-This implies that evalution of permissions across multiple implementations is 
+This implies that evaluation of permissions across multiple implementations is 
 strictly additive: as soon as one provider denies access (either by an explicit 
 deny or by a missing explicit allow) permissions are denied.
 
@@ -127,12 +127,12 @@ For a given permission provider this mea
 the context of the aggregation (i.e. single model setup), a 'limited' provider must 
 never grant access for permissions or items it isn't able to handle properly. 
 In other words: permissions that have not been explicitly granted within the scope 
-of an implemenation must be denied.
+of an implementation must be denied.
 
 #### Restriction Management
 
 Support for multiple restriction providers has already been been present with the 
-default authorization implementation since Oak 1.0. The mechnism described in 
+default authorization implementation since Oak 1.0. The mechanism described in 
 section [Restriction Management](restriction.html) is not affected by the new functionality.
 
 The `CompositeAuthorizationConfiguration` is in charge of collecting 
@@ -148,8 +148,8 @@ types of `AccessControlPolicy` where res
 <a name="configuration"/>
 ### Configuration
 
-There are no implementation specific configuration options associated with 
-the `CompositeAuthorizationConfiguration`.
+By default the `CompositeAuthorizationConfiguration` aggregates results by applying an `AND` operation to the current set of providers.
+This can be changed via configuration to an `OR`. See section [Introduction to Oak Security](../../introduction.html#configuration) for further details.
 
 <a name="pluggability"/>
 ### Pluggability
@@ -158,7 +158,7 @@ The following steps are required to plug
 the Oak repository:
 
 - Implement your custom `AuthorizationConfiguration`
-- Deploy the bundle containing the implemenation
+- Deploy the bundle containing the implementation
 - Bind your `AuthorizationConfiguration` to the `SecurityProvider`:
     - in an OSGi setup this is achieved by adding the configuration to the 
       `requiredServicePids` property of the `SecurityProviderRegistration` _("Apache Jackrabbit Oak SecurityProvider")_ 

Modified: jackrabbit/oak/trunk/oak-doc/src/site/markdown/security/introduction.md
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-doc/src/site/markdown/security/introduction.md?rev=1800063&r1=1800062&r2=1800063&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-doc/src/site/markdown/security/introduction.md (original)
+++ jackrabbit/oak/trunk/oak-doc/src/site/markdown/security/introduction.md Tue Jun 27 14:11:33 2017
@@ -73,7 +73,7 @@ successfully been resolved. This new app
 the initial security provider implementation and has been backported to existing 
 branches (see [OAK-3201] and [OAK-3441]).
 
-While optional configuration settting can be changed or extended at runtime, 
+While optional configuration setting can be changed or extended at runtime, 
 modules and extensions considered required for a functional security setup, need 
 to be listed in the _"Required Service PIDs"_ property. This asserts both reliable 
 security setup and proper initialization of the individual modules. See also 
@@ -126,7 +126,7 @@ only authentication and authorization ar
 
 This is compliant with the security requirements defined by JSR 283 which defines 
 API to login into the repository and mandates minimal permission evaluation, 
-be it implemenation specific of imposed by the optional access control management.
+be it implementation specific of imposed by the optional access control management.
 
 The minimal security setup may consequently be reduced to a setup as defined by 
 the following imaginary, custom `SecurityProvider` (see also [OpenSecurityProvider])
@@ -167,7 +167,7 @@ of view. Please note the following depen
 
 1. **Authentication** is mandatory and expected to bind a set of `Principal`s to 
    the `Subject`. This may happen before or during the repository login.
-2. **Permission Evalution** is mandatory and associated with the set of `Principal`s 
+2. **Permission Evaluation** is mandatory and associated with the set of `Principal`s 
    bound to to the `Subject` during the authentication step.
 3. `Principal`s represent the link between authentication and authorization and _MAY_ 
    be exposed by Principal Management module as described above.
@@ -178,7 +178,7 @@ of view. Please note the following depen
    along with exposing the corresponding principals as part of the Principal Management.
 6. **User Management** is optional and _MAY_ be used for credentials validation during the authentication 
    step. If present it is _usually_ used as a source for principals exposed by Principal Management.
-   
+
 <a name="configuration"/>
 ### Configuration 
 
@@ -208,6 +208,12 @@ The value of this configuration paramete
 module or functionality that is considered required for a successful security setup.
 See section [pluggability](#pluggability) below.
 
+| Parameter                | Type     | Default   | Description            |
+|--------------------------|----------|-----------|------------------------|
+| `Authorization Composition Type`  | String (AND|OR) | AND | The Composite Authorization model uses this flag to determine what type of logic to apply to the existing providers|
+
+Given a set of permission providers, the composite model can aggregate the results by applying an `AND` logic (for example all providers must allow a specific privilege in order to be granted), or an `OR` (for example any provider can allow a privilege). By default the `AND` version is used.
+
 #### CompositeConfiguration
 
 | Parameter       | Type  | Default                    | Description            |