[jira] [Comment Edited] (SLING-9212) Distribution code checks for jcr:removeNode permissions on importer side for DELETE request

["Ashish Chopra (Jira)" <ji...@apache.org>]

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

Ashish Chopra edited comment on SLING-9212 at 3/17/20, 6:33 PM:
----------------------------------------------------------------

bq. In case of DELETE request, apart from the configured permissions, it also checks for jcr:removeNode permissions for the user on the path. This check happens on the exporter side but AFAIU, the actual deletion happens on the importer endpoint. The content does not get deleted on exporter side. In that case, this permission check should happen on importer side.
While I agree that checking for a remove/delete privilege for creating a DELETE distribution action  at the source sling instance is incorrect, I'd argue against checking for any privilege on the target (recipient) Sling instance because:
* semantically it is entirely target sling instance's business what it does with incoming content. It might not even have the same access control paradigm as the source sling instance. The target sling instance can choose to reject an incoming distribution by performing any validation it wishes, but source sling instance need not be bothered about those rules. If the target instance doesn't like the distribution request, it responds with a failure - that should be the only contract. 
* implementation wise, such a "pre-validation" (which should be abstracted rather than just concerning itself with permission validation) might require in a remote call to target sling instance, which will not be desirable esp when there is a middleware involved (say, Apache Kafka)

I'd propose that validation of a custom privilege - specific to content-distribution - at the source instance should be _sufficient_ to determine if distribution request should be _created_ at all or not. This privilege should be enforced outside JCR and the enforcement should only be restricted to source sling instance.


was (Author: ashishc):
bq. In case of DELETE request, apart from the configured permissions, it also checks for jcr:removeNode permissions for the user on the path. This check happens on the exporter side but AFAIU, the actual deletion happens on the importer endpoint. The content does not get deleted on exporter side. In that case, this permission check should happen on importer side.
While I agree that checking for a DELETE distribution action remove/delete privilege at the source sling instance is incorrect, I'd argue against checking for any privilege on the target (recipient) Sling instance because:
* semantically it is entirely target sling instance's business what it does with incoming content. It might not even have the same access control paradigm as the source sling instance. The target sling instance can choose to reject an incoming distribution by performing any validation it wishes, but source sling instance need not be bothered about those rules. If the target instance doesn't like the distribution request, it responds with a failure - that should be the only contract. 
* implementation wise, such a "pre-validation" (which should be abstracted rather than just concerning itself with permission validation) might require in a remote call to target sling instance, which will not be desirable esp when there is a middleware involved (say, Apache Kafka)

I'd propose that validation of a custom privilege - specific to content-distribution - at the source instance should be _sufficient_ to determine if distribution request should be _created_ at all or not. This privilege should be enforced outside JCR and the enforcement should only be restricted to source sling instance.

> Distribution code checks for jcr:removeNode permissions on importer side for DELETE request
> -------------------------------------------------------------------------------------------
>
>                 Key: SLING-9212
>                 URL: https://issues.apache.org/jira/browse/SLING-9212
>             Project: Sling
>          Issue Type: Bug
>          Components: Content Distribution
>            Reporter: Mohit Arora
>            Priority: Major
>             Fix For: Content Distribution Core 0.4.4
>
>
> When a resource is distributed from one endpoint to other with RequestType set to DELETE, the execute method of SimpleDistributionAgent [checks the permissions for the passed resolver on given path(s)|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/agent/impl/SimpleDistributionAgent.java#L175]. In case of DELETE request, apart from the [configured permissions|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/agent/impl/PrivilegeDistributionRequestAuthorizationStrategy.java#L85], it also checks for {{jcr:removeNode}} permissions for the user on the path. This check happens on the exporter side but AFAIU, the actual deletion happens on the importer endpoint. The content does not get deleted on exporter side. In that case, this permission check should happen on importer side.
> cc - [~marett], [~ashishc]



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