Skip to content
Permalink
Browse files
OAK-9791 : Missing check for restriction node being present
  • Loading branch information
anchela committed Jun 2, 2022
1 parent b58916d commit ed43ffb81a13c769c7b202787f0e68550d336245
Showing 4 changed files with 61 additions and 9 deletions.
@@ -401,7 +401,7 @@ private AbstractEntry createEffectiveEntry(@NotNull Tree entryTree) throws Acces
return null;
}

Set<Restriction> restrictions = rp.readRestrictions(oakPath, entryTree);
Set<Restriction> restrictions = Utils.readRestrictions(rp, oakPath, entryTree);
NamePathMapper npMapper = getNamePathMapper();
return new AbstractEntry(oakPath, principal, bits, restrictions, npMapper) {
@Override
@@ -75,7 +75,7 @@ boolean addEntry(@NotNull Tree entryTree) throws AccessControlException {
String oakPath = Strings.emptyToNull(TreeUtil.getString(entryTree, REP_EFFECTIVE_PATH));
if (Utils.hasValidRestrictions(oakPath, entryTree, restrictionProvider)) {
PrivilegeBits bits = privilegeBitsProvider.getBits(entryTree.getProperty(Constants.REP_PRIVILEGES).getValue(Type.NAMES));
Set<Restriction> restrictions = restrictionProvider.readRestrictions(oakPath, entryTree);
Set<Restriction> restrictions = Utils.readRestrictions(restrictionProvider, oakPath, entryTree);
return addEntry(new EntryImpl(oakPath, bits, restrictions));
} else {
return false;
@@ -26,6 +26,7 @@
import org.apache.jackrabbit.oak.spi.security.authorization.permission.PermissionProvider;
import org.apache.jackrabbit.oak.spi.security.authorization.permission.Permissions;
import org.apache.jackrabbit.oak.spi.security.authorization.principalbased.Filter;
import org.apache.jackrabbit.oak.spi.security.authorization.restriction.Restriction;
import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionProvider;
import org.apache.jackrabbit.oak.spi.xml.ImportBehavior;
import org.jetbrains.annotations.NotNull;
@@ -151,4 +152,22 @@ public static boolean hasValidRestrictions(@Nullable String oakPath, @NotNull Tr
return true;
}
}

/**
* Utility method that conditionally reads restrictions from provider if the given {@code aceTree} has restriction
* child tree, i.e. combining {@link #hasRestrictions(Tree)} with {@link RestrictionProvider#readRestrictions(String, Tree)}.
*
* @param provider The restriction provider
* @param effectivePath The effective path
* @param aceTree The ace tree
* @return restrictions as read from the provider if the given {@code aceTree} has a rep:restriction child. otherwise,
* an empty set without calling the provider.
*/
public static Set<Restriction> readRestrictions(@NotNull RestrictionProvider provider, @Nullable String effectivePath, @NotNull Tree aceTree) {
if (hasRestrictions(aceTree)) {
return provider.readRestrictions(effectivePath, aceTree);
} else {
return Collections.emptySet();
}
}
}
@@ -21,12 +21,13 @@
import org.apache.jackrabbit.oak.api.Tree;
import org.apache.jackrabbit.oak.namepath.NamePathMapper;
import org.apache.jackrabbit.oak.spi.security.authorization.principalbased.Filter;
import org.apache.jackrabbit.oak.spi.security.authorization.restriction.AbstractRestrictionProvider;
import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionProvider;
import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
import org.apache.jackrabbit.oak.spi.xml.ImportBehavior;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.junit.Test;
import org.mockito.Mockito;
import org.mockito.stubbing.Answer;

import javax.jcr.RepositoryException;
@@ -39,8 +40,13 @@
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;

public class UtilsTest implements Constants {
@@ -53,15 +59,15 @@ private static Tree mockTree(boolean exists, @NotNull String name, @Nullable Str
}

private static Filter mockFilter(boolean canHandle) {
Filter filter = Mockito.mock(Filter.class);
Filter filter = mock(Filter.class);
when(filter.canHandle(any(Set.class))).thenReturn(canHandle);
return filter;
}

private static PrivilegeManager mockPrivilegeManager() throws RepositoryException {
PrivilegeManager privilegeManager = Mockito.mock(PrivilegeManager.class);
PrivilegeManager privilegeManager = mock(PrivilegeManager.class);
when(privilegeManager.getPrivilege(any(String.class))).then((Answer<Privilege>) invocationOnMock -> {
Privilege p = Mockito.mock(Privilege.class);
Privilege p = mock(Privilege.class);
when(p.getName()).thenReturn(invocationOnMock.getArgument(0));
return p;
});
@@ -100,7 +106,7 @@ public void testIsPrincipalPolicyTree() {

@Test(expected = AccessControlException.class)
public void testCanHandleNullName() throws Exception {
Utils.canHandle(Mockito.mock(Principal.class), mockFilter(true), ImportBehavior.ABORT);
Utils.canHandle(mock(Principal.class), mockFilter(true), ImportBehavior.ABORT);
}

@Test(expected = AccessControlException.class)
@@ -137,7 +143,7 @@ public void testCanHandle() throws Exception {

@Test
public void testPrivilegesFromNamesEmptyNames() {
assertArrayEquals(new Privilege[0], Utils.privilegesFromOakNames(Collections.emptySet(), Mockito.mock(PrivilegeManager.class), NamePathMapper.DEFAULT));
assertArrayEquals(new Privilege[0], Utils.privilegesFromOakNames(Collections.emptySet(), mock(PrivilegeManager.class), NamePathMapper.DEFAULT));
}

@Test
@@ -147,7 +153,7 @@ public void testPrivilegesFromNamesInvalidName() throws Exception {

@Test
public void testPrivilegesFromNamesRemapped() throws Exception {
NamePathMapper mapper = Mockito.mock(NamePathMapper.class);
NamePathMapper mapper = mock(NamePathMapper.class);
when(mapper.getJcrName(any())).thenReturn("c");

Privilege[] privs = Utils.privilegesFromOakNames(Sets.newHashSet("a", "b"), mockPrivilegeManager(), mapper);
@@ -156,4 +162,31 @@ public void testPrivilegesFromNamesRemapped() throws Exception {
assertEquals("c", p.getName());
}
}

@Test
public void testReadRestrictionsNoRestrictionTree() {
Tree entryTree = mock(Tree.class);

RestrictionProvider rp = mock(AbstractRestrictionProvider.class);

when(entryTree.hasChild(REP_RESTRICTIONS)).thenReturn(false);
assertSame(Collections.emptySet(), Utils.readRestrictions(rp, "/test/path", entryTree));
verifyNoInteractions(rp);
}

@Test
public void testReadRestrictions() {
Tree restTree = mock(Tree.class);
Tree entryTree = mock(Tree.class);

RestrictionProvider rp = mock(AbstractRestrictionProvider.class);

when(entryTree.hasChild(REP_RESTRICTIONS)).thenReturn(true);
when(entryTree.getChild(REP_RESTRICTIONS)).thenReturn(restTree);
when(restTree.getProperties()).thenReturn(Collections.emptyList());

Utils.readRestrictions(rp, "/test/path", entryTree);
verify(rp).readRestrictions("/test/path", entryTree);
verifyNoMoreInteractions(rp);
}
}

0 comments on commit ed43ffb

Please sign in to comment.