From 377fdf3860e317b68392c678fa25e081251baecc Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 19 Jan 2017 15:02:46 +0100 Subject: Skip null groups in group manager (#26871) (#26956) * Skip null groups in group manager (#26871) * Skip null groups in group manager * Also skip null groups in group manager's search function * Add more group null checks in sharing code * Add unit tests for null group safety in group manager * Add unit tests for sharing code null group checks * Added tests for null groups handling in sharing code * Ignore moveShare optional repair in mount provider In some cases, data is inconsistent in the oc_share table due to legacy data. The mount provider might attempt to make it consistent but if the target group does not exist any more it cannot work. In such case we simply ignore the exception as it is not critical. Keeping the exception would break user accounts as they would be unable to use their filesystem. * Adjust null group handing + tests * Fix new group manager tests Signed-off-by: Morris Jobke --- tests/lib/Group/ManagerTest.php | 56 ++++++++++++++++++++ tests/lib/Share20/DefaultShareProviderTest.php | 42 +++++++++++++++ tests/lib/Share20/ManagerTest.php | 73 ++++++++++++++++++++++++++ 3 files changed, 171 insertions(+) (limited to 'tests') diff --git a/tests/lib/Group/ManagerTest.php b/tests/lib/Group/ManagerTest.php index 1a7ced5f1ba..c57ef0b34cb 100644 --- a/tests/lib/Group/ManagerTest.php +++ b/tests/lib/Group/ManagerTest.php @@ -302,6 +302,32 @@ class ManagerTest extends \Test\TestCase { $this->assertEquals('group12', $group12->getGID()); } + public function testSearchResultExistsButGroupDoesNot() { + /** + * @var \PHPUnit_Framework_MockObject_MockObject | \OC\Group\Backend $backend + */ + $backend = $this->createMock('\OC\Group\Database'); + $backend->expects($this->once()) + ->method('getGroups') + ->with('1') + ->will($this->returnValue(['group1'])); + $backend->expects($this->once()) + ->method('groupExists') + ->with('group1') + ->will($this->returnValue(false)); + + /** + * @var \OC\User\Manager $userManager + */ + $userManager = $this->createMock('\OC\User\Manager'); + + $manager = new \OC\Group\Manager($userManager); + $manager->addBackend($backend); + + $groups = $manager->search('1'); + $this->assertEmpty($groups); + } + public function testGetUserGroups() { /** * @var \PHPUnit_Framework_MockObject_MockObject | \OC\Group\Backend $backend @@ -351,6 +377,36 @@ class ManagerTest extends \Test\TestCase { } } + public function testGetUserGroupsWithDeletedGroup() { + /** + * @var \PHPUnit_Framework_MockObject_MockObject | \OC\Group\Backend $backend + */ + $backend = $this->createMock('\OC\Group\Database'); + $backend->expects($this->once()) + ->method('getUserGroups') + ->with('user1') + ->will($this->returnValue(array('group1'))); + $backend->expects($this->any()) + ->method('groupExists') + ->with('group1') + ->will($this->returnValue(false)); + + /** + * @var \OC\User\Manager $userManager + */ + $userManager = $this->createMock('\OC\User\Manager'); + $userBackend = $this->createMock('\OC_User_Backend'); + $manager = new \OC\Group\Manager($userManager); + $manager->addBackend($backend); + + /** @var \OC\User\User $user */ + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('user1'); + + $groups = $manager->getUserGroups($user); + $this->assertEmpty($groups); + } + public function testInGroup() { /** * @var \PHPUnit_Framework_MockObject_MockObject | \OC\Group\Backend $backend diff --git a/tests/lib/Share20/DefaultShareProviderTest.php b/tests/lib/Share20/DefaultShareProviderTest.php index 5870da8d218..a1e78313558 100644 --- a/tests/lib/Share20/DefaultShareProviderTest.php +++ b/tests/lib/Share20/DefaultShareProviderTest.php @@ -1524,6 +1524,48 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->provider->deleteFromSelf($share, 'user2'); } + /** + * @expectedException \OC\Share20\Exception\ProviderException + * @expectedExceptionMessage Group "group" does not exist + */ + public function testDeleteFromSelfGroupDoesNotExist() { + $qb = $this->dbConn->getQueryBuilder(); + $stmt = $qb->insert('share') + ->values([ + 'share_type' => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_GROUP), + 'share_with' => $qb->expr()->literal('group'), + 'uid_owner' => $qb->expr()->literal('user1'), + 'uid_initiator' => $qb->expr()->literal('user1'), + 'item_type' => $qb->expr()->literal('file'), + 'file_source' => $qb->expr()->literal(1), + 'file_target' => $qb->expr()->literal('myTarget1'), + 'permissions' => $qb->expr()->literal(2) + ])->execute(); + $this->assertEquals(1, $stmt); + $id = $qb->getLastInsertId(); + + $user1 = $this->getMock('\OCP\IUser'); + $user1->method('getUID')->willReturn('user1'); + $user2 = $this->getMock('\OCP\IUser'); + $user2->method('getUID')->willReturn('user2'); + $this->userManager->method('get')->will($this->returnValueMap([ + ['user1', $user1], + ['user2', $user2], + ])); + + $this->groupManager->method('get')->with('group')->willReturn(null); + + $file = $this->getMock('\OCP\Files\File'); + $file->method('getId')->willReturn(1); + + $this->rootFolder->method('getUserFolder')->with('user1')->will($this->returnSelf()); + $this->rootFolder->method('getById')->with(1)->willReturn([$file]); + + $share = $this->provider->getShareById($id); + + $this->provider->deleteFromSelf($share, 'user2'); + } + public function testDeleteFromSelfUser() { $qb = $this->dbConn->getQueryBuilder(); $stmt = $qb->insert('share') diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index 7b01a8f9e70..25f406c1405 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -1139,6 +1139,39 @@ class ManagerTest extends \Test\TestCase { self::invokePrivate($this->manager, 'userCreateChecks', [$share]); } + public function testUserCreateChecksIdenticalPathSharedViaDeletedGroup() { + $share = $this->manager->newShare(); + + $sharedWith = $this->getMock('\OCP\IUser'); + $sharedWith->method('getUID')->willReturn('sharedWith'); + + $this->userManager->method('get')->with('sharedWith')->willReturn($sharedWith); + + $path = $this->getMock('\OCP\Files\Node'); + + $share->setSharedWith('sharedWith') + ->setNode($path) + ->setShareOwner('shareOwner') + ->setProviderId('foo') + ->setId('bar'); + + $share2 = $this->manager->newShare(); + $share2->setShareType(\OCP\Share::SHARE_TYPE_GROUP) + ->setShareOwner('shareOwner2') + ->setProviderId('foo') + ->setId('baz') + ->setSharedWith('group'); + + $this->groupManager->method('get')->with('group')->willReturn(null); + + $this->defaultProvider + ->method('getSharesByPath') + ->with($path) + ->willReturn([$share2]); + + $this->assertNull($this->invokePrivate($this->manager, 'userCreateChecks', [$share])); + } + public function testUserCreateChecksIdenticalPathNotSharedWithUser() { $share = $this->manager->newShare(); $sharedWith = $this->createMock(IUser::class); @@ -1216,6 +1249,29 @@ class ManagerTest extends \Test\TestCase { self::invokePrivate($this->manager, 'groupCreateChecks', [$share]); } + /** + * @expectedException Exception + * @expectedExceptionMessage Only sharing within your own groups is allowed + */ + public function testGroupCreateChecksShareWithGroupMembersOnlyNullGroup() { + $share = $this->manager->newShare(); + + $user = $this->getMock('\OCP\IUser'); + $share->setSharedBy('user')->setSharedWith('group'); + + $this->groupManager->method('get')->with('group')->willReturn(null); + $this->userManager->method('get')->with('user')->willReturn($user); + + $this->config + ->method('getAppValue') + ->will($this->returnValueMap([ + ['core', 'shareapi_only_share_with_group_members', 'no', 'yes'], + ['core', 'shareapi_allow_group_sharing', 'yes', 'yes'], + ])); + + $this->assertNull($this->invokePrivate($this->manager, 'groupCreateChecks', [$share])); + } + public function testGroupCreateChecksShareWithGroupMembersOnlyInGroup() { $share = $this->manager->newShare(); @@ -2666,6 +2722,23 @@ class ManagerTest extends \Test\TestCase { $this->manager->moveShare($share, 'recipient'); } + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Group "shareWith" does not exist + */ + public function testMoveShareGroupNull() { + $share = $this->manager->newShare(); + $share->setShareType(\OCP\Share::SHARE_TYPE_GROUP); + $share->setSharedWith('shareWith'); + + $recipient = $this->getMock('\OCP\IUser'); + + $this->groupManager->method('get')->with('shareWith')->willReturn(null); + $this->userManager->method('get')->with('recipient')->willReturn($recipient); + + $this->manager->moveShare($share, 'recipient'); + } + public function testMoveShareGroup() { $share = $this->manager->newShare(); $share->setShareType(\OCP\Share::SHARE_TYPE_GROUP) -- cgit v1.2.3