Uploaded image for project: 'Sling'
  1. Sling
  2. SLING-9970

SystemUser.getPath doesn't reflect the repository path

    XMLWordPrintableJSON

Details

    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:

      https://github.com/apache/sling-org-apache-sling-feature-cpconverter/blob/master/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java#L105

           // 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

      https://github.com/apache/sling-org-apache-sling-feature-cpconverter/blob/master/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java#L109

      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

          Activity

            People

              karlpauls Karl Pauls
              angela Angela Schreiber
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: