Skip to content

Commit fc37acb

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 <[email protected]>
1 parent 75cebed commit fc37acb

File tree

8 files changed

+146
-14
lines changed

8 files changed

+146
-14
lines changed

‎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: 57 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,8 @@ public function updateForm(int $formId, array $keyValuePairs): DataResponse {
267267
'keyValuePairs' => $keyValuePairs
268268
]);
269269

270-
$form = $this->getFormIfAllowed($formId);
270+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
271+
$this->obtainFormLock($form);
271272

272273
// Don't allow empty array
273274
if (sizeof($keyValuePairs) === 0) {
@@ -277,6 +278,12 @@ public function updateForm(int $formId, array $keyValuePairs): DataResponse {
277278

278279
// Process owner transfer
279280
if (sizeof($keyValuePairs) === 1 && key_exists('ownerId', $keyValuePairs)) {
281+
// Only allow owner transfer if current user is the form owner
282+
if ($this->currentUser->getUID() !== $form->getOwnerId()) {
283+
$this->logger->debug('Only the form owner can transfer ownership');
284+
throw new OCSForbiddenException('Only the form owner can transfer ownership');
285+
}
286+
280287
$this->logger->debug('Updating owner: formId: {formId}, userId: {uid}', [
281288
'formId' => $formId,
282289
'uid' => $keyValuePairs['ownerId']
@@ -477,7 +484,9 @@ public function getQuestion(int $formId, int $questionId): DataResponse {
477484
#[BruteForceProtection(action: 'form')]
478485
#[ApiRoute(verb: 'POST', url: '/api/v3/forms/{formId}/questions')]
479486
public function newQuestion(int $formId, ?string $type = null, string $text = '', ?int $fromId = null): DataResponse {
480-
$form = $this->getFormIfAllowed($formId);
487+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
488+
$this->obtainFormLock($form);
489+
481490
if ($this->formsService->isFormArchived($form)) {
482491
$this->logger->debug('This form is archived and can not be modified');
483492
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -608,7 +617,9 @@ public function updateQuestion(int $formId, int $questionId, array $keyValuePair
608617

609618
// Make sure we query the form first to check the user has permissions
610619
// So the user does not get information about "questions" if they do not even have permissions to the form
611-
$form = $this->getFormIfAllowed($formId);
620+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
621+
$this->obtainFormLock($form);
622+
612623
if ($this->formsService->isFormArchived($form)) {
613624
$this->logger->debug('This form is archived and can not be modified');
614625
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -682,7 +693,9 @@ public function deleteQuestion(int $formId, int $questionId): DataResponse {
682693
]);
683694

684695

685-
$form = $this->getFormIfAllowed($formId);
696+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
697+
$this->obtainFormLock($form);
698+
686699
if ($this->formsService->isFormArchived($form)) {
687700
$this->logger->debug('This form is archived and can not be modified');
688701
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -748,7 +761,9 @@ public function reorderQuestions(int $formId, array $newOrder): DataResponse {
748761
'newOrder' => $newOrder
749762
]);
750763

751-
$form = $this->getFormIfAllowed($formId);
764+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
765+
$this->obtainFormLock($form);
766+
752767
if ($this->formsService->isFormArchived($form)) {
753768
$this->logger->debug('This form is archived and can not be modified');
754769
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -847,7 +862,9 @@ public function newOption(int $formId, int $questionId, array $optionTexts): Dat
847862
'text' => $optionTexts,
848863
]);
849864

850-
$form = $this->getFormIfAllowed($formId);
865+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
866+
$this->obtainFormLock($form);
867+
851868
if ($this->formsService->isFormArchived($form)) {
852869
$this->logger->debug('This form is archived and can not be modified');
853870
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -929,7 +946,9 @@ public function updateOption(int $formId, int $questionId, int $optionId, array
929946
'keyValuePairs' => $keyValuePairs
930947
]);
931948

932-
$form = $this->getFormIfAllowed($formId);
949+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
950+
$this->obtainFormLock($form);
951+
933952
if ($this->formsService->isFormArchived($form)) {
934953
$this->logger->debug('This form is archived and can not be modified');
935954
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -996,7 +1015,9 @@ public function deleteOption(int $formId, int $questionId, int $optionId): DataR
9961015
'optionId' => $optionId
9971016
]);
9981017

999-
$form = $this->getFormIfAllowed($formId);
1018+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
1019+
$this->obtainFormLock($form);
1020+
10001021
if ($this->formsService->isFormArchived($form)) {
10011022
$this->logger->debug('This form is archived and can not be modified');
10021023
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -1052,7 +1073,9 @@ public function deleteOption(int $formId, int $questionId, int $optionId): DataR
10521073
#[BruteForceProtection(action: 'form')]
10531074
#[ApiRoute(verb: 'PATCH', url: '/api/v3/forms/{formId}/questions/{questionId}/options')]
10541075
public function reorderOptions(int $formId, int $questionId, array $newOrder) {
1055-
$form = $this->getFormIfAllowed($formId);
1076+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
1077+
$this->obtainFormLock($form);
1078+
10561079
if ($this->formsService->isFormArchived($form)) {
10571080
$this->logger->debug('This form is archived and can not be modified');
10581081
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -1790,6 +1813,12 @@ private function getFormIfAllowed(int $formId, string $permissions = 'all'): For
17901813
throw new NoSuchFormException('This form is not owned by the current user and user has no `results_delete` permission', Http::STATUS_FORBIDDEN);
17911814
}
17921815
break;
1816+
case Constants::PERMISSION_EDIT:
1817+
if (!$this->formsService->canEditForm($form)) {
1818+
$this->logger->debug('This form is not owned by the current user and user has no `edit` permission');
1819+
throw new NoSuchFormException('This form is not owned by the current user and user has no `edit` permission', Http::STATUS_FORBIDDEN);
1820+
}
1821+
break;
17931822
default:
17941823
// By default we request full permissions
17951824
if ($form->getOwnerId() !== $this->currentUser->getUID()) {
@@ -1800,4 +1829,23 @@ private function getFormIfAllowed(int $formId, string $permissions = 'all'): For
18001829
}
18011830
return $form;
18021831
}
1832+
1833+
/**
1834+
* Locks the given form for the current user for a duration of 15 minutes.
1835+
*
1836+
* @param Form $form The form instance to lock.
1837+
*/
1838+
private function obtainFormLock(Form $form): void {
1839+
// Only lock if not locked or locked by current user, or lock has expired
1840+
if (
1841+
$form->getLockedBy() === null ||
1842+
$form->getLockedBy() === $this->currentUser->getUID() ||
1843+
$form->getLockedUntil() < time()
1844+
) {
1845+
$form->setLockedBy($this->currentUser->getUID());
1846+
$form->setLockedUntil(time() + 15 * 60);
1847+
} else {
1848+
throw new OCSForbiddenException('Form is currently locked by another user.');
1849+
}
1850+
}
18031851
}

‎lib/Db/Form.php

Lines changed: 10 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.
@@ -149,6 +155,8 @@ public function setAccess(array $access): void {
149155
* lastUpdated: int,
150156
* submissionMessage: ?string,
151157
* state: 0|1|2,
158+
* locked_by: ?string,
159+
* locked_until: ?int,
152160
* }
153161
*/
154162
public function read() {
@@ -170,6 +178,8 @@ public function read() {
170178
'lastUpdated' => (int)$this->getLastUpdated(),
171179
'submissionMessage' => $this->getSubmissionMessage(),
172180
'state' => $this->getState(),
181+
'locked_by' => $this->getLockedBy(),
182+
'locked_until' => $this->getLockedUntil(),
173183
];
174184
}
175185
}

‎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)