[stable5.2] fix: require explicit share permission for bulk submission endpoints (#3292)

fix: require explicit share permission for bulk submission endpoints (#3290)

* fix: require explicit share permission for bulk submission actions

Add permission guard to deleteAllSubmissions() to verify the user
holds the required share-level permission, matching the pattern
already used by the single-item endpoint.
-e


* PR feedback
-e


---------

Signed-off-by: Peter Ringelmann <peter.ringelmann@nextcloud.com>

Author: pringelmann

lib/Controller/ApiController.php

@@ -1316,6 +1316,14 @@ public function getSubmission(int $formId, int $submissionId): DataResponse|Data
 	public function deleteAllSubmissions(int $formId): DataResponse {
 		$form = $this->formsService->getFormIfAllowed($formId, Constants::PERMISSION_RESULTS_DELETE);
 
+		// Require explicit results_delete permission for bulk deletion.
+		// canDeleteResults() also returns true for submitters with allowEditSubmissions,
+		// but those users should only be able to delete their own submissions individually.
+		if (!in_array(Constants::PERMISSION_RESULTS_DELETE, $this->formsService->getPermissions($form))) {
+			$this->logger->debug('User lacks results_delete permission for bulk deletion');
+			throw new OCSForbiddenException('No permission to delete all submissions');
+		}
+
 		// Delete all submissions (incl. Answers)
 		$this->submissionMapper->deleteByForm($formId);
 		$this->formMapper->update($form);

tests/Unit/Controller/ApiControllerTest.php

@@ -1030,6 +1030,124 @@ public function dataTestDeletePermission() {
 		];
 	}
 
+	/**
+	 * Test that a submitter with allowEditSubmissions but without
+	 * PERMISSION_RESULTS_DELETE cannot bulk-delete all submissions.
+	 */
+	public function testDeleteAllSubmissionsNoPermission(): void {
+		$form = Form::fromParams([
+			'id' => 1,
+			'title' => 'Test Form',
+			'hash' => 'hash',
+			'access' => [
+				'permitAllUsers' => false,
+				'showToAllUsers' => false,
+			],
+			'ownerId' => 'otherUser',
+			'description' => '',
+			'expires' => 0,
+			'isAnonymous' => false,
+			'submitMultiple' => false,
+			'showExpiration' => false,
+		]);
+
+		// getFormIfAllowed passes (canDeleteResults returns true due to allowEditSubmissions)
+		$this->formsService
+			->method('getFormIfAllowed')
+			->with(1, Constants::PERMISSION_RESULTS_DELETE)
+			->willReturn($form);
+
+		// But user only has submit permission, not results_delete
+		$this->formsService
+			->method('getPermissions')
+			->with($form)
+			->willReturn([Constants::PERMISSION_SUBMIT]);
+
+		// Bulk delete must NOT be called
+		$this->submissionMapper
+			->expects($this->never())
+			->method('deleteByForm');
+
+		$this->expectException(OCSForbiddenException::class);
+		$this->apiController->deleteAllSubmissions(1);
+	}
+
+	/**
+	 * Test that the form owner can bulk-delete all submissions.
+	 */
+	public function testDeleteAllSubmissionsAsOwner(): void {
+		$form = Form::fromParams([
+			'id' => 1,
+			'title' => 'Test Form',
+			'hash' => 'hash',
+			'access' => [
+				'permitAllUsers' => false,
+				'showToAllUsers' => false,
+			],
+			'ownerId' => 'currentUser',
+			'description' => '',
+			'expires' => 0,
+			'isAnonymous' => false,
+			'submitMultiple' => false,
+			'showExpiration' => false,
+		]);
+
+		$this->formsService
+			->method('getFormIfAllowed')
+			->with(1, Constants::PERMISSION_RESULTS_DELETE)
+			->willReturn($form);
+
+		$this->formsService
+			->method('getPermissions')
+			->with($form)
+			->willReturn(Constants::PERMISSION_ALL);
+
+		$this->submissionMapper
+			->expects($this->once())
+			->method('deleteByForm')
+			->with(1);
+
+		$this->assertEquals(new DataResponse(1), $this->apiController->deleteAllSubmissions(1));
+	}
+
+	/**
+	 * Test that a collaborator with PERMISSION_RESULTS_DELETE can bulk-delete.
+	 */
+	public function testDeleteAllSubmissionsWithResultsDeletePermission(): void {
+		$form = Form::fromParams([
+			'id' => 1,
+			'title' => 'Test Form',
+			'hash' => 'hash',
+			'access' => [
+				'permitAllUsers' => false,
+				'showToAllUsers' => false,
+			],
+			'ownerId' => 'otherUser',
+			'description' => '',
+			'expires' => 0,
+			'isAnonymous' => false,
+			'submitMultiple' => false,
+			'showExpiration' => false,
+		]);
+
+		$this->formsService
+			->method('getFormIfAllowed')
+			->with(1, Constants::PERMISSION_RESULTS_DELETE)
+			->willReturn($form);
+
+		$this->formsService
+			->method('getPermissions')
+			->with($form)
+			->willReturn([Constants::PERMISSION_RESULTS_DELETE, Constants::PERMISSION_SUBMIT]);
+
+		$this->submissionMapper
+			->expects($this->once())
+			->method('deleteByForm')
+			->with(1);
+
+		$this->assertEquals(new DataResponse(1), $this->apiController->deleteAllSubmissions(1));
+	}
+
 	public function testTransferOwnerNotOwner() {
 		$form = new Form();
 		$form->setId(1);