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>

Author: Chartman123

docs/API_v3.md

@@ -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,
@@ -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
   }
 ]
 ```
@@ -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",

docs/DataStructure.md

@@ -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                                                                                 |
@@ -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,

lib/Controller/ApiController.php

@@ -52,7 +52,7 @@
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\IUserSession;
-
+use Psalm\Internal\Scanner\UnresolvedConstant\KeyValuePair;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -190,6 +190,8 @@ public function newForm(?int $fromId = null): DataResponse {
 			unset($formData['state']);
 			unset($formData['fileId']);
 			unset($formData['fileFormat']);
+			unset($formData['locked_by']);
+			unset($formData['locked_until']);
 			$formData['hash'] = $this->formsService->generateFormHash();
 			// TRANSLATORS Appendix to the form Title of a duplicated/copied form.
 			$formData['title'] .= ' - ' . $this->l10n->t('Copy');
@@ -267,7 +269,9 @@ public function updateForm(int $formId, array $keyValuePairs): DataResponse {
 			'keyValuePairs' => $keyValuePairs
 		]);
 
-		$form = $this->getFormIfAllowed($formId);
+		$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
+		$currentUserId = $this->currentUser->getUID();
+		$this->obtainFormLock($form);
 
 		// Don't allow empty array
 		if (sizeof($keyValuePairs) === 0) {
@@ -277,6 +281,12 @@ public function updateForm(int $formId, array $keyValuePairs): DataResponse {
 
 		// Process owner transfer
 		if (sizeof($keyValuePairs) === 1 && key_exists('ownerId', $keyValuePairs)) {
+			// Only allow owner transfer if current user is the form owner
+			if ($currentUserId !== $form->getOwnerId()) {
+				$this->logger->debug('Only the form owner can transfer ownership');
+				throw new OCSForbiddenException('Only the form owner can transfer ownership');
+			}
+
 			$this->logger->debug('Updating owner: formId: {formId}, userId: {uid}', [
 				'formId' => $formId,
 				'uid' => $keyValuePairs['ownerId']
@@ -288,23 +298,55 @@ public function updateForm(int $formId, array $keyValuePairs): DataResponse {
 				throw new OCSBadRequestException('Could not find new form owner');
 			}
 
-			// update form owner
+			// update form owner and remove form lock
 			$form->setOwnerId($keyValuePairs['ownerId']);
+			$form->setLockedBy(null);
+			$form->setLockedUntil(null);
 
 			// Update changed Columns in Db.
 			$this->formMapper->update($form);
 
 			return new DataResponse($form->getOwnerId());
 		}
 
-		// Don't allow to change params id, hash, ownerId, created, lastUpdated, fileId
+		// Process form unlocking
 		if (
-			key_exists('id', $keyValuePairs) || key_exists('hash', $keyValuePairs) ||
-			key_exists('ownerId', $keyValuePairs) || key_exists('created', $keyValuePairs) ||
-			isset($keyValuePairs['fileId']) || key_exists('lastUpdated', $keyValuePairs)
+			sizeof($keyValuePairs) === 2
+			&& array_key_exists('locked_by', $keyValuePairs) && is_null($keyValuePairs['locked_by'])
+			&& array_key_exists('locked_until', $keyValuePairs) && is_null($keyValuePairs['locked_until'])
 		) {
-			$this->logger->info('Not allowed to update id, hash, ownerId, created, fileId or lastUpdated');
-			throw new OCSForbiddenException('Not allowed to update id, hash, ownerId, created, fileId or lastUpdated');
+			// Only allow form unlocking if for form owner or lock user
+			if ($currentUserId !== $form->getOwnerId() && $currentUserId !== $form->getLockedBy()) {
+				$this->logger->debug('Only the form owner or the user who obtained the lock can unlock the form');
+				throw new OCSForbiddenException('Only the form owner or the user who obtained the lock can unlock the form');
+			}
+
+			// remove form lock
+			$form->setLockedBy(null);
+			$form->setLockedUntil(null);
+
+			// Update changed Columns in Db.
+			$this->formMapper->update($form);
+
+			return new DataResponse($form->getId());
+		}
+
+		// Don't allow to change the following attributes
+		$forbiddenKeys = [
+			'id', 'hash', 'ownerId', 'created', 'lastUpdated', 'lockedBy', 'lockedUntil'
+		];
+
+		foreach ($forbiddenKeys as $key) {
+			if (array_key_exists($key, $keyValuePairs)) {
+				$this->logger->info("Not allowed to update {$key}");
+				throw new OCSForbiddenException("Not allowed to update {$key}");
+			}
+		}
+
+		// Don't allow to change fileId
+		if (isset($keyValuePairs['fileId'])) {
+			$this->logger->info('Not allowed to update fileId');
+			throw new OCSForbiddenException('Not allowed to update fileId');
 		}
 
 		// Do not allow changing showToAllUsers if disabled
@@ -477,7 +519,9 @@ public function getQuestion(int $formId, int $questionId): DataResponse {
 	#[BruteForceProtection(action: 'form')]
 	#[ApiRoute(verb: 'POST', url: '/api/v3/forms/{formId}/questions')]
 	public function newQuestion(int $formId, ?string $type = null, string $text = '', ?int $fromId = null): DataResponse {
-		$form = $this->getFormIfAllowed($formId);
+		$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
+		$this->obtainFormLock($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');
@@ -608,7 +652,9 @@ public function updateQuestion(int $formId, int $questionId, array $keyValuePair
 
 		// Make sure we query the form first to check the user has permissions
 		// So the user does not get information about "questions" if they do not even have permissions to the form
-		$form = $this->getFormIfAllowed($formId);
+		$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
+		$this->obtainFormLock($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');
@@ -682,7 +728,9 @@ public function deleteQuestion(int $formId, int $questionId): DataResponse {
 		]);
 
 
-		$form = $this->getFormIfAllowed($formId);
+		$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
+		$this->obtainFormLock($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');
@@ -748,7 +796,9 @@ public function reorderQuestions(int $formId, array $newOrder): DataResponse {
 			'newOrder' => $newOrder
 		]);
 
-		$form = $this->getFormIfAllowed($formId);
+		$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
+		$this->obtainFormLock($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');
@@ -847,7 +897,9 @@ public function newOption(int $formId, int $questionId, array $optionTexts): Dat
 			'text' => $optionTexts,
 		]);
 
-		$form = $this->getFormIfAllowed($formId);
+		$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
+		$this->obtainFormLock($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');
@@ -929,7 +981,9 @@ public function updateOption(int $formId, int $questionId, int $optionId, array
 			'keyValuePairs' => $keyValuePairs
 		]);
 
-		$form = $this->getFormIfAllowed($formId);
+		$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
+		$this->obtainFormLock($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');
@@ -996,7 +1050,9 @@ public function deleteOption(int $formId, int $questionId, int $optionId): DataR
 			'optionId' => $optionId
 		]);
 
-		$form = $this->getFormIfAllowed($formId);
+		$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
+		$this->obtainFormLock($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');
@@ -1052,7 +1108,9 @@ public function deleteOption(int $formId, int $questionId, int $optionId): DataR
 	#[BruteForceProtection(action: 'form')]
 	#[ApiRoute(verb: 'PATCH', url: '/api/v3/forms/{formId}/questions/{questionId}/options')]
 	public function reorderOptions(int $formId, int $questionId, array $newOrder) {
-		$form = $this->getFormIfAllowed($formId);
+		$form = $this->getFormIfAllowed($formId, Constants::PERMISSION_EDIT);
+		$this->obtainFormLock($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');
@@ -1797,6 +1855,12 @@ private function getFormIfAllowed(int $formId, string $permissions = 'all'): For
 					throw new NoSuchFormException('This form is not owned by the current user and user has no `results_delete` permission', Http::STATUS_FORBIDDEN);
 				}
 				break;
+			case Constants::PERMISSION_EDIT:
+				if (!$this->formsService->canEditForm($form)) {
+					$this->logger->debug('This form is not owned by the current user and user has no `edit` permission');
+					throw new NoSuchFormException('This form is not owned by the current user and user has no `edit` permission', Http::STATUS_FORBIDDEN);
+				}
+				break;
 			default:
 				// By default we request full permissions
 				if ($form->getOwnerId() !== $this->currentUser->getUID()) {
@@ -1807,4 +1871,23 @@ private function getFormIfAllowed(int $formId, string $permissions = 'all'): For
 		}
 		return $form;
 	}
+
+	/**
+	 * Locks the given form for the current user for a duration of 15 minutes.
+	 *
+	 * @param Form $form The form instance to lock.
+	 */
+	private function obtainFormLock(Form $form): void {
+		// Only lock if not locked or locked by current user, or lock has expired
+		if (
+			$form->getLockedBy() === null ||
+			$form->getLockedBy() === $this->currentUser->getUID() ||
+			$form->getLockedUntil() < time()
+		) {
+			$form->setLockedBy($this->currentUser->getUID());
+			$form->setLockedUntil(time() + 15 * 60);
+		} else {
+			throw new OCSForbiddenException('Form is currently locked by another user.');
+		}
+	}
 }

lib/Db/Form.php

@@ -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 $value)
+ * @method int getLockedUntil()
+ * @method void setLockedUntil(int $value)
  */
 class Form extends Entity {
 	protected $hash;
@@ -65,6 +69,8 @@ class Form extends Entity {
 	protected $submissionMessage;
 	protected $lastUpdated;
 	protected $state;
+	protected $lockedBy;
+	protected $lockedUntil;
 
 	/**
 	 * Form constructor.
@@ -78,6 +84,8 @@ public function __construct() {
 		$this->addType('showExpiration', 'boolean');
 		$this->addType('lastUpdated', 'integer');
 		$this->addType('state', 'integer');
+		$this->addType('locked_by', 'string');
+		$this->addType('locked_until', 'integer');
 	}
 
 	// JSON-Decoding of access-column.
@@ -149,6 +157,8 @@ public function setAccess(array $access): void {
 	 *   lastUpdated: int,
 	 *   submissionMessage: ?string,
 	 *   state: 0|1|2,
+	 *   locked_by: ?string,
+	 *   locked_until: ?int,
 	 *  }
 	 */
 	public function read() {
@@ -170,6 +180,8 @@ public function read() {
 			'lastUpdated' => (int)$this->getLastUpdated(),
 			'submissionMessage' => $this->getSubmissionMessage(),
 			'state' => $this->getState(),
+			'locked_by' => $this->getLockedBy(),
+			'locked_until' => $this->getLockedUntil(),
 		];
 	}
 }

lib/Migration/Version050200Date20250512004000.php

@@ -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 {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+		$table = $schema->getTable('forms_v2_forms');
+
+		if (!$table->hasColumn('locked_by')) {
+			$table->addColumn('locked_by', Types::STRING, [
+				'notnull' => false,
+				'default' => null,
+			]);
+		}
+
+		if (!$table->hascolumn('locked_until')) {
+			$table->addColumn('locked_until', Types::INTEGER, [
+				'notnull' => false,
+				'default' => null,
+				'comment' => 'unix-timestamp',
+			]);
+		}
+
+		return $schema;
+	}
+}

lib/ResponseDefinitions.php

@@ -118,7 +118,7 @@
  *   expires: int,
  *   fileFormat: ?string,
  *   fileId: ?int,
- *   filePath?: ?string,
+ *   filePath?: string,
  *   isAnonymous: bool,
  *   lastUpdated: int,
  *   submitMultiple: bool,
@@ -128,6 +128,8 @@
  *   permissions: list<FormsPermission>,
  *   questions: list<FormsQuestion>,
  *   state: 0|1|2,
+ *   lockedBy: ?string,
+ *   lockedUntil: ?int,
  *   shares: list<FormsShare>,
  *   submissionCount?: int,
  *   submissionMessage: ?string,