Skip to content

Commit 9341926

Browse files
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 bdacb88 commit 9341926

7 files changed

Lines changed: 227 additions & 24 deletions

File tree

docs/API_v3.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,9 @@ Returns condensed objects of all Forms beeing owned by the authenticated user.
9292
"submit"
9393
],
9494
"partial": true,
95-
"state": 0
95+
"state": 0,
96+
"lockedBy": null,
97+
"lockedUntil": null
9698
},
9799
{
98100
"id": 3,
@@ -105,7 +107,9 @@ Returns condensed objects of all Forms beeing owned by the authenticated user.
105107
"submit"
106108
],
107109
"partial": true,
108-
"state": 0
110+
"state": 0,
111+
"lockedBy": "someUser"
112+
"lockedUntil": 123456789
109113
}
110114
]
111115
```
@@ -169,6 +173,8 @@ Returns the full-depth object of the requested form (without submissions).
169173
"showExpiration": false,
170174
"canSubmit": true,
171175
"state": 0,
176+
"lockedBy": null,
177+
"lockedUntil": null,
172178
"permissions": [
173179
"edit",
174180
"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 |
@@ -59,6 +61,8 @@ This document describes the Object-Structure, that is used within the Forms App
5961
],
6062
"questions": [],
6163
"state": 0,
64+
"lockedBy": null,
65+
"lockedUntil": null,
6266
"shares": []
6367
"submissions": [],
6468
"submissionCount": 0,

lib/Controller/ApiController.php

Lines changed: 140 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
use OCP\IUser;
5353
use OCP\IUserManager;
5454
use OCP\IUserSession;
55-
5655
use Psr\Log\LoggerInterface;
5756

5857
/**
@@ -190,6 +189,8 @@ public function newForm(?int $fromId = null): DataResponse {
190189
unset($formData['state']);
191190
unset($formData['fileId']);
192191
unset($formData['fileFormat']);
192+
unset($formData['locked_by']);
193+
unset($formData['locked_until']);
193194
$formData['hash'] = $this->formsService->generateFormHash();
194195
// TRANSLATORS Appendix to the form Title of a duplicated/copied form.
195196
$formData['title'] .= ' - ' . $this->l10n->t('Copy');
@@ -267,16 +268,85 @@ public function updateForm(int $formId, array $keyValuePairs): DataResponse {
267268
'keyValuePairs' => $keyValuePairs
268269
]);
269270

270-
$form = $this->getFormIfAllowed($formId);
271+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
272+
$currentUserId = $this->currentUser->getUID();
271273

272274
// Don't allow empty array
273275
if (sizeof($keyValuePairs) === 0) {
274276
$this->logger->info('Empty keyValuePairs, will not update.');
275277
throw new OCSForbiddenException('Empty keyValuePairs, will not update.');
276278
}
277279

280+
// Process complete form locking (lockedUntil: 0)
281+
if (
282+
sizeof($keyValuePairs) === 2
283+
&& array_key_exists('lockedBy', $keyValuePairs) && $keyValuePairs['lockedBy'] === $form->getOwnerId()
284+
&& array_key_exists('lockedUntil', $keyValuePairs) && $keyValuePairs['lockedUntil'] === 0
285+
) {
286+
// Only allow form locking for form owner
287+
if ($currentUserId !== $form->getOwnerId() || $currentUserId !== $form->getLockedBy()) {
288+
$this->logger->debug('Only the form owner can lock the form permanently');
289+
throw new OCSForbiddenException('Only the form owner can lock the form permanently');
290+
}
291+
292+
// Only allow if the form is not currently locked by another user
293+
if (
294+
$form->getLockedBy() !== null
295+
&& $form->getLockedBy() !== $currentUserId
296+
&& $form->getLockedUntil() >= time()
297+
) {
298+
$this->logger->debug('Form is currently locked by another user.');
299+
throw new OCSForbiddenException('Form is currently locked by another user.');
300+
}
301+
302+
// Abort if form is already completely locked
303+
if ($form->getLockedUntil() === 0) {
304+
$this->logger->debug('Form is already locked completely.');
305+
throw new OCSBadRequestException('Form is already locked completely.');
306+
}
307+
308+
$form->setLockedBy($keyValuePairs['lockedBy']);
309+
$form->setLockedUntil($keyValuePairs['lockedUntil']);
310+
311+
// Update changed Columns in Db.
312+
$this->formMapper->update($form);
313+
314+
return new DataResponse($form->getId());
315+
}
316+
317+
// Process form unlocking
318+
if (
319+
sizeof($keyValuePairs) === 2
320+
&& array_key_exists('lockedBy', $keyValuePairs) && is_null($keyValuePairs['lockedBy'])
321+
&& array_key_exists('lockedUntil', $keyValuePairs) && is_null($keyValuePairs['lockedUntil'])
322+
) {
323+
// Only allow form unlocking if for form owner or lock user
324+
if ($currentUserId !== $form->getOwnerId() && $currentUserId !== $form->getLockedBy() && $form->getLockedUntil() !== 0) {
325+
$this->logger->debug('Only the form owner or the user who obtained the lock can unlock the form');
326+
throw new OCSForbiddenException('Only the form owner or the user who obtained the lock can unlock the form');
327+
}
328+
329+
// remove form lock
330+
$form->setLockedBy(null);
331+
$form->setLockedUntil(null);
332+
333+
// Update changed Columns in Db.
334+
$this->formMapper->update($form);
335+
336+
return new DataResponse($form->getId());
337+
}
338+
339+
// Lock form temporary
340+
$this->obtainFormLock($form);
341+
278342
// Process owner transfer
279343
if (sizeof($keyValuePairs) === 1 && key_exists('ownerId', $keyValuePairs)) {
344+
// Only allow owner transfer if current user is the form owner
345+
if ($currentUserId !== $form->getOwnerId()) {
346+
$this->logger->debug('Only the form owner can transfer ownership');
347+
throw new OCSForbiddenException('Only the form owner can transfer ownership');
348+
}
349+
280350
$this->logger->debug('Updating owner: formId: {formId}, userId: {uid}', [
281351
'formId' => $formId,
282352
'uid' => $keyValuePairs['ownerId']
@@ -288,23 +358,33 @@ public function updateForm(int $formId, array $keyValuePairs): DataResponse {
288358
throw new OCSBadRequestException('Could not find new form owner');
289359
}
290360

291-
// update form owner
361+
// update form owner and remove form lock
292362
$form->setOwnerId($keyValuePairs['ownerId']);
363+
$form->setLockedBy(null);
364+
$form->setLockedUntil(null);
293365

294366
// Update changed Columns in Db.
295367
$this->formMapper->update($form);
296368

297369
return new DataResponse($form->getOwnerId());
298370
}
299371

300-
// Don't allow to change params id, hash, ownerId, created, lastUpdated, fileId
301-
if (
302-
key_exists('id', $keyValuePairs) || key_exists('hash', $keyValuePairs) ||
303-
key_exists('ownerId', $keyValuePairs) || key_exists('created', $keyValuePairs) ||
304-
isset($keyValuePairs['fileId']) || key_exists('lastUpdated', $keyValuePairs)
305-
) {
306-
$this->logger->info('Not allowed to update id, hash, ownerId, created, fileId or lastUpdated');
307-
throw new OCSForbiddenException('Not allowed to update id, hash, ownerId, created, fileId or lastUpdated');
372+
// Don't allow to change the following attributes
373+
$forbiddenKeys = [
374+
'id', 'hash', 'ownerId', 'created', 'lastUpdated', 'lockedBy', 'lockedUntil'
375+
];
376+
377+
foreach ($forbiddenKeys as $key) {
378+
if (array_key_exists($key, $keyValuePairs)) {
379+
$this->logger->info("Not allowed to update {$key}");
380+
throw new OCSForbiddenException("Not allowed to update {$key}");
381+
}
382+
}
383+
384+
// Don't allow to change fileId
385+
if (isset($keyValuePairs['fileId'])) {
386+
$this->logger->info('Not allowed to update fileId');
387+
throw new OCSForbiddenException('Not allowed to update fileId');
308388
}
309389

310390
// Do not allow changing showToAllUsers if disabled
@@ -477,7 +557,9 @@ public function getQuestion(int $formId, int $questionId): DataResponse {
477557
#[BruteForceProtection(action: 'form')]
478558
#[ApiRoute(verb: 'POST', url: '/api/v3/forms/{formId}/questions')]
479559
public function newQuestion(int $formId, ?string $type = null, string $text = '', ?int $fromId = null): DataResponse {
480-
$form = $this->getFormIfAllowed($formId);
560+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
561+
$this->obtainFormLock($form);
562+
481563
if ($this->formsService->isFormArchived($form)) {
482564
$this->logger->debug('This form is archived and can not be modified');
483565
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -608,7 +690,9 @@ public function updateQuestion(int $formId, int $questionId, array $keyValuePair
608690

609691
// Make sure we query the form first to check the user has permissions
610692
// So the user does not get information about "questions" if they do not even have permissions to the form
611-
$form = $this->getFormIfAllowed($formId);
693+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
694+
$this->obtainFormLock($form);
695+
612696
if ($this->formsService->isFormArchived($form)) {
613697
$this->logger->debug('This form is archived and can not be modified');
614698
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -682,7 +766,9 @@ public function deleteQuestion(int $formId, int $questionId): DataResponse {
682766
]);
683767

684768

685-
$form = $this->getFormIfAllowed($formId);
769+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
770+
$this->obtainFormLock($form);
771+
686772
if ($this->formsService->isFormArchived($form)) {
687773
$this->logger->debug('This form is archived and can not be modified');
688774
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -748,7 +834,9 @@ public function reorderQuestions(int $formId, array $newOrder): DataResponse {
748834
'newOrder' => $newOrder
749835
]);
750836

751-
$form = $this->getFormIfAllowed($formId);
837+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
838+
$this->obtainFormLock($form);
839+
752840
if ($this->formsService->isFormArchived($form)) {
753841
$this->logger->debug('This form is archived and can not be modified');
754842
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -847,7 +935,9 @@ public function newOption(int $formId, int $questionId, array $optionTexts): Dat
847935
'text' => $optionTexts,
848936
]);
849937

850-
$form = $this->getFormIfAllowed($formId);
938+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
939+
$this->obtainFormLock($form);
940+
851941
if ($this->formsService->isFormArchived($form)) {
852942
$this->logger->debug('This form is archived and can not be modified');
853943
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -929,7 +1019,9 @@ public function updateOption(int $formId, int $questionId, int $optionId, array
9291019
'keyValuePairs' => $keyValuePairs
9301020
]);
9311021

932-
$form = $this->getFormIfAllowed($formId);
1022+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
1023+
$this->obtainFormLock($form);
1024+
9331025
if ($this->formsService->isFormArchived($form)) {
9341026
$this->logger->debug('This form is archived and can not be modified');
9351027
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -996,7 +1088,9 @@ public function deleteOption(int $formId, int $questionId, int $optionId): DataR
9961088
'optionId' => $optionId
9971089
]);
9981090

999-
$form = $this->getFormIfAllowed($formId);
1091+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
1092+
$this->obtainFormLock($form);
1093+
10001094
if ($this->formsService->isFormArchived($form)) {
10011095
$this->logger->debug('This form is archived and can not be modified');
10021096
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -1052,7 +1146,9 @@ public function deleteOption(int $formId, int $questionId, int $optionId): DataR
10521146
#[BruteForceProtection(action: 'form')]
10531147
#[ApiRoute(verb: 'PATCH', url: '/api/v3/forms/{formId}/questions/{questionId}/options')]
10541148
public function reorderOptions(int $formId, int $questionId, array $newOrder) {
1055-
$form = $this->getFormIfAllowed($formId);
1149+
$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
1150+
$this->obtainFormLock($form);
1151+
10561152
if ($this->formsService->isFormArchived($form)) {
10571153
$this->logger->debug('This form is archived and can not be modified');
10581154
throw new OCSForbiddenException('This form is archived and can not be modified');
@@ -1797,6 +1893,12 @@ private function getFormIfAllowed(int $formId, string $permissions = 'all'): For
17971893
throw new NoSuchFormException('This form is not owned by the current user and user has no `results_delete` permission', Http::STATUS_FORBIDDEN);
17981894
}
17991895
break;
1896+
case Constants::PERMISSION_EDIT:
1897+
if (!$this->formsService->canEditForm($form)) {
1898+
$this->logger->debug('This form is not owned by the current user and user has no `edit` permission');
1899+
throw new NoSuchFormException('This form is not owned by the current user and user has no `edit` permission', Http::STATUS_FORBIDDEN);
1900+
}
1901+
break;
18001902
default:
18011903
// By default we request full permissions
18021904
if ($form->getOwnerId() !== $this->currentUser->getUID()) {
@@ -1807,4 +1909,23 @@ private function getFormIfAllowed(int $formId, string $permissions = 'all'): For
18071909
}
18081910
return $form;
18091911
}
1912+
1913+
/**
1914+
* Locks the given form for the current user for a duration of 15 minutes.
1915+
*
1916+
* @param Form $form The form instance to lock.
1917+
*/
1918+
private function obtainFormLock(Form $form): void {
1919+
// Only lock if not locked or locked by current user, or lock has expired
1920+
if (
1921+
$form->getLockedBy() !== null
1922+
&& $form->getLockedBy() !== $this->currentUser->getUID()
1923+
&& ($form->getLockedUntil() >= time() || $form->getLockedUntil() === 0)
1924+
) {
1925+
throw new OCSForbiddenException('Form is currently locked by another user.');
1926+
}
1927+
1928+
$form->setLockedBy($this->currentUser->getUID());
1929+
$form->setLockedUntil(time() + 15 * 60);
1930+
}
18101931
}

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
}

0 commit comments

Comments
 (0)