Sort out deletion / trashbin issue

[PVince81]

Steps

1. Login as "user1"
2. Create a folder "test"
3. Put some files into "test", for example "test/test.txt"
4. Create a guest user "guest1" by sharing a folder "test"
5. Login as "guest1"
6. Delete the file "test/test.txt"

Expected result

File "test.txt" only appears in "user1" trashbin.
Nothing in "guest1"'s trashbin.
Ideally the trashbin entry should even be invisible.

Actual result

File "test.txt" does appear in "user1" trashbin.
Error occurred appears in the UI, probably because the jail prevented the file to be moved to "guest1"'s trashbin.

The code in question lives here: https://github.com/owncloud/core/blob/v9.1.4/apps/files_trashbin/lib/Trashbin.php#L273

@IljaN as discussed

[IljaN]

@PVince81 The current behaviour is that a error is shown, but the file is deleted without trashbin.

[PVince81]

Yes, so we need to make sure that there is no error shown.

Unfortunately this might mean we need to add specific code to the trashbin...

[PVince81]

that or introduce the concept of "this user has no trashbin so please don't bother trying to put stuff there" which is some variant of "files_trashbin app only enabled for some user". Note that we can't disable the trashbin for guest users because it would prevent some important logic to run like the one that makes a copy in the owner's trashbin.

[IljaN]

I find it kind of strange that the trashbins are connected between users

[PVince81]

They aren't really connected directly. Every user has their own trashbin, there is no such thing as shared trashbins.

However when a share recipient deletes a file, the requirement was that the owner still has a chance to restore it. The only way was to have the trashbin store a copy in both trashbins.

[IljaN]

And why don`t we want this behaviour for guest users?

[PVince81]

We want this behavior, but we do not want anything to appear in the guest user's trashbin because as that user has "no storage" then no files should be stored in "data/$userId/files_trashbin".

[PVince81]

refreshing the page shows that the file is deleted.

The reason is that the logic for trashbin and shares is in the files_trashbin app in core: the logic moves the file to the share owner's trashbin and also makes a copy of the file into the recipient's trashbin (the guest who deletes). That copy fails. But since this copy is happening at the end, after the actual deletion/moving to owner trashbin, the file is already removed from the storage.

So one idea to fix this would be to add some code in files_trashbin to check if the storage is read-only (guest). If yes, then omit the extra copy.

[PVince81]

checking isUpdatable() on the "files_trashbin" folder might be enough.

As far as I remember there is a storage wrapper in core for guests that will mask away write permissions.

[PVince81]

https://github.com/owncloud/core/blob/v10.0.3beta/apps/files_trashbin/lib/Trashbin.php#L309

[pmaier1]

Expected result: Nothing in "guest1"'s trashbin. Ideally the trashbin entry should even be invisible.

Guests should not be able to own files but I actually would want a guest to be able to restore files that he deleted. Behavior should be exactly as it is with usual shares:

• A shares a folder with B
• B deletes a file within the share
• Both users have the file in the trash bin and are able to restore

@PVince81 how is it actually? In the case above we don't have two files but one is linked to the other, right? (guest would not become owner if we adapt the behavior?!)

[PVince81]

@pmaier1 ownCloud doesn't support any kind of file linking, symlinking or hard linking.

The trashbin entries are full copies of the files. This means that to support a scenario where a guest is able to restore files, we'd need the guest user to have an own storage (with quota??) on which to store the copy of the trashed file.

The fact that trashbin entries are copies is problematic in different scenarios as well, see owncloud/core#22594.

We might need to rework/rewrite the trashbin app to work differently and have some sort of linking system.

[pmaier1]

Need to think about this and find a proper solution.
Cleaning up current milestone as we need a release soon -> no development started here (or no PR referenced), moving this to the next version.

[hodyroff]

Somehow I can imagine both scenarios: Guest users shall be able to delete and guest users shall generally not be able to delete anything ...
Agree that the trashbin for the non-owner should ideally be a "link" to the real file and not a copy of the file, so it can be restored with meta-data.

[PVince81]

It is already possible to remove delete permission from a guest by removing "can delete" from the permissions when sharing.

[PVince81]

Seems we should also remove the "Deleted files" entry for the guests, make it invisible, as there is no trashbin for said users.

Well, the alternative though would be to allow guest users to have a trash actually... This somehow contradicts the fact that they are not supposed to have a storage because their trashed files would go there.

Proposed approaches:

Approach 1:

• Remove trashbin from UI for guest users
• Fix core's Trashbin.php to not attempt to setup trashbin for guest user and fall back to owner's trashbin (POC here Check trashbin perms before moving to trash core#29820)

Approach 2:

• Let guest users have their own trashbin: tweak the permission mask in core to only disallow write operations on "files" but not on "files_trashbin".

Approach 3: (future)

• Implement link in trashbin (this is mostly a rework/rewrite of most of the trashbin logic): Meta data lost when recipient restores a file core#22594

I'm fine with either approach 1 or 2 for the short term.

@pmaier1 @hodyroff

[PVince81]

POC for approach 1: owncloud/core#29820

[PVince81]

I'll need a decision between Approach 1 and Approach 2 before continuing.

[PVince81]

I'd like to go with Approach 1 if no one else has objections

[pmaier1]

After discussing with @PVince81 we decided to go with approach 1 as it solves the problem and is least effort. Later we can discuss if guests need a trash bin / need to be able to restore files they deleted. Sharers can still recover deleted files from trash bin anyway.

[PVince81]

Fix will be in core 10.0.8 owncloud/core#29820

[patrickjahns]

While testing owncloud/core#30240 - I noticed that the trashbin icon is still visible

[PVince81]

needs new ticket and will probably need some CSS hackery unfortunately

[patrickjahns]

Created - #204