Let new shares only use the current users permissions

Signed-off-by: Julius Härtl <jus@bitgrid.net>

CHANGELOG.md

@@ -1,6 +1,23 @@
 # Changelog
 All notable changes to this project will be documented in this file.
 
+## 0.7.0 - 2019-08-20
+
+## Added
+- Make deck compatible to Nextcloud 17
+- Allow to set the description when creating cards though the REST API
+
+## 0.6.6 - 2019-08-01
+
+### Fixed
+- Bump security related dependencies
+
+## 0.6.5 - 2019-07-28
+
+### Fixed
+- Fix attachment upload/delete failures
+- Bump dependencies
+
 ## 0.6.4 - 2019-06-30
 
 ### Fixed

lib/Command/UserExport.php

@@ -88,13 +88,8 @@ class UserExport extends Command {
 
 		$userId = $input->getArgument('user-id');
 
-		$groups = $this->groupManager->getUserGroupIds(
-			$this->userManager->get($userId)
-		);
-		$boards = $this->boardService->findAll([
-			'user' => $userId,
-			'groups' => $groups
-		]);
+		$this->boardService->setUserId($userId);
+		$boards = $this->boardService->findAll();
 
 		$data = [];
 		foreach ($boards as $board) {

lib/Cron/ScheduledNotifications.php

@@ -27,6 +27,8 @@ use OC\BackgroundJob\Job;
 use OCA\Deck\Db\Card;
 use OCA\Deck\Db\CardMapper;
 use OCA\Deck\Notification\NotificationHelper;
+use OCP\AppFramework\Db\DoesNotExistException;
+use OCP\ILogger;
 
 class ScheduledNotifications extends Job {
 
@@ -34,13 +36,17 @@ class ScheduledNotifications extends Job {
 	protected $cardMapper;
 	/** @var NotificationHelper */
 	protected $notificationHelper;
+	/** @var ILogger */
+	protected $logger;
 
 	public function __construct(
 		CardMapper $cardMapper,
-		NotificationHelper $notificationHelper
+		NotificationHelper $notificationHelper,
+		ILogger $logger
 	) {
 		$this->cardMapper = $cardMapper;
 		$this->notificationHelper = $notificationHelper;
+		$this->logger = $logger;
 	}
 
 	/**
@@ -52,8 +58,13 @@ class ScheduledNotifications extends Job {
 		$cards = $this->cardMapper->findOverdue();
 		/** @var Card $card */
 		foreach ($cards as $card) {
-			$this->notificationHelper->sendCardDuedate($card);
+			try {
+				$this->notificationHelper->sendCardDuedate($card);
+			} catch (DoesNotExistException $e) {
+				// Skip if any error occurs
+				$this->logger->debug('Could not create overdue notification for card with id ' . $card->getId());
+			}
 		}
 	}
 
-}
+}

lib/Notification/NotificationHelper.php

@@ -67,6 +67,10 @@ class NotificationHelper {
 		$this->currentUser = $userId;
 	}
 
+	/**
+	 * @param $card
+	 * @throws \OCP\AppFramework\Db\DoesNotExistException
+	 */
 	public function sendCardDuedate($card) {
 		// check if notification has already been sent
 		// ideally notifications should not be deleted once seen by the user so we can

lib/Service/BoardService.php

@@ -98,6 +98,15 @@ class BoardService {
 		$this->userId = $userId;
 	}
 
+	/**
+	 * Set a different user than the current one, e.g. when no user is available in occ
+	 *
+	 * @param string $userId
+	 */
+	public function setUserId(string $userId): void {
+		$this->userId = $userId;
+	}
+
 	/**
 	 * @return array
 	 */
@@ -445,6 +454,17 @@ class BoardService {
 		return $board;
 	}
 
+	private function applyPermissions($boardId, $edit, $share, $manage) {
+		try {
+			$this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_MANAGE);
+		} catch (NoPermissionException $e) {
+			$acls = $this->aclMapper->findAll($boardId);
+			$edit = $this->permissionService->userCan($acls, Acl::PERMISSION_EDIT, $this->userId) && $edit;
+			$share = $this->permissionService->userCan($acls, Acl::PERMISSION_SHARE, $this->userId) && $share;
+			$manage = $this->permissionService->userCan($acls, Acl::PERMISSION_MANAGE, $this->userId) && $manage;
+		}
+		return [$edit, $share, $manage];
+	}
 
 	/**
 	 * @param $boardId
@@ -484,6 +504,8 @@ class BoardService {
 		}
 
 		$this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_SHARE);
+		[$edit, $share, $manage] = $this->applyPermissions($boardId, $edit, $share, $manage);
+
 		$acl = new Acl();
 		$acl->setBoardId($boardId);
 		$acl->setType($type);
@@ -546,8 +568,10 @@ class BoardService {
 		}
 
 		$this->permissionService->checkPermission($this->aclMapper, $id, Acl::PERMISSION_SHARE);
+
 		/** @var Acl $acl */
 		$acl = $this->aclMapper->find($id);
+		[$edit, $share, $manage] = $this->applyPermissions($acl->getBoardId(), $edit, $share, $manage);
 		$acl->setPermissionEdit($edit);
 		$acl->setPermissionShare($share);
 		$acl->setPermissionManage($manage);

tests/unit/Command/UserExportTest.php

@@ -89,28 +89,12 @@ class UserExportTest extends \Test\TestCase {
 		$input->expects($this->once())->method('getArgument')->with('user-id')->willReturn('admin');
 		$output = $this->createMock(OutputInterface::class);
 
-		$user = $this->createMock(IUser::class);
-		$this->userManager->expects($this->once())
-			->method('get')
-			->with('admin')
-			->willReturn($user);
-
-		$groups = [];
-		$this->groupManager->expects($this->once())
-			->method('getUserGroupIds')
-			->with($user)
-			->willReturn($groups);
-
 		$boards = [
 			$this->getBoard(1),
 			$this->getBoard(2),
 		];
 		$this->boardService->expects($this->once())
 			->method('findAll')
-			->with([
-				'user' => 'admin',
-				'groups' => $groups
-			])
 			->willReturn($boards);
 		$this->boardMapper->expects($this->exactly(count($boards)))
 			->method('find')

tests/unit/Cron/ScheduledNoificationsTest.php

@@ -28,6 +28,7 @@ use OCA\Deck\Db\BoardMapper;
 use OCA\Deck\Db\Card;
 use OCA\Deck\Db\CardMapper;
 use OCA\Deck\Notification\NotificationHelper;
+use OCP\ILogger;
 
 class ScheduledNoificationsTest extends \Test\TestCase {
 
@@ -35,6 +36,8 @@ class ScheduledNoificationsTest extends \Test\TestCase {
 	protected $cardMapper;
 	/** @var NotificationHelper|\PHPUnit\Framework\MockObject\MockObject */
 	protected $notificationHelper;
+	/** @var ILogger|\PHPUnit\Framework\MockObject\MockObject */
+	protected $logger;
 	/** @var ScheduledNotifications */
 	protected $scheduledNotifications;
 
@@ -42,7 +45,8 @@ class ScheduledNoificationsTest extends \Test\TestCase {
 		parent::setUp();
 		$this->cardMapper = $this->createMock(CardMapper::class);
 		$this->notificationHelper = $this->createMock(NotificationHelper::class);
-		$this->scheduledNotifications = new ScheduledNotifications($this->cardMapper, $this->notificationHelper);
+		$this->logger = $this->createMock(ILogger::class);
+		$this->scheduledNotifications = new ScheduledNotifications($this->cardMapper, $this->notificationHelper, $this->logger);
 	}
 
 	public function testDeleteCron() {

tests/unit/Service/BoardServiceTest.php

@@ -34,6 +34,7 @@ use OCA\Deck\Db\BoardMapper;
 use OCA\Deck\Db\ChangeHelper;
 use OCA\Deck\Db\LabelMapper;
 use OCA\Deck\Db\StackMapper;
+use OCA\Deck\NoPermissionException;
 use OCA\Deck\Notification\NotificationHelper;
 use OCP\IUser;
 use OCP\IUserManager;
@@ -262,6 +263,93 @@ class BoardServiceTest extends TestCase {
 		));
 	}
 
+	public function dataAddAclExtendPermission() {
+		return [
+			[[false, false, false], [false, false, false], [false, false, false]],
+			[[false, false, false], [true, true, true], [false, false, false]],
+
+			// user has share permissions -> can only reshare with those
+			[[false, true, false], [false, false, false], [false, false, false]],
+			[[false, true, false], [false, true, false], [false, true, false]],
+			[[false, true, false], [true, true, true], [false, true, false]],
+
+			// user has write permissions -> can only reshare with those
+			[[true, true, false], [false, false, false], [false, false, false]],
+			[[true, true, false], [false, true, false], [false, true, false]],
+			[[true, true, false], [true, true, true], [true, true, false]],
+
+			// user has manage permissions -> can upgrade acl permissions
+			[[false, false, true], [true, true, true], [true, true, true]],
+			[[true, true, true], [false, false, true], [false, false, true]],
+		];
+	}
+
+	/**
+	 * @dataProvider dataAddAclExtendPermission
+	 * @param $currentUserAcl
+	 * @param $providedAcl
+	 * @param $resultingAcl
+	 * @throws NoPermissionException
+	 * @throws \OCA\Deck\BadRequestException
+	 */
+	public function testAddAclExtendPermission($currentUserAcl, $providedAcl, $resultingAcl) {
+		$existingAcl = new Acl();
+		$existingAcl->setBoardId(123);
+		$existingAcl->setType('user');
+		$existingAcl->setParticipant('admin');
+		$existingAcl->setPermissionEdit($currentUserAcl[0]);
+		$existingAcl->setPermissionShare($currentUserAcl[1]);
+		$existingAcl->setPermissionManage($currentUserAcl[2]);
+		$this->permissionService->expects($this->at(0))
+			->method('checkPermission')
+			->with($this->boardMapper, 123, Acl::PERMISSION_SHARE, null);
+		if ($currentUserAcl[2]) {
+			$this->permissionService->expects($this->at(1))
+				->method('checkPermission')
+				->with($this->boardMapper, 123, Acl::PERMISSION_MANAGE, null);
+		} else {
+			$this->aclMapper->expects($this->once())
+				->method('findAll')
+				->willReturn([$existingAcl]);
+			$this->permissionService->expects($this->at(1))
+				->method('checkPermission')
+				->with($this->boardMapper, 123, Acl::PERMISSION_MANAGE, null)
+				->willThrowException(new NoPermissionException('No permission'));
+			$this->permissionService->expects($this->at(2))
+				->method('userCan')
+				->willReturn($currentUserAcl[0]);
+			$this->permissionService->expects($this->at(3))
+				->method('userCan')
+				->willReturn($currentUserAcl[1]);
+			$this->permissionService->expects($this->at(4))
+				->method('userCan')
+				->willReturn($currentUserAcl[2]);
+		}
+
+		$user = $this->createMock(IUser::class);
+		$user->method('getUID')->willReturn('admin');
+		$acl = new Acl();
+		$acl->setBoardId(123);
+		$acl->setType('user');
+		$acl->setParticipant('admin');
+		$acl->setPermissionEdit($resultingAcl[0]);
+		$acl->setPermissionShare($resultingAcl[1]);
+		$acl->setPermissionManage($resultingAcl[2]);
+		$acl->resolveRelation('participant', function($participant) use (&$user) {
+			return null;
+		});
+		$this->notificationHelper->expects($this->once())
+			->method('sendBoardShared');
+		$expected = clone $acl;
+		$this->aclMapper->expects($this->once())
+			->method('insert')
+			->with($acl)
+			->willReturn($acl);
+		$this->assertEquals($expected, $this->service->addAcl(
+			123, 'user', 'admin', $providedAcl[0], $providedAcl[1], $providedAcl[2]
+		));
+	}
+
 	public function testUpdateAcl() {
 		$acl = new Acl();
 		$acl->setBoardId(123);