[jira] [Commented] (JCRVLT-292) Order of ACLs are altered on installation of content packages
[
https://issues.apache.org/jira/browse/JCRVLT-292?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16496192#comment-16496192
]
angela commented on JCRVLT-292:
---
[~stillalex], [~reschke], I have been thinking about this issue over and over
again. I don't feel comfortable to push this patch without having proper test
coverage in place.
Therefore I created a new container for the test coverage and created specific
subtasks for the security related parts, marking the one for access control
import as blocking this issue.
> Order of ACLs are altered on installation of content packages
> -
>
> Key: JCRVLT-292
> URL: https://issues.apache.org/jira/browse/JCRVLT-292
> Project: Jackrabbit FileVault
> Issue Type: Bug
> Components: Packaging
>Reporter: angela
>Priority: Major
> Attachments: JCRVLT-292-2.patch, JCRVLT-292.patch
>
>
> When installing a content package with AccessControlHandling _overwrite_
> access control entries contained in a given list are grouped by principal and
> ultimately imported with a different order that originally defined in the
> package.
> This alters the effective permissions for those {{Subject}}s that contain the
> principals for which the ACEs got imported.
> Example:
> 1. grant group1 read at /testroot
> 2. deny group2 read at specific subset of items within the tree defined by
> /testroot
> 3. grant group1 read/write at specific subset of items within the tree
> defined by /testroot
> The ACL resulting from the package import will contain the entries in the
> following order: 1, 3, 2.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (JCRVLT-292) Order of ACLs are altered on installation of content packages
[
https://issues.apache.org/jira/browse/JCRVLT-292?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16495066#comment-16495066
]
angela commented on JCRVLT-292:
---
[~stillalex], [~reschke], slightly simplified patch... to be honest it somehow
worries me that no tests start failing with my changes. as a next step i will
take another look at existing tests and see what could be done with moderate
effort.
> Order of ACLs are altered on installation of content packages
> -
>
> Key: JCRVLT-292
> URL: https://issues.apache.org/jira/browse/JCRVLT-292
> Project: Jackrabbit FileVault
> Issue Type: Bug
> Components: Packaging
>Reporter: angela
>Priority: Major
> Attachments: JCRVLT-292-2.patch, JCRVLT-292.patch
>
>
> When installing a content package with AccessControlHandling _overwrite_
> access control entries contained in a given list are grouped by principal and
> ultimately imported with a different order that originally defined in the
> package.
> This alters the effective permissions for those {{Subject}}s that contain the
> principals for which the ACEs got imported.
> Example:
> 1. grant group1 read at /testroot
> 2. deny group2 read at specific subset of items within the tree defined by
> /testroot
> 3. grant group1 read/write at specific subset of items within the tree
> defined by /testroot
> The ACL resulting from the package import will contain the entries in the
> following order: 1, 3, 2.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (JCRVLT-292) Order of ACLs are altered on installation of content packages
[
https://issues.apache.org/jira/browse/JCRVLT-292?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16494871#comment-16494871
]
Alex Deparvu commented on JCRVLT-292:
-
I really can't contribute to the implications of the patch, but I think the
patch looks good and addresses the stated issue.
Seems to me the original code used a {{LinkedHashMap}} which guarantees
iteration of keys based on insertion order, so relative order of principals is
kept, unless there's a double (another entry on the same principal) in which
case some entry could be moved close to the initial location of the principal.
which is the entire point of this issue, so if this is a corner case or a
design decision only the author can clarify.
One nitpick is that leaving the map in there might cause even more confusion in
the future, as it looks after the patch a simple Set could be used to determine
if a given principal exists or not.
Also, pre-patch the number of {{getPrincipal(principalName)}} was proportional
with the number of principals, now it is proportional to the number of ACEs,
but I'm really not sure if this is an issue or not, it would probably make more
sense to move it up to the JackrabbitACLImporter as a principal cache or
something similar.
Finally, is there any way in which we can hide this behind a feature flag /
setting. I don't know this code well enough to say if there's support for this
kind of configs.
> Order of ACLs are altered on installation of content packages
> -
>
> Key: JCRVLT-292
> URL: https://issues.apache.org/jira/browse/JCRVLT-292
> Project: Jackrabbit FileVault
> Issue Type: Bug
> Components: Packaging
>Reporter: angela
>Priority: Major
> Attachments: JCRVLT-292.patch
>
>
> When installing a content package with AccessControlHandling _overwrite_
> access control entries contained in a given list are grouped by principal and
> ultimately imported with a different order that originally defined in the
> package.
> This alters the effective permissions for those {{Subject}}s that contain the
> principals for which the ACEs got imported.
> Example:
> 1. grant group1 read at /testroot
> 2. deny group2 read at specific subset of items within the tree defined by
> /testroot
> 3. grant group1 read/write at specific subset of items within the tree
> defined by /testroot
> The ACL resulting from the package import will contain the entries in the
> following order: 1, 3, 2.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
