From 260892eb3bef81af4ac01d36ba3dde5952abaa5d Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 9 Sep 2019 22:33:03 +0200 Subject: [PATCH] Disable link shares of disabled users Fixes #10869 Signed-off-by: Roeland Jago Douma --- .../lib/Controller/ShareController.php | 12 ++ .../tests/Controller/ShareControllerTest.php | 140 +++++++++++++++++- 2 files changed, 146 insertions(+), 6 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index 1542cbe4924..95e0097b91b 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -271,6 +271,18 @@ class ShareController extends AuthPublicShareController { * @return bool */ private function validateShare(\OCP\Share\IShare $share) { + // If the owner is disabled no access to the linke is granted + $owner = $this->userManager->get($share->getShareOwner()); + if ($owner === null || !$owner->isEnabled()) { + return false; + } + + // If the initiator of the share is disabled no access is granted + $initiator = $this->userManager->get($share->getSharedBy()); + if ($initiator === null || !$initiator->isEnabled()) { + return false; + } + return $share->getNode()->isReadable() && $share->getNode()->isShareable(); } diff --git a/apps/files_sharing/tests/Controller/ShareControllerTest.php b/apps/files_sharing/tests/Controller/ShareControllerTest.php index 8d9162f759d..fbce22b403f 100644 --- a/apps/files_sharing/tests/Controller/ShareControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareControllerTest.php @@ -44,6 +44,7 @@ use OCP\AppFramework\Http\Template\LinkMenuAction; use OCP\AppFramework\Http\Template\PublicTemplateResponse; use OCP\AppFramework\Http\Template\SimpleMenuAction; use OCP\Constants; +use OCP\Files\File; use OCP\Files\NotFoundException; use OCP\Files\Storage; use OCP\IConfig; @@ -201,11 +202,17 @@ class ShareControllerTest extends \Test\TestCase { $this->shareController->setToken('token'); - $owner = $this->getMockBuilder(IUser::class)->getMock(); + $owner = $this->createMock(IUser::class); $owner->method('getDisplayName')->willReturn('ownerDisplay'); $owner->method('getUID')->willReturn('ownerUID'); + $owner->method('isEnabled')->willReturn(true); - $file = $this->getMockBuilder('OCP\Files\File')->getMock(); + $initiator = $this->createMock(IUser::class); + $initiator->method('getDisplayName')->willReturn('initiatorDisplay'); + $initiator->method('getUID')->willReturn('initiatorUID'); + $initiator->method('isEnabled')->willReturn(true); + + $file = $this->createMock(File::class); $file->method('getName')->willReturn('file1.txt'); $file->method('getMimetype')->willReturn('text/plain'); $file->method('getSize')->willReturn(33); @@ -216,6 +223,7 @@ class ShareControllerTest extends \Test\TestCase { $share->setId(42); $share->setPassword('password') ->setShareOwner('ownerUID') + ->setSharedBy('initiatorUID') ->setNode($file) ->setNote($note) ->setTarget('/file1.txt'); @@ -253,7 +261,15 @@ class ShareControllerTest extends \Test\TestCase { ->with('core', 'shareapi_public_link_disclaimertext', null) ->willReturn('My disclaimer text'); - $this->userManager->method('get')->with('ownerUID')->willReturn($owner); + $this->userManager->method('get')->willReturnCallback(function(string $uid) use ($owner, $initiator) { + if ($uid === 'ownerUID') { + return $owner; + } + if ($uid === 'initiatorUID') { + return $initiator; + } + return null; + }); $this->eventDispatcher->expects($this->once()) ->method('dispatch') @@ -325,6 +341,12 @@ class ShareControllerTest extends \Test\TestCase { $owner = $this->getMockBuilder(IUser::class)->getMock(); $owner->method('getDisplayName')->willReturn('ownerDisplay'); $owner->method('getUID')->willReturn('ownerUID'); + $owner->method('isEnabled')->willReturn(true); + + $initiator = $this->createMock(IUser::class); + $initiator->method('getDisplayName')->willReturn('initiatorDisplay'); + $initiator->method('getUID')->willReturn('initiatorUID'); + $initiator->method('isEnabled')->willReturn(true); $file = $this->getMockBuilder('OCP\Files\File')->getMock(); $file->method('getName')->willReturn('file1.txt'); @@ -337,6 +359,7 @@ class ShareControllerTest extends \Test\TestCase { $share->setId(42); $share->setPassword('password') ->setShareOwner('ownerUID') + ->setSharedBy('initiatorUID') ->setNode($file) ->setNote($note) ->setTarget('/file1.txt') @@ -378,7 +401,15 @@ class ShareControllerTest extends \Test\TestCase { ->with('core', 'shareapi_public_link_disclaimertext', null) ->willReturn('My disclaimer text'); - $this->userManager->method('get')->with('ownerUID')->willReturn($owner); + $this->userManager->method('get')->willReturnCallback(function(string $uid) use ($owner, $initiator) { + if ($uid === 'ownerUID') { + return $owner; + } + if ($uid === 'initiatorUID') { + return $initiator; + } + return null; + }); $this->eventDispatcher->expects($this->once()) ->method('dispatch') @@ -451,6 +482,12 @@ class ShareControllerTest extends \Test\TestCase { $owner = $this->getMockBuilder(IUser::class)->getMock(); $owner->method('getDisplayName')->willReturn('ownerDisplay'); $owner->method('getUID')->willReturn('ownerUID'); + $owner->method('isEnabled')->willReturn(true); + + $initiator = $this->createMock(IUser::class); + $initiator->method('getDisplayName')->willReturn('initiatorDisplay'); + $initiator->method('getUID')->willReturn('initiatorUID'); + $initiator->method('isEnabled')->willReturn(true); /* @var MockObject|Storage $storage */ $storage = $this->getMockBuilder(Storage::class) @@ -472,6 +509,7 @@ class ShareControllerTest extends \Test\TestCase { $share->setId(42); $share->setPermissions(Constants::PERMISSION_CREATE) ->setShareOwner('ownerUID') + ->setSharedBy('initiatorUID') ->setNode($folder) ->setTarget('/fileDrop'); @@ -481,7 +519,15 @@ class ShareControllerTest extends \Test\TestCase { ->with('token') ->willReturn($share); - $this->userManager->method('get')->with('ownerUID')->willReturn($owner); + $this->userManager->method('get')->willReturnCallback(function(string $uid) use ($owner, $initiator) { + if ($uid === 'ownerUID') { + return $owner; + } + if ($uid === 'initiatorUID') { + return $initiator; + } + return null; + }); $this->l10n->expects($this->any()) ->method('t') @@ -535,7 +581,7 @@ class ShareControllerTest extends \Test\TestCase { self::assertEquals($expectedResponse, $response); } - + public function testShowShareInvalid() { $this->expectException(\OCP\Files\NotFoundException::class); @@ -604,4 +650,86 @@ class ShareControllerTest extends \Test\TestCase { $expectedResponse = new DataResponse('Share is read-only'); $this->assertEquals($expectedResponse, $response); } + + public function testDisabledOwner() { + $this->shareController->setToken('token'); + + $owner = $this->getMockBuilder(IUser::class)->getMock(); + $owner->method('isEnabled')->willReturn(false); + + $initiator = $this->createMock(IUser::class); + $initiator->method('isEnabled')->willReturn(false); + + /* @var MockObject|Folder $folder */ + $folder = $this->createMock(Folder::class); + + $share = \OC::$server->getShareManager()->newShare(); + $share->setId(42); + $share->setPermissions(Constants::PERMISSION_CREATE) + ->setShareOwner('ownerUID') + ->setSharedBy('initiatorUID') + ->setNode($folder) + ->setTarget('/share'); + + $this->shareManager + ->expects($this->once()) + ->method('getShareByToken') + ->with('token') + ->willReturn($share); + + $this->userManager->method('get')->willReturnCallback(function(string $uid) use ($owner, $initiator) { + if ($uid === 'ownerUID') { + return $owner; + } + if ($uid === 'initiatorUID') { + return $initiator; + } + return null; + }); + + $this->expectException(NotFoundException::class); + + $this->shareController->showShare(); + } + + public function testDisabledInitiator() { + $this->shareController->setToken('token'); + + $owner = $this->getMockBuilder(IUser::class)->getMock(); + $owner->method('isEnabled')->willReturn(false); + + $initiator = $this->createMock(IUser::class); + $initiator->method('isEnabled')->willReturn(true); + + /* @var MockObject|Folder $folder */ + $folder = $this->createMock(Folder::class); + + $share = \OC::$server->getShareManager()->newShare(); + $share->setId(42); + $share->setPermissions(Constants::PERMISSION_CREATE) + ->setShareOwner('ownerUID') + ->setSharedBy('initiatorUID') + ->setNode($folder) + ->setTarget('/share'); + + $this->shareManager + ->expects($this->once()) + ->method('getShareByToken') + ->with('token') + ->willReturn($share); + + $this->userManager->method('get')->willReturnCallback(function(string $uid) use ($owner, $initiator) { + if ($uid === 'ownerUID') { + return $owner; + } + if ($uid === 'initiatorUID') { + return $initiator; + } + return null; + }); + + $this->expectException(NotFoundException::class); + + $this->shareController->showShare(); + } } -- 2.39.5