From 9720a4464c41155212bcff6f4d1c4aa525846d14 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Julius=20H=C3=A4rtl?= <jus@bitgrid.net>
Date: Thu, 4 Jan 2024 00:30:41 +0100
Subject: [PATCH 01/11] tests: Add integration tests for deleted boards/cards
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Julius Härtl <jus@bitgrid.net>
---
 tests/integration/features/acl.feature        | 50 ++++++++++
 .../features/bootstrap/AttachmentContext.php  | 10 ++
 .../features/bootstrap/BoardContext.php       | 35 ++++++-
 .../features/bootstrap/CommentContext.php     | 31 ++++++
 .../features/bootstrap/ServerContext.php      |  8 +-
 tests/integration/features/decks.feature      | 98 +++++++++++++++++++
 6 files changed, 226 insertions(+), 6 deletions(-)

diff --git a/tests/integration/features/acl.feature b/tests/integration/features/acl.feature
index 2937a377f..1a9677fc1 100644
--- a/tests/integration/features/acl.feature
+++ b/tests/integration/features/acl.feature
@@ -90,3 +90,53 @@ Feature: acl
 		And the current user should not have "edit" permissions on the board
 		And the current user should have "share" permissions on the board
 		And the current user should not have "manage" permissions on the board
+
+	Scenario: Share a board multiple times
+		Given Logging in using web as "user0"
+		And creates a board named "Double shared board" with color "ff0000"
+		And shares the board with user "user1"
+		And shares the board with group "group1"
+		And creates a board named "Single shared board" with color "00ff00"
+		And shares the board with user "user1"
+		When Logging in using web as "user1"
+		And fetching the board list
+		Then the response should have a status code "200"
+		And the response should be a list of objects
+		And the response should contain an element with the properties
+			| property | value |
+			| title | Double shared board |
+
+
+	Scenario: Deleted board is inaccessible to share recipients
+		Given acting as user "user0"
+		When creates a board with example content
+		And remember the last card as "user0-card"
+		When post a comment with content "hello comment" on the card
+		And uploads an attachment to the last used card
+		And remember the last attachment as "user0-attachment"
+		And shares the board with user "user1"
+		Then the HTTP status code should be "200"
+		And delete the board
+
+		Given acting as user "user1"
+		When fetching the attachments for the card "user0-card"
+		Then the response should have a status code 403
+
+		When get the comments on the card
+		Then the response should have a status code 403
+
+		When update a comment with content "hello deleted" on the card
+		Then the response should have a status code 403
+
+		When delete the comment on the card
+		Then the response should have a status code 403
+		# 644
+		When post a comment with content "hello deleted" on the card
+		Then the response should have a status code 403
+
+		When get the card details
+		Then the response should have a status code 403
+		When fetching the attachment "user0-attachment" for the card "user0-card"
+		Then the response should have a status code 403
+		When deleting the attachment "user0-attachment" for the card "user0-card"
+		Then the response should have a status code 403
diff --git a/tests/integration/features/bootstrap/AttachmentContext.php b/tests/integration/features/bootstrap/AttachmentContext.php
index 1831bfaa9..b9a311957 100644
--- a/tests/integration/features/bootstrap/AttachmentContext.php
+++ b/tests/integration/features/bootstrap/AttachmentContext.php
@@ -87,4 +87,14 @@ public function fetchingTheAttachmentForTheCard($attachmentReference, $cardRefer
 
 		$this->requestContext->sendPlainRequest('GET', '/index.php/apps/deck/cards/' . $cardId . '/attachment/file:' . $attachmentId);
 	}
+
+	/**
+	 * @When fetching the attachments for the card :cardReference
+	 */
+	public function fetchingTheAttachmentsForTheCard($cardReference) {
+		$cardId = $this->boardContext->getRememberedCard($cardReference)['id'] ?? null;
+		Assert::assertNotNull($cardId, 'Card needs to be available');
+
+		$this->requestContext->sendPlainRequest('GET', '/index.php/apps/deck/cards/' . $cardId . '/attachments');
+	}
 }
diff --git a/tests/integration/features/bootstrap/BoardContext.php b/tests/integration/features/bootstrap/BoardContext.php
index cc4dbfa7a..e237f6927 100644
--- a/tests/integration/features/bootstrap/BoardContext.php
+++ b/tests/integration/features/bootstrap/BoardContext.php
@@ -186,7 +186,9 @@ public function setTheDescriptionTo($description) {
 			['description' => $description]
 		));
 		$this->requestContext->getResponse()->getBody()->seek(0);
-		$this->card = json_decode((string)$this->getResponse()->getBody(), true);
+		if ($this->requestContext->getResponse()->getStatusCode() === 200) {
+			$this->card = json_decode((string)$this->getResponse()->getBody(), true);
+		}
 	}
 
 	/**
@@ -198,7 +200,22 @@ public function setCardAttribute($attribute, $value) {
 			[$attribute => $value]
 		));
 		$this->requestContext->getResponse()->getBody()->seek(0);
-		$this->card = json_decode((string)$this->getResponse()->getBody(), true);
+		if ($this->requestContext->getResponse()->getStatusCode() === 200) {
+			$this->card = json_decode((string)$this->getResponse()->getBody(), true);
+		}
+	}
+
+	/**
+	 * @Given /^get the card details$/
+	 */
+	public function getCard() {
+		$this->requestContext->sendJSONrequest('GET', '/index.php/apps/deck/cards/' . $this->card['id'], array_merge(
+			$this->card
+		));
+		$this->requestContext->getResponse()->getBody()->seek(0);
+		if ($this->requestContext->getResponse()->getStatusCode() === 200) {
+			$this->card = json_decode((string)$this->getResponse()->getBody(), true);
+		}
 	}
 
 	/**
@@ -253,4 +270,18 @@ public function rememberTheLastCardAs($arg1) {
 	public function getRememberedCard($arg1) {
 		return $this->storedCards[$arg1] ?? null;
 	}
+
+	/**
+	 * @Given /^delete the card$/
+	 */
+	public function deleteTheCard() {
+		$this->requestContext->sendJSONrequest('DELETE', '/index.php/apps/deck/cards/' . $this->card['id']);
+	}
+
+	/**
+	 * @Given /^delete the board/
+	 */
+	public function deleteTheBoard() {
+		$this->requestContext->sendJSONrequest('DELETE', '/index.php/apps/deck/boards/' . $this->board['id']);
+	}
 }
diff --git a/tests/integration/features/bootstrap/CommentContext.php b/tests/integration/features/bootstrap/CommentContext.php
index 40b6f4e45..92bc9a347 100644
--- a/tests/integration/features/bootstrap/CommentContext.php
+++ b/tests/integration/features/bootstrap/CommentContext.php
@@ -11,6 +11,8 @@ class CommentContext implements Context {
 	/** @var BoardContext */
 	protected $boardContext;
 
+	private $lastComment = null;
+
 	/** @BeforeScenario */
 	public function gatherContexts(BeforeScenarioScope $scope) {
 		$environment = $scope->getEnvironment();
@@ -27,5 +29,34 @@ public function postACommentWithContentOnTheCard($content) {
 			'message' => $content,
 			'parentId' => null
 		]);
+		$this->lastComment = $this->requestContext->getResponseBodyFromJson()['ocs']['data'] ?? null;
+	}
+
+	/**
+	 * @Given /^get the comments on the card$/
+	 */
+	public function getCommentsOnTheCard() {
+		$card = $this->boardContext->getLastUsedCard();
+		$this->requestContext->sendOCSRequest('GET', '/apps/deck/api/v1.0/cards/' . $card['id'] . '/comments');
+	}
+
+	/**
+	 * @When /^update a comment with content "([^"]*)" on the card$/
+	 */
+	public function updateACommentWithContentOnTheCard($content) {
+		$card = $this->boardContext->getLastUsedCard();
+		$this->requestContext->sendOCSRequest('PUT', '/apps/deck/api/v1.0/cards/' . $card['id'] . '/comments/'. $this->lastComment['id'], [
+			'message' => $content,
+			'parentId' => null
+		]);
+	}
+
+	/**
+	 * @When /^delete the comment on the card$/
+	 */
+	public function deleteTheCommentOnTheCard() {
+		$card = $this->boardContext->getLastUsedCard();
+		$this->requestContext->sendOCSRequest('DELETE', '/apps/deck/api/v1.0/cards/' . $card['id'] . '/comments/'. $this->lastComment['id']);
 	}
+
 }
diff --git a/tests/integration/features/bootstrap/ServerContext.php b/tests/integration/features/bootstrap/ServerContext.php
index 87b068ec8..6ed36f320 100644
--- a/tests/integration/features/bootstrap/ServerContext.php
+++ b/tests/integration/features/bootstrap/ServerContext.php
@@ -10,15 +10,15 @@ class ServerContext implements Context {
 		WebDav::__construct as private __tConstruct;
 	}
 
+	private string $rawBaseUrl;
+	private string $mappedUserId;
+	private array $lastInsertIds = [];
+
 	public function __construct($baseUrl) {
 		$this->rawBaseUrl = $baseUrl;
 		$this->__tConstruct($baseUrl . '/index.php/ocs/', ['admin', 'admin'], '123456');
 	}
 
-	/** @var string */
-	private $mappedUserId;
-
-	private $lastInsertIds = [];
 
 	/**
 	 * @BeforeSuite
diff --git a/tests/integration/features/decks.feature b/tests/integration/features/decks.feature
index 22a74baec..3582af430 100644
--- a/tests/integration/features/decks.feature
+++ b/tests/integration/features/decks.feature
@@ -32,3 +32,101 @@ Feature: decks
     And creates a board named "MyBoard" with color "000000"
     And create a stack named "ToDo"
     When create a card named "This is a very ong name that exceeds the maximum length of a deck board created which is longer than 255 characters This is a very ong name that exceeds the maximum length of a deck board created which is longer than 255 characters This is a very ong name that exceeds the maximum length of a deck board created which is longer than 255 characters"
+
+	Scenario: Setting a duedate on a card
+		Given acting as user "user0"
+		And creates a board named "MyBoard" with color "000000"
+		And create a stack named "ToDo"
+		And create a card named "Overdue task"
+		When get the card details
+		And the response should be a JSON array with the following mandatory values
+			|key|value|
+			|title|Overdue task|
+			|duedate||
+			|overdue|0|
+		And set the card attribute "duedate" to "2020-12-12 13:37:00"
+		When get the card details
+		And the response should be a JSON array with the following mandatory values
+			|key|value|
+			|title|Overdue task|
+			|duedate|2020-12-12T13:37:00+00:00|
+			|overdue|3|
+		And set the card attribute "duedate" to ""
+		When get the card details
+		And the response should be a JSON array with the following mandatory values
+			|key|value|
+			|title|Overdue task|
+			|duedate||
+			|overdue|0|
+
+	Scenario: Cannot access card on a deleted board
+		Given acting as user "user0"
+		And creates a board named "MyBoard" with color "000000"
+		And create a stack named "ToDo"
+		And create a card named "Overdue task"
+		And remember the last card as "deletedCard"
+		And uploads an attachment to the last used card
+		And remember the last attachment as "my-attachment"
+		And post a comment with content "My first comment" on the card
+		And delete the board
+
+		When fetching the attachment "my-attachment" for the card "deletedCard"
+		Then the response should have a status code 403
+
+		When get the comments on the card
+		Then the response should have a status code 403
+
+		When post a comment with content "My second comment" on the card
+		Then the response should have a status code 403
+
+		When uploads an attachment to the last used card
+		Then the response should have a status code 403
+
+		When set the description to "Update some text"
+		Then the response should have a status code 403
+
+		When get the card details
+		Then the response should have a status code 403
+
+		When create a card named "Overdue task"
+		Then the response should have a status code 403
+
+		When create a stack named "ToDo"
+		Then the response should have a status code 403
+
+	Scenario: Cannot access card on a deleted card
+		Given acting as user "user0"
+		And creates a board named "MyBoard" with color "000000"
+		And create a stack named "ToDo"
+		And create a card named "Overdue task"
+		And remember the last card as "deletedCard"
+		And uploads an attachment to the last used card
+		And remember the last attachment as "my-attachment"
+		And post a comment with content "My first comment" on the card
+		And delete the card
+
+		When fetching the attachment "my-attachment" for the card "deletedCard"
+		Then the response should have a status code 403
+
+		When get the comments on the card
+		Then the response should have a status code 403
+
+		When post a comment with content "My second comment" on the card
+		Then the response should have a status code 403
+
+		When deleting the attachment "my-attachment" for the card "deletedCard"
+		Then the response should have a status code 403
+
+		When uploads an attachment to the last used card
+		Then the response should have a status code 403
+
+		When get the card details
+		Then the response should have a status code 403
+
+		# We currently still expect to be able to update the card as this is used to undo deletion
+		When set the description to "Update some text"
+		Then the response should have a status code 403
+		#When set the card attribute "deletedAt" to "0"
+		#Then the response should have a status code 200
+		#When set the description to "Update some text"
+		#Then the response should have a status code 200

From b135c1fa0659653df8b4e48a2dbb9370a4931836 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Julius=20H=C3=A4rtl?= <jus@bitgrid.net>
Date: Wed, 3 Jan 2024 15:32:40 +0100
Subject: [PATCH 02/11] fix: Consider a deleted board inaccessible to share
 recipients
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Only the owner can delete/undo a board deletion so there is no reason
other users should have any permission on a board marked as deleted

Signed-off-by: Julius Härtl <jus@bitgrid.net>
---
 lib/Service/PermissionService.php | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php
index c42ca8045..62a61dc6a 100644
--- a/lib/Service/PermissionService.php
+++ b/lib/Service/PermissionService.php
@@ -102,8 +102,9 @@ public function getPermissions($boardId) {
 			return $cached;
 		}
 
-		$owner = $this->userIsBoardOwner($boardId);
-		$acls = $this->aclMapper->findAll($boardId);
+		$board = $this->getBoard($boardId);
+		$owner = $this->userIsBoardOwner($boardId, $userId);
+		$acls = $board->getDeletedAt() === 0 ? $this->aclMapper->findAll($boardId) : [];
 		$permissions = [
 			Acl::PERMISSION_READ => $owner || $this->userCan($acls, Acl::PERMISSION_READ),
 			Acl::PERMISSION_EDIT => $owner || $this->userCan($acls, Acl::PERMISSION_EDIT),

From b812075e065c4e0d76638d8d8786f59475884c03 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Julius=20H=C3=A4rtl?= <jus@bitgrid.net>
Date: Thu, 4 Jan 2024 10:53:11 +0100
Subject: [PATCH 03/11] fix: limit to non-deleted cards
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Julius Härtl <jus@bitgrid.net>
---
 lib/Service/BoardService.php      |  2 +-
 lib/Service/CommentService.php    |  9 +++------
 lib/Service/PermissionService.php | 19 +++++++++++++------
 lib/Sharing/ShareAPIHelper.php    |  2 +-
 4 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php
index f1cf2b55f..09130d7cf 100644
--- a/lib/Service/BoardService.php
+++ b/lib/Service/BoardService.php
@@ -471,7 +471,7 @@ public function addAcl($boardId, $type, $participant, $edit, $share, $manage) {
 		$newAcl = $this->aclMapper->insert($acl);
 
 		$this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_BOARD, $newAcl, ActivityManager::SUBJECT_BOARD_SHARE, [], $this->userId);
-		$this->notificationHelper->sendBoardShared((int)$boardId, $acl);
+		$this->notificationHelper->sendBoardShared($boardId, $acl);
 		$this->boardMapper->mapAcl($newAcl);
 		$this->changeHelper->boardChanged($boardId);
 
diff --git a/lib/Service/CommentService.php b/lib/Service/CommentService.php
index c389a9ad2..d15c7cc88 100644
--- a/lib/Service/CommentService.php
+++ b/lib/Service/CommentService.php
@@ -90,17 +90,14 @@ public function list(string $cardId, int $limit = 20, int $offset = 0): DataResp
 	 * @throws BadRequestException
 	 * @throws NotFoundException|NoPermissionException
 	 */
-	public function create(string $cardId, string $message, string $replyTo = '0'): DataResponse {
-		if (!is_numeric($cardId)) {
-			throw new BadRequestException('A valid card id must be provided');
-		}
+	public function create(int $cardId, string $message, string $replyTo = '0'): DataResponse {
 		$this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_READ);
 
 		// Check if parent is a comment on the same card
 		if ($replyTo !== '0') {
 			try {
 				$comment = $this->commentsManager->get($replyTo);
-				if ($comment->getObjectType() !== Application::COMMENT_ENTITY_TYPE || $comment->getObjectId() !== $cardId) {
+				if ($comment->getObjectType() !== Application::COMMENT_ENTITY_TYPE || (int)$comment->getObjectId() !== $cardId) {
 					throw new CommentNotFoundException();
 				}
 			} catch (CommentNotFoundException $e) {
@@ -109,7 +106,7 @@ public function create(string $cardId, string $message, string $replyTo = '0'):
 		}
 
 		try {
-			$comment = $this->commentsManager->create('users', $this->userId, Application::COMMENT_ENTITY_TYPE, $cardId);
+			$comment = $this->commentsManager->create('users', $this->userId, Application::COMMENT_ENTITY_TYPE, (string)$cardId);
 			$comment->setMessage($message);
 			$comment->setVerb('comment');
 			$comment->setParentId($replyTo);
diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php
index 62a61dc6a..5b0f3ccdd 100644
--- a/lib/Service/PermissionService.php
+++ b/lib/Service/PermissionService.php
@@ -29,6 +29,7 @@
 use OCA\Deck\Db\AclMapper;
 use OCA\Deck\Db\Board;
 use OCA\Deck\Db\BoardMapper;
+use OCA\Deck\Db\CardMapper;
 use OCA\Deck\Db\IPermissionMapper;
 use OCA\Deck\Db\User;
 use OCA\Deck\NoPermissionException;
@@ -138,13 +139,10 @@ public function matchPermissions(Board $board) {
 	/**
 	 * check permissions for replacing dark magic middleware
 	 *
-	 * @param $mapper IPermissionMapper|null null if $id is a boardId
-	 * @param $id int unique identifier of the Entity
-	 * @param $permission int
-	 * @return bool
+	 * @param numeric $id
 	 * @throws NoPermissionException
 	 */
-	public function checkPermission($mapper, $id, $permission, $userId = null) {
+	public function checkPermission($mapper, $id, $permission, $userId = null, bool $allowDeletedCard = false) {
 		$boardId = $id;
 		if ($mapper instanceof IPermissionMapper && !($mapper instanceof BoardMapper)) {
 			$boardId = $mapper->findBoardId($id);
@@ -158,7 +156,16 @@ public function checkPermission($mapper, $id, $permission, $userId = null) {
 			throw new NoPermissionException('Permission denied');
 		}
 
-		if ($this->userIsBoardOwner($boardId, $userId)) {
+		$permissions = $this->getPermissions($boardId, $userId);
+		if ($permissions[$permission] === true) {
+
+			if (!$allowDeletedCard && $mapper instanceof CardMapper) {
+				$card = $mapper->find($id);
+				if ($card->getDeletedAt() > 0) {
+					throw new NoPermissionException('Card is deleted');
+				}
+			}
+
 			return true;
 		}
 
diff --git a/lib/Sharing/ShareAPIHelper.php b/lib/Sharing/ShareAPIHelper.php
index 5528b6a92..41a5dfc6d 100644
--- a/lib/Sharing/ShareAPIHelper.php
+++ b/lib/Sharing/ShareAPIHelper.php
@@ -115,7 +115,7 @@ private function parseDate(string $expireDate): \DateTime {
 	 */
 	public function canAccessShare(IShare $share, string $user): bool {
 		try {
-			$this->permissionService->checkPermission($this->cardMapper, $share->getSharedWith(), Acl::PERMISSION_READ, $user);
+			$this->permissionService->checkPermission($this->cardMapper, (int)$share->getSharedWith(), Acl::PERMISSION_READ, $user);
 		} catch (NoPermissionException $e) {
 			return false;
 		}

From 23a0ec226b90f3f481698983a3c42256e51c5845 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Julius=20H=C3=A4rtl?= <jus@bitgrid.net>
Date: Thu, 4 Jan 2024 14:01:24 +0100
Subject: [PATCH 04/11] fix: Further limit updating cards
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Julius Härtl <jus@bitgrid.net>
---
 lib/Service/CardService.php                           | 8 ++++----
 tests/integration/features/bootstrap/BoardContext.php | 1 +
 tests/integration/features/decks.feature              | 8 ++++----
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php
index c648e0979..909154166 100644
--- a/lib/Service/CardService.php
+++ b/lib/Service/CardService.php
@@ -1,4 +1,4 @@
-<?php
+lib/Service/CardService.php<?php
 /**
  * @copyright Copyright (c) 2016 Julius Härtl <jus@bitgrid.net>
  *
@@ -264,7 +264,7 @@ public function delete($id) {
 	public function update($id, $title, $stackId, $type, $owner, $description = '', $order = 0, $duedate = null, $deletedAt = null, $archived = null) {
 		$this->cardServiceValidator->check(compact('id', 'title', 'stackId', 'type', 'owner', 'order'));
 
-		$this->permissionService->checkPermission($this->cardMapper, $id, Acl::PERMISSION_EDIT);
+		$this->permissionService->checkPermission($this->cardMapper, $id, Acl::PERMISSION_EDIT, allowDeletedCard: true);
 		$this->permissionService->checkPermission($this->stackMapper, $stackId, Acl::PERMISSION_EDIT);
 
 		if ($this->boardService->isArchived($this->cardMapper, $id)) {
@@ -276,9 +276,9 @@ public function update($id, $title, $stackId, $type, $owner, $description = '',
 		}
 
 		if ($card->getDeletedAt() !== 0) {
-			if ($deletedAt === null) {
+			if ($deletedAt === null || $deletedAt > 0) {
 				// Only allow operations when restoring the card
-				throw new StatusException('Operation not allowed. This card was deleted.');
+				throw new NoPermissionException('Operation not allowed. This card was deleted.');
 			}
 		}
 
diff --git a/tests/integration/features/bootstrap/BoardContext.php b/tests/integration/features/bootstrap/BoardContext.php
index e237f6927..34647ce80 100644
--- a/tests/integration/features/bootstrap/BoardContext.php
+++ b/tests/integration/features/bootstrap/BoardContext.php
@@ -276,6 +276,7 @@ public function getRememberedCard($arg1) {
 	 */
 	public function deleteTheCard() {
 		$this->requestContext->sendJSONrequest('DELETE', '/index.php/apps/deck/cards/' . $this->card['id']);
+		$this->card['deletedAt'] = time();
 	}
 
 	/**
diff --git a/tests/integration/features/decks.feature b/tests/integration/features/decks.feature
index 3582af430..9656c2a23 100644
--- a/tests/integration/features/decks.feature
+++ b/tests/integration/features/decks.feature
@@ -126,7 +126,7 @@ Feature: decks
 		# We currently still expect to be able to update the card as this is used to undo deletion
 		When set the description to "Update some text"
 		Then the response should have a status code 403
-		#When set the card attribute "deletedAt" to "0"
-		#Then the response should have a status code 200
-		#When set the description to "Update some text"
-		#Then the response should have a status code 200
+		When set the card attribute "deletedAt" to "0"
+		Then the response should have a status code 200
+		When set the description to "Update some text"
+		Then the response should have a status code 200

From 2fc91455ee6cf676e30103e4949e5f592180b2a5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Julius=20H=C3=A4rtl?= <jus@bitgrid.net>
Date: Thu, 4 Jan 2024 14:01:49 +0100
Subject: [PATCH 05/11] fix: Limit card activities for deleted cards
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Julius Härtl <jus@bitgrid.net>
---
 lib/Activity/ActivityManager.php              | 21 +++++++++++++++++
 lib/Activity/DeckProvider.php                 |  6 +++++
 .../features/bootstrap/BoardContext.php       | 23 +++++++++++++++++--
 tests/integration/features/decks.feature      |  5 ++++
 tests/unit/Activity/DeckProviderTest.php      |  3 +++
 5 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/lib/Activity/ActivityManager.php b/lib/Activity/ActivityManager.php
index 10e1ceb74..49597eb78 100644
--- a/lib/Activity/ActivityManager.php
+++ b/lib/Activity/ActivityManager.php
@@ -39,6 +39,7 @@
 use OCA\Deck\Db\Label;
 use OCA\Deck\Db\Stack;
 use OCA\Deck\Db\StackMapper;
+use OCA\Deck\NoPermissionException;
 use OCA\Deck\Service\PermissionService;
 use OCP\Activity\IEvent;
 use OCP\Activity\IManager;
@@ -543,4 +544,24 @@ private function findDetailsForAcl($aclId) {
 			'board' => $board
 		];
 	}
+
+	public function canSeeCardActivity(int $cardId): bool {
+		try {
+			$this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_READ);
+			$card = $this->cardMapper->find($cardId);
+			return $card->getDeletedAt() === 0;
+		} catch (NoPermissionException $e) {
+			return false;
+		}
+	}
+
+	public function canSeeBoardActivity(int $boardId): bool {
+		try {
+			$this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ);
+			$board = $this->boardMapper->find($boardId);
+			return $board->getDeletedAt() === 0;
+		} catch (NoPermissionException $e) {
+			return false;
+		}
+	}
 }
diff --git a/lib/Activity/DeckProvider.php b/lib/Activity/DeckProvider.php
index 4235816e5..7779f5757 100644
--- a/lib/Activity/DeckProvider.php
+++ b/lib/Activity/DeckProvider.php
@@ -111,6 +111,9 @@ public function parse($language, IEvent $event, IEvent $previousEvent = null) {
 			$event->setAuthor($author);
 		}
 		if ($event->getObjectType() === ActivityManager::DECK_OBJECT_BOARD) {
+			if (!$this->activityManager->canSeeBoardActivity($event->getObjectId())) {
+				throw new \InvalidArgumentException();
+			}
 			if (isset($subjectParams['board']) && $event->getObjectName() === '') {
 				$event->setObject($event->getObjectType(), $event->getObjectId(), $subjectParams['board']['title']);
 			}
@@ -125,6 +128,9 @@ public function parse($language, IEvent $event, IEvent $previousEvent = null) {
 		}
 
 		if (isset($subjectParams['card']) && $event->getObjectType() === ActivityManager::DECK_OBJECT_CARD) {
+			if (!$this->activityManager->canSeeCardActivity($event->getObjectId())) {
+				throw new \InvalidArgumentException();
+			}
 			if ($event->getObjectName() === '') {
 				$event->setObject($event->getObjectType(), $event->getObjectId(), $subjectParams['card']['title']);
 			}
diff --git a/tests/integration/features/bootstrap/BoardContext.php b/tests/integration/features/bootstrap/BoardContext.php
index 34647ce80..dbd67687e 100644
--- a/tests/integration/features/bootstrap/BoardContext.php
+++ b/tests/integration/features/bootstrap/BoardContext.php
@@ -17,9 +17,9 @@ class BoardContext implements Context {
 	/** @var array last card response */
 	private $card = null;
 	private $storedCards = [];
+	private $activities = null;
 
-	/** @var ServerContext */
-	private $serverContext;
+	private ServerContext $serverContext;
 
 	/** @BeforeScenario */
 	public function gatherContexts(BeforeScenarioScope $scope) {
@@ -285,4 +285,23 @@ public function deleteTheCard() {
 	public function deleteTheBoard() {
 		$this->requestContext->sendJSONrequest('DELETE', '/index.php/apps/deck/boards/' . $this->board['id']);
 	}
+
+
+	/**
+	 * @Given /^get the activities for the last card$/
+	 */
+	public function getActivitiesForTheLastCard() {
+		$card = $this->getLastUsedCard();
+		$this->requestContext->sendOCSRequest('GET', '/apps/activity/api/v2/activity/filter?format=json&type=deck&since=0&object_type=deck_card&object_id=' . $card['id'] . '&limit=50');
+		$this->activities = json_decode((string)$this->getResponse()->getBody(), true)['ocs']['data'] ?? null;
+	}
+
+	/**
+	 * @Then the fetched activities should have :count entries
+	 */
+	public function theFetchedActivitiesShouldHaveEntries($count) {
+		Assert::assertEquals($count, count($this->activities ?? []));
+	}
+
+
 }
diff --git a/tests/integration/features/decks.feature b/tests/integration/features/decks.feature
index 9656c2a23..0351f6269 100644
--- a/tests/integration/features/decks.feature
+++ b/tests/integration/features/decks.feature
@@ -103,8 +103,13 @@ Feature: decks
 		And uploads an attachment to the last used card
 		And remember the last attachment as "my-attachment"
 		And post a comment with content "My first comment" on the card
+		When get the activities for the last card
+		Then the fetched activities should have 3 entries
 		And delete the card
 
+		When get the activities for the last card
+		Then the fetched activities should have 0 entries
+
 		When fetching the attachment "my-attachment" for the card "deletedCard"
 		Then the response should have a status code 403
 
diff --git a/tests/unit/Activity/DeckProviderTest.php b/tests/unit/Activity/DeckProviderTest.php
index 4302834cb..13cdf691e 100644
--- a/tests/unit/Activity/DeckProviderTest.php
+++ b/tests/unit/Activity/DeckProviderTest.php
@@ -74,6 +74,9 @@ public function setUp(): void {
 		$this->config = $this->createMock(IConfig::class);
 		$this->cardService = $this->createMock(CardService::class);
 		$this->provider = new DeckProvider($this->urlGenerator, $this->activityManager, $this->userManager, $this->commentsManager, $this->l10nFactory, $this->config, $this->userId, $this->cardService);
+
+		$this->activityManager->method('canSeeCardActivity')->willReturn(true);
+		$this->activityManager->method('canSeeBoardActivity')->willReturn(true);
 	}
 
 	private function mockEvent($objectType, $objectId, $objectName, $subject, $subjectParameters = []) {

From 6a3f4ecee945add78a514417fce2b9375ef863eb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Julius=20H=C3=A4rtl?= <jus@bitgrid.net>
Date: Thu, 4 Jan 2024 16:01:01 +0100
Subject: [PATCH 06/11] chore: Fix ci setup for activity
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Julius Härtl <jus@bitgrid.net>
---
 .github/workflows/integration.yml | 7 +++++++
 composer.json                     | 3 ++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml
index 994526fc5..95e6783d2 100644
--- a/.github/workflows/integration.yml
+++ b/.github/workflows/integration.yml
@@ -60,6 +60,13 @@ jobs:
         with:
           path: apps/${{ env.APP_NAME }}
 
+      -   name: Checkout activity
+          uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
+          with:
+              repository: nextcloud/activity
+              ref: ${{ matrix.server-versions }}
+              path: apps/activity
+
       - name: Set up php ${{ matrix.php-versions }}
         uses: shivammathur/setup-php@2.15.0
         with:
diff --git a/composer.json b/composer.json
index 413a7ecb2..e586c60cc 100644
--- a/composer.json
+++ b/composer.json
@@ -38,7 +38,8 @@
 			"@test:integration"
 		],
 		"test:unit": "phpunit -c tests/phpunit.xml",
-		"test:integration": "phpunit -c tests/phpunit.integration.xml && cd tests/integration && ./run.sh"
+		"test:integration": "phpunit -c tests/phpunit.integration.xml",
+		"test:api": "cd tests/integration && ./run.sh"
 	},
 	"autoload-dev": {
 		"psr-4": {

From 635880a27b31969eef9a45f09652b546bb6e1155 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Julius=20H=C3=A4rtl?= <jus@bitgrid.net>
Date: Tue, 9 Jan 2024 17:52:41 +0100
Subject: [PATCH 07/11] tests: Fix missing behat context methods
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Julius Härtl <jus@bitgrid.net>
---
 lib/Service/PermissionService.php             |  1 -
 .../features/bootstrap/BoardContext.php       |  2 --
 .../features/bootstrap/CommentContext.php     |  1 -
 .../features/bootstrap/RequestContext.php     | 25 +++++++++++++++++++
 4 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php
index 5b0f3ccdd..48d6af96d 100644
--- a/lib/Service/PermissionService.php
+++ b/lib/Service/PermissionService.php
@@ -158,7 +158,6 @@ public function checkPermission($mapper, $id, $permission, $userId = null, bool
 
 		$permissions = $this->getPermissions($boardId, $userId);
 		if ($permissions[$permission] === true) {
-
 			if (!$allowDeletedCard && $mapper instanceof CardMapper) {
 				$card = $mapper->find($id);
 				if ($card->getDeletedAt() > 0) {
diff --git a/tests/integration/features/bootstrap/BoardContext.php b/tests/integration/features/bootstrap/BoardContext.php
index dbd67687e..315aa8bcf 100644
--- a/tests/integration/features/bootstrap/BoardContext.php
+++ b/tests/integration/features/bootstrap/BoardContext.php
@@ -302,6 +302,4 @@ public function getActivitiesForTheLastCard() {
 	public function theFetchedActivitiesShouldHaveEntries($count) {
 		Assert::assertEquals($count, count($this->activities ?? []));
 	}
-
-
 }
diff --git a/tests/integration/features/bootstrap/CommentContext.php b/tests/integration/features/bootstrap/CommentContext.php
index 92bc9a347..aba5a6065 100644
--- a/tests/integration/features/bootstrap/CommentContext.php
+++ b/tests/integration/features/bootstrap/CommentContext.php
@@ -58,5 +58,4 @@ public function deleteTheCommentOnTheCard() {
 		$card = $this->boardContext->getLastUsedCard();
 		$this->requestContext->sendOCSRequest('DELETE', '/apps/deck/api/v1.0/cards/' . $card['id'] . '/comments/'. $this->lastComment['id']);
 	}
-
 }
diff --git a/tests/integration/features/bootstrap/RequestContext.php b/tests/integration/features/bootstrap/RequestContext.php
index 9df6be205..ebf01e80a 100644
--- a/tests/integration/features/bootstrap/RequestContext.php
+++ b/tests/integration/features/bootstrap/RequestContext.php
@@ -166,4 +166,29 @@ public function getResponseBodyFromJson() {
 		$this->getResponse()->getBody()->seek(0);
 		return json_decode((string)$this->getResponse()->getBody(), true);
 	}
+
+	/**
+	 * @Given /^the response should be a list of objects$/
+	 */
+	public function theResponseShouldBeAListOfObjects() {
+		$jsonResponse = $this->getResponseBodyFromJson();
+		Assert::assertEquals(array_keys($jsonResponse), range(0, count($jsonResponse) - 1));
+	}
+
+	/**
+	 * @When /^the response should contain an element with the properties$/
+	 */
+	public function responseContainsElement(TableNode $element) {
+		$json = $this->getResponseBodyFromJson();
+		$found = array_filter($json, function ($board) use ($element) {
+			foreach ($element as $row) {
+				if ($row['value'] !== $board[$row['property']]) {
+					return false;
+				}
+			}
+
+			return true;
+		});
+		Assert::assertEquals(1, count($found));
+	}
 }

From 16e5b36d2fd7634c5407dce8b9b286794caebb1b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Julius=20H=C3=A4rtl?= <jus@bitgrid.net>
Date: Tue, 9 Jan 2024 18:08:53 +0100
Subject: [PATCH 08/11] fix: PHP 7.4 compatibility
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Julius Härtl <jus@bitgrid.net>
---
 lib/Service/CardService.php                            |  4 ++--
 lib/Service/CommentService.php                         |  6 +-----
 lib/Service/PermissionService.php                      | 10 +++++++---
 tests/integration/features/bootstrap/BoardContext.php  |  2 +-
 tests/integration/features/bootstrap/ServerContext.php |  6 +++---
 5 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php
index 909154166..58861df84 100644
--- a/lib/Service/CardService.php
+++ b/lib/Service/CardService.php
@@ -1,4 +1,4 @@
-lib/Service/CardService.php<?php
+<?php
 /**
  * @copyright Copyright (c) 2016 Julius Härtl <jus@bitgrid.net>
  *
@@ -264,7 +264,7 @@ public function delete($id) {
 	public function update($id, $title, $stackId, $type, $owner, $description = '', $order = 0, $duedate = null, $deletedAt = null, $archived = null) {
 		$this->cardServiceValidator->check(compact('id', 'title', 'stackId', 'type', 'owner', 'order'));
 
-		$this->permissionService->checkPermission($this->cardMapper, $id, Acl::PERMISSION_EDIT, allowDeletedCard: true);
+		$this->permissionService->checkPermission($this->cardMapper, $id, Acl::PERMISSION_EDIT, null, true);
 		$this->permissionService->checkPermission($this->stackMapper, $stackId, Acl::PERMISSION_EDIT);
 
 		if ($this->boardService->isArchived($this->cardMapper, $id)) {
diff --git a/lib/Service/CommentService.php b/lib/Service/CommentService.php
index d15c7cc88..5f47e0893 100644
--- a/lib/Service/CommentService.php
+++ b/lib/Service/CommentService.php
@@ -83,10 +83,6 @@ public function list(string $cardId, int $limit = 20, int $offset = 0): DataResp
 	}
 
 	/**
-	 * @param string $cardId
-	 * @param string $message
-	 * @param string $replyTo
-	 * @return DataResponse
 	 * @throws BadRequestException
 	 * @throws NotFoundException|NoPermissionException
 	 */
@@ -142,7 +138,7 @@ public function update(string $cardId, string $commentId, string $message): Data
 			throw new NoPermissionException('Only authors are allowed to edit their comment.');
 		}
 		if ($comment->getParentId() !== '0') {
-			$this->permissionService->checkPermission($this->cardMapper, $comment->getParentId(), Acl::PERMISSION_READ);
+			$this->permissionService->checkPermission($this->cardMapper, (int)$comment->getParentId(), Acl::PERMISSION_READ);
 		}
 
 		$comment->setMessage($message);
diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php
index 48d6af96d..5fd657c37 100644
--- a/lib/Service/PermissionService.php
+++ b/lib/Service/PermissionService.php
@@ -98,7 +98,11 @@ public function __construct(
 	 * @param $boardId
 	 * @return bool|array
 	 */
-	public function getPermissions($boardId) {
+	public function getPermissions($boardId, ?string $userId = null) {
+		if ($userId === null) {
+			$userId = $this->userId;
+		}
+
 		if ($cached = $this->permissionCache->get($boardId)) {
 			return $cached;
 		}
@@ -113,7 +117,7 @@ public function getPermissions($boardId) {
 			Acl::PERMISSION_SHARE => ($owner || $this->userCan($acls, Acl::PERMISSION_SHARE))
 				&& (!$this->shareManager->sharingDisabledForUser($this->userId))
 		];
-		$this->permissionCache->set($boardId, $permissions);
+		$this->permissionCache->set((string)$boardId, $permissions);
 		return $permissions;
 	}
 
@@ -169,7 +173,7 @@ public function checkPermission($mapper, $id, $permission, $userId = null, bool
 		}
 
 		try {
-			$acls = $this->getBoard($boardId)->getAcl() ?? [];
+			$acls = $this->getBoard((int)$boardId)->getAcl() ?? [];
 			$result = $this->userCan($acls, $permission, $userId);
 			if ($result) {
 				return true;
diff --git a/tests/integration/features/bootstrap/BoardContext.php b/tests/integration/features/bootstrap/BoardContext.php
index 315aa8bcf..d0484a52b 100644
--- a/tests/integration/features/bootstrap/BoardContext.php
+++ b/tests/integration/features/bootstrap/BoardContext.php
@@ -19,7 +19,7 @@ class BoardContext implements Context {
 	private $storedCards = [];
 	private $activities = null;
 
-	private ServerContext $serverContext;
+	private $serverContext;
 
 	/** @BeforeScenario */
 	public function gatherContexts(BeforeScenarioScope $scope) {
diff --git a/tests/integration/features/bootstrap/ServerContext.php b/tests/integration/features/bootstrap/ServerContext.php
index 6ed36f320..b10429b61 100644
--- a/tests/integration/features/bootstrap/ServerContext.php
+++ b/tests/integration/features/bootstrap/ServerContext.php
@@ -10,9 +10,9 @@ class ServerContext implements Context {
 		WebDav::__construct as private __tConstruct;
 	}
 
-	private string $rawBaseUrl;
-	private string $mappedUserId;
-	private array $lastInsertIds = [];
+	private $rawBaseUrl;
+	private $mappedUserId;
+	private $lastInsertIds = [];
 
 	public function __construct($baseUrl) {
 		$this->rawBaseUrl = $baseUrl;

From 82deb677d2a70cca99767abcfc9ce633f857dd2d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Julius=20H=C3=A4rtl?= <jus@bitgrid.net>
Date: Tue, 9 Jan 2024 23:45:30 +0100
Subject: [PATCH 09/11] chore: update actions
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Julius Härtl <jus@bitgrid.net>
---
 .github/workflows/fixup.yml | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/.github/workflows/fixup.yml b/.github/workflows/fixup.yml
index 6092cc3a5..9548d19f2 100644
--- a/.github/workflows/fixup.yml
+++ b/.github/workflows/fixup.yml
@@ -3,18 +3,31 @@
 # https://github.com/nextcloud/.github
 # https://docs.github.com/en/actions/learn-github-actions/sharing-workflows-with-your-organization
 
-name: Pull request checks
+name: Block fixup and squash commits
 
-on: pull_request
+on:
+  pull_request:
+    types: [opened, ready_for_review, reopened, synchronize]
+
+permissions:
+  contents: read
+
+concurrency:
+  group: fixup-${{ github.head_ref || github.run_id }}
+  cancel-in-progress: true
 
 jobs:
   commit-message-check:
+    if: github.event.pull_request.draft == false
+
+    permissions:
+      pull-requests: write
     name: Block fixup and squash commits
 
     runs-on: ubuntu-latest
 
     steps:
       - name: Run check
-        uses: xt0rted/block-autosquash-commits-action@v2
+        uses: skjnldsv/block-fixup-merge-action@42d26e1b536ce61e5cf467d65fb76caf4aa85acf # v1
         with:
           repo-token: ${{ secrets.GITHUB_TOKEN }}

From 1be21db78e92cc87553772e75eb4c394a4148c1c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Julius=20H=C3=A4rtl?= <jus@bitgrid.net>
Date: Tue, 9 Jan 2024 23:48:51 +0100
Subject: [PATCH 10/11] ci: Remove unused test
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Julius Härtl <jus@bitgrid.net>
---
 tests/integration/features/decks.feature | 26 ------------------------
 1 file changed, 26 deletions(-)

diff --git a/tests/integration/features/decks.feature b/tests/integration/features/decks.feature
index 0351f6269..1913c8420 100644
--- a/tests/integration/features/decks.feature
+++ b/tests/integration/features/decks.feature
@@ -33,32 +33,6 @@ Feature: decks
     And create a stack named "ToDo"
     When create a card named "This is a very ong name that exceeds the maximum length of a deck board created which is longer than 255 characters This is a very ong name that exceeds the maximum length of a deck board created which is longer than 255 characters This is a very ong name that exceeds the maximum length of a deck board created which is longer than 255 characters"
 
-	Scenario: Setting a duedate on a card
-		Given acting as user "user0"
-		And creates a board named "MyBoard" with color "000000"
-		And create a stack named "ToDo"
-		And create a card named "Overdue task"
-		When get the card details
-		And the response should be a JSON array with the following mandatory values
-			|key|value|
-			|title|Overdue task|
-			|duedate||
-			|overdue|0|
-		And set the card attribute "duedate" to "2020-12-12 13:37:00"
-		When get the card details
-		And the response should be a JSON array with the following mandatory values
-			|key|value|
-			|title|Overdue task|
-			|duedate|2020-12-12T13:37:00+00:00|
-			|overdue|3|
-		And set the card attribute "duedate" to ""
-		When get the card details
-		And the response should be a JSON array with the following mandatory values
-			|key|value|
-			|title|Overdue task|
-			|duedate||
-			|overdue|0|
-
 	Scenario: Cannot access card on a deleted board
 		Given acting as user "user0"
 		And creates a board named "MyBoard" with color "000000"

From 227d9df978f8a57cabae511fd0e9dfc70067c8cb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Julius=20H=C3=A4rtl?= <jus@bitgrid.net>
Date: Thu, 11 Jan 2024 14:21:59 +0100
Subject: [PATCH 11/11] test: Adapt unit tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Julius Härtl <jus@bitgrid.net>
---
 lib/Service/PermissionService.php            | 20 ++++++++++----------
 tests/unit/Service/PermissionServiceTest.php |  4 ++++
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php
index 5fd657c37..32c0ed078 100644
--- a/lib/Service/PermissionService.php
+++ b/lib/Service/PermissionService.php
@@ -160,19 +160,19 @@ public function checkPermission($mapper, $id, $permission, $userId = null, bool
 			throw new NoPermissionException('Permission denied');
 		}
 
-		$permissions = $this->getPermissions($boardId, $userId);
-		if ($permissions[$permission] === true) {
-			if (!$allowDeletedCard && $mapper instanceof CardMapper) {
-				$card = $mapper->find($id);
-				if ($card->getDeletedAt() > 0) {
-					throw new NoPermissionException('Card is deleted');
+		try {
+			$permissions = $this->getPermissions($boardId, $userId);
+			if ($permissions[$permission] === true) {
+				if (!$allowDeletedCard && $mapper instanceof CardMapper) {
+					$card = $mapper->find($id);
+					if ($card->getDeletedAt() > 0) {
+						throw new NoPermissionException('Card is deleted');
+					}
 				}
-			}
 
-			return true;
-		}
+				return true;
+			}
 
-		try {
 			$acls = $this->getBoard((int)$boardId)->getAcl() ?? [];
 			$result = $this->userCan($acls, $permission, $userId);
 			if ($result) {
diff --git a/tests/unit/Service/PermissionServiceTest.php b/tests/unit/Service/PermissionServiceTest.php
index 0994a778e..1bf00c5f3 100644
--- a/tests/unit/Service/PermissionServiceTest.php
+++ b/tests/unit/Service/PermissionServiceTest.php
@@ -240,6 +240,8 @@ public function testCheckPermission($boardId, $permission, $result, $owner = 'fo
 			->method('sharingDisabledForUser')
 			->willReturn(false);
 
+		$this->aclMapper->method('findAll')->willReturn([]);
+
 		if ($result) {
 			$actual = $this->service->checkPermission($mapper, 1234, $permission);
 			$this->assertTrue($actual);
@@ -262,6 +264,8 @@ public function testCheckPermissionWithoutMapper($boardId, $permission, $result,
 			$this->boardMapper->expects($this->any())->method('find')->willReturn($board);
 		}
 
+		$this->aclMapper->method('findAll')->willReturn([]);
+
 		if ($result) {
 			$actual = $this->service->checkPermission($mapper, 1234, $permission);
 			$this->assertTrue($actual);