Merge pull request #3884 from nextcloud/downstream-26956

Skip null groups in group manager
This commit is contained in:
Joas Schilling 2017-03-20 12:27:38 +01:00 committed by GitHub
commit 35f6b8716e
11 changed files with 315 additions and 103 deletions

View File

@ -804,7 +804,7 @@ class ShareAPIController extends OCSController {
if ($checkGroups && $share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) { if ($checkGroups && $share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) {
$sharedWith = $this->groupManager->get($share->getSharedWith()); $sharedWith = $this->groupManager->get($share->getSharedWith());
$user = $this->userManager->get($this->currentUser); $user = $this->userManager->get($this->currentUser);
if ($user !== null && $sharedWith->inGroup($user)) { if ($user !== null && $sharedWith !== null && $sharedWith->inGroup($user)) {
return true; return true;
} }
} }

View File

@ -173,7 +173,23 @@ class MountProvider implements IMountProvider {
if ($share->getTarget() !== $superShare->getTarget()) { if ($share->getTarget() !== $superShare->getTarget()) {
// adjust target, for database consistency // adjust target, for database consistency
$share->setTarget($superShare->getTarget()); $share->setTarget($superShare->getTarget());
try {
$this->shareManager->moveShare($share, $user->getUID()); $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
$this->logger->debug(
'Could not adjust share target for share ' . $share->getId() . ' to make it consistent: ' . $e->getMessage(),
['app' => 'files_sharing']
);
}
} }
if (!is_null($share->getNodeCacheEntry())) { if (!is_null($share->getNodeCacheEntry())) {
$superShare->setNodeCacheEntry($share->getNodeCacheEntry()); $superShare->setNodeCacheEntry($share->getNodeCacheEntry());

View File

@ -544,14 +544,19 @@ class ShareAPIControllerTest extends \Test\TestCase {
$this->groupManager->method('get')->will($this->returnValueMap([ $this->groupManager->method('get')->will($this->returnValueMap([
['group', $group], ['group', $group],
['group2', $group2], ['group2', $group2],
['groupnull', null],
])); ]));
$this->assertTrue($this->invokePrivate($this->ocs, 'canAccessShare', [$share])); $this->assertTrue($this->invokePrivate($this->ocs, 'canAccessShare', [$share]));
$share = $this->getMockBuilder('OCP\Share\IShare')->getMock(); $share = $this->getMockBuilder('OCP\Share\IShare')->getMock();
$share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_GROUP); $share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_GROUP);
$share->method('getSharedWith')->willReturn('group2'); $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])); $this->assertFalse($this->invokePrivate($this->ocs, 'canAccessShare', [$share]));
$share = $this->getMockBuilder('OCP\Share\IShare')->getMock(); $share = $this->getMockBuilder('OCP\Share\IShare')->getMock();

View File

@ -269,6 +269,20 @@ class MountProviderTest extends \Test\TestCase {
['1', 100, 'user2', '/share2-renamed', 31], ['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
],
]; ];
} }
@ -284,7 +298,7 @@ class MountProviderTest extends \Test\TestCase {
* @param array $groupShares array of group share specs * @param array $groupShares array of group share specs
* @param array $expectedShares array of expected supershare 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); $rootFolder = $this->createMock(IRootFolder::class);
$userManager = $this->createMock(IUserManager::class); $userManager = $this->createMock(IUserManager::class);
@ -319,6 +333,12 @@ class MountProviderTest extends \Test\TestCase {
return new \OC\Share20\Share($rootFolder, $userManager); 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); $mounts = $this->provider->getMountsForUser($this->user, $this->loader);
$this->assertCount(count($expectedShares), $mounts); $this->assertCount(count($expectedShares), $mounts);

View File

@ -37,7 +37,10 @@ namespace OC\Group;
use OC\Hooks\PublicEmitter; use OC\Hooks\PublicEmitter;
use OCP\GroupInterface; use OCP\GroupInterface;
use OCP\IGroup;
use OCP\IGroupManager; use OCP\IGroupManager;
use OCP\ILogger;
use OCP\IUser;
/** /**
* Class Manager * Class Manager
@ -78,11 +81,16 @@ class Manager extends PublicEmitter implements IGroupManager {
/** @var \OC\SubAdmin */ /** @var \OC\SubAdmin */
private $subAdmin = null; private $subAdmin = null;
/** @var ILogger */
private $logger;
/** /**
* @param \OC\User\Manager $userManager * @param \OC\User\Manager $userManager
* @param ILogger $logger
*/ */
public function __construct(\OC\User\Manager $userManager) { public function __construct(\OC\User\Manager $userManager, ILogger $logger) {
$this->userManager = $userManager; $this->userManager = $userManager;
$this->logger = $logger;
$cachedGroups = & $this->cachedGroups; $cachedGroups = & $this->cachedGroups;
$cachedUserGroups = & $this->cachedUserGroups; $cachedUserGroups = & $this->cachedUserGroups;
$this->listen('\OC\Group', 'postDelete', function ($group) use (&$cachedGroups, &$cachedUserGroups) { $this->listen('\OC\Group', 'postDelete', function ($group) use (&$cachedGroups, &$cachedUserGroups) {
@ -186,7 +194,7 @@ class Manager extends PublicEmitter implements IGroupManager {
* @return bool * @return bool
*/ */
public function groupExists($gid) { public function groupExists($gid) {
return !is_null($this->get($gid)); return $this->get($gid) instanceof IGroup;
} }
/** /**
@ -194,7 +202,7 @@ class Manager extends PublicEmitter implements IGroupManager {
* @return \OC\Group\Group * @return \OC\Group\Group
*/ */
public function createGroup($gid) { public function createGroup($gid) {
if ($gid === '' || is_null($gid)) { if ($gid === '' || $gid === null) {
return false; return false;
} else if ($group = $this->get($gid)) { } else if ($group = $this->get($gid)) {
return $group; return $group;
@ -223,7 +231,12 @@ class Manager extends PublicEmitter implements IGroupManager {
foreach ($this->backends as $backend) { foreach ($this->backends as $backend) {
$groupIds = $backend->getGroups($search, $limit, $offset); $groupIds = $backend->getGroups($search, $limit, $offset);
foreach ($groupIds as $groupId) { foreach ($groupIds as $groupId) {
$groups[$groupId] = $this->get($groupId); $aGroup = $this->get($groupId);
if ($aGroup instanceof IGroup) {
$groups[$groupId] = $aGroup;
} else {
$this->logger->debug('Group "' . $groupId . '" was returned by search but not found through direct access', ['app' => 'core']);
}
} }
if (!is_null($limit) and $limit <= 0) { if (!is_null($limit) and $limit <= 0) {
return array_values($groups); return array_values($groups);
@ -237,7 +250,7 @@ class Manager extends PublicEmitter implements IGroupManager {
* @return \OC\Group\Group[] * @return \OC\Group\Group[]
*/ */
public function getUserGroups($user) { public function getUserGroups($user) {
if (is_null($user)) { if (!$user instanceof IUser) {
return []; return [];
} }
return $this->getUserIdGroups($user->getUID()); return $this->getUserIdGroups($user->getUID());
@ -256,7 +269,12 @@ class Manager extends PublicEmitter implements IGroupManager {
$groupIds = $backend->getUserGroups($uid); $groupIds = $backend->getUserGroups($uid);
if (is_array($groupIds)) { if (is_array($groupIds)) {
foreach ($groupIds as $groupId) { foreach ($groupIds as $groupId) {
$groups[$groupId] = $this->get($groupId); $aGroup = $this->get($groupId);
if ($aGroup instanceof IGroup) {
$groups[$groupId] = $aGroup;
} else {
$this->logger->debug('User "' . $uid . '" belongs to deleted group: "' . $groupId . '"', ['app' => 'core']);
}
} }
} }
} }

View File

@ -227,7 +227,7 @@ class Server extends ServerContainer implements IServerContainer {
return new \OC\User\Manager($config); return new \OC\User\Manager($config);
}); });
$this->registerService('GroupManager', function (Server $c) { $this->registerService('GroupManager', function (Server $c) {
$groupManager = new \OC\Group\Manager($this->getUserManager()); $groupManager = new \OC\Group\Manager($this->getUserManager(), $this->getLogger());
$groupManager->listen('\OC\Group', 'preCreate', function ($gid) { $groupManager->listen('\OC\Group', 'preCreate', function ($gid) {
\OC_Hook::emit('OC_Group', 'pre_createGroup', array('run' => true, 'gid' => $gid)); \OC_Hook::emit('OC_Group', 'pre_createGroup', array('run' => true, 'gid' => $gid));
}); });

View File

@ -329,6 +329,10 @@ class DefaultShareProvider implements IShareProvider {
$group = $this->groupManager->get($share->getSharedWith()); $group = $this->groupManager->get($share->getSharedWith());
$user = $this->userManager->get($recipient); $user = $this->userManager->get($recipient);
if (is_null($group)) {
throw new ProviderException('Group "' . $share->getSharedWith() . '" does not exist');
}
if (!$group->inGroup($user)) { if (!$group->inGroup($user)) {
throw new ProviderException('Recipient not in receiving group'); throw new ProviderException('Recipient not in receiving group');
} }

View File

@ -398,6 +398,7 @@ class Manager implements IManager {
// The share is already shared with this user via a group share // The share is already shared with this user via a group share
if ($existingShare->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) { if ($existingShare->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) {
$group = $this->groupManager->get($existingShare->getSharedWith()); $group = $this->groupManager->get($existingShare->getSharedWith());
if (!is_null($group)) {
$user = $this->userManager->get($share->getSharedWith()); $user = $this->userManager->get($share->getSharedWith());
if ($group->inGroup($user) && $existingShare->getShareOwner() !== $share->getShareOwner()) { if ($group->inGroup($user) && $existingShare->getShareOwner() !== $share->getShareOwner()) {
@ -406,6 +407,7 @@ class Manager implements IManager {
} }
} }
} }
}
/** /**
* Check for pre share requirements for group shares * Check for pre share requirements for group shares
@ -423,7 +425,7 @@ class Manager implements IManager {
if ($this->shareWithGroupMembersOnly()) { if ($this->shareWithGroupMembersOnly()) {
$sharedBy = $this->userManager->get($share->getSharedBy()); $sharedBy = $this->userManager->get($share->getSharedBy());
$sharedWith = $this->groupManager->get($share->getSharedWith()); $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'); throw new \Exception('Only sharing within your own groups is allowed');
} }
} }
@ -891,6 +893,9 @@ class Manager implements IManager {
if ($share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) { if ($share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) {
$sharedWith = $this->groupManager->get($share->getSharedWith()); $sharedWith = $this->groupManager->get($share->getSharedWith());
if (is_null($sharedWith)) {
throw new \InvalidArgumentException('Group "' . $share->getSharedWith() . '" does not exist');
}
$recipient = $this->userManager->get($recipientId); $recipient = $this->userManager->get($recipientId);
if (!$sharedWith->inGroup($recipient)) { if (!$sharedWith->inGroup($recipient)) {
throw new \InvalidArgumentException('Invalid recipient'); throw new \InvalidArgumentException('Invalid recipient');

View File

@ -22,18 +22,24 @@
*/ */
namespace Test\Group; namespace Test\Group;
use OC\Group\Database;
use OC\User\Manager; use OC\User\Manager;
use OCP\ILogger;
use OCP\IUser; use OCP\IUser;
use OCP\GroupInterface; use OCP\GroupInterface;
use Test\TestCase;
class ManagerTest extends \Test\TestCase { class ManagerTest extends TestCase {
/** @var Manager|\PHPUnit_Framework_MockObject_MockObject $userManager */ /** @var Manager|\PHPUnit_Framework_MockObject_MockObject $userManager */
protected $userManager; protected $userManager;
/** @var ILogger|\PHPUnit_Framework_MockObject_MockObject $userManager */
protected $logger;
protected function setUp() { protected function setUp() {
parent::setUp(); parent::setUp();
$this->userManager = $this->createMock(Manager::class); $this->userManager = $this->createMock(Manager::class);
$this->logger = $this->createMock(ILogger::class);
} }
private function getTestUser($userId) { private function getTestUser($userId) {
@ -47,8 +53,12 @@ class ManagerTest extends \Test\TestCase {
return $mockUser; return $mockUser;
} }
/**
* @param null|int $implementedActions
* @return \PHPUnit_Framework_MockObject_MockObject
*/
private function getTestBackend($implementedActions = null) { private function getTestBackend($implementedActions = null) {
if (is_null($implementedActions)) { if ($implementedActions === null) {
$implementedActions = $implementedActions =
GroupInterface::ADD_TO_GROUP | GroupInterface::ADD_TO_GROUP |
GroupInterface::REMOVE_FROM_GOUP | GroupInterface::REMOVE_FROM_GOUP |
@ -58,7 +68,7 @@ class ManagerTest extends \Test\TestCase {
} }
// need to declare it this way due to optional methods // need to declare it this way due to optional methods
// thanks to the implementsActions logic // thanks to the implementsActions logic
$backend = $this->getMockBuilder(\OCP\GroupInterface::class) $backend = $this->getMockBuilder(GroupInterface::class)
->disableOriginalConstructor() ->disableOriginalConstructor()
->setMethods([ ->setMethods([
'getGroupDetails', 'getGroupDetails',
@ -91,7 +101,7 @@ class ManagerTest extends \Test\TestCase {
->with('group1') ->with('group1')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$group = $manager->get('group1'); $group = $manager->get('group1');
@ -100,7 +110,7 @@ class ManagerTest extends \Test\TestCase {
} }
public function testGetNoBackend() { public function testGetNoBackend() {
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$this->assertNull($manager->get('group1')); $this->assertNull($manager->get('group1'));
} }
@ -115,7 +125,7 @@ class ManagerTest extends \Test\TestCase {
->with('group1') ->with('group1')
->will($this->returnValue(false)); ->will($this->returnValue(false));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$this->assertNull($manager->get('group1')); $this->assertNull($manager->get('group1'));
@ -125,7 +135,7 @@ class ManagerTest extends \Test\TestCase {
$backend = new \Test\Util\Group\Dummy(); $backend = new \Test\Util\Group\Dummy();
$backend->createGroup('group1'); $backend->createGroup('group1');
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$group = $manager->get('group1'); $group = $manager->get('group1');
@ -152,7 +162,7 @@ class ManagerTest extends \Test\TestCase {
->with('group1') ->with('group1')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend1); $manager->addBackend($backend1);
$manager->addBackend($backend2); $manager->addBackend($backend2);
@ -162,9 +172,7 @@ class ManagerTest extends \Test\TestCase {
} }
public function testCreate() { public function testCreate() {
/** /**@var \PHPUnit_Framework_MockObject_MockObject|\OC\Group\Backend $backend */
* @var \PHPUnit_Framework_MockObject_MockObject | \OC\Group\Backend $backend
*/
$backendGroupCreated = false; $backendGroupCreated = false;
$backend = $this->getTestBackend(); $backend = $this->getTestBackend();
$backend->expects($this->any()) $backend->expects($this->any())
@ -177,9 +185,9 @@ class ManagerTest extends \Test\TestCase {
->method('createGroup') ->method('createGroup')
->will($this->returnCallback(function () use (&$backendGroupCreated) { ->will($this->returnCallback(function () use (&$backendGroupCreated) {
$backendGroupCreated = true; $backendGroupCreated = true;
}));; }));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$group = $manager->createGroup('group1'); $group = $manager->createGroup('group1');
@ -187,9 +195,7 @@ class ManagerTest extends \Test\TestCase {
} }
public function testCreateExists() { public function testCreateExists() {
/** /** @var \PHPUnit_Framework_MockObject_MockObject|\OC\Group\Backend $backend */
* @var \PHPUnit_Framework_MockObject_MockObject | \OC\Group\Backend $backend
*/
$backend = $this->getTestBackend(); $backend = $this->getTestBackend();
$backend->expects($this->any()) $backend->expects($this->any())
->method('groupExists') ->method('groupExists')
@ -198,7 +204,7 @@ class ManagerTest extends \Test\TestCase {
$backend->expects($this->never()) $backend->expects($this->never())
->method('createGroup'); ->method('createGroup');
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$group = $manager->createGroup('group1'); $group = $manager->createGroup('group1');
@ -219,11 +225,11 @@ class ManagerTest extends \Test\TestCase {
->with('group1') ->with('group1')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$groups = $manager->search('1'); $groups = $manager->search('1');
$this->assertEquals(1, count($groups)); $this->assertCount(1, $groups);
$group1 = reset($groups); $group1 = reset($groups);
$this->assertEquals('group1', $group1->getGID()); $this->assertEquals('group1', $group1->getGID());
} }
@ -253,12 +259,12 @@ class ManagerTest extends \Test\TestCase {
->method('groupExists') ->method('groupExists')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend1); $manager->addBackend($backend1);
$manager->addBackend($backend2); $manager->addBackend($backend2);
$groups = $manager->search('1'); $groups = $manager->search('1');
$this->assertEquals(2, count($groups)); $this->assertCount(2, $groups);
$group1 = reset($groups); $group1 = reset($groups);
$group12 = next($groups); $group12 = next($groups);
$this->assertEquals('group1', $group1->getGID()); $this->assertEquals('group1', $group1->getGID());
@ -290,18 +296,40 @@ class ManagerTest extends \Test\TestCase {
->method('groupExists') ->method('groupExists')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend1); $manager->addBackend($backend1);
$manager->addBackend($backend2); $manager->addBackend($backend2);
$groups = $manager->search('1', 2, 1); $groups = $manager->search('1', 2, 1);
$this->assertEquals(2, count($groups)); $this->assertCount(2, $groups);
$group1 = reset($groups); $group1 = reset($groups);
$group12 = next($groups); $group12 = next($groups);
$this->assertEquals('group1', $group1->getGID()); $this->assertEquals('group1', $group1->getGID());
$this->assertEquals('group12', $group12->getGID()); $this->assertEquals('group12', $group12->getGID());
} }
public function testSearchResultExistsButGroupDoesNot() {
/** @var \PHPUnit_Framework_MockObject_MockObject|\OC\Group\Backend $backend */
$backend = $this->createMock(Database::class);
$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(Manager::class);
$manager = new \OC\Group\Manager($userManager, $this->logger);
$manager->addBackend($backend);
$groups = $manager->search('1');
$this->assertEmpty($groups);
}
public function testGetUserGroups() { public function testGetUserGroups() {
/** /**
* @var \PHPUnit_Framework_MockObject_MockObject | \OC\Group\Backend $backend * @var \PHPUnit_Framework_MockObject_MockObject | \OC\Group\Backend $backend
@ -316,18 +344,18 @@ class ManagerTest extends \Test\TestCase {
->with('group1') ->with('group1')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$groups = $manager->getUserGroups($this->getTestUser('user1')); $groups = $manager->getUserGroups($this->getTestUser('user1'));
$this->assertEquals(1, count($groups)); $this->assertCount(1, $groups);
$group1 = reset($groups); $group1 = reset($groups);
$this->assertEquals('group1', $group1->getGID()); $this->assertEquals('group1', $group1->getGID());
} }
public function testGetUserGroupIds() { public function testGetUserGroupIds() {
/** @var \PHPUnit_Framework_MockObject_MockObject|\OC\Group\Manager $manager */ /** @var \PHPUnit_Framework_MockObject_MockObject|\OC\Group\Manager $manager */
$manager = $this->getMockBuilder('OC\Group\Manager') $manager = $this->getMockBuilder(\OC\Group\Manager::class)
->disableOriginalConstructor() ->disableOriginalConstructor()
->setMethods(['getUserGroups']) ->setMethods(['getUserGroups'])
->getMock(); ->getMock();
@ -338,19 +366,44 @@ class ManagerTest extends \Test\TestCase {
'abc' => 'abc', 'abc' => 'abc',
]); ]);
/** @var \OC\User\User $user */ /** @var \OC\User\User|\PHPUnit_Framework_MockObject_MockObject $user */
$user = $this->getMockBuilder('OC\User\User') $user = $this->createMock(IUser::class);
->disableOriginalConstructor()
->getMock();
$groups = $manager->getUserGroupIds($user); $groups = $manager->getUserGroupIds($user);
$this->assertEquals(2, count($groups)); $this->assertCount(2, $groups);
foreach ($groups as $group) { foreach ($groups as $group) {
$this->assertInternalType('string', $group); $this->assertInternalType('string', $group);
} }
} }
public function testGetUserGroupsWithDeletedGroup() {
/**
* @var \PHPUnit_Framework_MockObject_MockObject | \OC\Group\Backend $backend
*/
$backend = $this->createMock(Database::class);
$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));
$manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend);
/** @var \OC\User\User|\PHPUnit_Framework_MockObject_MockObject $user */
$user = $this->createMock(IUser::class);
$user->expects($this->atLeastOnce())
->method('getUID')
->willReturn('user1');
$groups = $manager->getUserGroups($user);
$this->assertEmpty($groups);
}
public function testInGroup() { public function testInGroup() {
/** /**
* @var \PHPUnit_Framework_MockObject_MockObject | \OC\Group\Backend $backend * @var \PHPUnit_Framework_MockObject_MockObject | \OC\Group\Backend $backend
@ -364,7 +417,7 @@ class ManagerTest extends \Test\TestCase {
->method('groupExists') ->method('groupExists')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$this->assertTrue($manager->isInGroup('user1', 'group1')); $this->assertTrue($manager->isInGroup('user1', 'group1'));
@ -383,7 +436,7 @@ class ManagerTest extends \Test\TestCase {
->method('groupExists') ->method('groupExists')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$this->assertTrue($manager->isAdmin('user1')); $this->assertTrue($manager->isAdmin('user1'));
@ -402,7 +455,7 @@ class ManagerTest extends \Test\TestCase {
->method('groupExists') ->method('groupExists')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$this->assertFalse($manager->isAdmin('user1')); $this->assertFalse($manager->isAdmin('user1'));
@ -433,12 +486,12 @@ class ManagerTest extends \Test\TestCase {
->method('groupExists') ->method('groupExists')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend1); $manager->addBackend($backend1);
$manager->addBackend($backend2); $manager->addBackend($backend2);
$groups = $manager->getUserGroups($this->getTestUser('user1')); $groups = $manager->getUserGroups($this->getTestUser('user1'));
$this->assertEquals(2, count($groups)); $this->assertCount(2, $groups);
$group1 = reset($groups); $group1 = reset($groups);
$group2 = next($groups); $group2 = next($groups);
$this->assertEquals('group1', $group1->getGID()); $this->assertEquals('group1', $group1->getGID());
@ -468,14 +521,10 @@ class ManagerTest extends \Test\TestCase {
} }
})); }));
$userBackend = $this->getMockBuilder('\OC\User\Backend')
->disableOriginalConstructor()
->getMock();
$this->userManager->expects($this->any()) $this->userManager->expects($this->any())
->method('searchDisplayName') ->method('searchDisplayName')
->with('user3') ->with('user3')
->will($this->returnCallback(function($search, $limit, $offset) use ($userBackend) { ->will($this->returnCallback(function($search, $limit, $offset) {
switch($offset) { switch($offset) {
case 0 : return ['user3' => $this->getTestUser('user3'), case 0 : return ['user3' => $this->getTestUser('user3'),
'user33' => $this->getTestUser('user33')]; 'user33' => $this->getTestUser('user33')];
@ -485,7 +534,7 @@ class ManagerTest extends \Test\TestCase {
})); }));
$this->userManager->expects($this->any()) $this->userManager->expects($this->any())
->method('get') ->method('get')
->will($this->returnCallback(function($uid) use ($userBackend) { ->will($this->returnCallback(function($uid) {
switch($uid) { switch($uid) {
case 'user1' : return $this->getTestUser('user1'); case 'user1' : return $this->getTestUser('user1');
case 'user2' : return $this->getTestUser('user2'); case 'user2' : return $this->getTestUser('user2');
@ -496,11 +545,11 @@ class ManagerTest extends \Test\TestCase {
} }
})); }));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$users = $manager->displayNamesInGroup('testgroup', 'user3'); $users = $manager->displayNamesInGroup('testgroup', 'user3');
$this->assertEquals(1, count($users)); $this->assertCount(1, $users);
$this->assertFalse(isset($users['user1'])); $this->assertFalse(isset($users['user1']));
$this->assertFalse(isset($users['user2'])); $this->assertFalse(isset($users['user2']));
$this->assertFalse(isset($users['user3'])); $this->assertFalse(isset($users['user3']));
@ -531,14 +580,10 @@ class ManagerTest extends \Test\TestCase {
} }
})); }));
$userBackend = $this->getMockBuilder('\OC\User\Backend')
->disableOriginalConstructor()
->getMock();
$this->userManager->expects($this->any()) $this->userManager->expects($this->any())
->method('searchDisplayName') ->method('searchDisplayName')
->with('user3') ->with('user3')
->will($this->returnCallback(function($search, $limit, $offset) use ($userBackend) { ->will($this->returnCallback(function($search, $limit, $offset) {
switch($offset) { switch($offset) {
case 0 : return ['user3' => $this->getTestUser('user3'), case 0 : return ['user3' => $this->getTestUser('user3'),
'user33' => $this->getTestUser('user33')]; 'user33' => $this->getTestUser('user33')];
@ -548,7 +593,7 @@ class ManagerTest extends \Test\TestCase {
})); }));
$this->userManager->expects($this->any()) $this->userManager->expects($this->any())
->method('get') ->method('get')
->will($this->returnCallback(function($uid) use ($userBackend) { ->will($this->returnCallback(function($uid) {
switch($uid) { switch($uid) {
case 'user1' : return $this->getTestUser('user1'); case 'user1' : return $this->getTestUser('user1');
case 'user2' : return $this->getTestUser('user2'); case 'user2' : return $this->getTestUser('user2');
@ -560,11 +605,11 @@ class ManagerTest extends \Test\TestCase {
} }
})); }));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$users = $manager->displayNamesInGroup('testgroup', 'user3', 1); $users = $manager->displayNamesInGroup('testgroup', 'user3', 1);
$this->assertEquals(1, count($users)); $this->assertCount(1, $users);
$this->assertFalse(isset($users['user1'])); $this->assertFalse(isset($users['user1']));
$this->assertFalse(isset($users['user2'])); $this->assertFalse(isset($users['user2']));
$this->assertFalse(isset($users['user3'])); $this->assertFalse(isset($users['user3']));
@ -596,14 +641,10 @@ class ManagerTest extends \Test\TestCase {
} }
})); }));
$userBackend = $this->getMockBuilder('\OC\User\Backend')
->disableOriginalConstructor()
->getMock();
$this->userManager->expects($this->any()) $this->userManager->expects($this->any())
->method('searchDisplayName') ->method('searchDisplayName')
->with('user3') ->with('user3')
->will($this->returnCallback(function($search, $limit, $offset) use ($userBackend) { ->will($this->returnCallback(function($search, $limit, $offset) {
switch($offset) { switch($offset) {
case 0 : case 0 :
return [ return [
@ -616,7 +657,7 @@ class ManagerTest extends \Test\TestCase {
})); }));
$this->userManager->expects($this->any()) $this->userManager->expects($this->any())
->method('get') ->method('get')
->will($this->returnCallback(function($uid) use ($userBackend) { ->will($this->returnCallback(function($uid) {
switch($uid) { switch($uid) {
case 'user1' : return $this->getTestUser('user1'); case 'user1' : return $this->getTestUser('user1');
case 'user2' : return $this->getTestUser('user2'); case 'user2' : return $this->getTestUser('user2');
@ -628,11 +669,11 @@ class ManagerTest extends \Test\TestCase {
} }
})); }));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$users = $manager->displayNamesInGroup('testgroup', 'user3', 1, 1); $users = $manager->displayNamesInGroup('testgroup', 'user3', 1, 1);
$this->assertEquals(1, count($users)); $this->assertCount(1, $users);
$this->assertFalse(isset($users['user1'])); $this->assertFalse(isset($users['user1']));
$this->assertFalse(isset($users['user2'])); $this->assertFalse(isset($users['user2']));
$this->assertFalse(isset($users['user3'])); $this->assertFalse(isset($users['user3']));
@ -655,13 +696,9 @@ class ManagerTest extends \Test\TestCase {
->with('testgroup', '', -1, 0) ->with('testgroup', '', -1, 0)
->will($this->returnValue(array('user2', 'user33'))); ->will($this->returnValue(array('user2', 'user33')));
$userBackend = $this->getMockBuilder('\OC\User\Backend')
->disableOriginalConstructor()
->getMock();
$this->userManager->expects($this->any()) $this->userManager->expects($this->any())
->method('get') ->method('get')
->will($this->returnCallback(function($uid) use ($userBackend) { ->will($this->returnCallback(function($uid) {
switch($uid) { switch($uid) {
case 'user1' : return $this->getTestUser('user1'); case 'user1' : return $this->getTestUser('user1');
case 'user2' : return $this->getTestUser('user2'); case 'user2' : return $this->getTestUser('user2');
@ -672,11 +709,11 @@ class ManagerTest extends \Test\TestCase {
} }
})); }));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$users = $manager->displayNamesInGroup('testgroup', ''); $users = $manager->displayNamesInGroup('testgroup', '');
$this->assertEquals(2, count($users)); $this->assertCount(2, $users);
$this->assertFalse(isset($users['user1'])); $this->assertFalse(isset($users['user1']));
$this->assertTrue(isset($users['user2'])); $this->assertTrue(isset($users['user2']));
$this->assertFalse(isset($users['user3'])); $this->assertFalse(isset($users['user3']));
@ -698,13 +735,9 @@ class ManagerTest extends \Test\TestCase {
->with('testgroup', '', 1, 0) ->with('testgroup', '', 1, 0)
->will($this->returnValue(array('user2'))); ->will($this->returnValue(array('user2')));
$userBackend = $this->getMockBuilder('\OC\User\Backend')
->disableOriginalConstructor()
->getMock();
$this->userManager->expects($this->any()) $this->userManager->expects($this->any())
->method('get') ->method('get')
->will($this->returnCallback(function($uid) use ($userBackend) { ->will($this->returnCallback(function($uid) {
switch($uid) { switch($uid) {
case 'user1' : return $this->getTestUser('user1'); case 'user1' : return $this->getTestUser('user1');
case 'user2' : return $this->getTestUser('user2'); case 'user2' : return $this->getTestUser('user2');
@ -715,11 +748,11 @@ class ManagerTest extends \Test\TestCase {
} }
})); }));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$users = $manager->displayNamesInGroup('testgroup', '', 1); $users = $manager->displayNamesInGroup('testgroup', '', 1);
$this->assertEquals(1, count($users)); $this->assertCount(1, $users);
$this->assertFalse(isset($users['user1'])); $this->assertFalse(isset($users['user1']));
$this->assertTrue(isset($users['user2'])); $this->assertTrue(isset($users['user2']));
$this->assertFalse(isset($users['user3'])); $this->assertFalse(isset($users['user3']));
@ -741,13 +774,9 @@ class ManagerTest extends \Test\TestCase {
->with('testgroup', '', 1, 1) ->with('testgroup', '', 1, 1)
->will($this->returnValue(array('user33'))); ->will($this->returnValue(array('user33')));
$userBackend = $this->getMockBuilder('\OC\User\Backend')
->disableOriginalConstructor()
->getMock();
$this->userManager->expects($this->any()) $this->userManager->expects($this->any())
->method('get') ->method('get')
->will($this->returnCallback(function($uid) use ($userBackend) { ->will($this->returnCallback(function($uid) {
switch($uid) { switch($uid) {
case 'user1' : return $this->getTestUser('user1'); case 'user1' : return $this->getTestUser('user1');
case 'user2' : return $this->getTestUser('user2'); case 'user2' : return $this->getTestUser('user2');
@ -758,11 +787,11 @@ class ManagerTest extends \Test\TestCase {
} }
})); }));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$users = $manager->displayNamesInGroup('testgroup', '', 1, 1); $users = $manager->displayNamesInGroup('testgroup', '', 1, 1);
$this->assertEquals(1, count($users)); $this->assertCount(1, $users);
$this->assertFalse(isset($users['user1'])); $this->assertFalse(isset($users['user1']));
$this->assertFalse(isset($users['user2'])); $this->assertFalse(isset($users['user2']));
$this->assertFalse(isset($users['user3'])); $this->assertFalse(isset($users['user3']));
@ -786,7 +815,7 @@ class ManagerTest extends \Test\TestCase {
->with('group1') ->with('group1')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
// prime cache // prime cache
@ -797,11 +826,11 @@ class ManagerTest extends \Test\TestCase {
// add user // add user
$group = $manager->get('group1'); $group = $manager->get('group1');
$group->addUser($user1); $group->addUser($user1);
$expectedGroups = ['group1']; $expectedGroups[] = 'group1';
// check result // check result
$groups = $manager->getUserGroups($user1); $groups = $manager->getUserGroups($user1);
$this->assertEquals(1, count($groups)); $this->assertCount(1, $groups);
$group1 = reset($groups); $group1 = reset($groups);
$this->assertEquals('group1', $group1->getGID()); $this->assertEquals('group1', $group1->getGID());
} }
@ -829,13 +858,13 @@ class ManagerTest extends \Test\TestCase {
->method('removeFromGroup') ->method('removeFromGroup')
->will($this->returnValue(true)); ->will($this->returnValue(true));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
// prime cache // prime cache
$user1 = $this->getTestUser('user1'); $user1 = $this->getTestUser('user1');
$groups = $manager->getUserGroups($user1); $groups = $manager->getUserGroups($user1);
$this->assertEquals(1, count($groups)); $this->assertCount(1, $groups);
$group1 = reset($groups); $group1 = reset($groups);
$this->assertEquals('group1', $group1->getGID()); $this->assertEquals('group1', $group1->getGID());
@ -859,7 +888,7 @@ class ManagerTest extends \Test\TestCase {
->with('user1') ->with('user1')
->will($this->returnValue(null)); ->will($this->returnValue(null));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
$groups = $manager->getUserIdGroups('user1'); $groups = $manager->getUserIdGroups('user1');
@ -885,7 +914,7 @@ class ManagerTest extends \Test\TestCase {
['group2', ['gid' => 'group2']], ['group2', ['gid' => 'group2']],
])); ]));
$manager = new \OC\Group\Manager($this->userManager); $manager = new \OC\Group\Manager($this->userManager, $this->logger);
$manager->addBackend($backend); $manager->addBackend($backend);
// group with display name // group with display name

View File

@ -1524,6 +1524,48 @@ class DefaultShareProviderTest extends \Test\TestCase {
$this->provider->deleteFromSelf($share, 'user2'); $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() { public function testDeleteFromSelfUser() {
$qb = $this->dbConn->getQueryBuilder(); $qb = $this->dbConn->getQueryBuilder();
$stmt = $qb->insert('share') $stmt = $qb->insert('share')

View File

@ -1139,6 +1139,39 @@ class ManagerTest extends \Test\TestCase {
self::invokePrivate($this->manager, 'userCreateChecks', [$share]); 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() { public function testUserCreateChecksIdenticalPathNotSharedWithUser() {
$share = $this->manager->newShare(); $share = $this->manager->newShare();
$sharedWith = $this->createMock(IUser::class); $sharedWith = $this->createMock(IUser::class);
@ -1216,6 +1249,29 @@ class ManagerTest extends \Test\TestCase {
self::invokePrivate($this->manager, 'groupCreateChecks', [$share]); 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() { public function testGroupCreateChecksShareWithGroupMembersOnlyInGroup() {
$share = $this->manager->newShare(); $share = $this->manager->newShare();
@ -2666,6 +2722,23 @@ class ManagerTest extends \Test\TestCase {
$this->manager->moveShare($share, 'recipient'); $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() { public function testMoveShareGroup() {
$share = $this->manager->newShare(); $share = $this->manager->newShare();
$share->setShareType(\OCP\Share::SHARE_TYPE_GROUP) $share->setShareType(\OCP\Share::SHARE_TYPE_GROUP)