Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/restriction/GlobPattern.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/restriction/GlobPattern.java (revision 1880831) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/restriction/GlobPattern.java (date 1597830872000) @@ -42,8 +42,8 @@ * 
  * NodePath     |   Restriction   |   Matches
  * -----------------------------------------------------------------------------
- * /foo         |   null          |   matches /foo and all children of /foo
- * /foo         |   ""            |   matches /foo only
+ * /foo         |   null          |   matches /foo and all descendants of /foo
+ * /foo         |   ""            |   matches /foo only (no descendants, not even properties)
  *
*

* @@ -53,10 +53,10 @@ * NodePath = "/foo" * Restriction | Matches * ----------------------------------------------------------------------------- - * /cat | the node /foo/cat and all it's children - * /cat/ | the descendants of the node /foo/cat - * cat | the node /foocat and all it's children - * cat/ | all descendants of the node /foocat + * /cat | '/foo/cat' and all it's descendants + * /cat/ | all descendants of '/foocat' + * cat | '/foocat' and all it's descendants + * cat/ | all descendants of '/foocat' * *

* @@ -67,7 +67,7 @@ * Restriction | Matches * ----------------------------------------------------------------------------- * * | foo, all siblings of foo and their descendants - * /*cat | all children of /foo whose path ends with "cat" + * /*cat | all descendants of /foo whose path ends with "cat" * /*/cat | all non-direct descendants of /foo named "cat" * /cat* | all descendant path of /foo that have the direct foo-descendant segment starting with "cat" * *cat | all siblings and descendants of foo that have a name ending with cat Index: oak-doc/src/site/markdown/security/authorization/restriction.md IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-doc/src/site/markdown/security/authorization/restriction.md (revision 1880831) +++ oak-doc/src/site/markdown/security/authorization/restriction.md (date 1597840702000) @@ -130,15 +130,15 @@ | rep:glob | Result | |---------------|----------------------------------------------------------| -| null | i.e. no restriction: matches /foo and all children | -| "" | matches node /foo only | +| null | i.e. no restriction: matches /foo and all descendants | +| "" | matches node /foo only (no descendants, not even properties) | Examples including wildcard char: | rep:glob | Result | |---------------|----------------------------------------------------------| | \* | foo, siblings of foo and their descendants | -| /\*cat | all child items of /foo whose paths end with 'cat' | +| /\*cat | all descendants of /foo whose paths end with 'cat' | | \*cat | all siblings and descendants of foo that have a name ending with 'cat' | | /\*/cat | all non-direct descendants of /foo named 'cat' | | /cat\* | all descendant of /foo that have the direct foo-descendant segment starting with 'cat' | @@ -151,10 +151,10 @@ | rep:glob | Result | |---------------|----------------------------------------------------------| -| /cat | the node /foo/cat and all it's child items | -| /cat/ | the descendants of the node /foo/cat | -| cat | the node /foocat and all it's child items | -| cat/ | all descendants of the node /foocat | +| /cat | '/foo/cat' and all it's descendants | +| /cat/ | all descendants of '/foocat' | +| cat | '/foocat' and all it's descendants | +| cat/ | all descendants of '/foocat' | See also [GlobPattern] for implementation details. @@ -194,6 +194,7 @@ - implement `RestrictionProvider` interface exposing your custom restriction(s). - make the provider implementation an OSGi service and make it available to the Oak repository. +- make sure the `RestrictionProvider` is listed as required service with the `SecurityProvider` (see also [Introduction](../introduction.html#configuration])) Please make sure to consider the following recommendations when implementing a custom `RestrictionProvider`: - restrictions are part of the overall permission evaluation and thus may heavily impact overall read/write performance Index: oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadWithGlobRestrictionTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadWithGlobRestrictionTest.java (revision 1880831) +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadWithGlobRestrictionTest.java (date 1597828465000) @@ -20,16 +20,29 @@ import java.util.HashSet; import java.util.Set; import javax.jcr.Node; +import javax.jcr.RepositoryException; import javax.jcr.Session; import javax.jcr.security.Privilege; import org.apache.jackrabbit.JcrConstants; import org.apache.jackrabbit.api.security.JackrabbitAccessControlManager; import org.apache.jackrabbit.api.security.user.Group; +import org.apache.jackrabbit.oak.commons.PathUtils; import org.junit.Test; public class ReadWithGlobRestrictionTest extends AbstractEvaluationTest { + private String ccPath; + + @Override + protected void setUp() throws Exception { + super.setUp(); + + Node grandchild = superuser.getNode(childNPath).addNode("child"); + ccPath = grandchild.getPath(); + superuser.save(); + } + @Test public void testGlobRestriction() throws Exception { deny(path, readPrivileges, createGlobRestriction("*/" + jcrPrimaryType)); @@ -64,7 +77,7 @@ allow(path, group2.getPrincipal(), readPrivs); deny(path, group3.getPrincipal(), readPrivs); - Set principals = new HashSet(); + Set principals = new HashSet<>(); principals.add(getTestGroup().getPrincipal()); principals.add(group2.getPrincipal()); principals.add(group3.getPrincipal()); @@ -155,11 +168,7 @@ * @see OAK-2412 */ @Test - public void testEmptyGlobRestriction() throws Exception{ - Node grandchild = superuser.getNode(childNPath).addNode("child"); - String ccPath = grandchild.getPath(); - superuser.save(); - + public void testEmptyGlobRestriction() throws Exception { // first deny access to 'path' (read-access is granted in the test setup) deny(path, readPrivileges); assertFalse(canReadNode(testSession, path)); @@ -184,10 +193,6 @@ */ @Test public void testEmptyGlobRestriction2() throws Exception{ - Node grandchild = superuser.getNode(childNPath).addNode("child"); - String ccPath = grandchild.getPath(); - superuser.save(); - // first deny access to 'path' (read-access is granted in the test setup) deny(path, readPrivileges); assertFalse(canReadNode(testSession, path)); @@ -268,4 +273,34 @@ assertTrue(canReadNode(testSession, n121.getPath())); assertTrue(canReadNode(testSession, n122.getPath())); } + + /** + * OAK-9179 + */ + @Test + public void testGlobTrailingSlash() throws Exception { + // first deny access to 'path' (read-access is granted in the test setup) + deny(path, readPrivileges); + allow(path, readPrivileges, createGlobRestriction("/"+PathUtils.getName(childNPath) + "/")); + assertGlobTrailingSlashEffect(); + } + + /** + * OAK-9179 + */ + @Test + public void testGlobTrailingSlashWildcard() throws Exception { + // first deny access to 'path' (read-access is granted in the test setup) + deny(path, readPrivileges); + allow(path, readPrivileges, createGlobRestriction("/"+PathUtils.getName(childNPath) + "/*")); + assertGlobTrailingSlashEffect(); + } + + private void assertGlobTrailingSlashEffect() throws RepositoryException { + assertFalse(canReadNode(testSession, path)); + assertFalse(canReadNode(testSession, path+"/")); + assertFalse(canReadNode(testSession, childNPath)); + assertTrue(canReadNode(testSession, ccPath)); + assertTrue(testSession.propertyExists(childchildPPath)); + } } \ No newline at end of file Index: oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/WriteRestrictionTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/WriteRestrictionTest.java (revision 1880831) +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/WriteRestrictionTest.java (date 1597830389000) @@ -18,14 +18,19 @@ import javax.jcr.AccessDeniedException; import javax.jcr.Node; +import javax.jcr.RepositoryException; import javax.jcr.Session; import javax.jcr.security.Privilege; -import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants; +import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.test.api.util.Text; import org.junit.Before; import org.junit.Test; +import static javax.jcr.security.Privilege.JCR_ADD_CHILD_NODES; +import static javax.jcr.security.Privilege.JCR_REMOVE_CHILD_NODES; +import static javax.jcr.security.Privilege.JCR_REMOVE_NODE; + /** * WriteRestrictionTest: tests add and remove node in combination with glob restrictions. */ @@ -68,8 +73,8 @@ @Test public void testGlobRestriction2() throws Exception { - Privilege[] addNode = privilegesFromName(Privilege.JCR_ADD_CHILD_NODES); - Privilege[] rmNode = privilegesFromName(Privilege.JCR_REMOVE_NODE); + Privilege[] addNode = privilegesFromName(JCR_ADD_CHILD_NODES); + Privilege[] rmNode = privilegesFromName(JCR_REMOVE_NODE); // permissions defined @ path // restriction: grants write-priv to nodeName3 grand-children but not direct nodeName3 children. @@ -84,7 +89,7 @@ @Test public void testGlobRestriction3() throws Exception { - Privilege[] addNode = privilegesFromName(Privilege.JCR_ADD_CHILD_NODES); + Privilege[] addNode = privilegesFromName(JCR_ADD_CHILD_NODES); // permissions defined @ path // restriction: allows write to nodeName3 children @@ -104,7 +109,7 @@ @Test public void testGlobRestriction4() throws Exception { - Privilege[] addNode = privilegesFromName(Privilege.JCR_ADD_CHILD_NODES); + Privilege[] addNode = privilegesFromName(JCR_ADD_CHILD_NODES); allow(path, repWritePrivileges, createGlobRestriction("/*"+nodeName3)); deny(childNPath2, addNode); @@ -122,7 +127,7 @@ /* allow READ/WRITE privilege for testUser at 'path' */ allow(path, testUser.getPrincipal(), readWritePrivileges); /* deny REMOVE_NODE privileges at subtree. */ - deny(path, privilegesFromName(PrivilegeConstants.JCR_REMOVE_NODE), createGlobRestriction("*/" + nodeName3)); + deny(path, privilegesFromName(JCR_REMOVE_NODE), createGlobRestriction("*/" + nodeName3)); testSession.getNode(childNPath).getNode(nodeName3).remove(); try { @@ -138,7 +143,7 @@ /* allow READ/WRITE privilege for testUser at 'path' */ allow(path, testUser.getPrincipal(), readWritePrivileges); /* deny REMOVE_NODE privileges at subtree. */ - deny(path, privilegesFromName(PrivilegeConstants.JCR_REMOVE_CHILD_NODES), createGlobRestriction("*/" + Text.getName(childNPath))); + deny(path, privilegesFromName(JCR_REMOVE_CHILD_NODES), createGlobRestriction("*/" + Text.getName(childNPath))); testSession.getNode(childNPath).getNode(nodeName3).remove(); try { @@ -154,7 +159,7 @@ /* allow READ/WRITE privilege for testUser at 'path' */ allow(path, testUser.getPrincipal(), readWritePrivileges); /* deny ADD_CHILD_NODES privileges at subtree. */ - deny(path, privilegesFromName(PrivilegeConstants.JCR_ADD_CHILD_NODES), createGlobRestriction("*/"+nodeName3)); + deny(path, privilegesFromName(JCR_ADD_CHILD_NODES), createGlobRestriction("*/"+nodeName3)); Node node4 = testSession.getNode(nodePath3).addNode(nodeName4); try { @@ -164,4 +169,43 @@ // success } } + + /** + * OAK-9179 + */ + @Test + public void testGlobTrailingSlash() throws Exception { + Node grandchild = superuser.getNode(childNPath).addNode("child"); + String ccPath = grandchild.getPath(); + + // allow ADD_CHILD_NODES privileges at '/cat/' -> matches descendants of childNPath + allow(path, privilegesFromName(JCR_ADD_CHILD_NODES), createGlobRestriction("/"+PathUtils.getName(childNPath) + "/")); + assertGlobTrailingSlashEffect(ccPath); + } + + /** + * OAK-9179 + */ + @Test + public void testGlobTrailingSlashWildcard() throws Exception { + Node grandchild = superuser.getNode(childNPath).addNode("grandchild"); + String ccPath = grandchild.getPath(); + + // allow ADD_CHILD_NODES privileges at '/cat/*' -> matches descendants of childNPath + allow(path, privilegesFromName(JCR_ADD_CHILD_NODES), createGlobRestriction("/"+PathUtils.getName(childNPath) + "/*")); + assertGlobTrailingSlashEffect(ccPath); + } + + private void assertGlobTrailingSlashEffect(String ccPath) throws RepositoryException { + assertFalse(testSession.hasPermission(path, Session.ACTION_ADD_NODE)); + assertFalse(testSession.hasPermission(childNPath, Session.ACTION_ADD_NODE)); + assertFalse(testSession.hasPermission(childNPath+"/", Session.ACTION_ADD_NODE)); + assertFalse(testSession.hasPermission(ccPath, Session.ACTION_ADD_NODE)); + assertFalse(testSession.hasPermission(ccPath+"/", Session.ACTION_ADD_NODE)); + assertTrue(testSession.hasPermission(ccPath+"/greatgrandchild", Session.ACTION_ADD_NODE)); + assertTrue(testSession.hasPermission(ccPath+"/greatgrandchild/", Session.ACTION_ADD_NODE)); + assertTrue(testSession.hasPermission(ccPath+"/greatgrandchild/descendant", Session.ACTION_ADD_NODE)); + testSession.getNode(ccPath).addNode("greatgrandchild").addNode("descendant"); + testSession.save(); + } } \ No newline at end of file