upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-04-20 22:00:39 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 15

fix(sharing): Allow public share access for everyone

Browse source 
When a logged-in user accesses a public share link in the same browser,
the system was incorrectly checking if that user's groups were excluded
from creating link shares. This caused share not found errors for users
in excluded groups, even though public shares should be accessible to anyone
with the link.

The group exclusion setting (`shareapi_allow_links_exclude_groups`) is
intended to restrict share creation, not share access. Public shares
are meant to be anonymous and accessible regardless of the viewer identity
or group membership.

We now check the exclusion for the share creator and not the viewer.

Signed-off-by: nfebe <fenn25.fn@gmail.com>
This commit is contained in:
nfebe 2025-10-16 15:34:30 +02:00
parent 3b2a306a67
commit 6bccaf778a
3 changed files with 118 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
lib/private/Share20/Manager.php
View file

@ -1413,7 +1413,7 @@ class Manager implements IManager {
}
$share = null;
try {
if ($this->shareApiAllowLinks()) {
if ($this->config->getAppValue('core', 'shareapi_allow_links', 'yes') === 'yes') {
$provider = $this->factory->getProviderForType(IShare::TYPE_LINK);
$share = $provider->getShareByToken($token);
}
@ -1496,6 +1496,17 @@ class Manager implements IManager {
}
}
}
// For link and email shares, verify the share owner can still create such shares
if ($share->getShareType() === IShare::TYPE_LINK || $share->getShareType() === IShare::TYPE_EMAIL) {
$shareOwner = $this->userManager->get($share->getShareOwner());
if ($shareOwner === null) {
throw new ShareNotFound($this->l->t('The requested share does not exist anymore'));
}
if (!$this->userCanCreateLinkShares($shareOwner)) {
throw new ShareNotFound($this->l->t('The requested share does not exist anymore'));
}
}
}
/**
@ -1742,14 +1753,15 @@ class Manager implements IManager {
/**
* Is public link sharing enabled
*
* @param ?IUser $user User to check against group exclusions, defaults to current session user
* @return bool
*/
public function shareApiAllowLinks() {
public function shareApiAllowLinks(?IUser $user = null) {
if ($this->config->getAppValue('core', 'shareapi_allow_links', 'yes') !== 'yes') {
return false;
}
$user = $this->userSession->getUser();
$user = $user ?? $this->userSession->getUser();
if ($user) {
$excludedGroups = json_decode($this->config->getAppValue('core', 'shareapi_allow_links_exclude_groups', '[]'));
if ($excludedGroups) {
@ -1761,6 +1773,16 @@ class Manager implements IManager {
return true;
}
/**
* Check if a specific user can create link shares
*
* @param IUser $user The user to check
* @return bool
*/
protected function userCanCreateLinkShares(IUser $user): bool {
return $this->shareApiAllowLinks($user);
}
/**
* Is password on public link requires
*

4
lib/public/Share/IManager.php
View file

@ -298,10 +298,12 @@ interface IManager {
/**
* Is public link sharing enabled
*
* @param ?IUser $user User to check against group exclusions, defaults to current session user
* @return bool
* @since 9.0.0
* @since 33.0.0 Added optional $user parameter
*/
public function shareApiAllowLinks();
public function shareApiAllowLinks(?IUser $user = null);
/**
* Is password on public link required

91
tests/lib/Share20/ManagerTest.php
View file

@ -3325,21 +3325,29 @@ class ManagerTest extends \Test\TestCase {
public function testGetShareByTokenPublicUploadDisabled(): void {
$this->config
->expects($this->exactly(3))
->expects($this->exactly(5))
->method('getAppValue')
->willReturnMap([
['core', 'shareapi_allow_links', 'yes', 'yes'],
['core', 'shareapi_allow_public_upload', 'yes', 'no'],
['files_sharing', 'hide_disabled_user_shares', 'no', 'no'],
['core', 'shareapi_allow_links_exclude_groups', '[]', '[]'],
]);
$share = $this->manager->newShare();
$share->setShareType(IShare::TYPE_LINK)
->setPermissions(Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE);
$share->setSharedWith('sharedWith');
$share->setShareOwner('shareOwner');
$folder = $this->createMock(\OC\Files\Node\Folder::class);
$share->setNode($folder);
$shareOwner = $this->createMock(IUser::class);
$this->userManager->expects($this->once())
->method('get')
->with('shareOwner')
->willReturn($shareOwner);
$this->defaultProvider->expects($this->once())
->method('getShareByToken')
->willReturn('validToken')
@ -3350,6 +3358,87 @@ class ManagerTest extends \Test\TestCase {
$this->assertSame(Constants::PERMISSION_READ, $res->getPermissions());
}
public function testGetShareByTokenShareOwnerExcludedFromLinkShares(): void {
$this->expectException(ShareNotFound::class);
$this->expectExceptionMessage('The requested share does not exist anymore');
$this->config
->expects($this->exactly(4))
->method('getAppValue')
->willReturnMap([
['core', 'shareapi_allow_links', 'yes', 'yes'],
['files_sharing', 'hide_disabled_user_shares', 'no', 'no'],
['core', 'shareapi_allow_links_exclude_groups', '[]', '["excludedGroup"]'],
]);
$this->l->expects($this->once())
->method('t')
->willReturnArgument(0);
$share = $this->manager->newShare();
$share->setShareType(IShare::TYPE_LINK)
->setPermissions(Constants::PERMISSION_READ);
$share->setShareOwner('shareOwner');
$file = $this->createMock(File::class);
$share->setNode($file);
$shareOwner = $this->createMock(IUser::class);
$this->userManager->expects($this->once())
->method('get')
->with('shareOwner')
->willReturn($shareOwner);
$this->groupManager->expects($this->once())
->method('getUserGroupIds')
->with($shareOwner)
->willReturn(['excludedGroup', 'otherGroup']);
$this->defaultProvider->expects($this->once())
->method('getShareByToken')
->with('token')
->willReturn($share);
$this->manager->getShareByToken('token');
}
public function testGetShareByTokenShareOwnerNotExcludedFromLinkShares(): void {
$this->config
->expects($this->exactly(4))
->method('getAppValue')
->willReturnMap([
['core', 'shareapi_allow_links', 'yes', 'yes'],
['files_sharing', 'hide_disabled_user_shares', 'no', 'no'],
['core', 'shareapi_allow_links_exclude_groups', '[]', '["excludedGroup"]'],
]);
$share = $this->manager->newShare();
$share->setShareType(IShare::TYPE_LINK)
->setPermissions(Constants::PERMISSION_READ);
$share->setShareOwner('shareOwner');
$file = $this->createMock(File::class);
$share->setNode($file);
$shareOwner = $this->createMock(IUser::class);
$this->userManager->expects($this->once())
->method('get')
->with('shareOwner')
->willReturn($shareOwner);
$this->groupManager->expects($this->once())
->method('getUserGroupIds')
->with($shareOwner)
->willReturn(['allowedGroup', 'otherGroup']);
$this->defaultProvider->expects($this->once())
->method('getShareByToken')
->with('token')
->willReturn($share);
$res = $this->manager->getShareByToken('token');
$this->assertSame($share, $res);
}
public function testCheckPasswordNoLinkShare(): void {
$share = $this->createMock(IShare::class);
$share->method('getShareType')->willReturn(IShare::TYPE_USER);