Index: oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugAccessControlManager.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugAccessControlManager.java (revision 1849086) +++ oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugAccessControlManager.java (date 1545141677000) @@ -43,6 +43,7 @@ import org.apache.jackrabbit.oak.spi.nodetype.NodeTypeConstants; import org.apache.jackrabbit.oak.plugins.tree.TreeUtil; import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.PolicyOwner; +import org.apache.jackrabbit.oak.spi.security.authorization.cug.CugExclude; import org.apache.jackrabbit.oak.spi.security.authorization.cug.CugPolicy; import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters; import org.apache.jackrabbit.oak.spi.security.SecurityProvider; @@ -68,16 +69,19 @@ private static final Logger log = LoggerFactory.getLogger(CugAccessControlManager.class); private final Set supportedPaths; + private final CugExclude cugExclude; private final ConfigurationParameters config; private final PrincipalManager principalManager; CugAccessControlManager(@NotNull Root root, - @NotNull NamePathMapper namePathMapper, - @NotNull SecurityProvider securityProvider, - @NotNull Set supportedPaths) { + @NotNull NamePathMapper namePathMapper, + @NotNull SecurityProvider securityProvider, + @NotNull Set supportedPaths, + @NotNull CugExclude cugExclude) { super(root, namePathMapper, securityProvider); this.supportedPaths = supportedPaths; + this.cugExclude = cugExclude; config = securityProvider.getConfiguration(AuthorizationConfiguration.class).getParameters(); principalManager = securityProvider.getConfiguration(PrincipalConfiguration.class).getPrincipalManager(root, namePathMapper); @@ -139,7 +143,7 @@ } else { CugPolicy cug = getCugPolicy(oakPath); if (cug == null) { - cug = new CugPolicyImpl(oakPath, getNamePathMapper(), principalManager, CugUtil.getImportBehavior(config)); + cug = new CugPolicyImpl(oakPath, getNamePathMapper(), principalManager, CugUtil.getImportBehavior(config), cugExclude); return new AccessControlPolicyIteratorAdapter(ImmutableSet.of(cug)); } else { return AccessControlPolicyIteratorAdapter.EMPTY; @@ -246,7 +250,7 @@ private CugPolicy getCugPolicy(@NotNull String oakPath, @NotNull Tree tree) { Tree cug = tree.getChild(REP_CUG_POLICY); if (CugUtil.definesCug(cug)) { - return new CugPolicyImpl(oakPath, getNamePathMapper(), principalManager, CugUtil.getImportBehavior(config), getPrincipals(cug)); + return new CugPolicyImpl(oakPath, getNamePathMapper(), principalManager, CugUtil.getImportBehavior(config), cugExclude, getPrincipals(cug)); } else { return null; } Index: oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugConfiguration.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugConfiguration.java (revision 1849086) +++ oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugConfiguration.java (date 1545140657000) @@ -120,7 +120,7 @@ @NotNull @Override public AccessControlManager getAccessControlManager(@NotNull Root root, @NotNull NamePathMapper namePathMapper) { - return new CugAccessControlManager(root, namePathMapper, getSecurityProvider(), supportedPaths); + return new CugAccessControlManager(root, namePathMapper, getSecurityProvider(), supportedPaths, getExclude()); } @NotNull Index: oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugPolicyImpl.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugPolicyImpl.java (revision 1849086) +++ oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugPolicyImpl.java (date 1545140657000) @@ -27,6 +27,7 @@ import com.google.common.collect.Sets; import org.apache.jackrabbit.api.security.principal.PrincipalManager; import org.apache.jackrabbit.oak.namepath.NamePathMapper; +import org.apache.jackrabbit.oak.spi.security.authorization.cug.CugExclude; import org.apache.jackrabbit.oak.spi.security.authorization.cug.CugPolicy; import org.apache.jackrabbit.oak.spi.xml.ImportBehavior; import org.jetbrains.annotations.NotNull; @@ -46,22 +47,24 @@ private final NamePathMapper namePathMapper; private final PrincipalManager principalManager; private final int importBehavior; + private final CugExclude cugExclude; private final Set principals = new HashSet<>(); CugPolicyImpl(@NotNull String oakPath, @NotNull NamePathMapper namePathMapper, - @NotNull PrincipalManager principalManager, int importBehavior) { - this(oakPath, namePathMapper, principalManager, importBehavior, Collections.emptySet()); + @NotNull PrincipalManager principalManager, int importBehavior, @NotNull CugExclude cugExclude) { + this(oakPath, namePathMapper, principalManager, importBehavior, cugExclude, Collections.emptySet()); } CugPolicyImpl(@NotNull String oakPath, @NotNull NamePathMapper namePathMapper, @NotNull PrincipalManager principalManager, int importBehavior, - @NotNull Set principals) { + @NotNull CugExclude cugExclude, @NotNull Set principals) { ImportBehavior.nameFromValue(importBehavior); this.oakPath = oakPath; this.namePathMapper = namePathMapper; this.principalManager = principalManager; this.importBehavior = importBehavior; + this.cugExclude = cugExclude; this.principals.addAll(principals); } @@ -128,6 +131,11 @@ throw new AccessControlException("Invalid principal " + name); } + if (cugExclude.isExcluded(Collections.singleton(principal))) { + log.warn("Attempt to add excluded principal {} to CUG.", principal); + return false; + } + boolean isValid = true; switch (importBehavior) { case ImportBehavior.ABORT: @@ -137,7 +145,7 @@ break; case ImportBehavior.IGNORE: if (!principalManager.hasPrincipal(name)) { - log.debug("Ignoring unknown principal " + name); + log.debug("Ignoring unknown principal {}", name); isValid = false; } break; Index: oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/AbstractCugTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/AbstractCugTest.java (revision 1849086) +++ oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/AbstractCugTest.java (date 1545140657000) @@ -43,6 +43,7 @@ import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters; import org.apache.jackrabbit.oak.spi.security.SecurityProvider; import org.apache.jackrabbit.oak.spi.security.authorization.AuthorizationConfiguration; +import org.apache.jackrabbit.oak.spi.security.authorization.cug.CugExclude; import org.apache.jackrabbit.oak.spi.security.authorization.cug.CugPolicy; import org.apache.jackrabbit.oak.spi.security.authorization.permission.PermissionProvider; import org.apache.jackrabbit.oak.spi.security.authorization.permission.TreePermission; @@ -190,6 +191,10 @@ root.commit(); } + CugExclude getExclude() { + return new CugExclude.Default(); + } + void createCug(@NotNull String absPath, @NotNull Principal principal) throws RepositoryException { AccessControlManager acMgr = getAccessControlManager(root); AccessControlPolicyIterator it = acMgr.getApplicablePolicies(absPath); Index: oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugAccessControlManagerTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugAccessControlManagerTest.java (revision 1849086) +++ oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugAccessControlManagerTest.java (date 1545140657000) @@ -65,11 +65,11 @@ public void before() throws Exception { super.before(); - cugAccessControlManager = new CugAccessControlManager(root, NamePathMapper.DEFAULT, getSecurityProvider(), ImmutableSet.copyOf(SUPPORTED_PATHS)); + cugAccessControlManager = new CugAccessControlManager(root, NamePathMapper.DEFAULT, getSecurityProvider(), ImmutableSet.copyOf(SUPPORTED_PATHS), getExclude()); } private CugPolicy createCug(@NotNull String path) { - return new CugPolicyImpl(path, NamePathMapper.DEFAULT, getPrincipalManager(root), ImportBehavior.ABORT); + return new CugPolicyImpl(path, NamePathMapper.DEFAULT, getPrincipalManager(root), ImportBehavior.ABORT, getExclude()); } private CugPolicy getApplicableCug(@NotNull String path) throws RepositoryException { @@ -224,7 +224,7 @@ ConfigurationParameters config = ConfigurationParameters.of(AuthorizationConfiguration.NAME, ConfigurationParameters.of( CugConstants.PARAM_CUG_SUPPORTED_PATHS, SUPPORTED_PATHS, CugConstants.PARAM_CUG_ENABLED, false)); - CugAccessControlManager acMgr = new CugAccessControlManager(root, NamePathMapper.DEFAULT, CugSecurityProvider.newTestSecurityProvider(config), ImmutableSet.copyOf(SUPPORTED_PATHS)); + CugAccessControlManager acMgr = new CugAccessControlManager(root, NamePathMapper.DEFAULT, CugSecurityProvider.newTestSecurityProvider(config), ImmutableSet.copyOf(SUPPORTED_PATHS), getExclude()); AccessControlPolicy[] policies = acMgr.getEffectivePolicies(SUPPORTED_PATH); assertEquals(0, policies.length); @@ -313,7 +313,7 @@ Tree supportedTree = root.getTree(SUPPORTED_PATH); new NodeUtil(supportedTree).addChild(REP_CUG_POLICY, NodeTypeConstants.NT_OAK_UNSTRUCTURED); - cugAccessControlManager.setPolicy(SUPPORTED_PATH, new CugPolicyImpl(SUPPORTED_PATH, NamePathMapper.DEFAULT, getPrincipalManager(root), ImportBehavior.BESTEFFORT)); + cugAccessControlManager.setPolicy(SUPPORTED_PATH, new CugPolicyImpl(SUPPORTED_PATH, NamePathMapper.DEFAULT, getPrincipalManager(root), ImportBehavior.BESTEFFORT, getExclude())); } @Test @@ -380,7 +380,7 @@ Tree supportedTree = root.getTree(SUPPORTED_PATH); new NodeUtil(supportedTree).addChild(REP_CUG_POLICY, NodeTypeConstants.NT_OAK_UNSTRUCTURED); - cugAccessControlManager.removePolicy(SUPPORTED_PATH, new CugPolicyImpl(SUPPORTED_PATH, NamePathMapper.DEFAULT, getPrincipalManager(root), ImportBehavior.BESTEFFORT)); + cugAccessControlManager.removePolicy(SUPPORTED_PATH, new CugPolicyImpl(SUPPORTED_PATH, NamePathMapper.DEFAULT, getPrincipalManager(root), ImportBehavior.BESTEFFORT, getExclude())); } @Test(expected = PathNotFoundException.class) Index: oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugPolicyImplTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugPolicyImplTest.java (revision 1849086) +++ oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugPolicyImplTest.java (date 1545143256000) @@ -16,30 +16,32 @@ */ package org.apache.jackrabbit.oak.spi.security.authorization.cug.impl; -import java.security.Principal; -import java.util.Iterator; -import java.util.Set; -import javax.jcr.security.AccessControlException; - import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import org.apache.jackrabbit.api.security.authorization.PrincipalSetPolicy; import org.apache.jackrabbit.api.security.principal.PrincipalManager; import org.apache.jackrabbit.oak.AbstractSecurityTest; -import org.apache.jackrabbit.oak.namepath.impl.LocalNameMapper; import org.apache.jackrabbit.oak.namepath.NamePathMapper; +import org.apache.jackrabbit.oak.namepath.impl.LocalNameMapper; import org.apache.jackrabbit.oak.namepath.impl.NamePathMapperImpl; +import org.apache.jackrabbit.oak.spi.security.authorization.cug.CugExclude; import org.apache.jackrabbit.oak.spi.security.authorization.cug.CugPolicy; +import org.apache.jackrabbit.oak.spi.security.principal.AdminPrincipal; import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal; import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl; +import org.apache.jackrabbit.oak.spi.security.principal.SystemUserPrincipal; import org.apache.jackrabbit.oak.spi.xml.ImportBehavior; import org.jetbrains.annotations.NotNull; import org.junit.Test; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotSame; -import static org.junit.Assert.assertTrue; +import javax.jcr.security.AccessControlException; +import java.security.Principal; +import java.util.Collections; +import java.util.Iterator; +import java.util.Set; + +import static org.junit.Assert.*; public class CugPolicyImplTest extends AbstractSecurityTest { @@ -48,6 +50,8 @@ private Principal testPrincipal = new PrincipalImpl("test"); Set principals = ImmutableSet.of(testPrincipal); + private CugExclude exclude = new CugExclude.Default(); + @Override public void before() throws Exception { super.before(); @@ -56,13 +60,29 @@ } private CugPolicyImpl createEmptyCugPolicy() { - return new CugPolicyImpl(path, NamePathMapper.DEFAULT, principalManager, ImportBehavior.ABORT); + return createEmptyCugPolicy(ImportBehavior.ABORT); + } + + private CugPolicyImpl createEmptyCugPolicy(int importBehavior) { + return new CugPolicyImpl(path, NamePathMapper.DEFAULT, principalManager, importBehavior, exclude); } private CugPolicyImpl createCugPolicy(@NotNull Set principals) { - return new CugPolicyImpl(path, NamePathMapper.DEFAULT, principalManager, ImportBehavior.ABORT, principals); + return createCugPolicy(ImportBehavior.ABORT, principals); + } + + private CugPolicyImpl createCugPolicy(int importBehavior, @NotNull Set principals) { + return new CugPolicyImpl(path, NamePathMapper.DEFAULT, principalManager, importBehavior, exclude, principals); } + private Principal getExcludedPrincipal() { + return new SystemUserPrincipal() { + @Override + public String getName() { + return "excluded"; + } + }; + } @Test public void testPrincipalSetPolicy() { assertTrue(createCugPolicy(principals) instanceof PrincipalSetPolicy); @@ -115,7 +135,7 @@ @Test(expected = AccessControlException.class) public void testAddInvalidPrincipalsAbort() throws Exception { - CugPolicy cug = new CugPolicyImpl(path, NamePathMapper.DEFAULT, principalManager, ImportBehavior.ABORT); + CugPolicy cug = createEmptyCugPolicy(ImportBehavior.ABORT); cug.addPrincipals( EveryonePrincipal.getInstance(), new PrincipalImpl("unknown")); @@ -123,7 +143,7 @@ @Test public void testAddInvalidPrincipalsBestEffort() throws Exception { - CugPolicy cug = new CugPolicyImpl(path, NamePathMapper.DEFAULT, principalManager, ImportBehavior.BESTEFFORT, principals); + CugPolicy cug = createCugPolicy(ImportBehavior.BESTEFFORT, principals); assertTrue(cug.addPrincipals( EveryonePrincipal.getInstance(), new PrincipalImpl("unknown"))); @@ -134,7 +154,7 @@ @Test public void testAddInvalidPrincipalsIgnore() throws Exception { - CugPolicy cug = new CugPolicyImpl(path, NamePathMapper.DEFAULT, principalManager, ImportBehavior.IGNORE, principals); + CugPolicy cug = createCugPolicy(ImportBehavior.IGNORE, principals); assertTrue(cug.addPrincipals( new PrincipalImpl("unknown"), EveryonePrincipal.getInstance())); @@ -147,7 +167,7 @@ @Test public void testAddContainedPrincipal() throws Exception { - CugPolicy cug = new CugPolicyImpl(path, NamePathMapper.DEFAULT, principalManager, ImportBehavior.BESTEFFORT, principals); + CugPolicy cug = createCugPolicy(ImportBehavior.BESTEFFORT, principals); assertFalse(cug.addPrincipals( new PrincipalImpl("test"))); @@ -156,7 +176,7 @@ @Test public void testAddNullPrincipal() throws Exception { - CugPolicy cug = new CugPolicyImpl(path, NamePathMapper.DEFAULT, principalManager, ImportBehavior.ABORT, principals); + CugPolicy cug = createCugPolicy(ImportBehavior.ABORT, principals); assertTrue(cug.addPrincipals(EveryonePrincipal.getInstance(), null)); assertTrue(cug.getPrincipals().contains(EveryonePrincipal.getInstance())); @@ -165,13 +185,13 @@ @Test(expected = AccessControlException.class) public void testAddEmptyPrincipalName() throws Exception { - CugPolicy cug = new CugPolicyImpl(path, NamePathMapper.DEFAULT, principalManager, ImportBehavior.BESTEFFORT); + CugPolicy cug = createEmptyCugPolicy(ImportBehavior.BESTEFFORT); cug.addPrincipals(new PrincipalImpl("")); } @Test(expected = AccessControlException.class) public void testAddNullPrincipalName() throws Exception { - CugPolicy cug = new CugPolicyImpl(path, NamePathMapper.DEFAULT, principalManager, ImportBehavior.BESTEFFORT); + CugPolicy cug = createEmptyCugPolicy(ImportBehavior.BESTEFFORT); cug.addPrincipals(new Principal() { @Override public String getName() { @@ -182,9 +202,7 @@ @Test public void testRemovePrincipals() throws Exception { - CugPolicy cug = new CugPolicyImpl(path, NamePathMapper.DEFAULT, principalManager, - ImportBehavior.BESTEFFORT, - ImmutableSet.of(testPrincipal, EveryonePrincipal.getInstance())); + CugPolicy cug = createCugPolicy(ImportBehavior.BESTEFFORT, ImmutableSet.of(testPrincipal, EveryonePrincipal.getInstance())); assertFalse(cug.removePrincipals(new PrincipalImpl("unknown"))); assertTrue(cug.removePrincipals(testPrincipal, EveryonePrincipal.getInstance(), new PrincipalImpl("unknown"))); @@ -193,7 +211,7 @@ @Test public void testRemoveNullPrincipal() throws Exception { - CugPolicy cug = new CugPolicyImpl(path, NamePathMapper.DEFAULT, principalManager, ImportBehavior.ABORT, principals); + CugPolicy cug = createCugPolicy(ImportBehavior.ABORT, principals); assertTrue(cug.removePrincipals(testPrincipal, null)); assertTrue(cug.getPrincipals().isEmpty()); @@ -210,12 +228,42 @@ String oakPath = "/oak:testPath"; NamePathMapper mapper = new NamePathMapperImpl(new LocalNameMapper(root, ImmutableMap.of("quercus", "http://jackrabbit.apache.org/oak/ns/1.0"))); - CugPolicy empty = new CugPolicyImpl(oakPath, mapper, principalManager, ImportBehavior.ABORT); + CugPolicy empty = new CugPolicyImpl(oakPath, mapper, principalManager, ImportBehavior.ABORT, exclude); assertEquals("/quercus:testPath", empty.getPath()); } @Test(expected = IllegalArgumentException.class) public void testInvalidImportBehavior() { - CugPolicy cug = new CugPolicyImpl(path, NamePathMapper.DEFAULT, principalManager, -1, principals); + CugPolicy cug = createCugPolicy(-1, principals); + } + + @Test + public void testAddSingleExcludedPrincipal() throws Exception { + CugPolicy cug = createEmptyCugPolicy(ImportBehavior.ABORT); + + assertFalse(cug.addPrincipals(getExcludedPrincipal())); + } + + @Test + public void testAddExcludedPrincipal() throws Exception { + CugPolicyImpl cug = createEmptyCugPolicy(ImportBehavior.ABORT); + + Principal excluded = getExcludedPrincipal(); + assertTrue(cug.addPrincipals(EveryonePrincipal.getInstance(), excluded)); + assertFalse(Iterables.contains(cug.getPrincipalNames(), excluded.getName())); + } + + @Test + public void testExcludedPrincipalAddedBefore() throws Exception { + Principal excluded = getExcludedPrincipal(); + CugPolicyImpl cug = createCugPolicy(ImportBehavior.ABORT, Collections.singleton(excluded)); + assertTrue(Iterables.contains(cug.getPrincipalNames(), excluded.getName())); + } + + @Test + public void removeExcludedPrincipal() throws Exception { + Principal excluded = getExcludedPrincipal(); + CugPolicyImpl cug = createCugPolicy(ImportBehavior.ABORT, Collections.singleton(excluded)); + assertTrue(cug.removePrincipals(excluded)); } }