[
https://issues.apache.org/jira/browse/SLING-10299?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17339642#comment-17339642
]
Bertrand Delacretaz edited comment on SLING-10299 at 5/5/21, 1:07 PM:
----------------------------------------------------------------------
Looks good to me but I think there's a slight risk of introducing ambiguities
at the grammar level with the {{remove}} statement, see the explanation below.
Using {{delete}} instead would avoid this risk, and I think it makes some sense
as we are actually completely deleting the ACL entries, as opposed to removing
just a specific one which is what {{remove}} currently means.
Would this syntax work for you?
{code:java}
delete ACL on /libs,/apps
delete ACL for alice, bob, fred
delete principal ACL for alice, bob
{code}
h3. Potential ambiguity explanation
I have added tests in [commit
08dfdaa|https://github.com/apache/sling-org-apache-sling-repoinit-parser/commit/08dfdaa652e5196d3640df28851f639baf367ced]
that demonstrate this, currently the parser accepts
{code:java}
remove jcr:ACL for userTestingSpecificRemove
{code}
And I think with SLING-10277 we are planning to remove the requirement for
privilege names to be namespaced, which whould make
{code:java}
remove ACL for userTestingSpecificRemove
{code}
A valid statement meaning "remove the ACL privilege for
userTestingSpecificRemove". We currently only support {{remove *}} at the JCR
level, but that statement is still valid from a grammar point of view, and we
might fully support it later.
Using {{delete}} instead of {{remove}} would remove this risk of ambiguity.
That keyword is currently only used in {{delete group}} and {{delete user}}
which are not ambiguous with the syntax discussed here.
was (Author: bdelacretaz):
Looks good to be but I think there's a slight risk of introducing ambiguities
at the grammar level with the {{remove}} statement, see the explanation below.
Using {{delete}} instead would avoid this risk, and I think it makes some sense
as we are actually completely deleting the ACL entries, as opposed to removing
just a specific one which is what {{remove}} currently means.
Would this syntax work for you?
{code}
delete ACL on /libs,/apps
delete ACL for alice, bob, fred
delete principal ACL for alice, bob
{code}
h3. Potential ambiguity explanation
I have added tests in [commit
08dfdaa|https://github.com/apache/sling-org-apache-sling-repoinit-parser/commit/08dfdaa652e5196d3640df28851f639baf367ced]
that demonstrate this, currently the parser accepts
{code}
remove jcr:ACL for userTestingSpecificRemove
{code}
And I think with SLING-10277 we are planning to remove the requirement for
privilege names to be namespaced, which whould make
{code}
remove ACL for userTestingSpecificRemove
{code}
A valid statement meaning "remove the ACL privilege for
userTestingSpecificRemove". We currently only support {{remove *}} at the JCR
level, but that statement is still valid from a grammar point of view, and we
might fully support it later.
Using {{delete}} instead of {{remove}} would remove this risk of ambiguity.
That keyword is currently only used in {{delete group}} and {{delete user}}
which are not ambiguous with the syntax discussed here.
> Allow for removal of access control policies (not just individual entries)
> --------------------------------------------------------------------------
>
> Key: SLING-10299
> URL: https://issues.apache.org/jira/browse/SLING-10299
> Project: Sling
> Issue Type: New Feature
> Components: Repoinit
> Affects Versions: Repoinit JCR 1.1.32, Repoinit Parser 1.6.6
> Reporter: Angela Schreiber
> Assignee: Angela Schreiber
> Priority: Major
>
> hi [~bdelacretaz], as outline in SLING-10134 the ability to cleanup access
> control content with repo-init is currently limited. while investigating ways
> to remove resource-based service user permissions in existing installations i
> noticed that there is one piece from the Jackrabbit API missing altogether:
> {{AccessControlManager.removePolicy(String absPath, AccessControlPolicy}}.
> repo-init language today allows for removal of individual access control
> entries and all entries, it doesn't provide the means to drop a policy
> (without specifying which entries to drop).
> the langage extension could look as follows for the 3 main types to set
> access control:
> {code}
> remove ACL on /libs,/apps
> remove ACL for alice, bob, fred
> remove principal ACL for alice, bob
> {code}
> IMO no {{end}} statement would be required as there are no additional entry
> specific statements present.
> since this would also be needed to cleanup AC content for principals that are
> being removed, I would strongly suggest to leave the principal-validation
> step to the repository and not mandate the target principal to exist. In
> order to not break subsequent executions I would also suggest to only log an
> INFO if the policy to remove doesn't exist.
> implementation wise it could look as follows (untested pseudo-code):
> {code}
> JackrabbitAccessControlList acl =
> AccessControlUtils.getAccessControlList(acMgr, jcrPath);
> if (acl != null) {
> acMgr.removePolicy(acl.getPath(), acl)
> } else {
> log.info(".....");
> }
> {code}
> {code}
> PrincipalAccessControlList acl = getPrincipalAccessControlList(acMgr,
> principal)
> if (acl != null) {
> acMgr.removePolicy(acl.getPath(), acl)
> } else {
> log.info(".....");
> }
> {code}
> for the case {{remove ACL for alice, bob, fred}} multiple options exist.... i
> would need to dig into the repo-init code to see what was best. in theory
> {{JackrabbitAccessControlManager.getPolicies(principal)}} should work and one
> only need to make sure not to delete the {{PrincipalAccessControlList}} if
> that existed as well.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
