Skip to content
Open
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
fix(sharing): Allow public share access for everyone
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 <[email protected]>
  • Loading branch information
nfebe authored and AndyScherzinger committed Dec 3, 2025
commit fb6a8abd19609afbc9d28513dedf677d7bd7b48d
28 changes: 25 additions & 3 deletions lib/private/Share20/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -1413,7 +1413,7 @@ public function getShareByToken($token) {
}
$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);
}
Expand Down Expand Up @@ -1496,6 +1496,17 @@ protected function checkShare(IShare $share): void {
}
}
}

// 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'));
}
}
}

/**
Expand Down Expand Up @@ -1742,14 +1753,15 @@ public function shareApiEnabled() {
/**
* 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) {
Expand All @@ -1761,6 +1773,16 @@ public function shareApiAllowLinks() {
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
*
Expand Down
4 changes: 3 additions & 1 deletion lib/public/Share/IManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -298,10 +298,12 @@ public function shareApiEnabled();
/**
* 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 31.0.12 Added optional $user parameter
*/
public function shareApiAllowLinks();
public function shareApiAllowLinks(?IUser $user = null);

/**
* Is password on public link required
Expand Down
91 changes: 90 additions & 1 deletion tests/lib/Share20/ManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3260,21 +3260,29 @@ public function testGetShareByTokenWithPublicLinksDisabled(): void {

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(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\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')
Expand All @@ -3285,6 +3293,87 @@ public function testGetShareByTokenPublicUploadDisabled(): void {
$this->assertSame(\OCP\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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
->setPermissions(Constants::PERMISSION_READ);
->setPermissions(\OCP\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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
->setPermissions(Constants::PERMISSION_READ);
->setPermissions(\OCP\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);
Expand Down
Loading