Adobe-Consulting-Services / acs-aem-commons

http://adobe-consulting-services.github.io/acs-aem-commons/
Apache License 2.0
456 stars 604 forks source link

WorkflowPackageManagerImpl requires that the user creating workflow package has read access to /var/workflow/packages #2019

Closed gicig closed 4 years ago

gicig commented 5 years ago

Required Information

Expected Behavior

If user creating workflow package has read&write permissions for /var/workflow/packages/project1/tenant1/, it should be able to create packages there, even though it doesn't have any permissions for /var/workflow/packages

Actual Behavior

The user with permissions described above can not create packages for /var/workflow/packages/project1/tenant1/, unless it is granted read permissions for /var/workflow/packages/ - check here

Steps to Reproduce

Create user with permissions above and try to create a workflow package using WorkflowPackageManagerImpl#create(ResourceResolver , String, String, String...)

Links

https://github.com/Adobe-Consulting-Services/acs-aem-commons/blob/master/bundle/src/main/java/com/adobe/acs/commons/workflow/impl/WorkflowPackageManagerImpl.java

joerghoh commented 5 years ago

Seems to be caused by https://github.com/Adobe-Consulting-Services/acs-aem-commons/blob/master/bundle/src/main/java/com/adobe/acs/commons/workflow/impl/WorkflowPackageManagerImpl.java#L265

The detection if the "new" or the "legacy" path is used could be done on startup of the service; of course then you need to restart the service/instance when you migrate the workflows. But if that happens on the startup, you could use a session with elevated privileges to detect that and then the user does not have to have read access to /var/workflow/packages anymore.

joerghoh commented 5 years ago

@gicig Can you review the bugfix I provided in #2077 and check if it is working for you?

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

davidjgonzalez commented 4 years ago

Fixed via #2077

gicig commented 4 years ago

Sorry for not testing this earlier. Now that I did test it in my project I noticed something strange: acs-commons-workflowpackagemanager-service doesn't have read permissions on /var/workflow/packages. Please check https://drive.google.com/file/d/1q2y92bBgPzuGTZAjeYXPWIGVE6TsSkev/view?usp=sharing

Even though there is an allow permission under the respective rep:policy, useradmin doesn't show it does have read rights. Service resource resolver doesn't have those permissions either - getting resource from /var/workflow/package results in null.

However, when adding the permission through useradmin interface it can get that resource. Even though the rep:policy gets exact same allow permission: https://drive.google.com/open?id=16nJ6lCi7JPyH6hyivdsDLrSVlpWQfEEc

joerghoh commented 4 years ago

Strange. IIRC I heard that there were issues once when ACLs have been introduced with packages like this, that some internal cache got out of sync; adding a new (random) ACL fixed it. Can you test if you can reproduce the issue you reported and if adding a random ACL helps?

gicig commented 4 years ago

For acs-commons-workflowpackagemanager-service I added through CRX:

joerghoh commented 4 years ago

You tried my recommendation and added some random ACLs, but there was no effect and the required permissions /var/workflow/package were not there? In that case it's not this Oak issue, and we have to investigate further.