Skip to content

feat: add form locking mechanism and share edit permission #2737

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.

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
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
10 changes: 8 additions & 2 deletions docs/API_v3.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,9 @@ Returns condensed objects of all Forms beeing owned by the authenticated user.
"submit"
],
"partial": true,
"state": 0
"state": 0,
"lockedBy": null,
"lockedUntil": null
},
{
"id": 3,
Expand All @@ -105,7 +107,9 @@ Returns condensed objects of all Forms beeing owned by the authenticated user.
"submit"
],
"partial": true,
"state": 0
"state": 0,
"lockedBy": "someUser"
"lockedUntil": 123456789
}
]
```
Expand Down Expand Up @@ -169,6 +173,8 @@ Returns the full-depth object of the requested form (without submissions).
"showExpiration": false,
"canSubmit": true,
"state": 0,
"lockedBy": null,
"lockedUntil": null,
"permissions": [
"edit",
"results",
Expand Down
4 changes: 4 additions & 0 deletions docs/DataStructure.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ This document describes the Object-Structure, that is used within the Forms App
| expires | unix-timestamp | | When the form should expire. Timestamp `0` indicates _never_ |
| isAnonymous | Boolean | | If Answers will be stored anonymously |
| state | Integer | [Form state](#form-state) | The state of the form |
| lockedBy | String | | The user ID for who has exclusive edit access at the moment |
| lockedUntil | unix timestamp | | When the form lock will expire |
| submitMultiple | Boolean | | If users are allowed to submit multiple times to the form |
| allowEditSubmissions | Boolean | | If users are allowed to edit or delete their response |
| showExpiration | Boolean | | If the expiration date will be shown on the form |
Expand Down Expand Up @@ -59,6 +61,8 @@ This document describes the Object-Structure, that is used within the Forms App
],
"questions": [],
"state": 0,
"lockedBy": null,
"lockedUntil": null,
"shares": []
"submissions": [],
"submissionCount": 0,
Expand Down
1 change: 1 addition & 0 deletions img/lock_open.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
326 changes: 190 additions & 136 deletions lib/Controller/ApiController.php

Large diffs are not rendered by default.

62 changes: 23 additions & 39 deletions lib/Controller/ShareApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,12 @@
'permissions' => $permissions,
]);

$form = $this->formsService->getFormIfAllowed($formId);
if ($this->formsService->isFormArchived($form)) {
$this->logger->debug('This form is archived and can not be modified');
throw new OCSForbiddenException('This form is archived and can not be modified');

Check warning on line 110 in lib/Controller/ShareApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ShareApiController.php#L109-L110

Added lines #L109 - L110 were not covered by tests
}

// Only accept usable shareTypes
if (array_search($shareType, Constants::SHARE_TYPES_USED) === false) {
$this->logger->debug('Invalid shareType');
Expand All @@ -116,24 +122,6 @@
throw new OCSForbiddenException('Link share not allowed.');
}

try {
$form = $this->formMapper->findById($formId);
} catch (IMapperException $e) {
$this->logger->debug('Could not find form', ['exception' => $e]);
throw new OCSNotFoundException('Could not find form');
}

if ($this->formsService->isFormArchived($form)) {
$this->logger->debug('This form is archived and can not be modified');
throw new OCSForbiddenException('This form is archived and can not be modified');
}

// Check for permission to share form
if ($form->getOwnerId() !== $this->currentUser->getUID()) {
$this->logger->debug('This form is not owned by the current user');
throw new OCSForbiddenException('This form is not owned by the current user');
}

if (!$this->validatePermissions($permissions, $shareType)) {
throw new OCSBadRequestException('Invalid permission given');
}
Expand Down Expand Up @@ -194,6 +182,8 @@
throw new OCSBadRequestException('Unknown shareType.');
}

$this->formsService->obtainFormLock($form);

$share = new Share();
$share->setFormId($formId);
$share->setShareType($shareType);
Expand Down Expand Up @@ -240,29 +230,24 @@
'keyValuePairs' => $keyValuePairs
]);

$form = $this->formsService->getFormIfAllowed($formId);
if ($this->formsService->isFormArchived($form)) {
$this->logger->debug('This form is archived and can not be modified');
throw new OCSForbiddenException('This form is archived and can not be modified');

Check warning on line 236 in lib/Controller/ShareApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ShareApiController.php#L235-L236

Added lines #L235 - L236 were not covered by tests
}

try {
$formShare = $this->shareMapper->findById($shareId);
$form = $this->formMapper->findById($formId);
} catch (IMapperException $e) {
$this->logger->debug('Could not find share', ['exception' => $e]);
throw new OCSNotFoundException('Could not find share');
}

if ($this->formsService->isFormArchived($form)) {
$this->logger->debug('This form is archived and can not be modified');
throw new OCSForbiddenException('This form is archived and can not be modified');
}

if ($formId !== $formShare->getFormId()) {
$this->logger->debug('This share doesn\'t belong to the given Form');
throw new OCSBadRequestException('Share doesn\'t belong to given Form');
}

if ($form->getOwnerId() !== $this->currentUser->getUID()) {
$this->logger->debug('This form is not owned by the current user');
throw new OCSForbiddenException('This form is not owned by the current user');
}

// Don't allow empty array
if (sizeof($keyValuePairs) === 0) {
$this->logger->info('Empty keyValuePairs, will not update.');
Expand All @@ -279,6 +264,8 @@
throw new OCSBadRequestException('Invalid permission given');
}

$this->formsService->obtainFormLock($form);

$formShare->setPermissions($keyValuePairs['permissions']);
$formShare = $this->shareMapper->update($formShare);

Expand Down Expand Up @@ -338,28 +325,25 @@
'shareId' => $shareId,
]);

$form = $this->formsService->getFormIfAllowed($formId);
if ($this->formsService->isFormArchived($form)) {
$this->logger->debug('This form is archived and can not be modified');
throw new OCSForbiddenException('This form is archived and can not be modified');

Check warning on line 331 in lib/Controller/ShareApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ShareApiController.php#L330-L331

Added lines #L330 - L331 were not covered by tests
}

try {
$share = $this->shareMapper->findById($shareId);
$form = $this->formMapper->findById($formId);
} catch (IMapperException $e) {
$this->logger->debug('Could not find share', ['exception' => $e]);
throw new OCSNotFoundException('Could not find share');
}

if ($this->formsService->isFormArchived($form)) {
$this->logger->debug('This form is archived and can not be modified');
throw new OCSForbiddenException('This form is archived and can not be modified');
}

if ($formId !== $share->getFormId()) {
$this->logger->debug('This share doesn\'t belong to the given Form');
throw new OCSBadRequestException('Share doesn\'t belong to given Form');
}

if ($form->getOwnerId() !== $this->currentUser->getUID()) {
$this->logger->debug('This form is not owned by the current user');
throw new OCSForbiddenException('This form is not owned by the current user');
}
$this->formsService->obtainFormLock($form);

$this->shareMapper->delete($share);
$this->formMapper->update($form);
Expand Down
12 changes: 12 additions & 0 deletions lib/Db/Form.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@
* @psalm-method 0|1|2 getState()
* @method void setState(int|null $value)
* @psalm-method void setState(0|1|2|null $value)
* @method string getLockedBy()
* @method void setLockedBy(string|null $value)
* @method int getLockedUntil()
* @method void setLockedUntil(int|null $value)
*/
class Form extends Entity {
protected $hash;
Expand All @@ -65,6 +69,8 @@ class Form extends Entity {
protected $submissionMessage;
protected $lastUpdated;
protected $state;
protected $lockedBy;
protected $lockedUntil;

/**
* Form constructor.
Expand All @@ -78,6 +84,8 @@ public function __construct() {
$this->addType('showExpiration', 'boolean');
$this->addType('lastUpdated', 'integer');
$this->addType('state', 'integer');
$this->addType('lockedBy', 'string');
$this->addType('lockedUntil', 'integer');
}

// JSON-Decoding of access-column.
Expand Down Expand Up @@ -149,6 +157,8 @@ public function setAccess(array $access): void {
* lastUpdated: int,
* submissionMessage: ?string,
* state: 0|1|2,
* lockedBy: ?string,
* lockedUntil: ?int,
* }
*/
public function read() {
Expand All @@ -170,6 +180,8 @@ public function read() {
'lastUpdated' => (int)$this->getLastUpdated(),
'submissionMessage' => $this->getSubmissionMessage(),
'state' => $this->getState(),
'lockedBy' => $this->getLockedBy(),
'lockedUntil' => $this->getLockedUntil(),
];
}
}
48 changes: 48 additions & 0 deletions lib/Migration/Version050200Date20250512004000.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Forms\Migration;

use Closure;
use OCP\DB\ISchemaWrapper;
use OCP\DB\Types;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;

class Version050200Date20250512004000 extends SimpleMigrationStep {

/**
* @param IOutput $output
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
* @param array $options
* @return null|ISchemaWrapper
*/
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {

Check warning on line 26 in lib/Migration/Version050200Date20250512004000.php

View check run for this annotation

Codecov / codecov/patch

lib/Migration/Version050200Date20250512004000.php#L26

Added line #L26 was not covered by tests
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();
$table = $schema->getTable('forms_v2_forms');

Check warning on line 29 in lib/Migration/Version050200Date20250512004000.php

View check run for this annotation

Codecov / codecov/patch

lib/Migration/Version050200Date20250512004000.php#L28-L29

Added lines #L28 - L29 were not covered by tests

if (!$table->hasColumn('locked_by')) {
$table->addColumn('locked_by', Types::STRING, [
'notnull' => false,
'default' => null,
]);

Check warning on line 35 in lib/Migration/Version050200Date20250512004000.php

View check run for this annotation

Codecov / codecov/patch

lib/Migration/Version050200Date20250512004000.php#L31-L35

Added lines #L31 - L35 were not covered by tests
}

if (!$table->hascolumn('locked_until')) {
$table->addColumn('locked_until', Types::INTEGER, [
'notnull' => false,
'default' => null,
'comment' => 'unix-timestamp',
]);

Check warning on line 43 in lib/Migration/Version050200Date20250512004000.php

View check run for this annotation

Codecov / codecov/patch

lib/Migration/Version050200Date20250512004000.php#L38-L43

Added lines #L38 - L43 were not covered by tests
}

return $schema;

Check warning on line 46 in lib/Migration/Version050200Date20250512004000.php

View check run for this annotation

Codecov / codecov/patch

lib/Migration/Version050200Date20250512004000.php#L46

Added line #L46 was not covered by tests
}
}
6 changes: 5 additions & 1 deletion lib/ResponseDefinitions.php
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,9 @@
* expires: int,
* permissions: list<FormsPermission>,
* partial: true,
* state: int
* state: int,
* lockedBy: ?string,
* lockedUntil: ?int,
* }
*
* @psalm-type FormsForm = array{
Expand All @@ -128,6 +130,8 @@
* permissions: list<FormsPermission>,
* questions: list<FormsQuestion>,
* state: 0|1|2,
* lockedBy: ?string,
* lockedUntil: ?int,
* shares: list<FormsShare>,
* submissionCount?: int,
* submissionMessage: ?string,
Expand Down
Loading
Loading