You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by an...@apache.org on 2022/02/25 09:30:31 UTC

[sling-org-apache-sling-repoinit-parser] branch SLING-11160 created (now 280ac77)

This is an automated email from the ASF dual-hosted git repository.

angela pushed a change to branch SLING-11160
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-repoinit-parser.git.


      at 280ac77  SLING-11160 : Repoinit does not allow to remove individual ACEs (parser)

This branch includes the following new commits:

     new 280ac77  SLING-11160 : Repoinit does not allow to remove individual ACEs (parser)

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


[sling-org-apache-sling-repoinit-parser] 01/01: SLING-11160 : Repoinit does not allow to remove individual ACEs (parser)

Posted by an...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

angela pushed a commit to branch SLING-11160
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-repoinit-parser.git

commit 280ac77fede00b258fd969910e26385b664ac1ed
Author: angela <an...@adobe.com>
AuthorDate: Fri Feb 25 10:30:18 2022 +0100

    SLING-11160 : Repoinit does not allow to remove individual ACEs (parser)
---
 .../repoinit/parser/operations/AclGroupBase.java   | 12 +++++---
 .../sling/repoinit/parser/operations/AclLine.java  | 32 +++++++++++++++++++---
 .../repoinit/parser/operations/package-info.java   |  2 +-
 src/main/javacc/RepoInitGrammar.jjt                | 14 ++++++----
 src/test/resources/testcases/test-11-output.txt    |  2 +-
 src/test/resources/testcases/test-11.txt           |  2 +-
 src/test/resources/testcases/test-30-output.txt    |  2 +-
 src/test/resources/testcases/test-30.txt           |  2 +-
 src/test/resources/testcases/test-33-output.txt    |  2 +-
 src/test/resources/testcases/test-33.txt           |  2 +-
 10 files changed, 51 insertions(+), 21 deletions(-)

diff --git a/src/main/java/org/apache/sling/repoinit/parser/operations/AclGroupBase.java b/src/main/java/org/apache/sling/repoinit/parser/operations/AclGroupBase.java
index 8f1bd85..63cb885 100644
--- a/src/main/java/org/apache/sling/repoinit/parser/operations/AclGroupBase.java
+++ b/src/main/java/org/apache/sling/repoinit/parser/operations/AclGroupBase.java
@@ -68,7 +68,7 @@ abstract class AclGroupBase extends Operation {
         try (Formatter formatter = new Formatter()) {
             formatter.format("%s",topLine);
             for (AclLine line : lines) {
-                String action = actionToString(line.getAction());
+                String action = actionToString(line.getAction(), line.isAllow());
                 String privileges = privilegesToString(line.getAction(), line.getProperty(AclLine.PROP_PRIVILEGES));
                 String onOrFor;
                 if (hasPathLines) {
@@ -116,10 +116,14 @@ abstract class AclGroupBase extends Operation {
     }
 
     @NotNull
-    private static String actionToString(@NotNull AclLine.Action action) {
-        if(action.equals(Action.REMOVE_ALL)) {
+    private static String actionToString(@NotNull AclLine.Action action, boolean isAllow) {
+        if (action.equals(Action.REMOVE_ALL)) {
             return Action.REMOVE.toString().toLowerCase();
+        } else if (action.equals(Action.REMOVE)) {
+            String allowStr = (isAllow) ? Action.ALLOW.toString().toLowerCase() : Action.DENY.toString().toLowerCase();
+            return Action.REMOVE.toString().toLowerCase() + " " + allowStr;
+        } else {
+            return action.toString().toLowerCase();
         }
-        return action.toString().toLowerCase();
     }
 }
diff --git a/src/main/java/org/apache/sling/repoinit/parser/operations/AclLine.java b/src/main/java/org/apache/sling/repoinit/parser/operations/AclLine.java
index 437bea4..5086839 100644
--- a/src/main/java/org/apache/sling/repoinit/parser/operations/AclLine.java
+++ b/src/main/java/org/apache/sling/repoinit/parser/operations/AclLine.java
@@ -45,15 +45,34 @@ public class AclLine {
 
     private final Map<String, List<String>> properties;
     private List<RestrictionClause> restrictions;
+    private boolean isAllow = true;
 
     public AclLine(Action a) {
-        action = a;
+        this(a, false);
+    }
+
+    public AclLine(Action a, boolean isRemove) {
+        if (isRemove) {
+            if (!(a == Action.ALLOW || a == Action.DENY)) {
+                throw new IllegalArgumentException("Action.REMOVE can only be use in combination with an additional ALLOW or DENY.");
+            }
+            action = Action.REMOVE;
+        } else {
+            action = a;
+        }
         properties = new TreeMap<>();
+        if (a == Action.DENY) {
+            isAllow = false;
+        }
     }
 
     public Action getAction() {
         return action;
     }
+    
+    public boolean isAllow() {
+        return isAllow;
+    }
 
     /**
      * Return the named multi-value property, or an empty list if not found.
@@ -80,8 +99,13 @@ public class AclLine {
 
     @Override
     public String toString() {
-        return getClass().getSimpleName() + " " + action + " " + properties
-                + (restrictions == null || restrictions.isEmpty() ? "" : " restrictions=" + restrictions);
-
+        if (action.equals(Action.REMOVE)) {
+            String allowStr = isAllow ? Action.ALLOW.toString() : Action.DENY.toString();
+            return getClass().getSimpleName() + " " + action + " " + allowStr + " " + properties
+                    + (restrictions == null || restrictions.isEmpty() ? "" : " restrictions=" + restrictions);
+        } else {
+            return getClass().getSimpleName() + " " + action + " " + properties
+                    + (restrictions == null || restrictions.isEmpty() ? "" : " restrictions=" + restrictions);            
+        }
     }
 }
diff --git a/src/main/java/org/apache/sling/repoinit/parser/operations/package-info.java b/src/main/java/org/apache/sling/repoinit/parser/operations/package-info.java
index 737bebc..b82d630 100644
--- a/src/main/java/org/apache/sling/repoinit/parser/operations/package-info.java
+++ b/src/main/java/org/apache/sling/repoinit/parser/operations/package-info.java
@@ -17,6 +17,6 @@
 
  // DO NOT use version 5.x here, once a major change is
  // needed skip directly to 6.x (SLING-10139)
-@org.osgi.annotation.versioning.Version("4.8.0")
+@org.osgi.annotation.versioning.Version("4.9.0")
 package org.apache.sling.repoinit.parser.operations;
 
diff --git a/src/main/javacc/RepoInitGrammar.jjt b/src/main/javacc/RepoInitGrammar.jjt
index 1cf53e4..bf7f95e 100644
--- a/src/main/javacc/RepoInitGrammar.jjt
+++ b/src/main/javacc/RepoInitGrammar.jjt
@@ -360,13 +360,15 @@ void removeStarLine(List<AclLine> lines) :
 }
 
 AclLine privilegesLineOperation() :
-{}
 {
-    ( 
-        <REMOVE>        { return new AclLine(AclLine.Action.REMOVE); }
-        | ( <ALLOW>     { return new AclLine(AclLine.Action.ALLOW); } )
-        | ( <DENY>      { return new AclLine(AclLine.Action.DENY); } )    
-    ) 
+    boolean isRemove = false;
+    AclLine.Action action = null;
+}
+{
+    (<REMOVE> { isRemove = true; })?
+    
+    ((<ALLOW> { action = AclLine.Action.ALLOW; })
+    | (<DENY> { action = AclLine.Action.DENY; })) { return new AclLine(action, isRemove); } 
 }
 
 void userPrivilegesLine(List<AclLine> lines) :
diff --git a/src/test/resources/testcases/test-11-output.txt b/src/test/resources/testcases/test-11-output.txt
index f675cae..6c1bfde 100644
--- a/src/test/resources/testcases/test-11-output.txt
+++ b/src/test/resources/testcases/test-11-output.txt
@@ -3,4 +3,4 @@ SetAclPaths on /libs /apps
   AclLine ALLOW {principals=[user1, user2], privileges=[jcr:read]}
   AclLine REMOVE_ALL {principals=[another]}
   AclLine ALLOW {principals=[another], privileges=[x:y]}
-  AclLine REMOVE {principals=[userTestingSpecificRemove], privileges=[jcr:ACL]}
\ No newline at end of file
+  AclLine REMOVE DENY {principals=[userTestingSpecificRemove], privileges=[jcr:ACL]}
\ No newline at end of file
diff --git a/src/test/resources/testcases/test-11.txt b/src/test/resources/testcases/test-11.txt
index 9099e00..c18b035 100644
--- a/src/test/resources/testcases/test-11.txt
+++ b/src/test/resources/testcases/test-11.txt
@@ -10,5 +10,5 @@ set ACL on /libs,/apps
     remove * for another
     allow x:y for another
 
-    remove jcr:ACL for userTestingSpecificRemove
+    remove deny jcr:ACL for userTestingSpecificRemove
 end
\ No newline at end of file
diff --git a/src/test/resources/testcases/test-30-output.txt b/src/test/resources/testcases/test-30-output.txt
index a330041..8748832 100644
--- a/src/test/resources/testcases/test-30-output.txt
+++ b/src/test/resources/testcases/test-30-output.txt
@@ -4,7 +4,7 @@ SetAclPrincipals for user1 u2
   AclLine DENY {paths=[/apps], privileges=[jcr:write]}
   AclLine DENY {nodetypes=[sling:Folder, nt:unstructured], paths=[/apps, /content], privileges=[jcr:lockManagement]}
   AclLine DENY {nodetypes=[sling:Folder, nt:unstructured], paths=[/apps, /content], privileges=[jcr:modifyProperties]} restrictions=[rep:itemNames=[prop1, prop2]]
-  AclLine REMOVE {paths=[/apps], privileges=[jcr:understand, some:other]}
+  AclLine REMOVE ALLOW {paths=[/apps], privileges=[jcr:understand, some:other]}
   AclLine ALLOW {paths=[/apps], privileges=[jcr:addChildNodes]} restrictions=[rep:ntNames=[sling:Folder, nt:unstructured]]
   AclLine ALLOW {paths=[/apps], privileges=[jcr:modifyProperties]} restrictions=[rep:ntNames=[sling:Folder, nt:unstructured], rep:itemNames=[prop1, prop2]]
   AclLine ALLOW {paths=[/apps, /content], privileges=[jcr:addChildNodes]} restrictions=[rep:glob=[/cat, /cat/, cat]]
diff --git a/src/test/resources/testcases/test-30.txt b/src/test/resources/testcases/test-30.txt
index 6631865..4a9e6fd 100644
--- a/src/test/resources/testcases/test-30.txt
+++ b/src/test/resources/testcases/test-30.txt
@@ -10,7 +10,7 @@ set ACL for user1,u2
     deny jcr:lockManagement on /apps, /content nodetypes sling:Folder, nt:unstructured
     # nodetypes clause with restriction clause
     deny jcr:modifyProperties on /apps, /content nodetypes sling:Folder, nt:unstructured restriction(rep:itemNames,prop1,prop2)
-    remove jcr:understand,some:other on /apps
+    remove allow jcr:understand,some:other on /apps
 
     # multi value restriction
     allow jcr:addChildNodes on /apps restriction(rep:ntNames,sling:Folder,nt:unstructured)
diff --git a/src/test/resources/testcases/test-33-output.txt b/src/test/resources/testcases/test-33-output.txt
index cd489cc..9a2f655 100644
--- a/src/test/resources/testcases/test-33-output.txt
+++ b/src/test/resources/testcases/test-33-output.txt
@@ -4,7 +4,7 @@ SetAclPrincipalBased for principal1 principal2
   AclLine DENY {paths=[/apps], privileges=[jcr:write]}
   AclLine DENY {nodetypes=[sling:Folder, nt:unstructured], paths=[/apps, /content], privileges=[jcr:lockManagement]}
   AclLine DENY {nodetypes=[sling:Folder, nt:unstructured], paths=[/apps, /content], privileges=[jcr:modifyProperties]} restrictions=[rep:itemNames=[prop1, prop2]]
-  AclLine REMOVE {paths=[/apps], privileges=[jcr:understand, some:other]}
+  AclLine REMOVE ALLOW {paths=[/apps], privileges=[jcr:understand, some:other]}
   AclLine ALLOW {paths=[/apps], privileges=[jcr:addChildNodes]} restrictions=[rep:ntNames=[sling:Folder, nt:unstructured]]
   AclLine ALLOW {paths=[/apps], privileges=[jcr:modifyProperties]} restrictions=[rep:ntNames=[sling:Folder, nt:unstructured], rep:itemNames=[prop1, prop2]]
   AclLine ALLOW {paths=[/apps, /content], privileges=[jcr:addChildNodes]} restrictions=[rep:glob=[/cat, /cat/, cat]]
diff --git a/src/test/resources/testcases/test-33.txt b/src/test/resources/testcases/test-33.txt
index 846d0b5..c362625 100644
--- a/src/test/resources/testcases/test-33.txt
+++ b/src/test/resources/testcases/test-33.txt
@@ -16,7 +16,7 @@ set principal ACL for principal1,principal2
     deny jcr:lockManagement on /apps, /content nodetypes sling:Folder, nt:unstructured
     # nodetypes clause with restriction clause
     deny jcr:modifyProperties on /apps, /content nodetypes sling:Folder, nt:unstructured restriction(rep:itemNames,prop1,prop2)
-    remove jcr:understand,some:other on /apps
+    remove allow jcr:understand,some:other on /apps
 
     # multi value restriction
     allow jcr:addChildNodes on /apps restriction(rep:ntNames,sling:Folder,nt:unstructured)