From f2649e5fa64a09e10333b1dd0290558bbe93677d Mon Sep 17 00:00:00 2001 From: Christian Hartmann Date: Thu, 2 Jul 2026 00:35:44 +0200 Subject: [PATCH 1/2] feat: allow users with RESULTS_DELETE permission to edit all responses Signed-off-by: Christian Hartmann --- lib/Controller/ApiController.php | 8 ++------ lib/Controller/PageController.php | 5 ++++- src/views/Results.vue | 8 ++++++-- src/views/Submit.vue | 11 +++++++++-- 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 09a40d241..98b5aadd4 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -1517,8 +1517,8 @@ public function updateSubmission(int $formId, int $submissionId, array $answers) // submissions can't be updated on public shares, so passing empty shareHash $form = $this->formsService->loadFormForSubmission($formId, ''); - if (!$form->getAllowEditSubmissions()) { - throw new OCSBadRequestException('Can only update if allowEditSubmissions is set'); + if (!$this->formsService->canDeleteResults($form)) { + throw new OCSForbiddenException('You\'re not allowed to edit this submission'); } $questions = $this->formsService->getQuestions($formId); @@ -1540,10 +1540,6 @@ public function updateSubmission(int $formId, int $submissionId, array $answers) throw new OCSBadRequestException('Submission doesn\'t belong to given form'); } - if ($this->currentUser->getUID() !== $submission->getUserId()) { - throw new OCSForbiddenException('Can only update your own submissions'); - } - $submission->setTimestamp(time()); $this->submissionMapper->update($submission); diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index b378c7354..ff0ed6c71 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -119,7 +119,10 @@ public function views(string $hash): TemplateResponse { #[NoCSRFRequired()] #[FrontpageRoute(verb: 'GET', url: '/{hash}/submit/{submissionId}', requirements: ['hash' => '[a-zA-Z0-9]{16,}', 'submissionId' => '\d+'])] public function submitViewWithSubmission(string $hash, int $submissionId): TemplateResponse { - return $this->formMapper->findByHash($hash)->getAllowEditSubmissions() ? $this->index($hash, $submissionId) : $this->index($hash); + $form = $this->formMapper->findByHash($hash); + $canUserEditSubmission = $this->formsService->canDeleteResults($form); + + return $canUserEditSubmission ? $this->index($hash, $submissionId) : $this->index($hash); } /** diff --git a/src/views/Results.vue b/src/views/Results.vue index cfc61775a..1804e6547 100644 --- a/src/views/Results.vue +++ b/src/views/Results.vue @@ -746,11 +746,15 @@ export default { * * @param {string} submissionUser - The ID of the user who created the submission. * @return {boolean} - Returns true if the submission can be edited, otherwise false. + * A submission can be edited if: + * - The user has the `canDeleteSubmissions` permission, or + * - The form allows editing (`form.allowEditSubmissions`) and the current user is the owner of the submission. */ canEditSubmission(submissionUser) { return ( - this.form.allowEditSubmissions - && getCurrentUser().uid === submissionUser + this.canDeleteSubmissions + || (this.form.allowEditSubmissions + && getCurrentUser().uid === submissionUser) ) }, diff --git a/src/views/Submit.vue b/src/views/Submit.vue index 2d071caec..dd625bf5a 100644 --- a/src/views/Submit.vue +++ b/src/views/Submit.vue @@ -239,6 +239,7 @@ import QuestionLong from '../components/Questions/QuestionLong.vue' import QuestionMultiple from '../components/Questions/QuestionMultiple.vue' import QuestionShort from '../components/Questions/QuestionShort.vue' import TopBar from '../components/TopBar.vue' +import PermissionTypes from '../mixins/PermissionTypes.js' import ViewsMixin from '../mixins/ViewsMixin.js' import answerTypes from '../models/AnswerTypes.js' import { @@ -268,7 +269,7 @@ export default { TopBar, }, - mixins: [ViewsMixin], + mixins: [PermissionTypes, ViewsMixin], /* * This is used to confirm that the user wants to leave the page @@ -551,7 +552,13 @@ export default { } if (this.isLoggedIn) { - if (this.submissionId && this.form.allowEditSubmissions) { + if ( + this.submissionId + && (this.form.allowEditSubmissions + || this.form.permissions.includes( + this.PERMISSION_TYPES.PERMISSION_RESULTS_DELETE, + )) + ) { this.fetchSubmission() } else { this.initFromLocalStorage() From 2698aabe62c589f2cc5351fef568d84e2cb60b24 Mon Sep 17 00:00:00 2001 From: Christian Hartmann Date: Fri, 3 Jul 2026 00:28:16 +0200 Subject: [PATCH 2/2] add tests Signed-off-by: Christian Hartmann --- tests/Integration/Api/ApiV3Test.php | 112 +++++- tests/Unit/Controller/ApiControllerTest.php | 423 +++++++++++++++++++- tests/Unit/Service/FormsServiceTest.php | 84 ++++ 3 files changed, 604 insertions(+), 15 deletions(-) diff --git a/tests/Integration/Api/ApiV3Test.php b/tests/Integration/Api/ApiV3Test.php index 36e98f028..45a15d532 100644 --- a/tests/Integration/Api/ApiV3Test.php +++ b/tests/Integration/Api/ApiV3Test.php @@ -193,10 +193,22 @@ private function setTestForms() { 'shares' => [ [ 'shareType' => 0, - 'shareWith' => 'user2', + 'shareWith' => 'test', + 'permissions' => ['results', 'results_delete'], ], ], - 'submissions' => [] + 'submissions' => [ + [ + 'userId' => 'someUser', + 'timestamp' => 123456, + 'answers' => [ + [ + 'questionIndex' => 0, + 'text' => 'Answer from form owner' + ] + ] + ], + ] ], [ 'hash' => 'zyxwvutsrq654321', @@ -342,11 +354,13 @@ public static function dataGetSharedForms() { 'state' => 0, 'lastUpdated' => 123456789, 'permissions' => [ - 'submit' + 'results', + 'results_delete', ], 'partial' => true, 'lockedBy' => null, 'lockedUntil' => null, + 'submissionCount' => 1, ], ] ] @@ -1613,4 +1627,96 @@ public function testTransferOwner() { $this->assertEquals(200, $resp->getStatusCode()); $this->assertEquals('user1', $data); } + + /** + * Test editing submissions with RESULTS_DELETE permission + */ + public function testUpdateSubmission_formOwnerEditsOthersSubmission() { + // Test that a user with RESULTS_DELETE permission (but not owner) can edit another user's submission + // Form 1 is owned by 'someUser' but shared with 'test' (the auth user) with RESULTS_DELETE permission + $formId = $this->testForms[1]['id']; + $submissionId = $this->testForms[1]['submissions'][0]['id']; + + // Update the submission as 'test' who has RESULTS_DELETE permission via share + $resp = $this->http->request('PUT', "api/v3/forms/{$formId}/submissions/{$submissionId}", [ + 'json' => [ + 'answers' => [ + $this->testForms[1]['questions'][0]['id'] => ['Edited by RESULTS_DELETE user'], + ] + ] + ]); + + // Should succeed - 'test' has RESULTS_DELETE permission via share + $this->assertEquals(200, $resp->getStatusCode()); + $data = $this->OcsResponse2Data($resp); + $this->assertEquals($submissionId, $data); + } + + /** + * Test that form owner can edit submissions when allowEditSubmissions is enabled + */ + public function testUpdateSubmission_userEditsOwnSubmission() { + // Form 0 is owned by 'test' - test form owner editing with allowEditSubmissions=true + $formId = $this->testForms[0]['id']; + + // Enable allowEditSubmissions on the form + $resp = $this->http->request('PATCH', "api/v3/forms/{$formId}", [ + 'json' => [ + 'keyValuePairs' => [ + 'allowEditSubmissions' => true, + ] + ] + ]); + $this->assertEquals(200, $resp->getStatusCode()); + + // Form owner should be able to edit submission with allowEditSubmissions=true + $submissionId = $this->testForms[0]['submissions'][0]['id']; + + $resp = $this->http->request('PUT', "api/v3/forms/{$formId}/submissions/{$submissionId}", [ + 'json' => [ + 'answers' => [ + $this->testForms[0]['questions'][0]['id'] => ['Edited with allowEditSubmissions=true'], + ] + ] + ]); + + // Should succeed - form owner always has permission + $this->assertEquals(200, $resp->getStatusCode()); + $data = $this->OcsResponse2Data($resp); + $this->assertEquals($submissionId, $data); + } + + /** + * Test that form owner can edit even when allowEditSubmissions is disabled + */ + public function testUpdateSubmission_userCannotEditWhenDisabled() { + // Form 0 is owned by 'test' - test form owner can always edit + $formId = $this->testForms[0]['id']; + + // Disable allowEditSubmissions on the form + $resp = $this->http->request('PATCH', "api/v3/forms/{$formId}", [ + 'json' => [ + 'keyValuePairs' => [ + 'allowEditSubmissions' => false, + ] + ] + ]); + $this->assertEquals(200, $resp->getStatusCode()); + + // Form owner should still be able to edit even with allowEditSubmissions=false + $submissionId = $this->testForms[0]['submissions'][0]['id']; + + $resp = $this->http->request('PUT', "api/v3/forms/{$formId}/submissions/{$submissionId}", [ + 'json' => [ + 'answers' => [ + $this->testForms[0]['questions'][0]['id'] => ['Owner always can edit'], + ] + ] + ]); + + // Should succeed - form owner always has permission regardless of flag + $this->assertEquals(200, $resp->getStatusCode()); + $data = $this->OcsResponse2Data($resp); + $this->assertEquals($submissionId, $data); + } }; diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php index 79d908675..28173dee3 100644 --- a/tests/Unit/Controller/ApiControllerTest.php +++ b/tests/Unit/Controller/ApiControllerTest.php @@ -1638,6 +1638,12 @@ public function testUpdateSubmission_success() { ->with(1) ->willReturn($form); + // Mock canDeleteResults to return true (form owner can edit) + $this->formsService->expects($this->once()) + ->method('canDeleteResults') + ->with($form) + ->willReturn(true); + $this->formsService->expects($this->once()) ->method('getQuestions') ->with($formId) @@ -1671,23 +1677,29 @@ public function testUpdateSubmission_success() { $this->assertEquals(new DataResponse($submissionId), $response); } - public function testUpdateSubmission_formNotEditable() { + public function testUpdateSubmission_noPermission() { $formId = 1; $submissionId = 42; $answers = ['q1' => ['answer1']]; $form = new Form(); $form->setId($formId); - $form->setOwnerId('formOwner'); - $form->setAllowEditSubmissions(false); // Form does not allow edits + $form->setOwnerId('otherUser'); + $form->setAllowEditSubmissions(false); $this->formsService->expects($this->once()) ->method('loadFormForSubmission') ->with(1) ->willReturn($form); - $this->expectException(OCSBadRequestException::class); - $this->expectExceptionMessage('Can only update if allowEditSubmissions is set'); + // Mock canDeleteResults to return false (user lacks permission) + $this->formsService->expects($this->once()) + ->method('canDeleteResults') + ->with($form) + ->willReturn(false); + + $this->expectException(OCSForbiddenException::class); + $this->expectExceptionMessage('You\'re not allowed to edit this submission'); $this->apiController->updateSubmission($formId, $submissionId, $answers); } @@ -1706,6 +1718,12 @@ public function testUpdateSubmission_invalidAnswers() { ->with(1) ->willReturn($form); + // Mock canDeleteResults to return true (pass permission check) + $this->formsService->expects($this->once()) + ->method('canDeleteResults') + ->with($form) + ->willReturn(true); + $this->formsService->expects($this->once()) ->method('getQuestions') ->with($formId) @@ -1735,6 +1753,12 @@ public function testUpdateSubmission_submissionNotFound() { ->with(1) ->willReturn($form); + // Mock canDeleteResults to return true (pass permission check) + $this->formsService->expects($this->once()) + ->method('canDeleteResults') + ->with($form) + ->willReturn(true); + $this->formsService->expects($this->once()) ->method('getQuestions') ->with($formId) @@ -1742,7 +1766,7 @@ public function testUpdateSubmission_submissionNotFound() { $this->submissionService->expects($this->once()) ->method('validateSubmission') - ->with($this->anything(), $answers, 'formOwner'); // Removed ->willReturn(true) + ->with($this->anything(), $answers, 'formOwner'); $this->submissionMapper->expects($this->once()) ->method('findById') @@ -1774,6 +1798,12 @@ public function testUpdateSubmission_mismatchedFormId() { ->with(1) ->willReturn($form); + // Mock canDeleteResults to return true (pass permission check) + $this->formsService->expects($this->once()) + ->method('canDeleteResults') + ->with($form) + ->willReturn(true); + $this->formsService->expects($this->once()) ->method('getQuestions') ->with($formId) @@ -1781,7 +1811,7 @@ public function testUpdateSubmission_mismatchedFormId() { $this->submissionService->expects($this->once()) ->method('validateSubmission') - ->with($this->anything(), $answers, 'formOwner'); // Removed ->willReturn(true) + ->with($this->anything(), $answers, 'formOwner'); $this->submissionMapper->expects($this->once()) ->method('findById') @@ -1793,7 +1823,8 @@ public function testUpdateSubmission_mismatchedFormId() { $this->apiController->updateSubmission($formId, $submissionId, $answers); } - public function testUpdateSubmission_notOwnSubmission() { + public function testUpdateSubmission_canEditOthers_withDeletePermission() { + // Test that users with RESULTS_DELETE permission can edit any submission $formId = 1; $submissionId = 42; $answers = ['q1' => ['answer1']]; @@ -1813,6 +1844,12 @@ public function testUpdateSubmission_notOwnSubmission() { ->with(1) ->willReturn($form); + // Current user has RESULTS_DELETE permission (e.g., via share or ownership) + $this->formsService->expects($this->once()) + ->method('canDeleteResults') + ->with($form) + ->willReturn(true); + $this->formsService->expects($this->once()) ->method('getQuestions') ->with($formId) @@ -1820,16 +1857,31 @@ public function testUpdateSubmission_notOwnSubmission() { $this->submissionService->expects($this->once()) ->method('validateSubmission') - ->with($this->anything(), $answers, 'formOwner'); // Removed ->willReturn(true) + ->with($this->anything(), $answers, 'formOwner'); $this->submissionMapper->expects($this->once()) ->method('findById') ->with($submissionId) ->willReturn($submission); - $this->expectException(OCSForbiddenException::class); - $this->expectExceptionMessage('Can only update your own submissions'); - $this->apiController->updateSubmission($formId, $submissionId, $answers); + $this->submissionMapper->expects($this->once()) + ->method('update') + ->with($submission); + + $this->answerMapper->expects($this->once()) + ->method('deleteBySubmission') + ->with($submissionId); + + $this->answerMapper->expects($this->once()) + ->method('insert'); + + $this->formsService->expects($this->once()) + ->method('notifyNewSubmission') + ->with($form, $submission); + + // Should succeed - user has RESULTS_DELETE permission + $response = $this->apiController->updateSubmission($formId, $submissionId, $answers); + $this->assertEquals(new DataResponse($submissionId), $response); } public function testUpdateSubmission_loadForm_notFound() { @@ -1882,4 +1934,351 @@ public function testUpdateSubmission_loadForm_formExpired() { $this->expectExceptionMessage('This form is no longer taking answers'); $this->apiController->updateSubmission($formId, $submissionId, $answers); } + + public function testUpdateSubmission_resultsPermissionOnlyDenied() { + // Test that user with RESULTS permission only (no RESULTS_DELETE) cannot edit others' submissions + $formId = 1; + $submissionId = 42; + $answers = ['q1' => ['answer1']]; + + $form = new Form(); + $form->setId($formId); + $form->setOwnerId('otherUser'); + $form->setAllowEditSubmissions(true); + + $this->formsService->expects($this->once()) + ->method('loadFormForSubmission') + ->with(1) + ->willReturn($form); + + // Mock canDeleteResults to return false (no RESULTS_DELETE share) + $this->formsService->expects($this->once()) + ->method('canDeleteResults') + ->with($form) + ->willReturn(false); + + $this->expectException(OCSForbiddenException::class); + $this->expectExceptionMessage('You\'re not allowed to edit this submission'); + $this->apiController->updateSubmission($formId, $submissionId, $answers); + } + + public function testUpdateSubmission_submitPermissionOnlyDenied() { + // Test that user with SUBMIT permission only cannot edit submissions + $formId = 1; + $submissionId = 42; + $answers = ['q1' => ['answer1']]; + + $form = new Form(); + $form->setId($formId); + $form->setOwnerId('otherUser'); + $form->setAllowEditSubmissions(false); + + $this->formsService->expects($this->once()) + ->method('loadFormForSubmission') + ->with(1) + ->willReturn($form); + + // Mock canDeleteResults to return false (only SUBMIT permission) + $this->formsService->expects($this->once()) + ->method('canDeleteResults') + ->with($form) + ->willReturn(false); + + $this->expectException(OCSForbiddenException::class); + $this->expectExceptionMessage('You\'re not allowed to edit this submission'); + $this->apiController->updateSubmission($formId, $submissionId, $answers); + } + + public function testUpdateSubmission_ownSubmissionWithAllowEditSubmissions() { + // Test backward compatibility: user can edit own submission when allowEditSubmissions=true + $formId = 1; + $submissionId = 42; + $answers = ['q1' => ['answer1']]; + $userId = 'currentUser'; + + $form = new Form(); + $form->setId($formId); + $form->setOwnerId('otherUser'); + $form->setAllowEditSubmissions(true); + + $submission = new Submission(); + $submission->setId($submissionId); + $submission->setFormId($formId); + $submission->setUserId($userId); + + $this->formsService->expects($this->once()) + ->method('loadFormForSubmission') + ->with(1) + ->willReturn($form); + + // canDeleteResults returns true because user has submitted and allowEditSubmissions=true + $this->formsService->expects($this->once()) + ->method('canDeleteResults') + ->with($form) + ->willReturn(true); + + $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') + ->with($this->anything(), $answers, 'otherUser'); + + $this->submissionMapper->expects($this->once()) + ->method('findById') + ->with($submissionId) + ->willReturn($submission); + + $this->submissionMapper->expects($this->once()) + ->method('update') + ->with($submission); + + $this->answerMapper->expects($this->once()) + ->method('deleteBySubmission') + ->with($submissionId); + + $this->answerMapper->expects($this->once()) + ->method('insert'); + + $this->formsService->expects($this->once()) + ->method('notifyNewSubmission') + ->with($form, $submission); + + // Should succeed - user is owner and allowEditSubmissions=true + $response = $this->apiController->updateSubmission($formId, $submissionId, $answers); + $this->assertEquals(new DataResponse($submissionId), $response); + } + + public function testUpdateSubmission_ownSubmissionWithoutAllowEditSubmissions() { + // Test that user cannot edit own submission if form disallows editing + $formId = 1; + $submissionId = 42; + $answers = ['q1' => ['answer1']]; + $userId = 'currentUser'; + + $form = new Form(); + $form->setId($formId); + $form->setOwnerId('otherUser'); + $form->setAllowEditSubmissions(false); + + $this->formsService->expects($this->once()) + ->method('loadFormForSubmission') + ->with(1) + ->willReturn($form); + + // Mock canDeleteResults to return false (allowEditSubmissions=false, no RESULTS_DELETE) + $this->formsService->expects($this->once()) + ->method('canDeleteResults') + ->with($form) + ->willReturn(false); + + $this->expectException(OCSForbiddenException::class); + $this->expectExceptionMessage('You\'re not allowed to edit this 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; + $submissionId = 42; + $answers = ['q1' => ['answer1']]; + + $form = new Form(); + $form->setId($formId); + $form->setOwnerId('otherUser'); + $form->setAllowEditSubmissions(true); + + $this->formsService->expects($this->once()) + ->method('loadFormForSubmission') + ->with(1) + ->willReturn($form); + + // Mock canDeleteResults to return false (not owner, no RESULTS_DELETE share) + $this->formsService->expects($this->once()) + ->method('canDeleteResults') + ->with($form) + ->willReturn(false); + + $this->expectException(OCSForbiddenException::class); + $this->expectExceptionMessage('You\'re not allowed to edit this submission'); + $this->apiController->updateSubmission($formId, $submissionId, $answers); + } + + public function testUpdateSubmission_archivedFormDenied() { + // Test that cannot edit submission on archived form (regardless of permission) + $formId = 1; + $submissionId = 42; + $answers = ['q1' => ['answer1']]; + + $form = new Form(); + $form->setId($formId); + $form->setOwnerId('currentUser'); + $form->setAllowEditSubmissions(true); + + $this->formsService->expects($this->once()) + ->method('loadFormForSubmission') + ->with(1) + ->willReturn($form); + + // Mock canDeleteResults to return false (form is archived) + $this->formsService->expects($this->once()) + ->method('canDeleteResults') + ->with($form) + ->willReturn(false); + + $this->expectException(OCSForbiddenException::class); + $this->expectExceptionMessage('You\'re not allowed to edit this submission'); + $this->apiController->updateSubmission($formId, $submissionId, $answers); + } + + public function testUpdateSubmission_anonymousUserDenied() { + // Test that unauthenticated user cannot edit submissions (even on public forms) + $formId = 1; + $submissionId = 42; + $answers = ['q1' => ['answer1']]; + + $form = new Form(); + $form->setId($formId); + $form->setOwnerId('someUser'); + $form->setAllowEditSubmissions(true); + + $this->formsService->expects($this->once()) + ->method('loadFormForSubmission') + ->with(1) + ->willReturn($form); + + // Mock canDeleteResults to return false (no permissions) + $this->formsService->expects($this->once()) + ->method('canDeleteResults') + ->with($form) + ->willReturn(false); + + $this->expectException(OCSForbiddenException::class); + $this->expectExceptionMessage('You\'re not allowed to edit this submission'); + $this->apiController->updateSubmission($formId, $submissionId, $answers); + } + + public function testUpdateSubmission_formOwnerCanEditOthers() { + // Test that form owner can edit any submission + $formId = 1; + $submissionId = 42; + $answers = ['q1' => ['answer1']]; + + $form = new Form(); + $form->setId($formId); + $form->setOwnerId('currentUser'); // Current user is form owner + $form->setAllowEditSubmissions(false); // Editing is not allowed for regular users + + $submission = new Submission(); + $submission->setId($submissionId); + $submission->setFormId($formId); + $submission->setUserId('otherUser'); // But submission is from another user + + $this->formsService->expects($this->once()) + ->method('loadFormForSubmission') + ->with(1) + ->willReturn($form); + + // Mock canDeleteResults to return true (form owner) + $this->formsService->expects($this->once()) + ->method('canDeleteResults') + ->with($form) + ->willReturn(true); + + $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') + ->with($this->anything(), $answers, 'currentUser'); + + $this->submissionMapper->expects($this->once()) + ->method('findById') + ->with($submissionId) + ->willReturn($submission); + + $this->submissionMapper->expects($this->once()) + ->method('update') + ->with($submission); + + $this->answerMapper->expects($this->once()) + ->method('deleteBySubmission') + ->with($submissionId); + + $this->answerMapper->expects($this->once()) + ->method('insert'); + + $this->formsService->expects($this->once()) + ->method('notifyNewSubmission') + ->with($form, $submission); + + // Should succeed - user is form owner + $response = $this->apiController->updateSubmission($formId, $submissionId, $answers); + $this->assertEquals(new DataResponse($submissionId), $response); + } + + public function testUpdateSubmission_resultsDeleteShareCanEditOthers() { + // Test that user with RESULTS_DELETE share permission can edit any submission + $formId = 1; + $submissionId = 42; + $answers = ['q1' => ['answer1']]; + + $form = new Form(); + $form->setId($formId); + $form->setOwnerId('formOwner'); + $form->setAllowEditSubmissions(false); // Editing not allowed for regular users + + $submission = new Submission(); + $submission->setId($submissionId); + $submission->setFormId($formId); + $submission->setUserId('submissionUser'); // Different from current user + + $this->formsService->expects($this->once()) + ->method('loadFormForSubmission') + ->with(1) + ->willReturn($form); + + // Mock canDeleteResults to return true (shared user with RESULTS_DELETE) + $this->formsService->expects($this->once()) + ->method('canDeleteResults') + ->with($form) + ->willReturn(true); + + $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') + ->with($this->anything(), $answers, 'formOwner'); + + $this->submissionMapper->expects($this->once()) + ->method('findById') + ->with($submissionId) + ->willReturn($submission); + + $this->submissionMapper->expects($this->once()) + ->method('update') + ->with($submission); + + $this->answerMapper->expects($this->once()) + ->method('deleteBySubmission') + ->with($submissionId); + + $this->answerMapper->expects($this->once()) + ->method('insert'); + + $this->formsService->expects($this->once()) + ->method('notifyNewSubmission') + ->with($form, $submission); + + // Should succeed - user has RESULTS_DELETE via share + $response = $this->apiController->updateSubmission($formId, $submissionId, $answers); + $this->assertEquals(new DataResponse($submissionId), $response); + } } diff --git a/tests/Unit/Service/FormsServiceTest.php b/tests/Unit/Service/FormsServiceTest.php index 3aa911ea3..d718c3790 100644 --- a/tests/Unit/Service/FormsServiceTest.php +++ b/tests/Unit/Service/FormsServiceTest.php @@ -755,6 +755,47 @@ public static function dataCanDeleteResults() { 'permissions' => [Constants::PERMISSION_SUBMIT, Constants::PERMISSION_RESULTS], ]], 'expected' => false + ], + 'disallowMultipleSharesMissingDelete' => [ + 'ownerId' => 'someUser', + 'sharesArray' => [ + [ + 'with' => 'currentUser', + 'type' => 0, + 'permissions' => [Constants::PERMISSION_SUBMIT, Constants::PERMISSION_RESULTS], + ], + [ + 'with' => 'otherUser', + 'type' => 0, + 'permissions' => [Constants::PERMISSION_RESULTS_DELETE], + ] + ], + 'expected' => false + ], + 'allowMultipleSharesWithDelete' => [ + 'ownerId' => 'someUser', + 'sharesArray' => [ + [ + 'with' => 'currentUser', + 'type' => 0, + 'permissions' => [Constants::PERMISSION_SUBMIT], + ], + [ + 'with' => 'currentUser', + 'type' => 0, + 'permissions' => [Constants::PERMISSION_RESULTS_DELETE], + ] + ], + 'expected' => true + ], + 'disallowResultsDeleteOnlyWithoutOthers' => [ + 'ownerId' => 'someUser', + 'sharesArray' => [[ + 'with' => 'otherUser', + 'type' => 0, + 'permissions' => [Constants::PERMISSION_RESULTS_DELETE], + ]], + 'expected' => false ] ]; } @@ -788,6 +829,49 @@ public function testCanDeleteResults(string $ownerId, array $sharesArray, bool $ $this->assertEquals($expected, $this->formsService->canDeleteResults($form)); } + public function testCanDeleteResults_archivedFormDenied() { + // Test that archived forms deny deletion regardless of ownership or permissions + $form = new Form(); + $form->setId(42); + $form->setOwnerId('currentUser'); + $form->setState(Constants::FORM_STATE_ARCHIVED); + $form->setAccess([ + 'permitAllUsers' => false, + 'showToAllUsers' => false, + ]); + + $this->shareMapper->expects($this->any()) + ->method('findByForm') + ->with(42) + ->willReturn([]); + + $this->assertEquals(false, $this->formsService->canDeleteResults($form)); + } + + public function testCanDeleteResults_archivedFormDeniedWithResultsDeleteShare() { + // Test that archived forms deny deletion even with RESULTS_DELETE permission + $form = new Form(); + $form->setId(42); + $form->setOwnerId('someUser'); + $form->setState(Constants::FORM_STATE_ARCHIVED); + $form->setAccess([ + 'permitAllUsers' => false, + 'showToAllUsers' => false, + ]); + + $share = new Share(); + $share->setFormId(42); + $share->setShareWith('currentUser'); + $share->setPermissions([Constants::PERMISSION_RESULTS_DELETE]); + + $this->shareMapper->expects($this->any()) + ->method('findByForm') + ->with(42) + ->willReturn([$share]); + + $this->assertEquals(false, $this->formsService->canDeleteResults($form)); + } + public static function dataCanSubmit() { return [ 'allowFormOwner' => [