Details
-
Bug
-
Status: Closed
-
Major
-
Resolution: Fixed
-
None
-
None
Description
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)
// 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(); } } }
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:
// TODO does it harm?!?
addSystemUserPath(formatter, systemUser.getPath());
which the issues the following repo-init statement
formatter.format("create path (rep:AuthorizableFolder) %s%n", path);
and
formatter.format("create service user %s with path %s%n", systemUser.getId(), systemUser.getPath());
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.
Attachments
Issue Links
- links to