[ 
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, 2:36 PM:
-----------------------------------------------------------------------

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:java}
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
    # this was "remove *" so far in "set ACL"
    * 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)

Reply via email to