owncloud / core

:cloud: ownCloud web server core (Files, DAV, etc.)
https://owncloud.com
GNU Affero General Public License v3.0
8.36k stars 2.06k forks source link

multiple clients having each own shared locks should be able to use the resource sending the lock token #34360

Open individual-it opened 5 years ago

individual-it commented 5 years ago

Steps to reproduce

  1. user1 share file to user2
  2. as user1 lock the shared file with an shared lock
  3. as user2 lock the shared file with an shared lock
  4. as user1 try to overwrite the file sending the own lock token

Expected behaviour

in case of multiple shared locks every client should be able to use the own token to change the resource. RFC 4518: http://www.webdav.org/specs/rfc4918.html#exclusive-lock

However, there are times when the goal of a lock is not to exclude others from exercising an access right but rather to provide a mechanism for principals to indicate that they intend to exercise their access rights. Shared locks are provided for this case. A shared lock allows multiple principals to receive a lock. Hence any principal that has both access privileges and a valid lock can use the locked resource.

Actual behaviour

both users lock each other, no one can use the resource

ownclouders commented 5 years ago

GitMate.io thinks the contributor most likely able to help you is @PVince81.

Possibly related issues are https://github.com/owncloud/core/issues/20741 (Sharing tokens should be unique), https://github.com/owncloud/core/issues/19745 (Optimize multiple shared locks for a single process), https://github.com/owncloud/core/issues/34338 (share receiver can rename a file in a locked folder by using the lock token of the owner), https://github.com/owncloud/core/issues/19242 (Locking should be able to handle exceptions), and https://github.com/owncloud/core/issues/24543 (Don't use session if the client uses an auth token).

PVince81 commented 5 years ago

I debugged into Sabre code https://github.com/sabre-io/dav/blob/master/lib/DAV/Locks/Plugin.php#L487 and it finds both tokens. Then it detects that there is a match and happily marks the condition as true.

However when exiting the outer loop, $mustLocks still has the second lock, so it fails. What is missing is a test whether the found lock is a shared lock and whether the remaining one also is one. Seems support for access through shared locks has not been implemented in Sabre.

PVince81 commented 5 years ago

Upstream bug: https://github.com/sabre-io/dav/issues/1134

phil-davis commented 5 years ago

When fixing the bug, we should also test the case when 2 clients of the same user have taken out a shared lock on the same file/folder, and make sure those can update the file. (e.g. the user's tablet and laptop both have the file open and want to shared-lock it)

Also stuff like: 1) One of the clients holding a shared lock renames or moves the file. What happens to the other shared lock - does it still work on the "new" file/folder? 2) One of the clients holding a shared lock deletes the file/folder. What happens to the other shared lock - does it go away nicely? Does anything explode?

PVince81 commented 5 years ago

shared locks currently not a priority, focus is on exclusive lock.

moving to backlog

PVince81 commented 5 years ago

@individual-it is the file owner able to break both locks ?

individual-it commented 5 years ago

@PVince81 no every user can only unlock the own lock