Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions lib/Controller/ApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);

Expand Down
5 changes: 4 additions & 1 deletion lib/Controller/PageController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
8 changes: 6 additions & 2 deletions src/views/Results.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
},

Expand Down
11 changes: 9 additions & 2 deletions src/views/Submit.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
112 changes: 109 additions & 3 deletions tests/Integration/Api/ApiV3Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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,
],
]
]
Expand Down Expand Up @@ -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);
}
};
Loading
Loading