Merge pull request #40332 from nextcloud/backport/39699/stable23

[stable23] Hide shares by disabled users
This commit is contained in:
Stephan Orbaugh 2024-02-27 15:54:46 +01:00 committed by GitHub
commit 941dee9df1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 100 additions and 29 deletions

View file

@ -82,7 +82,6 @@ use Symfony\Component\EventDispatcher\GenericEvent;
* This class is the communication hub for all sharing related operations.
*/
class Manager implements IManager {
/** @var IProviderFactory */
private $factory;
/** @var ILogger */
@ -669,7 +668,6 @@ class Manager implements IManager {
* @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();
@ -1318,7 +1316,7 @@ class Manager implements IManager {
$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;
@ -1377,7 +1375,7 @@ class Manager implements IManager {
// 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]);
}
@ -1423,7 +1421,7 @@ class Manager implements IManager {
$share = $provider->getShareById($id, $recipient);
$this->checkExpireDate($share);
$this->checkShare($share);
return $share;
}
@ -1507,7 +1505,7 @@ class Manager implements IManager {
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
@ -1520,11 +1518,25 @@ class Manager implements IManager {
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'));
}
}
}
}
/**

View file

@ -2714,10 +2714,12 @@ class ManagerTest extends \Test\TestCase {
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);
@ -2760,10 +2762,12 @@ class ManagerTest extends \Test\TestCase {
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);
@ -2813,10 +2817,12 @@ class ManagerTest extends \Test\TestCase {
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);
@ -2866,6 +2872,61 @@ class ManagerTest extends \Test\TestCase {
}
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');
@ -2903,10 +2964,12 @@ class ManagerTest extends \Test\TestCase {
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);
@ -2938,22 +3001,18 @@ class ManagerTest extends \Test\TestCase {
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)
->setPermissions(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE);
$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')