mirror of
https://github.com/nextcloud/server.git
synced 2026-06-10 17:23:59 -04:00
Merge pull request #43749 from nextcloud/backport/40332/stable25
[stable25] Hide shares by disabled users
This commit is contained in:
Andy Scherzinger
GitHub
commit
0a6586bd56
2 changed files with 98 additions and 22 deletions
|
|
@ -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;
|
||||
private LoggerInterface $logger;
|
||||
|
|
@ -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();
|
||||
|
|
@ -1350,7 +1348,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;
|
||||
|
|
@ -1409,7 +1407,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]);
|
||||
}
|
||||
|
|
@ -1455,7 +1453,7 @@ class Manager implements IManager {
|
|||
|
||||
$share = $provider->getShareById($id, $recipient);
|
||||
|
||||
$this->checkExpireDate($share);
|
||||
$this->checkShare($share);
|
||||
|
||||
return $share;
|
||||
}
|
||||
|
|
@ -1539,7 +1537,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
|
||||
|
|
@ -1552,11 +1550,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'));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -552,7 +552,7 @@ class ManagerTest extends \Test\TestCase {
|
|||
/** @var ValidatePasswordPolicyEvent $event */
|
||||
$this->assertSame('password', $event->getPassword());
|
||||
}
|
||||
);
|
||||
);
|
||||
|
||||
$result = self::invokePrivate($this->manager, 'verifyPassword', ['password']);
|
||||
$this->assertNull($result);
|
||||
|
|
@ -575,7 +575,7 @@ class ManagerTest extends \Test\TestCase {
|
|||
$this->assertSame('password', $event->getPassword());
|
||||
throw new HintException('message', 'password not accepted');
|
||||
}
|
||||
);
|
||||
);
|
||||
|
||||
self::invokePrivate($this->manager, 'verifyPassword', ['password']);
|
||||
}
|
||||
|
|
@ -2715,10 +2715,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);
|
||||
|
||||
|
|
@ -2761,10 +2763,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);
|
||||
|
||||
|
|
@ -2814,10 +2818,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);
|
||||
|
||||
|
|
@ -2865,6 +2871,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');
|
||||
|
|
@ -2902,10 +2963,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);
|
||||
|
|
@ -2937,11 +3000,12 @@ class ManagerTest extends \Test\TestCase {
|
|||
|
||||
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();
|
||||
|
|
|
|||
Loading…
Reference in a new issue