[ 
https://issues.apache.org/jira/browse/SLING-9961?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17242128#comment-17242128
 ] 

Angela Schreiber edited comment on SLING-9961 at 12/2/20, 8:18 AM:
-------------------------------------------------------------------

[~cziegeler], i investigated a bit further while trying to work on an initial 
draft for SLING-9692. i found that is was an attempt to fix SLING-8586 _[cp2fm] 
"create service user" repoinit instruction throws 
javax.jcr.nodetype.ConstraintViolationException_. IMHO that exception was 
caused by another bug as the exception mentioned in the description indicates 
that an attempt was made to create  a node named _replication-user_ with type 
_sling:Folder_. the node name suggests that this should actually be a node of 
type _rep:User_ or _rep:SystemUser_ and the mentioned exception would be raised 
if one would attempt to create a node with type _sling:Folder_ below a parent 
node with primary type _rep:AuthorizableFolder_.

looking at the usage of _sling:Folder_ in the converter code i found 
{{DefaultAclManager.computePathType}} which returns it as default path. this 
method is used here 
https://github.com/apache/sling-org-apache-sling-feature-cpconverter/blob/master/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java#L160.
 that might again be a side effect of the missing/wrong handling of access 
control entries located below or at the user node (see SLING-9953). in other 
words: IMO there needs to be code in place that properly detects special paths 
like any user home node or the repo-level 'null' path and doing so may actually 
fix the root cause for the exception.

one final note regarding the {{addPaths}} method: when addressing SLING-9692 
the creation of access control paths where a given ACE will take effect anyway 
should be limited to resource-based access control. with principal-based access 
control it's not longer required that the effective path actually exists as the 
entries are not stored there [0].

[0] 
http://jackrabbit.apache.org/oak/docs/security/authorization/principalbased.html#Implementation_Details


was (Author: anchela):
[~cziegeler], i investigated a bit further while trying to work on an initial 
draft for SLING-9692. i found that is was an attempt to fix SLING-8586 _[cp2fm] 
"create service user" repoinit instruction throws 
javax.jcr.nodetype.ConstraintViolationException_. IMHO that exception was 
caused by another bug as the exception mentioned in the description indicates 
that an attempt was made to create  a node named _replication-user_ with type 
_sling:Folder_. the node name suggests that this should actually be a node of 
type _rep:User_ or _rep:SystemUser_ and the mentioned exception would be raised 
if one would attempt to create a node with type _sling:Folder_ below a parent 
node with primary type _rep:AuthorizableFolder_.

looking at the usage of _sling:Folder_ in the converter code i found 
{{DefaultAclManager.computePathType}} which returns it as default path. this 
method is used here 
https://github.com/apache/sling-org-apache-sling-feature-cpconverter/blob/master/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java#L160.
 that might again be a side effect of the missing/wrong handling of access 
control entries located below or at the user node (see SLING-9953). in other 
words: IMO there needs to be code in place that properly detects special paths 
like any user home node or the repo-level 'null' path and doing so may actually 
fix the root cause for the exception.

> DefaultAclManager: redundant creation of intermediate path
> ----------------------------------------------------------
>
>                 Key: SLING-9961
>                 URL: https://issues.apache.org/jira/browse/SLING-9961
>             Project: Sling
>          Issue Type: Bug
>          Components: Content-Package to Feature Model Converter
>            Reporter: Angela Schreiber
>            Priority: Minor
>             Fix For: Content-Package to Feature Model Converter 1.0.24
>
>
> {code}
> // TODO does it harm?!?
> addSystemUserPath(formatter, systemUser.getPath());
> {code}
> where the method looks as follows:
> {code}
> private final void addSystemUserPath(Formatter formatter, RepoPath path) {
>         if (preProvidedSystemPaths.add(path)) {
>             formatter.format("create path (rep:AuthorizableFolder) %s%n", 
> path);
>         }
>     }
> {code}
> i would strong recommend to drop that. the 
> content-package-feature-model-converter already has quite some hardcoded 
> stuff.... how the intermediate path argument passed to 
> {{UserManager.createSystemUser(String id, String intermediatePath)}} is used 
> and what node types are used to create the hierarchy should be considered an 
> implementation detail and it doesn't make sense IMHO to eagerly create the 
> tree structure in a feature like this. in fact i would consider this a bug 
> that may sooner or later cause the converter to fail during repo-init 
> execution.
> btw: the comment already implies that the author was not sure of the 
> usefulness.... why not verifying and fixing the code or dropping the TODO 
> before releasing the module?



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to