diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 0d8a7528dd..ca230682be 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -357,7 +357,7 @@ jobs: if: always() - name: phpunit-oci-summary + name: phpunit-integration-summary steps: - name: Summary status diff --git a/lib/Controller/WopiController.php b/lib/Controller/WopiController.php index 60ee8d393f..607cf035cb 100644 --- a/lib/Controller/WopiController.php +++ b/lib/Controller/WopiController.php @@ -879,6 +879,21 @@ private function normalizePath(string $path, ?string $parent = null): string { return $parent . $path; } + /** + * Acquire a server-side lock for the current WOPI file. + * + * This method uses the file lock manager to enforce file-level exclusivity. + * The client-provided WOPI lock token is currently ignored. + * + * If the file is already manually locked by the same editor user, the request + * is still allowed so WOPI write flows can continue. + * + * Returns: + * - 200 on success + * - 400 if the lock provider is unavailable or the lock precondition fails + * - 423 if another user owns a conflicting lock + * - 500 for unexpected failures + */ private function lock(Wopi $wopi, string $lock): JSONResponse { try { $lock = $this->lockManager->lock(new LockContext( @@ -890,19 +905,36 @@ private function lock(Wopi $wopi, string $lock): JSONResponse { } catch (NoLockProviderException|PreConditionNotMetException) { return new JSONResponse([], Http::STATUS_BAD_REQUEST); } catch (OwnerLockedException $e) { - // If a file is manually locked by a user we want to all this user to still perform a WOPI lock and write + // The file is already manually locked by this editor, so allow the WOPI lock request to continue. if ($e->getLock()->getType() === ILock::TYPE_USER && $e->getLock()->getOwner() === $wopi->getEditorUid()) { return new JSONResponse(); } + // Another user owns the lock, so reject this WOPI lock request. return new JSONResponse([], Http::STATUS_LOCKED); } catch (\Exception) { return new JSONResponse([], Http::STATUS_INTERNAL_SERVER_ERROR); } } + /** + * Release the server-side lock for the current WOPI file. + * + * This method uses the file lock manager to release the application lock + * associated with the target file. The client-provided WOPI lock token is + * currently ignored. + * + * If the file is already manually locked by the same editor user, the request + * is still allowed so WOPI write flows can continue. + * + * Returns: + * - 200 on success + * - 400 if the lock provider is unavailable or the lock precondition fails + * - 500 for unexpected failures + */ private function unlock(Wopi $wopi, string $lock): JSONResponse { try { + // Try to release the application lock for the current file. $this->lockManager->unlock(new LockContext( $this->getFileForWopiToken($wopi), ILock::TYPE_APP, @@ -911,18 +943,37 @@ private function unlock(Wopi $wopi, string $lock): JSONResponse { return new JSONResponse(); } catch (NoLockProviderException|PreConditionNotMetException) { $locks = $this->lockManager->getLocks($wopi->getFileid()); + // Allow the request to succeed if manually locked unless the conflictinglock belongs to another editor. $canWriteThroughLock = count($locks) > 0 && $locks[0]->getType() === ILock::TYPE_USER && $locks[0]->getOwner() !== $wopi->getEditorUid() ? false : true; if ($canWriteThroughLock) { return new JSONResponse(); } + + // A conflicting lock is present, so this unlock request cannot be satisfied. return new JSONResponse([], Http::STATUS_BAD_REQUEST); } catch (\Exception) { return new JSONResponse([], Http::STATUS_INTERNAL_SERVER_ERROR); } } + /** + * Refresh the server-side lock for the current WOPI file. + * + * This method attempts to acquire the file lock again through the lock manager + * rather than updating a stored WOPI lock token. The client-provided WOPI lock token + * is currently ignored. + * + * Note that Collabora just calls LOCK again to do a refresh; this is currently unused. + * + * Returns: + * - 200 on success + * - 400 if the lock provider is unavailable or the lock precondition fails + * - 423 if another user owns a conflicting lock + * - 500 for unexpected failures + */ private function refreshLock(Wopi $wopi, string $lock): JSONResponse { try { + // Refresh the application lock by acquiring it again through the server-side lock manager. $lock = $this->lockManager->lock(new LockContext( $this->getFileForWopiToken($wopi), ILock::TYPE_APP, @@ -932,14 +983,26 @@ private function refreshLock(Wopi $wopi, string $lock): JSONResponse { } catch (NoLockProviderException|PreConditionNotMetException) { return new JSONResponse([], Http::STATUS_BAD_REQUEST); } catch (OwnerLockedException) { + // Another user owns the lock, so the refresh cannot proceed. return new JSONResponse([], Http::STATUS_LOCKED); } catch (\Exception) { return new JSONResponse([], Http::STATUS_INTERNAL_SERVER_ERROR); } } + /** + * Inspect the current lock state for the WOPI file. + * + * This method queries the file lock manager for the target file, but this implementation + * is effectively a no-op currently: it does not return the lock information to the caller + * nor do anything with the result. The client-provided WOPI lock token is also ignored. + * + * Note that Collabora currently never calls GET_LOCK; this is currently unused. + */ private function getLock(Wopi $wopi, string $lock): JSONResponse { + // Query the current lock state from the server-side file lock manager. $locks = $this->lockManager->getLocks($wopi->getFileid()); + // The queried lock state is not returned to the caller. return new JSONResponse(); } diff --git a/tests/features/bootstrap/WopiContext.php b/tests/features/bootstrap/WopiContext.php index 0e569d287e..ff2fc696b7 100644 --- a/tests/features/bootstrap/WopiContext.php +++ b/tests/features/bootstrap/WopiContext.php @@ -31,6 +31,8 @@ class WopiContext implements Context { private $fileIds = []; private $wopiToken; private $checkFileInfoResult; + private $wopiLock; + private $lockResponseBody; public function __construct() { $this->downloadedFile = tempnam(sys_get_temp_dir(), 'downloadedFile'); @@ -354,4 +356,95 @@ public function performGuestCheckFileInfoRequests($count) { $this->response = $last; } } + + /** + * @When /^Collabora locks the file with id "([^"]*)"$/ + */ + public function collaboraLocksTheFileWithId(string $lockId): void { + $this->wopiLock = $lockId; + $this->collaboraPostOverride('LOCK', [ + 'X-WOPI-Lock' => $lockId, + ]); + } + + /** + * @When /^Collabora refreshes the lock with id "([^"]*)"$/ + */ + public function collaboraRefreshesTheLockWithId(string $lockId): void { + $this->wopiLock = $lockId; + $this->collaboraPostOverride('REFRESH_LOCK', [ + 'X-WOPI-Lock' => $lockId, + ]); + } + + /** + * @When /^Collabora unlocks the file with id "([^"]*)"$/ + */ + public function collaboraUnlocksTheFileWithId(string $lockId): void { + $this->collaboraPostOverride('UNLOCK', [ + 'X-WOPI-Lock' => $lockId, + ]); + } + + /** + * @When /^Collabora gets the current lock$/ + */ + public function collaboraGetsTheCurrentLock(): void { + $this->collaboraPostOverride('GET_LOCK'); + $this->lockResponseBody = (string)$this->response->getBody(); + } + + /** + * @Then /^the WOPI lock should be "([^"]*)"$/ + */ + public function theWopiLockShouldBe(string $expected): void { + Assert::assertEquals($expected, trim($this->lockResponseBody ?? '')); + } + + /** + * @Then /^the WOPI lock response should be empty$/ + */ + public function theWopiLockResponseShouldBeEmpty(): void { + Assert::assertSame('', trim($this->lockResponseBody ?? '')); + } + + /** + * @When /^Collabora unlocks the file with wrong id "([^"]*)"$/ + */ + public function collaboraUnlocksTheFileWithWrongId(string $lockId): void { + $this->collaboraPostOverride('UNLOCK', [ + 'X-WOPI-Lock' => $lockId, + ]); + } + + /** + * @Then /^the WOPI HTTP status code should be one of "([^"]*)"$/ + */ + public function theWopiHttpStatusCodeShouldBeOneOf(string $codes): void { + $validCodes = array_map('trim', explode(',', $codes)); + Assert::assertContains((string)$this->response->getStatusCode(), $validCodes); + } + + private function collaboraPostOverride(string $override, array $headers = [], ?string $body = null): void { + $client = new Client(); + $options = [ + 'headers' => array_merge([ + 'X-COOL-WOPI-Timestamp' => $this->checkFileInfoResult['LastModifiedTime'] ?? null, + 'X-WOPI-Override' => $override, + ], $headers), + ]; + + if ($body !== null) { + $options['body'] = $body; + } + + try { + $this->response = $client->post( + $this->getWopiEndpointBaseUrl() . 'index.php/apps/richdocuments/wopi/files/' . $this->fileId . '?access_token=' . $this->wopiToken, + $options + ); + } catch (\GuzzleHttp\Exception\ClientException $e) { + $this->response = $e->getResponse(); + } + } } diff --git a/tests/features/wopi.feature b/tests/features/wopi.feature index a514739340..547e302828 100644 --- a/tests/features/wopi.feature +++ b/tests/features/wopi.feature @@ -378,6 +378,49 @@ Feature: WOPI And as "user1" the file "/renamed_file.odt" does not exist + Scenario: Lock, refresh, get and unlock a file + Given as user "user1" + And User "user1" uploads file "./../emptyTemplates/template.odt" to "/locked.odt" + Then User "user1" opens "/locked.odt" + And Collabora fetches checkFileInfo + + When Collabora locks the file with id "lock-123" + Then the WOPI HTTP status code should be "200" + + # GET_LOCK not current implemented/supported + #When Collabora gets the current lock + #Then the WOPI lock should be "lock-123" + + When Collabora refreshes the lock with id "lock-123" + Then the WOPI HTTP status code should be "200" + + #When Collabora gets the current lock + #Then the WOPI lock should be "lock-123" + + When Collabora unlocks the file with id "lock-123" + Then the WOPI HTTP status code should be "200" + + #When Collabora gets the current lock + #Then the WOPI lock response should be empty + + Scenario: Unlock with wrong lock token fails + Given as user "user1" + And User "user1" uploads file "./../emptyTemplates/template.odt" to "/wrong-unlock.odt" + Then User "user1" opens "/wrong-unlock.odt" + And Collabora fetches checkFileInfo + + When Collabora locks the file with id "lock-123" + Then the WOPI HTTP status code should be "200" + + When Collabora unlocks the file with wrong id "lock-456" + # UNLOCK currently ignores client provided lock identifier + #Then the WOPI HTTP status code should be "400" + Then the WOPI HTTP status code should be "200" + + #When Collabora gets the current lock + #Then the WOPI lock should be "lock-123" + + Scenario: Public share cannot request a specific saved version Given as user "user1" And User "user1" uploads file "./../emptyTemplates/template.odt" to "/file.odt" @@ -398,4 +441,4 @@ Feature: WOPI Then Using web as guest And a guest opens the share link When I perform "11" guest checkFileInfo requests - Then the WOPI HTTP status code should be "429" \ No newline at end of file + Then the WOPI HTTP status code should be "429" diff --git a/tests/run-integration.sh b/tests/run-integration.sh index d04e46bc75..4bcc5e320e 100755 --- a/tests/run-integration.sh +++ b/tests/run-integration.sh @@ -47,6 +47,9 @@ PHPPIDA=$! PHP_CLI_SERVER_WORKERS=10 php -S localhost:$PORT_SERVERB -t $OC_PATH & PHPPIDB=$! +if [ "$SCENARIO_TO_RUN" == "features/wopi.feature" ]; then + $OCC app:enable --force files_lock +fi $OCC config:app:set richdocuments wopi_url --value="http://localhost:9980" $OCC config:app:set richdocuments public_wopi_url --value="http://localhost:9980"