Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[stable26] Hide shares by disabled users #40476

Merged
merged 3 commits into from
Sep 18, 2023
Merged

[stable26] Hide shares by disabled users #40476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f0c3cbe
Set files_sharing:hide_disabled_user_shares to 'yes' to hide shares f…
come-nc Aug 3, 2023
90a14db
Fix tests, add test for the new feature
come-nc Aug 7, 2023
49d52dd
Use nullsafe call syntax instead of additionnal check
come-nc Aug 10, 2023
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
24 changes: 19 additions & 5 deletions lib/private/Share20/Manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1347,7 +1347,7 @@ public function getSharesBy($userId, $shareType, $path = null, $reshares = false
$added = 0;
foreach ($shares as $share) {
try {
$this->checkExpireDate($share);
$this->checkShare($share);
} catch (ShareNotFound $e) {
//Ignore since this basically means the share is deleted
continue;
Expand Down Expand Up @@ -1406,7 +1406,7 @@ public function getSharedWith($userId, $shareType, $node = null, $limit = 50, $o
// remove all shares which are already expired
foreach ($shares as $key => $share) {
try {
$this->checkExpireDate($share);
$this->checkShare($share);
} catch (ShareNotFound $e) {
unset($shares[$key]);
}
Expand Down Expand Up @@ -1452,7 +1452,7 @@ public function getShareById($id, $recipient = null) {

$share = $provider->getShareById($id, $recipient);

$this->checkExpireDate($share);
$this->checkShare($share);

return $share;
}
Expand Down Expand Up @@ -1536,7 +1536,7 @@ public function getShareByToken($token) {
throw new ShareNotFound($this->l->t('The requested share does not exist anymore'));
}

$this->checkExpireDate($share);
$this->checkShare($share);

/*
* Reduce the permissions for link or email shares if public upload is not enabled
Expand All @@ -1549,11 +1549,25 @@ public function getShareByToken($token) {
return $share;
}

protected function checkExpireDate($share) {
/**
* Check expire date and disabled owner
*
* @throws ShareNotFound
*/
protected function checkShare(IShare $share): void {
if ($share->isExpired()) {
$this->deleteShare($share);
throw new ShareNotFound($this->l->t('The requested share does not exist anymore'));
}
if ($this->config->getAppValue('files_sharing', 'hide_disabled_user_shares', 'no') === 'yes') {
$uids = array_unique([$share->getShareOwner(),$share->getSharedBy()]);
foreach ($uids as $uid) {
$user = $this->userManager->get($uid);
if ($user?->isEnabled() === false) {
throw new ShareNotFound($this->l->t('The requested share comes from a disabled user'));
}
}
}
}

/**
Expand Down
90 changes: 77 additions & 13 deletions tests/lib/Share20/ManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2721,10 +2721,12 @@ public function testGetSharesByExpiredLinkShares() {

public function testGetShareByToken() {
$this->config
->expects($this->once())
->expects($this->exactly(2))
->method('getAppValue')
->with('core', 'shareapi_allow_links', 'yes')
->willReturn('yes');
->willReturnMap([
['core', 'shareapi_allow_links', 'yes', 'yes'],
['files_sharing', 'hide_disabled_user_shares', 'no', 'no'],
]);

$factory = $this->createMock(IProviderFactory::class);

Expand Down Expand Up @@ -2768,10 +2770,12 @@ public function testGetShareByToken() {

public function testGetShareByTokenRoom() {
$this->config
->expects($this->once())
->expects($this->exactly(2))
->method('getAppValue')
->with('core', 'shareapi_allow_links', 'yes')
->willReturn('no');
->willReturnMap([
['core', 'shareapi_allow_links', 'yes', 'no'],
['files_sharing', 'hide_disabled_user_shares', 'no', 'no'],
]);

$factory = $this->createMock(IProviderFactory::class);

Expand Down Expand Up @@ -2822,10 +2826,12 @@ public function testGetShareByTokenRoom() {

public function testGetShareByTokenWithException() {
$this->config
->expects($this->once())
->expects($this->exactly(2))
->method('getAppValue')
->with('core', 'shareapi_allow_links', 'yes')
->willReturn('yes');
->willReturnMap([
['core', 'shareapi_allow_links', 'yes', 'yes'],
['files_sharing', 'hide_disabled_user_shares', 'no', 'no'],
]);

$factory = $this->createMock(IProviderFactory::class);

Expand Down Expand Up @@ -2874,6 +2880,61 @@ public function testGetShareByTokenWithException() {
}


public function testGetShareByTokenHideDisabledUser() {
$this->expectException(\OCP\Share\Exceptions\ShareNotFound::class);
$this->expectExceptionMessage('The requested share comes from a disabled user');

$this->config
->expects($this->exactly(2))
->method('getAppValue')
->willReturnMap([
['core', 'shareapi_allow_links', 'yes', 'yes'],
['files_sharing', 'hide_disabled_user_shares', 'no', 'yes'],
]);

$this->l->expects($this->once())
->method('t')
->willReturnArgument(0);

$manager = $this->createManagerMock()
->setMethods(['deleteShare'])
->getMock();

$date = new \DateTime();
$date->setTime(0, 0, 0);
$date->add(new \DateInterval('P2D'));
$share = $this->manager->newShare();
$share->setExpirationDate($date);
$share->setShareOwner('owner');
$share->setSharedBy('sharedBy');

$sharedBy = $this->createMock(IUser::class);
$owner = $this->createMock(IUser::class);

$this->userManager->method('get')->willReturnMap([
['sharedBy', $sharedBy],
['owner', $owner],
]);

$owner->expects($this->once())
->method('isEnabled')
->willReturn(true);
$sharedBy->expects($this->once())
->method('isEnabled')
->willReturn(false);

$this->defaultProvider->expects($this->once())
->method('getShareByToken')
->with('expiredToken')
->willReturn($share);

$manager->expects($this->never())
->method('deleteShare');

$manager->getShareByToken('expiredToken');
}


public function testGetShareByTokenExpired() {
$this->expectException(\OCP\Share\Exceptions\ShareNotFound::class);
$this->expectExceptionMessage('The requested share does not exist anymore');
Expand Down Expand Up @@ -2911,10 +2972,12 @@ public function testGetShareByTokenExpired() {

public function testGetShareByTokenNotExpired() {
$this->config
->expects($this->once())
->expects($this->exactly(2))
->method('getAppValue')
->with('core', 'shareapi_allow_links', 'yes')
->willReturn('yes');
->willReturnMap([
['core', 'shareapi_allow_links', 'yes', 'yes'],
['files_sharing', 'hide_disabled_user_shares', 'no', 'no'],
]);

$date = new \DateTime();
$date->setTime(0, 0, 0);
Expand Down Expand Up @@ -2946,11 +3009,12 @@ public function testGetShareByTokenWithPublicLinksDisabled() {

public function testGetShareByTokenPublicUploadDisabled() {
$this->config
->expects($this->exactly(2))
->expects($this->exactly(3))
->method('getAppValue')
->willReturnMap([
['core', 'shareapi_allow_links', 'yes', 'yes'],
['core', 'shareapi_allow_public_upload', 'yes', 'no'],
['files_sharing', 'hide_disabled_user_shares', 'no', 'no'],
]);

$share = $this->manager->newShare();
Expand Down
Loading