diff --git a/README.md b/README.md index 7fdc8991..afe19e03 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,12 @@ Locks use an ownership and authorization model (**lock type**) and are created o Token-owned locks are primarily intended for automatic client locking, for example when a file is opened by a desktop client or another WebDAV client that supports native WebDAV locking. The lock type alone does not imply that a lock was created automatically. + A token-owned lock records both the token and the user who created it. Modifying the file requires the token and the request must come from that user (RFC 4918, section 6.4); a request from another user is refused even if it carries the token, and a write through the PHP file system API requires the token to be presented through the lock scope of `ILockManager`. The lock is released by a native `UNLOCK` that presents the token and comes from someone who may write the file, by the user recorded as its owner, by the file owner, or by an administrator. Possession of the token alone is not enough: the token is a publicly readable value (RFC 4918, section 6.5), so privileges are enforced through the normal permission mechanism as section 6.4 requires. + +### Enforcement + +Locks are enforced by file identity on every storage a file can be reached through: the owner's home storage, shares, group and team folders, external storages and custom mounts. A user lock blocks every other user, an app lock blocks everything outside the owning app's lock scope, and a token lock blocks everything that does not present the token as described above. Deleting or moving a folder is refused while it contains a file locked by someone else, so a locked file can neither be removed nor relocated through its parent. + ### Access paths An access path is the mechanism used to create or remove a lock. Locks can be created and removed through the Web UI, the OCS API, native WebDAV `LOCK`/`UNLOCK`, or the `X-User-Lock` WebDAV extension. These are access paths to the same lock implementation; they are not separate lock types. @@ -48,7 +54,7 @@ An access path is the mechanism used to create or remove a lock. Locks can be cr Native WebDAV locking is the token-based access path for Type 2 locks. - A `LOCK` request creates a token-owned lock and associates it with a WebDAV lock token. A subsequent `UNLOCK` request must provide that token. + A `LOCK` request creates a token-owned lock for the authenticated user and associates it with a WebDAV lock token. A subsequent `UNLOCK` request must provide that token and must come from a user who may write the file. A `Timeout` header sets the lifetime of the lock (`Second-N` or `Infinite`); without it the configured `lock_timeout` applies. A `LOCK` refresh (no body, the token in the `If` header) moves the expiry. The `` element of the request is not stored; the lock reports the display name of the user who created it. A `LOCK` on a file that already carries another lock answers `423 Locked`, an `UNLOCK` with an unknown token answers `409 Conflict`, and an `UNLOCK` the caller is not allowed to perform answers `403 Forbidden`. - **WebDAV with `X-User-Lock`** @@ -68,16 +74,18 @@ An access path is the mechanism used to create or remove a lock. Locks can be cr ### Unlocking -The file owner can override existing locks through access paths that support user override. Administrators can also force-unlock files using `occ files:lock --unlock `. In the case of automatic locks, apps and client applications are typically responsible for removing no longer needed locks. +The file owner can override existing locks on files stored in their own home storage (directly or through a share of it); on group folders, external storages and other mounts every user is reported as owner, so no owner override applies there. The user recorded as the owner of a lock can always release it, whatever the lock type. Administrators can force-unlock files using `occ files:lock --unlock `. In the case of automatic locks, apps and client applications are typically responsible for removing no longer needed locks. By default, files are locked indefinitely. -When unlocking an app-owned lock, provide a user ID that has access to the file: +A forced unlock removes the lock of any type and does not require the user who created it to still have access to the file: -`occ files:lock --unlock ` +`occ files:lock --unlock ` This command can be helpful when locks become stale. For example, when a user forgets to remove a manually created lock or a desktop client remains offline and automatic unlocking is not configured. +Deleting a file removes its lock. A file restored from the trash bin starts unlocked. + ### Lock timeout Locks have no expiry by default (`lock_timeout = -1`). @@ -88,12 +96,16 @@ Administrators can change the time of the maximum lock time in minutes (30) usin Set `lock_timeout` to `-1` to disable expiration. +Every lock carries an absolute expiry. Locking a file again (OCS, `X-User-Lock`, or a native WebDAV refresh) moves the expiry to the configured timeout, or to the timeout requested through the WebDAV `Timeout` header, counted from the moment of the refresh. Expired locks stop blocking immediately and a background job removes them from the database. + ### Locking Administrators can manually lock files using `occ`: `occ files:lock [] [--status] [--unlock]` +Only files can be locked through the Web UI, the OCS API, `X-User-Lock`, the command line and the PHP `ILockManager` API; a folder is refused there. Native WebDAV clients may still lock a collection as RFC 4918 requires; such a lock protects the collection's own name and location, not its members. + ## Capabilities API If locking is available the app will expose itself through the capabilities endpoint under the `files` key: @@ -128,9 +140,9 @@ WebDAV returns the following additional properties in response to a `PROPFIND` r - `2` represents a token-owned lock, typically created through native WebDAV locking - `{http://nextcloud.org/ns}lock-owner`: User ID of the lock owner for user-owned and token-owned locks. This property is empty for app-owned locks. - `{http://nextcloud.org/ns}lock-owner-displayname`: Display name of the lock owner -- `{http://nextcloud.org/ns}lock-owner-editor`: App ID for an app-owned lock. Clients can use it to suggest joining the collaborative editing session in the web interface or through direct editing. In the response to an `X-User-Lock` `LOCK` request, this property currently contains the lock owner regardless of lock type. +- `{http://nextcloud.org/ns}lock-owner-editor`: App ID for an app-owned lock. Clients can use it to suggest joining the collaborative editing session in the web interface or through direct editing. It is empty for user-owned and token-owned locks on every access path. - `{http://nextcloud.org/ns}lock-time`: Timestamp at which the lock was created -- `{http://nextcloud.org/ns}lock-timeout`: Configured lock timeout in seconds from creation. A value of `0` indicates that the lock does not expire. +- `{http://nextcloud.org/ns}lock-timeout`: Lifetime of the lock in seconds counted from `lock-time`; it grows when the lock is refreshed. A value of `0` indicates that the lock does not expire. - `{http://nextcloud.org/ns}lock-token`: Lock token. Clients using native WebDAV locking must retain it while holding the lock and provide it when unlocking. ```bash @@ -185,6 +197,9 @@ The response will give back the updated properties after obtaining the lock with #### Error status codes +- 400 Unsupported `X-User-Lock-Type` +- 403 The caller lacks update permission, or the resource is a folder +- 404 The resource does not exist - 423 Unable to lock because the file is already locked by another owner ### Manually unlock a file @@ -221,8 +236,10 @@ curl -X UNLOCK \ #### Error status codes +- 400 Unsupported `X-User-Lock-Type` +- 404 The resource does not exist - 412 Unable to unlock because the file is not locked -- 423 Unable to unlock if the lock is owned by another user +- 423 Unable to unlock if the lock is owned by another user; the response carries the existing lock ## OCS API @@ -254,22 +271,37 @@ curl -X PUT 'http://admin:admin@nextcloud.local/ocs/v2.php/apps/files_lock/lock/ ``` #### Failure + +The file is already locked by someone else. The response carries the existing lock, without its token: ``` failure - 500 - + 423 + File is currently locked by admin - -1 - OCA\FilesLock\Exceptions\AlreadyLockedException - File is already locked by admin + 12 + admin + admin + 123 + -1 + 1648046707 + + 0 ``` +#### Status codes + +- 200 Lock created or refreshed +- 400 Unsupported `lockType`, invalid file ID, or the ID belongs to a folder +- 403 The caller lacks update permission on the file +- 404 The file does not exist or is not accessible to the caller +- 423 The file is locked by another owner + ### Unlocking a file `DELETE /apps/files_lock/lock/{fileId}` @@ -280,7 +312,7 @@ curl -X DELETE 'http://admin:admin@nextcloud.local/ocs/v2.php/apps/files_lock/lo #### Parameters -Not applicable. +- `lockType` (optional): The lock type the caller asserts, matching the value used when locking. The user recorded as the owner of a lock can release it whatever the type, and the file owner can release any lock on a file of their home storage. #### Success ``` @@ -295,18 +327,24 @@ Not applicable. ``` #### Failure + +The file is not locked: ``` failure - 500 - + 412 + File is not locked - - -1 - OCA\FilesLock\Exceptions\LockNotFoundException - - + ``` + +#### Status codes + +- 200 Lock released +- 400 Unsupported `lockType` or invalid file ID +- 404 The file does not exist or is not accessible to the caller +- 412 The file is not locked +- 423 The lock is held by someone else and the caller may not release it; the response carries the existing lock diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index c0ea6f91..94b73211 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -14,19 +14,25 @@ use OCA\FilesLock\ConfigLexicon; use OCA\FilesLock\Listeners\BeforeFileSystemSetupListener; use OCA\FilesLock\Listeners\LoadAdditionalScripts; +use OCA\FilesLock\Listeners\NodeDeletedListener; use OCA\FilesLock\Listeners\PropfindPropertiesListener; use OCA\FilesLock\LockProvider; use OCP\AppFramework\App; use OCP\AppFramework\Bootstrap\IBootContext; use OCP\AppFramework\Bootstrap\IBootstrap; use OCP\AppFramework\Bootstrap\IRegistrationContext; +use OCP\Files\Cache\CacheEntryRemovedEvent; use OCP\Files\Events\BeforeFileSystemSetupEvent; use OCP\Files\Events\BeforeRemotePropfindEvent; +use OCP\Files\Events\Node\NodeDeletedEvent; +use OCP\Files\Lock\ILock; use OCP\Files\Lock\ILockManager; class Application extends App implements IBootstrap { public const APP_ID = 'files_lock'; + public const array SUPPORTED_LOCK_TYPES = [ILock::TYPE_USER, ILock::TYPE_APP, ILock::TYPE_TOKEN]; + public const DAV_PROPERTY_LOCK = '{http://nextcloud.org/ns}lock'; public const DAV_PROPERTY_LOCK_OWNER_TYPE = '{http://nextcloud.org/ns}lock-owner-type'; public const DAV_PROPERTY_LOCK_OWNER = '{http://nextcloud.org/ns}lock-owner'; @@ -55,6 +61,14 @@ public function register(IRegistrationContext $context): void { BeforeFileSystemSetupEvent::class, BeforeFileSystemSetupListener::class ); + $context->registerEventListener( + NodeDeletedEvent::class, + NodeDeletedListener::class + ); + $context->registerEventListener( + CacheEntryRemovedEvent::class, + NodeDeletedListener::class + ); $context->registerConfigLexicon(ConfigLexicon::class); } diff --git a/lib/Command/Lock.php b/lib/Command/Lock.php index 569a64c1..1862bb2b 100644 --- a/lib/Command/Lock.php +++ b/lib/Command/Lock.php @@ -26,6 +26,7 @@ use OCP\Files\InvalidPathException; use OCP\Files\Lock\ILock; use OCP\Files\Lock\LockContext; +use OCP\Files\Lock\OwnerLockedException; use OCP\Files\NotFoundException; use OCP\IUserManager; use OCP\User\Exceptions\UserNotFoundException; @@ -42,11 +43,10 @@ occ files:lock --status <file_id> Forcibly unlock a file: - occ files:lock --unlock <file_id> [<user_id>] + occ files:lock --unlock <file_id> -For app-owned locks, provide a user ID that has access to the file. This can be -needed for files stored in Groupfolders: - occ files:lock --unlock <file_id> <user_id> +A forced unlock removes the lock of any type regardless of who holds it and +does not require the lock owner to still have access to the file. Uninstall the app and delete all locks: occ files:lock --uninstall @@ -63,9 +63,6 @@ public function __construct( } /** - * @throws NotFoundException - * @throws UnauthorizedUnlockException - * @throws NotFileException * @throws InvalidPathException */ public function __invoke( @@ -73,7 +70,7 @@ public function __invoke( IInput $input, #[Argument(description: 'ID of the file to lock, unlock, or inspect', name: 'file_id')] ?string $fileId = null, - #[Argument(description: 'Lock owner when locking; user with file access when unlocking an app-owned lock', name: 'user_id')] + #[Argument(description: 'Lock owner when locking', name: 'user_id')] ?string $userId = null, #[Option(description: 'Fully uninstall the app from your Nextcloud')] bool $uninstall = false, @@ -98,14 +95,26 @@ public function __invoke( } if ($unlock === true) { - return $this->unlockFile($output, $userId, $fileId); + return $this->unlockFile($output, $fileId); } if ($userId === null || $userId === '') { throw new InvalidArgumentException('Not enough arguments (missing: "user_id")'); } - return $this->lockFile($output, $fileId, $userId); + try { + return $this->lockFile($output, $fileId, $userId); + } catch (OwnerLockedException $e) { + $output->writeln('File #' . $fileId . ' is already locked by ' . $e->getLock()->getOwner() . ''); + } catch (NotFileException) { + $output->writeln('#' . $fileId . ' is not a file; only files can be locked'); + } catch (UnauthorizedUnlockException|UserNotFoundException $e) { + $output->writeln('' . $e->getMessage() . ''); + } catch (NotFoundException) { + $output->writeln('File #' . $fileId . ' not found for user ' . $userId . ''); + } + + return ExitCode::Failure; } private function getStatus(IOutput $output, int $fileId): ExitCode { @@ -144,18 +153,15 @@ private function lockFile(IOutput $output, int $fileId, string $userId): ExitCod $file = $this->fileService->getFileFromId($user->getUID(), $fileId); $output->writeln('locking ' . $file->getName() . ' to ' . $userId . ''); - $this->lockService->lock(new LockContext( + $this->lockService->acquire(new LockContext( $file, ILock::TYPE_USER, $userId )); return ExitCode::Success; } - /** - * @throws UnauthorizedUnlockException - */ - private function unlockFile(IOutput $output, ?string $userId, int $fileId): ExitCode { + private function unlockFile(IOutput $output, int $fileId): ExitCode { try { - $this->lockService->unlockFile($fileId, $userId, true); + $this->lockService->forceUnlock($fileId); $output->writeln('Unlocked file #' . $fileId . ''); } catch (LockNotFoundException) { $output->writeln('File #' . $fileId . ' was already unlocked'); diff --git a/lib/Controller/LockController.php b/lib/Controller/LockController.php index c07d159e..ca8e2944 100644 --- a/lib/Controller/LockController.php +++ b/lib/Controller/LockController.php @@ -14,6 +14,7 @@ use OC\AppFramework\OCS\V2Response; use OCA\FilesLock\AppInfo\Application; use OCA\FilesLock\Exceptions\LockNotFoundException; +use OCA\FilesLock\Exceptions\NotFileException; use OCA\FilesLock\Exceptions\UnauthorizedUnlockException; use OCA\FilesLock\Model\FileLock; use OCA\FilesLock\Service\FileService; @@ -26,6 +27,8 @@ use OCP\Files\Lock\ILock; use OCP\Files\Lock\LockContext; use OCP\Files\Lock\OwnerLockedException; +use OCP\Files\NotFoundException; +use OCP\Files\NotPermittedException; use OCP\IL10N; use OCP\IRequest; use OCP\IUserSession; @@ -66,6 +69,13 @@ public function __construct( #[NoAdminRequired] #[NoSubAdminRequired] public function locking(string $fileId, int $lockType = ILock::TYPE_USER): DataResponse { + if (!in_array($lockType, Application::SUPPORTED_LOCK_TYPES, true)) { + return $this->fail(new \InvalidArgumentException('Unsupported lock type'), [], Http::STATUS_BAD_REQUEST, false); + } + if (!is_numeric($fileId)) { + return $this->fail(new \InvalidArgumentException('Invalid file id'), [], Http::STATUS_BAD_REQUEST, false); + } + try { $user = $this->userSession->getUser(); if ($user === null) { @@ -73,13 +83,19 @@ public function locking(string $fileId, int $lockType = ILock::TYPE_USER): DataR } $file = $this->fileService->getFileFromId($user->getUID(), (int)$fileId); - $lock = $this->lockService->lock(new LockContext( + $lock = $this->lockService->acquire(new LockContext( $file, $lockType, $user->getUID() )); return new DataResponse($lock, Http::STATUS_OK); } catch (OwnerLockedException $e) { return new DataResponse($e->getLock(), Http::STATUS_LOCKED); + } catch (NotFoundException $e) { + return $this->fail($e, [], Http::STATUS_NOT_FOUND, false); + } catch (NotFileException $e) { + return $this->fail($e, [], Http::STATUS_BAD_REQUEST, false); + } catch (UnauthorizedUnlockException|NotPermittedException $e) { + return $this->fail($e, [], Http::STATUS_FORBIDDEN, false); } catch (Exception $e) { return $this->fail($e); } @@ -88,14 +104,20 @@ public function locking(string $fileId, int $lockType = ILock::TYPE_USER): DataR #[NoAdminRequired] #[NoSubAdminRequired] public function unlocking(string $fileId, int $lockType = ILock::TYPE_USER): DataResponse { + if (!in_array($lockType, Application::SUPPORTED_LOCK_TYPES, true)) { + return $this->fail(new \InvalidArgumentException('Unsupported lock type'), [], Http::STATUS_BAD_REQUEST, false); + } + if (!is_numeric($fileId)) { + return $this->fail(new \InvalidArgumentException('Invalid file id'), [], Http::STATUS_BAD_REQUEST, false); + } + try { $user = $this->userSession->getUser(); if ($user === null) { throw new \LogicException('User not logged in'); } - $this->lockService->enableUserOverride(); - $this->lockService->unlockFile((int)$fileId, $user->getUID()); + $this->lockService->unlockFile((int)$fileId, $user->getUID(), false, $lockType); return new DataResponse(); } catch (LockNotFoundException) { @@ -103,14 +125,15 @@ public function unlocking(string $fileId, int $lockType = ILock::TYPE_USER): Dat $response->setStatus(Http::STATUS_PRECONDITION_FAILED); return $response; } catch (UnauthorizedUnlockException) { - try { - $lock = $this->lockService->getLockFromFileId((int)$fileId); - } catch (LockNotFoundException) { + $lock = $this->lockService->getActiveLock((int)$fileId); + if ($lock === null) { $response = new DataResponse(); $response->setStatus(Http::STATUS_PRECONDITION_FAILED); return $response; } return new DataResponse($lock, Http::STATUS_LOCKED); + } catch (NotFoundException $e) { + return $this->fail($e, [], Http::STATUS_NOT_FOUND, false); } catch (Exception $e) { return $this->fail($e); } @@ -133,7 +156,13 @@ private function buildOCSResponse(string $format, DataResponse $data): V1Respons } if ($containedData instanceof FileLock) { - $data->setData($containedData->jsonSerialize()); + $payload = $containedData->jsonSerialize(); + if ($data->getStatus() === Http::STATUS_LOCKED) { + // the token is the credential of a token lock and OCS never accepts + // one, so the caller that just lost the conflict has no use for it + unset($payload['token']); + } + $data->setData($payload); } if ($this->ocsVersion === 1) { diff --git a/lib/Cron/Unlock.php b/lib/Cron/Unlock.php index 41f2b9d7..c2aaa848 100644 --- a/lib/Cron/Unlock.php +++ b/lib/Cron/Unlock.php @@ -25,10 +25,6 @@ public function __construct( #[\Override] protected function run($argument): void { - $this->deleteExpiredLocks(); - } - - private function deleteExpiredLocks(): void { - $this->lockService->removeLocks($this->lockService->getDeprecatedLocks(1000)); + $this->lockService->removeLocksIfExpired($this->lockService->getExpiredLocks(1000)); } } diff --git a/lib/DAV/LockBackend.php b/lib/DAV/LockBackend.php index a21d63bc..c5afeec0 100644 --- a/lib/DAV/LockBackend.php +++ b/lib/DAV/LockBackend.php @@ -9,23 +9,36 @@ namespace OCA\FilesLock\DAV; -use Exception; -use OCA\FilesLock\Service\FileService; +use Closure; +use OCA\FilesLock\Exceptions\LockNotFoundException; +use OCA\FilesLock\Exceptions\NotFileException; +use OCA\FilesLock\Exceptions\UnauthorizedUnlockException; +use OCA\FilesLock\Model\FileLock; use OCA\FilesLock\Service\LockService; +use OCP\Files\Folder; use OCP\Files\Lock\ILock; use OCP\Files\Lock\LockContext; use OCP\Files\Lock\OwnerLockedException; use OCP\Files\Node; -use OCP\Files\NotFoundException; use OCP\IUserSession; +use Sabre\DAV\Exception\Forbidden; +use Sabre\DAV\Exception\Locked; +use Sabre\DAV\Exception\NotFound; use Sabre\DAV\Locks\Backend\BackendInterface; use Sabre\DAV\Locks\LockInfo; +/** + * Thin adapter between Sabre's lock backend contract and the canonical lock + * service. Every lock stored by the app is presented to Sabre as an exclusive + * write lock, so Sabre's token validation applies to all lock types. + */ class LockBackend implements BackendInterface { + /** + * @param Closure(string): Node $nodeResolver resolves a request uri to a node, throws Sabre NotFound + */ public function __construct( - private readonly FileService $fileService, private readonly LockService $lockService, - private readonly bool $absolute, + private readonly Closure $nodeResolver, private readonly IUserSession $userSession, ) { } @@ -36,86 +49,115 @@ public function __construct( */ #[\Override] public function getLocks($uri, $returnChildLocks): array { - $locks = []; - try { - // TODO: check parent - $file = $this->getFileFromUri($uri); - $lock = $this->lockService->getLockFromFileId($file->getId()); + return array_map( + fn (FileLock $lock): LockInfo => $lock->toLockInfo(), + $this->getFileLocks($uri, (bool)$returnChildLocks) + ); + } - if ($lock->getType() === ILock::TYPE_USER && $lock->getOwner() === $this->userSession->getUser()?->getUID()) { - return []; - } + /** + * Active locks of the resource at $uri, optionally including locks on files below it. + * + * @return list + */ + public function getFileLocks(string $uri, bool $returnChildLocks): array { + try { + $node = $this->resolve($uri); + } catch (NotFound) { + return []; + } + $locks = []; + $lock = $this->lockService->getActiveLock($node->getId()); + if ($lock !== null) { $lock->setUri($uri); + $locks[] = $lock; + } - return [$lock->toLockInfo()]; - } catch (Exception) { - return $locks; + if ($returnChildLocks && $node instanceof Folder) { + foreach ($this->lockService->getLocksBelow($node->getId()) as $entry) { + $entry['lock']->setUri(rtrim($uri, '/') . '/' . $entry['path']); + $locks[] = $entry['lock']; + } } + + return $locks; } /** - * Locks a uri - * + * Create or refresh a token lock. The complete lock is built before it is + * persisted, so a refused request never leaves a partial row behind. * + * @throws Locked when another lock is held on the resource + * @throws Forbidden when the caller may not lock the resource */ #[\Override] public function lock($uri, LockInfo $lockInfo): bool { - try { - $file = $this->getFileFromUri($uri); - $lock = $this->lockService->lock(new LockContext( - $file, - ILock::TYPE_TOKEN, - $lockInfo->token - )); - - $user = $this->userSession->getUser(); - if ($user === null) { - return false; - } + $node = $this->resolve($uri); + $user = $this->userSession->getUser(); + if ($user === null) { + throw new Forbidden('Locking requires an authenticated user'); + } - $lock->setUserId($user->getUID()); - $lock->setTimeout($lockInfo->timeout ?? 0); - $lock->setToken($lockInfo->token); - $lock->setDisplayName($lockInfo->owner); - $lock->setScope($lockInfo->scope); - $this->lockService->update($lock); - return true; - } catch (NotFoundException) { - return true; - } catch (OwnerLockedException) { - return false; + $timeout = null; + if ($lockInfo->timeout !== null) { + $seconds = (int)$lockInfo->timeout; + $timeout = $seconds > 0 ? $seconds : FileLock::ETA_INFINITE; + } + + try { + $lock = $this->lockService->acquire( + new LockContext($node, ILock::TYPE_TOKEN, $user->getUID()), + $timeout, + $lockInfo->token, + null, + false + ); + } catch (OwnerLockedException $e) { + /** @var FileLock $existing */ + $existing = $e->getLock(); + $existing->setUri($uri); + throw new Locked($existing->toLockInfo()); + } catch (NotFileException|UnauthorizedUnlockException $e) { + throw new Forbidden($e->getMessage()); } + + $lockInfo->token = $lock->getToken(); + $lockInfo->owner = $lock->getDisplayName(); + $lockInfo->created = $lock->getCreatedAt(); + $lockInfo->timeout = $lock->isInfinite() ? LockInfo::TIMEOUT_INFINITE : $lock->getETA(); + $lockInfo->depth = 0; + $lockInfo->uri = $uri; + return true; } /** * Removes a lock from a uri * - * + * @throws Forbidden when the presented token or the caller may not release the lock */ #[\Override] public function unlock($uri, LockInfo $lockInfo): bool { try { - $file = $this->getFileFromUri($uri); - } catch (NotFoundException) { + $node = $this->resolve($uri); + } catch (NotFound) { + return true; + } + $owner = $this->userSession->getUser()?->getUID() ?? $lockInfo->token; + try { + $this->lockService->unlock(new LockContext($node, ILock::TYPE_TOKEN, $owner), false, $lockInfo->token); + } catch (LockNotFoundException) { return true; + } catch (UnauthorizedUnlockException $e) { + throw new Forbidden($e->getMessage()); } - $this->lockService->unlock(new LockContext( - $file, - ILock::TYPE_TOKEN, - $lockInfo->token - )); return true; } /** - * @throws NotFoundException + * @throws NotFound */ - private function getFileFromUri(string $uri): Node { - if ($this->absolute) { - return $this->fileService->getFileFromAbsoluteUri($uri); - } - - return $this->fileService->getFileFromUri($uri); + private function resolve(string $uri): Node { + return ($this->nodeResolver)($uri); } } diff --git a/lib/DAV/LockPlugin.php b/lib/DAV/LockPlugin.php index b2ceccaa..f33cf63e 100644 --- a/lib/DAV/LockPlugin.php +++ b/lib/DAV/LockPlugin.php @@ -7,17 +7,16 @@ namespace OCA\FilesLock\DAV; -use OCA\DAV\Connector\Sabre\CachingTree; use OCA\DAV\Connector\Sabre\Directory; use OCA\DAV\Connector\Sabre\FakeLockerPlugin; use OCA\DAV\Connector\Sabre\File; use OCA\DAV\Connector\Sabre\FilesPlugin; -use OCA\DAV\Connector\Sabre\ObjectTree; +use OCA\DAV\Connector\Sabre\Node as SabreNode; use OCA\FilesLock\AppInfo\Application; use OCA\FilesLock\Exceptions\LockNotFoundException; +use OCA\FilesLock\Exceptions\NotFileException; use OCA\FilesLock\Exceptions\UnauthorizedUnlockException; use OCA\FilesLock\Model\FileLock; -use OCA\FilesLock\Service\FileService; use OCA\FilesLock\Service\LockService; use OCP\AppFramework\Http; use OCP\Files\Lock\ILock; @@ -25,7 +24,9 @@ use OCP\Files\Lock\OwnerLockedException; use OCP\Files\Node; use OCP\IUserSession; -use Sabre\DAV\Exception\LockTokenMatchesRequestUri; +use Sabre\DAV\Exception\Forbidden; +use Sabre\DAV\Exception\Locked; +use Sabre\DAV\Exception\NotFound; use Sabre\DAV\INode; use Sabre\DAV\Locks\Plugin as SabreLockPlugin; use Sabre\DAV\PropFind; @@ -34,9 +35,10 @@ use Sabre\HTTP\ResponseInterface; class LockPlugin extends SabreLockPlugin { + private const string TOKEN_PREFIX = 'opaquelocktoken:'; + public function __construct( private readonly LockService $lockService, - private readonly FileService $fileService, private readonly IUserSession $userSession, ) { } @@ -51,21 +53,28 @@ public function initialize(Server $server): void { $server->removeListener('validateTokens', [$fakePlugin, 'validateTokens']); } - $absolute = false; - switch ($server->tree::class) { - case ObjectTree::class: - $absolute = false; - break; - - case CachingTree::class: - $absolute = true; - break; - } - $this->locksBackend = new LockBackend($this->fileService, $this->lockService, $absolute, $this->userSession); + $this->locksBackend = new LockBackend( + $this->lockService, + fn (string $uri): Node => $this->resolveNode($uri), + $this->userSession, + ); $server->on('propFind', $this->customProperties(...)); parent::initialize($server); } + /** + * Resolve a request uri through the DAV tree, whichever tree the server uses. + * + * @throws NotFound + */ + private function resolveNode(string $uri): Node { + $node = $this->server->tree->getNodeForPath($uri); + if (!$node instanceof SabreNode) { + throw new NotFound('Resource is not a file system node'); + } + return $node->getNode(); + } + private function cacheDirectory(Directory $directory): void { $children = $directory->getChildren(); @@ -80,10 +89,10 @@ private function cacheDirectory(Directory $directory): void { continue; } - $ids[] = (string)$id; + $ids[] = (int)$id; } - $ids[] = (string)$directory->getId(); + $ids[] = (int)$directory->getId(); // the lock service will take care of the caching $this->lockService->getLockForNodeIds($ids); } @@ -102,102 +111,145 @@ public function customProperties(PropFind $propFind, INode $node): void { $nodeId = $node->getId(); - $propFind->handle(Application::DAV_PROPERTY_LOCK, function () use ($nodeId, $node): bool { - $lock = $this->lockService->getLockForNodeId($nodeId, $node->getNode()); - return $lock instanceof FileLock; - }); - - $propFind->handle(Application::DAV_PROPERTY_LOCK_OWNER, function () use ($nodeId, $node): ?string { - $lock = $this->lockService->getLockForNodeId($nodeId, $node->getNode()); + $lockOf = fn (): ?FileLock => $this->lockService->getLockForNodeId($nodeId, $node->getNode()); - if ($lock === false) { - return null; - } - - if ($lock->getType() === ILock::TYPE_APP) { - return null; - } + $propFind->handle(Application::DAV_PROPERTY_LOCK, fn (): bool => $lockOf() !== null); - return $lock->getOwner(); + $propFind->handle(Application::DAV_PROPERTY_LOCK_OWNER, function () use ($lockOf): ?string { + $lock = $lockOf(); + return $lock === null || $lock->getType() === ILock::TYPE_APP ? null : $lock->getOwner(); }); - $propFind->handle(Application::DAV_PROPERTY_LOCK_TIME, function () use ($nodeId, $node): ?int { - $lock = $this->lockService->getLockForNodeId($nodeId, $node->getNode()); + $propFind->handle(Application::DAV_PROPERTY_LOCK_TIME, fn (): ?int => $lockOf()?->getCreatedAt()); - if ($lock === false) { - return null; - } - - return $lock->getCreatedAt(); + $propFind->handle(Application::DAV_PROPERTY_LOCK_TIMEOUT, function () use ($lockOf): ?int { + $lock = $lockOf(); + return $lock === null ? null : $this->davTimeout($lock); }); - $propFind->handle(Application::DAV_PROPERTY_LOCK_TIMEOUT, function () use ($nodeId, $node): ?int { - $lock = $this->lockService->getLockForNodeId($nodeId, $node->getNode()); - - if ($lock === false) { - return null; - } - - return $this->davTimeout($lock); + $propFind->handle(Application::DAV_PROPERTY_LOCK_OWNER_DISPLAYNAME, function () use ($lockOf): ?string { + $lock = $lockOf(); + return $lock === null ? null : $this->lockService->injectMetadata($lock)->getDisplayName(); }); - $propFind->handle(Application::DAV_PROPERTY_LOCK_OWNER_DISPLAYNAME, function () use ($nodeId, $node): ?string { - $lock = $this->lockService->getLockForNodeId($nodeId, $node->getNode()); - - if ($lock === false) { - return null; - } + $propFind->handle(Application::DAV_PROPERTY_LOCK_OWNER_TYPE, fn (): ?int => $lockOf()?->getType()); - $this->lockService->injectMetadata($lock); - - return $lock->getDisplayName(); + $propFind->handle(Application::DAV_PROPERTY_LOCK_EDITOR, function () use ($lockOf): ?string { + $lock = $lockOf(); + return $lock?->getType() === ILock::TYPE_APP ? $lock->getOwner() : null; }); - $propFind->handle(Application::DAV_PROPERTY_LOCK_OWNER_TYPE, function () use ($nodeId, $node): ?int { - $lock = $this->lockService->getLockForNodeId($nodeId, $node->getNode()); + $propFind->handle(Application::DAV_PROPERTY_LOCK_TOKEN, fn (): ?string => $lockOf()?->getToken()); + } - if ($lock === false) { - return null; + /** + * The lock token carried by one entry of an If header, or null when it is not + * one of ours. + * + * @param array{token: mixed} $token + */ + private static function lockToken(array $token): ?string { + $value = (string)$token['token']; + return str_starts_with($value, self::TOKEN_PREFIX) + ? substr($value, strlen(self::TOKEN_PREFIX)) + : null; + } + + /** + * Replace Sabre's token-only validation with the application policy: a lock + * blocks a modifying request unless the acting principal may write the file + * (owner of a user lock, or owner of a token lock presenting its token). + * + * @param mixed $conditions + */ + #[\Override] + public function validateTokens(RequestInterface $request, &$conditions): void { + $this->lockService->resetPresentedTokens(); + foreach ($conditions as $condition) { + foreach ($condition['tokens'] as $token) { + $presented = self::lockToken($token); + if ($presented !== null) { + $this->lockService->presentToken($presented); + } } + } - return $lock->getType(); - }); + $method = $request->getMethod(); + if ($method === 'LOCK') { + parent::validateTokens($request, $conditions); + return; + } - $propFind->handle(Application::DAV_PROPERTY_LOCK_EDITOR, function () use ($nodeId, $node): ?string { - $lock = $this->lockService->getLockForNodeId($nodeId, $node->getNode()); - if ($lock === false || $lock->getType() !== ILock::TYPE_APP) { - return null; - } + /** @var LockBackend $backend */ + $backend = $this->locksBackend; + $mustLocks = []; + switch ($method) { + case 'DELETE': + $mustLocks = $backend->getFileLocks($request->getPath(), true); + break; + case 'MKCOL': + case 'MKCALENDAR': + case 'PROPPATCH': + case 'PUT': + case 'PATCH': + $mustLocks = $backend->getFileLocks($request->getPath(), false); + break; + case 'MOVE': + $mustLocks = array_merge( + $backend->getFileLocks($request->getPath(), true), + $backend->getFileLocks($this->server->calculateUri($request->getHeader('Destination')), false) + ); + break; + case 'COPY': + $mustLocks = $backend->getFileLocks($this->server->calculateUri($request->getHeader('Destination')), false); + break; + } - return $lock->getOwner(); - }); + $byToken = []; + foreach ($mustLocks as $lock) { + $byToken[$lock->getToken()] = $lock; + } - $propFind->handle(Application::DAV_PROPERTY_LOCK_TOKEN, function () use ($nodeId, $node): ?string { - $lock = $this->lockService->getLockForNodeId($nodeId, $node->getNode()); - if ($lock === false) { - return null; + foreach ($conditions as $kk => $condition) { + foreach ($condition['tokens'] as $ii => $token) { + $checkToken = self::lockToken($token); + if ($checkToken === null) { + continue; + } + if (isset($byToken[$checkToken])) { + $conditions[$kk]['tokens'][$ii]['validToken'] = true; + continue; + } + foreach ($backend->getFileLocks($condition['uri'], false) as $oddLock) { + if ($oddLock->getToken() === $checkToken) { + $conditions[$kk]['tokens'][$ii]['validToken'] = true; + continue 2; + } + } } + } - return $lock->getToken(); - }); + foreach ($byToken as $lock) { + if (!$this->lockService->canWrite($lock, null)) { + throw new Locked($lock->toLockInfo()); + } + } } #[\Override] public function httpLock(RequestInterface $request, ResponseInterface $response) { if ($request->getHeader('X-User-Lock')) { - /** @var ILock::TYPE_* $lockType */ - $lockType = (int)($request->getHeader('X-User-Lock-Type') ?? ILock::TYPE_USER); + $lockType = $this->getRequestedLockType($request); $response->setHeader('Content-Type', 'application/xml; charset=utf-8'); - $file = $this->fileService->getFileFromAbsoluteUri($this->server->getRequestUri()); - + $file = $this->resolveNode($this->server->getRequestUri()); $user = $this->userSession->getUser(); if ($user === null) { - throw new \LogicException('User not logged in'); + throw new Forbidden('Locking requires an authenticated user'); } try { - $lockInfo = $this->lockService->lock(new LockContext( + $lockInfo = $this->lockService->acquire(new LockContext( $file, $lockType, $user->getUID() )); $response->setStatus(200); @@ -208,13 +260,16 @@ public function httpLock(RequestInterface $request, ResponseInterface $response) ) ); } catch (OwnerLockedException $e) { + $existing = $e->getLock(); $response->setStatus(423); $response->setBody( $this->server->xml->write( '{DAV:}prop', - $this->getLockProperties($e->getLock(), $file) + $this->getLockProperties($existing instanceof FileLock ? $existing : null, $file) ) ); + } catch (NotFileException|UnauthorizedUnlockException $e) { + throw new Forbidden($e->getMessage()); } return false; @@ -226,16 +281,18 @@ public function httpLock(RequestInterface $request, ResponseInterface $response) #[\Override] public function httpUnlock(RequestInterface $request, ResponseInterface $response) { if ($request->getHeader('X-User-Lock')) { - /** @var ILock::TYPE_* $lockType */ - $lockType = (int)($request->getHeader('X-User-Lock-Type') ?? ILock::TYPE_USER); + $lockType = $this->getRequestedLockType($request); $response->setHeader('Content-Type', 'application/xml; charset=utf-8'); - $file = $this->fileService->getFileFromAbsoluteUri($this->server->getRequestUri()); + $file = $this->resolveNode($this->server->getRequestUri()); + $user = $this->userSession->getUser(); + if ($user === null) { + throw new Forbidden('Unlocking requires an authenticated user'); + } try { - $this->lockService->enableUserOverride(); $this->lockService->unlock(new LockContext( - $file, $lockType, $this->userSession->getUser()->getUID() + $file, $lockType, $user->getUID() )); $response->setStatus(200); $response->setBody( @@ -253,7 +310,7 @@ public function httpUnlock(RequestInterface $request, ResponseInterface $respons ) ); } catch (UnauthorizedUnlockException) { - $lock = $this->lockService->getLockFromFileId($file->getId()); + $lock = $this->lockService->getActiveLock($file->getId()); $response->setStatus(Http::STATUS_LOCKED); $response->setBody( $this->server->xml->write( @@ -266,20 +323,28 @@ public function httpUnlock(RequestInterface $request, ResponseInterface $respons return false; } - try { - return parent::httpUnlock($request, $response); - } catch (LockTokenMatchesRequestUri) { - // Skip logging with wrong lock token - return false; + return parent::httpUnlock($request, $response); + } + + private function getRequestedLockType(RequestInterface $request): int { + $header = $request->getHeader('X-User-Lock-Type'); + if ($header === null || $header === '') { + return ILock::TYPE_USER; } + if (!is_numeric($header) || !in_array((int)$header, Application::SUPPORTED_LOCK_TYPES, true)) { + throw new \Sabre\DAV\Exception\BadRequest('Unsupported lock type'); + } + return (int)$header; } private function getLockProperties(?FileLock $lock, Node $file): array { - // We need to fetch the node again to get the proper new Etag - $actingUser = ($file->getOwner() ? $file->getOwner()->getUID() : null) ?? $this->userSession->getUser()->getUID(); - $file = $this->fileService->getFileFromId($actingUser, $file->getId()); + if ($lock !== null) { + $this->lockService->injectMetadata($lock); + } + // the lock change updated the etag in the cache, read it back from there + $etag = $file->getStorage()->getCache()->get($file->getInternalPath())?->getEtag() ?? $file->getEtag(); return [ - FilesPlugin::GETETAG_PROPERTYNAME => $file->getEtag(), + FilesPlugin::GETETAG_PROPERTYNAME => $etag, Application::DAV_PROPERTY_LOCK => $lock !== null, Application::DAV_PROPERTY_LOCK_OWNER_TYPE => $lock ? $lock->getType() : null, Application::DAV_PROPERTY_LOCK_OWNER => $lock ? $lock->getOwner() : null, diff --git a/lib/Db/LocksRequest.php b/lib/Db/LocksRequest.php index fb3c8187..cc4b1818 100644 --- a/lib/Db/LocksRequest.php +++ b/lib/Db/LocksRequest.php @@ -9,17 +9,14 @@ namespace OCA\FilesLock\Db; -use OCA\FilesLock\ConfigLexicon; use OCA\FilesLock\Cron\Unlock; +use OCA\FilesLock\Exceptions\LockConflictException; use OCA\FilesLock\Exceptions\LockNotFoundException; use OCA\FilesLock\Model\FileLock; -use OCP\AppFramework\Services\IAppConfig; -use OCP\AppFramework\Utility\ITimeFactory; use OCP\DB\Exception; use OCP\DB\IResult; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; -use OCP\Server; /** * Class LocksRequest @@ -28,71 +25,128 @@ */ class LocksRequest { public const string TABLE_LOCKS = 'files_lock'; - private readonly int $timeout; + private const array COLUMNS = ['id', 'user_id', 'file_id', 'token', 'creation', 'type', 'ttl', 'owner', 'scope', 'expires_at']; public function __construct( - IAppConfig $appConfig, private readonly IDBConnection $connection, ) { - $this->timeout = $appConfig->getAppValueInt(ConfigLexicon::LOCK_TIMEOUT) * 60; } + /** + * Insert a new lock. The unique index on file_id guarantees at most one row per + * file; a violation is reported as LockConflictException so the caller can + * re-read the winning lock. + * + * @throws LockConflictException + * @throws Exception + */ public function save(FileLock $lock): void { $qb = $this->connection->getQueryBuilder(); $qb->insert(self::TABLE_LOCKS); $qb->setValue('user_id', $qb->createNamedParameter($lock->getOwner())) - ->setValue('file_id', $qb->createNamedParameter($lock->getFileId())) + ->setValue('file_id', $qb->createNamedParameter($lock->getFileId(), IQueryBuilder::PARAM_INT)) ->setValue('token', $qb->createNamedParameter($lock->getToken())) - ->setValue('creation', $qb->createNamedParameter($lock->getCreatedAt())) - ->setValue('type', $qb->createNamedParameter($lock->getType())) - ->setValue('ttl', $qb->createNamedParameter($lock->getTimeout())) - ->setValue('owner', $qb->createNamedParameter($lock->getDisplayName() ?? '')); + ->setValue('creation', $qb->createNamedParameter($lock->getCreatedAt(), IQueryBuilder::PARAM_INT)) + ->setValue('type', $qb->createNamedParameter($lock->getType(), IQueryBuilder::PARAM_INT)) + ->setValue('ttl', $qb->createNamedParameter(max(0, $lock->getTimeout()), IQueryBuilder::PARAM_INT)) + ->setValue('owner', $qb->createNamedParameter($lock->getDisplayName() ?? '')) + ->setValue('scope', $qb->createNamedParameter($lock->getScope(), IQueryBuilder::PARAM_INT)) + ->setValue('expires_at', $qb->createNamedParameter($lock->getExpiresAt(), IQueryBuilder::PARAM_INT)); try { $qb->executeStatement(); - $lock->setId($qb->getLastInsertId()); } catch (Exception $e) { if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { - return; + throw new LockConflictException('A lock already exists for file ' . $lock->getFileId(), 0, $e); } throw $e; } + $lock->setId($qb->getLastInsertId()); } public function update(FileLock $lock): void { $qb = $this->connection->getQueryBuilder(); $qb->update(self::TABLE_LOCKS); $qb->set('token', $qb->createNamedParameter($lock->getToken())) - ->set('ttl', $qb->createNamedParameter($lock->getTimeout())) + ->set('ttl', $qb->createNamedParameter(max(0, $lock->getTimeout()), IQueryBuilder::PARAM_INT)) + ->set('expires_at', $qb->createNamedParameter($lock->getExpiresAt(), IQueryBuilder::PARAM_INT)) ->set('user_id', $qb->createNamedParameter($lock->getOwner())) ->set('owner', $qb->createNamedParameter($lock->getDisplayName() ?? '')) - ->set('scope', $qb->createNamedParameter($lock->getScope())) - ->where($qb->expr()->eq('id', $qb->createNamedParameter($lock->getId()))); + ->set('scope', $qb->createNamedParameter($lock->getScope(), IQueryBuilder::PARAM_INT)) + ->where($qb->expr()->eq('id', $qb->createNamedParameter($lock->getId(), IQueryBuilder::PARAM_INT))); $qb->executeStatement(); } public function delete(FileLock $lock): void { - $qb = $this->connection->getQueryBuilder(); - $qb->delete(self::TABLE_LOCKS) - ->where($qb->expr()->eq('id', $qb->createNamedParameter($lock->getId()))); - - $qb->executeStatement(); + $this->removeIds([$lock->getId()]); } /** * @param int[] $ids */ public function removeIds(array $ids): void { - if (empty($ids)) { - return; + foreach (array_chunk($ids, IQueryBuilder::MAX_IN_PARAMETERS) as $chunk) { + $qb = $this->connection->getQueryBuilder(); + $qb->delete(self::TABLE_LOCKS) + ->where($qb->expr()->in('id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))); + + $qb->executeStatement(); + } + } + + /** + * Remove the given locks, but only those that are still expired at $now. + * + * The cleanup paths read a batch of expired locks and delete it afterwards; + * in between, the owner may have refreshed one of them, which reuses the same + * row. Deleting by id alone would drop a lock that is valid again by then. + * + * @param int[] $ids + * + * @return int number of rows removed + */ + public function removeExpiredIds(array $ids, int $now): int { + $removed = 0; + foreach (array_chunk($ids, IQueryBuilder::MAX_IN_PARAMETERS) as $chunk) { + $qb = $this->connection->getQueryBuilder(); + $qb->delete(self::TABLE_LOCKS) + ->where($qb->expr()->in('id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))) + ->andWhere($qb->expr()->isNotNull('expires_at')) + ->andWhere($qb->expr()->lte('expires_at', $qb->createNamedParameter($now, IQueryBuilder::PARAM_INT))); + + $removed += $qb->executeStatement(); + } + + return $removed; + } + + /** + * @param list $fileIds + */ + public function removeByFileIds(array $fileIds): void { + foreach (array_chunk($fileIds, IQueryBuilder::MAX_IN_PARAMETERS) as $chunk) { + $qb = $this->connection->getQueryBuilder(); + $qb->delete(self::TABLE_LOCKS) + ->where($qb->expr()->in('file_id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))); + + $qb->executeStatement(); } + } + /** + * Remove the lock of a file if it has expired at $now. + * + * @return bool whether a row was removed + */ + public function removeExpired(int $fileId, int $now): bool { $qb = $this->connection->getQueryBuilder(); $qb->delete(self::TABLE_LOCKS) - ->where($qb->expr()->in('id', $qb->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY))); + ->where($qb->expr()->eq('file_id', $qb->createNamedParameter($fileId, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->isNotNull('expires_at')) + ->andWhere($qb->expr()->lte('expires_at', $qb->createNamedParameter($now, IQueryBuilder::PARAM_INT))); - $qb->executeStatement(); + return $qb->executeStatement() > 0; } /** @@ -100,9 +154,9 @@ public function removeIds(array $ids): void { */ public function getFromFileId(int $fileId): FileLock { $qb = $this->connection->getQueryBuilder(); - $qb->select('id', 'user_id', 'file_id', 'token', 'creation', 'type', 'ttl', 'owner') + $qb->select(...self::COLUMNS) ->from(self::TABLE_LOCKS) - ->where($qb->expr()->eq('file_id', $qb->createNamedParameter($fileId))); + ->where($qb->expr()->eq('file_id', $qb->createNamedParameter($fileId, IQueryBuilder::PARAM_INT))); return $this->getLockFromRequest($qb->executeQuery()); } @@ -111,11 +165,10 @@ public function getFromFileId(int $fileId): FileLock { * @param list $fileIds * * @return list - * @throws LockNotFoundException */ public function getFromFileIds(array $fileIds): array { $qb = $this->connection->getQueryBuilder(); - $qb->select('id', 'user_id', 'file_id', 'token', 'creation', 'type', 'ttl', 'owner') + $qb->select(...self::COLUMNS) ->from(self::TABLE_LOCKS) ->where($qb->expr()->in('file_id', $qb->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY))); @@ -127,26 +180,26 @@ public function getFromFileIds(array $fileIds): array { */ public function getAll(): array { $qb = $this->connection->getQueryBuilder(); - $qb->select('id', 'user_id', 'file_id', 'token', 'creation', 'type', 'ttl', 'owner') + $qb->select(...self::COLUMNS) ->from(self::TABLE_LOCKS); return $this->getLocksFromRequest($qb->executeQuery()); } /** - * @param int $timeout in minutes + * Locks whose expiry lies at or before $now. + * * @param int $limit how many locks to retrieve (0 for all, default) * * @return list * @throws Exception */ - public function getLocksOlderThan(int $timeout, int $limit = 0): array { - $now = Server::get(ITimeFactory::class)->getTime(); - $oldCreationTime = $now - $timeout * 60; + public function getExpired(int $now, int $limit = 0): array { $qb = $this->connection->getQueryBuilder(); - $qb->select('id', 'user_id', 'file_id', 'token', 'creation', 'type', 'ttl', 'owner') + $qb->select(...self::COLUMNS) ->from(self::TABLE_LOCKS) - ->andWhere($qb->expr()->lt('creation', $qb->createNamedParameter($oldCreationTime, IQueryBuilder::PARAM_INT))); + ->where($qb->expr()->isNotNull('expires_at')) + ->andWhere($qb->expr()->lte('expires_at', $qb->createNamedParameter($now, IQueryBuilder::PARAM_INT))); if ($limit !== 0) { $qb->setMaxResults($limit); @@ -155,11 +208,56 @@ public function getLocksOlderThan(int $timeout, int $limit = 0): array { return $this->getLocksFromRequest($qb->executeQuery()); } + /** + * Locks on files stored below a folder, resolved through the file cache so it + * works for every storage type. Each entry is the lock plus the path of the + * locked file relative to the folder. + * + * @return list + */ + public function getLocksBelow(int $folderId): array { + $qb = $this->connection->getQueryBuilder(); + $qb->select('storage', 'path') + ->from('filecache') + ->where($qb->expr()->eq('fileid', $qb->createNamedParameter($folderId, IQueryBuilder::PARAM_INT))); + $result = $qb->executeQuery(); + $folder = $result->fetch(); + $result->closeCursor(); + if ($folder === false) { + return []; + } + + $prefix = ($folder['path'] === null || $folder['path'] === '') ? '' : $folder['path'] . '/'; + + $qb = $this->connection->getQueryBuilder(); + $qb->select(...array_map(static fn (string $column): string => 'l.' . $column, self::COLUMNS)) + ->addSelect('f.path') + ->from(self::TABLE_LOCKS, 'l') + ->innerJoin('l', 'filecache', 'f', $qb->expr()->eq('l.file_id', 'f.fileid')) + ->where($qb->expr()->eq('f.storage', $qb->createNamedParameter((int)$folder['storage'], IQueryBuilder::PARAM_INT))); + if ($prefix !== '') { + $qb->andWhere($qb->expr()->like('f.path', $qb->createNamedParameter($this->connection->escapeLikeParameter($prefix) . '%'))); + } + + $locks = []; + $result = $qb->executeQuery(); + while ($row = $result->fetch()) { + $locks[] = [ + 'lock' => $this->parseLockSelectSql($row), + 'path' => substr((string)$row['path'], strlen($prefix)), + ]; + } + $result->closeCursor(); + + return $locks; + } + /** * @throws LockNotFoundException */ protected function getLockFromRequest(IResult $result): FileLock { $row = $result->fetch(); + $result->closeCursor(); if ($row === false) { throw new LockNotFoundException('Lock not found'); } @@ -175,11 +273,12 @@ public function getLocksFromRequest(IResult $result): array { while ($row = $result->fetch()) { $locks[] = $this->parseLockSelectSql($row); } + $result->closeCursor(); return $locks; } public function parseLockSelectSql(array $data): FileLock { - $lock = new FileLock($this->timeout); + $lock = new FileLock(); $lock->importFromDatabase($data); return $lock; diff --git a/lib/Exceptions/LockConflictException.php b/lib/Exceptions/LockConflictException.php new file mode 100644 index 00000000..e04c1808 --- /dev/null +++ b/lib/Exceptions/LockConflictException.php @@ -0,0 +1,19 @@ +addStorageWrapper( LockWrapper::class, fn (string $mountPoint, IStorage $storage): LockWrapper => new LockWrapper( [ 'storage' => $storage, 'lock_manager' => $this->lockManager, - 'user_session' => $this->userSession, - 'file_service' => $this->fileService, 'lock_service' => $this->lockService, ] - ), 10); + ), 0); } } diff --git a/lib/Listeners/NodeDeletedListener.php b/lib/Listeners/NodeDeletedListener.php new file mode 100644 index 00000000..45e1aa1c --- /dev/null +++ b/lib/Listeners/NodeDeletedListener.php @@ -0,0 +1,45 @@ + + */ +class NodeDeletedListener implements IEventListener { + public function __construct( + private readonly LockService $lockService, + ) { + } + + #[\Override] + public function handle(Event $event): void { + if ($event instanceof NodeDeletedEvent) { + $fileId = $event->getNode()->getId(); + } elseif ($event instanceof CacheEntryRemovedEvent) { + $fileId = $event->getFileId(); + } else { + return; + } + + if ($fileId !== null && $fileId > 0) { + $this->lockService->removeLocksForFileIds([(int)$fileId]); + } + } +} diff --git a/lib/Migration/Version36000Date20260906120000.php b/lib/Migration/Version36000Date20260906120000.php new file mode 100644 index 00000000..042c0bf7 --- /dev/null +++ b/lib/Migration/Version36000Date20260906120000.php @@ -0,0 +1,128 @@ +hasTable(LocksRequest::TABLE_LOCKS)) { + return; + } + if ($schema->getTable(LocksRequest::TABLE_LOCKS)->hasIndex(self::INDEX_FILE_ID)) { + return; + } + + $removed = $this->removeDuplicateLocks(); + if ($removed > 0) { + $output->info('Removed ' . $removed . ' superseded duplicate file lock rows'); + } + } + + #[\Override] + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + $table = $schema->getTable(LocksRequest::TABLE_LOCKS); + $changed = false; + + if (!$table->hasColumn('expires_at')) { + $table->addColumn('expires_at', Types::BIGINT, [ + 'notnull' => false, + 'default' => null, + ]); + $changed = true; + } + + // the unique index replacing it is created by the next migration step, + // some databases refuse a second index on the same column list + foreach ($table->getIndexes() as $index) { + if ($index->isSimpleIndex() && $index->spansColumns(['file_id'])) { + $table->dropIndex($index->getName()); + $changed = true; + } + } + + if (!$table->hasIndex(self::INDEX_EXPIRES)) { + $table->addIndex(['expires_at'], self::INDEX_EXPIRES); + $changed = true; + } + + return $changed ? $schema : null; + } + + #[\Override] + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + $qb = $this->connection->getQueryBuilder(); + $qb->update(LocksRequest::TABLE_LOCKS) + ->set('expires_at', $qb->func()->add('creation', 'ttl')) + ->where($qb->expr()->isNull('expires_at')) + ->andWhere($qb->expr()->gt('ttl', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))); + $qb->executeStatement(); + } + + /** + * Keep the row with the highest id for every file that has more than one lock. + * + * @return int number of rows removed + */ + public function removeDuplicateLocks(): int { + $qb = $this->connection->getQueryBuilder(); + $qb->select('file_id') + ->selectAlias($qb->func()->max('id'), 'keep_id') + ->selectAlias($qb->func()->count('id'), 'lock_count') + ->from(LocksRequest::TABLE_LOCKS) + ->groupBy('file_id') + ->having($qb->expr()->gt($qb->func()->count('id'), $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT))); + + $result = $qb->executeQuery(); + $duplicates = []; + while ($row = $result->fetch()) { + $duplicates[(int)$row['file_id']] = (int)$row['keep_id']; + } + $result->closeCursor(); + + $removed = 0; + foreach ($duplicates as $fileId => $keepId) { + $delete = $this->connection->getQueryBuilder(); + $delete->delete(LocksRequest::TABLE_LOCKS) + ->where($delete->expr()->eq('file_id', $delete->createNamedParameter($fileId, IQueryBuilder::PARAM_INT))) + ->andWhere($delete->expr()->neq('id', $delete->createNamedParameter($keepId, IQueryBuilder::PARAM_INT))); + $removed += $delete->executeStatement(); + } + + return $removed; + } +} diff --git a/lib/Migration/Version36000Date20260906120100.php b/lib/Migration/Version36000Date20260906120100.php new file mode 100644 index 00000000..31cf143c --- /dev/null +++ b/lib/Migration/Version36000Date20260906120100.php @@ -0,0 +1,35 @@ +getTable(LocksRequest::TABLE_LOCKS); + if ($table->hasIndex(Version36000Date20260906120000::INDEX_FILE_ID)) { + return null; + } + + $table->addUniqueIndex(['file_id'], Version36000Date20260906120000::INDEX_FILE_ID); + return $schema; + } +} diff --git a/lib/Model/FileLock.php b/lib/Model/FileLock.php index c9c7c16a..a0ede657 100644 --- a/lib/Model/FileLock.php +++ b/lib/Model/FileLock.php @@ -19,6 +19,9 @@ /** * Class FileLock * + * Expiry is modelled by a single absolute timestamp: null means the lock never + * expires, any other value is the unix time at which it stops being valid. + * * @package OCA\FilesLock\Service */ class FileLock implements ILock, JsonSerializable { @@ -36,22 +39,22 @@ class FileLock implements ILock, JsonSerializable { private int $creation = 0; + private ?int $expiresAt = null; + private int $lockType = ILock::TYPE_USER; private ?string $displayName = null; private int $scope = ILock::LOCK_EXCLUSIVE; - /** - * FileLock constructor. - */ - public function __construct( - private int $timeout = 1800, - ) { + public function __construct() { $this->creation = Server::get(ITimeFactory::class)->getTime(); } - public static function fromLockScope(LockContext $lockScope, int $timeout): FileLock { - $lock = new FileLock($timeout); + /** + * The lock never expires until a caller sets an expiry on it. + */ + public static function fromLockScope(LockContext $lockScope): FileLock { + $lock = new FileLock(); $lock->setUserId($lockScope->getOwner()); $lock->setLockType($lockScope->getType()); $lock->setFileId($lockScope->getNode()->getId()); @@ -111,23 +114,52 @@ public function setToken(string $token): self { return $this; } + /** + * Lifetime of the lock in seconds counted from its creation, ETA_INFINITE when it never expires. + */ #[\Override] public function getTimeout(): int { - return $this->timeout; + if ($this->expiresAt === null) { + return self::ETA_INFINITE; + } + return max(0, $this->expiresAt - $this->creation); } + /** + * @param int $timeout lifetime in seconds counted from creation, <= 0 for a lock that never expires + */ public function setTimeout(int $timeout): self { - $this->timeout = $timeout; + $this->expiresAt = $timeout > 0 ? $this->creation + $timeout : null; return $this; } + public function getExpiresAt(): ?int { + return $this->expiresAt; + } + + public function setExpiresAt(?int $expiresAt): self { + $this->expiresAt = $expiresAt; + + return $this; + } + + public function isInfinite(): bool { + return $this->expiresAt === null; + } + + public function isExpired(int $now): bool { + return $this->expiresAt !== null && $this->expiresAt <= $now; + } + + /** + * Seconds until the lock expires, 0 when it is already expired, ETA_INFINITE when it never expires. + */ public function getETA(): int { - if ($this->getTimeout() <= 0) { + if ($this->expiresAt === null) { return self::ETA_INFINITE; } - $end = $this->getCreatedAt() + $this->getTimeout(); - $eta = $end - Server::get(ITimeFactory::class)->getTime(); + $eta = $this->expiresAt - Server::get(ITimeFactory::class)->getTime(); return ($eta < 1) ? 0 : $eta; } @@ -181,10 +213,10 @@ public function toLockInfo(): LockInfo { $lock = new LockInfo(); $lock->owner = $this->getDisplayName(); $lock->token = $this->getToken(); - $lock->timeout = $this->getTimeout() <= 0 ? LockInfo::TIMEOUT_INFINITE : $this->getTimeout(); + $lock->timeout = $this->isInfinite() ? LockInfo::TIMEOUT_INFINITE : $this->getETA(); $lock->created = $this->getCreatedAt(); $lock->scope = LockInfo::EXCLUSIVE; - $lock->depth = 1; + $lock->depth = 0; $lock->uri = $this->getUri(); return $lock; @@ -197,14 +229,18 @@ public function importFromDatabase(array $data): self { $this->setToken($data['token'] ?? ''); $this->setCreation((int)$data['creation']); $this->setLockType((int)$data['type']); - $this->setTimeout((int)$data['ttl']); + $this->setExpiresAt(isset($data['expires_at']) ? (int)$data['expires_at'] : null); $this->setDisplayName($data['owner'] ?? ''); + // rows written before the scope was persisted on insert hold 0, which is + // not a valid scope; those locks are exclusive like every other one + $scope = (int)($data['scope'] ?? 0); + $this->setScope(in_array($scope, [ILock::LOCK_EXCLUSIVE, ILock::LOCK_SHARED], true) ? $scope : ILock::LOCK_EXCLUSIVE); return $this; } /** - * Import the shape produced by jsonSerialize() (also accepts database column names). + * Import a lock from the properties a remote DAV storage reports. */ public function import(array $data): void { $this->setId((int)($data['id'] ?? 0)); @@ -229,6 +265,7 @@ public function jsonSerialize(): array { 'token' => $this->getToken(), 'eta' => $this->getETA(), 'creation' => $this->getCreatedAt(), + 'expiresAt' => $this->getExpiresAt(), 'type' => $this->getType(), ]; } diff --git a/lib/Service/FileService.php b/lib/Service/FileService.php index b0217219..9610cde3 100644 --- a/lib/Service/FileService.php +++ b/lib/Service/FileService.php @@ -12,9 +12,6 @@ use OCP\Files\IRootFolder; use OCP\Files\Node; use OCP\Files\NotFoundException; -use OCP\Files\NotPermittedException; -use OCP\IUserSession; -use OCP\Session\Exceptions\SessionNotAvailableException; /** * Class FileService @@ -23,78 +20,21 @@ */ class FileService { public function __construct( - private readonly IUserSession $userSession, private readonly IRootFolder $rootFolder, ) { } /** + * Resolve a file id from the point of view of a user. * * @throws NotFoundException */ public function getFileFromId(string $userId, int $fileId): Node { - $files = $this->rootFolder->getUserFolder($userId) - ->getById($fileId); - - if (sizeof($files) === 0) { - throw new NotFoundException(); - } - - return array_shift($files); - } - - /** - * - * @throws NotFoundException - */ - public function getFileFromPath(string $userId, string $path): Node { - if (!str_starts_with($path, 'files/')) { - throw new NotFoundException(); - } - - $path = '/' . substr($path, 6); - - return $this->rootFolder->getUserFolder($userId) - ->get($path); - } - - /** - * @throws NotFoundException - */ - public function getFileFromUri(string $uri): Node { - $user = $this->userSession->getUser(); - if (is_null($user)) { - throw new SessionNotAvailableException(); - } - - $userId = $user->getUID(); - - $path = '/' . $uri; - - return $this->rootFolder->getUserFolder($userId) - ->get($path); - } - - /** - * - * @throws NotFoundException - * @throws NotPermittedException - */ - public function getFileFromAbsoluteUri(string $uri): Node { - $user = $this->userSession->getUser(); - if ($user === null) { - throw new SessionNotAvailableException(); - } - - $userId = $user->getUID(); - - [$root, , $path] = explode('/', trim($uri, '/') . '/', 3); - if ($root !== 'files') { + $node = $this->rootFolder->getUserFolder($userId)->getFirstNodeById($fileId); + if ($node === null) { throw new NotFoundException(); } - $path = '/' . $path; - return $this->rootFolder->getUserFolder($userId) - ->get($path); + return $node; } } diff --git a/lib/Service/LockPolicy.php b/lib/Service/LockPolicy.php new file mode 100644 index 00000000..94c6f66e --- /dev/null +++ b/lib/Service/LockPolicy.php @@ -0,0 +1,99 @@ +getType() !== $lock->getType()) { + return false; + } + if ($context->getOwner() === $lock->getOwner()) { + return true; + } + return $lock->getType() === ILock::TYPE_TOKEN && $context->getOwner() === $lock->getToken(); + } + + /** + * @param string|null $viewerId user performing the write, null for a sessionless caller + * @param list $presentedTokens lock tokens presented with the request + * @param LockContext|null $scope active ILockManager scope + */ + public function canWrite(FileLock $lock, ?string $viewerId, array $presentedTokens, ?LockContext $scope): bool { + switch ($lock->getType()) { + case ILock::TYPE_USER: + return $viewerId !== null && $viewerId === $lock->getOwner(); + case ILock::TYPE_APP: + return $scope !== null + && $scope->getType() === ILock::TYPE_APP + && $scope->getOwner() === $lock->getOwner(); + case ILock::TYPE_TOKEN: + $tokenPresented = in_array($lock->getToken(), $presentedTokens, true) + || ($scope !== null && $scope->getType() === ILock::TYPE_TOKEN && $scope->getOwner() === $lock->getToken()); + if (!$tokenPresented) { + return false; + } + return $viewerId === null || $viewerId === $lock->getOwner(); + } + return false; + } + + /** + * @param LockContext $context asserted identity of the caller (owner string is a + * user id, an app id, or a lock token) + * @param string|null $token lock token presented separately from the context + * @param bool $isFileOwner whether the caller owns the file (see LockService::isFileOwner) + * @param bool $canModify whether the caller may write the file + */ + public function canUnlock(FileLock $lock, LockContext $context, ?string $token, bool $isFileOwner, bool $force, bool $canModify = true): bool { + if ($force || $isFileOwner) { + return true; + } + // whoever the lock was recorded for releases it, even if their access to + // the file was reduced while they held it + if ($context->getOwner() === $lock->getOwner()) { + return true; + } + if ($lock->getType() === ILock::TYPE_TOKEN) { + // the token is a public credential (RFC 4918 section 6.5), so possession + // alone cannot be the whole authorization: RFC 4918 section 6.4 requires + // privileges to be enforced by the normal mechanisms rather than by the + // obscurity of the token. Someone who may not write the file could never + // have taken the lock, so they may not release it either. + return $canModify && ($token === $lock->getToken() || $context->getOwner() === $lock->getToken()); + } + return false; + } +} diff --git a/lib/Service/LockService.php b/lib/Service/LockService.php index cbc643c6..7d3ef5b0 100644 --- a/lib/Service/LockService.php +++ b/lib/Service/LockService.php @@ -15,13 +15,17 @@ use OCA\FilesLock\AppInfo\Application; use OCA\FilesLock\ConfigLexicon; use OCA\FilesLock\Db\LocksRequest; +use OCA\FilesLock\Exceptions\LockConflictException; use OCA\FilesLock\Exceptions\LockNotFoundException; +use OCA\FilesLock\Exceptions\NotFileException; use OCA\FilesLock\Exceptions\UnauthorizedUnlockException; use OCA\FilesLock\Model\FileLock; use OCP\App\IAppManager; use OCP\AppFramework\Services\IAppConfig; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\Constants; -use OCP\EventDispatcher\IEventDispatcher; +use OCP\Files\File; +use OCP\Files\IHomeStorage; use OCP\Files\InvalidPathException; use OCP\Files\IRootFolder; use OCP\Files\Lock\ILock; @@ -33,13 +37,17 @@ use OCP\IRequest; use OCP\IUserManager; use OCP\IUserSession; +use OCP\Server; use Psr\Log\LoggerInterface; class LockService { public const PREFIX = 'files_lock'; + /** @var array */ private array $lockCache = []; + /** @var array */ private array $remoteLockCache = []; - private bool $allowUserOverride = false; + /** @var list */ + private array $presentedTokens = []; public function __construct( private readonly IL10N $l10n, @@ -48,40 +56,77 @@ public function __construct( private readonly FileService $fileService, private readonly IAppConfig $appConfig, private readonly IAppManager $appManager, - IEventDispatcher $eventDispatcher, private readonly IUserSession $userSession, private readonly IRequest $request, private readonly LoggerInterface $logger, private readonly IRootFolder $rootFolder, + private readonly LockPolicy $policy, ) { } + /** + * Resolved lazily so that a clock replaced in the container after this + * service was built (tests) is honoured everywhere. + */ + private function now(): int { + return Server::get(ITimeFactory::class)->getTime(); + } + + /** + * Register a lock token presented by the current request (WebDAV If header). + */ + public function presentToken(string $token): void { + if ($token !== '' && !in_array($token, $this->presentedTokens, true)) { + $this->presentedTokens[] = $token; + } + } + + public function resetPresentedTokens(): void { + $this->presentedTokens = []; + } + public function clearCache(): void { $this->lockCache = []; $this->remoteLockCache = []; + $this->presentedTokens = []; } - public function getLockForNodeId(int $nodeId, ?Node $node = null): FileLock|false { - if (array_key_exists($nodeId, $this->lockCache) && $this->lockCache[$nodeId] !== false) { - return $this->lockCache[$nodeId]; + /** + * Active local lock of a file, using the per-request cache. Never consults remote storages. + */ + public function getActiveLock(int $fileId): ?FileLock { + if (array_key_exists($fileId, $this->lockCache)) { + $cached = $this->lockCache[$fileId]; + $now = $this->now(); + if ($cached instanceof FileLock && $cached->isExpired($now)) { + $this->locksRequest->removeExpired($fileId, $now); + $this->lockCache[$fileId] = false; + return null; + } + return $cached ?: null; } - if (array_key_exists($nodeId, $this->remoteLockCache)) { - return $this->remoteLockCache[$nodeId]; + try { + return $this->getLockFromFileId($fileId); + } catch (LockNotFoundException) { + return null; } + } - if (!array_key_exists($nodeId, $this->lockCache)) { - try { - $this->lockCache[$nodeId] = $this->getLockFromFileId($nodeId); - return $this->lockCache[$nodeId]; - } catch (LockNotFoundException) { - $this->lockCache[$nodeId] = false; - } + /** + * Lock of a file, falling back to the lock a remote DAV storage reports. + */ + public function getLockForNodeId(int $nodeId, ?Node $node = null): ?FileLock { + $lock = $this->getActiveLock($nodeId); + if ($lock !== null) { + return $lock; + } + + if (!array_key_exists($nodeId, $this->remoteLockCache)) { + $this->remoteLockCache[$nodeId] = $this->getRemoteLockFromDav($nodeId, $node) ?: false; } - $remoteLock = $this->getRemoteLockFromDav($nodeId, $node); - $this->remoteLockCache[$nodeId] = $remoteLock ?: false; - return $this->remoteLockCache[$nodeId]; + return $this->remoteLockCache[$nodeId] ?: null; } /** @@ -105,9 +150,10 @@ public function getLockForNodeIds(array $nodeIds): array { return $locks; } - // pre-fill the cache with negative hits for all requested ids - // so if no lock is found for the file we store the negative hit + // pre-fill with negative hits for all requested ids, so that a file with no + // lock is reported as such instead of being left out of the result foreach ($locksToRequest as $fileId) { + $locks[$fileId] = false; $this->lockCache[$fileId] = false; } @@ -117,9 +163,10 @@ public function getLockForNodeIds(array $nodeIds): array { } $newLocks = array_merge(...$newLocks); + $now = $this->now(); $expiredLocks = []; foreach ($newLocks as $lock) { - if ($lock->getETA() === 0) { + if ($lock->isExpired($now)) { $expiredLocks[] = $lock->getId(); $locks[$lock->getFileId()] = false; $this->lockCache[$lock->getFileId()] = false; @@ -130,46 +177,128 @@ public function getLockForNodeIds(array $nodeIds): array { } if (count($expiredLocks) > 0) { - $this->locksRequest->removeIds($expiredLocks); + $this->locksRequest->removeExpiredIds($expiredLocks, $now); } return $locks; } + /** + * Configured lock lifetime in seconds, ETA_INFINITE when locks never expire. + */ + private function getConfiguredTimeout(): int { + $minutes = $this->appConfig->getAppValueInt(ConfigLexicon::LOCK_TIMEOUT); + return $minutes > 0 ? $minutes * 60 : FileLock::ETA_INFINITE; + } + public function lock(LockContext $lockScope): FileLock { - $this->canLock($lockScope); - $timeout = $this->appConfig->getAppValueInt(ConfigLexicon::LOCK_TIMEOUT) * 60; + return $this->acquire($lockScope); + } + + /** + * Acquire or refresh the lock described by $lockScope. + * + * The database enforces at most one lock per file; a lost race is reported as + * OwnerLockedException carrying the winning lock. + * + * @param int|null $timeout lifetime in seconds, <= 0 for no expiry, null for the configured value + * @param string|null $token token to record for a new lock (native WebDAV), generated when null + * @param string|null $displayName display name to record, resolved from the owner when null + * @param bool $filesOnly refuse folders; native WebDAV may lock a collection (RFC 4918) + * + * @throws OwnerLockedException + * @throws UnauthorizedUnlockException + * @throws NotFileException + */ + public function acquire(LockContext $lockScope, ?int $timeout = null, ?string $token = null, ?string $displayName = null, bool $filesOnly = true): FileLock { + $this->canLock($lockScope, $filesOnly); + $fileId = $lockScope->getNode()->getId(); + $timeout ??= $this->getConfiguredTimeout(); + $now = $this->now(); + + unset($this->lockCache[$fileId]); try { - $known = $this->getLockFromFileId($lockScope->getNode()->getId()); - - // Extend lock expiry if matching - if ( - $known->getType() === $lockScope->getType() && ($known->getOwner() === $lockScope->getOwner() || $known->getToken() === $lockScope->getOwner()) - ) { - $known->setTimeout($known->getETA() !== FileLock::ETA_INFINITE ? $known->getTimeout() - $known->getETA() + $timeout : 0); - $this->logger->notice('extending existing lock', ['fileLock' => $known]); - $this->locksRequest->update($known); - $this->lockCache[$lockScope->getNode()->getId()] = $known; - $this->injectMetadata($known); - return $known; + $known = $this->locksRequest->getFromFileId($fileId); + if (!$known->isExpired($now)) { + return $this->refreshOrConflict($known, $lockScope, $timeout, $now); } - - $this->injectMetadata($known); - throw new OwnerLockedException($known); + // the delete keeps its own expiry condition, so a refresh that lands in + // between is not dropped + $this->locksRequest->removeExpired($fileId, $now); } catch (LockNotFoundException) { - $lock = FileLock::fromLockScope($lockScope, $timeout); - $this->generateToken($lock); - $this->logger->notice('locking file', ['fileLock' => $lock]); + } + + $lock = FileLock::fromLockScope($lockScope); + $lock->setCreation($now); + $lock->setExpiresAt($timeout > 0 ? $now + $timeout : null); + $lock->setToken($token ?? $this->newToken()); + if ($displayName !== null) { + $lock->setDisplayName($displayName); + } else { $this->injectMetadata($lock); + } + + try { $this->locksRequest->save($lock); - $this->propagateEtag($lockScope); - return $lock; + } catch (LockConflictException) { + $known = $this->storeAfterConflict($lock); + if ($known !== null) { + return $this->refreshOrConflict($known, $lockScope, $timeout, $now); + } } + + $this->logger->notice('locking file', ['fileLock' => $lock]); + $this->lockCache[$fileId] = $lock; + $this->propagateEtag($lockScope->getNode()); + return $lock; } - public function update(FileLock $lock): void { - $this->locksRequest->update($lock); + /** + * Recover from an insert the database refused. + * + * A row for this file means another request won the race. Otherwise the index + * that fired was the one on the token, and a fresh token gets one more + * attempt; losing that one as well means a competing insert landed in + * between, which is a conflict on the file like any other. + * + * @return FileLock|null the lock that won the file, or null once $lock is stored + */ + private function storeAfterConflict(FileLock $lock): ?FileLock { + try { + return $this->locksRequest->getFromFileId($lock->getFileId()); + } catch (LockNotFoundException) { + } + + $lock->setToken($this->newToken()); + try { + $this->locksRequest->save($lock); + } catch (LockConflictException) { + return $this->locksRequest->getFromFileId($lock->getFileId()); + } + + return null; + } + + /** + * @throws OwnerLockedException + */ + private function refreshOrConflict(FileLock $known, LockContext $lockScope, int $timeout, int $now): FileLock { + $this->injectMetadata($known); + if (!$this->policy->isHolder($known, $lockScope)) { + $this->lockCache[$known->getFileId()] = $known; + throw new OwnerLockedException($known); + } + + $known->setExpiresAt($timeout > 0 ? $now + $timeout : null); + $this->logger->notice('extending existing lock', ['fileLock' => $known]); + $this->locksRequest->update($known); + $this->lockCache[$known->getFileId()] = $known; + return $known; + } + + private function newToken(): string { + return self::PREFIX . '/' . uuid_create(UUID_TYPE_RANDOM); } public function getAppName(string $appId): ?string { @@ -179,157 +308,218 @@ public function getAppName(string $appId): ?string { } /** + * Release the lock on the node of $lock. + * + * @param string|null $token lock token presented with the request + * * @throws InvalidPathException * @throws LockNotFoundException * @throws NotFoundException * @throws UnauthorizedUnlockException */ - public function unlock(LockContext $lock, bool $force = false): FileLock { + public function unlock(LockContext $lock, bool $force = false, ?string $token = null): FileLock { $this->logger->notice('unlocking file', ['fileLock' => $lock]); $known = $this->getLockFromFileId($lock->getNode()->getId()); - if (!$force) { - $this->canUnlock($lock, $known); + if (!$this->policy->canUnlock($known, $lock, $token, $this->isFileOwner($lock->getNode()), $force, $this->canModify($lock->getNode()))) { + $this->injectMetadata($known); + throw new UnauthorizedUnlockException( + $known->getType() === ILock::TYPE_TOKEN + ? $this->l10n->t('File can only be unlocked by providing a valid owner lock token') + : $this->l10n->t('File can only be unlocked by the owner of the lock') + ); } $this->locksRequest->delete($known); $this->lockCache[$lock->getNode()->getId()] = false; - $this->propagateEtag($lock); + $this->propagateEtag($lock->getNode()); $this->injectMetadata($known); return $known; } - public function enableUserOverride(): void { - $this->allowUserOverride = true; - } - - public function canLock(LockContext $request, ?FileLock $current = null): void { - if (($request->getNode()->getPermissions() & Constants::PERMISSION_UPDATE) === 0) { + /** + * @throws UnauthorizedUnlockException when the node cannot be locked by the caller + * @throws NotFileException when the node is not a file + */ + public function canLock(LockContext $request, bool $filesOnly = true): void { + if ($filesOnly && !$request->getNode() instanceof File) { + throw new NotFileException($this->l10n->t('Only files can be locked.')); + } + if (!$this->canModify($request->getNode())) { throw new UnauthorizedUnlockException( $this->l10n->t('File can only be locked with update permissions.') ); } } - public function canUnlock(LockContext $request, FileLock $current): void { - $isSameUser = $current->getOwner() === $this->userSession->getUser()?->getUID(); - $isSameToken = $request->getOwner() === $current->getToken(); - $isSameOwner = $request->getOwner() === $current->getOwner(); - $isSameType = $request->getType() === $current->getType(); - - // we need to ignore some filesystem that return current user as file owner - $ignoreFileOwnership = [ - 'OCA\GroupFolders\Mount\MountProvider', - 'OCA\Files_External\Config\ConfigAdapter' - ]; - - $isFileOwner = $request->getNode()->getOwner()->getUID() === $this->userSession->getUser()?->getUID() - && !in_array($request->getNode()->getMountPoint()->getMountProvider(), $ignoreFileOwnership); + /** + * Whether the current user may write $lock's file. + * + * @param LockContext|null $scope active ILockManager scope of the caller + */ + public function canWrite(FileLock $lock, ?LockContext $scope): bool { + return $this->policy->canWrite($lock, $this->userSession->getUser()?->getUID(), $this->presentedTokens, $scope); + } - // Check the token for token based locks - if ($current->getType() === ILock::TYPE_TOKEN) { - // token holder can unlock - if ($isSameToken) { - return; - } - // file owner or lock owner can unlock - if ($this->allowUserOverride && ($isSameUser || $isFileOwner)) { - return; - } - throw new UnauthorizedUnlockException( - $this->l10n->t('File can only be unlocked by providing a valid owner lock token') - ); + /** + * Whether the current caller may write the node at all, independently of any lock. + */ + public function canModify(Node $node): bool { + try { + return ($node->getPermissions() & Constants::PERMISSION_UPDATE) !== 0; + } catch (Exception) { + return false; } + } - // Otherwise, we check if the owner (user id OR app id) for a match - if ($isSameOwner && $isSameType) { - return; + /** + * The file owner override applies only to files stored in a user's own home + * storage (directly or through a share of it). Group folders, external + * storages and other mounts report the current user as owner of every file, + * so they never grant the override. + */ + public function isFileOwner(Node $node): bool { + $user = $this->userSession->getUser(); + if ($user === null) { + return false; } - - if ($request->getType() === ILock::TYPE_USER && $isFileOwner) { - return; + try { + if (!$node->getStorage()->instanceOfStorage(IHomeStorage::class)) { + return false; + } + return $node->getOwner()?->getUID() === $user->getUID(); + } catch (Exception) { + return false; } - - throw new UnauthorizedUnlockException( - $this->l10n->t('File can only be unlocked by the owner of the lock') - ); } /** + * Release the lock on a file. With $force the lock row is removed without + * resolving the file through anyone's file system. + * * @throws InvalidPathException * @throws LockNotFoundException * @throws NotFoundException * @throws UnauthorizedUnlockException */ - public function unlockFile(int $fileId, ?string $userId, bool $force = false, int $lockType = ILock::TYPE_USER): FileLock { - $lock = $this->getLockForNodeId($fileId); - if (!$lock) { - throw new LockNotFoundException(); - } - + public function unlockFile(int $fileId, string $userId, bool $force = false, int $lockType = ILock::TYPE_USER): FileLock { if ($force) { - $userId = in_array($lock->getType(), [ILock::TYPE_USER, ILock::TYPE_TOKEN]) ? $lock->getOwner() : $userId; - $lockType = $lock->getType(); + return $this->forceUnlock($fileId); } $node = $this->fileService->getFileFromId($userId, $fileId); - $lock = new LockContext( - $node, - $lockType, - $userId, - ); - $this->propagateEtag($lock); - return $this->unlock($lock, $force); + return $this->unlock(new LockContext($node, $lockType, $userId)); } /** - * @param int $limit how many locks to retrieve (0 for all, default) + * Administrative removal of a lock by file id. * - * @return FileLock[] + * @throws LockNotFoundException */ - public function getDeprecatedLocks(int $limit = 0): array { - $timeout = $this->appConfig->getAppValueInt(ConfigLexicon::LOCK_TIMEOUT); - if ($timeout === FileLock::ETA_INFINITE) { - return []; + public function forceUnlock(int $fileId): FileLock { + $known = $this->getLockFromFileId($fileId); + $this->logger->notice('force unlocking file', ['fileLock' => $known]); + $this->locksRequest->delete($known); + $this->lockCache[$fileId] = false; + + $node = null; + try { + if ($known->getType() !== ILock::TYPE_APP && $this->userManager->userExists($known->getOwner())) { + $node = $this->rootFolder->getUserFolder($known->getOwner())->getFirstNodeById($fileId); + } + $node ??= $this->rootFolder->getFirstNodeById($fileId); + } catch (Exception) { + } + if ($node !== null) { + $this->propagateEtag($node); + } + + $this->injectMetadata($known); + return $known; + } + + /** + * Remove every lock of the given files, regardless of ownership. + * + * @param list $fileIds + */ + public function removeLocksForFileIds(array $fileIds): void { + if (empty($fileIds)) { + return; + } + $this->locksRequest->removeByFileIds($fileIds); + foreach ($fileIds as $fileId) { + $this->lockCache[$fileId] = false; } + } + /** + * Locks whose expiry has passed. + * + * @param int $limit how many locks to retrieve (0 for all, default) + * + * @return FileLock[] + */ + public function getExpiredLocks(int $limit = 0): array { try { - $locks = $this->locksRequest->getLocksOlderThan($timeout, $limit); + return $this->locksRequest->getExpired($this->now(), $limit); } catch (Exception $e) { - $this->logger->warning('Failed to get locks older then timeout', ['exception' => $e]); + $this->logger->warning('Failed to get expired locks', ['exception' => $e]); return []; } - - return $locks; } /** + * Active lock of a file, removing it first when it has expired. + * * @throws LockNotFoundException */ public function getLockFromFileId(int $fileId): FileLock { - $lock = $this->locksRequest->getFromFileId($fileId); - if ($lock->getETA() === 0) { + try { + $lock = $this->locksRequest->getFromFileId($fileId); + } catch (LockNotFoundException $e) { + $this->lockCache[$fileId] = false; + throw $e; + } + if ($lock->isExpired($this->now())) { $this->locksRequest->delete($lock); + $this->lockCache[$fileId] = false; throw new LockNotFoundException('lock is ignored and deleted as being too old.'); } + $this->lockCache[$fileId] = $lock; return $lock; } + /** + * /** + * Active locks on files below a folder (relative path included). + * + * @return list + */ + public function getLocksBelow(int $folderId): array { + $now = $this->now(); + return array_values(array_filter( + $this->locksRequest->getLocksBelow($folderId), + fn (array $entry): bool => !$entry['lock']->isExpired($now) + )); + } + public function injectMetadata(FileLock $lock): FileLock { $displayName = null; if ($lock->getType() === ILock::TYPE_USER) { $displayName = $this->userManager->getDisplayName($lock->getOwner()); } if ($lock->getType() === ILock::TYPE_APP) { - $displayName = $this->getAppName($lock->getOwner()) ?? null; + $displayName = $this->getAppName($lock->getOwner()); } if ($lock->getType() === ILock::TYPE_TOKEN) { - $clientHint = $this->getClientHint(); - $displayName = $lock->getDisplayName() ?: ( - $this->userManager->getDisplayName($lock->getOwner()) . ' ' - . ($clientHint ? ('(' . $clientHint . ')') : '') - ); + $displayName = $lock->getDisplayName(); + if ($displayName === null || $displayName === '') { + $clientHint = $this->getClientHint(); + $displayName = trim(($this->userManager->getDisplayName($lock->getOwner()) ?? $lock->getOwner()) + . ($clientHint ? (' (' . $clientHint . ')') : '')); + } } if ($displayName) { @@ -354,15 +544,31 @@ private function getClientHint(): ?string { return null; } - public function generateToken(FileLock $lock): void { - if ($lock->getToken() !== '') { + /** + * Remove the given locks, skipping any that are no longer expired because + * their owner refreshed them after the batch was read. + * + * @param FileLock[] $locks + */ + public function removeLocksIfExpired(array $locks): void { + if (empty($locks)) { return; } - $lock->setToken(self::PREFIX . '/' . uuid_create(UUID_TYPE_RANDOM)); + $ids = array_map(fn (FileLock $lock): int => $lock->getId(), $locks); + $removed = $this->locksRequest->removeExpiredIds($ids, $this->now()); + $this->logger->notice('removing expired locks', ['candidates' => count($ids), 'removed' => $removed]); + + foreach ($locks as $lock) { + unset($this->lockCache[$lock->getFileId()]); + } } /** + * Remove the given locks unconditionally. The cleanup job wants + * removeLocksIfExpired() instead, which will not drop a lock that was + * refreshed after the batch was read. + * * @param FileLock[] $locks */ public function removeLocks(array $locks): void { @@ -377,6 +583,9 @@ public function removeLocks(array $locks): void { $this->logger->notice('removing locks', ['ids' => $ids]); $this->locksRequest->removeIds($ids); + foreach ($locks as $lock) { + $this->lockCache[$lock->getFileId()] = false; + } } public function getRemoteLockFromDav(int $nodeId, ?Node $node = null): ?FileLock { @@ -390,7 +599,7 @@ public function getRemoteLockFromDav(int $nodeId, ?Node $node = null): ?FileLock $userFolder = $this->rootFolder->getUserFolder($user->getUID()); $node = $userFolder->getFirstNodeById($nodeId); } - if (empty($node)) { + if ($node === null) { return null; } @@ -404,13 +613,13 @@ public function getRemoteLockFromDav(int $nodeId, ?Node $node = null): ?FileLock return null; } - $path = $node->getInternalPath(); - $storage->getMetaData($path); - if (!method_exists($storage, 'getPropfindPropertyValue')) { return null; } + $path = $node->getInternalPath(); + $storage->getMetaData($path); + $isLocked = $storage->getPropfindPropertyValue($path, Application::DAV_PROPERTY_LOCK); if (!$isLocked) { return null; @@ -436,11 +645,14 @@ public function getRemoteLockFromDav(int $nodeId, ?Node $node = null): ?FileLock } } - private function propagateEtag(LockContext $lockContext): void { - $node = $lockContext->getNode(); - $node->getStorage()->getCache()->update($node->getId(), [ - 'etag' => uniqid(), - ]); - $node->getStorage()->getUpdater()->propagate($node->getInternalPath(), $node->getMTime()); + private function propagateEtag(Node $node): void { + try { + $node->getStorage()->getCache()->update($node->getId(), [ + 'etag' => uniqid(), + ]); + $node->getStorage()->getUpdater()->propagate($node->getInternalPath(), $node->getMTime()); + } catch (Exception $e) { + $this->logger->debug('Failed to propagate etag after lock change: ' . $e->getMessage(), ['exception' => $e]); + } } } diff --git a/lib/Storage/LockWrapper.php b/lib/Storage/LockWrapper.php index ba9dc283..03594141 100644 --- a/lib/Storage/LockWrapper.php +++ b/lib/Storage/LockWrapper.php @@ -8,113 +8,100 @@ namespace OCA\FilesLock\Storage; use OC\Files\Storage\Wrapper\Wrapper; -use OCA\FilesLock\Exceptions\LockNotFoundException; use OCA\FilesLock\Model\FileLock; -use OCA\FilesLock\Service\FileService; use OCA\FilesLock\Service\LockService; use OCP\Constants; -use OCP\Files\InvalidPathException; -use OCP\Files\Lock\ILock; use OCP\Files\Lock\ILockManager; -use OCP\Files\Lock\NoLockProviderException; -use OCP\Files\NotFoundException; use OCP\Files\Storage\IStorage; -use OCP\IUserSession; use OCP\Lock\LockedException; use OCP\Lock\ManuallyLockedException; +/** + * Enforces file locks for every write that reaches a storage, whatever mount the + * storage is attached to. Files are identified through the wrapped storage's own + * cache, so the check does not depend on the shape of the storage path. + */ class LockWrapper extends Wrapper { private readonly ILockManager $lockManager; + private readonly LockService $lockService; - /** @var FileService */ - private $fileService; - - /** @var LockService */ - private $lockService; - - /** @var IUserSession */ - private $userSession; - - /** - * LockWrapper constructor. - * - * @param $arguments - */ public function __construct(array $arguments) { parent::__construct($arguments); $this->lockManager = $arguments['lock_manager']; - $this->userSession = $arguments['user_session']; - $this->fileService = $arguments['file_service']; $this->lockService = $arguments['lock_service']; } /** - * @param $path - * @param $permissions - * * @throws LockedException */ - protected function checkPermissions($path, $permissions): bool { - $viewerId = ''; - $user = $this->userSession->getUser(); - if ($user !== null) { - $viewerId = $user->getUID(); - $ownerId = $viewerId; - } else { - $ownerId = $this->getOwner($path); + protected function checkPermissions(string $path, int $permissions): bool { + if ($permissions === Constants::PERMISSION_READ) { + return true; } - /** @var FileLock $lock */ - if (!$this->isPathLocked($ownerId, $path, $viewerId, $lock)) { + $fileId = $this->getCache()->getId($path); + if ($fileId === -1) { return true; } - switch ($permissions) { - case Constants::PERMISSION_READ: - return true; - case Constants::PERMISSION_DELETE: - case Constants::PERMISSION_UPDATE: - throw new ManuallyLockedException( - $path, null, $lock->getToken(), $lock->getOwner(), $lock->getETA() - ); - - default: - return false; + $lock = $this->lockService->getActiveLock($fileId); + if ($lock === null || $this->lockService->canWrite($lock, $this->lockManager->getLockInScope())) { + return true; } + + throw new ManuallyLockedException( + $path, null, $lock->getToken(), $lock->getOwner(), $lock->getETA() + ); } - protected function isPathLocked(string $ownerId, string $path, string $viewerId, ?FileLock &$lock = null): bool { - try { - $file = $this->fileService->getFileFromPath($ownerId, $path); - } catch (NotFoundException) { - return false; + /** + * Refuse an operation on a directory that would delete or relocate a locked + * descendant the current user may not write. + * + * @return list the active locks below $path + * @throws LockedException + */ + protected function checkDescendants(IStorage $storage, string $path): array { + if (!$storage->is_dir($path)) { + return []; + } + $folderId = $storage->getCache()->getId($path); + if ($folderId === -1) { + return []; } - if ($file->getId() === null) { - return false; + $below = $this->lockService->getLocksBelow($folderId); + $scope = $this->lockManager->getLockInScope(); + foreach ($below as $entry) { + $lock = $entry['lock']; + if (!$this->lockService->canWrite($lock, $scope)) { + throw new ManuallyLockedException( + rtrim($path, '/') . '/' . $entry['path'], null, $lock->getToken(), $lock->getOwner(), $lock->getETA() + ); + } } - return $this->isFileLocked($file->getId(), $viewerId, $lock); + return $below; } - protected function isFileLocked(int $fileId, string $viewerId, ?FileLock &$lock = null): bool { - try { - $lock = $this->lockService->getLockFromFileId($fileId); - if ($lock->getType() === ILock::TYPE_USER && $lock->getOwner() !== $viewerId) { - return true; - } + /** + * Refuse an operation that would take a locked file out of its source storage. + * + * @throws LockedException + */ + protected function checkSourceLock(IStorage $sourceStorage, string $path): void { + $fileId = $sourceStorage->getCache()->getId($path); + if ($fileId <= 0) { + return; + } - if ($lock->getType() === ILock::TYPE_APP) { - $lockScope = $this->lockManager->getLockInScope(); - if (!$lockScope || $lockScope->getType() !== $lock->getType() || $lockScope->getOwner() !== $lock->getOwner()) { - return true; - } - } - } catch (NoLockProviderException|LockNotFoundException|InvalidPathException|NotFoundException) { + $lock = $this->lockService->getActiveLock($fileId); + if ($lock === null || $this->lockService->canWrite($lock, $this->lockManager->getLockInScope())) { + return; } - return false; + throw new ManuallyLockedException($path, null, $lock->getToken(), $lock->getOwner(), $lock->getETA()); } #[\Override] @@ -123,19 +110,16 @@ public function rename(string $source, string $target): bool { $part = substr($source, strlen($target)); //This is a rename of the transfer file to the original file if (str_starts_with($part, '.ocTransferId')) { - return $this->checkPermissions($target, Constants::PERMISSION_CREATE) + return $this->checkPermissions($target, Constants::PERMISSION_UPDATE) && parent::rename($source, $target); } } $permissions = $this->file_exists($target) ? Constants::PERMISSION_UPDATE : Constants::PERMISSION_CREATE; - $sourceParent = dirname($source); - if ($sourceParent === '.') { - $sourceParent = ''; - } - return $this->checkPermissions($sourceParent, Constants::PERMISSION_DELETE) - && $this->checkPermissions($source, Constants::PERMISSION_UPDATE & Constants::PERMISSION_READ) + $this->checkDescendants($this, $source); + + return $this->checkPermissions($source, Constants::PERMISSION_UPDATE) && $this->checkPermissions($target, $permissions) && parent::rename($source, $target); } @@ -153,17 +137,21 @@ public function copy(string $source, string $target): bool { #[\Override] public function copyFromStorage(IStorage $sourceStorage, string $sourceInternalPath, string $targetInternalPath): bool { - $cache = $sourceStorage->getCache(); - $fileId = $cache->getId($sourceInternalPath); - - $user = $this->userSession->getUser(); - if ($fileId > 0 && $this->isFileLocked($fileId, $user?->getUID() ?? '', $lock)) { - throw new ManuallyLockedException($sourceInternalPath, null, $lock->getToken(), $lock->getOwner(), $lock->getETA()); - } + $this->checkSourceLock($sourceStorage, $sourceInternalPath); return parent::copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); } + #[\Override] + public function moveFromStorage(IStorage $sourceStorage, string $sourceInternalPath, string $targetInternalPath): bool { + $this->checkDescendants($sourceStorage, $sourceInternalPath); + $this->checkSourceLock($sourceStorage, $sourceInternalPath); + $permissions = $this->file_exists($targetInternalPath) ? Constants::PERMISSION_UPDATE : Constants::PERMISSION_CREATE; + + return $this->checkPermissions($targetInternalPath, $permissions) + && parent::moveFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); + } + #[\Override] public function touch(string $path, ?int $mtime = null): bool { $permissions @@ -179,14 +167,29 @@ public function mkdir(string $path): bool { #[\Override] public function rmdir(string $path): bool { - return $this->checkPermissions($path, Constants::PERMISSION_DELETE) - && parent::rmdir($path); + $lockedIds = array_map( + fn (array $entry): int => $entry['lock']->getFileId(), + $this->checkDescendants($this, $path) + ); + $this->checkPermissions($path, Constants::PERMISSION_DELETE); + + $result = parent::rmdir($path); + if ($result && $lockedIds !== []) { + $this->lockService->removeLocksForFileIds($lockedIds); + } + return $result; } #[\Override] public function unlink(string $path): bool { - return $this->checkPermissions($path, Constants::PERMISSION_DELETE) - && parent::unlink($path); + $this->checkPermissions($path, Constants::PERMISSION_DELETE); + $fileId = $this->getCache()->getId($path); + + $result = parent::unlink($path); + if ($result && $fileId > 0) { + $this->lockService->removeLocksForFileIds([$fileId]); + } + return $result; } #[\Override] diff --git a/tests/Feature/AcquisitionTest.php b/tests/Feature/AcquisitionTest.php new file mode 100644 index 00000000..8e76e0b3 --- /dev/null +++ b/tests/Feature/AcquisitionTest.php @@ -0,0 +1,249 @@ +loginAndGetUserFolder(self::USER1)->newFile('dup.txt', 'AAA'); + $request = \OCP\Server::get(LocksRequest::class); + + $first = FileLock::fromLockScope(new LockContext($file, ILock::TYPE_USER, self::USER1)); + $first->setToken('files_lock/dup-1'); + $request->save($first); + self::assertGreaterThan(0, $first->getId()); + + $second = FileLock::fromLockScope(new LockContext($file, ILock::TYPE_USER, self::USER2)); + $second->setToken('files_lock/dup-2'); + try { + $request->save($second); + self::fail('second row for the same file must be rejected'); + } catch (LockConflictException) { + } + self::assertSame(1, $this->lockRowCount($file->getId())); + } + + /** + * The token has a unique index of its own, so an insert can be refused over a + * token another file already holds. That is not a conflict on this file: the + * acquisition takes a fresh token and completes. + */ + public function testTokenCollisionTakesAFreshToken(): void { + $folder = $this->loginAndGetUserFolder(self::USER1); + $taken = $folder->newFile('token-taken.txt', 'AAA'); + $free = $folder->newFile('token-free.txt', 'AAA'); + + $first = $this->lockService()->acquire(new LockContext($taken, ILock::TYPE_TOKEN, self::USER1), null, 'files_lock/collide'); + self::assertSame('files_lock/collide', $first->getToken(), 'without this the second acquisition never collides'); + + $second = $this->lockService()->acquire(new LockContext($free, ILock::TYPE_TOKEN, self::USER1), null, 'files_lock/collide'); + + self::assertNotSame('files_lock/collide', $second->getToken(), 'the colliding token must be replaced'); + self::assertStringStartsWith('files_lock/', $second->getToken()); + self::assertSame(1, $this->lockRowCount($free->getId())); + self::assertSame('files_lock/collide', $this->storedLock($taken->getId())?->getToken(), 'the first lock keeps its token'); + } + + /** + * The scope belongs to the row from the start. Rows written before it was + * stored on insert hold 0, which is not a valid scope, and read back as + * exclusive like every other lock. + */ + public function testScopeIsWrittenOnInsertAndLegacyRowsReadAsExclusive(): void { + $file = $this->loginAndGetUserFolder(self::USER1)->newFile('scope.txt', 'AAA'); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + + $connection = \OCP\Server::get(IDBConnection::class); + $stored = (int)$connection->executeQuery('SELECT `scope` FROM `*PREFIX*files_lock` WHERE `file_id` = ?', [$file->getId()])->fetchOne(); + self::assertSame(ILock::LOCK_EXCLUSIVE, $stored, 'read raw, because reading it back through the model would coerce it'); + + $connection->executeStatement('UPDATE `*PREFIX*files_lock` SET `scope` = 0 WHERE `file_id` = ?', [$file->getId()]); + + self::assertSame(ILock::LOCK_EXCLUSIVE, $this->storedLock($file->getId())?->getScope(), 'a row predating the stored scope reads back as exclusive'); + } + + /** + * The bulk read reports every id it was asked about, so a caller can index the + * result by file id. A file with no lock is reported as false, not left out. + */ + public function testBulkReadReportsEveryRequestedId(): void { + $folder = $this->loginAndGetUserFolder(self::USER1); + $locked = $folder->newFile('bulk-locked.txt', 'AAA'); + $free = $folder->newFile('bulk-free.txt', 'AAA'); + $this->lockManager->lock(new LockContext($locked, ILock::TYPE_USER, self::USER1)); + $this->lockService()->clearCache(); + + $locks = $this->lockService()->getLockForNodeIds([$locked->getId(), $free->getId()]); + + self::assertArrayHasKey($free->getId(), $locks, 'a file with no lock must still be reported'); + self::assertFalse($locks[$free->getId()]); + self::assertInstanceOf(FileLock::class, $locks[$locked->getId()]); + } + + public function testConflictReportsWinningLock(): void { + $file = $this->sharedFile('conflict.txt'); + $mine = $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + + $shared = $this->loginAndGetUserFolder(self::USER2)->get('conflict.txt'); + try { + $this->lockManager->lock(new LockContext($shared, ILock::TYPE_USER, self::USER2)); + self::fail('expected OwnerLockedException'); + } catch (OwnerLockedException $e) { + self::assertSame($mine->getId(), $e->getLock()->getId()); + self::assertSame(self::USER1, $e->getLock()->getOwner()); + } + self::assertSame(1, $this->lockRowCount($file->getId())); + } + + public function testRefreshKeepsTheSameRow(): void { + $file = $this->loginAndGetUserFolder(self::USER1)->newFile('refresh.txt', 'AAA'); + $first = $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + $second = $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + self::assertSame($first->getId(), $second->getId()); + self::assertSame($first->getToken(), $second->getToken()); + self::assertSame(1, $this->lockRowCount($file->getId())); + } + + public function testExpiredLockIsReplaced(): void { + $this->setLockTimeoutMinutes(10); + $file = $this->sharedFile('expired.txt'); + $old = $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + $this->toTheFuture(11 * 60); + + $shared = $this->loginAndGetUserFolder(self::USER2)->get('expired.txt'); + $new = $this->lockManager->lock(new LockContext($shared, ILock::TYPE_USER, self::USER2)); + self::assertNotSame($old->getId(), $new->getId()); + self::assertSame(self::USER2, $new->getOwner()); + self::assertSame(1, $this->lockRowCount($file->getId())); + } + + public function testFoldersCannotBeLocked(): void { + $folder = $this->loginAndGetUserFolder(self::USER1)->newFolder('a-folder'); + $this->expectException(NotFileException::class); + $this->lockManager->lock(new LockContext($folder, ILock::TYPE_USER, self::USER1)); + } + + public function testLockingNeedsUpdatePermission(): void { + $file = $this->sharedFile('readonly.txt', 1); + $shared = $this->loginAndGetUserFolder(self::USER2)->get('readonly.txt'); + try { + $this->lockManager->lock(new LockContext($shared, ILock::TYPE_USER, self::USER2)); + self::fail('expected UnauthorizedUnlockException'); + } catch (UnauthorizedUnlockException) { + } + self::assertSame(0, $this->lockRowCount($file->getId())); + } + + /** + * Two independent PHP processes race for the same file: exactly one wins, + * the other gets a conflict, and the table holds exactly one row. + */ + public function testConcurrentAcquisitionYieldsOneLock(): void { + // the child processes boot a normal server, so the users must live in the database backend + $userManager = \OCP\Server::get(IUserManager::class); + $databaseBackend = new \OC\User\Database(); + $userManager->registerBackend($databaseBackend); + foreach ([self::RACE_USER1, self::RACE_USER2] as $uid) { + if (!$databaseBackend->userExists($uid)) { + $databaseBackend->createUser($uid, 'race-password-' . $uid); + } + } + + try { + $file = $this->loginAndGetUserFolder(self::RACE_USER1)->newFile('race.txt', 'AAA'); + $shareManager = \OCP\Server::get(IShareManager::class); + $share = $shareManager->newShare(); + $share->setNode($file)->setSharedBy(self::RACE_USER1)->setSharedWith(self::RACE_USER2) + ->setShareType(IShare::TYPE_USER)->setPermissions(19); + $share = $shareManager->createShare($share); + $share->setStatus(IShare::STATUS_ACCEPTED); + $shareManager->updateShare($share); + + $script = __DIR__ . '/fixtures/concurrent-lock.php'; + $locked = $conflicts = 0; + for ($round = 0; $round < self::RACE_ROUNDS; $round++) { + $this->clearLocks(); + $startAt = microtime(true) + 3; + $outputs = $this->runConcurrently([ + [PHP_BINARY, $script, self::RACE_USER1, (string)$file->getId(), (string)$startAt], + [PHP_BINARY, $script, self::RACE_USER2, (string)$file->getId(), (string)$startAt], + ]); + $results = array_map(trim(...), $outputs); + sort($results); + self::assertSame(['CONFLICT', 'LOCKED'], $results, 'round ' . $round . ': ' . implode(' | ', $results)); + self::assertSame(1, $this->lockRowCount($file->getId()), 'round ' . $round); + $locked += count(array_keys($results, 'LOCKED', true)); + $conflicts += count(array_keys($results, 'CONFLICT', true)); + } + self::assertSame(self::RACE_ROUNDS, $locked); + self::assertSame(self::RACE_ROUNDS, $conflicts); + } finally { + $this->clearLocks(); + foreach ([self::RACE_USER1, self::RACE_USER2] as $uid) { + $userManager->get($uid)?->delete(); + $databaseBackend->deleteUser($uid); + } + $userManager->removeBackend($databaseBackend); + } + } + + /** + * @param list> $commands + * @return list stdout of each process + */ + private function runConcurrently(array $commands): array { + $processes = []; + $pipes = []; + foreach ($commands as $i => $command) { + $descriptors = [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']]; + $process = proc_open($command, $descriptors, $procPipes, \OC::$SERVERROOT); + self::assertIsResource($process, 'failed to start child process'); + fclose($procPipes[0]); + $processes[$i] = $process; + $pipes[$i] = $procPipes; + } + + $outputs = []; + foreach ($processes as $i => $process) { + $stdout = stream_get_contents($pipes[$i][1]); + $stderr = stream_get_contents($pipes[$i][2]); + fclose($pipes[$i][1]); + fclose($pipes[$i][2]); + $exitCode = proc_close($process); + if ($exitCode !== 0) { + self::fail('child process failed (' . $exitCode . '): ' . $stdout . ' ' . $stderr); + } + $outputs[] = $stdout; + } + + return $outputs; + } +} diff --git a/tests/Feature/AncestorProtectionTest.php b/tests/Feature/AncestorProtectionTest.php new file mode 100644 index 00000000..2d7f3f8c --- /dev/null +++ b/tests/Feature/AncestorProtectionTest.php @@ -0,0 +1,152 @@ +lockService()->clearCache(); + } + + /** + * USER1 owns dir/sub/inner/locked.txt (locked by USER1) and shares dir with USER2. + * + * @return array{File, Folder} the locked file (USER1 view) and dir (USER2 view) + */ + private function sharedTree(): array { + $root = $this->loginAndGetUserFolder(self::USER1); + $dir = $root->newFolder('dir'); + $inner = $dir->newFolder('sub')->newFolder('inner'); + $locked = $inner->newFile('locked.txt', 'important'); + $this->shareWith($dir, self::USER1, self::USER2, 31); + $this->lockManager->lock(new LockContext($locked, ILock::TYPE_USER, self::USER1)); + $this->trashBefore = $this->trashEntries(); + + \OC_Util::setupFS(self::USER2); + $this->actAs(self::USER2); + return [$locked, $this->rootFolder->getUserFolder(self::USER2)->get('dir')]; + } + + private function trashEntries(): int { + try { + $trash = $this->rootFolder->getUserFolder(self::USER1)->getParent()->get('files_trashbin/files'); + return count($trash->getDirectoryListing()); + } catch (NotFoundException) { + return 0; + } + } + + private function assertIntact(File $locked): void { + $this->actAs(self::USER1); + $this->lockService()->clearCache(); + $file = $this->rootFolder->getUserFolder(self::USER1)->get('dir/sub/inner/locked.txt'); + self::assertSame($locked->getId(), $file->getId()); + self::assertSame('important', $file->getContent()); + self::assertSame(1, $this->lockRowCount($locked->getId())); + self::assertSame($this->trashBefore, $this->trashEntries(), 'nothing was moved to the trash bin'); + $this->actAs(self::USER2); + $this->lockService()->clearCache(); + } + + private function assertLocked(callable $operation, File $locked): void { + try { + $operation(); + self::fail('operation on an ancestor of a locked file should be refused'); + } catch (LockedException) { + } + $this->assertIntact($locked); + } + + public function testDirectOperationsOnLockedFileAreRefused(): void { + [$locked, $dir] = $this->sharedTree(); + $file = $dir->get('sub/inner/locked.txt'); + $this->assertLocked(fn () => $file->delete(), $locked); + $this->assertLocked(fn () => $file->move($dir->getPath() . '/moved.txt'), $locked); + } + + public function testParentOperationsAreRefused(): void { + [$locked, $dir] = $this->sharedTree(); + $inner = $dir->get('sub/inner'); + $this->assertLocked(fn () => $inner->delete(), $locked); + $this->assertLocked(fn () => $inner->move($dir->getPath() . '/inner-moved'), $locked); + $this->assertLocked(fn () => $inner->move($dir->getPath() . '/sub/renamed'), $locked); + } + + public function testNestedParentOperationsAreRefused(): void { + [$locked, $dir] = $this->sharedTree(); + $sub = $dir->get('sub'); + $this->assertLocked(fn () => $sub->delete(), $locked); + $this->assertLocked(fn () => $sub->move($dir->getPath() . '/sub-moved'), $locked); + } + + public function testStorageLevelOperationsAreRefused(): void { + // equivalent of a deployment without the trash bin: the storage is hit directly + [$locked, $dir] = $this->sharedTree(); + $storage = $dir->getStorage(); + $internal = $dir->getInternalPath(); + $subPath = ($internal === '' ? '' : $internal . '/') . 'sub'; + $this->assertLocked(fn () => $storage->rmdir($subPath), $locked); + $this->assertLocked(fn () => $storage->rename($subPath, $subPath . '-moved'), $locked); + self::assertTrue($storage->file_exists($subPath . '/inner/locked.txt')); + } + + public function testLockOwnerMayOperateOnAncestors(): void { + [$locked] = $this->sharedTree(); + $this->actAs(self::USER1); + $root = $this->rootFolder->getUserFolder(self::USER1); + $root->get('dir/sub')->move($root->getPath() . '/dir/sub-moved'); + self::assertSame('important', $root->get('dir/sub-moved/inner/locked.txt')->getContent()); + self::assertSame(1, $this->lockRowCount($locked->getId()), 'a move keeps the lock'); + $root->get('dir/sub-moved')->delete(); + self::assertSame(0, $this->lockRowCount($locked->getId()), 'deleting the file removes its lock'); + } + + public function testAncestorProtectionOnNonHomeMount(): void { + $storage = new Temporary([]); + $this->loginAndGetUserFolder(self::USER1); + \OC_Util::setupFS(self::USER2); + Filesystem::mount($storage, [], '/' . self::USER1 . '/files/ext/'); + Filesystem::mount($storage, [], '/' . self::USER2 . '/files/ext/'); + $locked = $this->rootFolder->getUserFolder(self::USER1)->get('ext')->newFolder('sub')->newFile('locked.txt', 'important'); + $this->lockManager->lock(new LockContext($locked, ILock::TYPE_USER, self::USER1)); + + $this->actAs(self::USER2); + $sub = $this->rootFolder->getUserFolder(self::USER2)->get('ext/sub'); + try { + $sub->delete(); + self::fail('deleting the parent on a non-home mount must be refused'); + } catch (LockedException) { + } + try { + $sub->move($this->rootFolder->getUserFolder(self::USER2)->getPath() . '/ext/sub2'); + self::fail('moving the parent on a non-home mount must be refused'); + } catch (LockedException) { + } + self::assertSame('important', $this->rootFolder->getUserFolder(self::USER2)->get('ext/sub/locked.txt')->getContent()); + self::assertSame(1, $this->lockRowCount($locked->getId())); + } +} diff --git a/tests/Feature/CommandTest.php b/tests/Feature/CommandTest.php index 696e030c..704c7844 100644 --- a/tests/Feature/CommandTest.php +++ b/tests/Feature/CommandTest.php @@ -13,12 +13,14 @@ use OCA\FilesLock\Command\Lock; use OCP\Files\Lock\ILock; use OCP\Files\Lock\LockContext; +use OCP\Share\IManager as IShareManager; +use OCP\Share\IShare; use PHPUnit\Framework\Attributes\Group; use Psr\Container\ContainerInterface; use Symfony\Component\Console\Tester\CommandTester; /** - * occ files:lock: status, locking and unlocking. + * occ files:lock: status, locking, and forced unlock as an administrative operation. */ #[Group(name: 'DB')] class CommandTest extends LockTestCase { @@ -43,11 +45,60 @@ public function testStatusAndLock(): void { self::assertStringContainsString('locked by ' . self::USER1, $tester->getDisplay()); } - public function testUnlock(): void { - $file = $this->loginAndGetUserFolder(self::USER1)->newFile('cli-unlock.txt', 'AAA'); + public function testLockingAnAlreadyLockedFileFailsCleanly(): void { + $file = $this->sharedFile('cli-conflict.txt'); $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); - self::assertSame(0, $this->tester()->execute(['file_id' => (string)$file->getId(), '--unlock' => true])); - self::assertSame(0, $this->lockRowCount($file->getId())); + $tester = $this->tester(); + self::assertSame(1, $tester->execute(['file_id' => (string)$file->getId(), 'user_id' => self::USER2])); + self::assertStringContainsString('already locked by ' . self::USER1, $tester->getDisplay()); + self::assertSame(1, $this->lockRowCount($file->getId()), 'the existing lock is untouched'); + } + + public function testLockingAFolderFailsCleanly(): void { + $folder = $this->loginAndGetUserFolder(self::USER1)->newFolder('cli-folder'); + + $tester = $this->tester(); + self::assertSame(1, $tester->execute(['file_id' => (string)$folder->getId(), 'user_id' => self::USER1])); + self::assertStringContainsString('not a file', $tester->getDisplay()); + self::assertSame(0, $this->lockRowCount($folder->getId())); + } + + public function testForcedUnlockAfterOwnerLostAccess(): void { + $file = $this->sharedFile('cli-force.txt'); + $id = $file->getId(); + $shared = $this->loginAndGetUserFolder(self::USER2)->get('cli-force.txt'); + $this->lockManager->lock(new LockContext($shared, ILock::TYPE_USER, self::USER2)); + + $shareManager = \OCP\Server::get(IShareManager::class); + foreach ($shareManager->getSharesBy(self::USER1, IShare::TYPE_USER, $file) as $share) { + $shareManager->deleteShare($share); + } + $this->logout(); + $tester = $this->tester(); + + self::assertSame(0, $tester->execute(['file_id' => (string)$id, '--unlock' => true])); + self::assertStringContainsString('Unlocked file #' . $id, $tester->getDisplay()); + self::assertSame(0, $this->lockRowCount($id)); + + self::assertSame(0, $tester->execute(['file_id' => (string)$id, '--unlock' => true])); + self::assertStringContainsString('already unlocked', $tester->getDisplay()); + } + + public function testForcedUnlockOfAppAndTokenLocks(): void { + $file = $this->loginAndGetUserFolder(self::USER1)->newFile('cli-app.txt', 'AAA'); + $id = $file->getId(); + $tester = $this->tester(); + + $this->lockManager->lock(new LockContext($file, ILock::TYPE_APP, 'text')); + $this->logout(); + self::assertSame(0, $tester->execute(['file_id' => (string)$id, '--unlock' => true])); + self::assertSame(0, $this->lockRowCount($id)); + + $this->loginAndGetUserFolder(self::USER1); + $this->lockService()->acquire(new LockContext($file, ILock::TYPE_TOKEN, self::USER1), null, 'cli-token'); + $this->logout(); + self::assertSame(0, $tester->execute(['file_id' => (string)$id, '--unlock' => true])); + self::assertSame(0, $this->lockRowCount($id)); } } diff --git a/tests/Feature/DavLockTest.php b/tests/Feature/DavLockTest.php new file mode 100644 index 00000000..2dc81df3 --- /dev/null +++ b/tests/Feature/DavLockTest.php @@ -0,0 +1,469 @@ +user]; + } + + public function challenge(RequestInterface $request, ResponseInterface $response) { + } +} + +class CapturingSapi { + private ?Response $response = null; + + public function __construct( + private readonly Request $request, + ) { + } + + public function getRequest(): Request { + return $this->request; + } + + public function sendResponse(Response $response): void { + $copy = fopen('php://temp', 'r+'); + $body = $response->getBody(); + if (is_string($body)) { + fwrite($copy, $body); + } elseif (is_resource($body)) { + stream_copy_to_stream($body, $copy); + } elseif (is_callable($body)) { + ob_start(); + $body(); + fwrite($copy, (string)ob_get_clean()); + } + rewind($copy); + $this->response = new Response($response->getStatus(), $response->getHeaders(), $copy); + } + + public function getResponse(): Response { + return $this->response; + } +} + +/** + * Native WebDAV LOCK/UNLOCK, If-header validation and X-User-Lock through a + * real Sabre server built by the DAV app. + */ +#[Group(name: 'DB')] +class DavLockTest extends LockTestCase { + private const string LOCK_BODY = '%s'; + + private ServerFactory $serverFactory; + + protected function setUp(): void { + parent::setUp(); + $this->serverFactory = new ServerFactory( + \OCP\Server::get(IConfig::class), + \OCP\Server::get(LoggerInterface::class), + \OCP\Server::get(IDBConnection::class), + \OCP\Server::get(IUserSession::class), + \OCP\Server::get(IMountManager::class), + \OCP\Server::get(ITagManager::class), + $this->createMock(IRequest::class), + \OCP\Server::get(IPreview::class), + \OCP\Server::get(IEventDispatcher::class), + \OCP\Server::get(IFactory::class)->get('dav'), + ); + } + + /** + * @param array $headers + */ + private function request(string $user, string $method, string $path, ?string $body = null, array $headers = []): Response { + $this->loginAsUser($user); + $this->lockService()->clearCache(); + $view = new View('/' . $user . '/files'); + $server = $this->serverFactory->createServer(false, '/', 'dummy', new \Sabre\DAV\Auth\Plugin(new StaticAuthBackend($user)), fn (): \OC\Files\View => $view); + $stream = null; + if ($body !== null) { + $stream = fopen('php://temp', 'r+'); + fwrite($stream, $body); + rewind($stream); + } + $request = new Request($method, $path, $headers, $stream); + $sapi = new CapturingSapi($request); + $server->sapi = $sapi; + $server->httpRequest = $request; + $server->exec(); + // a refused request never reaches Sabre's afterMethod, so the transactional + // file lock taken for PUT stays behind; every real request is its own process + \OCP\Server::get(ILockingProvider::class)->releaseAll(); + $this->lockService()->clearCache(); + return $sapi->getResponse(); + } + + private function body(Response $response): string { + $body = $response->getBody(); + if (is_resource($body)) { + rewind($body); + return (string)stream_get_contents($body); + } + return (string)$body; + } + + private function nativeLock(string $user, string $path, string $owner = 'client', array $headers = []): Response { + return $this->request($user, 'LOCK', $path, sprintf(self::LOCK_BODY, $owner), $headers + ['Content-Type' => 'application/xml']); + } + + private function tokenOf(Response $response): string { + $header = (string)$response->getHeader('Lock-Token'); + self::assertMatchesRegularExpression('/^$/', $header); + return substr($header, strlen('request($user, 'PROPFIND', $path, '', ['Depth' => '0']); + self::assertSame(207, $response->getStatus()); + $body = $this->body($response); + $props = []; + foreach (['lock', 'lock-owner', 'lock-owner-type', 'lock-timeout', 'lock-token'] as $prop) { + preg_match('#([^<]*)#', $body, $m); + $props[$prop] = $m[1] ?? null; + } + $props['lockdiscovery'] = str_contains($body, ''); + return $props; + } + + public function testNativeLockLifecycle(): void { + $this->setLockTimeoutMinutes(-1); + $this->toTheFuture(0); + $file = $this->loginAndGetUserFolder(self::USER1)->newFile('native.txt', 'AAA'); + + $response = $this->nativeLock(self::USER1, '/native.txt'); + self::assertSame(200, $response->getStatus()); + $token = $this->tokenOf($response); + self::assertStringContainsString('Infinite', $this->body($response)); + $stored = $this->storedLock($file->getId()); + self::assertSame(self::USER1, $stored?->getOwner()); + self::assertSame(ILock::TYPE_TOKEN, $stored?->getType()); + self::assertSame($token, $stored?->getToken()); + self::assertNull($stored?->getExpiresAt()); + self::assertSame(self::USER1, $stored?->getDisplayName(), 'display name comes from the user, not from the client'); + + // refresh with a timeout + $response = $this->request(self::USER1, 'LOCK', '/native.txt', null, ['If' => '()', 'Timeout' => 'Second-1200']); + self::assertSame(200, $response->getStatus()); + self::assertStringContainsString('Second-1200', $this->body($response)); + self::assertSame($this->time + 1200, $this->storedLock($file->getId())?->getExpiresAt()); + self::assertSame(1, $this->lockRowCount($file->getId())); + + // creator writes with the token, not without it + self::assertSame(423, $this->request(self::USER1, 'PUT', '/native.txt', 'BBB')->getStatus()); + self::assertContains($this->request(self::USER1, 'PUT', '/native.txt', 'CCC', ['If' => '()'])->getStatus(), [200, 204]); + self::assertSame('CCC', $this->rootFolder->getUserFolder(self::USER1)->get('native.txt')->getContent()); + + // wrong token, then the right one + self::assertSame(409, $this->request(self::USER1, 'UNLOCK', '/native.txt', null, ['Lock-Token' => ''])->getStatus()); + self::assertSame(1, $this->lockRowCount($file->getId())); + self::assertSame(204, $this->request(self::USER1, 'UNLOCK', '/native.txt', null, ['Lock-Token' => ''])->getStatus()); + self::assertSame(0, $this->lockRowCount($file->getId())); + } + + public function testNativeTimeoutHeaders(): void { + $this->setLockTimeoutMinutes(30); + $this->toTheFuture(0); + $file = $this->loginAndGetUserFolder(self::USER1)->newFile('timeout.txt', 'AAA'); + + $response = $this->nativeLock(self::USER1, '/timeout.txt'); + self::assertStringContainsString('Second-1800', $this->body($response), 'no header falls back to the configured timeout'); + self::assertSame($this->time + 1800, $this->storedLock($file->getId())?->getExpiresAt()); + self::assertSame('1800', $this->lockProps(self::USER1, '/timeout.txt')['lock-timeout']); + $this->request(self::USER1, 'UNLOCK', '/timeout.txt', null, ['Lock-Token' => 'tokenOf($response) . '>']); + + $response = $this->nativeLock(self::USER1, '/timeout.txt', 'client', ['Timeout' => 'Second-600']); + self::assertStringContainsString('Second-600', $this->body($response)); + self::assertSame($this->time + 600, $this->storedLock($file->getId())?->getExpiresAt()); + self::assertSame('600', $this->lockProps(self::USER1, '/timeout.txt')['lock-timeout']); + $this->request(self::USER1, 'UNLOCK', '/timeout.txt', null, ['Lock-Token' => 'tokenOf($response) . '>']); + + $response = $this->nativeLock(self::USER1, '/timeout.txt', 'client', ['Timeout' => 'Infinite']); + self::assertStringContainsString('Infinite', $this->body($response)); + self::assertNull($this->storedLock($file->getId())?->getExpiresAt()); + // clients read 0 as "never expires"; a negative value lands in the past + self::assertSame('0', $this->lockProps(self::USER1, '/timeout.txt')['lock-timeout']); + } + + public function testOwnerMetadataIsNotPersisted(): void { + $file = $this->loginAndGetUserFolder(self::USER1)->newFile('owner.txt', 'AAA'); + $response = $this->nativeLock(self::USER1, '/owner.txt', 'Admin (Desktop client)'); + self::assertSame(200, $response->getStatus()); + self::assertSame(self::USER1, $this->storedLock($file->getId())?->getDisplayName()); + $this->request(self::USER1, 'UNLOCK', '/owner.txt', null, ['Lock-Token' => 'tokenOf($response) . '>']); + + $response = $this->nativeLock(self::USER1, '/owner.txt', str_repeat('x', 5000)); + self::assertSame(200, $response->getStatus()); + self::assertSame(1, $this->lockRowCount($file->getId())); + self::assertSame(self::USER1, $this->storedLock($file->getId())?->getDisplayName()); + } + + public function testForeignPrincipalCannotUseTheToken(): void { + $file = $this->sharedFile('foreign.txt'); + $response = $this->nativeLock(self::USER1, '/foreign.txt'); + $token = $this->tokenOf($response); + + self::assertSame($token, $this->lockProps(self::USER2, '/foreign.txt')['lock-token'], 'the token stays discoverable'); + self::assertSame(423, $this->request(self::USER2, 'PUT', '/foreign.txt', 'BBB')->getStatus()); + self::assertSame(423, $this->request(self::USER2, 'PUT', '/foreign.txt', 'BBB', ['If' => '()'])->getStatus(), 'RFC 4918 6.4: the principal must match the lock creator'); + self::assertSame('AAA', $this->rootFolder->getUserFolder(self::USER1)->get('foreign.txt')->getContent()); + self::assertSame(423, $this->request(self::USER2, 'MOVE', '/foreign.txt', null, ['Destination' => '/moved.txt', 'If' => '()'])->getStatus()); + self::assertSame(423, $this->request(self::USER2, 'DELETE', '/foreign.txt', null, ['If' => '()'])->getStatus()); + self::assertSame(1, $this->lockRowCount($file->getId())); + } + + public function testReadOnlyRecipientCannotReleaseWithThePublishedToken(): void { + $file = $this->sharedFile('ro-token.txt', 19, 1); + $response = $this->nativeLock(self::USER2, '/ro-token.txt'); + self::assertSame(200, $response->getStatus()); + $token = $this->tokenOf($response); + + // the token stays discoverable, as RFC 4918 section 6.5 allows + self::assertSame($token, $this->lockProps(self::USER3, '/ro-token.txt')['lock-token']); + + // but a user who may not write the file may not release its lock + self::assertSame(403, $this->request(self::USER3, 'UNLOCK', '/ro-token.txt', null, ['Lock-Token' => ''])->getStatus()); + self::assertSame(1, $this->lockRowCount($file->getId())); + + // the user it was created for still can + self::assertSame(204, $this->request(self::USER2, 'UNLOCK', '/ro-token.txt', null, ['Lock-Token' => ''])->getStatus()); + self::assertSame(0, $this->lockRowCount($file->getId())); + } + + public function testNativeLockOverExistingLockIsRefused(): void { + $file = $this->sharedFile('taken.txt'); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + $response = $this->nativeLock(self::USER2, '/taken.txt'); + self::assertSame(423, $response->getStatus()); + self::assertSame(ILock::TYPE_USER, $this->storedLock($file->getId())?->getType()); + self::assertSame(1, $this->lockRowCount($file->getId())); + + // the owner's own user lock is not a fake success either + $response = $this->nativeLock(self::USER1, '/taken.txt'); + self::assertSame(423, $response->getStatus()); + self::assertSame(ILock::TYPE_USER, $this->storedLock($file->getId())?->getType()); + } + + public function testUserLockCreatorHasTheNativeLifecycle(): void { + $file = $this->sharedFile('userlock.txt'); + $lock = $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + + $props = $this->lockProps(self::USER1, '/userlock.txt'); + self::assertSame('1', $props['lock']); + self::assertSame($lock->getToken(), $props['lock-token']); + self::assertTrue($props['lockdiscovery'], 'the creator sees the lock in lockdiscovery'); + + self::assertContains($this->request(self::USER1, 'PUT', '/userlock.txt', 'BBB')->getStatus(), [200, 204]); + self::assertContains($this->request(self::USER1, 'PUT', '/userlock.txt', 'CCC', ['If' => '(getToken() . '>)'])->getStatus(), [200, 204]); + self::assertSame(423, $this->request(self::USER2, 'PUT', '/userlock.txt', 'DDD')->getStatus()); + self::assertSame(403, $this->request(self::USER2, 'UNLOCK', '/userlock.txt', null, ['Lock-Token' => 'getToken() . '>'])->getStatus(), 'a user lock is bound to its user'); + self::assertSame(1, $this->lockRowCount($file->getId())); + self::assertSame(204, $this->request(self::USER1, 'UNLOCK', '/userlock.txt', null, ['Lock-Token' => 'getToken() . '>'])->getStatus()); + self::assertSame(0, $this->lockRowCount($file->getId())); + } + + public function testXUserLock(): void { + $file = $this->sharedFile('xuser.txt'); + $folder = $this->loginAndGetUserFolder(self::USER1)->newFolder('xfolder'); + + $response = $this->request(self::USER1, 'LOCK', '/xuser.txt', null, ['X-User-Lock' => '1']); + self::assertSame(200, $response->getStatus()); + self::assertStringContainsString('' . self::USER1 . '', $this->body($response)); + self::assertStringContainsString('0', $this->body($response)); + + $response = $this->request(self::USER2, 'LOCK', '/xuser.txt', null, ['X-User-Lock' => '1']); + self::assertSame(423, $response->getStatus()); + self::assertStringContainsString('' . self::USER1 . '', $this->body($response)); + + self::assertSame(423, $this->request(self::USER2, 'UNLOCK', '/xuser.txt', null, ['X-User-Lock' => '1'])->getStatus()); + self::assertSame(400, $this->request(self::USER1, 'UNLOCK', '/xuser.txt', null, ['X-User-Lock' => '1', 'X-User-Lock-Type' => '99'])->getStatus()); + self::assertSame(200, $this->request(self::USER1, 'UNLOCK', '/xuser.txt', null, ['X-User-Lock' => '1'])->getStatus()); + self::assertSame(412, $this->request(self::USER1, 'UNLOCK', '/xuser.txt', null, ['X-User-Lock' => '1'])->getStatus()); + self::assertSame(404, $this->request(self::USER1, 'LOCK', '/missing.txt', null, ['X-User-Lock' => '1'])->getStatus()); + self::assertSame(403, $this->request(self::USER1, 'LOCK', '/xfolder', null, ['X-User-Lock' => '1'])->getStatus()); + self::assertSame(0, $this->lockRowCount($folder->getId())); + + // desktop client style token lock: the creator releases it with the header, others do not + $response = $this->request(self::USER1, 'LOCK', '/xuser.txt', null, ['X-User-Lock' => '1', 'X-User-Lock-Type' => '2']); + self::assertSame(200, $response->getStatus()); + self::assertSame(ILock::TYPE_TOKEN, $this->storedLock($file->getId())?->getType()); + self::assertSame(423, $this->request(self::USER2, 'UNLOCK', '/xuser.txt', null, ['X-User-Lock' => '1', 'X-User-Lock-Type' => '2'])->getStatus()); + self::assertSame(200, $this->request(self::USER1, 'UNLOCK', '/xuser.txt', null, ['X-User-Lock' => '1', 'X-User-Lock-Type' => '2'])->getStatus()); + + // The desktop client always takes the lock as a user lock but releases it + // with whatever type its own journal recorded, so the two disagree as soon + // as that record goes stale. Asserted through the recipient, who does not + // own the file, because the file owner is let through before any of this. + self::assertSame(200, $this->request(self::USER2, 'LOCK', '/xuser.txt', null, ['X-User-Lock' => '1', 'X-User-Lock-Type' => '0'])->getStatus()); + self::assertSame(200, $this->request(self::USER2, 'UNLOCK', '/xuser.txt', null, ['X-User-Lock' => '1', 'X-User-Lock-Type' => '2'])->getStatus()); + self::assertSame(0, $this->lockRowCount($file->getId()), 'a lock the holder asked to release must not survive'); + } + + public function testAncestorOperationsAreRefusedBeforeMutation(): void { + $root = $this->loginAndGetUserFolder(self::USER1); + $dir = $root->newFolder('tree'); + $locked = $dir->newFolder('sub')->newFile('locked.txt', 'important'); + $this->shareWith($dir, self::USER1, self::USER2, 31); + $this->lockManager->lock(new LockContext($locked, ILock::TYPE_USER, self::USER1)); + + self::assertSame(423, $this->request(self::USER2, 'DELETE', '/tree/sub')->getStatus()); + self::assertSame(423, $this->request(self::USER2, 'MOVE', '/tree/sub', null, ['Destination' => '/tree/sub2'])->getStatus()); + self::assertSame(423, $this->request(self::USER2, 'DELETE', '/tree/sub/locked.txt')->getStatus()); + + $this->loginAsUser(self::USER1); + $this->lockService()->clearCache(); + self::assertSame('important', $this->rootFolder->getUserFolder(self::USER1)->get('tree/sub/locked.txt')->getContent()); + self::assertSame(1, $this->lockRowCount($locked->getId())); + self::assertSame(0, count(\OCP\Server::get(LocksRequest::class)->getLocksBelow($dir->getId())) - 1); + } + + public function testNativeClientMayLockACollection(): void { + $folder = $this->loginAndGetUserFolder(self::USER1)->newFolder('coll'); + $folder->newFile('inside.txt', 'AAA'); + $response = $this->nativeLock(self::USER1, '/coll/'); + self::assertSame(200, $response->getStatus()); + $token = $this->tokenOf($response); + self::assertSame(1, $this->lockRowCount($folder->getId())); + self::assertSame(403, $this->request(self::USER1, 'LOCK', '/coll/', null, ['X-User-Lock' => '1'])->getStatus(), 'the application paths still refuse folders'); + self::assertSame(204, $this->request(self::USER1, 'UNLOCK', '/coll/', null, ['Lock-Token' => ''])->getStatus()); + self::assertSame(0, $this->lockRowCount($folder->getId())); + } + + public function testOtherUsersLockIsVisibleButNotUsable(): void { + $file = $this->sharedFile('visible.txt'); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + $props = $this->lockProps(self::USER2, '/visible.txt'); + self::assertSame('1', $props['lock']); + self::assertSame(self::USER1, $props['lock-owner']); + self::assertSame('0', $props['lock-owner-type']); + self::assertSame(423, $this->request(self::USER2, 'PROPPATCH', '/visible.txt', '1')->getStatus()); + } + + public function testALockedFileStaysLockedAfterItsHolderMovesIt(): void { + $dir = $this->loginAndGetUserFolder(self::USER1)->newFolder('churn'); + $file = $dir->newFile('travel.txt', 'AAA'); + $this->shareWith($dir, self::USER1, self::USER2, 31); + $id = $file->getId(); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + + self::assertContains($this->request(self::USER1, 'MOVE', '/churn/travel.txt', null, ['Destination' => '/churn/travelled.txt'])->getStatus(), [201, 204]); + self::assertSame(1, $this->lockRowCount($id)); + + $props = $this->lockProps(self::USER2, '/churn/travelled.txt'); + self::assertSame('1', $props['lock']); + self::assertSame(self::USER1, $props['lock-owner']); + self::assertSame(423, $this->request(self::USER2, 'PUT', '/churn/travelled.txt', 'BBB')->getStatus()); + } + + /** + * The web client never holds the lock token of a lock its own desktop client + * took, so it is refused like any other client until that client releases it. + */ + public function testOnlyTheClientHoldingATokenLockMovesTheFile(): void { + $dir = $this->loginAndGetUserFolder(self::USER1)->newFolder('churn'); + $file = $dir->newFile('doc.odt', 'AAA'); + $this->shareWith($dir, self::USER1, self::USER2, 31); + $id = $file->getId(); + $token = $this->tokenOf($this->nativeLock(self::USER1, '/churn/doc.odt')); + $if = ['If' => '()']; + + self::assertSame(423, $this->request(self::USER1, 'MOVE', '/churn/doc.odt', null, ['Destination' => '/churn/renamed.odt'])->getStatus()); + + self::assertContains($this->request(self::USER1, 'MOVE', '/churn/doc.odt', null, ['Destination' => '/churn/renamed.odt'] + $if)->getStatus(), [201, 204]); + self::assertSame(1, $this->lockRowCount($id)); + self::assertSame($token, $this->lockProps(self::USER2, '/churn/renamed.odt')['lock-token']); + + self::assertSame(204, $this->request(self::USER1, 'UNLOCK', '/churn/renamed.odt', null, ['Lock-Token' => ''])->getStatus()); + self::assertContains($this->request(self::USER2, 'MOVE', '/churn/renamed.odt', null, ['Destination' => '/churn/theirs.odt'])->getStatus(), [201, 204]); + } + + /** + * A client that never sends UNLOCK (crashed editor, killed sync client) must + * not hold the file for the rest of the team beyond the configured timeout. + */ + public function testAnAbandonedClientLockStopsBlockingWhenItExpires(): void { + $this->setLockTimeoutMinutes(30); + $this->toTheFuture(0); + $dir = $this->loginAndGetUserFolder(self::USER1)->newFolder('churn'); + $dir->newFile('abandoned.txt', 'AAA'); + $this->shareWith($dir, self::USER1, self::USER2, 31); + $this->nativeLock(self::USER1, '/churn/abandoned.txt'); + + self::assertSame(423, $this->request(self::USER2, 'MOVE', '/churn/abandoned.txt', null, ['Destination' => '/churn/mine.txt'])->getStatus()); + + $this->toTheFuture(1801); + self::assertSame('', $this->lockProps(self::USER2, '/churn/abandoned.txt')['lock']); + self::assertContains($this->request(self::USER2, 'MOVE', '/churn/abandoned.txt', null, ['Destination' => '/churn/mine.txt'])->getStatus(), [201, 204]); + } + + /** + * Locks other clients take and release between two listings, which is where a + * stale hit in the bulk PROPFIND cache would surface as a phantom lock. + */ + public function testDirectoryListingsFollowLocksTakenByOtherClients(): void { + $dir = $this->loginAndGetUserFolder(self::USER1)->newFolder('churn'); + $one = $dir->newFile('one.txt', 'AAA'); + $dir->newFile('two.txt', 'BBB'); + $this->shareWith($dir, self::USER1, self::USER2, 31); + + self::assertSame(0, $this->countLockOwners(self::USER2, '/churn/')); + + $lock = $this->lockManager->lock(new LockContext($one, ILock::TYPE_USER, self::USER1)); + self::assertSame(1, $this->countLockOwners(self::USER2, '/churn/')); + self::assertSame(423, $this->request(self::USER2, 'PUT', '/churn/one.txt', 'CCC')->getStatus()); + self::assertContains($this->request(self::USER2, 'PUT', '/churn/two.txt', 'CCC')->getStatus(), [200, 204]); + + self::assertSame(204, $this->request(self::USER1, 'UNLOCK', '/churn/one.txt', null, ['Lock-Token' => 'getToken() . '>'])->getStatus()); + self::assertSame(0, $this->countLockOwners(self::USER2, '/churn/')); + self::assertContains($this->request(self::USER2, 'PUT', '/churn/one.txt', 'DDD')->getStatus(), [200, 204]); + } + + private function countLockOwners(string $user, string $path): int { + $response = $this->request($user, 'PROPFIND', $path, '', ['Depth' => '1']); + self::assertSame(207, $response->getStatus()); + return preg_match_all('#[^<]+#', $this->body($response)); + } + + #[\Override] + protected function sharedFile(string $name, int $permissions = 19, ?int $permissionsUser3 = null): File { + return parent::sharedFile($name, $permissions, $permissionsUser3); + } +} diff --git a/tests/Feature/ExpirationTest.php b/tests/Feature/ExpirationTest.php new file mode 100644 index 00000000..a37052f5 --- /dev/null +++ b/tests/Feature/ExpirationTest.php @@ -0,0 +1,224 @@ +invoke($job, null); + } + + public function testFiniteLockHasAbsoluteExpiry(): void { + $this->setLockTimeoutMinutes(15); + $this->toTheFuture(0); + $file = $this->loginAndGetUserFolder(self::USER1)->newFile('finite.txt', 'AAA'); + $lock = $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + + self::assertSame($this->time + 15 * 60, $lock->getExpiresAt()); + self::assertSame(15 * 60, $lock->getTimeout()); + self::assertSame(15 * 60, $lock->getETA()); + self::assertSame($this->time + 15 * 60, $this->storedLock($file->getId())?->getExpiresAt()); + } + + public function testInfiniteLock(): void { + $this->setLockTimeoutMinutes(-1); + $file = $this->loginAndGetUserFolder(self::USER1)->newFile('infinite.txt', 'AAA'); + $lock = $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + + self::assertNull($lock->getExpiresAt()); + self::assertSame(FileLock::ETA_INFINITE, $lock->getETA()); + self::assertSame(FileLock::ETA_INFINITE, $lock->getTimeout()); + $this->toTheFuture(365 * 24 * 3600); + self::assertCount(1, $this->lockManager->getLocks($file->getId())); + self::assertSame([], $this->lockService()->getExpiredLocks()); + } + + public function testRefreshMovesExpiry(): void { + $this->setLockTimeoutMinutes(30); + $this->toTheFuture(0); + $file = $this->loginAndGetUserFolder(self::USER1)->newFile('refresh.txt', 'AAA'); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + + $this->toTheFuture(20 * 60); + $refreshed = $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + self::assertSame($this->time + 30 * 60, $refreshed->getExpiresAt()); + self::assertSame(30 * 60, $refreshed->getETA()); + self::assertSame(50 * 60, $refreshed->getTimeout(), 'lifetime counted from creation'); + self::assertSame($this->time + 30 * 60, $this->storedLock($file->getId())?->getExpiresAt()); + } + + public function testCronKeepsRefreshedLockAndRemovesExpiredOne(): void { + $this->setLockTimeoutMinutes(30); + $this->toTheFuture(0); + $file = $this->loginAndGetUserFolder(self::USER1)->newFile('cron.txt', 'AAA'); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + + $this->toTheFuture(20 * 60); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + + $this->toTheFuture(11 * 60); + self::assertSame([], $this->lockService()->getExpiredLocks(), 'a refreshed lock is not expired 31 minutes after creation'); + $this->runCron(); + self::assertSame(1, $this->lockRowCount($file->getId())); + + $this->toTheFuture(20 * 60); + self::assertCount(1, $this->lockService()->getExpiredLocks()); + $this->runCron(); + self::assertSame(0, $this->lockRowCount($file->getId())); + } + + public function testExpiredLockNoLongerBlocksAndIsRemovedOnRead(): void { + $this->setLockTimeoutMinutes(30); + $file = $this->sharedFile('expire-write.txt'); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + + $shared = $this->loginAndGetUserFolder(self::USER2)->get('expire-write.txt'); + try { + $shared->putContent('BBB'); + self::fail('lock should block before expiry'); + } catch (ManuallyLockedException) { + } + + $this->toTheFuture(31 * 60); + self::assertSame([], $this->lockManager->getLocks($file->getId())); + $shared->putContent('CCC'); + self::assertSame('CCC', $shared->getContent()); + self::assertSame(0, $this->lockRowCount($file->getId())); + } + + public function testRefreshAfterExpiryCreatesANewLock(): void { + $this->setLockTimeoutMinutes(10); + $this->toTheFuture(0); + $file = $this->loginAndGetUserFolder(self::USER1)->newFile('late.txt', 'AAA'); + $first = $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + + $this->toTheFuture(11 * 60); + $second = $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + self::assertNotSame($first->getId(), $second->getId()); + self::assertSame($this->time + 10 * 60, $second->getExpiresAt()); + } + + public function testRefreshRacingCronSurvives(): void { + $this->setLockTimeoutMinutes(30); + $this->toTheFuture(0); + $file = $this->loginAndGetUserFolder(self::USER1)->newFile('race-cron.txt', 'AAA'); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + + // cron selects its batch at t+29, the client refreshes at the same moment, cron deletes afterwards + $this->toTheFuture(29 * 60); + $batch = $this->lockService()->getExpiredLocks(1000); + self::assertSame([], $batch); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + $this->lockService()->removeLocks($batch); + self::assertSame(1, $this->lockRowCount($file->getId())); + + // a refresh that lands after cron selected an expired batch replaces the expired row, + // and cron's deletion by id cannot remove the replacement + $this->toTheFuture(31 * 60); + $batch = $this->lockService()->getExpiredLocks(1000); + self::assertCount(1, $batch); + $fresh = $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + $this->lockService()->removeLocks($batch); + self::assertSame(1, $this->lockRowCount($file->getId())); + self::assertSame($fresh->getId(), $this->storedLock($file->getId())?->getId()); + } + + /** + * The cleanup reads a batch of expired locks and deletes it afterwards. A + * request whose clock was read before the expiry can refresh one of them in + * between, which reuses the same row; deleting that row by id would drop a + * lock that is valid again. + */ + public function testLockRefreshedAfterTheCleanupBatchWasReadSurvives(): void { + $this->setLockTimeoutMinutes(30); + $this->toTheFuture(0); + $t0 = $this->time; + $folder = $this->loginAndGetUserFolder(self::USER1); + $refreshedFile = $folder->newFile('cron-race.txt', 'AAA'); + $staleFile = $folder->newFile('cron-stale.txt', 'AAA'); + $lock = $this->lockManager->lock(new LockContext($refreshedFile, ILock::TYPE_USER, self::USER1)); + $this->lockManager->lock(new LockContext($staleFile, ILock::TYPE_USER, self::USER1)); + + // the background job wakes up after the expiry and reads its batch + $this->atTime($t0 + 1801); + $batch = $this->lockService()->getExpiredLocks(1000); + self::assertCount(2, $batch); + + // a request that read its clock a moment before the expiry refreshes one + // of them, updating the very row the job is about to delete + $this->atTime($t0 + 1799); + $refreshed = $this->lockManager->lock(new LockContext($refreshedFile, ILock::TYPE_USER, self::USER1)); + self::assertSame($lock->getId(), $refreshed->getId(), 'the refresh updated the same row'); + self::assertSame($t0 + 1799 + 1800, $refreshed->getExpiresAt()); + + // the job now deletes the batch it read earlier + $this->atTime($t0 + 1802); + $this->lockService()->removeLocksIfExpired($batch); + + self::assertSame(1, $this->lockRowCount($refreshedFile->getId()), 'the refreshed lock survives its own cleanup batch'); + self::assertSame(0, $this->lockRowCount($staleFile->getId()), 'a genuinely expired lock is still removed'); + } + + /** + * expires_at is the last second the lock is invalid, not the last second it + * is valid, and every layer has to agree on that. + */ + public function testTheExpirySecondItselfCountsAsExpired(): void { + $this->setLockTimeoutMinutes(10); + $this->toTheFuture(0); + $t0 = $this->time; + $folder = $this->loginAndGetUserFolder(self::USER1); + $file = $folder->newFile('boundary.txt', 'AAA'); + $other = $folder->newFile('boundary-2.txt', 'AAA'); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + $this->lockManager->lock(new LockContext($other, ILock::TYPE_USER, self::USER1)); + + // one second earlier nothing is expired anywhere + $this->atTime($t0 + 599); + self::assertSame([], $this->lockService()->getExpiredLocks(), 'not expired one second early'); + self::assertCount(1, $this->lockManager->getLocks($file->getId())); + + // on the second itself the query, the targeted delete and the model agree + $this->atTime($t0 + 600); + self::assertCount(2, $this->lockService()->getExpiredLocks(), 'the cleanup query includes the boundary second'); + self::assertTrue( + \OCP\Server::get(LocksRequest::class)->removeExpired($other->getId(), $t0 + 600), + 'the targeted delete includes the boundary second' + ); + self::assertSame([], $this->lockManager->getLocks($file->getId()), 'the model treats the boundary second as expired'); + self::assertSame(0, $this->lockRowCount($file->getId()), 'and reading it away removed the row'); + } + + public function testAcquireWithExplicitTimeout(): void { + $this->setLockTimeoutMinutes(-1); + $this->toTheFuture(0); + $file = $this->loginAndGetUserFolder(self::USER1)->newFile('explicit.txt', 'AAA'); + $lock = $this->lockService()->acquire(new LockContext($file, ILock::TYPE_TOKEN, self::USER1), 600, 'native-token'); + self::assertSame($this->time + 600, $lock->getExpiresAt()); + self::assertSame('native-token', $lock->getToken()); + + $refreshed = $this->lockService()->acquire(new LockContext($file, ILock::TYPE_TOKEN, 'native-token'), FileLock::ETA_INFINITE); + self::assertSame($lock->getId(), $refreshed->getId()); + self::assertNull($refreshed->getExpiresAt()); + } +} diff --git a/tests/Feature/LifecycleTest.php b/tests/Feature/LifecycleTest.php new file mode 100644 index 00000000..f28d948b --- /dev/null +++ b/tests/Feature/LifecycleTest.php @@ -0,0 +1,171 @@ +loginAndGetUserFolder(self::USER1)->newFile('deleted.txt', 'AAA'); + $id = $file->getId(); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + self::assertSame(1, $this->lockRowCount($id)); + + $file->delete(); + self::assertSame(0, $this->lockRowCount($id)); + } + + /** + * Two mechanisms drop the lock of a deleted file: the storage wrapper and the + * event listener. Each has to work on its own, so that a change to one of them + * cannot leave a deleted file's lock behind. + */ + public function testStorageDeletionAloneRemovesTheLock(): void { + $file = $this->loginAndGetUserFolder(self::USER1)->newFile('storage-delete.txt', 'AAA'); + $id = $file->getId(); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + self::assertSame(1, $this->lockRowCount($id)); + + // straight to the storage, so no node event is dispatched for this deletion + self::assertTrue($file->getStorage()->unlink($file->getInternalPath())); + self::assertSame(0, $this->lockRowCount($id), 'the storage wrapper drops the lock by itself'); + } + + public function testNodeDeletedEventAloneRemovesTheLock(): void { + $file = $this->loginAndGetUserFolder(self::USER1)->newFile('event-delete.txt', 'AAA'); + $id = $file->getId(); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + self::assertSame(1, $this->lockRowCount($id)); + + // the file itself is left alone; only the event fires + \OCP\Server::get(IEventDispatcher::class)->dispatchTyped(new NodeDeletedEvent($file)); + self::assertSame(0, $this->lockRowCount($id), 'the listener drops the lock by itself'); + } + + /** + * Oracle refuses more than 1000 expressions in one IN list, so bulk removal + * has to be chunked. + */ + public function testBulkRemovalHandlesMoreIdsThanOneStatementAllows(): void { + $file = $this->loginAndGetUserFolder(self::USER1)->newFile('bulk-delete.txt', 'AAA'); + $id = $file->getId(); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + + $this->lockService()->removeLocksForFileIds(array_merge(range(900000, 901500), [$id])); + self::assertSame(0, $this->lockRowCount($id)); + } + + public function testRestoredFileIsNotLocked(): void { + $file = $this->loginAndGetUserFolder(self::USER1)->newFile('restored.txt', 'AAA'); + $id = $file->getId(); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + $file->delete(); + + $trashed = null; + foreach (Helper::getTrashFiles('/', self::USER1) as $item) { + if ($item->getName() === 'restored.txt') { + $trashed = $item; + } + } + if ($trashed === null) { + self::markTestSkipped('trash bin did not keep the file'); + } + self::assertTrue(Trashbin::restore('/restored.txt.d' . $trashed->getMtime(), 'restored.txt', $trashed->getMtime())); + + $this->lockService()->clearCache(); + $restored = $this->rootFolder->getUserFolder(self::USER1)->get('restored.txt'); + self::assertSame($id, $restored->getId()); + self::assertSame([], $this->lockManager->getLocks($id)); + self::assertSame(0, $this->lockRowCount($id)); + } + + /** + * Moving a file between storages copies it under a new id and deletes the + * source, so the lock must not survive as a row pointing at a file that is + * gone, and it must not follow onto a file nobody locked. + */ + public function testMovingALockedFileToAnotherStorageStrandsNoLock(): void { + $folder = $this->loginAndGetUserFolder(self::USER1); + $storage = new Temporary([]); + Filesystem::mount($storage, [], '/' . self::USER1 . '/files/ext/'); + $file = $folder->newFile('crossing.txt', 'AAA'); + $id = $file->getId(); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + + $file->move($folder->get('ext')->getPath() . '/crossing.txt'); + + $moved = $this->rootFolder->getUserFolder(self::USER1)->get('ext/crossing.txt'); + self::assertSame(0, $this->lockRowCount($id)); + self::assertSame(0, $this->lockRowCount($moved->getId())); + } + + public function testAnotherUserCannotMoveALockedFileOffItsStorage(): void { + $folder = $this->loginAndGetUserFolder(self::USER1); + $dir = $folder->newFolder('shared-tree'); + $file = $dir->newFile('held.txt', 'AAA'); + $this->shareWith($dir, self::USER1, self::USER2, 31); + $id = $file->getId(); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + + \OC_Util::setupFS(self::USER2); + \OC_User::setUserId(self::USER2); + $this->lockService()->clearCache(); + $storage = new Temporary([]); + Filesystem::mount($storage, [], '/' . self::USER2 . '/files/ext/'); + $theirs = $this->rootFolder->getUserFolder(self::USER2)->get('shared-tree/held.txt'); + + try { + $theirs->move($this->rootFolder->getUserFolder(self::USER2)->get('ext')->getPath() . '/held.txt'); + self::fail('a locked file should not leave its storage under another user'); + } catch (LockedException) { + } + self::assertSame(1, $this->lockRowCount($id)); + } + + public function testPurgingTheCacheEntryRemovesTheLock(): void { + $file = $this->loginAndGetUserFolder(self::USER1)->newFile('purged.txt', 'AAA'); + $id = $file->getId(); + $file->delete(); + + // a lock that somehow survived the deletion (created before this version) + $lock = new FileLock(); + $lock->setUserId(self::USER1)->setLockType(ILock::TYPE_USER)->setFileId($id)->setToken('files_lock/stale'); + \OCP\Server::get(LocksRequest::class)->save($lock); + self::assertSame(1, $this->lockRowCount($id)); + + $trashed = null; + foreach (Helper::getTrashFiles('/', self::USER1) as $item) { + if ($item->getName() === 'purged.txt') { + $trashed = $item; + } + } + if ($trashed === null) { + self::markTestSkipped('trash bin did not keep the file'); + } + Trashbin::delete('purged.txt', self::USER1, $trashed->getMtime()); + self::assertSame(0, $this->lockRowCount($id)); + } +} diff --git a/tests/Feature/LockFeatureTest.php b/tests/Feature/LockFeatureTest.php index 75a5047f..798dcea3 100644 --- a/tests/Feature/LockFeatureTest.php +++ b/tests/Feature/LockFeatureTest.php @@ -26,7 +26,6 @@ use OCP\Share\IShare; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\MockObject\MockObject; -use Sabre\DAV\Locks\LockInfo; use Sabre\DAV\PropFind; use Test\TestCase; use Test\Util\User\Dummy; @@ -41,7 +40,7 @@ class LockFeatureTest extends TestCase { * * @var list */ - private const TEST_FILES = [ + private const array TEST_FILES = [ 'test-file', 'test-file2', 'test-file3', @@ -50,11 +49,16 @@ class LockFeatureTest extends TestCase { 'test-file-creation-clock', 'test-file-dav-infinite', 'test-file-dav-expiring', + 'test-file-extend', + 'test-file-extend-infinite', + 'test-file-remove-lock', + 'test-file-token', 'test-file_public', 'test-file-client', 'etag_test', - 'test-expired-lock-is-deprecated', - 'test-expired-lock-is-deprecated-2', + 'test-expired-lock', + 'test-expired-lock-remove-1', + 'test-expired-lock-remove-2', ]; protected LockManager $lockManager; @@ -244,7 +248,7 @@ public function testExpiredLocksAreDeprecated(): void { // Travel past the 30m timeout window. $this->toTheFuture(30 * 60 + 1); $mapToIds = fn (ILock $deprecatedLock): int => $deprecatedLock->getId(); - $deprecated = array_map($mapToIds, $service->getDeprecatedLocks()); + $deprecated = array_map($mapToIds, $service->getExpiredLocks()); self::assertContains( $lock->getId(), @@ -280,7 +284,7 @@ public function testLockCreationUsesTheInjectedClock(): void { } // Use expired locks to model the cron cleanup workflow: - // getDeprecatedLocks() selects stale locks and removeLocks() deletes them. + // getExpiredLocks() selects stale locks and removeLocks() deletes them. public function testRemoveDeprecatedLocks(): void { $service = \OCP\Server::get(LockService::class); \OCP\Server::get(IConfig::class)->setAppValue(Application::APP_ID, ConfigLexicon::LOCK_TIMEOUT, 30); @@ -290,13 +294,13 @@ public function testRemoveDeprecatedLocks(): void { $lock2 = $this->lockManager->lock(new LockContext($file2, ILock::TYPE_USER, self::TEST_USER1)); $this->toTheFuture(30 * 60 + 1); $mapToIds = fn (ILock $lock): int => $lock->getId(); - $deprecated = array_map($mapToIds, $service->getDeprecatedLocks()); + $deprecated = array_map($mapToIds, $service->getExpiredLocks()); self::assertContains($lock1->getId(), $deprecated); self::assertContains($lock2->getId(), $deprecated); $service->removeLocks([$lock1, $lock2]); - $deprecated = array_map($mapToIds, $service->getDeprecatedLocks()); + $deprecated = array_map($mapToIds, $service->getExpiredLocks()); self::assertNotContains($lock1->getId(), $deprecated); self::assertNotContains($lock2->getId(), $deprecated); @@ -352,19 +356,6 @@ public function testInfiniteLockReportsNoExpiryOverWebdav(): void { self::assertSame(30 * 60, $this->davLockTimeout($expiring)); } - /** - * The standard {DAV:}timeout property has its own sentinel for a lock that - * never expires (RFC4918 renders it as "Infinite", which Sabre only emits - * for a timeout of exactly -1). Sending the raw negative internal lifetime - * there produced the invalid "Second--60"; -60 and 0 are what - * LockService::lock() actually stores for the two "never expires" configs. - */ - public function testInfiniteLockReportsStandardWebdavTimeoutAsInfinite(): void { - self::assertSame(LockInfo::TIMEOUT_INFINITE, (new FileLock(-60))->toLockInfo()->timeout); - self::assertSame(LockInfo::TIMEOUT_INFINITE, (new FileLock(0))->toLockInfo()->timeout); - self::assertSame(30 * 60, (new FileLock(30 * 60))->toLockInfo()->timeout); - } - public function testLockApp(): void { $file = $this->loginAndGetUserFolder(self::TEST_USER1) ->newFile('test-file2', 'AAA'); @@ -530,16 +521,17 @@ public function testUnlockStaleClientLock(): void { $this->assertCount(1, $locks); // Other users cannot unlock + $sharedFile = $this->loginAndGetUserFolder(self::TEST_USER2)->get('test-file-client'); try { - $this->lockManager->unlock(new LockContext($file, ILock::TYPE_TOKEN, self::TEST_USER2)); + $this->lockManager->unlock(new LockContext($sharedFile, ILock::TYPE_TOKEN, self::TEST_USER2)); $locks = []; } catch (\OCP\PreConditionNotMetException) { $locks = $this->lockManager->getLocks($file->getId()); } $this->assertCount(1, $locks); - // The owner can stil force unlock it as done through the OCS controller - \OCP\Server::get(\OCA\FilesLock\Service\LockService::class)->enableUserOverride(); + // The owner can still unlock it, the override is part of the policy on every path + $file = $this->loginAndGetUserFolder(self::TEST_USER1)->get('test-file-client'); $this->lockManager->unlock(new LockContext($file, ILock::TYPE_USER, self::TEST_USER1)); $locks = $this->lockManager->getLocks($file->getId()); @@ -618,8 +610,13 @@ private function toTheFuture(int $seconds): void { } private function deleteTestFiles(\OCP\Files\Folder $folder): void { + $lockService = \OCP\Server::get(LockService::class); + $lockService->removeLocks(\OCP\Server::get(\OCA\FilesLock\Db\LocksRequest::class)->getAll()); foreach (self::TEST_FILES as $filename) { - $folder->delete($filename); + try { + $folder->get($filename)->delete(); + } catch (\OCP\Files\NotFoundException) { + } } } diff --git a/tests/Feature/MigrationTest.php b/tests/Feature/MigrationTest.php new file mode 100644 index 00000000..aadde4b1 --- /dev/null +++ b/tests/Feature/MigrationTest.php @@ -0,0 +1,132 @@ +connection = \OCP\Server::get(IDBConnection::class); + } + + protected function tearDown(): void { + $this->runMigration(); + parent::tearDown(); + } + + private function schemaWrapper(): SchemaWrapper { + /** @var ConnectionAdapter $adapter */ + $adapter = $this->connection; + return new SchemaWrapper($adapter->getInner()); + } + + private function runMigration(): void { + $output = $this->createMock(IOutput::class); + $schemaClosure = fn (): SchemaWrapper => $this->schemaWrapper(); + $step1 = new Version36000Date20260906120000($this->connection); + $step1->preSchemaChange($output, $schemaClosure, []); + $schema = $step1->changeSchema($output, $schemaClosure, []); + if ($schema !== null) { + $this->connection->migrateToSchema($schema->getWrappedSchema()); + } + $step1->postSchemaChange($output, $schemaClosure, []); + $step2 = new Version36000Date20260906120100(); + $schema = $step2->changeSchema($output, $schemaClosure, []); + if ($schema !== null) { + $this->connection->migrateToSchema($schema->getWrappedSchema()); + } + } + + /** + * Bring the table back to the shape of the previous release. + */ + private function downgradeSchema(): void { + $schema = $this->schemaWrapper(); + $table = $schema->getTable(LocksRequest::TABLE_LOCKS); + foreach ([Version36000Date20260906120000::INDEX_FILE_ID, Version36000Date20260906120000::INDEX_EXPIRES] as $index) { + if ($table->hasIndex($index)) { + $table->dropIndex($index); + } + } + if ($table->hasColumn('expires_at')) { + $table->dropColumn('expires_at'); + } + $table->addIndex(['file_id'], 'files_lock_old_file_id'); + $this->connection->migrateToSchema($schema->getWrappedSchema()); + } + + private function insertLegacyRow(int $fileId, string $user, string $token, int $creation, int $ttl): int { + $qb = $this->connection->getQueryBuilder(); + $qb->insert(LocksRequest::TABLE_LOCKS) + ->setValue('user_id', $qb->createNamedParameter($user)) + ->setValue('file_id', $qb->createNamedParameter($fileId, IQueryBuilder::PARAM_INT)) + ->setValue('token', $qb->createNamedParameter($token)) + ->setValue('creation', $qb->createNamedParameter($creation, IQueryBuilder::PARAM_INT)) + ->setValue('type', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)) + ->setValue('ttl', $qb->createNamedParameter($ttl, IQueryBuilder::PARAM_INT)) + ->setValue('owner', $qb->createNamedParameter($user)); + $qb->executeStatement(); + return $qb->getLastInsertId(); + } + + public function testDuplicatesAreReconciledBeforeUniquenessIsEnforced(): void { + $this->downgradeSchema(); + $table = $this->schemaWrapper()->getTable(LocksRequest::TABLE_LOCKS); + self::assertFalse($table->hasColumn('expires_at')); + self::assertFalse($table->hasIndex(Version36000Date20260906120000::INDEX_FILE_ID)); + + $creation = 1700000000; + $this->insertLegacyRow(4242, 'alice', 'files_lock/a', $creation, 1800); + $this->insertLegacyRow(4242, 'bob', 'files_lock/b', $creation + 10, 1800); + $keep = $this->insertLegacyRow(4242, 'carol', 'files_lock/c', $creation + 20, 0); + $finite = $this->insertLegacyRow(4243, 'dave', 'files_lock/d', $creation, 600); + $infinite = $this->insertLegacyRow(4244, 'erin', 'files_lock/e', $creation, -60); + self::assertSame(3, $this->lockRowCount(4242)); + + $this->runMigration(); + + self::assertSame(1, $this->lockRowCount(4242)); + $request = \OCP\Server::get(LocksRequest::class); + $survivor = $request->getFromFileId(4242); + self::assertSame($keep, $survivor->getId(), 'the most recently created row wins'); + self::assertNull($survivor->getExpiresAt(), 'ttl 0 stays infinite'); + self::assertSame($creation + 600, $request->getFromFileId(4243)->getExpiresAt(), 'creation + ttl becomes the absolute expiry'); + self::assertNull($request->getFromFileId(4244)->getExpiresAt(), 'negative ttl stays infinite'); + self::assertSame($finite, $request->getFromFileId(4243)->getId()); + self::assertSame($infinite, $request->getFromFileId(4244)->getId()); + + $table = $this->schemaWrapper()->getTable(LocksRequest::TABLE_LOCKS); + self::assertTrue($table->hasColumn('expires_at')); + self::assertTrue($table->hasIndex(Version36000Date20260906120000::INDEX_FILE_ID)); + self::assertTrue($table->getIndex(Version36000Date20260906120000::INDEX_FILE_ID)->isUnique()); + self::assertTrue($table->hasIndex(Version36000Date20260906120000::INDEX_EXPIRES)); + self::assertFalse($table->hasIndex('files_lock_old_file_id'), 'the old non-unique index is gone'); + + // running it again is a no-op + $this->runMigration(); + self::assertSame(1, $this->lockRowCount(4242)); + } +} diff --git a/tests/Feature/OcsControllerTest.php b/tests/Feature/OcsControllerTest.php index b112ecfe..cb2d82a6 100644 --- a/tests/Feature/OcsControllerTest.php +++ b/tests/Feature/OcsControllerTest.php @@ -48,6 +48,10 @@ public function testLockUnlockRoundTrip(): void { self::assertSame(self::USER1, $body['ocs']['data']['userId']); self::assertSame(ILock::TYPE_USER, $body['ocs']['data']['type']); self::assertStringStartsWith('files_lock/', $body['ocs']['data']['token']); + self::assertSame(-1, $body['ocs']['data']['eta']); + + [$status, $body] = $this->render($this->controller()->locking((string)$file->getId())); + self::assertSame(Http::STATUS_OK, $status, 'locking again refreshes'); [$status] = $this->render($this->controller()->unlocking((string)$file->getId())); self::assertSame(Http::STATUS_OK, $status); @@ -70,15 +74,70 @@ public function testConflictIsAStructured423(): void { self::assertSame(self::USER1, $body['ocs']['data']['userId']); self::assertSame($file->getId(), $body['ocs']['data']['fileId']); self::assertStringContainsString('locked by', $body['ocs']['meta']['message']); + self::assertArrayNotHasKey('token', $body['ocs']['data'], 'the conflict payload must not hand out the lock token'); } else { self::assertStringContainsString('' . self::USER1 . '', $body); self::assertStringContainsString('423', $body); + self::assertStringNotContainsString('', $body, 'the conflict payload must not hand out the lock token'); } } [$status, $body] = $this->render($this->controller()->unlocking((string)$file->getId())); self::assertSame(Http::STATUS_LOCKED, $status, 'a recipient may not release the owner lock'); self::assertSame(self::USER1, $body['ocs']['data']['userId']); + self::assertArrayNotHasKey('token', $body['ocs']['data'], 'nor may the failed unlock hand it out'); self::assertSame(1, $this->lockRowCount($file->getId())); } + + public function testFileOwnerReleasesRecipientLock(): void { + $file = $this->sharedFile('override.txt'); + $this->loginAndGetUserFolder(self::USER2); + [$status] = $this->render($this->controller()->locking((string)$file->getId())); + self::assertSame(Http::STATUS_OK, $status); + + $this->loginAndGetUserFolder(self::USER1); + [$status] = $this->render($this->controller()->unlocking((string)$file->getId())); + self::assertSame(Http::STATUS_OK, $status); + self::assertSame(0, $this->lockRowCount($file->getId())); + } + + public function testLockTypeIsHonouredOnUnlock(): void { + $file = $this->sharedFile('typed.txt'); + $this->loginAndGetUserFolder(self::USER2); + [$status, $body] = $this->render($this->controller()->locking((string)$file->getId(), ILock::TYPE_APP)); + self::assertSame(Http::STATUS_OK, $status); + self::assertSame(ILock::TYPE_APP, $body['ocs']['data']['type']); + self::assertSame(self::USER2, $body['ocs']['data']['userId'], 'the session user is the recorded owner'); + + [$status] = $this->render($this->controller()->unlocking((string)$file->getId(), ILock::TYPE_APP)); + self::assertSame(Http::STATUS_OK, $status, 'the recorded owner releases through the same path'); + + [$status, $body] = $this->render($this->controller()->locking((string)$file->getId(), ILock::TYPE_TOKEN)); + self::assertSame(Http::STATUS_OK, $status); + self::assertSame(ILock::TYPE_TOKEN, $body['ocs']['data']['type']); + [$status] = $this->render($this->controller()->unlocking((string)$file->getId())); + self::assertSame(Http::STATUS_OK, $status, 'the recorded owner releases a token lock without the token'); + } + + public function testClientErrors(): void { + $folder = $this->loginAndGetUserFolder(self::USER1)->newFolder('folder'); + [$status] = $this->render($this->controller()->locking('999999999')); + self::assertSame(Http::STATUS_NOT_FOUND, $status); + [$status] = $this->render($this->controller()->unlocking('999999999')); + self::assertSame(Http::STATUS_NOT_FOUND, $status); + [$status] = $this->render($this->controller()->locking((string)$folder->getId())); + self::assertSame(Http::STATUS_BAD_REQUEST, $status); + [$status] = $this->render($this->controller()->locking((string)$folder->getId(), 99)); + self::assertSame(Http::STATUS_BAD_REQUEST, $status); + [$status] = $this->render($this->controller()->locking((string)$folder->getId(), -1)); + self::assertSame(Http::STATUS_BAD_REQUEST, $status); + [$status] = $this->render($this->controller()->locking('abc')); + self::assertSame(Http::STATUS_BAD_REQUEST, $status); + + $file = $this->sharedFile('readonly.txt', 1); + $this->loginAndGetUserFolder(self::USER2); + [$status, $body] = $this->render($this->controller()->locking((string)$file->getId())); + self::assertSame(Http::STATUS_FORBIDDEN, $status); + self::assertSame(0, $this->lockRowCount($file->getId())); + } } diff --git a/tests/Feature/UnlockPolicyTest.php b/tests/Feature/UnlockPolicyTest.php new file mode 100644 index 00000000..1a29b986 --- /dev/null +++ b/tests/Feature/UnlockPolicyTest.php @@ -0,0 +1,215 @@ +lockService()->clearCache(); + } + + private function assertRefused(callable $unlock, int $fileId): void { + try { + $unlock(); + self::fail('unlock should have been refused'); + } catch (UnauthorizedUnlockException|PreConditionNotMetException) { + } + self::assertSame(1, $this->lockRowCount($fileId)); + } + + /** + * @return array{File, File, File} as USER1 (file owner), USER2 (writer), USER3 (writer) + */ + private function sharedViews(string $name): array { + $owner = $this->sharedFile($name, 19, 19); + \OC_Util::setupFS(self::USER2); + \OC_Util::setupFS(self::USER3); + return [ + $owner, + $this->rootFolder->getUserFolder(self::USER2)->get($name), + $this->rootFolder->getUserFolder(self::USER3)->get($name), + ]; + } + + public function testUserLockRelease(): void { + [$owner, $creator, $other] = $this->sharedViews('user.txt'); + $this->actAs(self::USER2); + $this->lockManager->lock(new LockContext($creator, ILock::TYPE_USER, self::USER2)); + $id = $owner->getId(); + + $this->actAs(self::USER3); + $this->assertRefused(fn () => $this->lockManager->unlock(new LockContext($other, ILock::TYPE_USER, self::USER3)), $id); + + $this->actAs(self::USER2); + $this->lockManager->unlock(new LockContext($creator, ILock::TYPE_USER, self::USER2)); + self::assertSame(0, $this->lockRowCount($id)); + + $this->lockManager->lock(new LockContext($creator, ILock::TYPE_USER, self::USER2)); + $this->actAs(self::USER1); + $this->lockManager->unlock(new LockContext($owner, ILock::TYPE_USER, self::USER1)); + self::assertSame(0, $this->lockRowCount($id), 'the file owner overrides a user lock on a file of their home storage'); + } + + public function testTokenLockRelease(): void { + [$owner, $creator, $other] = $this->sharedViews('token.txt'); + $this->actAs(self::USER2); + $lock = $this->lockService()->acquire(new LockContext($creator, ILock::TYPE_TOKEN, self::USER2), null, self::TOKEN); + $id = $owner->getId(); + + $this->actAs(self::USER3); + $this->assertRefused(fn () => $this->lockManager->unlock(new LockContext($other, ILock::TYPE_TOKEN, self::USER3)), $id); + + // the recorded owner releases through the public API without the token (N-01) + $this->actAs(self::USER2); + $this->lockManager->unlock(new LockContext($creator, ILock::TYPE_TOKEN, self::USER2)); + self::assertSame(0, $this->lockRowCount($id)); + + // whoever presents the token releases (RFC 4918 section 6.5 semantics of the native path) + $this->lockService()->acquire(new LockContext($creator, ILock::TYPE_TOKEN, self::USER2), null, self::TOKEN); + $this->actAs(self::USER3); + $this->lockManager->unlock(new LockContext($other, ILock::TYPE_TOKEN, $lock->getToken())); + self::assertSame(0, $this->lockRowCount($id)); + + $this->actAs(self::USER2); + $this->lockService()->acquire(new LockContext($creator, ILock::TYPE_TOKEN, self::USER2), null, self::TOKEN); + $this->actAs(self::USER3); + $this->lockService()->unlock(new LockContext($other, ILock::TYPE_TOKEN, self::USER3), false, $lock->getToken()); + self::assertSame(0, $this->lockRowCount($id)); + + // the file owner overrides a stale client lock + $this->actAs(self::USER2); + $this->lockService()->acquire(new LockContext($creator, ILock::TYPE_TOKEN, self::USER2), null, self::TOKEN); + $this->actAs(self::USER1); + $this->lockManager->unlock(new LockContext($owner, ILock::TYPE_USER, self::USER1)); + self::assertSame(0, $this->lockRowCount($id)); + } + + /** + * The token is publicly readable, so it cannot be the whole authorization: + * a user who may not write the file could never have taken the lock and may + * not release it either. + */ + public function testTokenLockNeedsWritePermissionToRelease(): void { + $owner = $this->sharedFile('token-readonly.txt', 19, 1); + \OC_Util::setupFS(self::USER2); + \OC_Util::setupFS(self::USER3); + $id = $owner->getId(); + + $this->actAs(self::USER2); + $writerView = $this->rootFolder->getUserFolder(self::USER2)->get('token-readonly.txt'); + $lock = $this->lockService()->acquire(new LockContext($writerView, ILock::TYPE_TOKEN, self::USER2), null, self::TOKEN); + + // the read-only recipient can see the token but may not act on it + $this->actAs(self::USER3); + $readerView = $this->rootFolder->getUserFolder(self::USER3)->get('token-readonly.txt'); + $this->assertRefused( + fn (): FileLock => $this->lockService()->unlock(new LockContext($readerView, ILock::TYPE_TOKEN, self::USER3), false, $lock->getToken()), + $id + ); + + // someone who may write the file and presents the token still releases it + $this->actAs(self::USER2); + $this->lockService()->unlock(new LockContext($writerView, ILock::TYPE_TOKEN, self::USER3), false, $lock->getToken()); + self::assertSame(0, $this->lockRowCount($id)); + } + + public function testAppLockRelease(): void { + [$owner, $writer] = $this->sharedViews('app.txt'); + $this->lockManager->lock(new LockContext($owner, ILock::TYPE_APP, 'text')); + $id = $owner->getId(); + + $this->actAs(self::USER2); + $this->assertRefused(fn () => $this->lockManager->unlock(new LockContext($writer, ILock::TYPE_USER, self::USER2)), $id); + $this->assertRefused(fn () => $this->lockManager->unlock(new LockContext($writer, ILock::TYPE_APP, 'other')), $id); + + $this->lockManager->unlock(new LockContext($writer, ILock::TYPE_APP, 'text')); + self::assertSame(0, $this->lockRowCount($id)); + + $this->lockManager->lock(new LockContext($owner, ILock::TYPE_APP, 'text')); + $this->actAs(self::USER1); + $this->lockManager->unlock(new LockContext($owner, ILock::TYPE_USER, self::USER1)); + self::assertSame(0, $this->lockRowCount($id), 'the file owner overrides an app lock'); + } + + public function testOwnerOverrideIsLimitedToHomeStorage(): void { + $storage = new OwnerIsViewerStorage(['storage' => new Temporary([])]); + $this->loginAndGetUserFolder(self::USER1); + \OC_Util::setupFS(self::USER2); + Filesystem::mount($storage, [], '/' . self::USER1 . '/files/shared-mount/'); + Filesystem::mount($storage, [], '/' . self::USER2 . '/files/shared-mount/'); + $file = $this->rootFolder->getUserFolder(self::USER2)->get('shared-mount')->newFile('doc.txt', 'AAA'); + + $this->actAs(self::USER2); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER2)); + + $this->actAs(self::USER1); + $asUser1 = $this->rootFolder->getUserFolder(self::USER1)->get('shared-mount/doc.txt'); + self::assertSame(self::USER1, $asUser1->getOwner()?->getUID(), 'the mount reports the viewer as owner'); + $this->assertRefused(fn () => $this->lockManager->unlock(new LockContext($asUser1, ILock::TYPE_USER, self::USER1)), $file->getId()); + + $this->lockService()->forceUnlock($file->getId()); + self::assertSame(0, $this->lockRowCount($file->getId())); + } + + public function testForcedUnlockDoesNotNeedOwnerAccess(): void { + [$owner, $creator] = $this->sharedViews('force.txt'); + $this->actAs(self::USER2); + $this->lockManager->lock(new LockContext($creator, ILock::TYPE_USER, self::USER2)); + + $shareManager = \OCP\Server::get(IShareManager::class); + foreach ($shareManager->getSharesBy(self::USER1, \OCP\Share\IShare::TYPE_USER, $owner) as $share) { + $shareManager->deleteShare($share); + } + $this->actAs(''); + + $removed = $this->lockService()->unlockFile($owner->getId(), '', true); + self::assertSame(self::USER2, $removed->getOwner()); + self::assertSame(0, $this->lockRowCount($owner->getId())); + + $this->expectException(LockNotFoundException::class); + $this->lockService()->forceUnlock($owner->getId()); + } + + public function testUnlockWithoutLockReportsNotFound(): void { + $file = $this->loginAndGetUserFolder(self::USER1)->newFile('none.txt', 'AAA'); + $this->expectException(PreConditionNotMetException::class); + $this->lockManager->unlock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + } +} diff --git a/tests/Feature/WritePolicyTest.php b/tests/Feature/WritePolicyTest.php new file mode 100644 index 00000000..48dedd8e --- /dev/null +++ b/tests/Feature/WritePolicyTest.php @@ -0,0 +1,186 @@ +setUser(null); + } else { + \OC_User::setUserId($userId); + } + $this->lockService()->clearCache(); + } + + private function assertBlocked(File $file): void { + try { + $file->putContent('blocked'); + self::fail('write should have been blocked by the lock'); + } catch (ManuallyLockedException) { + } + } + + private function assertWritable(File $file, string $content): void { + $file->putContent($content); + self::assertSame($content, $file->getContent()); + } + + /** + * @return array{File, File, File} the file as seen by USER1 (owner), USER2 (writer) and USER3 (read-only) + */ + private function homeFile(string $name): array { + $owner = $this->sharedFile($name, 19, 1); + \OC_Util::setupFS(self::USER2); + \OC_Util::setupFS(self::USER3); + $writer = $this->rootFolder->getUserFolder(self::USER2)->get($name); + $reader = $this->rootFolder->getUserFolder(self::USER3)->get($name); + return [$owner, $writer, $reader]; + } + + /** + * @return array{File, File} the file as seen by USER1 and USER2 on a shared non-home mount + */ + private function mountedFile(string $name): array { + $storage = new Temporary([]); + $this->loginAndGetUserFolder(self::USER1); + \OC_Util::setupFS(self::USER2); + Filesystem::mount($storage, [], '/' . self::USER1 . '/files/ext/'); + Filesystem::mount($storage, [], '/' . self::USER2 . '/files/ext/'); + $file = $this->rootFolder->getUserFolder(self::USER1)->get('ext')->newFile($name, 'AAA'); + $other = $this->rootFolder->getUserFolder(self::USER2)->get('ext/' . $name); + self::assertSame($file->getId(), $other->getId()); + return [$file, $other]; + } + + public function testUserLockOnHomeFile(): void { + [$owner, $writer, $reader] = $this->homeFile('user-home.txt'); + $this->lockManager->lock(new LockContext($owner, ILock::TYPE_USER, self::USER1)); + + $this->actAs(self::USER1); + $this->assertWritable($owner, 'owner'); + $this->actAs(self::USER2); + $this->assertBlocked($writer); + $this->actAs(self::USER3); + $this->expectException(NotPermittedException::class); + $reader->putContent('reader'); + } + + public function testUserLockOnNonHomeMount(): void { + [$file, $other] = $this->mountedFile('user-ext.txt'); + $this->lockManager->lock(new LockContext($file, ILock::TYPE_USER, self::USER1)); + + $this->actAs(self::USER2); + self::assertCount(1, $this->lockManager->getLocks($other->getId())); + $this->assertBlocked($other); + $this->actAs(self::USER1); + $this->assertWritable($file, 'owner'); + } + + public function testAppLockOnHomeAndMount(): void { + [$owner, $writer] = $this->homeFile('app-home.txt'); + $scope = new LockContext($owner, ILock::TYPE_APP, 'text'); + $this->lockManager->lock($scope); + + $this->actAs(self::USER1); + $this->assertBlocked($owner); + $this->actAs(self::USER2); + $this->assertBlocked($writer); + $this->lockManager->runInScope($scope, fn () => $this->assertWritable($writer, 'in scope')); + $this->lockManager->runInScope(new LockContext($owner, ILock::TYPE_APP, 'other'), fn () => $this->assertBlocked($writer)); + + [$file, $other] = $this->mountedFile('app-ext.txt'); + $scope = new LockContext($file, ILock::TYPE_APP, 'text'); + $this->lockManager->lock($scope); + $this->actAs(self::USER2); + $this->assertBlocked($other); + $this->lockManager->runInScope($scope, fn () => $this->assertWritable($other, 'in scope')); + } + + public function testTokenLockRequiresTokenAndPrincipal(): void { + [$owner, $writer, $reader] = $this->homeFile('token-home.txt'); + $this->actAs(self::USER1); + $lock = $this->lockService()->acquire(new LockContext($owner, ILock::TYPE_TOKEN, self::USER1), null, self::TOKEN); + $tokenScope = new LockContext($owner, ILock::TYPE_TOKEN, $lock->getToken()); + + // creator without the token + $this->assertBlocked($owner); + // creator presenting the token through the lock scope + $this->lockManager->runInScope($tokenScope, fn () => $this->assertWritable($owner, 'creator with token')); + // creator presenting the token the way the WebDAV adapter does + $this->lockService()->presentToken($lock->getToken()); + $this->assertWritable($owner, 'creator via presented token'); + + // another writer, with and without the token + $this->actAs(self::USER2); + $this->assertBlocked($writer); + $this->lockManager->runInScope($tokenScope, fn () => $this->assertBlocked($writer)); + + // file owner is the creator here; a read-only user is stopped by permissions + $this->actAs(self::USER3); + try { + $reader->putContent('reader'); + self::fail('read-only user must not write'); + } catch (NotPermittedException) { + } + + // a sessionless trusted caller presenting the token acts for the creator + $this->actAs(''); + $this->lockManager->runInScope($tokenScope, fn () => $this->assertWritable($owner, 'internal with token')); + $this->assertBlocked($owner); + } + + public function testTokenLockCreatedByRecipientBlocksTheFileOwner(): void { + [$owner, $writer] = $this->homeFile('token-recipient.txt'); + $this->actAs(self::USER2); + $lock = $this->lockService()->acquire(new LockContext($writer, ILock::TYPE_TOKEN, self::USER2), null, self::TOKEN); + $tokenScope = new LockContext($writer, ILock::TYPE_TOKEN, $lock->getToken()); + + $this->actAs(self::USER1); + $this->assertBlocked($owner); + $this->lockManager->runInScope($tokenScope, fn () => $this->assertBlocked($owner)); + + $this->actAs(self::USER2); + $this->lockManager->runInScope($tokenScope, fn () => $this->assertWritable($writer, 'recipient with token')); + } + + public function testTokenLockOnNonHomeMount(): void { + [$file, $other] = $this->mountedFile('token-ext.txt'); + $this->actAs(self::USER1); + $lock = $this->lockService()->acquire(new LockContext($file, ILock::TYPE_TOKEN, self::USER1), null, self::TOKEN); + $tokenScope = new LockContext($file, ILock::TYPE_TOKEN, $lock->getToken()); + + $this->actAs(self::USER2); + $this->assertBlocked($other); + $this->lockManager->runInScope($tokenScope, fn () => $this->assertBlocked($other)); + $this->actAs(self::USER1); + $this->assertBlocked($file); + $this->lockManager->runInScope($tokenScope, fn () => $this->assertWritable($file, 'creator with token')); + } +} diff --git a/tests/Feature/fixtures/concurrent-lock.php b/tests/Feature/fixtures/concurrent-lock.php new file mode 100644 index 00000000..0d92ed82 --- /dev/null +++ b/tests/Feature/fixtures/concurrent-lock.php @@ -0,0 +1,50 @@ + + * Prints LOCKED, CONFLICT or ERROR
after racing ILockManager::lock(). + */ + +use OCP\Files\IRootFolder; +use OCP\Files\Lock\ILock; +use OCP\Files\Lock\ILockManager; +use OCP\Files\Lock\LockContext; +use OCP\Files\Lock\OwnerLockedException; + +define('OC_CONSOLE', 1); +require_once __DIR__ . '/../../../../../lib/base.php'; +while (ob_get_level() > 0) { + ob_end_clean(); +} + +[, $userId, $fileId, $startAt] = $argv; + +try { + \OC_User::setUserId($userId); + \OC_Util::setupFS($userId); + $node = \OCP\Server::get(IRootFolder::class)->getUserFolder($userId)->getFirstNodeById((int)$fileId); + if ($node === null) { + fwrite(STDOUT, "ERROR file not found\n"); + exit(1); + } + $lockManager = \OCP\Server::get(ILockManager::class); + while (microtime(true) < (float)$startAt) { + usleep(100); + } + $lockManager->lock(new LockContext($node, ILock::TYPE_USER, $userId)); + fwrite(STDOUT, "LOCKED\n"); +} catch (OwnerLockedException) { + fwrite(STDOUT, "CONFLICT\n"); +} catch (\Throwable $e) { + fwrite(STDOUT, 'ERROR ' . $e::class . ': ' . $e->getMessage() . "\n"); + exit(1); +} diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index 7b4cfb5f..5f03c12b 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -2,12 +2,11 @@ - - +