Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Chartman123 merged 2 commits into from
Apr 14, 2026
+126 −0
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ae7e5b5
fix: require explicit share permission for bulk submission actions
pringelmann Apr 8, 2026
6039113
PR feedback
pringelmann Apr 8, 2026
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: 8 additions & 0 deletions lib/Controller/ApiController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1322,6 +1322,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);
Expand Down
118 changes: 118 additions & 0 deletions tests/Unit/Controller/ApiControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1031,6 +1031,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);
Expand Down
Loading