diff --git a/lib/Controller/WopiController.php b/lib/Controller/WopiController.php index 8f9593d892..60ee8d393f 100644 --- a/lib/Controller/WopiController.php +++ b/lib/Controller/WopiController.php @@ -21,6 +21,7 @@ use OCA\Richdocuments\Service\FederationService; use OCA\Richdocuments\Service\SettingsService; use OCA\Richdocuments\Service\UserScopeService; +use OCA\Richdocuments\Service\WopiRateLimitService; use OCA\Richdocuments\TaskProcessingManager; use OCA\Richdocuments\TemplateManager; use OCA\Richdocuments\TokenManager; @@ -59,6 +60,7 @@ use OCP\IUserManager; use OCP\Lock\LockedException; use OCP\PreConditionNotMetException; +use OCP\Security\RateLimiting\IRateLimitExceededException; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager as IShareManager; use OCP\Share\IShare; @@ -97,6 +99,7 @@ public function __construct( private SettingsService $settingsService, private CapabilitiesService $capabilitiesService, private Helper $helper, + private WopiRateLimitService $wopiRateLimitService, ) { parent::__construct($appName, $request); } @@ -132,11 +135,35 @@ public function checkFileInfo( return new JSONResponse([], Http::STATUS_FORBIDDEN); } + if ($wopi->isGuest()) { + try { + $this->wopiRateLimitService->registerRequest($wopi, 'checkFileInfo'); + } catch (IRateLimitExceededException $e) { + $this->logger->warning('WOPI checkFileInfo rate limit exceeded for token {wopiId} from {remoteAddress}', [ + 'wopiId' => $wopi->getId(), + 'remoteAddress' => $this->request->getRemoteAddress(), + ]); + return new JSONResponse([], Http::STATUS_TOO_MANY_REQUESTS); + } + } + $isPublic = empty($wopi->getEditorUid()); + $isVersion = $version !== '0'; + if ($isPublic && $isVersion) { + $this->logger->debug( + 'Version access with public link is not allowed', + [ + 'fileId' => $fileId, + 'version' => $version + ], + ); + + return new JSONResponse([], Http::STATUS_FORBIDDEN); + } + $guestUserId = 'Guest-' . \OCP\Server::get(\OCP\Security\ISecureRandom::class)->generate(8); $user = $this->userManager->get($wopi->getEditorUid()); $userDisplayName = $user !== null && !$isPublic ? $user->getDisplayName() : $wopi->getGuestDisplayname(); - $isVersion = $version !== '0'; $isSmartPickerEnabled = (bool)$wopi->getCanwrite() && !$isPublic && !$wopi->getDirect(); $isTaskProcessingEnabled = $isSmartPickerEnabled && $this->taskProcessingManager->isTaskProcessingEnabled(); @@ -353,6 +380,18 @@ public function getFile( return new JSONResponse([], Http::STATUS_FORBIDDEN); } + if ($wopi->isGuest()) { + try { + $this->wopiRateLimitService->registerRequest($wopi, 'getFile'); + } catch (IRateLimitExceededException $e) { + $this->logger->warning('WOPI getFile rate limit exceeded for token {wopiId} from {remoteAddress}', [ + 'wopiId' => $wopi->getId(), + 'remoteAddress' => $this->request->getRemoteAddress(), + ]); + return new JSONResponse([], Http::STATUS_TOO_MANY_REQUESTS); + } + } + if ((int)$fileId !== $wopi->getFileid()) { return new JSONResponse([], Http::STATUS_FORBIDDEN); } @@ -362,6 +401,18 @@ public function getFile( $file = $this->getFileForWopiToken($wopi); \OC_User::setIncognitoMode(true); if ($version !== '0') { + if (empty($wopi->getEditorUid())) { + $this->logger->debug( + 'Version access with public link is not allowed', + [ + 'fileId' => $fileId, + 'version' => $version + ], + ); + + return new JSONResponse([], Http::STATUS_FORBIDDEN); + } + $versionManager = \OCP\Server::get(IVersionManager::class); $info = $versionManager->getVersionFile($this->userManager->get($wopi->getUserForFileAccess()), $file, $version); if ($info->getSize() === 0) { diff --git a/lib/Service/WopiRateLimitService.php b/lib/Service/WopiRateLimitService.php new file mode 100644 index 0000000000..77e2720923 --- /dev/null +++ b/lib/Service/WopiRateLimitService.php @@ -0,0 +1,33 @@ +limiter->registerAnonRequest( + 'richdocuments::wopi::' . $action . '::' . $wopi->getId(), + 10, + 120, + $this->request->getRemoteAddress() + ); + } +} diff --git a/tests/features/bootstrap/WopiContext.php b/tests/features/bootstrap/WopiContext.php index 8b5fd6e5f7..0e569d287e 100644 --- a/tests/features/bootstrap/WopiContext.php +++ b/tests/features/bootstrap/WopiContext.php @@ -315,4 +315,43 @@ public function collaboraRenamesTo($fileId, $newName) { $this->response = $e->getResponse(); } } + + /** + * @When /^Collabora fetches checkFileInfo for version "([^"]*)"$/ + */ + public function collaboraFetchesCheckFileInfoForVersion($version) { + $client = new Client(); + // Ensure we set the version as the third underscore-separated part + $arr = explode('_', $this->fileId); + if (count($arr) >= 3) { + $arr[2] = (string)$version; + } else { + $arr[] = (string)$version; + } + $fid = implode('_', $arr); + $options = []; + try { + $this->response = $client->get($this->getWopiEndpointBaseUrl() . 'index.php/apps/richdocuments/wopi/files/' . $fid . '?access_token=' . $this->wopiToken, $options); + $this->checkFileInfoResult = json_decode($this->response->getBody()->getContents(), true); + } catch (\GuzzleHttp\Exception\ClientException $e) { + $this->response = $e->getResponse(); + } + } + + /** + * @When /^I perform "(\d+)" guest checkFileInfo requests$/ + */ + public function performGuestCheckFileInfoRequests($count) { + $client = new Client(); + $last = null; + for ($i = 0; $i < intval($count); $i++) { + try { + $resp = $client->get($this->getWopiEndpointBaseUrl() . 'index.php/apps/richdocuments/wopi/files/' . $this->fileId . '?access_token=' . $this->wopiToken); + $last = $resp; + } catch (\GuzzleHttp\Exception\ClientException $e) { + $last = $e->getResponse(); + } + $this->response = $last; + } + } } diff --git a/tests/features/wopi.feature b/tests/features/wopi.feature index be7ebcb894..a514739340 100644 --- a/tests/features/wopi.feature +++ b/tests/features/wopi.feature @@ -375,4 +375,27 @@ Feature: WOPI And as "user1" rename "/SharedFolder/file.odt" to "renamed_file" And as "user1" the file "/SharedFolder/renamed_file.odt" exists And as "user1" the file "/SharedFolder/file.odt" does not exist - And as "user1" the file "/renamed_file.odt" does not exist \ No newline at end of file + And as "user1" the file "/renamed_file.odt" does not exist + + + Scenario: Public share cannot request a specific saved version + Given as user "user1" + And User "user1" uploads file "./../emptyTemplates/template.odt" to "/file.odt" + And as "user1" create a share with + | path | /file.odt | + | shareType | 3 | + Then Using web as guest + And a guest opens the share link + When Collabora fetches checkFileInfo for version "1" + Then the WOPI HTTP status code should be "403" + + Scenario: Guest repeated checkFileInfo requests are rate-limited + Given as user "user1" + And User "user1" uploads file "./../emptyTemplates/template.odt" to "/file.odt" + And as "user1" create a share with + | path | /file.odt | + | shareType | 3 | + 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 diff --git a/tests/lib/Service/WopiRateLimitServiceTest.php b/tests/lib/Service/WopiRateLimitServiceTest.php new file mode 100644 index 0000000000..05e4545436 --- /dev/null +++ b/tests/lib/Service/WopiRateLimitServiceTest.php @@ -0,0 +1,64 @@ +limiter = $this->createMock(ILimiter::class); + $this->request = $this->createMock(IRequest::class); + $this->service = new WopiRateLimitService($this->limiter, $this->request); + } + + private function createWopiMock(int $id = 42): Wopi&MockObject { + $wopi = $this->getMockBuilder(Wopi::class) + ->addMethods(['getId']) + ->getMock(); + $wopi->method('getId')->willReturn($id); + return $wopi; + } + + public function testRegisterRequestCallsLimiterWithCorrectParameters(): void { + $wopi = $this->createWopiMock(7); + $this->request->method('getRemoteAddress')->willReturn('127.0.0.1'); + + $this->limiter->expects($this->once()) + ->method('registerAnonRequest') + ->with( + 'richdocuments::wopi::checkFileInfo::7', + 10, + 120, + '127.0.0.1' + ); + + $this->service->registerRequest($wopi, 'checkFileInfo'); + } + + public function testRegisterRequestPropagatesRateLimitExceeded(): void { + $wopi = $this->createWopiMock(); + $this->request->method('getRemoteAddress')->willReturn('127.0.0.1'); + + $exception = $this->createMock(IRateLimitExceededException::class); + $this->limiter->method('registerAnonRequest')->willThrowException($exception); + + $this->expectException(IRateLimitExceededException::class); + $this->service->registerRequest($wopi, 'checkFileInfo'); + } +}