SLING-8810 make DeleteAce response to remove a non-existing ACE
consistent
diff --git a/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/AbstractAccessPostServlet.java b/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/AbstractAccessPostServlet.java
index 2887921..63d9eae 100644
--- a/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/AbstractAccessPostServlet.java
+++ b/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/AbstractAccessPostServlet.java
@@ -402,6 +402,45 @@
}
+ /**
+ * Returns an <code>AccessControlList</code> to edit for the node at the
+ * given <code>resourcePath</code>.
+ *
+ * @param accessControlManager The manager providing access control lists
+ * @param resourcePath The node path for which to return an access control
+ * list
+ * @param mayCreate <code>true</code> if an access control list should be
+ * created if the node does not have one yet.
+ * @return The <code>AccessControlList</code> to modify to control access to
+ * the node or null if one could not be located or created
+ */
+ protected AccessControlList getAccessControlListOrNull(
+ final AccessControlManager accessControlManager,
+ final String resourcePath, final boolean mayCreate)
+ throws RepositoryException {
+ AccessControlList acl = null;
+ // check for an existing access control list to edit
+ AccessControlPolicy[] policies = accessControlManager.getPolicies(resourcePath);
+ for (AccessControlPolicy policy : policies) {
+ if (policy instanceof AccessControlList) {
+ acl = (AccessControlList) policy;
+ }
+ }
+
+ if (acl == null) {
+ // no existing access control list, try to create if allowed
+ if (mayCreate) {
+ AccessControlPolicyIterator applicablePolicies = accessControlManager.getApplicablePolicies(resourcePath);
+ while (applicablePolicies.hasNext()) {
+ AccessControlPolicy policy = applicablePolicies.nextAccessControlPolicy();
+ if (policy instanceof AccessControlList) {
+ acl = (AccessControlList) policy;
+ }
+ }
+ }
+ }
+ return acl;
+ }
/**
* Bind a new post response creator
diff --git a/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/DeleteAcesServlet.java b/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/DeleteAcesServlet.java
index da8e3ec..f58eceb 100644
--- a/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/DeleteAcesServlet.java
+++ b/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/DeleteAcesServlet.java
@@ -43,6 +43,8 @@
import org.osgi.service.component.annotations.Reference;
import org.osgi.service.component.annotations.ReferenceCardinality;
import org.osgi.service.component.annotations.ReferencePolicy;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* <p>
@@ -85,6 +87,11 @@
public class DeleteAcesServlet extends AbstractAccessPostServlet implements DeleteAces {
private static final long serialVersionUID = 3784866802938282971L;
+ /**
+ * default log
+ */
+ private final Logger log = LoggerFactory.getLogger(getClass());
+
/**
* Overridden since the @Reference annotation is not inherited from the super method
*
@@ -151,26 +158,46 @@
try {
AccessControlManager accessControlManager = AccessControlUtil.getAccessControlManager(jcrSession);
- AccessControlList updatedAcl = getAccessControlList(accessControlManager, resourcePath, false);
-
- //keep track of the existing Aces for the target principal
- AccessControlEntry[] accessControlEntries = updatedAcl.getAccessControlEntries();
- List<AccessControlEntry> oldAces = new ArrayList<AccessControlEntry>();
- for (AccessControlEntry ace : accessControlEntries) {
- if (pidSet.contains(ace.getPrincipal().getName())) {
- oldAces.add(ace);
+ AccessControlList updatedAcl = getAccessControlListOrNull(accessControlManager, resourcePath, false);
+ // if there is no AccessControlList, then there is nothing to be deleted
+ if (updatedAcl == null) {
+ // log the warning about principals where no ACE was found
+ for (String pid : pidSet) {
+ log.warn("No AccessControlEntry was found to be deleted for principal: " + pid);
}
- }
-
- //remove the old aces
- if (!oldAces.isEmpty()) {
- for (AccessControlEntry ace : oldAces) {
- updatedAcl.removeAccessControlEntry(ace);
+ } else {
+ //keep track of the existing Aces for the target principal
+ AccessControlEntry[] accessControlEntries = updatedAcl.getAccessControlEntries();
+ List<AccessControlEntry> oldAces = new ArrayList<AccessControlEntry>();
+ for (AccessControlEntry ace : accessControlEntries) {
+ if (pidSet.contains(ace.getPrincipal().getName())) {
+ oldAces.add(ace);
+ }
}
- }
- //apply the changed policy
- accessControlManager.setPolicy(resourcePath, updatedAcl);
+ // track which of the submitted principals had an ACE removed
+ Set<String> removedPidSet = new HashSet<>();
+
+ //remove the old aces
+ if (!oldAces.isEmpty()) {
+ for (AccessControlEntry ace : oldAces) {
+ updatedAcl.removeAccessControlEntry(ace);
+
+ // remove from the candidate set
+ removedPidSet.add(ace.getPrincipal().getName());
+ }
+ }
+
+ // log the warning about principals where no ACE was found
+ for (String pid : pidSet) {
+ if (!removedPidSet.contains(pid)) {
+ log.warn("No AccessControlEntry was found to be deleted for principal: " + pid);
+ }
+ }
+
+ //apply the changed policy
+ accessControlManager.setPolicy(resourcePath, updatedAcl);
+ }
} catch (RepositoryException re) {
throw new RepositoryException("Failed to delete access control.", re);
}
