fix: permissions when copying from readonly storage

[rotdrop]

• Resolves: [Bug]: Can't edit/delete files copied from read-only shared folders #51846

Summary

#51846 (comment)

TODO

• Cleanup. Perhaps a bit hacky.

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[susnux]

@rotdrop please use conventional commit messages, e.g.:
fix: permissions when copying from readonly storage

See also https://www.conventionalcommits.org/en/v1.0.0/

[rotdrop]

I wonder if similar permission problems could occur with moveFromStorage, at least in edge cases.

[come-nc]

This is quite mysterious to me, maybe @icewind1991 understands better what this is about?
It’s at least missing comments to make it understandable.

[come-nc]

(did not meant to close, it was a misclick)

[rotdrop]

This is quite mysterious to me, maybe @icewind1991 understands better what this is about? It’s at least missing comments to make it understandable.

Well, but we have the linked issue #51846 with quite a bunch of comments. So in summary:

• when copying files out of read-only storages like read-only shares or read-only external mounts
• ... then the read-only permissions are copied over to the destination
• if you then try to modify the copied files in the destination location then you find them read-only
• this is -- at the very least -- very unexpected: as end-user you of course assume that copying stuff to a location where you have write permissions results in modifiable data
• also: the web-frontend does not provide any means to the end-user to correct this problem

[come-nc]

Thank you, I understand the problem now, but I’m not sure about the fix. It feels a bit hackish.
scan_permissions comes from the scanner?

There should be a way to have something more straightforward to set permissions after copy, and avoid having so many special handling.

Gentle ping @icewind1991

[rotdrop]

Thank you, I understand the problem now, but I’m not sure about the fix. It feels a bit hackish. scan_permissions comes from the scanner?

There should be a way to have something more straightforward to set permissions after copy, and avoid having so many special handling.

Gentle ping @icewind1991

Thank you for coming back to this. Yeah, the fix is hackish (see TODO topic in my initial comment). I think that the scan_permissions come from the scanner code. This also somehow indicates that this problem was already popping up earlier ...

[rotdrop]

Thank you, I understand the problem now, but I’m not sure about the fix. It feels a bit hackish. scan_permissions comes from the scanner?
There should be a way to have something more straightforward to set permissions after copy, and avoid having so many special handling.
Gentle ping @icewind1991

Thank you for coming back to this. Yeah, the fix is hackish (see TODO topic in my initial comment). I think that the scan_permissions come from the scanner code. This also somehow indicates that this problem was already popping up earlier ...

See #48769, commit hash 6419de7

[icewind1991]

#53733 attempts to fix the same issue I believe.

From what I can tell the main difference is that this PR has logic to take the permissions of the target parent folder into account. I'm not sure if that's actually needed though.

[rotdrop]

#53733 attempts to fix the same issue I believe.

From what I can tell the main difference is that this PR has logic to take the permissions of the target parent folder into account. I'm not sure if that's actually needed though.

I am also not sure. However, I was thinking of target folder with write permissions, but without read permissions (e.g. a write-only share which is used as a p.o box for public upload). Don't known if this is relevant.

[github-actions]

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)