Disable link shares of disabled users

Fixes #10869

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php
index 1542cbe4924600db285d380334f38a5955f034b7..95e0097b91bc5ec09c56e0fc8cbc26fc2df99678 100644 (file)
--- 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 8d9162f759d40db3af33466f18cb8d41288d1e5c..fbce22b403ff15b7d2bc1325dff7536697fa534a 100644 (file)
--- 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();
+       }
 }