Correctly validate lock state

Signed-off-by: Louis Chemineau <louis@chmn.me>
nextcloud · Jan 16, 2024 · 33d0c1d · 33d0c1d
1 parent 8ff567b
commit 33d0c1d
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 9 deletions.
diff --git a/lib/Connector/Sabre/LockPlugin.php b/lib/Connector/Sabre/LockPlugin.php
@@ -162,7 +162,7 @@ protected function verifyTokenOnWriteAccess(INode $node, ?string $token): void {
 			throw new Forbidden('Write access to end-to-end encrypted folder requires token - no token sent');
 		}
 
-		if ($this->lockManager->isLocked($node->getId(), $token)) {
+		if ($this->lockManager->isLocked($node->getId(), $token, null, true)) {
 			throw new FileLocked('Write access to end-to-end encrypted folder requires token - resource not locked or wrong token sent', Http::STATUS_FORBIDDEN);
 		}
 	}

diff --git a/lib/Controller/MetaDataController.php b/lib/Controller/MetaDataController.php
@@ -120,7 +120,7 @@ public function setMetaData(int $id, string $metaData): DataResponse {
 			throw new OCSPreconditionFailedException($this->l10n->t('X-NC-E2EE-SIGNATURE is empty'));
 		}
 
-		if ($this->lockManager->isLocked($id, $e2eToken)) {
+		if ($this->lockManager->isLocked($id, $e2eToken, null, true)) {
 			throw new OCSForbiddenException($this->l10n->t('You are not allowed to edit the file, make sure to first lock it, and then send the right token'));
 		}
 
@@ -159,7 +159,7 @@ public function updateMetaData(int $id, string $metaData): DataResponse {
 			throw new OCSPreconditionFailedException($this->l10n->t('X-NC-E2EE-SIGNATURE is empty'));
 		}
 
-		if ($this->lockManager->isLocked($id, $e2eToken)) {
+		if ($this->lockManager->isLocked($id, $e2eToken, null, true)) {
 			throw new OCSForbiddenException($this->l10n->t('You are not allowed to edit the file, make sure to first lock it, and then send the right token'));
 		}
 
@@ -196,7 +196,7 @@ public function deleteMetaData(int $id): DataResponse {
 			throw new OCSPreconditionFailedException($this->l10n->t('e2e-token is empty'));
 		}
 
-		if ($this->lockManager->isLocked($id, $e2eToken)) {
+		if ($this->lockManager->isLocked($id, $e2eToken, null, true)) {
 			throw new OCSForbiddenException($this->l10n->t('You are not allowed to edit the file, make sure to first lock it, and then send the right token'));
 		}
 
@@ -232,7 +232,7 @@ public function addMetadataFileDrop(int $id, string $filedrop, ?string $shareTok
 			throw new OCSPreconditionFailedException($this->l10n->t('e2e-token is empty'));
 		}
 
-		if ($this->lockManager->isLocked($id, $e2eToken, $ownerId)) {
+		if ($this->lockManager->isLocked($id, $e2eToken, $ownerId, true)) {
 			throw new OCSForbiddenException($this->l10n->t('You are not allowed to edit the file, make sure to first lock it, and then send the right token'));
 		}
 

diff --git a/lib/Controller/V1/MetaDataController.php b/lib/Controller/V1/MetaDataController.php
@@ -136,7 +136,7 @@ public function updateMetaData(int $id, string $metaData): DataResponse {
 		}
 		// End
 
-		if ($this->lockManager->isLocked($id, $e2eToken)) {
+		if ($this->lockManager->isLocked($id, $e2eToken, null, true)) {
 			throw new OCSForbiddenException($this->l10n->t('You are not allowed to edit the file, make sure to first lock it, and then send the right token'));
 		}
 
@@ -195,7 +195,7 @@ public function addMetadataFileDrop(int $id, string $fileDrop, ?string $shareTok
 		$e2eToken = $this->request->getParam('e2e-token');
 		$ownerId = $this->getOwnerId($shareToken);
 
-		if ($this->lockManager->isLocked($id, $e2eToken, $ownerId)) {
+		if ($this->lockManager->isLocked($id, $e2eToken, $ownerId, true)) {
 			throw new OCSForbiddenException($this->l10n->t('You are not allowed to edit the file, make sure to first lock it, and then send the right token'));
 		}
 

diff --git a/lib/LockManager.php b/lib/LockManager.php
@@ -125,7 +125,7 @@ public function unlockFile(int $id, string $token): void {
 	 * @throws NotFoundException
 	 * @throws \OCP\Files\NotPermittedException
 	 */
-	public function isLocked(int $id, string $token, ?string $ownerId = null): bool {
+	public function isLocked(int $id, string $token, ?string $ownerId = null, bool $requireLock = false): bool {
 		if ($ownerId === null) {
 			$user = $this->userSession->getUser();
 			if ($user === null) {
@@ -134,6 +134,8 @@ public function isLocked(int $id, string $token, ?string $ownerId = null): bool
 			$ownerId = $user->getUid();
 		}
 
+		$lockedByGivenToken = false;
+
 		$userRoot = $this->rootFolder->getUserFolder($ownerId);
 		$nodes = $userRoot->getById($id);
 		foreach ($nodes as $node) {
@@ -149,14 +151,16 @@ public function isLocked(int $id, string $token, ?string $ownerId = null): bool
 				// If it's locked with a different token, return true
 				if ($lock->getToken() !== $token) {
 					return true;
+				} else {
+					$lockedByGivenToken = true;
 				}
 
 				// If it's locked with the expected token, check the parent node
 				$node = $node->getParent();
 			}
 		}
 
-		return false;
+		return $requireLock && !$lockedByGivenToken;
 	}