actinium15 commented on a change in pull request #40:
URL:
https://github.com/apache/sling-org-apache-sling-distribution-core/pull/40#discussion_r413450284
##########
File path:
src/main/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyFactory.java
##########
@@ -59,13 +59,26 @@
@Property(label = "Jcr Privilege", description = "Jcr privilege to check
for authorizing distribution requests. The privilege is checked for the calling
user session.")
private static final String JCR_PRIVILEGE = "jcrPrivilege";
+ /**
+ * 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.")
Review comment:
```suggestion
@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.")
```
##########
File path:
src/main/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyFactory.java
##########
@@ -59,13 +59,26 @@
@Property(label = "Jcr Privilege", description = "Jcr privilege to check
for authorizing distribution requests. The privilege is checked for the calling
user session.")
private static final String JCR_PRIVILEGE = "jcrPrivilege";
+ /**
+ * 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";
Review comment:
```suggestion
private static final String JCR_ADD_PRIVILEGES =
"additionalJcrPrivilegesForAdd";
```
##########
File path:
src/main/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyFactory.java
##########
@@ -59,13 +59,26 @@
@Property(label = "Jcr Privilege", description = "Jcr privilege to check
for authorizing distribution requests. The privilege is checked for the calling
user session.")
private static final String JCR_PRIVILEGE = "jcrPrivilege";
+ /**
+ * 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";
+
+ /**
+ * 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.")
Review comment:
```suggestion
@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.")
```
##########
File path:
src/main/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyFactory.java
##########
@@ -59,13 +59,26 @@
@Property(label = "Jcr Privilege", description = "Jcr privilege to check
for authorizing distribution requests. The privilege is checked for the calling
user session.")
private static final String JCR_PRIVILEGE = "jcrPrivilege";
+ /**
+ * 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";
+
+ /**
+ * 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";
Review comment:
```suggestion
private static final String JCR_DELETE_PRIVILEGES =
"additionalJcrPrivilegesForDelete";
```
##########
File path:
src/main/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyFactory.java
##########
@@ -59,13 +59,26 @@
@Property(label = "Jcr Privilege", description = "Jcr privilege to check
for authorizing distribution requests. The privilege is checked for the calling
user session.")
private static final String JCR_PRIVILEGE = "jcrPrivilege";
+ /**
+ * 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";
+
+ /**
+ * 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";
private DistributionRequestAuthorizationStrategy authorizationStrategy;
@Activate
public void activate(BundleContext context, Map<String, Object> config) {
String jcrPrivilege =
PropertiesUtil.toString(config.get(JCR_PRIVILEGE), null);
- authorizationStrategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege);
+ 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);
Review comment:
```suggestion
authorizationStrategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege,
jcrAddPrivileges, jcrDeletePrivileges);
```
##########
File path:
src/main/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyFactory.java
##########
@@ -59,13 +59,26 @@
@Property(label = "Jcr Privilege", description = "Jcr privilege to check
for authorizing distribution requests. The privilege is checked for the calling
user session.")
private static final String JCR_PRIVILEGE = "jcrPrivilege";
+ /**
+ * 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";
+
+ /**
+ * 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";
private DistributionRequestAuthorizationStrategy authorizationStrategy;
@Activate
public void activate(BundleContext context, Map<String, Object> config) {
String jcrPrivilege =
PropertiesUtil.toString(config.get(JCR_PRIVILEGE), null);
- authorizationStrategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege);
+ String[] jcrAddPrivilege =
PropertiesUtil.toStringArray(config.get(JCR_ADD_PRIVILEGE), null);
+ String[] jcrDeletePrivilege =
PropertiesUtil.toStringArray(config.get(JCR_DELETE_PRIVILEGE), null);
Review comment:
```suggestion
String[] jcrDeletePrivileges =
PropertiesUtil.toStringArray(config.get(JCR_DELETE_PRIVILEGES), null);
```
##########
File path:
src/main/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyFactory.java
##########
@@ -59,13 +59,26 @@
@Property(label = "Jcr Privilege", description = "Jcr privilege to check
for authorizing distribution requests. The privilege is checked for the calling
user session.")
private static final String JCR_PRIVILEGE = "jcrPrivilege";
+ /**
+ * 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";
+
+ /**
+ * 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";
private DistributionRequestAuthorizationStrategy authorizationStrategy;
@Activate
public void activate(BundleContext context, Map<String, Object> config) {
String jcrPrivilege =
PropertiesUtil.toString(config.get(JCR_PRIVILEGE), null);
- authorizationStrategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege);
+ String[] jcrAddPrivilege =
PropertiesUtil.toStringArray(config.get(JCR_ADD_PRIVILEGE), null);
Review comment:
```suggestion
String[] jcrAddPrivileges =
PropertiesUtil.toStringArray(config.get(JCR_ADD_PRIVILEGES), null);
```
##########
File path:
src/test/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyTest.java
##########
@@ -58,8 +60,7 @@ public void testCheckPermissionWithSession() throws Exception
{
@Test(expected = DistributionException.class)
public void testNoPermissionOnAdd() throws Exception {
- String jcrPrivilege = "somePermission";
- PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege);
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege,
additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
Review comment:
```suggestion
String jcrPrivilege = "somePermission";
PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, null, null);
```
##########
File path:
src/test/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyTest.java
##########
@@ -132,22 +158,49 @@ public void testNoPermissionOnDelete() throws Exception {
@Test
public void testPermissionOnDelete() throws Exception {
- String jcrPrivilege = "somePermission";
- PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege);
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege,
additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
DistributionRequest distributionRequest =
mock(DistributionRequest.class);
ResourceResolver resourceResolver = mock(ResourceResolver.class);
Session session = mock(Session.class);
AccessControlManager acm = mock(AccessControlManager.class);
Privilege privilege = mock(Privilege.class);
when(acm.privilegeFromName(jcrPrivilege)).thenReturn(privilege);
- Privilege jcrReadPrivilege = mock(Privilege.class);
-
when(acm.privilegeFromName(Privilege.JCR_REMOVE_NODE)).thenReturn(jcrReadPrivilege);
+ Privilege jcrDeletePrivilege = mock(Privilege.class);
+
when(acm.privilegeFromName(Privilege.JCR_REMOVE_NODE)).thenReturn(jcrDeletePrivilege);
+
+ 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[]{jcrDeletePrivilege,
privilege})).thenReturn(true);
+ when(session.nodeExists(path)).thenReturn(true);
+ }
+ when(distributionRequest.getPaths()).thenReturn(paths);
+
+
when(distributionRequest.getRequestType()).thenReturn(DistributionRequestType.DELETE);
+ strategy.checkPermission(resourceResolver, distributionRequest);
+ }
+
+ @Test
+ public void testAdditionalPermissionsOnDelete() throws Exception {
+ additionalJcrPrivilegesForDelete = new String[] {"deletePermission"};
Review comment:
```suggestion
String jcrPrivilege = "somePermission";
additionalJcrPrivilegesForDelete = new String[] {"deletePermission"};
```
##########
File path:
src/test/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyTest.java
##########
@@ -97,7 +97,34 @@ public void testPermissionOnAdd() throws Exception {
when(resourceResolver.adaptTo(Session.class)).thenReturn(session);
String[] paths = new String[]{"/foo"};
for (String path : paths) {
- when(acm.hasPrivileges(path, new Privilege[]{privilege,
jcrReadPrivilege})).thenReturn(true);
+ when(acm.hasPrivileges(path, new Privilege[]{jcrReadPrivilege,
privilege})).thenReturn(true);
+ }
+ when(distributionRequest.getPaths()).thenReturn(paths);
+
+
when(distributionRequest.getRequestType()).thenReturn(DistributionRequestType.ADD);
+ strategy.checkPermission(resourceResolver, distributionRequest);
+ }
+
+ @Test
+ public void testAdditionalPermissionsOnAdd() throws Exception {
+ additionalJcrPrivilegesForAdd = new String[] {"addPermission"};
Review comment:
```suggestion
String jcrPrivilege = "somePermission";
additionalJcrPrivilegesForAdd = new String[] {"addPermission"};
```
##########
File path:
src/main/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategy.java
##########
@@ -105,5 +113,14 @@ private String getClosestParent(Session session, String
path) throws RepositoryE
return null;
}
+ private Privilege[] computePrivileges(AccessControlManager acMgr, String[]
jcrPrivileges)
Review comment:
this method should be made `static` and `jcrPrivilege` should be
supplied to it as its 2nd argument.
##########
File path:
src/test/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyTest.java
##########
@@ -36,19 +36,21 @@
*/
public class PrivilegeDistributionRequestAuthorizationStrategyTest {
+ String jcrPrivilege = "foo";
+ String[] additionalJcrPrivilegesForAdd;
+ String[] additionalJcrPrivilegesForDelete;
+
@Test(expected = DistributionException.class)
public void testCheckPermissionWithoutSession() throws Exception {
- String jcrPrivilege = "foo";
- PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege);
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege,
additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
DistributionRequest distributionRequest =
mock(DistributionRequest.class);
ResourceResolver resourceResolver = mock(ResourceResolver.class);
strategy.checkPermission(resourceResolver, distributionRequest);
}
@Test
public void testCheckPermissionWithSession() throws Exception {
- String jcrPrivilege = "foo";
- PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege);
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege,
additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
Review comment:
```suggestion
String jcrPrivilege = "foo";
PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, null, null);
```
##########
File path:
src/test/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyTest.java
##########
@@ -97,7 +97,34 @@ public void testPermissionOnAdd() throws Exception {
when(resourceResolver.adaptTo(Session.class)).thenReturn(session);
String[] paths = new String[]{"/foo"};
for (String path : paths) {
- when(acm.hasPrivileges(path, new Privilege[]{privilege,
jcrReadPrivilege})).thenReturn(true);
+ when(acm.hasPrivileges(path, new Privilege[]{jcrReadPrivilege,
privilege})).thenReturn(true);
Review comment:
Unchanged test-setup and asserts are crucial in clarifying that the
changes are backwards compatible.
Though I _know_ that's not an issue here, still, rather than changing the
test, please swap the order in which the privileges are added in
`PrivilegeDistributionRequestAuthorizationStrategy.computePrivileges` itself
and revert the changes here.
##########
File path:
src/test/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyTest.java
##########
@@ -107,8 +134,7 @@ public void testPermissionOnAdd() throws Exception {
@Test(expected = DistributionException.class)
public void testNoPermissionOnDelete() throws Exception {
- String jcrPrivilege = "somePermission";
- PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege);
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege,
additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
Review comment:
```suggestion
String jcrPrivilege = "somePermission";
PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, null, null);
```
##########
File path:
src/test/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyTest.java
##########
@@ -97,7 +97,34 @@ public void testPermissionOnAdd() throws Exception {
when(resourceResolver.adaptTo(Session.class)).thenReturn(session);
String[] paths = new String[]{"/foo"};
for (String path : paths) {
- when(acm.hasPrivileges(path, new Privilege[]{privilege,
jcrReadPrivilege})).thenReturn(true);
+ when(acm.hasPrivileges(path, new Privilege[]{jcrReadPrivilege,
privilege})).thenReturn(true);
+ }
+ when(distributionRequest.getPaths()).thenReturn(paths);
+
+
when(distributionRequest.getRequestType()).thenReturn(DistributionRequestType.ADD);
+ strategy.checkPermission(resourceResolver, distributionRequest);
+ }
+
+ @Test
+ public void testAdditionalPermissionsOnAdd() throws Exception {
+ additionalJcrPrivilegesForAdd = new String[] {"addPermission"};
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege,
additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
+ DistributionRequest distributionRequest =
mock(DistributionRequest.class);
+ ResourceResolver resourceResolver = mock(ResourceResolver.class);
+ Session session = mock(Session.class);
+ AccessControlManager acm = mock(AccessControlManager.class);
+ Privilege privilege = mock(Privilege.class);
+ when(acm.privilegeFromName(jcrPrivilege)).thenReturn(privilege);
+ Privilege additionalPrivilegeForAdd = mock(Privilege.class);
+ for (String privilegeAdd : additionalJcrPrivilegesForAdd) {
Review comment:
either this `for` loop should be dropped, or the test adjusted to
actually create correct fixtures. Right now, if one were to add another
permission to `additionalJcrPrivilegesForAdd` array, the fixture below will
return _identical_ mock for both of them. (not to mention the test will likely
error-out because the fixture for `acm.hasPrivileges` won't work due to
unexpected arguments).
Similarly, if you decide that `for` loop _IS NOT_ to be dropped, use
`Answer` to verify the arguments sent while setting up the fixture for
`acm.hasPrivileges`
##########
File path:
src/test/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyTest.java
##########
@@ -82,8 +83,7 @@ public void testNoPermissionOnAdd() throws Exception {
@Test
public void testPermissionOnAdd() throws Exception {
- String jcrPrivilege = "somePermission";
- PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege);
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege,
additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
Review comment:
```suggestion
String jcrPrivilege = "somePermission";
PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, null, null);
```
##########
File path:
src/test/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyTest.java
##########
@@ -36,19 +36,21 @@
*/
public class PrivilegeDistributionRequestAuthorizationStrategyTest {
+ String jcrPrivilege = "foo";
+ String[] additionalJcrPrivilegesForAdd;
Review comment:
consider removing fields `jcrPrivilege`, `additionalJcrPrivilegesForAdd`
and `additionalJcrPrivilegesForDelete`. Restore earlier, method-local
`jcrPrivilege` and pass `null` directly while intiallizing
`PrivilegeDistributionRequestAuthorizationStrategy` in the existing tests
asserting existing behaviour (of adding default JCR_READ/JCR_REMOVE_NODE
privileges as applicable)
##########
File path:
src/test/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyTest.java
##########
@@ -97,7 +97,34 @@ public void testPermissionOnAdd() throws Exception {
when(resourceResolver.adaptTo(Session.class)).thenReturn(session);
String[] paths = new String[]{"/foo"};
for (String path : paths) {
- when(acm.hasPrivileges(path, new Privilege[]{privilege,
jcrReadPrivilege})).thenReturn(true);
+ when(acm.hasPrivileges(path, new Privilege[]{jcrReadPrivilege,
privilege})).thenReturn(true);
+ }
+ when(distributionRequest.getPaths()).thenReturn(paths);
+
+
when(distributionRequest.getRequestType()).thenReturn(DistributionRequestType.ADD);
+ strategy.checkPermission(resourceResolver, distributionRequest);
+ }
+
+ @Test
+ public void testAdditionalPermissionsOnAdd() throws Exception {
+ additionalJcrPrivilegesForAdd = new String[] {"addPermission"};
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege,
additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
Review comment:
```suggestion
PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege,
additionalJcrPrivilegesForAdd, null);
```
##########
File path:
src/test/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyTest.java
##########
@@ -132,22 +158,49 @@ public void testNoPermissionOnDelete() throws Exception {
@Test
public void testPermissionOnDelete() throws Exception {
- String jcrPrivilege = "somePermission";
- PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege);
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege,
additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
DistributionRequest distributionRequest =
mock(DistributionRequest.class);
ResourceResolver resourceResolver = mock(ResourceResolver.class);
Session session = mock(Session.class);
AccessControlManager acm = mock(AccessControlManager.class);
Privilege privilege = mock(Privilege.class);
when(acm.privilegeFromName(jcrPrivilege)).thenReturn(privilege);
- Privilege jcrReadPrivilege = mock(Privilege.class);
-
when(acm.privilegeFromName(Privilege.JCR_REMOVE_NODE)).thenReturn(jcrReadPrivilege);
+ Privilege jcrDeletePrivilege = mock(Privilege.class);
+
when(acm.privilegeFromName(Privilege.JCR_REMOVE_NODE)).thenReturn(jcrDeletePrivilege);
+
+ 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[]{jcrDeletePrivilege,
privilege})).thenReturn(true);
+ when(session.nodeExists(path)).thenReturn(true);
+ }
+ when(distributionRequest.getPaths()).thenReturn(paths);
+
+
when(distributionRequest.getRequestType()).thenReturn(DistributionRequestType.DELETE);
+ strategy.checkPermission(resourceResolver, distributionRequest);
+ }
+
+ @Test
+ public void testAdditionalPermissionsOnDelete() throws Exception {
+ additionalJcrPrivilegesForDelete = new String[] {"deletePermission"};
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege,
additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
+ DistributionRequest distributionRequest =
mock(DistributionRequest.class);
+ ResourceResolver resourceResolver = mock(ResourceResolver.class);
+ Session session = mock(Session.class);
+ AccessControlManager acm = mock(AccessControlManager.class);
+ Privilege privilege = mock(Privilege.class);
+ when(acm.privilegeFromName(jcrPrivilege)).thenReturn(privilege);
+ Privilege additionalPrivilegeForDelete = mock(Privilege.class);
+ for (String privilegeDelete : additionalJcrPrivilegesForDelete) {
Review comment:
similar arguments and suggestions as made for
`testAdditionalPermissionsOnAdd`
##########
File path:
src/test/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyTest.java
##########
@@ -132,22 +158,49 @@ public void testNoPermissionOnDelete() throws Exception {
@Test
public void testPermissionOnDelete() throws Exception {
- String jcrPrivilege = "somePermission";
- PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege);
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege,
additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
Review comment:
```suggestion
String jcrPrivilege = "somePermission";
PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, null, null);
```
##########
File path:
src/test/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategyTest.java
##########
@@ -36,19 +36,21 @@
*/
public class PrivilegeDistributionRequestAuthorizationStrategyTest {
+ String jcrPrivilege = "foo";
+ String[] additionalJcrPrivilegesForAdd;
+ String[] additionalJcrPrivilegesForDelete;
+
@Test(expected = DistributionException.class)
public void testCheckPermissionWithoutSession() throws Exception {
- String jcrPrivilege = "foo";
- PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege);
+ PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege,
additionalJcrPrivilegesForAdd, additionalJcrPrivilegesForDelete);
Review comment:
```suggestion
String jcrPrivilege = "foo";
PrivilegeDistributionRequestAuthorizationStrategy strategy = new
PrivilegeDistributionRequestAuthorizationStrategy(jcrPrivilege, null, null);
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
