[ https://issues.apache.org/jira/browse/SLING-11160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17498862#comment-17498862 ]
Bertrand Delacretaz edited comment on SLING-11160 at 2/28/22, 11:53 AM: ------------------------------------------------------------------------ The more I look at this the more I think having a "remove" statement in "set ACL" is a design mistake (which I'm probably responsible for). It naturally leads to the "remove allow" and "remove deny" statements that you are suggesting, which do not look natural nor obvious. I think we should rather introduce a new top-level REMOVE ACL statement as in the examples below. Basically, reproducing the "set ACL" statements with "remove ACL" for removal. And maybe deprecate the existing "remove *", or allow it only in "remove ACL" statements. Not sure if we need all the below variants for now, we can also introduce just the ones that are needed now. WDYT? {code} remove ACL on /someting allow jcr:read for userA,userB end remove repository ACL for user1,user2 allow jcr:read,jcr:lockManagement deny jcr:write end remove ACL for user1,u2 allow jcr:read on /content deny jcr:write on /apps end remove principal ACL for principal1,principal2 # this was "remove *" so far in "set ACL" * on /libs,/apps allow jcr:read on /content {code} was (Author: bdelacretaz): The more I look at this the more I think having a "remove" statement in "set ACL" is a design mistake (which I'm probably responsible for). It naturally leads to the "remove allow" and "remove deny" statements that you are suggesting, which do not look natural nor obvious. I think we should rather introduce a new top-level REMOVE ACL statement as in the examples below. Basically, reproducing the "set ACL" statements with "remove ACL" for removal. And maybe deprecate the existing "remove *", or allow it only in "remove ACL" statements. Not sure if we need all the below variants for now, we can also introduce just the ones that are needed now. WDYT? {code} remove ACL on /someting allow jcr:read for userA,userB end remove repository ACL for user1,user2 allow jcr:read,jcr:lockManagement deny jcr:write end remove ACL for user1,u2 allow jcr:read on /content deny jcr:write on /apps end remove principal ACL for principal1,principal2 remove * on /libs,/apps allow jcr:read on /content {code} > Repoinit does not allow to remove individual ACEs > ------------------------------------------------- > > Key: SLING-11160 > URL: https://issues.apache.org/jira/browse/SLING-11160 > Project: Sling > Issue Type: Bug > Components: Repoinit > Reporter: Angela Schreiber > Assignee: Angela Schreiber > Priority: Major > Attachments: SLING-11160-initial-draft.patch > > Time Spent: 10m > Remaining Estimate: 0h > > With SLING-9090 support for using _REMOVE *_ for all entries at a given path > or for a given principal has been implemented. > However as indicated in the same issue the intended usage of _REMOVE > some-thing-specific_ is not clear. > What is therefore missing with repo-init is the ability to remove a single > access control entry that matches > - prinicipal > - privileges > - allow-status > - single value restriction > - mv restrictions. > As far as I can see the biggest issue is the fact that REMOVE vs ALLOW/DENY > are mutually exclusive as the other params listed above can be extracted from > a given AclLine in combination with the set-ACL statement. > This could be fixed by adjusting the following parser method > {code} > AclLine privilegesLineOperation() : > {} > { > ( > <REMOVE> { return new AclLine(AclLine.Action.REMOVE); } > | ( <ALLOW> { return new AclLine(AclLine.Action.ALLOW); } ) > | ( <DENY> { return new AclLine(AclLine.Action.DENY); } ) > ) > } > {code} > such that > - REMOVE is optional, followed by > - ALLOW or DENY > The {{AclLine}} would then need to be slightly adjusted such that REMOVE can > be combined with either ALLOW or DENY. > Otherwise, I don't see how > {{AccessControlList.removeAccessControlEntry(AccessControlEntry)}} could be > implemented in org.apache.sling.jcr.repoinit for a single ACE. > Or maybe the intention was something different in the first place? > [~bdelacretaz], I would appreciate if you had time to comment on this. > cc: [~kpauls], [~cziegeler] -- This message was sent by Atlassian Jira (v8.20.1#820001)