[
https://issues.apache.org/jira/browse/SLING-8607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16900826#comment-16900826
]
angela commented on SLING-8607:
-------------------------------
[~rombert], it's an implementation detail whether or not adding an entry will
change the policy (and thus potentially the effect). note that even if the
entry you are about to add was already present with the exact same values, it
doesn't necessarily mean that it has the same effect that adding it again.
that's essentially what tried to highlight when the repo-init feature was
introduced.... it's just not totally clear to me if the language describes the
desired outcome or if it describes the access control setup actions to be
performed. and i hope that everyone agrees that repo-init should not rely on
implementation details of a given JCR implementation... and doing the manual
testing essentially does that IMO.
take a simple example with the default implementation:
- a given, existing list contains with two entries: the first one grants read
access on /content and the second revokes read-access for some items in the
subtree.
- now repo-init contains a statement allow read permission on /content
questions:
- does the repo init statement want to make all of /content is readable (i.e.
is it an instruction about effective permissions)? the existing entry would
need to be reordered in order (essentially removing the effect of the second
entry
- does the repo init statement just describe the access control setup? in this
case the followup question was: do you care about the net effect at all? do you
want to append the entry to the list (potentially reordering an existing
entry)? if not: do you want to insert it? and if yes, at which position?
further more:
if a given node was checked-in or locked, you may end up with a similar
problem, that access control setup will fail for non-accesscontrol related
issues. for that matter, the JCR specifications has defined
{{Session.hasCapability}}.
so, i would suggest to adjust the implementation of t{{Session.hasCapability}}
support access control operations and also to respect immutable mounts... the
current implementation in Oak is limited for no other reason than
time-constraint and nobody asked for it.
> AclUtil.setAcl ignores return value of JackrabbitAccessControlList.addEntry
> ---------------------------------------------------------------------------
>
> Key: SLING-8607
> URL: https://issues.apache.org/jira/browse/SLING-8607
> Project: Sling
> Issue Type: Bug
> Components: Repoinit
> Reporter: angela
> Priority: Minor
>
> [~rombert], {{AclUtil.setAcl}} attempts to avoid adding redundant entries
> (and writing an unmodified policy back to the repository).
> however, it ignores the return value of
> {{JackrabbitAccessControlList.addEntry}}, which as far as i remember
> indicates if the given policy has been modified by the given call. i would
> recommend to take the return value into consideration instead of always
> setting 'changed = true'.
> in addition: i am not totally convinced that it is wise to 'manually' compare
> the existing entries with the ones to add and it might well lead to subtle
> inconsistencies. also, depending on the exact implementation of the
> {{JackrabbitAccessControlList}}, adding an entry (even if already contained)
> may effectively alter the policy (e.g. by appending the entry and thus
> affecting the overall outcome of the evaluation). in other words: IMO it
> would be better to rely on the capability of the
> {{JackrabbitAccessControlList}} to determine if an entry was added or not
> (also the {{expandPrivileges}} may be achieved in an optimized fashion with
> the acl-implementation).
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
