Skip to content
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.

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

[stable28] fix: do not treat guest as share owner #4308

Merged
merged 1 commit into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion lib/Controller/DirectViewController.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public function show($token) {
try {
$urlSrc = $this->tokenManager->getUrlSrc($item);

$wopi = $this->tokenManager->generateWopiTokenForTemplate($item, $direct->getUid(), $direct->getTemplateDestination(), true);
$wopi = $this->tokenManager->generateWopiTokenForTemplate($item, $direct->getTemplateDestination(), $direct->getUid(), false, true);

$targetFile = $folder->getById($direct->getTemplateDestination())[0];
$relativePath = $folder->getRelativePath($targetFile->getPath());
Expand Down
19 changes: 15 additions & 4 deletions lib/Controller/DocumentController.php
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ public function createFromTemplate(int $templateId, string $fileName, string $di

$template = $this->templateManager->get($templateId);
$urlSrc = $this->tokenManager->getUrlSrc($file);
$wopi = $this->tokenManager->generateWopiTokenForTemplate($template, $this->userId, $file->getId());
$isGuest = $this->userId === null;
$wopi = $this->tokenManager->generateWopiTokenForTemplate($template, $file->getId(), $this->userId, $isGuest);

$params = [
'permissions' => $template->getPermissions(),
Expand Down Expand Up @@ -400,7 +401,8 @@ public function token(int $fileId, ?string $shareToken = null, ?string $path = n
]);
}

$wopi = $this->getToken($file, $share);
$isGuest = $guestName || !$this->userId;
$wopi = $this->getToken($file, $share, null, $isGuest);

$this->tokenManager->setGuestName($wopi, $guestName);

Expand Down Expand Up @@ -485,11 +487,20 @@ private function getFileForShare(IShare $share, ?int $fileId, ?string $path = nu
throw new NotFoundException();
}

private function getToken(File $file, ?IShare $share = null, int $version = null): Wopi {
private function getToken(File $file, ?IShare $share = null, ?int $version = null, bool $isGuest = false): Wopi {
// Pass through $version
$templateFile = $this->templateManager->getTemplateSource($file->getId());
if ($templateFile) {
return $this->tokenManager->generateWopiTokenForTemplate($templateFile, $share?->getShareOwner() ?? $this->userId, $file->getId());
$owneruid = $share?->getShareOwner() ?? $file->getOwner()->getUID();

return $this->tokenManager->generateWopiTokenForTemplate(
$templateFile,
$file->getId(),
$owneruid,
$isGuest,
false,
$share?->getPermissions()
);
}

return $this->tokenManager->generateWopiToken($this->getWopiFileId($file->getId(), $version), $share?->getToken(), $this->userId);
Expand Down
40 changes: 15 additions & 25 deletions lib/Controller/WopiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,8 @@ public function postFile(string $fileId, string $access_token): JSONResponse {

// Unless the editor is empty (public link) we modify the files as the current editor
$editor = $wopi->getEditorUid();
if ($editor === null && !$wopi->isRemoteToken()) {
$isPublic = $editor === null && !$wopi->isRemoteToken();
if ($isPublic) {
$editor = $wopi->getOwnerUid();
}

Expand All @@ -627,16 +628,10 @@ public function postFile(string $fileId, string $access_token): JSONResponse {
$file = $this->getFileForWopiToken($wopi);

$suggested = $this->request->getHeader('X-WOPI-RequestedName');

$suggested = mb_convert_encoding($suggested, 'utf-8', 'utf-7') . '.' . $file->getExtension();

if (strpos($suggested, '.') === 0) {
$path = dirname($file->getPath()) . '/New File' . $suggested;
} elseif (strpos($suggested, '/') !== 0) {
$path = dirname($file->getPath()) . '/' . $suggested;
} else {
$path = $userFolder->getPath() . $suggested;
}
$parent = $isPublic ? dirname($file->getPath()) : $userFolder->getPath();
$path = $this->normalizePath($suggested, $parent);

if ($path === '') {
return new JSONResponse([
Expand Down Expand Up @@ -665,20 +660,8 @@ public function postFile(string $fileId, string $access_token): JSONResponse {
$suggested = $this->request->getHeader('X-WOPI-SuggestedTarget');
$suggested = mb_convert_encoding($suggested, 'utf-8', 'utf-7');

if ($suggested[0] === '.') {
$path = dirname($file->getPath()) . '/New File' . $suggested;
} elseif ($suggested[0] !== '/') {
$path = dirname($file->getPath()) . '/' . $suggested;
} else {
$path = $userFolder->getPath() . $suggested;
}

if ($path === '') {
return new JSONResponse([
'status' => 'error',
'message' => 'Cannot create the file'
]);
}
$parent = $isPublic ? dirname($file->getPath()) : $userFolder->getPath();
$path = $this->normalizePath($suggested, $parent);

// create the folder first
if (!$this->rootFolder->nodeExists(dirname($path))) {
Expand All @@ -692,8 +675,8 @@ public function postFile(string $fileId, string $access_token): JSONResponse {

$content = fopen('php://input', 'rb');
// Set the user to register the change under his name
$this->userScopeService->setUserScope($wopi->getEditorUid());
$this->userScopeService->setFilesystemScope($wopi->getEditorUid());
$this->userScopeService->setUserScope($editor);
$this->userScopeService->setFilesystemScope($editor);

try {
$this->wrappedFilesystemOperation($wopi, function () use ($file, $content) {
Expand Down Expand Up @@ -722,6 +705,13 @@ public function postFile(string $fileId, string $access_token): JSONResponse {
}
}

private function normalizePath(string $path, ?string $parent = null): string {
$path = str_starts_with($path, '/') ? $path : '/' . $path;
$parent = is_null($parent) ? '' : rtrim($parent, '/');

return $parent . $path;
}

private function lock(Wopi $wopi, string $lock): JSONResponse {
try {
$lock = $this->lockManager->lock(new LockContext(
Expand Down
4 changes: 4 additions & 0 deletions lib/Db/WopiMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ public function __construct(IDBConnection $db,
* @param string $serverHost
* @param string $guestDisplayname
* @param int $templateDestination
* @param bool $hideDownload
* @param bool $direct
* @param int $templateId
* @param string $share
* @return Wopi
*/
public function generateFileToken($fileId, $owner, $editor, $version, $updatable, $serverHost, $guestDisplayname = null, $templateDestination = 0, $hideDownload = false, $direct = false, $templateId = 0, $share = null) {
Expand Down
60 changes: 49 additions & 11 deletions lib/TokenManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
use OCP\Share\IManager;
use OCP\Share\IShare;
use OCP\Util;
use Psr\Log\LoggerInterface;

class TokenManager {
/** @var IRootFolder */
Expand All @@ -63,6 +64,8 @@ class TokenManager {
private $helper;
/** @var PermissionManager */
private $permissionManager;
/** @var LoggerInterface */
private $logger;

public function __construct(
IRootFolder $rootFolder,
Expand All @@ -74,7 +77,8 @@ public function __construct(
WopiMapper $wopiMapper,
IL10N $trans,
Helper $helper,
PermissionManager $permissionManager
PermissionManager $permissionManager,
LoggerInterface $logger
) {
$this->rootFolder = $rootFolder;
$this->shareManager = $shareManager;
Expand All @@ -86,6 +90,7 @@ public function __construct(
$this->wopiMapper = $wopiMapper;
$this->helper = $helper;
$this->permissionManager = $permissionManager;
$this->logger = $logger;
}

/**
Expand Down Expand Up @@ -151,7 +156,7 @@ public function generateWopiToken(string $fileId, ?string $shareToken = null, ?s
// no active user login while generating the token
// this is required during WopiPutRelativeFile
if (is_null($editoruid)) {
\OC::$server->getLogger()->warning('Generating token for SaveAs without editoruid');
$this->logger->warning('Generating token for SaveAs without editoruid');
$updatable = true;
} else {
// Make sure we use the user folder if available since fetching all files by id from the root might be expensive
Expand Down Expand Up @@ -237,12 +242,18 @@ public function upgradeFromDirectInitiator(Direct $direct, Wopi $wopi) {
return $wopi;
}

public function generateWopiTokenForTemplate(File $templateFile, ?string $userId, int $targetFileId, bool $direct = false): Wopi {
$owneruid = $userId;
$editoruid = $userId;
$rootFolder = $this->rootFolder->getUserFolder($editoruid);
$targetFile = $rootFolder->getById($targetFileId);
$targetFile = array_shift($targetFile);
public function generateWopiTokenForTemplate(
File $templateFile,
int $targetFileId,
string $owneruid,
bool $isGuest,
bool $direct = false,
?int $sharePermissions = null,
): Wopi {
$editoruid = $isGuest ? null : $owneruid;

$rootFolder = $this->rootFolder->getUserFolder($owneruid);
$targetFile = $rootFolder->getById($targetFileId)[0];
if (!$targetFile instanceof File) {
throw new NotFoundException();
}
Expand All @@ -252,16 +263,43 @@ public function generateWopiTokenForTemplate(File $templateFile, ?string $userId
throw new NotPermittedException();
}

$updatable = $targetFile->isUpdateable() && $this->permissionManager->userCanEdit($editoruid);
$updatable = $targetFile->isUpdateable();
if (!is_null($sharePermissions)) {
$shareUpdatable = (bool)($sharePermissions & \OCP\Constants::PERMISSION_UPDATE);
$updatable = $updatable && $shareUpdatable;
}

$serverHost = $this->urlGenerator->getAbsoluteURL('/');

if ($this->capabilitiesService->hasTemplateSource()) {
return $this->wopiMapper->generateFileToken($targetFile->getId(), $owneruid, $editoruid, 0, $updatable, $serverHost, null, 0, false, $direct, $templateFile->getId());
return $this->wopiMapper->generateFileToken(
$targetFile->getId(),
$owneruid,
$editoruid,
0,
$updatable,
$serverHost,
$isGuest ? '' : null,
0,
false,
$direct,
$templateFile->getId()
);
}

// Legacy way of creating new documents from a template
return $this->wopiMapper->generateFileToken($templateFile->getId(), $owneruid, $editoruid, 0, $updatable, $serverHost, null, $targetFile->getId(), $direct);
return $this->wopiMapper->generateFileToken(
$templateFile->getId(),
$owneruid,
$editoruid,
0,
$updatable,
$serverHost,
$isGuest ? '' : null,
$targetFile->getId(),
false,
$direct
);
}

public function newInitiatorToken($sourceServer, Node $node = null, $shareToken = null, bool $direct = false, $userId = null): Wopi {
Expand Down
30 changes: 30 additions & 0 deletions tests/features/bootstrap/WopiContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,16 @@
use GuzzleHttp\Psr7\Utils;
use JuliusHaertl\NextcloudBehat\Context\FilesContext;
use JuliusHaertl\NextcloudBehat\Context\ServerContext;
use JuliusHaertl\NextcloudBehat\Context\SharingContext;
use PHPUnit\Framework\Assert;

class WopiContext implements Context {
/** @var ServerContext */
private $serverContext;
/** @var FilesContext */
private $filesContext;
/** @var SharingContext */
private $sharingContext;

private $downloadedFile;
private $response;
Expand All @@ -56,6 +59,7 @@ public function gatherContexts(BeforeScenarioScope $scope) {
$environment = $scope->getEnvironment();
$this->serverContext = $environment->getContext(ServerContext::class);
$this->filesContext = $environment->getContext(FilesContext::class);
$this->sharingContext = $environment->getContext(SharingContext::class);
}

public function getWopiEndpointBaseUrl() {
Expand Down Expand Up @@ -105,6 +109,32 @@ public function collaboraPuts($source) {
}
}

/**
* @Then /^Create new document as guest with file name "([^"]*)"$/
*/
public function createDocumentAsGuest(string $name) {
$client = new Client();
$options = [
'body' => json_encode([
'directoryPath' => '/',
'fileName' => $name,
'mimeType' => 'application/vnd.oasis.opendocument.text',
'shareToken' => $this->sharingContext->getLastShareData()['token'],
'templateId' => 0,
]),
'headers' => [
'Content-Type' => 'application/json',
'OCS-ApiRequest' => 'true'
],
];

try {
$this->response = $client->post($this->getWopiEndpointBaseUrl() . 'ocs/v2.php/apps/richdocuments/api/v1/file', $options);
} catch (\GuzzleHttp\Exception\ClientException $e) {
$this->response = $e->getResponse();
}
}

/**
* @Then /^the WOPI HTTP status code should be "([^"]*)"$/
* @param int $statusCode
Expand Down
16 changes: 16 additions & 0 deletions tests/features/wopi.feature
Original file line number Diff line number Diff line change
Expand Up @@ -342,3 +342,19 @@ Feature: WOPI
| UserFriendlyName | user2-displayname |
And Collabora downloads the file
And Collabora downloads the file and it is equal to "./../emptyTemplates/template.ods"

Scenario: Save as guest user to owner root
Given as user "user1"
And User "user1" creates a folder "SharedFolder"
And as "user1" create a share with
| path | /SharedFolder |
| shareType | 3 |
And Updating last share with
| permissions | 31 |
And Create new document as guest with file name "some-guest-document.odt"
And as "user1" the file "/SharedFolder/some-guest-document.odt" exists
And a guest opens the file "some-guest-document.odt" of the shared link
And Collabora fetches checkFileInfo
And Collabora saves the content of "./../emptyTemplates/template.ods" as "/saved-as-guest-document.odt"
And as "user1" the file "/SharedFolder/saved-as-guest-document.odt" exists
And as "user1" the file "/saved-as-guest-document.odt" does not exist
4 changes: 4 additions & 0 deletions tests/stub.phpstub
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ class OC_User {
public static function setIncognitoMode($status) {}
}

class OC_Hook {
public static function emit($signalClass, $signalName, $params = []);
}

namespace OC\Hooks {
interface Emitter {
/**
Expand Down
Loading