From d23e0d74363ccc77dd25c7c72d8d0f19c55f313b Mon Sep 17 00:00:00 2001
From: Ferdinand Thiessen <opensource@fthiessen.de>
Date: Tue, 28 Nov 2023 22:23:43 +0100
Subject: [PATCH] enh: Allow to set results delete permission on the frontend

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
---
 lib/Controller/ApiController.php              |   5 +-
 lib/Controller/ShareApiController.php         |   5 +
 lib/Service/FormsService.php                  |  12 ++-
 .../SidebarTabs/SharingShareDiv.vue           |  17 +++
 src/views/Results.vue                         |   1 +
 tests/Unit/Controller/ApiControllerTest.php   | 101 ++++++++++++++++++
 .../Controller/ShareApiControllerTest.php     |  46 ++++++++
 tests/Unit/Service/FormsServiceTest.php       |  67 ++++++++++++
 8 files changed, 251 insertions(+), 3 deletions(-)

diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php
index 7e4c6cfc0..d5abc9ed6 100644
--- a/lib/Controller/ApiController.php
+++ b/lib/Controller/ApiController.php
@@ -1016,8 +1016,9 @@ public function deleteSubmission(int $id): DataResponse {
 			throw new OCSBadRequestException();
 		}
 
-		if ($form->getOwnerId() !== $this->currentUser->getUID()) {
-			$this->logger->debug('This form is not owned by the current user');
+		// The current user has permissions to remove submissions
+		if (!$this->formsService->canDeleteResults($form)) {
+			$this->logger->debug('This form is not owned by the current user and user has no `results_delete` permission');
 			throw new OCSForbiddenException();
 		}
 
diff --git a/lib/Controller/ShareApiController.php b/lib/Controller/ShareApiController.php
index 75924e4e1..29fffefc0 100644
--- a/lib/Controller/ShareApiController.php
+++ b/lib/Controller/ShareApiController.php
@@ -313,6 +313,11 @@ protected function validatePermissions(array $permissions, int $shareType): bool
 			return false;
 		}
 
+		if (in_array(Constants::PERMISSION_RESULTS_DELETE, $sanitizedPermissions) && !in_array(Constants::PERMISSION_RESULTS, $sanitizedPermissions)) {
+			$this->logger->debug('Permission results_delete is only allowed when permission results is also set');
+			return false;
+		}
+
 		// Make sure only users can have special permissions
 		if (count($sanitizedPermissions) > 1) {
 			switch ($shareType) {
diff --git a/lib/Service/FormsService.php b/lib/Service/FormsService.php
index 8a0a8e2ef..0af552978 100644
--- a/lib/Service/FormsService.php
+++ b/lib/Service/FormsService.php
@@ -252,6 +252,16 @@ public function canSeeResults(Form $form): bool {
 		return in_array(Constants::PERMISSION_RESULTS, $this->getPermissions($form));
 	}
 
+	/**
+	 * Can the current user delete results of a form
+	 *
+	 * @param Form $form
+	 * @return boolean
+	 */
+	public function canDeleteResults(Form $form): bool {
+		return in_array(Constants::PERMISSION_RESULTS_DELETE, $this->getPermissions($form));
+	}
+
 	/**
 	 * Can the user submit a form
 	 *
@@ -475,7 +485,7 @@ public function notifyNewSubmission(Form $form, string $submitter): void {
 	 *
 	 * @param int $formId The form to query shares for
 	 * @param string $userId The user to check if shared with
-	 * @return array
+	 * @return Share[]
 	 */
 	protected function getSharesWithUser(int $formId, string $userId): array {
 		$shareEntities = $this->shareMapper->findByForm($formId);
diff --git a/src/components/SidebarTabs/SharingShareDiv.vue b/src/components/SidebarTabs/SharingShareDiv.vue
index 462432c12..32795e70c 100644
--- a/src/components/SidebarTabs/SharingShareDiv.vue
+++ b/src/components/SidebarTabs/SharingShareDiv.vue
@@ -35,6 +35,9 @@
 			<NcActionCheckbox :checked="canAccessResults" @update:checked="updatePermissionResults">
 				{{ t('forms', 'View responses') }}
 			</NcActionCheckbox>
+			<NcActionCheckbox :checked="canDeleteResults" :disabled="!canAccessResults" @update:checked="updatePermissionDeleteResults">
+				{{ t('forms', 'Delete responses') }}
+			</NcActionCheckbox>
 			<NcActionSeparator />
 			<NcActionButton @click="removeShare">
 				<template #icon>
@@ -82,6 +85,9 @@ export default {
 		canAccessResults() {
 			return this.share.permissions.includes(this.PERMISSION_TYPES.PERMISSION_RESULTS)
 		},
+		canDeleteResults() {
+			return this.share.permissions.includes(this.PERMISSION_TYPES.PERMISSION_RESULTS_DELETE)
+		},
 		isNoUser() {
 			return this.share.shareType !== this.SHARE_TYPES.SHARE_TYPE_USER
 		},
@@ -109,9 +115,20 @@ export default {
 		 * @param {boolean} hasPermission If the results permission should be granted
 		 */
 		updatePermissionResults(hasPermission) {
+			if (hasPermission === false) {
+				// ensure to remove the delete permission if results permission is dropped
+				this.updatePermission(this.PERMISSION_TYPES.PERMISSION_RESULTS_DELETE, false)
+			}
 			return this.updatePermission(this.PERMISSION_TYPES.PERMISSION_RESULTS, hasPermission)
 		},
 
+		/**
+		 * @param {boolean} hasPermission If the results_delete permission should be granted
+		 */
+		updatePermissionDeleteResults(hasPermission) {
+			return this.updatePermission(this.PERMISSION_TYPES.PERMISSION_RESULTS_DELETE, hasPermission)
+		},
+
 		/**
 		 * Grant or remove permission from share
 		 *
diff --git a/src/views/Results.vue b/src/views/Results.vue
index 49a32ab7d..71204990d 100644
--- a/src/views/Results.vue
+++ b/src/views/Results.vue
@@ -275,6 +275,7 @@ export default {
 
 			try {
 				await axios.delete(generateOcsUrl('apps/forms/api/v2.2/submission/{id}', { id }))
+				showSuccess(t('forms', 'Submission deleted'))
 				const index = this.form.submissions.findIndex(search => search.id === id)
 				this.form.submissions.splice(index, 1)
 				emit('forms:last-updated:set', this.form.id)
diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php
index 5c3c6bd6f..4a02e76e5 100644
--- a/tests/Unit/Controller/ApiControllerTest.php
+++ b/tests/Unit/Controller/ApiControllerTest.php
@@ -53,6 +53,7 @@ function time($expected = null) {
 use OCA\Forms\Db\OptionMapper;
 use OCA\Forms\Db\QuestionMapper;
 use OCA\Forms\Db\ShareMapper;
+use OCA\Forms\Db\Submission;
 use OCA\Forms\Db\SubmissionMapper;
 use OCA\Forms\Service\ConfigService;
 use OCA\Forms\Service\FormsService;
@@ -726,4 +727,104 @@ public function testInsertSubmission_validateSubmission() {
 		$this->apiController->insertSubmission(1, [], '');
 	}
 
+	public function testDeleteSubmissionNotFound() {
+		$exception = $this->createMock(MapperException::class);
+
+		$this->submissionMapper
+			->expects($this->once())
+			->method('findById')
+			->with(42)
+			->willThrowException($exception);
+
+		$this->expectException(OCSBadRequestException::class);
+		$this->apiController->deleteSubmission(42);
+	}
+
+	/**
+	 * @dataProvider dataTestDeletePermission
+	 */
+	public function testDeleteSubmissionNoPermission($submissionData, $formData) {
+		$submission = Submission::fromParams($submissionData);
+		$form = Form::fromParams($formData);
+
+		$this->submissionMapper
+			->method('findById')
+			->with(42)
+			->willReturn($submission);
+
+		$this->formMapper
+			->method('findById')
+			->with(1)
+			->willReturn($form);
+
+		$this->formsService
+			->expects($this->once())
+			->method('canDeleteResults')
+			->with($form)
+			->willReturn(false);
+
+		$this->expectException(OCSForbiddenException::class);
+		$this->apiController->deleteSubmission(42);
+	}
+
+	/**
+	 * @dataProvider dataTestDeletePermission
+	 */
+	public function testDeleteSubmission($submissionData, $formData) {
+		$submission = Submission::fromParams($submissionData);
+		$form = Form::fromParams($formData);
+
+		$this->submissionMapper
+			->method('findById')
+			->with(42)
+			->willReturn($submission);
+
+		$this->formMapper
+			->method('findById')
+			->with(1)
+			->willReturn($form);
+
+		$this->formsService
+			->expects($this->once())
+			->method('canDeleteResults')
+			->with($form)
+			->willReturn(true);
+
+		$this->submissionMapper
+			->expects($this->once())
+			->method('deleteById')
+			->with(42);
+
+		$this->formsService
+			->expects($this->once())
+			->method('setLastUpdatedTimestamp')
+			->with($formData['id']);
+
+		$this->assertEquals(new DataResponse(42), $this->apiController->deleteSubmission(42));
+	}
+
+	public function dataTestDeletePermission() {
+		return [
+			[
+				[
+					'formId' => 1,
+				],
+				[
+					'id' => 1,
+					'title' => 'Name',
+					'hash' => 'hash',
+					'access' => [
+						'permitAllUsers' => false,
+						'showToAllUsers' => false,
+					],
+					'ownerId' => 'currentUser',
+					'description' => '',
+					'expires' => 0,
+					'isAnonymous' => false,
+					'submitMultiple' => false,
+					'showExpiration' => false
+				],
+			]
+		];
+	}
 }
diff --git a/tests/Unit/Controller/ShareApiControllerTest.php b/tests/Unit/Controller/ShareApiControllerTest.php
index d235a612c..97fff8ad7 100644
--- a/tests/Unit/Controller/ShareApiControllerTest.php
+++ b/tests/Unit/Controller/ShareApiControllerTest.php
@@ -594,6 +594,21 @@ public function dataUpdateShare() {
 				'expected' => 1,
 				'exception' => null
 			],
+			'valid-permissions-share-delete' => [
+				'share' => [
+					'id' => 1,
+					'formId' => 5,
+					'shareType' => 0,
+					'shareWith' => 'user1',
+					'permissions' => [Constants::PERMISSION_SUBMIT]
+				],
+				'formOwner' => 'currentUser',
+				'keyValuePairs' => [
+					'permissions' => [Constants::PERMISSION_RESULTS, Constants::PERMISSION_RESULTS_DELETE, Constants::PERMISSION_SUBMIT]
+				],
+				'expected' => 1,
+				'exception' => null
+			],
 			'no-permission' => [
 				'share' => [
 					'id' => 1,
@@ -609,6 +624,37 @@ public function dataUpdateShare() {
 				'expected' => null,
 				'exception' => '\OCP\AppFramework\OCS\OCSBadRequestException'
 			],
+			'invalid-permission-missing-submit' => [
+				'share' => [
+					'id' => 1,
+					'formId' => 5,
+					'shareType' => 0,
+					'shareWith' => 'user1',
+					'permissions' => [Constants::PERMISSION_SUBMIT],
+				],
+				'formOwner' => 'currentUser',
+				'keyValuePairs' => [
+					'permissions' => [Constants::PERMISSION_RESULTS_DELETE],
+				],
+				'expected' => null,
+				'exception' => '\OCP\AppFramework\OCS\OCSBadRequestException'
+			],
+			// PERMISSION_RESULTS_DELETE is only allowed if PERMISSION_RESULTS is set
+			'invalid-permission-missing-results' => [
+				'share' => [
+					'id' => 1,
+					'formId' => 5,
+					'shareType' => 0,
+					'shareWith' => 'user1',
+					'permissions' => [Constants::PERMISSION_SUBMIT],
+				],
+				'formOwner' => 'currentUser',
+				'keyValuePairs' => [
+					'permissions' => [Constants::PERMISSION_SUBMIT, Constants::PERMISSION_RESULTS_DELETE],
+				],
+				'expected' => null,
+				'exception' => '\OCP\AppFramework\OCS\OCSBadRequestException'
+			],
 			'invalid-permission' => [
 				'share' => [
 					'id' => 1,
diff --git a/tests/Unit/Service/FormsServiceTest.php b/tests/Unit/Service/FormsServiceTest.php
index b03841cc4..1f62e4bd1 100644
--- a/tests/Unit/Service/FormsServiceTest.php
+++ b/tests/Unit/Service/FormsServiceTest.php
@@ -666,6 +666,73 @@ public function testCanSeeResults(string $ownerId, array $sharesArray, bool $exp
 		$this->assertEquals($expected, $this->formsService->canSeeResults($form));
 	}
 
+	public function dataCanDeleteResults() {
+		return [
+			'allowFormOwner' => [
+				'ownerId' => 'currentUser',
+				'sharesArray' => [],
+				'expected' => true
+			],
+			'allowShared' => [
+				'ownerId' => 'someUser',
+				'sharesArray' => [[
+					'with' => 'currentUser',
+					'type' => 0,
+					'permissions' => [Constants::PERMISSION_SUBMIT, Constants::PERMISSION_RESULTS, Constants::PERMISSION_RESULTS_DELETE],
+				]],
+				'expected' => true
+			],
+			'disallowNotowned' => [
+				'ownerId' => 'someUser',
+				'sharesArray' => [],
+				'expected' => false
+			],
+			'allowNotShared' => [
+				'ownerId' => 'someUser',
+				'sharesArray' => [[
+					'with' => 'currentUser',
+					'type' => 0,
+					'permissions' => [Constants::PERMISSION_SUBMIT, Constants::PERMISSION_RESULTS],
+				]],
+				'expected' => false
+			]
+		];
+	}
+	/**
+	 * @dataProvider dataCanDeleteResults
+	 *
+	 * @param string $ownerId
+	 * @param array $sharesArray
+	 * @param bool $expected
+	 */
+	public function testCanDeleteResults(string $ownerId, array $sharesArray, bool $expected) {
+		$form = new Form();
+		$form->setId(42);
+		$form->setOwnerId($ownerId);
+		$form->setAccess([
+			'permitAllUsers' => false,
+			'showToAllUsers' => false,
+		]);
+
+		$shares = [];
+		foreach ($sharesArray as $id => $share) {
+			$shareEntity = new Share();
+			$shareEntity->setId($id);
+			$shareEntity->setFormId(42);
+			$shareEntity->setShareType($share['type']);
+			$shareEntity->setShareWith($share['with']);
+			$shareEntity->setPermissions($share['permissions']);
+			$shares[] = $shareEntity;
+		}
+
+		$this->shareMapper->expects($this->any())
+			->method('findByForm')
+			->with(42)
+			->willReturn($shares);
+
+		$this->assertEquals($expected, $this->formsService->canDeleteResults($form));
+	}
+
 	public function dataCanSubmit() {
 		return [
 			'allowFormOwner' => [