From f094bce28636e66fc2c689eebb3a01cd2a57a926 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Fri, 17 Nov 2023 10:29:41 +0100 Subject: [PATCH 1/5] fix: Only query boards not marked for deletion unless we want to undo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Collaboration/Resources/ResourceProvider.php | 2 +- lib/Db/BoardMapper.php | 4 +++- lib/Service/BoardService.php | 8 ++++---- lib/Service/CardService.php | 8 ++++++++ lib/Service/PermissionService.php | 8 ++++---- tests/unit/Activity/ActivityManagerTest.php | 1 + 6 files changed, 21 insertions(+), 10 deletions(-) diff --git a/lib/Collaboration/Resources/ResourceProvider.php b/lib/Collaboration/Resources/ResourceProvider.php index b4677148d..974976b0f 100644 --- a/lib/Collaboration/Resources/ResourceProvider.php +++ b/lib/Collaboration/Resources/ResourceProvider.php @@ -108,7 +108,7 @@ public function canAccessResource(IResource $resource, ?IUser $user): bool { private function getBoard(IResource $resource) { try { - return $this->boardMapper->find($resource->getId(), false, true); + return $this->boardMapper->find((int)$resource->getId(), false, true); } catch (DoesNotExistException $e) { } catch (MultipleObjectsReturnedException $e) { return null; diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index 93be76d7e..5b1e6407e 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -79,12 +79,14 @@ public function __construct( * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException * @throws DoesNotExistException */ - public function find($id, $withLabels = false, $withAcl = false): Board { + public function find(int $id, bool $withLabels = false, bool $withAcl = false, bool $allowDeleted = false): Board { if (!isset($this->boardCache[$id])) { $qb = $this->db->getQueryBuilder(); + $deletedWhere = $allowDeleted ? $qb->expr()->gte('deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)) : $qb->expr()->eq('deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)); $qb->select('*') ->from('deck_boards') ->where($qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT))) + ->andWhere($deletedWhere) ->orderBy('id'); $this->boardCache[$id] = $this->findEntity($qb); } diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index 9a98decbf..2c3bca46b 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -181,7 +181,7 @@ public function findAll($since = -1, $details = null, $includeArchived = true) { * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException * @throws BadRequestException */ - public function find($boardId) { + public function find($boardId, bool $allowDeleted = false) { $this->boardServiceValidator->check(compact('boardId')); if ($this->boardsCache && isset($this->boardsCache[$boardId])) { return $this->boardsCache[$boardId]; @@ -192,7 +192,7 @@ public function find($boardId) { $this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ); /** @var Board $board */ - $board = $this->boardMapper->find($boardId, true, true); + $board = $this->boardMapper->find($boardId, true, true, $allowDeleted); $this->boardMapper->mapOwner($board); if ($board->getAcl() !== null) { foreach ($board->getAcl() as $acl) { @@ -367,7 +367,7 @@ public function deleteUndo($id) { $this->boardServiceValidator->check(compact('id')); $this->permissionService->checkPermission($this->boardMapper, $id, Acl::PERMISSION_MANAGE); - $board = $this->find($id); + $board = $this->find($id, true); $board->setDeletedAt(0); $board = $this->boardMapper->update($board); $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_BOARD, $board, ActivityManager::SUBJECT_BOARD_RESTORE); @@ -388,7 +388,7 @@ public function deleteForce($id) { $this->boardServiceValidator->check(compact('id')); $this->permissionService->checkPermission($this->boardMapper, $id, Acl::PERMISSION_MANAGE); - $board = $this->find($id); + $board = $this->find($id, true); $delete = $this->boardMapper->delete($board); return $delete; diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index 8d8522790..c648e0979 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -274,6 +274,14 @@ public function update($id, $title, $stackId, $type, $owner, $description = '', if ($archived !== null && $card->getArchived() && $archived === true) { throw new StatusException('Operation not allowed. This card is archived.'); } + + if ($card->getDeletedAt() !== 0) { + if ($deletedAt === null) { + // Only allow operations when restoring the card + throw new StatusException('Operation not allowed. This card was deleted.'); + } + } + $changes = new ChangeSet($card); if ($card->getLastEditor() !== $this->currentUser && $card->getLastEditor() !== null) { $this->activityManager->triggerEvent( diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index 06a6da62d..c42ca8045 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -194,11 +194,11 @@ public function userIsBoardOwner($boardId, $userId = null) { * @throws MultipleObjectsReturnedException * @throws DoesNotExistException */ - private function getBoard($boardId): Board { - if (!isset($this->boardCache[$boardId])) { - $this->boardCache[$boardId] = $this->boardMapper->find($boardId, false, true); + private function getBoard(int $boardId): Board { + if (!isset($this->boardCache[(string)$boardId])) { + $this->boardCache[(string)$boardId] = $this->boardMapper->find($boardId, false, true); } - return $this->boardCache[$boardId]; + return $this->boardCache[(string)$boardId]; } /** diff --git a/tests/unit/Activity/ActivityManagerTest.php b/tests/unit/Activity/ActivityManagerTest.php index d292d9023..f7b011c05 100644 --- a/tests/unit/Activity/ActivityManagerTest.php +++ b/tests/unit/Activity/ActivityManagerTest.php @@ -124,6 +124,7 @@ public function testGetActivityFormatOwn() { public function testCreateEvent() { $board = new Board(); + $board->setId(123); $board->setTitle(''); $this->boardMapper->expects($this->once()) ->method('find') From d88844bd7a0993d05b4dde2d45e47e0dc3b7be8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Tue, 9 Jan 2024 17:58:38 +0100 Subject: [PATCH 2/5] ci: Update phpunit github action MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- .github/workflows/phpunit.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml index 4f3d5cea4..0cfda0dd1 100644 --- a/.github/workflows/phpunit.yml +++ b/.github/workflows/phpunit.yml @@ -67,6 +67,7 @@ jobs: php-version: ${{ matrix.php-versions }} tools: phpunit extensions: zip, gd, mbstring, iconv, fileinfo, intl, sqlite, pdo_sqlite, mysql, pdo_mysql, pgsql, pdo_pgsql + ini-file: development coverage: none - name: Set up PHPUnit From e53938038e6cdcef1f42f5b01abe37bcab87f553 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Tue, 9 Jan 2024 18:01:37 +0100 Subject: [PATCH 3/5] fix: Psalm and CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Service/BoardService.php | 2 +- tests/integration/base-query-count.txt | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 tests/integration/base-query-count.txt diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index 2c3bca46b..f1cf2b55f 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -192,7 +192,7 @@ public function find($boardId, bool $allowDeleted = false) { $this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ); /** @var Board $board */ - $board = $this->boardMapper->find($boardId, true, true, $allowDeleted); + $board = $this->boardMapper->find((int)$boardId, true, true, $allowDeleted); $this->boardMapper->mapOwner($board); if ($board->getAcl() !== null) { foreach ($board->getAcl() as $acl) { diff --git a/tests/integration/base-query-count.txt b/tests/integration/base-query-count.txt new file mode 100644 index 000000000..4fe36e923 --- /dev/null +++ b/tests/integration/base-query-count.txt @@ -0,0 +1 @@ +68024 From f9a76508a63594e59cb51f0bab22b9cd43ef2977 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Tue, 9 Jan 2024 18:11:57 +0100 Subject: [PATCH 4/5] ci: Bump setup-php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- .github/workflows/phpunit.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml index 0cfda0dd1..ebf850300 100644 --- a/.github/workflows/phpunit.yml +++ b/.github/workflows/phpunit.yml @@ -62,7 +62,7 @@ jobs: path: apps/${{ env.APP_NAME }} - name: Set up php ${{ matrix.php-versions }} - uses: shivammathur/setup-php@2.15.0 + uses: shivammathur/setup-php@2.24.0 with: php-version: ${{ matrix.php-versions }} tools: phpunit From eac673c1299b8c66b76294002f6b6703547ce707 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Tue, 9 Jan 2024 22:14:41 +0100 Subject: [PATCH 5/5] ci: Update actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- .github/workflows/appbuild.yml | 39 ---------------------------------- .github/workflows/phpunit.yml | 9 ++++---- 2 files changed, 4 insertions(+), 44 deletions(-) delete mode 100644 .github/workflows/appbuild.yml diff --git a/.github/workflows/appbuild.yml b/.github/workflows/appbuild.yml deleted file mode 100644 index de98779e4..000000000 --- a/.github/workflows/appbuild.yml +++ /dev/null @@ -1,39 +0,0 @@ -name: Package build - -on: - pull_request: - -jobs: - build: - runs-on: ubuntu-18.04 - - strategy: - matrix: - node-version: [14.x] - - steps: - - uses: actions/checkout@v2.4.0 - - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v2.4.1 - with: - node-version: ${{ matrix.node-version }} - - name: Set up npm7 - run: npm i -g npm@7 - - name: Setup PHP - uses: shivammathur/setup-php@2.15.0 - with: - php-version: '7.4' - tools: composer - - name: install dependencies - run: | - wget https://github.com/ChristophWurst/krankerl/releases/download/v0.12.2/krankerl_0.12.2_amd64.deb - sudo dpkg -i krankerl_0.12.2_amd64.deb - - name: package - run: | - uname -a - RUST_BACKTRACE=1 krankerl --version - RUST_BACKTRACE=1 krankerl package - - uses: actions/upload-artifact@v2 - with: - name: Deck app tarball - path: build/artifacts/deck.tar.gz diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml index ebf850300..48c5ee220 100644 --- a/.github/workflows/phpunit.yml +++ b/.github/workflows/phpunit.yml @@ -13,7 +13,7 @@ env: jobs: integration: - runs-on: ubuntu-18.04 + runs-on: ubuntu-latest strategy: fail-fast: false @@ -44,7 +44,7 @@ jobs: steps: - name: Checkout server - uses: actions/checkout@v2.4.0 + uses: actions/checkout@v3 with: repository: nextcloud/server ref: ${{ matrix.server-versions }} @@ -57,17 +57,16 @@ jobs: git -c "http.extraheader=$auth_header" -c protocol.version=2 submodule update --init --force --recursive --depth=1 - name: Checkout app - uses: actions/checkout@v2.4.0 + uses: actions/checkout@v3 with: path: apps/${{ env.APP_NAME }} - name: Set up php ${{ matrix.php-versions }} - uses: shivammathur/setup-php@2.24.0 + uses: shivammathur/setup-php@2.18.0 with: php-version: ${{ matrix.php-versions }} tools: phpunit extensions: zip, gd, mbstring, iconv, fileinfo, intl, sqlite, pdo_sqlite, mysql, pdo_mysql, pgsql, pdo_pgsql - ini-file: development coverage: none - name: Set up PHPUnit