nextcloud / files_lock

GNU Affero General Public License v3.0
25 stars 8 forks source link

WebDAV PROPPATCH doesn't allow changing the favourite status of locked files #68

Open thisIsTheFoxe opened 2 years ago

thisIsTheFoxe commented 2 years ago

Related

27:

  • (+) keep a possibility to add locked file to favorites

Steps

  1. Lock admin/Nextcloud Manual.pdf
  2. Use WebDAV to change the favourite status, see logs

Expected

Changing the favourite should still be possible using WebDAV. The web UI uses POST /apps/files/api/v1/files/Nextcloud%20Manual.pdf to change the favourite tag which still works as expected.

Actual

423: Sabre\DAV\Exception\Locked, see logs

Logs:

```xml [Request]: PROPPATCH http://localhost:8080/remote.php/dav/files/admin/Nextcloud%20Manual.pdf [Headers]: Authorization: Basic YWRtaW46cmlZemFUZzYyS2tZdzF0NXg3RmRyd3VYd1Y1VkxNU2s4ZDgxQ05UVEVFanBxb0h4Y0hGT09CSE8wMXBpUVpBWExNM1YwaEdP Content-Type: application/x-www-form-urlencoded OCS-APIRequest: true User-Agent: Mozilla/5.0 (iOS) Nextcloud-iOS/4.4.0 [Body]: 1 [Response]: [Status Code]: 423 [Headers]: Cache-Control: no-store, no-cache, must-revalidate Connection: Keep-Alive Content-Length: 288 Content-Security-Policy: default-src 'none'; Content-Type: application/xml; charset=utf-8 Date: Mon, 02 May 2022 12:36:09 GMT Expires: Thu, 19 Nov 1981 08:52:00 GMT Keep-Alive: timeout=5, max=99 Pragma: no-cache Referrer-Policy: no-referrer Server: Apache/2.4.53 (Debian) X-Content-Type-Options: nosniff X-Frame-Options: SAMEORIGIN X-Permitted-Cross-Domain-Policies: none X-Powered-By: PHP/8.0.18 X-Robots-Tag: none X-XSS-Protection: 1; mode=block [Body]: Sabre\DAV\Exception\Locked files/admin/Nextcloud Manual.pdf ```
juliusknorr commented 2 years ago

As I already assumed this comes from the Sabre implementation of WebDAV locking which generally blocks PROPPATCH if a write lock is active http://webdav.org/specs/rfc2518.html#rfc.section.7.1

Now the property of favourite is of course a custom one and unique per user, so the write lock for that could be skipped. A quick attempt for allowing only that didn't work out well, as the validation is handled way before the prop patch is even parsed, so this needs a bit more of thinking.

For later reference, just skipping validation for any PROPPATCH would work easily:

--- a/lib/DAV/LockPlugin.php
+++ b/lib/DAV/LockPlugin.php
@@ -6,6 +6,7 @@ use OCA\DAV\Connector\Sabre\CachingTree;
 use OCA\DAV\Connector\Sabre\FakeLockerPlugin;
 use OCA\DAV\Connector\Sabre\Node as SabreNode;
 use OCA\DAV\Connector\Sabre\ObjectTree;
 use OCA\FilesLock\AppInfo\Application;
 use OCA\FilesLock\Exceptions\LockNotFoundException;
 use OCA\FilesLock\Exceptions\UnauthorizedUnlockException;
@@ -25,6 +26,7 @@ use Sabre\DAV\Exception\LockTokenMatchesRequestUri;
 use Sabre\DAV\INode;
 use Sabre\DAV\Locks\Plugin as SabreLockPlugin;
 use Sabre\DAV\PropFind;
+use Sabre\DAV\PropPatch;
 use Sabre\DAV\Server;
 use Sabre\HTTP\RequestInterface;
 use Sabre\HTTP\ResponseInterface;
@@ -63,9 +65,23 @@ class LockPlugin extends SabreLockPlugin {
                }
                $this->locksBackend = new LockBackend($server, $this->fileService, $this->lockService, $absolute);
                $server->on('propFind', [$this, 'customProperties']);
+               $server->on('propPatch', [$this, 'propPatch']);
                parent::initialize($server);
        }

+       public function validateTokens(RequestInterface $request, &$conditions) {
+               if ($request->getMethod() === 'PROPPATCH') {
+                       return;
+               }
+               return parent::validateTokens($request, $conditions);
+       }
+