Skip to content

Commit 2f22218

Browse files
elzodyelzody
authored
Merge pull request #4309 from nextcloud/backport/4306/stable29
[stable29] fix: do not treat guest as share owner
2 parents 107acd1 + b4b76b4 commit 2f22218

File tree

8 files changed

+134
-41
lines changed

8 files changed

+134
-41
lines changed

‎lib/Controller/DirectViewController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ public function show($token) {
103103
try {
104104
$urlSrc = $this->tokenManager->getUrlSrc($item);
105105

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

108108
$targetFile = $folder->getById($direct->getTemplateDestination())[0];
109109
$relativePath = $folder->getRelativePath($targetFile->getPath());

‎lib/Controller/DocumentController.php‎

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

188188
$template = $this->templateManager->get($templateId);
189189
$urlSrc = $this->tokenManager->getUrlSrc($file);
190-
$wopi = $this->tokenManager->generateWopiTokenForTemplate($template, $this->userId, $file->getId());
190+
$isGuest = $this->userId === null;
191+
$wopi = $this->tokenManager->generateWopiTokenForTemplate($template, $file->getId(), $this->userId, $isGuest);
191192

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

403-
$wopi = $this->getToken($file, $share);
404+
$isGuest = $guestName || !$this->userId;
405+
$wopi = $this->getToken($file, $share, null, $isGuest);
404406

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

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

488-
private function getToken(File $file, ?IShare $share = null, ?int $version = null): Wopi {
490+
private function getToken(File $file, ?IShare $share = null, ?int $version = null, bool $isGuest = false): Wopi {
489491
// Pass through $version
490492
$templateFile = $this->templateManager->getTemplateSource($file->getId());
491493
if ($templateFile) {
492-
return $this->tokenManager->generateWopiTokenForTemplate($templateFile, $share?->getShareOwner() ?? $this->userId, $file->getId());
494+
$owneruid = $share?->getShareOwner() ?? $file->getOwner()->getUID();
495+
496+
return $this->tokenManager->generateWopiTokenForTemplate(
497+
$templateFile,
498+
$file->getId(),
499+
$owneruid,
500+
$isGuest,
501+
false,
502+
$share?->getPermissions()
503+
);
493504
}
494505

495506
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
@@ -587,7 +587,8 @@ public function postFile(string $fileId, string $access_token): JSONResponse {
587587

588588
// Unless the editor is empty (public link) we modify the files as the current editor
589589
$editor = $wopi->getEditorUid();
590-
if ($editor === null && !$wopi->isRemoteToken()) {
590+
$isPublic = $editor === null && !$wopi->isRemoteToken();
591+
if ($isPublic) {
591592
$editor = $wopi->getOwnerUid();
592593
}
593594

@@ -603,16 +604,10 @@ public function postFile(string $fileId, string $access_token): JSONResponse {
603604
$file = $this->getFileForWopiToken($wopi);
604605

605606
$suggested = $this->request->getHeader('X-WOPI-RequestedName');
606-
607607
$suggested = mb_convert_encoding($suggested, 'utf-8', 'utf-7') . '.' . $file->getExtension();
608608

609-
if (strpos($suggested, '.') === 0) {
610-
$path = dirname($file->getPath()) . '/New File' . $suggested;
611-
} elseif (strpos($suggested, '/') !== 0) {
612-
$path = dirname($file->getPath()) . '/' . $suggested;
613-
} else {
614-
$path = $userFolder->getPath() . $suggested;
615-
}
609+
$parent = $isPublic ? dirname($file->getPath()) : $userFolder->getPath();
610+
$path = $this->normalizePath($suggested, $parent);
616611

617612
if ($path === '') {
618613
return new JSONResponse([
@@ -641,20 +636,8 @@ public function postFile(string $fileId, string $access_token): JSONResponse {
641636
$suggested = $this->request->getHeader('X-WOPI-SuggestedTarget');
642637
$suggested = mb_convert_encoding($suggested, 'utf-8', 'utf-7');
643638

644-
if ($suggested[0] === '.') {
645-
$path = dirname($file->getPath()) . '/New File' . $suggested;
646-
} elseif ($suggested[0] !== '/') {
647-
$path = dirname($file->getPath()) . '/' . $suggested;
648-
} else {
649-
$path = $userFolder->getPath() . $suggested;
650-
}
651-
652-
if ($path === '') {
653-
return new JSONResponse([
654-
'status' => 'error',
655-
'message' => 'Cannot create the file'
656-
]);
657-
}
639+
$parent = $isPublic ? dirname($file->getPath()) : $userFolder->getPath();
640+
$path = $this->normalizePath($suggested, $parent);
658641

659642
// create the folder first
660643
if (!$this->rootFolder->nodeExists(dirname($path))) {
@@ -668,8 +651,8 @@ public function postFile(string $fileId, string $access_token): JSONResponse {
668651

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

674657
try {
675658
$this->wrappedFilesystemOperation($wopi, function () use ($file, $content) {
@@ -698,6 +681,13 @@ public function postFile(string $fileId, string $access_token): JSONResponse {
698681
}
699682
}
700683

684+
private function normalizePath(string $path, ?string $parent = null): string {
685+
$path = str_starts_with($path, '/') ? $path : '/' . $path;
686+
$parent = is_null($parent) ? '' : rtrim($parent, '/');
687+
688+
return $parent . $path;
689+
}
690+
701691
private function lock(Wopi $wopi, string $lock): JSONResponse {
702692
try {
703693
$lock = $this->lockManager->lock(new LockContext(

‎lib/Db/WopiMapper.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ public function __construct(IDBConnection $db,
6868
* @param string $serverHost
6969
* @param string $guestDisplayname
7070
* @param int $templateDestination
71+
* @param bool $hideDownload
72+
* @param bool $direct
73+
* @param int $templateId
74+
* @param string $share
7175
* @return Wopi
7276
*/
7377
public function generateFileToken($fileId, $owner, $editor, $version, $updatable, $serverHost, $guestDisplayname = null, $templateDestination = 0, $hideDownload = false, $direct = false, $templateId = 0, $share = null) {

‎lib/TokenManager.php‎

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
use OCP\Share\IManager;
4242
use OCP\Share\IShare;
4343
use OCP\Util;
44+
use Psr\Log\LoggerInterface;
4445

4546
class TokenManager {
4647
/** @var IRootFolder */
@@ -63,6 +64,8 @@ class TokenManager {
6364
private $helper;
6465
/** @var PermissionManager */
6566
private $permissionManager;
67+
/** @var LoggerInterface */
68+
private $logger;
6669

6770
public function __construct(
6871
IRootFolder $rootFolder,
@@ -74,7 +77,8 @@ public function __construct(
7477
WopiMapper $wopiMapper,
7578
IL10N $trans,
7679
Helper $helper,
77-
PermissionManager $permissionManager
80+
PermissionManager $permissionManager,
81+
LoggerInterface $logger
7882
) {
7983
$this->rootFolder = $rootFolder;
8084
$this->shareManager = $shareManager;
@@ -86,6 +90,7 @@ public function __construct(
8690
$this->wopiMapper = $wopiMapper;
8791
$this->helper = $helper;
8892
$this->permissionManager = $permissionManager;
93+
$this->logger = $logger;
8994
}
9095

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

240-
public function generateWopiTokenForTemplate(File $templateFile, ?string $userId, int $targetFileId, bool $direct = false): Wopi {
241-
$owneruid = $userId;
242-
$editoruid = $userId;
243-
$rootFolder = $this->rootFolder->getUserFolder($editoruid);
244-
$targetFile = $rootFolder->getById($targetFileId);
245-
$targetFile = array_shift($targetFile);
245+
public function generateWopiTokenForTemplate(
246+
File $templateFile,
247+
int $targetFileId,
248+
string $owneruid,
249+
bool $isGuest,
250+
bool $direct = false,
251+
?int $sharePermissions = null,
252+
): Wopi {
253+
$editoruid = $isGuest ? null : $owneruid;
254+
255+
$rootFolder = $this->rootFolder->getUserFolder($owneruid);
256+
$targetFile = $rootFolder->getFirstNodeById($targetFileId);
246257
if (!$targetFile instanceof File) {
247258
throw new NotFoundException();
248259
}
@@ -252,16 +263,43 @@ public function generateWopiTokenForTemplate(File $templateFile, ?string $userId
252263
throw new NotPermittedException();
253264
}
254265

255-
$updatable = $targetFile->isUpdateable() && $this->permissionManager->userCanEdit($editoruid);
266+
$updatable = $targetFile->isUpdateable();
267+
if (!is_null($sharePermissions)) {
268+
$shareUpdatable = (bool)($sharePermissions & \OCP\Constants::PERMISSION_UPDATE);
269+
$updatable = $updatable && $shareUpdatable;
270+
}
256271

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

259274
if ($this->capabilitiesService->hasTemplateSource()) {
260-
return $this->wopiMapper->generateFileToken($targetFile->getId(), $owneruid, $editoruid, 0, $updatable, $serverHost, null, 0, false, $direct, $templateFile->getId());
275+
return $this->wopiMapper->generateFileToken(
276+
$targetFile->getId(),
277+
$owneruid,
278+
$editoruid,
279+
0,
280+
$updatable,
281+
$serverHost,
282+
$isGuest ? '' : null,
283+
0,
284+
false,
285+
$direct,
286+
$templateFile->getId()
287+
);
261288
}
262289

263290
// Legacy way of creating new documents from a template
264-
return $this->wopiMapper->generateFileToken($templateFile->getId(), $owneruid, $editoruid, 0, $updatable, $serverHost, null, $targetFile->getId(), $direct);
291+
return $this->wopiMapper->generateFileToken(
292+
$templateFile->getId(),
293+
$owneruid,
294+
$editoruid,
295+
0,
296+
$updatable,
297+
$serverHost,
298+
$isGuest ? '' : null,
299+
$targetFile->getId(),
300+
false,
301+
$direct
302+
);
265303
}
266304

267305
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
@@ -31,13 +31,16 @@
3131
use GuzzleHttp\Psr7\Utils;
3232
use JuliusHaertl\NextcloudBehat\Context\FilesContext;
3333
use JuliusHaertl\NextcloudBehat\Context\ServerContext;
34+
use JuliusHaertl\NextcloudBehat\Context\SharingContext;
3435
use PHPUnit\Framework\Assert;
3536

3637
class WopiContext implements Context {
3738
/** @var ServerContext */
3839
private $serverContext;
3940
/** @var FilesContext */
4041
private $filesContext;
42+
/** @var SharingContext */
43+
private $sharingContext;
4144

4245
private $downloadedFile;
4346
private $response;
@@ -56,6 +59,7 @@ public function gatherContexts(BeforeScenarioScope $scope) {
5659
$environment = $scope->getEnvironment();
5760
$this->serverContext = $environment->getContext(ServerContext::class);
5861
$this->filesContext = $environment->getContext(FilesContext::class);
62+
$this->sharingContext = $environment->getContext(SharingContext::class);
5963
}
6064

6165
public function getWopiEndpointBaseUrl() {
@@ -105,6 +109,32 @@ public function collaboraPuts($source) {
105109
}
106110
}
107111

112+
/**
113+
* @Then /^Create new document as guest with file name "([^"]*)"$/
114+
*/
115+
public function createDocumentAsGuest(string $name) {
116+
$client = new Client();
117+
$options = [
118+
'body' => json_encode([
119+
'directoryPath' => '/',
120+
'fileName' => $name,
121+
'mimeType' => 'application/vnd.oasis.opendocument.text',
122+
'shareToken' => $this->sharingContext->getLastShareData()['token'],
123+
'templateId' => 0,
124+
]),
125+
'headers' => [
126+
'Content-Type' => 'application/json',
127+
'OCS-ApiRequest' => 'true'
128+
],
129+
];
130+
131+
try {
132+
$this->response = $client->post($this->getWopiEndpointBaseUrl() . 'ocs/v2.php/apps/richdocuments/api/v1/file', $options);
133+
} catch (\GuzzleHttp\Exception\ClientException $e) {
134+
$this->response = $e->getResponse();
135+
}
136+
}
137+
108138
/**
109139
* @Then /^the WOPI HTTP status code should be "([^"]*)"$/
110140
* @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

‎tests/stub.phpstub‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ class OC_User {
77
public static function setIncognitoMode($status) {}
88
}
99

10+
class OC_Hook {
11+
public static function emit($signalClass, $signalName, $params = []);
12+
}
13+
1014
namespace OC\Hooks {
1115
interface Emitter {
1216
/**

0 commit comments

Comments
 (0)