Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositePermissionProvider.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/composite/CompositePermissionProvider.java (revision 1858010) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositePermissionProvider.java (date 1556031409000) @@ -94,7 +94,7 @@ @NotNull @Override public Set getPrivileges(@Nullable Tree tree) { - Tree immutableTree = PermissionUtil.getReadOnlyTree(tree, immutableRoot); + Tree immutableTree = PermissionUtil.getReadOnlyTreeOrNull(tree, immutableRoot); PrivilegeBits result = PrivilegeBits.getInstance(); PrivilegeBits denied = PrivilegeBits.getInstance(); @@ -123,7 +123,7 @@ @Override public boolean hasPrivileges(@Nullable Tree tree, @NotNull String... privilegeNames) { - Tree immutableTree = PermissionUtil.getReadOnlyTree(tree, immutableRoot); + Tree immutableTree = PermissionUtil.getReadOnlyTreeOrNull(tree, immutableRoot); PrivilegeBits privilegeBits = privilegeBitsProvider.getBits(privilegeNames); if (privilegeBits.isEmpty()) { return true; Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionProviderImpl.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/permission/PermissionProviderImpl.java (revision 1858010) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionProviderImpl.java (date 1556030609000) @@ -88,12 +88,12 @@ @NotNull @Override public Set getPrivileges(@Nullable Tree tree) { - return getCompiledPermissions().getPrivileges(PermissionUtil.getReadOnlyTree(tree, immutableRoot)); + return getCompiledPermissions().getPrivileges(PermissionUtil.getReadOnlyTreeOrNull(tree, immutableRoot)); } @Override public boolean hasPrivileges(@Nullable Tree tree, @NotNull String... privilegeNames) { - return getCompiledPermissions().hasPrivileges(PermissionUtil.getReadOnlyTree(tree, immutableRoot), privilegeNames); + return getCompiledPermissions().hasPrivileges(PermissionUtil.getReadOnlyTreeOrNull(tree, immutableRoot), privilegeNames); } @NotNull Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionUtil.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/permission/PermissionUtil.java (revision 1858010) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionUtil.java (date 1556030256000) @@ -107,11 +107,20 @@ } @Nullable - public static Tree getReadOnlyTree(@Nullable Tree tree, @NotNull Root readOnlyRoot) { + public static Tree getReadOnlyTreeOrNull(@Nullable Tree tree, @NotNull Root readOnlyRoot) { if (tree instanceof ReadOnly) { return tree; } else { return (tree == null) ? null : readOnlyRoot.getTree(tree.getPath()); } } + + @NotNull + public static Tree getReadOnlyTree(@NotNull Tree tree, @NotNull Root readOnlyRoot) { + if (tree instanceof ReadOnly) { + return tree; + } else { + return readOnlyRoot.getTree(tree.getPath()); + } + } } Index: oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeProviderCustomMixTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeProviderCustomMixTest.java (revision 1858010) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/composite/CompositeProviderCustomMixTest.java (date 1556031409000) @@ -28,6 +28,8 @@ import org.apache.jackrabbit.oak.api.PropertyState; import org.apache.jackrabbit.oak.api.Root; import org.apache.jackrabbit.oak.api.Tree; +import org.apache.jackrabbit.oak.commons.PathUtils; +import org.apache.jackrabbit.oak.plugins.tree.ReadOnly; import org.apache.jackrabbit.oak.plugins.tree.TreeLocation; import org.apache.jackrabbit.oak.plugins.tree.TreeType; import org.apache.jackrabbit.oak.security.authorization.composite.CompositeAuthorizationConfiguration.CompositionType; @@ -49,6 +51,8 @@ import static org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants.JCR_READ; import static org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants.JCR_WRITE; import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.withSettings; public class CompositeProviderCustomMixTest extends AbstractSecurityTest { @@ -93,6 +97,7 @@ actionMap.put(JCR_NODE_TYPE_MANAGEMENT, JackrabbitSession.ACTION_NODE_TYPE_MANAGEMENT); actionMap.put(JCR_WRITE, JackrabbitSession.ACTION_ADD_NODE); + Tree tree = mock(Tree.class, withSettings().extraInterfaces(ReadOnly.class)); // tests all possible 256 shuffles for (CompositionType type : CompositionType.values()) { for (Set granted1 : Sets.powerSet(supp1)) { @@ -101,7 +106,7 @@ CompositePermissionProvider cpp = buildCpp(supp1, granted1, supp2, granted2, type, grantMap); boolean expected = expected(ps, supp1, granted1, supp2, granted2, type, false); - boolean result1 = cpp.isGranted(null, null, mapToPermissions(ps, grantMap)); + boolean result1 = cpp.isGranted(tree, null, mapToPermissions(ps, grantMap)); String err1 = "[isGranted1] Checking " + ps + " in {supported: " + supp1 + ", granted: " + granted1 + "} " + type + " {supported: " + supp2 + ", granted: " + granted2 + "}"; assertEquals(err1, expected, result1); Index: oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionUtilTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionUtilTest.java (revision 1858010) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionUtilTest.java (date 1556030627000) @@ -130,6 +130,34 @@ assertEquals("/path", PermissionUtil.getPath(t, afterT)); } + @Test + public void testGetReadOnlyTreeOrNullFromNull() { + Root r = mock(Root.class); + + assertNull(PermissionUtil.getReadOnlyTreeOrNull(null, r)); + verify(r, never()).getTree(anyString()); + } + + @Test + public void testGetReadOnlyTreeOrNull() { + Tree readOnlyTree = mock(Tree.class, withSettings().extraInterfaces(ReadOnly.class)); + Root r = mock(Root.class); + + assertSame(readOnlyTree, PermissionUtil.getReadOnlyTreeOrNull(readOnlyTree, r)); + verify(r, never()).getTree(anyString()); + } + + @Test + public void testGetReadOnlyTreeOrNullFromTree() { + Tree readOnlyTree = mock(Tree.class, withSettings().extraInterfaces(ReadOnly.class)); + + Root r = when(mock(Root.class).getTree("/path")).thenReturn(readOnlyTree).getMock(); + Tree t = when(mock(Tree.class).getPath()).thenReturn("/path").getMock(); + + assertSame(readOnlyTree, PermissionUtil.getReadOnlyTreeOrNull(t, r)); + verify(r, times(1)).getTree("/path"); + } + @Test public void testGetReadOnlyTree() { Tree readOnlyTree = mock(Tree.class, withSettings().extraInterfaces(ReadOnly.class));