SLING-9212 Distribution code checks for jcr:removeNode permissions on importer side for DELETE request
Incorporating review feedback.
diff --git a/src/main/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategy.java b/src/main/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategy.java
index 95d1266..97b4826 100644
--- a/src/main/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategy.java
+++ b/src/main/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategy.java
@@ -77,7 +77,7 @@
throws RepositoryException, DistributionException {
AccessControlManager acMgr = session.getAccessControlManager();
additionalJcrPrivilegesForAdd = additionalJcrPrivilegesForAdd != null ? additionalJcrPrivilegesForAdd : new String[] {Privilege.JCR_READ};
- Privilege[] privileges = computePrivileges(acMgr, additionalJcrPrivilegesForAdd);
+ Privilege[] privileges = computePrivileges(acMgr, additionalJcrPrivilegesForAdd, jcrPrivilege);
for (String path : paths) {
if (!acMgr.hasPrivileges(path, privileges)) {
throw new DistributionException("Not enough privileges");
@@ -90,7 +90,7 @@
throws RepositoryException, DistributionException {
AccessControlManager acMgr = session.getAccessControlManager();
additionalJcrPrivilegesForDelete = additionalJcrPrivilegesForDelete != null ? additionalJcrPrivilegesForDelete : new String[] {Privilege.JCR_REMOVE_NODE};
- Privilege[] privileges = computePrivileges(acMgr, additionalJcrPrivilegesForDelete);
+ Privilege[] privileges = computePrivileges(acMgr, additionalJcrPrivilegesForDelete, jcrPrivilege);
for (String path : paths) {
String closestParentPath = getClosestParent(session, path);
@@ -113,13 +113,13 @@
return null;
}
- private Privilege[] computePrivileges(AccessControlManager acMgr, String[] jcrPrivileges)
+ private static Privilege[] computePrivileges(AccessControlManager acMgr, String[] jcrPrivileges, String jcrPrivilege)
throws RepositoryException {
ArrayList<Privilege> privileges = new ArrayList<Privilege>();
+ privileges.add(acMgr.privilegeFromName(jcrPrivilege));
for (String privilege : jcrPrivileges) {
privileges.add(acMgr.privilegeFromName(privilege));
}
- privileges.add(acMgr.privilegeFromName(jcrPrivilege));
return privileges.toArray(new Privilege[0]);
}
diff --git a/src/main/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyFactory.java b/src/main/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyFactory.java
index a2b5ba4..e19bc7b 100644
--- a/src/main/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyFactory.java
+++ b/src/main/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyFactory.java
@@ -62,23 +62,23 @@
/**
* privilege ADD request authorization strategy jcr privilege property
*/
- @Property(cardinality = 100, label = "Jcr Privilege Add", description = "Additional Jcr privileges to check for authorizing ADD distribution requests. The privilege is checked for the calling user session.")
- private static final String JCR_ADD_PRIVILEGE = "additionalJcrPrivilegesForAdd";
+ @Property(cardinality = 100, label = "Additional Jcr Privileges for Add", description = "Additional Jcr privileges to check for authorizing ADD distribution requests. The privilege is checked for the calling user session.")
+ private static final String JCR_ADD_PRIVILEGES = "additionalJcrPrivilegesForAdd";
/**
* privilege DELETE request authorization strategy jcr privilege property
*/
- @Property(cardinality = 100, label = "Jcr Privilege Delete", description = "Additional Jcr privileges to check for authorizing DELETE distribution requests. The privilege is checked for the calling user session.")
- private static final String JCR_DELETE_PRIVILEGE = "additionalJcrPrivilegesForDelete";
+ @Property(cardinality = 100, label = "Additional Jcr Privileges for Delete", description = "Additional Jcr privileges to check for authorizing DELETE distribution requests. The privilege is checked for the calling user session.")
+ private static final String JCR_DELETE_PRIVILEGES = "additionalJcrPrivilegesForDelete";
private DistributionRequestAuthorizationStrategy authorizationStrategy;
@Activate
public void activate(BundleContext context, Map<String, Object> config) {
String jcrPrivilege = PropertiesUtil.toString(config.get(JCR_PRIVILEGE), null);
- String[] jcrAddPrivilege = PropertiesUtil.toStringArray(config.get(JCR_ADD_PRIVILEGE), null);
- String[] jcrDeletePrivilege = PropertiesUtil.toStringArray(config.get(JCR_DELETE_PRIVILEGE), null);
- authorizationStrategy = new PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, jcrAddPrivilege, jcrDeletePrivilege);
+ String[] jcrAddPrivileges = PropertiesUtil.toStringArray(config.get(JCR_ADD_PRIVILEGES), null);
+ String[] jcrDeletePrivileges = PropertiesUtil.toStringArray(config.get(JCR_DELETE_PRIVILEGES), null);
+ authorizationStrategy = new PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, jcrAddPrivileges, jcrDeletePrivileges);
}
public void checkPermission(@NotNull ResourceResolver resourceResolver, @NotNull DistributionRequest distributionRequest) throws DistributionException {
diff --git a/src/test/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyTest.java b/src/test/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyTest.java
index 274052b..0335961 100644
--- a/src/test/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyTest.java
+++ b/src/test/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyTest.java
@@ -36,13 +36,10 @@
*/
public class PrivilegeDistributionRequestAuthorizationStrategyTest {
- String jcrPrivilege = "foo";
- String[] additionalJcrPrivilegesForAdd;
- String[] additionalJcrPrivilegesForDelete;
-
@Test(expected = DistributionException.class)
public void testCheckPermissionWithoutSession() throws Exception {
- PrivilegeDistributionRequestAuthorizationStrategy strategy = new PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
+ String jcrPrivilege = "foo";
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, null, null);
DistributionRequest distributionRequest = mock(DistributionRequest.class);
ResourceResolver resourceResolver = mock(ResourceResolver.class);
strategy.checkPermission(resourceResolver, distributionRequest);
@@ -50,7 +47,8 @@
@Test
public void testCheckPermissionWithSession() throws Exception {
- PrivilegeDistributionRequestAuthorizationStrategy strategy = new PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
+ String jcrPrivilege = "foo";
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, null, null);
DistributionRequest distributionRequest = mock(DistributionRequest.class);
ResourceResolver resourceResolver = mock(ResourceResolver.class);
Session session = mock(Session.class);
@@ -60,7 +58,8 @@
@Test(expected = DistributionException.class)
public void testNoPermissionOnAdd() throws Exception {
- PrivilegeDistributionRequestAuthorizationStrategy strategy = new PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
+ String jcrPrivilege = "somePermission";
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, null, null);
DistributionRequest distributionRequest = mock(DistributionRequest.class);
ResourceResolver resourceResolver = mock(ResourceResolver.class);
Session session = mock(Session.class);
@@ -83,7 +82,8 @@
@Test
public void testPermissionOnAdd() throws Exception {
- PrivilegeDistributionRequestAuthorizationStrategy strategy = new PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
+ String jcrPrivilege = "somePermission";
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, null, null);
DistributionRequest distributionRequest = mock(DistributionRequest.class);
ResourceResolver resourceResolver = mock(ResourceResolver.class);
Session session = mock(Session.class);
@@ -97,7 +97,7 @@
when(resourceResolver.adaptTo(Session.class)).thenReturn(session);
String[] paths = new String[]{"/foo"};
for (String path : paths) {
- when(acm.hasPrivileges(path, new Privilege[]{jcrReadPrivilege, privilege})).thenReturn(true);
+ when(acm.hasPrivileges(path, new Privilege[]{privilege, jcrReadPrivilege})).thenReturn(true);
}
when(distributionRequest.getPaths()).thenReturn(paths);
@@ -107,8 +107,9 @@
@Test
public void testAdditionalPermissionsOnAdd() throws Exception {
- additionalJcrPrivilegesForAdd = new String[] {"addPermission"};
- PrivilegeDistributionRequestAuthorizationStrategy strategy = new PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
+ String jcrPrivilege = "somePermission";
+ String[] additionalJcrPrivilegesForAdd = new String[]{"addPermission"};
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, additionalJcrPrivilegesForAdd, null);
DistributionRequest distributionRequest = mock(DistributionRequest.class);
ResourceResolver resourceResolver = mock(ResourceResolver.class);
Session session = mock(Session.class);
@@ -116,15 +117,13 @@
Privilege privilege = mock(Privilege.class);
when(acm.privilegeFromName(jcrPrivilege)).thenReturn(privilege);
Privilege additionalPrivilegeForAdd = mock(Privilege.class);
- for (String privilegeAdd : additionalJcrPrivilegesForAdd) {
- when(acm.privilegeFromName(privilegeAdd)).thenReturn(additionalPrivilegeForAdd);
- }
+ when(acm.privilegeFromName(additionalJcrPrivilegesForAdd[0])).thenReturn(additionalPrivilegeForAdd);
when(session.getAccessControlManager()).thenReturn(acm);
when(resourceResolver.adaptTo(Session.class)).thenReturn(session);
String[] paths = new String[]{"/foo"};
for (String path : paths) {
- when(acm.hasPrivileges(path, new Privilege[]{additionalPrivilegeForAdd, privilege})).thenReturn(true);
+ when(acm.hasPrivileges(path, new Privilege[]{privilege, additionalPrivilegeForAdd})).thenReturn(true);
}
when(distributionRequest.getPaths()).thenReturn(paths);
@@ -134,7 +133,8 @@
@Test(expected = DistributionException.class)
public void testNoPermissionOnDelete() throws Exception {
- PrivilegeDistributionRequestAuthorizationStrategy strategy = new PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
+ String jcrPrivilege = "somePermission";
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, null, null);
DistributionRequest distributionRequest = mock(DistributionRequest.class);
ResourceResolver resourceResolver = mock(ResourceResolver.class);
Session session = mock(Session.class);
@@ -158,7 +158,8 @@
@Test
public void testPermissionOnDelete() throws Exception {
- PrivilegeDistributionRequestAuthorizationStrategy strategy = new PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
+ String jcrPrivilege = "somePermission";
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, null, null);
DistributionRequest distributionRequest = mock(DistributionRequest.class);
ResourceResolver resourceResolver = mock(ResourceResolver.class);
Session session = mock(Session.class);
@@ -172,7 +173,7 @@
when(resourceResolver.adaptTo(Session.class)).thenReturn(session);
String[] paths = new String[]{"/foo"};
for (String path : paths) {
- when(acm.hasPrivileges(path, new Privilege[]{jcrDeletePrivilege, privilege})).thenReturn(true);
+ when(acm.hasPrivileges(path, new Privilege[]{privilege, jcrDeletePrivilege})).thenReturn(true);
when(session.nodeExists(path)).thenReturn(true);
}
when(distributionRequest.getPaths()).thenReturn(paths);
@@ -183,8 +184,9 @@
@Test
public void testAdditionalPermissionsOnDelete() throws Exception {
- additionalJcrPrivilegesForDelete = new String[] {"deletePermission"};
- PrivilegeDistributionRequestAuthorizationStrategy strategy = new PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
+ String jcrPrivilege = "somePermission";
+ String[] additionalJcrPrivilegesForDelete = new String[]{"deletePermission"};
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, null, additionalJcrPrivilegesForDelete);
DistributionRequest distributionRequest = mock(DistributionRequest.class);
ResourceResolver resourceResolver = mock(ResourceResolver.class);
Session session = mock(Session.class);
@@ -192,15 +194,13 @@
Privilege privilege = mock(Privilege.class);
when(acm.privilegeFromName(jcrPrivilege)).thenReturn(privilege);
Privilege additionalPrivilegeForDelete = mock(Privilege.class);
- for (String privilegeDelete : additionalJcrPrivilegesForDelete) {
- when(acm.privilegeFromName(privilegeDelete)).thenReturn(additionalPrivilegeForDelete);
- }
+ when(acm.privilegeFromName(additionalJcrPrivilegesForDelete[0])).thenReturn(additionalPrivilegeForDelete);
when(session.getAccessControlManager()).thenReturn(acm);
when(resourceResolver.adaptTo(Session.class)).thenReturn(session);
String[] paths = new String[]{"/foo"};
for (String path : paths) {
- when(acm.hasPrivileges(path, new Privilege[]{additionalPrivilegeForDelete, privilege})).thenReturn(true);
+ when(acm.hasPrivileges(path, new Privilege[]{privilege, additionalPrivilegeForDelete})).thenReturn(true);
when(session.nodeExists(path)).thenReturn(true);
}
when(distributionRequest.getPaths()).thenReturn(paths);
