Skip to content

feat: make direct-edit token consumption transactional and race-safe; defer deletion to successful open paths #5465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
joshtrichards wants to merge 5 commits into
base: main
Choose a base branch
from
+96 −71
Draft

feat: make direct-edit token consumption transactional and race-safe; defer deletion to successful open paths #5465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ab9cb1b
feat(db): add optional row-level locking to getByToken
joshtrichards Mar 7, 2026
da9bd8f
feat(DirectView): only delete one-time token on success paths
joshtrichards Mar 7, 2026
e3913bb
refactor: modernize DirectViewController
joshtrichards Mar 7, 2026
48c0bcb
chore: fixup dbConnection
joshtrichards Mar 7, 2026
a0f2f2e
chore: fixup use alpha
joshtrichards Mar 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
150 changes: 81 additions & 69 deletions lib/Controller/DirectViewController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php

declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2018 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
Expand All @@ -16,8 +17,11 @@
use OCA\Richdocuments\TokenManager;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Db\TTransactional;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\PublicPage;
use OCP\AppFramework\Http\RedirectResponse;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\Files\File;
Expand All @@ -26,12 +30,17 @@
use OCP\Files\Node;
use OCP\Files\NotFoundException;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IRequest;
use OCP\Share\IManager as ShareManager;
use Psr\Log\LoggerInterface;

/**
* Controller for resolving one-time direct-edit tokens and opening documents.
*/
class DirectViewController extends Controller {
use DocumentTrait;
use TTransactional;

public function __construct(
string $appName,
Expand All @@ -47,95 +56,98 @@ public function __construct(
private TemplateManager $templateManager,
private FederationService $federationService,
private LoggerInterface $logger,
private IDBConnection $dbConnection,
) {
parent::__construct($appName, $request);
}

/**
* @NoAdminRequired
* @NoCSRFRequired
* @PublicPage
*
* @param string $token
* @return JSONResponse|RedirectResponse|TemplateResponse
* @throws NotFoundException
* Resolve a one-time direct-edit token and open the requested document.
*/
public function show($token) {
#[NoAdminRequired]
#[NoCSRFRequired]
#[PublicPage]
public function show(string $token): RedirectResponse|TemplateResponse {
try {
$direct = $this->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());

Expand Down Expand Up @@ -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]]
];
Expand Down
17 changes: 15 additions & 2 deletions lib/Db/DirectMapper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
}
Expand Down
Loading