Merge pull request #3786 from nextcloud/backport/3785/stable5.4

[stable5.4] fix: respondent check

Author: samin-z

lib/Controller/ApiController.php

@@ -1519,7 +1519,6 @@ public function updateSubmission(int $formId, int $submissionId, array $answers)
 			throw new OCSBadRequestException($e->getMessage());
 		}
 
-		// get existing submission of this user
 		try {
 			$submission = $this->submissionMapper->findById($submissionId);
 		} catch (DoesNotExistException) {
@@ -1530,6 +1529,13 @@ public function updateSubmission(int $formId, int $submissionId, array $answers)
 			throw new OCSBadRequestException('Submission doesn\'t belong to given form');
 		}
 
+		if (
+			!in_array(Constants::PERMISSION_RESULTS_DELETE, $this->formsService->getPermissions($form))
+			&& $this->currentUser->getUID() !== $submission->getUserId()
+		) {
+			throw new OCSForbiddenException('You can only update your own submission!');
+		}
+
 		$submission->setTimestamp(time());
 		$this->submissionMapper->update($submission);
 

tests/Unit/Controller/ApiControllerTest.php

@@ -1673,6 +1673,11 @@ public function testUpdateSubmission_success() {
 			->with($form)
 			->willReturn(true);
 
+		$this->formsService->expects($this->once())
+			->method('getPermissions')
+			->with($form)
+			->willReturn([]);
+
 		$this->formsService->expects($this->once())
 			->method('getQuestions')
 			->with($formId)
@@ -1883,6 +1888,11 @@ public function testUpdateSubmission_canEditOthers_withDeletePermission() {
 			->with($form)
 			->willReturn(true);
 
+		$this->formsService->expects($this->once())
+			->method('getPermissions')
+			->with($form)
+			->willReturn([Constants::PERMISSION_RESULTS_DELETE]);
+
 		$this->formsService->expects($this->once())
 			->method('getQuestions')
 			->with($formId)
@@ -2050,6 +2060,11 @@ public function testUpdateSubmission_ownSubmissionWithAllowEditSubmissions() {
 			->with($form)
 			->willReturn(true);
 
+		$this->formsService->expects($this->once())
+			->method('getPermissions')
+			->with($form)
+			->willReturn([]);
+
 		$this->formsService->expects($this->once())
 			->method('getQuestions')
 			->with($formId)
@@ -2116,6 +2131,60 @@ public function testUpdateSubmission_ownSubmissionWithoutAllowEditSubmissions()
 		$this->apiController->updateSubmission($formId, $submissionId, $answers);
 	}
 
+	public function testUpdateSubmission_otherUsersSubmissionDeniedWhenAllowEditSubmissions() {
+		$formId = 1;
+		$submissionId = 42;
+		$answers = ['q1' => ['answer1']];
+
+		$form = new Form();
+		$form->setId($formId);
+		$form->setOwnerId('formOwner');
+		$form->setAllowEditSubmissions(true);
+
+		$submission = new Submission();
+		$submission->setId($submissionId);
+		$submission->setFormId($formId);
+		$submission->setUserId('anotherUser');
+
+		$this->formsService->expects($this->once())
+			->method('loadFormForSubmission')
+			->with(1)
+			->willReturn($form);
+
+		$this->formsService->expects($this->once())
+			->method('canDeleteResults')
+			->with($form)
+			->willReturn(true);
+
+		$this->formsService->expects($this->once())
+			->method('getPermissions')
+			->with($form)
+			->willReturn([Constants::PERMISSION_SUBMIT]);
+
+		$this->formsService->expects($this->once())
+			->method('getQuestions')
+			->with($formId)
+			->willReturn([['id' => 'q1', 'type' => Constants::ANSWER_TYPE_SHORT, 'options' => []]]);
+
+		$this->submissionService->expects($this->once())
+			->method('validateSubmission');
+
+		$this->submissionMapper->expects($this->once())
+			->method('findById')
+			->with($submissionId)
+			->willReturn($submission);
+
+		$this->submissionMapper->expects($this->never())
+			->method('update');
+
+		$this->answerMapper->expects($this->never())
+			->method('deleteBySubmission');
+
+		$this->expectException(OCSForbiddenException::class);
+		$this->expectExceptionMessage('You can only update your own submission!');
+		$this->apiController->updateSubmission($formId, $submissionId, $answers);
+	}
+
 	public function testUpdateSubmission_otherSubmissionWithoutPermission() {
 		// Test that user without RESULTS_DELETE cannot edit others' submissions even if allowEditSubmissions=true
 		$formId = 1;
@@ -2224,6 +2293,11 @@ public function testUpdateSubmission_formOwnerCanEditOthers() {
 			->with($form)
 			->willReturn(true);
 
+		$this->formsService->expects($this->once())
+			->method('getPermissions')
+			->with($form)
+			->willReturn([Constants::PERMISSION_RESULTS_DELETE]);
+
 		$this->formsService->expects($this->once())
 			->method('getQuestions')
 			->with($formId)
@@ -2285,6 +2359,11 @@ public function testUpdateSubmission_resultsDeleteShareCanEditOthers() {
 			->with($form)
 			->willReturn(true);
 
+		$this->formsService->expects($this->once())
+			->method('getPermissions')
+			->with($form)
+			->willReturn([Constants::PERMISSION_RESULTS_DELETE]);
+
 		$this->formsService->expects($this->once())
 			->method('getQuestions')
 			->with($formId)