Shared image file appears to have "SRGDNVW" permissions (although PROPFIND ground truth has "SRGD")

[marcelklehr]

Describe the bug
A sharee of an image can still open the image editor even though the have no editing permissions from the share. Saving the edited image will fail without an explanation.

To Reproduce
Steps to reproduce the behavior:

1. Open using another account
2. Share a picture to test account
3. Uncheck "Editing" rights
4. Go to web app via test account
5. Preview shared picture
6. Edit button is visible and picture can be edited (saving fails)

Expected behavior
Edit button is not visible

Additional context
The Viewer#canEdit computed property reveals "SRGDNVW" permissions for this.currentFile, but the PROPFIND-returned xml data shows permissions correctly as "SRGD". Presumably, somewhere that information is lost or overridden.

[marcelklehr]

cc @skjnldsv

[marcelklehr]

It turns out that the PROPFIND on the containing folder lists the file permissions incorrectly as "SRGDNVW"
while the PROPFIND directly on the file yields the correct "SRGD".

[marcelklehr]

Debugging this with @artonge we found that this line

https://github.com/nextcloud/server/blob/df5283ad05f8ffa1a0fe979429746ca071a54326/lib/private/Files/View.php#L1502

automatically adds UPDATE permissions on any movable mount in order for users to able to move and rename the shared file, which however then leads to single file shares being displayed as editable in the directory listing, even if the sharer disabled editing.

@icewind1991 Do you have an idea on how we could mitigate this?

From my point of view we have four options forward:

1. Do not offer sharers to restrict editing permissions on single-file shares and thus always assume they are not editable
2. Do not automatically add update permissions to single-file shares to avoid edit buttons being displayed in the UI that don't work, this would also mean that people cannot move or rename these shares
3. Introduce a new EDIT permission that is only about changing the file content
4. Implement a hack in all clients to make sure we use the permissions property of a single-file PROPFIND instead of the bulk directory PROPFIND

also cc @jancborchardt

[jancborchardt]

For this specific case at least I’m thinking the use-case is valid:
You get a read-only share, but you could want to edit it and save it as a new file. Of course saving it over the original file is not possible, but "Save as" should be.

Or am I misunderstanding this case?

[marcelklehr]

You get a read-only share, but you could want to edit it and save it as a new file. Of course saving it over the original file is not possible, but "Save as" should be.

Agreed. The question is if there are any other instances of "Edit" functionality that would break due to this permissions inconsistency. Text editor opens the file in read-only mode, for example, which I'd say makes sense, given that it's a read-only share, but then it doesn't allow editing and saving to a new file.

[icewind1991]

Fix is here: nextcloud/server#38179