Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restoring of deleted shared file inside folder inside groupfolder not possible #3339

Open
x7airworker opened this issue Oct 10, 2024 · 17 comments · May be fixed by #3358
Open

Restoring of deleted shared file inside folder inside groupfolder not possible #3339

x7airworker opened this issue Oct 10, 2024 · 17 comments · May be fixed by #3358
Labels
1. to develop Issues that are ready for development bug feature: trashbin Items related to the trashbin feature

Comments

@x7airworker
Copy link
Contributor

x7airworker commented Oct 10, 2024

How to use GitHub

  • Please use the 👍 reaction to show that you are affected by the same issue.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

Steps to reproduce

  1. Create a groupfolder "A" with ACL and give readonly permissions to user "A"
  2. Create a folder "B" inside groupfolder "A" and give full access to user "A"
  3. Share the folder with full permissions to another user "B"
  4. Create a file inside "C" and share it with another user "B"
  5. Login to the other user "B" and delete file "C"
  6. Try to restore the file with any of the users "A" or "B".

Expected behaviour

The file should be able to be restorable, with at least user "A".

Actual behaviour

The request results in http status 500 with a NotPermittedException.
I've noticed that the original_location column in table oc_group_folders_trash doesn't contain the folder B, but just the name of file C with the folder_id of groupfolder A. I think this results in a bad ACL check.

Server configuration

Operating system: Alpine; Docker

Web server: Nginx

Database: MySQL (Galera)

PHP version: 8.2.7

Nextcloud version: 28.0.9

Group folders version: 16.0.8

Updated from an older Nextcloud/ownCloud or fresh install: fresh install

Where did you install Nextcloud from: official source; self-built docker image

Are you using external storage, if yes which one: s3

Are you using encryption: no

Are you using an external user-backend, if yes which one: no

Client configuration

Browser: Chrome

Operating system: MacOS 15

Logs

Web server error log

Web server error log
127.0.0.1 -  10/Oct/2024:08:22:58 +0000 "MOVE /remote.php" 500

Nextcloud log (data/nextcloud.log)

Nextcloud log
{"reqId":"EEyWFwynx1WX0OSYUjAx","level":3,"time":"2024-10-10T08:22:58+00:00","remoteAddr":"CENSORED","user":"jesser","app":"webdav","method":"MOVE","url":"/remote.php/dav/trashbin/jesser/trash/Neue%20Textdatei.txt.d1728548573","message":"Exception thrown: OCP\\Files\\NotPermittedException","userAgent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/129.0.0.0 Safari/537.36","version":"28.0.9.1","exception":{"Exception":"OCP\\Files\\NotPermittedException","Message":"","Code":0,"Trace":[{"file":"/var/www/html/apps/files_trashbin/lib/Trash/TrashManager.php","line":64,"function":"restoreItem","class":"OCA\\GroupFolders\\Trash\\TrashBackend","type":"->","args":[["OCA\\GroupFolders\\Trash\\GroupTrashItem"]]},{"file":"/var/www/html/apps/files_trashbin/lib/Sabre/AbstractTrash.php","line":97,"function":"restoreItem","class":"OCA\\Files_Trashbin\\Trash\\TrashManager","type":"->","args":[["OCA\\GroupFolders\\Trash\\GroupTrashItem"]]},{"file":"/var/www/html/apps/files_trashbin/lib/Sabre/RestoreFolder.php","line":75,"function":"restore","class":"OCA\\Files_Trashbin\\Sabre\\AbstractTrash","type":"->","args":[]},{"file":"/var/www/html/3rdparty/sabre/dav/lib/DAV/Tree.php","line":178,"function":"moveInto","class":"OCA\\Files_Trashbin\\Sabre\\RestoreFolder","type":"->","args":["Neue Textdatei.txt.d1728548573","trashbin/jbebendorf/trash/Neue Textdatei.txt.d1728548573",["OCA\\Files_Trashbin\\Sabre\\TrashFile"]]},{"file":"/var/www/html/3rdparty/sabre/dav/lib/DAV/CorePlugin.php","line":612,"function":"move","class":"Sabre\\DAV\\Tree","type":"->","args":["trashbin/jbebendorf/trash/Neue Textdatei.txt.d1728548573","trashbin/jbebendorf/restore/Neue Textdatei.txt.d1728548573"]},{"file":"/var/www/html/3rdparty/sabre/event/lib/WildcardEmitterTrait.php","line":89,"function":"httpMove","class":"Sabre\\DAV\\CorePlugin","type":"->","args":[["Sabre\\HTTP\\Request"],["Sabre\\HTTP\\Response"]]},{"file":"/var/www/html/3rdparty/sabre/dav/lib/DAV/Server.php","line":472,"function":"emit","class":"Sabre\\DAV\\Server","type":"->","args":["method:MOVE",[["Sabre\\HTTP\\Request"],["Sabre\\HTTP\\Response"]]]},{"file":"/var/www/html/3rdparty/sabre/dav/lib/DAV/Server.php","line":253,"function":"invokeMethod","class":"Sabre\\DAV\\Server","type":"->","args":[["Sabre\\HTTP\\Request"],["Sabre\\HTTP\\Response"]]},{"file":"/var/www/html/3rdparty/sabre/dav/lib/DAV/Server.php","line":321,"function":"start","class":"Sabre\\DAV\\Server","type":"->","args":[]},{"file":"/var/www/html/apps/dav/lib/Server.php","line":382,"function":"exec","class":"Sabre\\DAV\\Server","type":"->","args":[]},{"file":"/var/www/html/apps/dav/appinfo/v2/remote.php","line":35,"function":"exec","class":"OCA\\DAV\\Server","type":"->","args":[]},{"file":"/var/www/html/remote.php","line":172,"args":["/var/www/html/apps/dav/appinfo/v2/remote.php"],"function":"require_once"}],"File":"/var/www/html/custom_apps/groupfolders/lib/Trash/TrashBackend.php","Line":129,"message":"","exception":{},"CustomMessage":"Exception thrown: OCP\\Files\\NotPermittedException"}}
@x7airworker x7airworker added 0. Needs triage Issues that need to be triaged bug labels Oct 10, 2024
@x7airworker x7airworker changed the title Restoring of shared file inside folder inside groupfolder not possible Restoring of deleted shared file inside folder inside groupfolder not possible Oct 10, 2024
@x7airworker
Copy link
Contributor Author

@unfixa1 Patching the original_location column to match the expected value allows the restore again. So I can confirm that my assumptions were correct.

@provokateurin
Copy link
Member

Thanks for the debugging, consider this an acknowledged bug that will be fixed!

@provokateurin provokateurin added 1. to develop Issues that are ready for development and removed 0. Needs triage Issues that need to be triaged labels Oct 11, 2024
@provokateurin
Copy link
Member

@x7airworker Can you please post more comprehensive reproduction steps?
They are currently not very clear to me and I was not able to create your setup.
Please always say which user, which group and so on does something.

Create a groupfolder "A" with ACL and give readonly permissions to user "A"

How do you give readonly permissions to user A, through a group the user is part of?

Share the folder with full permissions to another user "B"

From user A or a different user?

Create a file inside "C" and share it with another user "B"

You probably meant Create a file "C" inside "B" and share it with another user "B"?

Login to the other user "B" and delete file "C"

Delete it from where? The shared file, the shared folder or the groupfolder (does user B have access to that as well?)

Try to restore the file with any of the users "A" or "B".

Sounds like user A and B are part of a group that has access to Groupfolder A including delete permissions?

@x7airworker
Copy link
Contributor Author

x7airworker commented Oct 14, 2024

@provokateurin Sure, I've automated the whole setup so I might missed some parts.
I've also describe the folder structure a bit too simple.

How do you give readonly permissions to user A, through a group the user is part of?

There is a group "A".
The groupfolder is configured to give full permissions to group "A".
The folder "B" of groupfolder "A" has a ACL rule which denies every access to group "A".
The folder "B" of groupfolder "A" has a second ACL rule which allows all-access except delete and share to user "A".

Image

All files inside folder "B" will automatically get full-access so this enables the user to create folders inside "B" with delete permissions.
Lets say we create a folder "C" inside "B" of groupfolder "A" and inside of that folder a file "D".

From user A or a different user?

Yes

You probably meant Create a file "C" inside "B" and share it with another user "B"?

Yes

Delete it from where? The shared file, the shared folder or the groupfolder (does user B have access to that as well?)

User B only has read-only access to the groupfolder. So the permsisions are extended through the share.
The file "D" inside folder "C" needs to be deleted. It then appears in the deleted files for both users, but no one can restore it.

Sounds like user A and B are part of a group that has access to Groupfolder A including delete permissions?

A has access to his own folder, while B is getting access to a share initiated by A.

Let me know if you need more information. I could also try to create a bash or SQL script which creates this exact setup.

@provokateurin
Copy link
Member

Thanks for the quick reply! I will try to reproduce it again with these steps. A script might not be necessary, but if I'm still unable to reproduce it it would be very helpful.

@provokateurin
Copy link
Member

The folder "B" of groupfolder "A" has a ACL rule which denies every access to group "A".
The folder "B" of groupfolder "A" has a second ACL rule which allows all-access except delete and share to user "A".

In the screenshot you deny the group read permission and the user inherits it. This way the user can never see the folder, so something is still not right with your steps.
Also without sharing allowed user A can not share the folder to user B 🤔

To continue with your steps I allowed these two for the user A as well.
When creating the share from user A to user B I encountered this error: Error creating the share: Cannot increase permissions of /A/B which I can not get past.

@x7airworker
Copy link
Contributor Author

x7airworker commented Oct 14, 2024

In the screenshot you deny the group read permission and the user inherits it

I agree that it looks wrong, but it works without problems for me (maybe because they are created programmatically and the mask isn't like the frontend expects it). So I would just give the read permission to the user.

Also without sharing allowed user A can not share the folder to user B

all permissions are allowed for files inside folder "B" -> see my previous answer "All files inside folder "B" will automatically get full-access.". The folder inside "B" then looks like this:

Image

The folder structure then looks like this:

  • A (groupfolder; group allowed)
    • B (folder; group denied; user allowed read-write)
      • C (folder; user allowed everything)

@provokateurin
Copy link
Member

Ah I think I missed the ACL for folder C!
Let me give it another try.

@provokateurin provokateurin linked a pull request Oct 14, 2024 that will close this issue
@provokateurin
Copy link
Member

@x7airworker I tried to reproduce it with a unit test, but I still couldn't get it to work: #3358
Can you check if any of the steps are wrong?

What is confusing to me is that the trash of the user A seems to be empty. I wonder if it is related to #3281. @x7airworker can you make sure you are on the latest Groupfolders release?

@provokateurin
Copy link
Member

Also if you can share your script that has all the steps it would also be super helpful.

@x7airworker
Copy link
Contributor Author

x7airworker commented Oct 14, 2024

@provokateurin I've just took a short look at your test.
In my code I explicitly enable ACL for the folder after the creation using
$this->folderManager->setFolderACL($groupFolderId, true);
this is currently missing.

This would be my first guess, but I would take a closer look tomorrow.

@provokateurin
Copy link
Member

Added the line, but nothing changed.
What is also odd is that the permissions are wrong when querying them (see the commented out line), so that hints at the fact that there is still something not making the ACL apply.

@joshtrichards joshtrichards added the feature: trashbin Items related to the trashbin feature label Oct 14, 2024
@x7airworker
Copy link
Contributor Author

Hey @provokateurin I've forked your test and tried to replicate my logic.
I'm now at the point that the file gets deleted and the original_location is not as expected.
See https://github.com/x7airworker/groupfolders/blob/fix/trash/empty-original-location/tests/Trash/TrashBackendTest.php
Is this test case enough for further debugging?

@provokateurin
Copy link
Member

Thank you!! Having a test to replicate this consistently is super helpful, I should be able to fix it now :)

@x7airworker
Copy link
Contributor Author

@provokateurin Any news on this?

@provokateurin
Copy link
Member

Sorry, I'm busy with other tasks at the moment. I will come back to it hopefully soon.

@provokateurin
Copy link
Member

I improved the test by also checking for the trash of the user A and it also has the wrong original location :/

@provokateurin provokateurin moved this to 📄 To do (~10 entries) in 📁 Files team Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Issues that are ready for development bug feature: trashbin Items related to the trashbin feature
Projects
Status: 📄 To do (~10 entries)
Development

Successfully merging a pull request may close this issue.

4 participants
@joshtrichards @provokateurin @x7airworker and others