From d871deb940c631d8bbf00bda65eaefda63219091 Mon Sep 17 00:00:00 2001 From: Kent Delante Date: Tue, 20 Oct 2026 22:38:24 +0800 Subject: [PATCH 1/3] fix: unique custom tokens for link shares Signed-off-by: Kent Delante --- .../lib/Controller/ShareAPIController.php | 2 ++ lib/private/Share20/Manager.php | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 854c92aad0060..d13ea2b551922 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -1389,6 +1389,8 @@ public function updateShare( } catch (HintException $e) { $code = $e->getCode() === 0 ? 403 : $e->getCode(); throw new OCSException($e->getHint(), (int)$code); + } catch (ShareTokenException $e) { + throw new OCSBadRequestException($e->getMessage(), $e); } catch (\Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); throw new OCSBadRequestException('Failed to update share.', $e); diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 1971abbdb6508..042f98529b3fd 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -717,6 +717,16 @@ public function updateShare(IShare $share, bool $onlyValid = true): IShare { $plainTextPassword = $share->getPassword(); $updatedPassword = $this->updateSharePasswordIfNeeded($share, $originalShare); + if ($share->getToken() !== '') { + try { + $shareFound = $this->getShareByToken($share->getToken()); + if ($share->getId() !== $shareFound->getId()) { + throw new ShareTokenException('Unable to update share. Share token already exists'); + } + } catch (ShareNotFound $e) { + } + } + /** * Cannot enable the getSendPasswordByTalk if there is no password set */ From d5142e66060814690c2d9cab884e4997ec7e776f Mon Sep 17 00:00:00 2001 From: Kent Delante Date: Tue, 20 Oct 2026 22:39:50 +0800 Subject: [PATCH 2/3] test: unique custom tokens for link shares Signed-off-by: Kent Delante --- tests/lib/Share20/ManagerTest.php | 97 +++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index a706eff66d1f3..339c9a833d69e 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -58,6 +58,7 @@ use OCP\Share\Exceptions\AlreadySharedException; use OCP\Share\Exceptions\GenericShareException; use OCP\Share\Exceptions\ShareNotFound; +use OCP\Share\Exceptions\ShareTokenException; use OCP\Share\IManager; use OCP\Share\IProviderFactory; use OCP\Share\IShare; @@ -5815,6 +5816,102 @@ public function testCurrentUserCanEnumerateTargetUser(bool $currentUserIsGuest, $this->assertSame($expected, $manager->currentUserCanEnumerateTargetUser($currentUser, $targetUser)); } + + public function testDuplicateCustomTokens(): void { + $factory = new DummyFactory2($this->createMock(ContainerInterface::class)); + $manager = $this->createManager($factory); + $factory->setProvider($this->defaultProvider); + + $this->config + ->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_enabled', 'yes', 'yes'], + ['core', 'shareapi_allow_links', 'yes', 'yes'], + ['core', 'shareapi_allow_links_exclude_groups', '[]', '[]'], + ]); + + $owner1 = $this->createMock(IUser::class); + $owner1->method('getUID')->willReturn('alice'); + $owner1->method('isEnabled')->willReturn(true); + + $this->userManager->method('get')->willReturn($owner1); + + $storage1 = $this->createMock(IStorage::class); + $storage1->method('instanceOfStorage')->willReturn(false); + + $node1 = $this->createMock(File::class); + $node1->method('getId')->willReturn(1); + $node1->method('getPermissions')->willReturn(Constants::PERMISSION_ALL); + $node1->method('getStorage')->willReturn($storage1); + + $userFolder1 = $this->createMock(IUserFolder::class); + $userFolder1->method('getById')->willReturn([$node1]); + $userFolder1->method('getFirstNodeById')->willReturn($node1); + $this->rootFolder->method('getUserFolder')->willReturn($userFolder1); + + $share1 = $manager->newShare(); + $share1->setId(1) + ->setProviderId('prov') + ->setShareType(IShare::TYPE_LINK) + ->setSharedBy('alice') + ->setShareOwner('alice') + ->setNode($node1) + ->setPermissions(Constants::PERMISSION_READ) + ->setToken('abc123'); + + $owner2 = $this->createMock(IUser::class); + $owner2->method('getUID')->willReturn('bob'); + $owner2->method('isEnabled')->willReturn(true); + + $this->userManager->method('get')->willReturn($owner2); + + $storage2 = $this->createMock(IStorage::class); + $storage2->method('instanceOfStorage')->willReturn(false); + + $node2 = $this->createMock(File::class); + $node2->method('getId')->willReturn(2); + $node2->method('getPermissions')->willReturn(Constants::PERMISSION_ALL); + $node2->method('getStorage')->willReturn($storage2); + + $userFolder2 = $this->createMock(IUserFolder::class); + $userFolder2->method('getById')->willReturn([$node2]); + $userFolder2->method('getFirstNodeById')->willReturn($node2); + $this->rootFolder->method('getUserFolder')->willReturn($userFolder2); + + $share2 = $manager->newShare(); + $share2->setId(2) + ->setProviderId('prov') + ->setShareType(IShare::TYPE_LINK) + ->setSharedBy('bob') + ->setShareOwner('bob') + ->setNode($node2) + ->setPermissions(Constants::PERMISSION_READ) + ->setToken('def456'); + + $this->defaultProvider + ->method('create') + ->willReturnCallback(fn (IShare $share) => $share->getId() === $share2->getId() ? $share2 : $share1); + $this->defaultProvider + ->method('getShareById') + ->willReturnCallback(fn ($id) => $id === '2' ? $share2 : $share1); + $this->defaultProvider->method('update') + ->willReturnCallback(fn (IShare $share) => $share->getId() === $share2->getId() ? $share2 : $share1); + $this->defaultProvider->method('getShareByToken')->with('abc123')->willReturn($share1); + + $this->manager->createShare($share1); + + // It is currently not possible to set a custome token on share creation + // so emulate the behavior by creating the share first with a randomly + // generated unique share token and then update the share with the + // duplicate token + $share2 = $manager->createShare($share2); + $share2->setToken('abc123'); + + $this->expectException(ShareTokenException::class); + $share2 = $manager->updateShare($share2); + + $this->assertNotEquals($share1->getToken(), $share2->getToken()); + } } class DummyFactory implements IProviderFactory { From 665e61d75f68a1f17fb3af00c9790af1cc266118 Mon Sep 17 00:00:00 2001 From: Kent Delante Date: Tue, 20 Oct 2026 22:40:06 +0800 Subject: [PATCH 3/3] fix: reset token to original value on error Signed-off-by: Kent Delante --- apps/files_sharing/src/mixins/SharesMixin.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apps/files_sharing/src/mixins/SharesMixin.js b/apps/files_sharing/src/mixins/SharesMixin.js index 9040196c16720..fb3ecbdef7a3e 100644 --- a/apps/files_sharing/src/mixins/SharesMixin.js +++ b/apps/files_sharing/src/mixins/SharesMixin.js @@ -433,6 +433,11 @@ export default { this.share.sendPasswordByTalk = !this.share.sendPasswordByTalk break } + case 'token': { + this.$set(this.errors, property, message) + this.$set(this.share, 'token', this.initialToken) + break + } } }, /**