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

[stable24] Hide shares by disabled users #43748

Merged
merged 4 commits into from
Feb 24, 2024
Merged

[stable24] Hide shares by disabled users #43748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
99142a6
Set files_sharing:hide_disabled_user_shares to 'yes' to hide shares f…
come-nc Aug 3, 2023
da391a5
Fix tests, add test for the new feature
come-nc Aug 7, 2023
92862be
Use nullsafe call syntax instead of additionnal check
come-nc Aug 10, 2023
6e6797b
Remove modern PHP syntax, and apply codesniffer fixes in Share20/Mana…
come-nc Sep 7, 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
26 changes: 19 additions & 7 deletions lib/private/Share20/Manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@
* This class is the communication hub for all sharing related operations.
*/
class Manager implements IManager {

/** @var IProviderFactory */
private $factory;
private LoggerInterface $logger;
Expand Down Expand Up @@ -668,7 +667,6 @@ protected function linkCreateChecks(IShare $share) {
* @param IShare $share
*/
protected function setLinkParent(IShare $share) {

// No sense in checking if the method is not there.
if (method_exists($share, 'setParent')) {
$storage = $share->getNode()->getStorage();
Expand Down Expand Up @@ -1349,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 @@ -1408,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 @@ -1454,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 @@ -1538,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 @@ -1551,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 !== null) && ($user->isEnabled() === false)) {
throw new ShareNotFound($this->l->t('The requested share comes from a disabled user'));
}
}
}
}

/**
Expand Down
103 changes: 81 additions & 22 deletions tests/lib/Share20/ManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2739,10 +2739,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 @@ -2785,10 +2787,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 @@ -2838,10 +2842,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 @@ -2891,6 +2897,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 @@ -2928,10 +2989,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 @@ -2963,10 +3026,13 @@ public function testGetShareByTokenWithPublicLinksDisabled() {

public function testGetShareByTokenPublicUploadDisabled() {
$this->config
->expects($this->at(0))
->expects($this->exactly(3))
->method('getAppValue')
->with('core', 'shareapi_allow_links', 'yes')
->willReturn('yes');
->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();
$share->setShareType(IShare::TYPE_LINK)
Expand All @@ -2975,13 +3041,6 @@ public function testGetShareByTokenPublicUploadDisabled() {
$folder = $this->createMock(\OC\Files\Node\Folder::class);
$share->setNode($folder);

$this->config
->expects($this->at(1))
->method('getAppValue')
->willReturnMap([
['core', 'shareapi_allow_public_upload', 'yes', 'no'],
]);

$this->defaultProvider->expects($this->once())
->method('getShareByToken')
->willReturn('validToken')
Expand Down
Loading