Skip to content

Commit a3198a4

Browse files
Chartman123Chartman123
committed
feat: add form locking mechanism
- Implemented form locking functionality to prevent concurrent edits. - Updated API responses to include `lockedBy` and `lockedUntil` fields. - Enhanced permission checks to ensure only the form owner can transfer ownership. - Revised documentation to reflect changes in form data structure and API behavior. Signed-off-by: Christian Hartmann <chris-hartmann@gmx.de>
1 parent 75cebed commit a3198a4

8 files changed

Lines changed: 150 additions & 14 deletions

File tree

‎docs/API_v3.md‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,9 @@ Returns condensed objects of all Forms beeing owned by the authenticated user.
9090
"submit"
9191
],
9292
"partial": true,
93-
"state": 0
93+
"state": 0,
94+
"lockedBy": null,
95+
"lockedUntil": null
9496
},
9597
{
9698
"id": 3,
@@ -103,7 +105,9 @@ Returns condensed objects of all Forms beeing owned by the authenticated user.
103105
"submit"
104106
],
105107
"partial": true,
106-
"state": 0
108+
"state": 0,
109+
"lockedBy": "someUser"
110+
"lockedUntil": 123456789
107111
}
108112
]
109113
```
@@ -166,6 +170,8 @@ Returns the full-depth object of the requested form (without submissions).
166170
"showExpiration": false,
167171
"canSubmit": true,
168172
"state": 0,
173+
"lockedBy": null,
174+
"lockedUntil": null,
169175
"permissions": [
170176
"edit",
171177
"results",

‎docs/DataStructure.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ This document describes the Object-Structure, that is used within the Forms App
2626
| expires | unix-timestamp | | When the form should expire. Timestamp `0` indicates _never_ |
2727
| isAnonymous | Boolean | | If Answers will be stored anonymously |
2828
| state | Integer | [Form state](#form-state) | The state of the form |
29+
| lockedBy | String | | The user ID for who has exclusive edit access at the moment |
30+
| lockedUntil | unix timestamp | | When the form lock will expire |
2931
| submitMultiple | Boolean | | If users are allowed to submit multiple times to the form |
3032
| allowEditSubmissions | Boolean | | If users are allowed to edit or delete their response |
3133
| showExpiration | Boolean | | If the expiration date will be shown on the form |
@@ -57,6 +59,8 @@ This document describes the Object-Structure, that is used within the Forms App
5759
],
5860
"questions": [],
5961
"state": 0,
62+
"lockedBy": null,
63+
"lockedUntil": null,
6064
"shares": []
6165
"submissions": [],
6266
}

‎lib/Controller/ApiController.php‎

Lines changed: 59 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,8 @@ public function newForm(?int $fromId = null): DataResponse {
190190
unset($formData['state']);
191191
unset($formData['fileId']);
192192
unset($formData['fileFormat']);
193+
unset($formData['locked_by']);
194+
unset($formData['locked_until']);
193195
$formData['hash'] = $this->formsService->generateFormHash();
194196
// TRANSLATORS Appendix to the form Title of a duplicated/copied form.
195197
$formData['title'] .= ' - ' . $this->l10n->t('Copy');
@@ -267,7 +269,8 @@ public function updateForm(int $formId, array $keyValuePairs): DataResponse {
267269
'keyValuePairs' => $keyValuePairs
268270
]);
269271

270-
$form = $this->getFormIfAllowed($formId);
272+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
273+
$this->obtainFormLock($form);
271274

272275
// Don't allow empty array
273276
if (sizeof($keyValuePairs) === 0) {
@@ -277,6 +280,12 @@ public function updateForm(int $formId, array $keyValuePairs): DataResponse {
277280

278281
// Process owner transfer
279282
if (sizeof($keyValuePairs) === 1 && key_exists('ownerId', $keyValuePairs)) {
283+
// Only allow owner transfer if current user is the form owner
284+
if ($this->currentUser->getUID() !== $form->getOwnerId()) {
285+
$this->logger->debug('Only the form owner can transfer ownership');
286+
throw new OCSForbiddenException('Only the form owner can transfer ownership');
287+
}
288+
280289
$this->logger->debug('Updating owner: formId: {formId}, userId: {uid}', [
281290
'formId' => $formId,
282291
'uid' => $keyValuePairs['ownerId']
@@ -477,7 +486,9 @@ public function getQuestion(int $formId, int $questionId): DataResponse {
477486
#[BruteForceProtection(action: 'form')]
478487
#[ApiRoute(verb: 'POST', url: '/api/v3/forms/{formId}/questions')]
479488
public function newQuestion(int $formId, ?string $type = null, string $text = '', ?int $fromId = null): DataResponse {
480-
$form = $this->getFormIfAllowed($formId);
489+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
490+
$this->obtainFormLock($form);
491+
481492
if ($this->formsService->isFormArchived($form)) {
482493
$this->logger->debug('This form is archived and can not be modified');
483494
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -608,7 +619,9 @@ public function updateQuestion(int $formId, int $questionId, array $keyValuePair
608619

609620
// Make sure we query the form first to check the user has permissions
610621
// So the user does not get information about "questions" if they do not even have permissions to the form
611-
$form = $this->getFormIfAllowed($formId);
622+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
623+
$this->obtainFormLock($form);
624+
612625
if ($this->formsService->isFormArchived($form)) {
613626
$this->logger->debug('This form is archived and can not be modified');
614627
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -682,7 +695,9 @@ public function deleteQuestion(int $formId, int $questionId): DataResponse {
682695
]);
683696

684697

685-
$form = $this->getFormIfAllowed($formId);
698+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
699+
$this->obtainFormLock($form);
700+
686701
if ($this->formsService->isFormArchived($form)) {
687702
$this->logger->debug('This form is archived and can not be modified');
688703
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -748,7 +763,9 @@ public function reorderQuestions(int $formId, array $newOrder): DataResponse {
748763
'newOrder' => $newOrder
749764
]);
750765

751-
$form = $this->getFormIfAllowed($formId);
766+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
767+
$this->obtainFormLock($form);
768+
752769
if ($this->formsService->isFormArchived($form)) {
753770
$this->logger->debug('This form is archived and can not be modified');
754771
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -847,7 +864,9 @@ public function newOption(int $formId, int $questionId, array $optionTexts): Dat
847864
'text' => $optionTexts,
848865
]);
849866

850-
$form = $this->getFormIfAllowed($formId);
867+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
868+
$this->obtainFormLock($form);
869+
851870
if ($this->formsService->isFormArchived($form)) {
852871
$this->logger->debug('This form is archived and can not be modified');
853872
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -929,7 +948,9 @@ public function updateOption(int $formId, int $questionId, int $optionId, array
929948
'keyValuePairs' => $keyValuePairs
930949
]);
931950

932-
$form = $this->getFormIfAllowed($formId);
951+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
952+
$this->obtainFormLock($form);
953+
933954
if ($this->formsService->isFormArchived($form)) {
934955
$this->logger->debug('This form is archived and can not be modified');
935956
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -996,7 +1017,9 @@ public function deleteOption(int $formId, int $questionId, int $optionId): DataR
9961017
'optionId' => $optionId
9971018
]);
9981019

999-
$form = $this->getFormIfAllowed($formId);
1020+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
1021+
$this->obtainFormLock($form);
1022+
10001023
if ($this->formsService->isFormArchived($form)) {
10011024
$this->logger->debug('This form is archived and can not be modified');
10021025
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -1052,7 +1075,9 @@ public function deleteOption(int $formId, int $questionId, int $optionId): DataR
10521075
#[BruteForceProtection(action: 'form')]
10531076
#[ApiRoute(verb: 'PATCH', url: '/api/v3/forms/{formId}/questions/{questionId}/options')]
10541077
public function reorderOptions(int $formId, int $questionId, array $newOrder) {
1055-
$form = $this->getFormIfAllowed($formId);
1078+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
1079+
$this->obtainFormLock($form);
1080+
10561081
if ($this->formsService->isFormArchived($form)) {
10571082
$this->logger->debug('This form is archived and can not be modified');
10581083
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -1790,6 +1815,12 @@ private function getFormIfAllowed(int $formId, string $permissions = 'all'): For
17901815
throw new NoSuchFormException('This form is not owned by the current user and user has no `results_delete` permission', Http::STATUS_FORBIDDEN);
17911816
}
17921817
break;
1818+
case Constants::PERMISSION_EDIT:
1819+
if (!$this->formsService->canEditForm($form)) {
1820+
$this->logger->debug('This form is not owned by the current user and user has no `edit` permission');
1821+
throw new NoSuchFormException('This form is not owned by the current user and user has no `edit` permission', Http::STATUS_FORBIDDEN);
1822+
}
1823+
break;
17931824
default:
17941825
// By default we request full permissions
17951826
if ($form->getOwnerId() !== $this->currentUser->getUID()) {
@@ -1800,4 +1831,23 @@ private function getFormIfAllowed(int $formId, string $permissions = 'all'): For
18001831
}
18011832
return $form;
18021833
}
1834+
1835+
/**
1836+
* Locks the given form for the current user for a duration of 15 minutes.
1837+
*
1838+
* @param Form $form The form instance to lock.
1839+
*/
1840+
private function obtainFormLock(Form $form): void {
1841+
// Only lock if not locked or locked by current user, or lock has expired
1842+
if (
1843+
$form->getLockedBy() === null ||
1844+
$form->getLockedBy() === $this->currentUser->getUID() ||
1845+
$form->getLockedUntil() < time()
1846+
) {
1847+
$form->setLockedBy($this->currentUser->getUID());
1848+
$form->setLockedUntil(time() + 15 * 60);
1849+
} else {
1850+
throw new OCSForbiddenException('Form is currently locked by another user.');
1851+
}
1852+
}
18031853
}

‎lib/Db/Form.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@
4747
* @psalm-method 0|1|2 getState()
4848
* @method void setState(int|null $value)
4949
* @psalm-method void setState(0|1|2|null $value)
50+
* @method string getLockedBy()
51+
* @method void setLockedBy(string $value)
52+
* @method int getLockedUntil()
53+
* @method void setLockedUntil(int $value)
5054
*/
5155
class Form extends Entity {
5256
protected $hash;
@@ -65,6 +69,8 @@ class Form extends Entity {
6569
protected $submissionMessage;
6670
protected $lastUpdated;
6771
protected $state;
72+
protected $lockedBy;
73+
protected $lockedUntil;
6874

6975
/**
7076
* Form constructor.
@@ -78,6 +84,8 @@ public function __construct() {
7884
$this->addType('showExpiration', 'boolean');
7985
$this->addType('lastUpdated', 'integer');
8086
$this->addType('state', 'integer');
87+
$this->addType('locked_by', 'string');
88+
$this->addType('locked_until', 'integer');
8189
}
8290

8391
// JSON-Decoding of access-column.
@@ -149,6 +157,8 @@ public function setAccess(array $access): void {
149157
* lastUpdated: int,
150158
* submissionMessage: ?string,
151159
* state: 0|1|2,
160+
* locked_by: ?string,
161+
* locked_until: ?int,
152162
* }
153163
*/
154164
public function read() {
@@ -170,6 +180,8 @@ public function read() {
170180
'lastUpdated' => (int)$this->getLastUpdated(),
171181
'submissionMessage' => $this->getSubmissionMessage(),
172182
'state' => $this->getState(),
183+
'locked_by' => $this->getLockedBy(),
184+
'locked_until' => $this->getLockedUntil(),
173185
];
174186
}
175187
}

‎lib/Migration/Version050200Date20250512004000.php‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\Forms\Migration;
11+
12+
use Closure;
13+
use OCP\DB\ISchemaWrapper;
14+
use OCP\DB\Types;
15+
use OCP\Migration\IOutput;
16+
use OCP\Migration\SimpleMigrationStep;
17+
18+
class Version050200Date20250512004000 extends SimpleMigrationStep {
19+
20+
/**
21+
* @param IOutput $output
22+
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
23+
* @param array $options
24+
* @return null|ISchemaWrapper
25+
*/
26+
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
27+
/** @var ISchemaWrapper $schema */
28+
$schema = $schemaClosure();
29+
$table = $schema->getTable('forms_v2_forms');
30+
31+
if (!$table->hasColumn('locked_by')) {
32+
$table->addColumn('locked_by', Types::STRING, [
33+
'notnull' => false,
34+
'default' => null,
35+
]);
36+
}
37+
38+
if (!$table->hascolumn('locked_until')) {
39+
$table->addColumn('locked_until', Types::INTEGER, [
40+
'notnull' => false,
41+
'default' => null,
42+
'comment' => 'unix-timestamp',
43+
]);
44+
}
45+
46+
return $schema;
47+
}
48+
}

‎lib/ResponseDefinitions.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@
117117
* expires: int,
118118
* fileFormat: ?string,
119119
* fileId: ?int,
120-
* filePath?: ?string,
120+
* filePath?: string,
121121
* isAnonymous: bool,
122122
* lastUpdated: int,
123123
* submitMultiple: bool,
@@ -127,6 +127,8 @@
127127
* permissions: list<FormsPermission>,
128128
* questions: list<FormsQuestion>,
129129
* state: 0|1|2,
130+
* lockedBy: ?string,
131+
* lockedUntil: ?int,
130132
* shares: list<FormsShare>,
131133
* submissionCount?: int,
132134
* submissionMessage: ?string,

‎lib/Service/FormsService.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,10 @@ public function getPermissions(Form $form): array {
319319
* @return boolean
320320
*/
321321
public function canEditForm(Form $form): bool {
322+
// Check if form is currently locked by another user
323+
if ($form->getLockedBy() !== $this->currentUser->getUID() && $form->getLockedUntil() >= time()) {
324+
return false;
325+
}
322326
return in_array(Constants::PERMISSION_EDIT, $this->getPermissions($form));
323327
}
324328

‎openapi.json‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,8 @@
111111
"permissions",
112112
"questions",
113113
"state",
114+
"lockedBy",
115+
"lockedUntil",
114116
"shares",
115117
"submissionMessage"
116118
],
@@ -152,8 +154,7 @@
152154
"nullable": true
153155
},
154156
"filePath": {
155-
"type": "string",
156-
"nullable": true
157+
"type": "string"
157158
},
158159
"isAnonymous": {
159160
"type": "boolean"
@@ -195,6 +196,15 @@
195196
2
196197
]
197198
},
199+
"lockedBy": {
200+
"type": "string",
201+
"nullable": true
202+
},
203+
"lockedUntil": {
204+
"type": "integer",
205+
"format": "int64",
206+
"nullable": true
207+
},
198208
"shares": {
199209
"type": "array",
200210
"items": {

0 commit comments

Comments
 (0)