Don't check group shares if shareWithGroupMembersOnly is false

Signed-off-by: Thomas Citharel <tcit@tcit.fr>

Remove the system user check

Signed-off-by: Thomas Citharel <tcit@tcit.fr>

apps/dav/lib/Command/MoveCalendar.php

@@ -28,7 +28,7 @@ use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\IUserManager;
 use OCP\IUserSession;
-use OCP\Share\IManager;
+use OCP\Share\IManager as IShareManager;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
@@ -41,9 +41,12 @@ class MoveCalendar extends Command {
 	/** @var IUserManager */
 	private $userManager;
 
-	/** @var IGroupManager $groupManager */
+	/** @var IGroupManager */
 	private $groupManager;
 
+	/** @var IShareManager */
+	private $shareManager;
+
 	/** @var IConfig $config */
 	private $config;
 
@@ -61,6 +64,7 @@ class MoveCalendar extends Command {
 	/**
 	 * @param IUserManager $userManager
 	 * @param IGroupManager $groupManager
+	 * @param IShareManager $shareManager
 	 * @param IConfig $config
 	 * @param IL10N $l10n
 	 * @param CalDavBackend $calDav
@@ -68,6 +72,7 @@ class MoveCalendar extends Command {
 	function __construct(
 		IUserManager $userManager,
 		IGroupManager $groupManager,
+		IShareManager $shareManager,
 		IConfig $config,
 		IL10N $l10n,
 		CalDavBackend $calDav
@@ -75,6 +80,7 @@ class MoveCalendar extends Command {
 		parent::__construct();
 		$this->userManager = $userManager;
 		$this->groupManager = $groupManager;
+		$this->shareManager = $shareManager;
 		$this->config = $config;
 		$this->l10n = $l10n;
 		$this->calDav = $calDav;
@@ -102,15 +108,10 @@ class MoveCalendar extends Command {
 
 		$this->io = new SymfonyStyle($input, $output);
 
-		if (in_array('system', [$userOrigin, $userDestination], true)) {
-			throw new \InvalidArgumentException("User can't be system");
-		}
-
 		if (!$this->userManager->userExists($userOrigin)) {
 			throw new \InvalidArgumentException("User <$userOrigin> is unknown.");
 		}
 
-
 		if (!$this->userManager->userExists($userDestination)) {
 			throw new \InvalidArgumentException("User <$userDestination> is unknown.");
 		}
@@ -127,7 +128,9 @@ class MoveCalendar extends Command {
 			throw new \InvalidArgumentException("User <$userDestination> already has a calendar named <$name>.");
 		}
 
-		$this->checkShares($calendar, $userDestination, $input->getOption('force'));
+		if ($this->shareManager->shareWithGroupMembersOnly() === true) {
+			$this->checkShares($calendar, $userDestination, $input->getOption('force'));
+		}
 
 		$this->calDav->moveCalendar($name, self::URI_USERS . $userOrigin, self::URI_USERS . $userDestination);
 

apps/dav/tests/unit/Command/MoveCalendarTest.php

@@ -27,6 +27,7 @@ use OCP\IConfig;
 use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\IUserManager;
+use OCP\Share\IManager;
 use Symfony\Component\Console\Tester\CommandTester;
 use Test\TestCase;
 
@@ -44,6 +45,9 @@ class MoveCalendarTest extends TestCase {
 	/** @var \OCP\IGroupManager|\PHPUnit_Framework_MockObject_MockObject $groupManager */
 	private $groupManager;
 
+	/** @var \OCP\Share\IManager|\PHPUnit_Framework_MockObject_MockObject $shareManager */
+	private $shareManager;
+
 	/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject $l10n */
 	private $config;
 
@@ -61,6 +65,7 @@ class MoveCalendarTest extends TestCase {
 
 		$this->userManager = $this->createMock(IUserManager::class);
 		$this->groupManager = $this->createMock(IGroupManager::class);
+		$this->shareManager = $this->createMock(IManager::class);
 		$this->config = $this->createMock(IConfig::class);
 		$this->l10n = $this->createMock(IL10N::class);
 		$this->calDav = $this->createMock(CalDavBackend::class);
@@ -68,6 +73,7 @@ class MoveCalendarTest extends TestCase {
 		$this->command = new MoveCalendar(
 			$this->userManager,
 			$this->groupManager,
+			$this->shareManager,
 			$this->config,
 			$this->l10n,
 			$this->calDav
@@ -110,20 +116,6 @@ class MoveCalendarTest extends TestCase {
 		]);
 	}
 
-	/**
-	 * @expectedException InvalidArgumentException
-	 * @expectedExceptionMessage User can't be system
-	 */
-	public function testTryToMoveToOrFromSystem()
-	{
-		$commandTester = new CommandTester($this->command);
-		$commandTester->execute([
-			'name' => $this->command->getName(),
-			'userorigin' => 'system',
-			'userdestination' => 'user2',
-		]);
-	}
-
 	/**
 	 * @expectedException InvalidArgumentException
 	 * @expectedExceptionMessage User <user> has no calendar named <personal>. You can run occ dav:list-calendars to list calendars URIs for this user.
@@ -214,6 +206,9 @@ class MoveCalendarTest extends TestCase {
 			->with(1234)
 			->willReturn([]);
 
+		$this->shareManager->expects($this->once())->method('shareWithGroupMembersOnly')
+			->willReturn(true);
+
 		$commandTester = new CommandTester($this->command);
 		$commandTester->execute([
 			'name' => 'personal',
@@ -224,11 +219,18 @@ class MoveCalendarTest extends TestCase {
 		$this->assertContains("[OK] Calendar <personal> was moved from user <user> to <user2>", $commandTester->getDisplay());
 	}
 
+	public function dataTestMoveWithDestinationNotPartOfGroup(): array
+	{
+		return [
+			[true],
+			[false]
+		];
+	}
+
 	/**
-	 * @expectedException InvalidArgumentException
-	 * @expectedExceptionMessage User <user2> is not part of the group <nextclouders> with whom the calendar <personal> was shared. You may use -f to move the calendar while deleting this share.
+	 * @dataProvider dataTestMoveWithDestinationNotPartOfGroup
 	 */
-	public function testMoveWithDestinationNotPartOfGroup()
+	public function testMoveWithDestinationNotPartOfGroup(bool $shareWithGroupMembersOnly)
 	{
 		$this->userManager->expects($this->at(0))
 			->method('userExists')
@@ -251,12 +253,20 @@ class MoveCalendarTest extends TestCase {
 			->with('principals/users/user2', 'personal')
 			->willReturn(null);
 
-		$this->calDav->expects($this->once())->method('getShares')
-			->with(1234)
-			->willReturn([
-				['href' => 'principal:principals/groups/nextclouders']
+		$this->shareManager->expects($this->once())->method('shareWithGroupMembersOnly')
+			->willReturn($shareWithGroupMembersOnly);
+
+		if ($shareWithGroupMembersOnly === true) {
+			$this->calDav->expects($this->once())->method('getShares')
+				->with(1234)
+				->willReturn([
+					['href' => 'principal:principals/groups/nextclouders']
 			]);
 
+			$this->expectException(InvalidArgumentException::class);
+			$this->expectExceptionMessage("User <user2> is not part of the group <nextclouders> with whom the calendar <personal> was shared. You may use -f to move the calendar while deleting this share.");
+		}
+
 		$commandTester = new CommandTester($this->command);
 		$commandTester->execute([
 			'name' => 'personal',
@@ -288,6 +298,9 @@ class MoveCalendarTest extends TestCase {
 			->with('principals/users/user2', 'personal')
 			->willReturn(null);
 
+		$this->shareManager->expects($this->once())->method('shareWithGroupMembersOnly')
+			->willReturn(true);
+
 		$this->calDav->expects($this->once())->method('getShares')
 			->with(1234)
 			->willReturn([
@@ -332,6 +345,9 @@ class MoveCalendarTest extends TestCase {
 			->with('principals/users/user2', 'personal')
 			->willReturn(null);
 
+		$this->shareManager->expects($this->once())->method('shareWithGroupMembersOnly')
+			->willReturn(true);
+
 		$this->calDav->expects($this->once())->method('getShares')
 			->with(1234)
 			->willReturn([