summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--apps/files_sharing/lib/Controller/ShareAPIController.php2
-rw-r--r--apps/files_sharing/lib/MountProvider.php18
-rw-r--r--apps/files_sharing/tests/Controller/ShareAPIControllerTest.php7
-rw-r--r--apps/files_sharing/tests/MountProviderTest.php22
-rw-r--r--lib/private/Group/Manager.php14
-rw-r--r--lib/private/Share20/DefaultShareProvider.php4
-rw-r--r--lib/private/Share20/Manager.php13
-rw-r--r--tests/lib/Group/ManagerTest.php56
-rw-r--r--tests/lib/Share20/DefaultShareProviderTest.php42
-rw-r--r--tests/lib/Share20/ManagerTest.php73
10 files changed, 241 insertions, 10 deletions
diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php
index 6181cde6fe6..fa81e6364a2 100644
--- a/apps/files_sharing/lib/Controller/ShareAPIController.php
+++ b/apps/files_sharing/lib/Controller/ShareAPIController.php
@@ -779,7 +779,7 @@ class ShareAPIController extends OCSController {
if ($checkGroups && $share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) {
$sharedWith = $this->groupManager->get($share->getSharedWith());
$user = $this->userManager->get($this->currentUser);
- if ($user !== null && $sharedWith->inGroup($user)) {
+ if ($user !== null && $sharedWith !== null && $sharedWith->inGroup($user)) {
return true;
}
}
diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php
index 5c5d57057b2..93f85a888b0 100644
--- a/apps/files_sharing/lib/MountProvider.php
+++ b/apps/files_sharing/lib/MountProvider.php
@@ -170,7 +170,23 @@ class MountProvider implements IMountProvider {
if ($share->getTarget() !== $superShare->getTarget()) {
// adjust target, for database consistency
$share->setTarget($superShare->getTarget());
- $this->shareManager->moveShare($share, $user->getUID());
+ try {
+ $this->shareManager->moveShare($share, $user->getUID());
+ } catch (\InvalidArgumentException $e) {
+ // ignore as it is not important and we don't want to
+ // block FS setup
+
+ // the subsequent code anyway only uses the target of the
+ // super share
+
+ // such issue can usually happen when dealing with
+ // null groups which usually appear with group backend
+ // caching inconsistencies
+ \OC::$server->getLogger()->debug(
+ 'Could not adjust share target for share ' . $share->getId() . ' to make it consistent: ' . $e->getMessage(),
+ ['app' => 'files_sharing']
+ );
+ }
}
if (!is_null($share->getNodeCacheEntry())) {
$superShare->setNodeCacheEntry($share->getNodeCacheEntry());
diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
index 97774081b6a..2bf9a40b2cc 100644
--- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
+++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
@@ -544,14 +544,19 @@ class ShareAPIControllerTest extends \Test\TestCase {
$this->groupManager->method('get')->will($this->returnValueMap([
['group', $group],
['group2', $group2],
+ ['groupnull', null],
]));
$this->assertTrue($this->invokePrivate($this->ocs, 'canAccessShare', [$share]));
$share = $this->getMockBuilder('OCP\Share\IShare')->getMock();
$share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_GROUP);
$share->method('getSharedWith')->willReturn('group2');
+ $this->assertFalse($this->invokePrivate($this->ocs, 'canAccessShare', [$share]));
- $this->groupManager->method('get')->with('group2')->willReturn($group);
+ // null group
+ $share = $this->getMock('OCP\Share\IShare');
+ $share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_GROUP);
+ $share->method('getSharedWith')->willReturn('groupnull');
$this->assertFalse($this->invokePrivate($this->ocs, 'canAccessShare', [$share]));
$share = $this->getMockBuilder('OCP\Share\IShare')->getMock();
diff --git a/apps/files_sharing/tests/MountProviderTest.php b/apps/files_sharing/tests/MountProviderTest.php
index 0be74a645a9..902c81c7136 100644
--- a/apps/files_sharing/tests/MountProviderTest.php
+++ b/apps/files_sharing/tests/MountProviderTest.php
@@ -263,6 +263,20 @@ class MountProviderTest extends \Test\TestCase {
['1', 100, 'user2', '/share2-renamed', 31],
],
],
+ // #9: share as outsider with "nullgroup" and "user1" where recipient renamed in between
+ [
+ [
+ [2, 100, 'user2', '/share2', 31],
+ ],
+ [
+ [1, 100, 'nullgroup', '/share2-renamed', 31],
+ ],
+ [
+ // use target of least recent share
+ ['1', 100, 'nullgroup', '/share2-renamed', 31],
+ ],
+ true
+ ],
];
}
@@ -278,7 +292,7 @@ class MountProviderTest extends \Test\TestCase {
* @param array $groupShares array of group share specs
* @param array $expectedShares array of expected supershare specs
*/
- public function testMergeShares($userShares, $groupShares, $expectedShares) {
+ public function testMergeShares($userShares, $groupShares, $expectedShares, $moveFails = false) {
$rootFolder = $this->createMock(IRootFolder::class);
$userManager = $this->createMock(IUserManager::class);
@@ -307,6 +321,12 @@ class MountProviderTest extends \Test\TestCase {
return new \OC\Share20\Share($rootFolder, $userManager);
}));
+ if ($moveFails) {
+ $this->shareManager->expects($this->any())
+ ->method('moveShare')
+ ->will($this->throwException(new \InvalidArgumentException()));
+ }
+
$mounts = $this->provider->getMountsForUser($this->user, $this->loader);
$this->assertCount(count($expectedShares), $mounts);
diff --git a/lib/private/Group/Manager.php b/lib/private/Group/Manager.php
index 944598a8296..8b18abd5e95 100644
--- a/lib/private/Group/Manager.php
+++ b/lib/private/Group/Manager.php
@@ -223,7 +223,12 @@ class Manager extends PublicEmitter implements IGroupManager {
foreach ($this->backends as $backend) {
$groupIds = $backend->getGroups($search, $limit, $offset);
foreach ($groupIds as $groupId) {
- $groups[$groupId] = $this->get($groupId);
+ $aGroup = $this->get($groupId);
+ if (!is_null($aGroup)) {
+ $groups[$groupId] = $aGroup;
+ } else {
+ \OC::$server->getLogger()->debug('Group "' . $groupId . '" was returned by search but not found through direct access', array('app' => 'core'));
+ }
}
if (!is_null($limit) and $limit <= 0) {
return array_values($groups);
@@ -256,7 +261,12 @@ class Manager extends PublicEmitter implements IGroupManager {
$groupIds = $backend->getUserGroups($uid);
if (is_array($groupIds)) {
foreach ($groupIds as $groupId) {
- $groups[$groupId] = $this->get($groupId);
+ $aGroup = $this->get($groupId);
+ if (!is_null($aGroup)) {
+ $groups[$groupId] = $aGroup;
+ } else {
+ \OC::$server->getLogger()->debug('User "' . $uid . '" belongs to deleted group: "' . $groupId . '"', array('app' => 'core'));
+ }
}
}
}
diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php
index fe6472c31a0..e4ae26be13d 100644
--- a/lib/private/Share20/DefaultShareProvider.php
+++ b/lib/private/Share20/DefaultShareProvider.php
@@ -329,6 +329,10 @@ class DefaultShareProvider implements IShareProvider {
$group = $this->groupManager->get($share->getSharedWith());
$user = $this->userManager->get($recipient);
+ if (is_null($group)) {
+ throw new ProviderException('Group "' . $share->getSharedWith() . '" does not exist');
+ }
+
if (!$group->inGroup($user)) {
throw new ProviderException('Recipient not in receiving group');
}
diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
index acc142f62be..6b86ec4f456 100644
--- a/lib/private/Share20/Manager.php
+++ b/lib/private/Share20/Manager.php
@@ -393,10 +393,12 @@ class Manager implements IManager {
// The share is already shared with this user via a group share
if ($existingShare->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) {
$group = $this->groupManager->get($existingShare->getSharedWith());
- $user = $this->userManager->get($share->getSharedWith());
+ if (!is_null($group)) {
+ $user = $this->userManager->get($share->getSharedWith());
- if ($group->inGroup($user) && $existingShare->getShareOwner() !== $share->getShareOwner()) {
- throw new \Exception('Path already shared with this user');
+ if ($group->inGroup($user) && $existingShare->getShareOwner() !== $share->getShareOwner()) {
+ throw new \Exception('Path already shared with this user');
+ }
}
}
}
@@ -418,7 +420,7 @@ class Manager implements IManager {
if ($this->shareWithGroupMembersOnly()) {
$sharedBy = $this->userManager->get($share->getSharedBy());
$sharedWith = $this->groupManager->get($share->getSharedWith());
- if (!$sharedWith->inGroup($sharedBy)) {
+ if (is_null($sharedWith) || !$sharedWith->inGroup($sharedBy)) {
throw new \Exception('Only sharing within your own groups is allowed');
}
}
@@ -886,6 +888,9 @@ class Manager implements IManager {
if ($share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) {
$sharedWith = $this->groupManager->get($share->getSharedWith());
+ if (is_null($sharedWith)) {
+ throw new \InvalidArgumentException('Group "' . $share->getSharedWith() . '" does not exist');
+ }
$recipient = $this->userManager->get($recipientId);
if (!$sharedWith->inGroup($recipient)) {
throw new \InvalidArgumentException('Invalid recipient');
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)
generated by cgit v1.2.3 (git 2.39.1) at 2025-01-09 22:57:41 +0000