public locking on folder returning 403 instead of 405 in new web dav api version

[KarunAtreya]

Steps to reproduce

1. admin creates folders parent and parent/child
2. admin created files parent/parent.txt and parent/child/child.txt
3. admin creates a public share for folder parent
4. public tries to lock folder child using new webdav api endpoint version

 curl -kv -XLOCK -uDPTMsy4r2TtFBpN: http://localhost/core/remote.php/dav/public-files/DPTMsy4r2TtFBpN/child --data \
 '<?xml version='1.0' encoding='UTF-8'?>
<d:lockinfo xmlns:d='DAV:'>                                  
        <d:lockscope>      
                <d:exclusive/>
        </d:lockscope>        
</d:lockinfo>' | xmllint --format -

Expected behaviour

the response should be:

HTTP/1.1 405 Method Not Allowed
<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
    <s:exception>Sabre\DAV\Exception\MethodNotAllowed</s:exception>
    <s:message>Locking not allowed from public endpoint</s:message>
</d:error>

Actual behaviour

but the response is:

< HTTP/1.1 403 Forbidden
<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Sabre\DAVACL\Exception\NeedPrivileges</s:exception>
  <s:message>User did not have the required privileges ({DAV:}write-content) for path "public-files/DPTMsy4r2TtFBpN/child"</s:message>
  <d:need-privileges>
    <d:resource>
      <d:href>/core/remote.php/dav/public-files/DPTMsy4r2TtFBpN/child</d:href>
      <d:privilege>
        <d:write-content/>
      </d:privilege>
    </d:resource>
  </d:need-privileges>
</d:error>

Note: this issue does not persist on files of pubic share but only on the subfolders in the public share in new webdav api version

Server configuration

Operating system: ubuntu

Database: mysql

PHP version: 7.4

ownCloud version: ownCloud 10.13.0 prealpha (git)

[phil-davis]

A bug-demo test scenario was added in PR #40883
When this issue is being fixed, the real expected-behavior scenario should be enabled, and the bug-demo scenario should be deleted. Then the real expected-behavior scenario should be made to pass.

[phil-davis]

@jvillafanez I had a quick look at this and it seems that PR #36402 implemented the code that checks for public-link endpoints and returns MethodNotAllowed when "the public" tries to lock something.

Somehow the behavior is different when trying to lock a resource that is a folder.
Using a path with public.php/webdav/ results in a 405 MethodNotAllowed being returned, as expected.
Using a path with remote.php/dav/public-files/ results in a 403 Forbidden being returned, but we expect a 405.

But I don't see anything special in that PR about folders, and I don't see code that specifically handles the 2 different public link paths.

Do you have any ideas? Is this something that can be fixed easily?

[jvillafanez]

No idea why the code isn't going through https://github.com/owncloud/core/blob/master/apps/dav/lib/Connector/Sabre/PublicDavLocksPlugin.php#L63
Just changing the method to "UNLOCK" (trying to unlock the same file) goes through the "httpUnlock" method of the plugin.

So, it doesn't seems the code is wrong, but something is interfering with the code flow and it doesn't go through the "httpLock" method.
Do we know when this issue started? Trying to limit the changes we need to check.

[jvillafanez]

It might be an issue with the dav ACLs... following patch seems to fix the issue, although I'm not sure if we want to go that way

diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php
index 338be6e781..3070d4c6b7 100644
--- a/apps/dav/lib/Server.php
+++ b/apps/dav/lib/Server.php
@@ -173,7 +173,7 @@ class Server {
                // with performance and locking issues because it will query
                // every parent node which might trigger an implicit rescan in the
                // case of external storages with update detection
-               if (!$this->isRequestForSubtree(['files'])) {
+               if (!$this->isRequestForSubtree(['files', 'public-files'])) {
                        // acl
                        $acl = new DavAclPlugin();
                        $acl->principalCollectionSet = [

I can make a PR after @DeepDiver1975 confirmation.

[phil-davis]

Do we know when this issue started? Trying to limit the changes we need to check.

The issue has probably been there the whole time. We discovered that the test scenario for "new" public webdav API was not actually doing what it said. We fixed the test code in #40883 and noticed the scenario that fails.