diff --git a/apps/dav/lib/CalDAV/PublicCalendarRoot.php b/apps/dav/lib/CalDAV/PublicCalendarRoot.php index 96d5ca8a46039..e9fa5acac1a07 100644 --- a/apps/dav/lib/CalDAV/PublicCalendarRoot.php +++ b/apps/dav/lib/CalDAV/PublicCalendarRoot.php @@ -67,12 +67,12 @@ public function getChildren() { /** * Checks if the public calendar should be visible or not, based on - * the configuration of the `hide_disabled_user_shares` setting within + * the configuration of the `hide_disabled_user_external_shares` setting within * `files_sharing` and the status of the owning user (disabled or not). */ private function validateVisibility(string $principalUri): bool { $hideCalendarsOfDisabledUsers = $this->appConfig->getValueBool( - 'files_sharing', 'hide_disabled_user_shares', true + 'files_sharing', 'hide_disabled_user_external_shares', true ); if (!$hideCalendarsOfDisabledUsers) { diff --git a/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php b/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php index 256ddd194b969..fc687d88bbe61 100644 --- a/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php +++ b/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php @@ -150,7 +150,7 @@ public function testGetChildHidesCalendarOfDisabledUser(): void { $publicUri = $calendar->getPublishStatus(); $this->mockDisabledOwner(); - $this->setHideDisabledUserShares(true); + $this->setHideDisabledUserExternalShares(true); $this->expectException(NotFound::class); $this->publicCalendarRoot->getChild($publicUri); @@ -161,7 +161,7 @@ public function testGetChildServesCalendarOfDisabledUserWhenHidingIsDisabled(): $publicUri = $calendar->getPublishStatus(); $this->mockDisabledOwner(); - $this->setHideDisabledUserShares(false); + $this->setHideDisabledUserExternalShares(false); $calendarResult = $this->publicCalendarRoot->getChild($publicUri); $this->assertEquals($calendar, $calendarResult); @@ -188,9 +188,9 @@ private function mockDisabledOwner(): void { ->willReturn($disabledUser); } - private function setHideDisabledUserShares(bool $hide): void { + private function setHideDisabledUserExternalShares(bool $hide): void { $this->appConfig->method('getValueBool') - ->with('files_sharing', 'hide_disabled_user_shares', 'yes') + ->with('files_sharing', 'hide_disabled_user_external_shares', true) ->willReturn($hide); } } diff --git a/apps/files_sharing/lib/Config/ConfigLexicon.php b/apps/files_sharing/lib/Config/ConfigLexicon.php index c0f57a932bba3..5bd7c561654bd 100644 --- a/apps/files_sharing/lib/Config/ConfigLexicon.php +++ b/apps/files_sharing/lib/Config/ConfigLexicon.php @@ -25,6 +25,8 @@ class ConfigLexicon implements ILexicon { public const SHOW_FEDERATED_TO_TRUSTED_AS_INTERNAL = 'show_federated_shares_to_trusted_servers_as_internal'; public const EXCLUDE_RESHARE_FROM_EDIT = 'shareapi_exclude_reshare_from_edit'; public const UPDATE_CUTOFF_TIME = 'update_cutoff_time'; + public const HIDE_DISABLED_USER_SHARES = 'hide_disabled_user_shares'; + public const HIDE_DISABLED_USER_EXTERNAL_SHARES = 'hide_disabled_user_external_shares'; public const USER_NEEDS_SHARE_REFRESH = 'user_needs_share_refresh'; #[\Override] @@ -40,6 +42,8 @@ public function getAppConfigs(): array { new Entry(self::EXCLUDE_RESHARE_FROM_EDIT, ValueType::BOOL, false, 'Exclude reshare permission from "Allow editing" bundled permissions'), new Entry(self::UPDATE_CUTOFF_TIME, ValueType::FLOAT, 3.0, 'Maximum time in second during which we update the share data immediately before switching to only marking the user'), + new Entry(self::HIDE_DISABLED_USER_SHARES, ValueType::BOOL, true, 'Hide internal shares (to accounts, groups, teams, …) owned or created by a disabled account'), + new Entry(self::HIDE_DISABLED_USER_EXTERNAL_SHARES, ValueType::BOOL, true, 'Hide public, mail and federated shares owned or created by a disabled account'), ]; } diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 042f98529b3fd..4e7aa76c6f73d 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1173,7 +1173,7 @@ public function getSharesBy(string $userId, int $shareType, ?Node $path = null, return []; } - if ($onlyValid && $this->config->getAppValue('files_sharing', 'hide_disabled_user_shares', 'no') === 'yes') { + if ($onlyValid && $this->hidesDisabledUserSharesOfType($shareType)) { /* * If shares from disabled users are hidden, check user status first to avoid useless work. * Otherwise all shares would’ve been filtered out by checkShare anyway. @@ -1425,6 +1425,27 @@ public function getShareByToken(string $token): IShare { return $share; } + /** + * Whether shares of the given type are hidden when an account involved in them + * is disabled. Public, mail and federated shares are external, any other type + * is internal. + */ + private function hidesDisabledUserSharesOfType(?int $shareType): bool { + $external = in_array($shareType, [ + IShare::TYPE_LINK, + IShare::TYPE_EMAIL, + IShare::TYPE_REMOTE, + IShare::TYPE_REMOTE_GROUP, + IShare::TYPE_SCIENCEMESH, + ], true); + + return $this->appConfig->getValueBool( + 'files_sharing', + $external ? 'hide_disabled_user_external_shares' : 'hide_disabled_user_shares', + true, + ); + } + /** * Whether a share has to be hidden because one of the accounts involved in it * is disabled. @@ -1435,7 +1456,7 @@ public function getShareByToken(string $token): IShare { * @param ?string $viewer Account the share is checked for, if known */ private function isHiddenDisabledUserShare(IShare $share, ?string $viewer = null): bool { - if ($this->config->getAppValue('files_sharing', 'hide_disabled_user_shares', 'yes') !== 'yes') { + if (!$this->hidesDisabledUserSharesOfType($share->getShareType())) { return false; } diff --git a/lib/private/Sharing/SharingBackend.php b/lib/private/Sharing/SharingBackend.php index 1e3bcc1ee321b..7e0cfb7ac8273 100644 --- a/lib/private/Sharing/SharingBackend.php +++ b/lib/private/Sharing/SharingBackend.php @@ -31,6 +31,8 @@ use NCU\Sharing\Source\IShareSourceMetadata; use NCU\Sharing\Source\IShareSourceType; use NCU\Sharing\Source\ShareSource; +use OC\Core\Sharing\Recipient\EmailShareRecipientType; +use OC\Core\Sharing\Recipient\TokenShareRecipientType; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; use OCP\IAppConfig; @@ -634,8 +636,26 @@ public function setLastUpdated(array $ids, \DateTimeImmutable $lastUpdated): voi } } - private function hideDisabledUserShares(): bool { - return $this->appConfig->getValueString('files_sharing', 'hide_disabled_user_shares', 'yes') === 'yes'; + /** + * Whether recipients of the given kind are hidden when the account that owns the + * share or added the recipient is disabled. + * Public, mail and federated recipients are external, any other recipient is internal. + */ + private function hideDisabledUserShares(bool $external): bool { + return $this->appConfig->getValueBool( + 'files_sharing', + $external ? 'hide_disabled_user_external_shares' : 'hide_disabled_user_shares', + true, + ); + } + + /** + * @param class-string $class + */ + private function isExternalRecipient(string $class, ?string $instance): bool { + return $instance !== null + || $class === TokenShareRecipientType::class + || $class === EmailShareRecipientType::class; } // TODO: Split up the method @@ -731,6 +751,13 @@ private function list( } } + // Whether internal and external recipients are hidden when their share owner or initiator is disabled + $hideInternal = $this->hideDisabledUserShares(false); + $hideExternal = $this->hideDisabledUserShares(true); + // Shares of a disabled owner that are only partly hidden, their recipients are filtered one by one + /** @var array $disabledOwnerShareIds */ + $disabledOwnerShareIds = []; + // The key type is array-key, because PHP will automatically cast the value. We can't type it as integer though, because we need to also support 32 bit systems and there the autocasting doesn't happen, if the value is too large. /** @var array, recipients: array, properties: array, ShareProperty>, permissions: array, SharePermission>}> $shares */ $shares = []; @@ -785,15 +812,22 @@ private function list( /** @var ?non-empty-string $ownerInstance */ $ownerInstance = $row['owner_instance']; + /** @var non-empty-string $shareId */ + $shareId = (string)$row['id']; + /** @psalm-suppress PossiblyNullReference The share is automatically deleted, when the owner is deleted. */ - if ($ownerInstance === null && !$accessContext->overrideChecks && $this->hideDisabledUserShares() && !$this->userManager->get( + if ($ownerInstance === null && !$accessContext->overrideChecks && ($hideInternal || $hideExternal) && !$this->userManager->get( $ownerUserId )->isEnabled()) { - continue; + // Every recipient would be hidden, so skip the whole share + if ($hideInternal && $hideExternal) { + continue; + } + + // Only the recipients of the hidden kind are skipped, see below. + $disabledOwnerShareIds[$shareId] = true; } - /** @var non-empty-string $shareId */ - $shareId = (string)$row['id']; /** @var numeric-string $lastUpdated */ $lastUpdated = (string)$row['last_updated']; $state = (int)$row['state_id']; @@ -963,18 +997,27 @@ private function list( $initiatorUserId = $row['initiator_user_id']; /** @var ?non-empty-string $initiatorInstance */ $initiatorInstance = $row['initiator_instance']; + /** @var ?non-empty-string $instance */ + $instance = $row['recipient_instance']; - /** @psalm-suppress PossiblyNullReference The initiator is automatically promoted to the owner, when the initiator is deleted. */ - if ($initiatorInstance === null && !$accessContext->overrideChecks && !$shares[$shareId]['owner']->isCurrentUser( - $accessContext - ) && $this->hideDisabledUserShares() && !$this->userManager->get($initiatorUserId)->isEnabled()) { - continue; + // Recipients of a kind that is hidden are skipped when the share owner or the account that added them is disabled + $hideIfDisabledUser = $this->isExternalRecipient($typeClass, $instance) ? $hideExternal : $hideInternal; + if ($hideIfDisabledUser && !$accessContext->overrideChecks) { + if (isset($disabledOwnerShareIds[$shareId])) { + continue; + } + + // The owner still sees the recipients added by a disabled initiator, to manage them + /** @psalm-suppress PossiblyNullReference The initiator is automatically promoted to the owner, when the initiator is deleted. */ + if ($initiatorInstance === null && !$shares[$shareId]['owner']->isCurrentUser( + $accessContext + ) && !$this->userManager->get($initiatorUserId)->isEnabled()) { + continue; + } } /** @var non-empty-string $value */ $value = $row['recipient_value']; - /** @var ?non-empty-string $instance */ - $instance = $row['recipient_instance']; // The secret is only removed in the next step, because we still need it to check if the current access context still has access to the share, after the recipients of disabled initiators have been skipped. /** @var non-empty-string $secret */ $secret = $row['recipient_secret']; @@ -1221,7 +1264,8 @@ private function list( if (!$accessContext->overrideChecks) { $shares = array_filter( $shares, - fn (Share $share): bool => $share->owner->isCurrentUser($accessContext) || $hasRecipientAccess[$share->id] || $hasSourceAccess[$share->id] + // Source access is only checked for a logged-in user, not for an access by secret + fn (Share $share): bool => $share->owner->isCurrentUser($accessContext) || $hasRecipientAccess[$share->id] || ($hasSourceAccess[$share->id] ?? false) ); } diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index 339c9a833d69e..176f158998424 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -814,9 +814,9 @@ public function testGetShareById(): void { } public function testGetShareByIdOfDisabledInitiatorAsShareOwner(): void { - $this->config->method('getAppValue') + $this->appConfig->method('getValueBool') ->willReturnMap([ - ['files_sharing', 'hide_disabled_user_shares', 'yes', 'yes'], + ['files_sharing', 'hide_disabled_user_shares', true, true], ]); $share = $this->manager->newShare(); @@ -843,9 +843,9 @@ public function testGetShareByIdOfDisabledInitiatorAsOtherUser(): void { $this->expectException(ShareNotFound::class); $this->expectExceptionMessage('The requested share does not exist anymore'); - $this->config->method('getAppValue') + $this->appConfig->method('getValueBool') ->willReturnMap([ - ['files_sharing', 'hide_disabled_user_shares', 'yes', 'yes'], + ['files_sharing', 'hide_disabled_user_shares', true, true], ]); $share = $this->manager->newShare(); @@ -868,6 +868,83 @@ public function testGetShareByIdOfDisabledInitiatorAsOtherUser(): void { $this->manager->getShareById('default:42', 'recipient'); } + public static function dataHideDisabledUserSharesByType(): array { + $internal = [IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_USERGROUP, IShare::TYPE_CIRCLE, IShare::TYPE_GUEST, IShare::TYPE_ROOM, IShare::TYPE_DECK, IShare::TYPE_DECK_USER]; + $external = [IShare::TYPE_LINK, IShare::TYPE_EMAIL, IShare::TYPE_REMOTE, IShare::TYPE_REMOTE_GROUP, IShare::TYPE_SCIENCEMESH]; + + $data = []; + foreach ([[true, true], [true, false], [false, true], [false, false]] as [$hideInternal, $hideExternal]) { + foreach ($internal as $shareType) { + $data[] = [$shareType, $hideInternal, $hideExternal, $hideInternal]; + } + foreach ($external as $shareType) { + $data[] = [$shareType, $hideInternal, $hideExternal, $hideExternal]; + } + } + + return $data; + } + + #[DataProvider('dataHideDisabledUserSharesByType')] + public function testIsHiddenDisabledUserShareByType(int $shareType, bool $hideInternal, bool $hideExternal, bool $expected): void { + $this->appConfig->method('getValueBool') + ->willReturnMap([ + ['files_sharing', 'hide_disabled_user_shares', true, $hideInternal], + ['files_sharing', 'hide_disabled_user_external_shares', true, $hideExternal], + ]); + + $share = $this->manager->newShare(); + $share->setShareType($shareType) + ->setShareOwner('owner') + ->setSharedBy('owner'); + + $this->userManager->method('get')->willReturnMap([ + ['owner', $this->createEnabledUser(false)], + ]); + + $this->assertSame($expected, self::invokePrivate($this->manager, 'isHiddenDisabledUserShare', [$share, 'recipient'])); + } + + public function testGetSharesByOfDisabledUserKeepsExternalSharesWhenOnlyInternalAreHidden(): void { + $this->appConfig->method('getValueBool') + ->willReturnMap([ + ['files_sharing', 'hide_disabled_user_shares', true, true], + ['files_sharing', 'hide_disabled_user_external_shares', true, false], + ]); + + $share = $this->manager->newShare(); + $share->setShareType(IShare::TYPE_REMOTE) + ->setShareOwner('owner') + ->setSharedBy('owner') + ->setSharedWith('recipient@remote.example'); + + $this->userManager->method('get')->willReturnMap([ + ['owner', $this->createEnabledUser(false)], + ]); + + $this->defaultProvider->method('getSharesBy') + ->willReturn([$share]); + + $this->assertSame([$share], $this->manager->getSharesBy('owner', IShare::TYPE_REMOTE)); + } + + public function testGetSharesByOfDisabledUserHidesInternalSharesWhenOnlyInternalAreHidden(): void { + $this->appConfig->method('getValueBool') + ->willReturnMap([ + ['files_sharing', 'hide_disabled_user_shares', true, true], + ['files_sharing', 'hide_disabled_user_external_shares', true, false], + ]); + + $this->userManager->method('get')->willReturnMap([ + ['owner', $this->createEnabledUser(false)], + ]); + + $this->defaultProvider->expects($this->never()) + ->method('getSharesBy'); + + $this->assertSame([], $this->manager->getSharesBy('owner', IShare::TYPE_USER)); + } + public function testGetExpiredShareById(): void { $this->expectException(ShareNotFound::class); @@ -3210,9 +3287,9 @@ public function testCreateShareUserAlreadySharedReusesExistingShare(): void { $share = $this->createShare(null, IShare::TYPE_USER, $path, 'sharedWith', 'sharedBy', null, Constants::PERMISSION_ALL); $existingShare = $this->createShare('42', IShare::TYPE_USER, $path, 'sharedWith', 'shareOwner', 'shareOwner', Constants::PERMISSION_READ); - $this->config->method('getAppValue') + $this->appConfig->method('getValueBool') ->willReturnMap([ - ['files_sharing', 'hide_disabled_user_shares', 'yes', 'yes'], + ['files_sharing', 'hide_disabled_user_shares', true, true], ]); $this->userManager->method('get')->willReturnMap([ ['shareOwner', $this->createEnabledUser(true)], @@ -3250,9 +3327,9 @@ public function testCreateShareUserAlreadySharedByDisabledInitiator(): void { $share = $this->createShare(null, IShare::TYPE_USER, $path, 'sharedWith', 'shareOwner', null, Constants::PERMISSION_ALL); $existingShare = $this->createShare('42', IShare::TYPE_USER, $path, 'sharedWith', 'initiator', 'shareOwner', Constants::PERMISSION_READ); - $this->config->method('getAppValue') + $this->appConfig->method('getValueBool') ->willReturnMap([ - ['files_sharing', 'hide_disabled_user_shares', 'yes', 'yes'], + ['files_sharing', 'hide_disabled_user_shares', true, true], ]); $this->userManager->method('get')->willReturnMap([ ['shareOwner', $this->createEnabledUser(true)], @@ -3757,9 +3834,9 @@ public function testGetSharesByOwnerless(): void { } public function testGetSharesByOfDisabledInitiatorAsShareOwner(): void { - $this->config->method('getAppValue') + $this->appConfig->method('getValueBool') ->willReturnMap([ - ['files_sharing', 'hide_disabled_user_shares', 'yes', 'yes'], + ['files_sharing', 'hide_disabled_user_shares', true, true], ]); $share = $this->manager->newShare(); @@ -3784,9 +3861,9 @@ public function testGetSharesByOfDisabledInitiatorAsShareOwner(): void { } public function testGetSharesByOfDisabledInitiatorAsOtherUser(): void { - $this->config->method('getAppValue') + $this->appConfig->method('getValueBool') ->willReturnMap([ - ['files_sharing', 'hide_disabled_user_shares', 'yes', 'yes'], + ['files_sharing', 'hide_disabled_user_shares', true, true], ]); $share = $this->manager->newShare(); @@ -3811,9 +3888,9 @@ public function testGetSharesByOfDisabledInitiatorAsOtherUser(): void { } public function testGetSharedWithOfDisabledInitiator(): void { - $this->config->method('getAppValue') + $this->appConfig->method('getValueBool') ->willReturnMap([ - ['files_sharing', 'hide_disabled_user_shares', 'yes', 'yes'], + ['files_sharing', 'hide_disabled_user_shares', true, true], ]); $share = $this->manager->newShare(); @@ -3919,11 +3996,10 @@ public function testGetSharesByExpiredLinkShares(): void { public function testGetShareByToken(): void { $this->config - ->expects($this->exactly(2)) + ->expects($this->exactly(1)) ->method('getAppValue') ->willReturnMap([ ['core', 'shareapi_allow_links', 'yes', 'yes'], - ['files_sharing', 'hide_disabled_user_shares', 'no', 'no'], ]); $factory = $this->createMock(IProviderFactory::class); @@ -3948,11 +4024,10 @@ public function testGetShareByToken(): void { public function testGetShareByTokenRoom(): void { $this->config - ->expects($this->exactly(2)) + ->expects($this->exactly(1)) ->method('getAppValue') ->willReturnMap([ ['core', 'shareapi_allow_links', 'yes', 'no'], - ['files_sharing', 'hide_disabled_user_shares', 'no', 'no'], ]); $factory = $this->createMock(IProviderFactory::class); @@ -3984,11 +4059,10 @@ public function testGetShareByTokenRoom(): void { public function testGetShareByTokenWithException(): void { $this->config - ->expects($this->exactly(2)) + ->expects($this->exactly(1)) ->method('getAppValue') ->willReturnMap([ ['core', 'shareapi_allow_links', 'yes', 'yes'], - ['files_sharing', 'hide_disabled_user_shares', 'no', 'no'], ]); $factory = $this->createMock(IProviderFactory::class); @@ -4026,11 +4100,14 @@ public function testGetShareByTokenHideDisabledUser(): void { $this->expectExceptionMessage('The requested share does not exist anymore'); $this->config - ->expects($this->exactly(2)) + ->expects($this->exactly(1)) ->method('getAppValue') ->willReturnMap([ ['core', 'shareapi_allow_links', 'yes', 'yes'], - ['files_sharing', 'hide_disabled_user_shares', 'yes', 'yes'], + ]); + $this->appConfig->method('getValueBool') + ->willReturnMap([ + ['files_sharing', 'hide_disabled_user_external_shares', true, true], ]); $this->l->expects($this->once()) @@ -4045,6 +4122,7 @@ public function testGetShareByTokenHideDisabledUser(): void { $date->setTime(0, 0, 0); $date->add(new \DateInterval('P2D')); $share = $this->manager->newShare(); + $share->setShareType(IShare::TYPE_LINK); $share->setExpirationDate($date); $share->setShareOwner('owner'); $share->setSharedBy('sharedBy'); @@ -4112,11 +4190,10 @@ public function testGetShareByTokenExpired(): void { public function testGetShareByTokenNotExpired(): void { $this->config - ->expects($this->exactly(2)) + ->expects($this->exactly(1)) ->method('getAppValue') ->willReturnMap([ ['core', 'shareapi_allow_links', 'yes', 'yes'], - ['files_sharing', 'hide_disabled_user_shares', 'no', 'no'], ]); $date = new \DateTime(); @@ -4148,12 +4225,11 @@ public function testGetShareByTokenWithPublicLinksDisabled(): void { public function testGetShareByTokenPublicUploadDisabled(): void { $this->config - ->expects($this->exactly(5)) + ->expects($this->exactly(4)) ->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', '[]', '[]'], ]); @@ -4186,11 +4262,10 @@ public function testGetShareByTokenShareOwnerExcludedFromLinkShares(): void { $this->expectExceptionMessage('The requested share does not exist anymore'); $this->config - ->expects($this->exactly(4)) + ->expects($this->exactly(3)) ->method('getAppValue') ->willReturnMap([ ['core', 'shareapi_allow_links', 'yes', 'yes'], - ['files_sharing', 'hide_disabled_user_shares', 'no', 'no'], ['core', 'shareapi_allow_links_exclude_groups', '[]', '["excludedGroup"]'], ]); @@ -4226,11 +4301,10 @@ public function testGetShareByTokenShareOwnerExcludedFromLinkShares(): void { public function testGetShareByTokenShareOwnerNotExcludedFromLinkShares(): void { $this->config - ->expects($this->exactly(4)) + ->expects($this->exactly(3)) ->method('getAppValue') ->willReturnMap([ ['core', 'shareapi_allow_links', 'yes', 'yes'], - ['files_sharing', 'hide_disabled_user_shares', 'no', 'no'], ['core', 'shareapi_allow_links_exclude_groups', '[]', '["excludedGroup"]'], ]); diff --git a/tests/lib/Sharing/AbstractSharingManagerTests.php b/tests/lib/Sharing/AbstractSharingManagerTests.php index df4ec3fe5cd35..747f171304037 100644 --- a/tests/lib/Sharing/AbstractSharingManagerTests.php +++ b/tests/lib/Sharing/AbstractSharingManagerTests.php @@ -22,9 +22,11 @@ use NCU\Sharing\ShareUserStatus; use NCU\Sharing\Source\ShareSource; use OC\Core\Sharing\Permission\ReshareSharePermissionType; +use OC\Core\Sharing\Recipient\TokenShareRecipientType; use OC\Sharing\SharingManager; use OCP\EventDispatcher\IEventDispatcher; use OCP\HintException; +use OCP\IAppConfig; use OCP\IDBConnection; use OCP\Interaction\Actions\ShareAction; use OCP\Interaction\InteractionRestrictedException; @@ -268,6 +270,10 @@ protected function tearDown(): void { $this->registry->clear(); + $appConfig = Server::get(IAppConfig::class); + $appConfig->deleteKey('files_sharing', 'hide_disabled_user_shares'); + $appConfig->deleteKey('files_sharing', 'hide_disabled_user_external_shares'); + foreach ([ 'sharing_share', 'sharing_share_permissions', @@ -3729,6 +3735,93 @@ public function testGetShareDisabledOwner(): void { ], $formatted['owner']); } + public static function dataHideDisabledUserShares(): array { + return [ + 'hide all' => [true, true], + 'hide internal' => [true, false], + 'hide external' => [false, true], + 'hide none' => [false, false], + ]; + } + + private function setHideDisabledUserShares(bool $hideInternal, bool $hideExternal): void { + $appConfig = Server::get(IAppConfig::class); + $appConfig->setValueBool('files_sharing', 'hide_disabled_user_shares', $hideInternal); + $appConfig->setValueBool('files_sharing', 'hide_disabled_user_external_shares', $hideExternal); + } + + private function isShareVisible(ShareAccessContext $accessContext, string $id): bool { + try { + $this->getShare($accessContext, $id); + return true; + } catch (HintException $hintException) { + $this->assertEquals('Share not found.', $hintException->getHint()); + return false; + } + } + + /** + * @return non-empty-string + */ + private function getTokenRecipientSecret(Share $share): string { + foreach ($share->recipients as $recipient) { + if ($recipient->class === TokenShareRecipientType::class) { + $this->assertNotNull($recipient->secret); + return $recipient->secret; + } + } + + $this->fail('No token recipient.'); + } + + #[DataProvider('dataHideDisabledUserShares')] + public function testGetShareDisabledOwnerByRecipientKind(bool $hideInternal, bool $hideExternal): void { + $this->registry->registerRecipientType(Server::get(TokenShareRecipientType::class)); + $this->setHideDisabledUserShares($hideInternal, $hideExternal); + + $accessContext = new ShareAccessContext($this->owner); + + $this->dbConnection->beginTransaction(); + $share = $this->manager->createShare($accessContext); + $share = $this->manager->addShareSource($accessContext, $share, new ShareSource(TestShareSourceType1::class, 'source1')); + $share = $this->manager->addShareRecipient($accessContext, $share, new ShareRecipient(TestShareRecipientType1::class, 'recipient1', null)); + $share = $this->manager->addShareRecipient($accessContext, $share, new ShareRecipient(TokenShareRecipientType::class, str_repeat('t', 32), null)); + $share = $this->manager->updateSharePermission($accessContext, $share, new SharePermission(TestSharePermissionType1::class, true)); + $share = $this->manager->updateShareState($accessContext, $share, ShareState::Active); + $this->dbConnection->commit(); + + $secret = $this->getTokenRecipientSecret($share); + $this->owner->setEnabled(false); + + $this->assertSame(!$hideInternal, $this->isShareVisible(new ShareAccessContext(currentUser: $this->user1), $share->id)); + $this->assertSame(!$hideExternal, $this->isShareVisible(new ShareAccessContext(secret: $secret), $share->id)); + } + + #[DataProvider('dataHideDisabledUserShares')] + public function testGetShareDisabledInitiatorByRecipientKind(bool $hideInternal, bool $hideExternal): void { + $this->registry->registerRecipientType(Server::get(TokenShareRecipientType::class)); + $this->setHideDisabledUserShares($hideInternal, $hideExternal); + + $accessContext = new ShareAccessContext($this->owner); + $initiatorAccessContext = new ShareAccessContext(currentUser: $this->user1); + + $this->dbConnection->beginTransaction(); + $share = $this->manager->createShare($accessContext); + $share = $this->manager->addShareSource($accessContext, $share, new ShareSource(TestShareSourceType1::class, 'source1')); + $share = $this->manager->addShareRecipient($accessContext, $share, new ShareRecipient(TestShareRecipientType1::class, 'recipient1', null)); + $share = $this->manager->updateSharePermission($accessContext, $share, new SharePermission(ReshareSharePermissionType::class, true)); + $share = $this->manager->updateShareState($accessContext, $share, ShareState::Active); + $share = $this->manager->addShareRecipient($initiatorAccessContext, $share, new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null)); + $share = $this->manager->addShareRecipient($initiatorAccessContext, $share, new ShareRecipient(TokenShareRecipientType::class, str_repeat('t', 32), null)); + $this->dbConnection->commit(); + + $secret = $this->getTokenRecipientSecret($share); + $this->user1->setEnabled(false); + + $this->assertSame(!$hideInternal, $this->isShareVisible(new ShareAccessContext(currentUser: $this->user2), $share->id)); + $this->assertSame(!$hideExternal, $this->isShareVisible(new ShareAccessContext(secret: $secret), $share->id)); + } + public function testGetShareDisabledInitiator(): void { $accessContext = new ShareAccessContext($this->owner);