Skip to content

Commit

Permalink
enh: Allow to set results delete permission on the frontend
Browse files Browse the repository at this point in the history
Signed-off-by: Ferdinand Thiessen <[email protected]>
  • Loading branch information
susnux authored and Chartman123 committed Nov 28, 2023
1 parent 5a1f047 commit 93ecf2c
Show file tree
Hide file tree
Showing 7 changed files with 153 additions and 3 deletions.
8 changes: 6 additions & 2 deletions lib/Controller/ApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -1016,9 +1016,13 @@ public function deleteSubmission(int $id): DataResponse {
throw new OCSBadRequestException();
}

// Either the current user needs to be the owner
if ($form->getOwnerId() !== $this->currentUser->getUID()) {
$this->logger->debug('This form is not owned by the current user');
throw new OCSForbiddenException();
// Or has permissions to remove submissions
if (!$this->formsService->canDeleteResults($form)) {
$this->logger->debug('This form is not owned by the current user and user has no results_delete permission');
throw new OCSForbiddenException();

Check warning on line 1024 in lib/Controller/ApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ApiController.php#L1022-L1024

Added lines #L1022 - L1024 were not covered by tests
}
}

// Delete submission (incl. Answers)
Expand Down
5 changes: 5 additions & 0 deletions lib/Controller/ShareApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,11 @@ protected function validatePermissions(array $permissions, int $shareType): bool
return false;
}

if (in_array(Constants::PERMISSION_RESULTS_DELETE, $sanitizedPermissions) && !in_array(Constants::PERMISSION_RESULTS, $sanitizedPermissions)) {
$this->logger->debug('Permission results_delete is only allowed when permission results is also set');
return false;
}

// Make sure only users can have special permissions
if (count($sanitizedPermissions) > 1) {
switch ($shareType) {
Expand Down
12 changes: 11 additions & 1 deletion lib/Service/FormsService.php
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,16 @@ public function canSeeResults(Form $form): bool {
return in_array(Constants::PERMISSION_RESULTS, $this->getPermissions($form));
}

/**
* Can the current user delete results of a form
*
* @param Form $form
* @return boolean
*/
public function canDeleteResults(Form $form): bool {
return in_array(Constants::PERMISSION_RESULTS_DELETE, $this->getPermissions($form));
}

/**
* Can the user submit a form
*
Expand Down Expand Up @@ -475,7 +485,7 @@ public function notifyNewSubmission(Form $form, string $submitter): void {
*
* @param int $formId The form to query shares for
* @param string $userId The user to check if shared with
* @return array
* @return Share[]
*/
protected function getSharesWithUser(int $formId, string $userId): array {
$shareEntities = $this->shareMapper->findByForm($formId);
Expand Down
17 changes: 17 additions & 0 deletions src/components/SidebarTabs/SharingShareDiv.vue
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
<NcActionCheckbox :checked="canAccessResults" @update:checked="updatePermissionResults">
{{ t('forms', 'View responses') }}
</NcActionCheckbox>
<NcActionCheckbox :checked="canDeleteResults" :disabled="!canAccessResults" @update:checked="updatePermissionDeleteResults">
{{ t('forms', 'Delete responses') }}
</NcActionCheckbox>
<NcActionSeparator />
<NcActionButton @click="removeShare">
<template #icon>
Expand Down Expand Up @@ -82,6 +85,9 @@ export default {
canAccessResults() {
return this.share.permissions.includes(this.PERMISSION_TYPES.PERMISSION_RESULTS)
},
canDeleteResults() {
return this.share.permissions.includes(this.PERMISSION_TYPES.PERMISSION_RESULTS_DELETE)
},
isNoUser() {
return this.share.shareType !== this.SHARE_TYPES.SHARE_TYPE_USER
},
Expand Down Expand Up @@ -109,9 +115,20 @@ export default {
* @param {boolean} hasPermission If the results permission should be granted
*/
updatePermissionResults(hasPermission) {
if (hasPermission === false) {
// ensure to remove the delete permission if results permission is dropped
this.updatePermission(this.PERMISSION_TYPES.PERMISSION_RESULTS_DELETE, false)
}
return this.updatePermission(this.PERMISSION_TYPES.PERMISSION_RESULTS, hasPermission)
},
/**
* @param {boolean} hasPermission If the results_delete permission should be granted
*/
updatePermissionDeleteResults(hasPermission) {
return this.updatePermission(this.PERMISSION_TYPES.PERMISSION_RESULTS_DELETE, hasPermission)
},
/**
* Grant or remove permission from share
*
Expand Down
1 change: 1 addition & 0 deletions src/views/Results.vue
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ export default {
try {
await axios.delete(generateOcsUrl('apps/forms/api/v2.1/submission/{id}', { id }))
showSuccess(t('forms', 'Submission deleted'))
const index = this.form.submissions.findIndex(search => search.id === id)
this.form.submissions.splice(index, 1)
emit('forms:last-updated:set', this.form.id)
Expand Down
46 changes: 46 additions & 0 deletions tests/Unit/Controller/ShareApiControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,21 @@ public function dataUpdateShare() {
'expected' => 1,
'exception' => null
],
'valid-permissions-share-delete' => [
'share' => [
'id' => 1,
'formId' => 5,
'shareType' => 0,
'shareWith' => 'user1',
'permissions' => [Constants::PERMISSION_SUBMIT]
],
'formOwner' => 'currentUser',
'keyValuePairs' => [
'permissions' => [Constants::PERMISSION_RESULTS, Constants::PERMISSION_RESULTS_DELETE, Constants::PERMISSION_SUBMIT]
],
'expected' => 1,
'exception' => null
],
'no-permission' => [
'share' => [
'id' => 1,
Expand All @@ -609,6 +624,37 @@ public function dataUpdateShare() {
'expected' => null,
'exception' => '\OCP\AppFramework\OCS\OCSBadRequestException'
],
'invalid-permission-missing-submit' => [
'share' => [
'id' => 1,
'formId' => 5,
'shareType' => 0,
'shareWith' => 'user1',
'permissions' => [Constants::PERMISSION_SUBMIT],
],
'formOwner' => 'currentUser',
'keyValuePairs' => [
'permissions' => [Constants::PERMISSION_RESULTS_DELETE],
],
'expected' => null,
'exception' => '\OCP\AppFramework\OCS\OCSBadRequestException'
],
// PERMISSION_RESULTS_DELETE is only allowed if PERMISSION_RESULTS is set
'invalid-permission-missing-results' => [
'share' => [
'id' => 1,
'formId' => 5,
'shareType' => 0,
'shareWith' => 'user1',
'permissions' => [Constants::PERMISSION_SUBMIT],
],
'formOwner' => 'currentUser',
'keyValuePairs' => [
'permissions' => [Constants::PERMISSION_SUBMIT, Constants::PERMISSION_RESULTS_DELETE],
],
'expected' => null,
'exception' => '\OCP\AppFramework\OCS\OCSBadRequestException'
],
'invalid-permission' => [
'share' => [
'id' => 1,
Expand Down
67 changes: 67 additions & 0 deletions tests/Unit/Service/FormsServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,73 @@ public function testCanSeeResults(string $ownerId, array $sharesArray, bool $exp
$this->assertEquals($expected, $this->formsService->canSeeResults($form));
}

public function dataCanDeleteResults() {
return [
'allowFormOwner' => [
'ownerId' => 'currentUser',
'sharesArray' => [],
'expected' => true
],
'allowShared' => [
'ownerId' => 'someUser',
'sharesArray' => [[
'with' => 'currentUser',
'type' => 0,
'permissions' => [Constants::PERMISSION_SUBMIT, Constants::PERMISSION_RESULTS, Constants::PERMISSION_RESULTS_DELETE],
]],
'expected' => true
],
'disallowNotowned' => [
'ownerId' => 'someUser',
'sharesArray' => [],
'expected' => false
],
'allowNotShared' => [
'ownerId' => 'someUser',
'sharesArray' => [[
'with' => 'currentUser',
'type' => 0,
'permissions' => [Constants::PERMISSION_SUBMIT, Constants::PERMISSION_RESULTS],
]],
'expected' => false
]
];
}
/**
* @dataProvider dataCanDeleteResults
*
* @param string $ownerId
* @param array $sharesArray
* @param bool $expected
*/
public function testCanDeleteResults(string $ownerId, array $sharesArray, bool $expected) {
$form = new Form();
$form->setId(42);
$form->setOwnerId($ownerId);
$form->setAccess([
'permitAllUsers' => false,
'showToAllUsers' => false,
]);

$shares = [];
foreach ($sharesArray as $id => $share) {
$shareEntity = new Share();
$shareEntity->setId($id);
$shareEntity->setFormId(42);
$shareEntity->setShareType($share['type']);
$shareEntity->setShareWith($share['with']);
$shareEntity->setPermissions($share['permissions']);
$shares[] = $shareEntity;
}

$this->shareMapper->expects($this->any())
->method('findByForm')
->with(42)
->willReturn($shares);

$this->assertEquals($expected, $this->formsService->canDeleteResults($form));
}

public function dataCanSubmit() {
return [
'allowFormOwner' => [
Expand Down

0 comments on commit 93ecf2c

Please sign in to comment.