Skip to content

Commit d6e2585

Browse files
elzodyelzody
authored
Merge pull request #4310 from nextcloud/backport/4306/stable30
[stable30] fix: do not treat guest as share owner
2 parents 57ab5f5 + 565f825 commit d6e2585

File tree

5 files changed

+107
-37
lines changed

5 files changed

+107
-37
lines changed

‎lib/Controller/DocumentController.php‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,8 @@ public function createFromTemplate(int $templateId, string $fileName, string $di
181181

182182
$template = $this->templateManager->get($templateId);
183183
$urlSrc = $this->tokenManager->getUrlSrc($file);
184-
$wopi = $this->tokenManager->generateWopiTokenForTemplate($template, $this->userId, $file->getId());
184+
$isGuest = $this->userId === null;
185+
$wopi = $this->tokenManager->generateWopiTokenForTemplate($template, $file->getId(), $this->userId, $isGuest);
185186

186187
$params = [
187188
'permissions' => $template->getPermissions(),
@@ -394,7 +395,8 @@ public function token(int $fileId, ?string $shareToken = null, ?string $path = n
394395
]);
395396
}
396397

397-
$wopi = $this->getToken($file, $share);
398+
$isGuest = $guestName || !$this->userId;
399+
$wopi = $this->getToken($file, $share, null, $isGuest);
398400

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

@@ -477,11 +479,20 @@ private function getFileForShare(IShare $share, ?int $fileId, ?string $path = nu
477479
throw new NotFoundException();
478480
}
479481

480-
private function getToken(File $file, ?IShare $share = null, ?int $version = null): Wopi {
482+
private function getToken(File $file, ?IShare $share = null, ?int $version = null, bool $isGuest = false): Wopi {
481483
// Pass through $version
482484
$templateFile = $this->templateManager->getTemplateSource($file->getId());
483485
if ($templateFile) {
484-
return $this->tokenManager->generateWopiTokenForTemplate($templateFile, $share?->getShareOwner() ?? $this->userId, $file->getId());
486+
$owneruid = $share?->getShareOwner() ?? $file->getOwner()->getUID();
487+
488+
return $this->tokenManager->generateWopiTokenForTemplate(
489+
$templateFile,
490+
$file->getId(),
491+
$owneruid,
492+
$isGuest,
493+
false,
494+
$share?->getPermissions()
495+
);
485496
}
486497

487498
return $this->tokenManager->generateWopiToken($this->getWopiFileId($file->getId(), $version), $share?->getToken(), $this->userId);

‎lib/Controller/WopiController.php‎

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,8 @@ public function postFile(string $fileId, string $access_token): JSONResponse {
560560

561561
// Unless the editor is empty (public link) we modify the files as the current editor
562562
$editor = $wopi->getEditorUid();
563-
if ($editor === null && !$wopi->isRemoteToken()) {
563+
$isPublic = $editor === null && !$wopi->isRemoteToken();
564+
if ($isPublic) {
564565
$editor = $wopi->getOwnerUid();
565566
}
566567

@@ -573,16 +574,10 @@ public function postFile(string $fileId, string $access_token): JSONResponse {
573574
$file = $this->getFileForWopiToken($wopi);
574575

575576
$suggested = $this->request->getHeader('X-WOPI-RequestedName');
576-
577577
$suggested = mb_convert_encoding($suggested, 'utf-8', 'utf-7') . '.' . $file->getExtension();
578578

579-
if (strpos($suggested, '.') === 0) {
580-
$path = dirname($file->getPath()) . '/New File' . $suggested;
581-
} elseif (strpos($suggested, '/') !== 0) {
582-
$path = dirname($file->getPath()) . '/' . $suggested;
583-
} else {
584-
$path = $userFolder->getPath() . $suggested;
585-
}
579+
$parent = $isPublic ? dirname($file->getPath()) : $userFolder->getPath();
580+
$path = $this->normalizePath($suggested, $parent);
586581

587582
if ($path === '') {
588583
return new JSONResponse([
@@ -611,20 +606,8 @@ public function postFile(string $fileId, string $access_token): JSONResponse {
611606
$suggested = $this->request->getHeader('X-WOPI-SuggestedTarget');
612607
$suggested = mb_convert_encoding($suggested, 'utf-8', 'utf-7');
613608

614-
if ($suggested[0] === '.') {
615-
$path = dirname($file->getPath()) . '/New File' . $suggested;
616-
} elseif ($suggested[0] !== '/') {
617-
$path = dirname($file->getPath()) . '/' . $suggested;
618-
} else {
619-
$path = $userFolder->getPath() . $suggested;
620-
}
621-
622-
if ($path === '') {
623-
return new JSONResponse([
624-
'status' => 'error',
625-
'message' => 'Cannot create the file'
626-
]);
627-
}
609+
$parent = $isPublic ? dirname($file->getPath()) : $userFolder->getPath();
610+
$path = $this->normalizePath($suggested, $parent);
628611

629612
// create the folder first
630613
if (!$this->rootFolder->nodeExists(dirname($path))) {
@@ -638,8 +621,8 @@ public function postFile(string $fileId, string $access_token): JSONResponse {
638621

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

644627
try {
645628
$this->wrappedFilesystemOperation($wopi, function () use ($file, $content) {
@@ -668,6 +651,13 @@ public function postFile(string $fileId, string $access_token): JSONResponse {
668651
}
669652
}
670653

654+
private function normalizePath(string $path, ?string $parent = null): string {
655+
$path = str_starts_with($path, '/') ? $path : '/' . $path;
656+
$parent = is_null($parent) ? '' : rtrim($parent, '/');
657+
658+
return $parent . $path;
659+
}
660+
671661
private function lock(Wopi $wopi, string $lock): JSONResponse {
672662
try {
673663
$lock = $this->lockManager->lock(new LockContext(

‎lib/TokenManager.php‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
use OCP\Share\IManager;
2727
use OCP\Share\IShare;
2828
use OCP\Util;
29+
use Psr\Log\LoggerInterface;
2930

3031
class TokenManager {
3132
public function __construct(
@@ -39,6 +40,7 @@ public function __construct(
3940
private Helper $helper,
4041
private PermissionManager $permissionManager,
4142
private IEventDispatcher $eventDispatcher,
43+
private LoggerInterface $logger,
4244
) {
4345
}
4446

@@ -105,7 +107,7 @@ public function generateWopiToken(string $fileId, ?string $shareToken = null, ?s
105107
// no active user login while generating the token
106108
// this is required during WopiPutRelativeFile
107109
if (is_null($editoruid)) {
108-
\OC::$server->getLogger()->warning('Generating token for SaveAs without editoruid');
110+
$this->logger->warning('Generating token for SaveAs without editoruid');
109111
$updatable = true;
110112
} else {
111113
// Make sure we use the user folder if available since fetching all files by id from the root might be expensive
@@ -188,10 +190,17 @@ public function upgradeFromDirectInitiator(Direct $direct, Wopi $wopi) {
188190
return $wopi;
189191
}
190192

191-
public function generateWopiTokenForTemplate(File $templateFile, ?string $userId, int $targetFileId, bool $direct = false): Wopi {
192-
$owneruid = $userId;
193-
$editoruid = $userId;
194-
$rootFolder = $this->rootFolder->getUserFolder($editoruid);
193+
public function generateWopiTokenForTemplate(
194+
File $templateFile,
195+
int $targetFileId,
196+
string $owneruid,
197+
bool $isGuest,
198+
bool $direct = false,
199+
?int $sharePermissions = null,
200+
): Wopi {
201+
$editoruid = $isGuest ? null : $owneruid;
202+
203+
$rootFolder = $this->rootFolder->getUserFolder($owneruid);
195204
$targetFile = $rootFolder->getFirstNodeById($targetFileId);
196205
if (!$targetFile instanceof File) {
197206
throw new NotFoundException();
@@ -202,12 +211,26 @@ public function generateWopiTokenForTemplate(File $templateFile, ?string $userId
202211
throw new NotPermittedException();
203212
}
204213

205-
$updatable = $targetFile->isUpdateable() && $this->permissionManager->userCanEdit($editoruid);
214+
$updatable = $targetFile->isUpdateable();
215+
if (!is_null($sharePermissions)) {
216+
$shareUpdatable = (bool)($sharePermissions & \OCP\Constants::PERMISSION_UPDATE);
217+
$updatable = $updatable && $shareUpdatable;
218+
}
206219

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

209-
return $this->wopiMapper->generateFileToken($targetFile->getId(), $owneruid, $editoruid, 0, $updatable, $serverHost, null,
210-
false, $direct, $templateFile->getId());
222+
return $this->wopiMapper->generateFileToken(
223+
$targetFile->getId(),
224+
$owneruid,
225+
$editoruid,
226+
0,
227+
$updatable,
228+
$serverHost,
229+
$isGuest ? '' : null,
230+
false,
231+
$direct,
232+
$templateFile->getId()
233+
);
211234
}
212235

213236
public function newInitiatorToken($sourceServer, ?Node $node = null, $shareToken = null, bool $direct = false, $userId = null): Wopi {

‎tests/features/bootstrap/WopiContext.php‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,16 @@
1313
use GuzzleHttp\Psr7\Utils;
1414
use JuliusHaertl\NextcloudBehat\Context\FilesContext;
1515
use JuliusHaertl\NextcloudBehat\Context\ServerContext;
16+
use JuliusHaertl\NextcloudBehat\Context\SharingContext;
1617
use PHPUnit\Framework\Assert;
1718

1819
class WopiContext implements Context {
1920
/** @var ServerContext */
2021
private $serverContext;
2122
/** @var FilesContext */
2223
private $filesContext;
24+
/** @var SharingContext */
25+
private $sharingContext;
2326

2427
private $downloadedFile;
2528
private $response;
@@ -38,6 +41,7 @@ public function gatherContexts(BeforeScenarioScope $scope) {
3841
$environment = $scope->getEnvironment();
3942
$this->serverContext = $environment->getContext(ServerContext::class);
4043
$this->filesContext = $environment->getContext(FilesContext::class);
44+
$this->sharingContext = $environment->getContext(SharingContext::class);
4145
}
4246

4347
public function getWopiEndpointBaseUrl() {
@@ -87,6 +91,32 @@ public function collaboraPuts($source) {
8791
}
8892
}
8993

94+
/**
95+
* @Then /^Create new document as guest with file name "([^"]*)"$/
96+
*/
97+
public function createDocumentAsGuest(string $name) {
98+
$client = new Client();
99+
$options = [
100+
'body' => json_encode([
101+
'directoryPath' => '/',
102+
'fileName' => $name,
103+
'mimeType' => 'application/vnd.oasis.opendocument.text',
104+
'shareToken' => $this->sharingContext->getLastShareData()['token'],
105+
'templateId' => 0,
106+
]),
107+
'headers' => [
108+
'Content-Type' => 'application/json',
109+
'OCS-ApiRequest' => 'true'
110+
],
111+
];
112+
113+
try {
114+
$this->response = $client->post($this->getWopiEndpointBaseUrl() . 'ocs/v2.php/apps/richdocuments/api/v1/file', $options);
115+
} catch (\GuzzleHttp\Exception\ClientException $e) {
116+
$this->response = $e->getResponse();
117+
}
118+
}
119+
90120
/**
91121
* @Then /^the WOPI HTTP status code should be "([^"]*)"$/
92122
* @param int $statusCode

‎tests/features/wopi.feature‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,3 +342,19 @@ Feature: WOPI
342342
| UserFriendlyName | user2-displayname |
343343
And Collabora downloads the file
344344
And Collabora downloads the file and it is equal to "./../emptyTemplates/template.ods"
345+
346+
Scenario: Save as guest user to owner root
347+
Given as user "user1"
348+
And User "user1" creates a folder "SharedFolder"
349+
And as "user1" create a share with
350+
| path | /SharedFolder |
351+
| shareType | 3 |
352+
And Updating last share with
353+
| permissions | 31 |
354+
And Create new document as guest with file name "some-guest-document.odt"
355+
And as "user1" the file "/SharedFolder/some-guest-document.odt" exists
356+
And a guest opens the file "some-guest-document.odt" of the shared link
357+
And Collabora fetches checkFileInfo
358+
And Collabora saves the content of "./../emptyTemplates/template.ods" as "/saved-as-guest-document.odt"
359+
And as "user1" the file "/SharedFolder/saved-as-guest-document.odt" exists
360+
And as "user1" the file "/saved-as-guest-document.odt" does not exist

0 commit comments

Comments
 (0)