Change wording of Role Descriptions

[kulmann]

Describe the bug

OnlyOffice can write documents from a public link share with "Can upload" role.

Steps to reproduce

1. As "admin", create asdf/test.docx via OnlyOffice
2. As "admin", create a public link to the folder asdf with Can upload role (which states permissions view, download and upload)
3. As anonymous, receive and follow the public link
4. As anonymous, open asdf/test.docx via OnlyOffice
5. As anonymous, write some content into the file and leave the document again
6. As "admin", see that the changes have been written to disk (e.g. by downloading the file after the lock has been released)

Expected behavior

"anonymous" should have a read only view or at least an error message when trying to save.

Actual behavior

"anonymous" can edit and save the document.

[kulmann]

confirmed in both v5.0.0-rc.5 and v4.0.6

[2403905]

Step 2. Is admin create a public link to this file with the Can upload role using API or UI ?
The Can upload role has to be used for folders, bu not for files.

[kulmann]

Step 2. Is admin create a public link to this file with the Can upload role using API or UI ?

The Can upload role has to be used for folders, bu not for files.

Updated the steps to reproduce. Thank you. I tested it with a folder and thought it can be just done the same way with a file, sorry. Everything done via UI, nothing directly via API.

[phil-davis]

To clarify, the "upload" permission should allow "anonymous" to create new files in the folder, but not overwrite (=edit) existing files.

After "anonymous" has uploaded a file, they should not be able to change what they uploaded (can't edit it or delete it), but should be able to list the files and download files.

Correct?

[individual-it]

What API is OnlyOffice using for that? I'm surprised the API tests did not catch it

[micbar]

Wopi

[kulmann]

To clarify, the "upload" permission should allow "anonymous" to create new files in the folder, but not overwrite (=edit) existing files.

After "anonymous" has uploaded a file, they should not be able to change what they uploaded (can't edit it or delete it), but should be able to list the files and download files.

Correct?

Correct, thank you!

[ScharfViktor]

We have e2e coverage for an app provider. It is not very large, because we wanted to implement only primitive tests as a first step (create and open a docx or odt document).
Maybe we should increase the coverage with e2e?

[individual-it]

Yes I think we should. I believe the decision to make only small coverage was because the security related code path should be the same as with webDAV, but that does not look like its true

[2403905]

@kulmann I found the place in a code when we give the write permissions to OnlyOffice but it is not a root issue. Because the permissions statements (view, download and upload) do not contradict that.
If a user can upload then the user can change the file, right?

[2403905]

@micbar @kulmann Is the upload permission redundant in this case?

[kulmann]

If a user can upload then the user can change the file, right?

No... @phil-davis interpretation is what we want: The user can upload files, but not overwrite existing files.

[2403905]

We should clarify the public permission definitions.
FYI @tbsbdr @micbar

[micbar]

If a user can upload then the user can change the file, right?

No... @phil-davis interpretation is what we want: The user can upload files, but not overwrite existing files.

Hm, from a feature POV i tend to agree.

In the CS3 API there is no distinction, upload means also overwrite.

IMHO not solvable in this API version.

[individual-it]

How does the WebDAV API currently react in that case?

[tbsbdr]

Currently I see 3 options

1. fix the bug
2. Add "edit" to the role description
3. remove the whole role "Uploader"

I'd favour the 3rd option for now, as Uploader and Editor would then be almost the same. what do you think @kulmann @micbar ?

[kulmann]

Option 3 would be fine by me. UX-wise the differentiation is hard to understand anyway. So might actually be an improvement...

[micbar]

yes. Option 3 is the best.

[tbsbdr]

ok, so lets remove this role from the link 😬

[micbar]

Hm, that would need a migration 😱

[2403905]

Is the "can upload" should be changed to "can edit" or "can view"?

[micbar]

Yes. We need a better description which matches our behavior

[micbar]

@tbsbdr @kulmann
Let us fix the description.

[tbsbdr]

@tbsbdr propose a text

[tbsbdr]

🙈 adding "edit" to the description won't work, as text or md files can not be edited.
@micbar @kulmann I currently see 2 options and would choose the one with least effort. do you agree?

1. fix the bug
2. Add "edit" to the role description
3. remove the whole role "Uploader"

[micbar]

@tbsbdr Not quite how i understand it.

The main difference is the "delete" permission. Edit is also true for txt and md files.

[tbsbdr]

I could Not Edit txt, can you?

[micbar]

you can upload a new txt file version.

[micbar]

The main problem here is the "edit" vs. "upload" semantics. From the ocis backend perspective, both are uploads and require the upload permission. This is different for directories. They are controlled by the create-directories permission.

[phil-davis]

Note: "edit-save" and "upload-overwrite" really mean the same thing. There is no such thing as "editing" a file that is sitting on a server. Actually, there is also no such thing as "editing" a file on the local filesystem on my laptop. "editing" is a thing that is done in-memory with a user-interface (graphical (LibreOffice...) or textual (VI, nano, a line-based command-line editor...)

When the person doing the "editing" asks to "save", then the editor software does an "upload-overwrite" to the file, wherever the persistent storage is. It might rewrite the whole file, it might append to the end (if the change just adds to the file), it might overwrite just some bytes of the file (if it implements some optimized way of saving just the bits that changed).

[JustKiddingCode]

From a linux file system perspective, which is similar to webdav:
"upload" rights should mean "write permissions to directory, but not the files".
Uploading a file should check if the file exists and the user has write permissions to that file.

mkdir test
echo "bar" > test/foo # works, because of write permissions to test
chmod -w > test/foo
echo "foo" > test/foo # fails, permission denied

[tbsbdr]

@tbsbdr adjust wording for the role

[tbsbdr]

Suggestion for the rewording of the roles description.

• Assuming that "Only for invited ppl" will be removed
• Adds edit to Can upload
• Adds delete to Can edit
• I'd prefer to be less verbose in the role description (option 2)
• Note: In general I'd prefer to remove the Uploader role at all, as it adds little value for the user, but if I understand @micbar correctly this would be expensive due to a migration necessity.

@kulmann is option 2 - less verbose, fine for you?

1) Verbose	2) Less verbose

General overview

	Needs authentication	delete	upload/edit	view	download	upload (existing content not revealed)
Invited People	✅	(✅)	(✅)	(✅)	(✅)	❌
Can edit	❌	✅	✅	✅	✅	❌
Can upload	❌	❌	✅	✅	✅	❌
Can view	❌	❌	❌	✅	✅	❌
Secret File Drop	❌	❌	❌	❌	❌	✅

[tbsbdr]

@kulmann whats your opinion - go or no? 👆

[kulmann]

We had that in the past and changed it to being more verbose because people had difficulties understanding the implications. Now that the whole context is more verbose we may as well go back to less verbose within the dropdown. Fine by me.

[rhafer]

@kulmann So I guess this would be a web ticket now? On the server side we don't have any descriptive texts for the public link types AFAICS.

[kulmann]

@kulmann So I guess this would be a web ticket now? On the server side we don't have any descriptive texts for the public link types AFAICS.

Seems like it, yes. Transferred it to web now.