Persistent lock on folder still allows to create files but not overwrite

[PVince81]

Steps

1. Two users "user1" and "user2"
2. Login as "user1"
3. Share a new folder "test" with "user2"
4. As user1 lock the folder "test": curl -u user1:test -X LOCK http://localhost/owncloud/remote.php/webdav/test -d "<?xml version='1.0' encoding='UTF-8'?><d:lockinfo xmlns:d='DAV:'> <d:lockscope><d:shared/></d:lockscope></d:lockinfo>"
5. Login as "user2"
6. Upload a file "test.txt" into the "test" folder
7. Overwrite file "test.txt" into the "test" folder by uploading again

Expected result

Neither initial upload nor overwrite should be allowed.

Actual result

Initial upload goes through but overwrite says "Locked".

Version

Observed while testing #32250, likely also present on master ac0f01b

@DeepDiver1975 is this by design ?

[ownclouders]

GitMate.io thinks possibly related issues are #24737 (File Locked), #26838 (File locking), #26896 (Unable to rename a newly created Folder, locked), #1775 (Not possible to create folders on IE9), and #16246 (Restore from trash overwrites existing files/folders).

[PVince81]

This feels like the lock is not doing its protective work correctly. => p2

[PVince81]

https://tools.ietf.org/html/rfc4918#section-6.2

as far as I understand Webdav methods must pass the lock token to be able to perform operations (with exclusive lock)

[PVince81]

check litmus tests which seem to pass, and see what expectations are there

[PVince81]

agreed with @DeepDiver1975 that it looks like a bug due to inconsistency.

[PVince81]

need to clarify how expected behavior for shared lock vs exclusive lock for regular Webdav requests

[PVince81]

check if exclusive locks consistently block webdav methods, this is what Litmus already tests

[PVince81]

From the RFC: https://tools.ietf.org/html/rfc4918#section-7.4

Expressed otherwise, a write lock of either kind protects any request
that would create a new resource in a write locked collection, any
request that would remove an internal member URL of a write locked
collection, and any request that would change the segment name of any
internal member.

To me this means that a user cannot create, delete nor rename any resource inside the collection.
However overwriting would be allowed as it doesn't change URL of a sub-resource.

There's also a thing about the lock depth: if the depth is 0, the lock only affects the direct members of the collection, not sub-collections. With depth infinite, all children are affected / locked.

From a quick debug I see that the locks from the collection are not retrieved when Sabre queries them on the path "files/user2/test/test.txt"

[PVince81]

as discussed with @DeepDiver1975:

• we need to query the locks on the parent when the node does not exist
• we agreed that overwriting should be disallowed as well

[PVince81]

for depth:0, Sabre doesn't support it currently: sabre-io/dav#297

[PVince81]

For depth: infinite, here's a PR that fixes it: #34171

[davitol]

I think step 3 is not needed for reproducing the bug. Without sharing it also happens

3\. Share a new folder "test" with "user2"

[PVince81]

indeed

[davitol]

Same behavior happens with Public Link:

Steps

1. Share a new folder "test" via sharelink with upload permissions
2. As user1 lock the folder "test": curl -u user1:test -X LOCK http://localhost/owncloud/remote.php/webdav/test -d "<?xml version='1.0' encoding='UTF-8'?><d:lockinfo xmlns:d='DAV:'> <d:lockscope><d:shared/></d:lockscope></d:lockinfo>"
3. Copy the sharelink and paste it in a browser
4. Upload a file "test.txt" into the "test" folder
5. Overwrite file "test.txt" into the "test" folder by uploading again

Expected result

Neither initial upload nor overwrite should be allowed.

Actual result

Initial upload goes through but overwrite says "Locked".

[PVince81]

@davitol does the PR solve that too or need extra work ?

[davitol]

@PVince81 No, #34171 does not solve the Public Link issue

[PVince81]

reopening as we missed the public link case

[individual-it]

I think the public link issue is fixed by #34224

[PVince81]

it is, closing