[
https://issues.apache.org/jira/browse/SLING-9970?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17243079#comment-17243079
]
Angela Schreiber commented on SLING-9970:
-----------------------------------------
[~cziegeler], patch at
https://github.com/apache/sling-org-apache-sling-feature-cpconverter/tree/SLING-9970.
I decided not to change the name of {{SystemUser.getPath()}} in order to keep
the number of modifications as limited as possible.
hope it helps. but please be aware that i have very limited insight into the
usage and the intentions of the converter tool... so careful review is needed
from someone in the sling team that is familiar and in charge of this code.
> SystemUser.getPath doesn't reflect the repository path
> ------------------------------------------------------
>
> Key: SLING-9970
> URL: https://issues.apache.org/jira/browse/SLING-9970
> Project: Sling
> Issue Type: Bug
> Components: Content-Package to Feature Model Converter
> Reporter: Angela Schreiber
> Priority: Major
>
> I tried to find out why {{AccessControlEntry}} is constructed with 2
> different {{RepoPath}}s one reflecting the path as obtained from the parser
> and one containing the path converted to 'repository path' using
> {{PlatformNameFormat.getRepositoryPath(resourcePath)}}.
> from what i see the 'repository' path contained in the entry is later used to
> create the hierarchy down to access controlled nodes that hold the
> resource-based access control policy with the entries.
> but looking at the usages of the 'path' field i found that it is only used in
> https://github.com/apache/sling-org-apache-sling-feature-cpconverter/blob/master/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java#L152
> (see also SLING-9953)
> {code}
> // clean the unneeded ACLs, see SLING-8561
> if (authorizations != null) {
> Iterator<AccessControlEntry> authorizationsIterator =
> authorizations.iterator();
> while (authorizationsIterator.hasNext()) {
> AccessControlEntry acl = authorizationsIterator.next();
> if (acl.getPath().startsWith(systemUser.getPath())) {
> authorizationsIterator.remove();
> }
> }
> }
> {code}
> this finding lead me to the conclusion that the {{SystemUser}} object is in
> fact created with a path that doesn't actually represent the JCR path as
> found in the repository.
> so, all usages of {{SystemUser.getPath}} used to create the system user will
> potentially use the 'wrong' path. for example:
> https://github.com/apache/sling-org-apache-sling-feature-cpconverter/blob/master/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java#L105
> {code}
> // TODO does it harm?!?
> addSystemUserPath(formatter, systemUser.getPath());
> {code}
> which the issues the following repo-init statement
> {code}
> formatter.format("create path (rep:AuthorizableFolder) %s%n", path);
> {code}
> and
> https://github.com/apache/sling-org-apache-sling-feature-cpconverter/blob/master/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java#L109
> {code}
> formatter.format("create service user %s with path %s%n", systemUser.getId(),
> systemUser.getPath());
> {code}
> upon a quick (but maybe incomplete) search I didn't find any usage of
> {{SystemUser.getPath()}} that would require it to reflect the path as
> extracted from the content package.... if that was true, the method should be
> renamed to 'getRepositoryPath' and should return the converted path (see
> above).
> Having this addressed would IMO subsequently allow to drop the duplicated
> path argument from the {{AccessControlEntry}} and drop the {{getPath}} method
> altogether. which anyway seems a bit confusing to have. let me know if i
> should create a separate issue for this follow up clean up.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
