diff --git a/lib/Controller/DirectViewController.php b/lib/Controller/DirectViewController.php index eb62a97b34..2bf653325c 100644 --- a/lib/Controller/DirectViewController.php +++ b/lib/Controller/DirectViewController.php @@ -1,5 +1,6 @@ directMapper->getByToken($token); - } catch (DoesNotExistException $e) { - $response = $this->renderErrorPage('Failed to open the requested file.'); - $response->setStatus(Http::STATUS_FORBIDDEN); - return $response; - } + return $this->atomic(function () use ($token) { + try { + $direct = $this->directMapper->getByToken($token, true); + } catch (DoesNotExistException $e) { + $response = $this->renderErrorPage('Failed to open the requested file.'); + $response->setStatus(Http::STATUS_FORBIDDEN); + return $response; + } - // Delete the token. They are for 1 time use only - $this->directMapper->delete($direct); + // Direct token for share link + if (!empty($direct->getShare())) { + $response = $this->showPublicShare($direct); - // Direct token for share link - if (!empty($direct->getShare())) { - return $this->showPublicShare($direct); - } + // Consume one-time token only for successful(ish) outcomes (2xx/3xx) + if ($response->getStatus() < 400) { + $this->directMapper->delete($direct); + } - $this->userScopeService->setUserScope($direct->getUid()); - $this->userScopeService->setFilesystemScope($direct->getUid()); + return $response; + } - $folder = $this->rootFolder->getUserFolder($direct->getUid()); + $this->userScopeService->setUserScope($direct->getUid()); + $this->userScopeService->setFilesystemScope($direct->getUid()); - try { - $item = $folder->getFirstNodeById($direct->getFileid()); - if (!($item instanceof File)) { - throw new \Exception(); - } + $folder = $this->rootFolder->getUserFolder($direct->getUid()); - /** Open file from remote collabora */ - $federatedUrl = $this->federationService->getRemoteRedirectURL($item, $direct); - if ($federatedUrl !== null) { - $response = new RedirectResponse($federatedUrl); - $response->addHeader('X-Frame-Options', 'ALLOW'); - return $response; - } + $item = $folder->getFirstNodeById($direct->getFileid()); + if (!($item instanceof File)) { + throw new \RuntimeException('Direct target is not a file'); + } - $wopi = null; - $template = $direct->getTemplateId() ? $this->templateManager->get($direct->getTemplateId()) : null; + /** Open file from remote collabora */ + $federatedUrl = $this->federationService->getRemoteRedirectURL($item, $direct); + if ($federatedUrl !== null) { + $this->directMapper->delete($direct); // consume one-time token success path - if ($template !== null) { - $wopi = $this->tokenManager->generateWopiTokenForTemplate($template, $item->getId(), $direct->getUid(), false, true); - } + $response = new RedirectResponse($federatedUrl); + $response->addHeader('X-Frame-Options', 'ALLOW'); + return $response; + } - if ($wopi === null) { - $wopi = $this->tokenManager->generateWopiToken((string)$item->getId(), null, $direct->getUid(), true); - } + $wopi = null; + $template = $direct->getTemplateId() ? $this->templateManager->get($direct->getTemplateId()) : null; + if ($template !== null) { + $wopi = $this->tokenManager->generateWopiTokenForTemplate($template, $item->getId(), $direct->getUid(), false, true); + } + if ($wopi === null) { + $wopi = $this->tokenManager->generateWopiToken((string)$item->getId(), null, $direct->getUid(), true); + } - $urlSrc = $this->tokenManager->getUrlSrc($item); - } catch (\Exception $e) { - $this->logger->error('Failed to generate token for existing file on direct editing', ['exception' => $e]); - return $this->renderErrorPage('Failed to open the requested file.'); - } + $urlSrc = $this->tokenManager->getUrlSrc($item); + $relativePath = $folder->getRelativePath($item->getPath()); - $relativePath = $folder->getRelativePath($item->getPath()); + $params = [ + 'permissions' => $item->getPermissions(), + 'title' => basename($relativePath), + 'fileId' => $wopi->getFileid() . '_' . $this->config->getSystemValue('instanceid'), + 'token' => $wopi->getToken(), + 'token_ttl' => $wopi->getExpiry(), + 'urlsrc' => $urlSrc, + 'path' => $relativePath, + 'direct' => true, + 'userId' => $direct->getUid(), + ]; - try { - $params = [ - 'permissions' => $item->getPermissions(), - 'title' => basename($relativePath), - 'fileId' => $wopi->getFileid() . '_' . $this->config->getSystemValue('instanceid'), - 'token' => $wopi->getToken(), - 'token_ttl' => $wopi->getExpiry(), - 'urlsrc' => $urlSrc, - 'path' => $relativePath, - 'direct' => true, - 'userId' => $direct->getUid(), - ]; - - return $this->documentTemplateResponse($wopi, $params); - } catch (\Exception $e) { - $this->logger->error($e->getMessage(), ['exception' => $e]); - return $this->renderErrorPage('Failed to open the requested file.'); + $response = $this->documentTemplateResponse($wopi, $params); + $this->directMapper->delete($direct); // consume one-time token on success path + return $response; + }, $this->dbConnection); + } catch (\Throwable $e) { + $this->logger->error('Failed to open the requested file for direct editing', ['exception' => $e]); + return $this->renderErrorPage('Failed to open the requested file.'); } } - public function showPublicShare(Direct $direct) { + /** + * Open a direct-edit request for a public share. + */ + public function showPublicShare(Direct $direct): RedirectResponse|TemplateResponse { try { $share = $this->shareManager->getShareByToken($direct->getShare()); @@ -185,7 +197,7 @@ public function showPublicShare(Direct $direct) { return new TemplateResponse('core', '403', [], 'guest'); } - private function renderErrorPage($message) { + private function renderErrorPage($message): TemplateResponse { $params = [ 'errors' => [['error' => $message]] ]; diff --git a/lib/Db/DirectMapper.php b/lib/Db/DirectMapper.php index 925dbe98bd..79b12edc58 100644 --- a/lib/Db/DirectMapper.php +++ b/lib/Db/DirectMapper.php @@ -47,17 +47,30 @@ public function newDirect($uid, $fileid, $template = null, $share = null, $initi } /** - * @throws DoesNotExistException + * Fetch a direct-link token record. + * + * @param string $token Token value from direct-link URL + * @param bool $forUpdate When true, issues a row-level lock (FOR UPDATE). + * Call only within a DB transaction when token consumption + * must be serialized to avoid concurrent reuse. + * + * @throws DoesNotExistException If token does not exist or is expired (or deletion otherwise fails). */ - public function getByToken(string $token): Direct { + public function getByToken(string $token, bool $forUpdate = false): Direct { $qb = $this->db->getQueryBuilder(); $qb->select('*') ->from('richdocuments_direct') ->where($qb->expr()->eq('token', $qb->createNamedParameter($token))); + if ($forUpdate) { + // Lock token row so concurrent requests cannot consume it in parallel. + $qb->forUpdate(); + } + try { $direct = $this->findEntity($qb); if (($direct->getTimestamp() + self::TOKEN_TTL) < $this->timeFactory->getTime()) { + // Opportunistic cleanup: expired tokens are removed on read. $this->delete($direct); throw new DoesNotExistException('Could not find token.'); }