Folder shared with Secure view can be downloaded.

S-Panta commented 4 weeks ago

Describe the bug

When a folder is shared with secure view role, it could be downloaded. However if the folder is empty, the file inside get downloaded but the content won't be available for preview.

Steps to reproduce

1.Make a new user Einstein

Create a new folder test
Admin shares test with Einstein with Secure view role.

Einstein download the folder.

curl -k  --location 'https://host.docker.internal:9200/archiver?id={folder-id}' -ueinstein:relativity --output -

Expected behavior

The folder download should not be possible.

Actual behavior

The folder could be downloaded. Suppose there are files and folders inside the folder. They could be downloaded but the content couldn't be opened.

Setup

Please describe how you started the server and provide a list of relevant environment variables or configuration files.

```console OCIS_XXX= Infinite Scale 5.1.0-prealpha+1ef7292e21 Community ```

Additional context

Add any other context about the problem here.

saw-jan commented 4 weeks ago

Similar but with project space: https://github.com/owncloud/ocis/issues/9303

phil-davis commented 4 weeks ago

Suppose there are files and folders inside the folder. They could be downloaded but the content couldn't be opened.

What is received? A "zip" of the folder? Or some other data format?

What do you mean by "the content couldn't be opened"? I suppose that "something" is returned in response to the curl GET. Is the "something" just nonsense data? Or is there any information in the response that has metadata or content of the file(s) inside it?

S-Panta commented 4 weeks ago

@phil-davis This is the binary response of folder when there is a text file upon sharing the parent folder via secure view role

test0000755000000000000000000000000014632541116010147 5ustar0000000000000000test/New file.txt0000644000000000000000000000001314632541114012410 0ustar0000000000000000error: permission denied: storage_id:"f76b0374-d9b4-4bf1-bee4-1b8990a50656" opaque_id:"96eeb8b3-6d43-436d-9406-85b2f07b3aa1" space_id:"bf1498ca-77ce-4ee6-b398-4c59201599da" %

If this is the text file, the downloaded zip file upon extraction can be opened but the content in the file is read as error: perm

phil-davis commented 4 weeks ago

the downloaded zip file upon extraction can be opened but the content in the file is read as error: perm

Interesting behavior. Anyway, the file content is not returned - that is "a good thing".

For Secure View, the resource name (folder/file path) is OK to be seen by the user. For example, in a UI the user should be able to list files in a folder that is shared with "Secure View" permissions, and select which file they want to view.

So I don't think that there is any data leak here.

But someone should give an opinion about if this response should be "tidied up".

if the whole folder has only Secure View permissions, then it would be nicer to just return a "no permissions" HTTP status, and don't deliver any zip file payload. (Having the "error: perm" embedded in the zip file is a bit tricky)
if only some files in the folder have Secure View permission and others have ordinary read permission, then maybe the zip file could completely exclude the files that do not have read permission. Having embedded "error:perm" in a zip file might be a bit painful for the receiver?

S-Panta commented 3 weeks ago

I've manually tested this with the latest build today. These are my findings. In UI, the download option won't be available if shared via a secure role. However, it could be downloaded via API that gives weird binary responses above https://github.com/owncloud/ocis/issues/9369#issuecomment-2165191385 . But if the folder has a view or other role but the file inside it has secure view, somehow the parent role shows dominance over the child role. The folder could be downloaded and the file be viewed.

micbar commented 3 weeks ago

We should return 403 on that api call if the folder is shared via secure view.

phil-davis commented 3 weeks ago

But if the folder has a view or other role but the file inside it has secure view, somehow the parent role shows dominance over the child role. The folder could be downloaded and the file be viewed.

That case needs a decision about the requirements. Normally, so far, permissions have been cumulative

Alice shares folder with GroupA with Editor role
Alice shares folder/file.doc with Brian with just Read permission
Brian is a member of GroupA Brian gets full Editor access to folder/file.doc If Brian is removed from GroupA, then Brian will only have read access to folder/file.doc

Is the requirement the same for Secure View? i.e. cumulative permissions

The consequence of cumulative permissions is that sharers can sometimes not realize what total permissions someone has, and they get surprised that they explicitly share to Brian with Secure View, but that Brian can still download the document because he gets Read privs via some other permission grant.

kobergj commented 2 weeks ago

But if the folder has a view or other role but the file inside it has secure view, somehow the parent role shows dominance over the child role. The folder could be downloaded and the file be viewed.

All permissions in ocis are cumulative except for the denial permission which is in technical preview. So this is expected behaviour, the highest permission wins.

if only some files in the folder have Secure View permission and others have ordinary read permission, then maybe the zip file could completely exclude the files that do not have read permission

This is a much more interesting case. We need to clarify: Should you be able to download a folder that is shared via secure view? It makes sense if you have read access to the children.

Having embedded "error:perm" in a zip file might be a bit painful for the receiver?

This is definitely a bug. Files should not be added to the archive when there is some sort of error.

owncloud / ocis