Merge pull request #3492 from nextcloud/fix/1854-missing-formid-uploadedfileid

Chartman123 · web-flow · commit 9014225c3664 · 2026-07-15T14:35:56.000+02:00
fix: improve user validation on file question type
diff --git a/docs/API_v3.md b/docs/API_v3.md
@@ -54,7 +54,7 @@ This file contains the API-Documentation. For more information on the returned D
     - `GET /api/v3/forms/{formId}/questions/{questionId}` to get a single question
     - `POST /api/v3/forms/{formId}/questions/{questionId}/options` does now accept more options at once
     - `PATCH /api/v3/forms/{formId}/questions/{questionId}/options` to reorder the options (request body `newOrder`, optional `optionType`)
-    - `POST /api/v3/forms/{formId}/submissions/files/{questionId}` to upload a file to a file question before submitting the form
+    - `POST /api/v3/forms/{formId}/submissions/files/{questionId}` to upload a file to a file question before submitting the form (response includes `uploadToken`; submissions must include it with `uploadedFileId`)
     - `GET /api/v3/forms/{formId}/submissions/{submissionId}` to get a single submission
     - `PUT /api/v3/forms/{formId}/submissions/{submissionId}` to update an existing submission
 - In API version 2.5 the following endpoints were introduced:
@@ -858,25 +858,34 @@ Delete all Submissions to a form
 
 ### Upload a file
 
-Upload a file to an answer before form submission
+Upload a file to a file question before form submission. Each uploaded file is stored temporarily and bound to the given `formId` and `questionId`. The response includes an `uploadToken` that must be sent back when submitting the form, together with `uploadedFileId`.
 
 - Endpoint: `/api/v3/forms/{formId}/submissions/files/{questionId}`
 - Method: `POST`
 - Url-Parameters:
   | Parameter | Type | Description |
   |--------------|----------------|-------------|
   | _formId_ | Integer | ID of the form to upload the file to |
-  | _questionId_ | Integer | ID of the question to upload the file to |
+  | _questionId_ | Integer | ID of the file question to upload the file to |
 - Parameters:
   | Parameter | Type | Description |
   |--------------|----------------|-------------|
   | _files_ | Array of files | Files to upload |
-- Response: **Status-Code OK**, as well as the id of the uploaded file and it's name.
+  | _shareHash_ | String | optional, only necessary for uploads on a public share link |
+- Response: **Status-Code OK**, as well as a list of uploaded files.
 
 ```
-"data": {"uploadedFileId": integer, "fileName": "string"}
+"data": [
+  {
+    "uploadedFileId": 42,
+    "fileName": "document.pdf",
+    "uploadToken": "a1b2c3d4e5f6..."
+  }
+]
 ```
 
+When submitting the form, each file answer must include both `uploadedFileId` and `uploadToken` from this response for the same `questionId`. Uploads from other forms, questions, or sessions are rejected.
+
 ### Get a specific submission
 
 Get a specific submission of a form. Viewing another user's submission requires the `results` permission; otherwise only the submission owner may view it.
@@ -933,15 +942,15 @@ Store Submission to Database
     - QuestionID as key
     - An **array** of values as value --> Even for short Text Answers, wrapped into Array.
     - For Question-Types with pre-defined answers (`multiple`, `multiple_unique`, `dropdown`), the array contains the corresponding option-IDs.
-    - For File-Uploads, the array contains the objects with key `uploadedFileId` (value from Upload a file endpoint).
+    - For File-Uploads, the array contains objects with `uploadedFileId` and `uploadToken` (both from the [Upload a file] endpoint for the same `questionId`).
 
 ```
   {
     "1":[27,32],              // dropdown or multiple
     "2":["ShortTextAnswer"],  // All Text-Based Question-Types
     "3":[                     // File-Upload
-      {"uploadedFileId": integer},
-      {"uploadedFileId": integer}
+      {"uploadedFileId": 42, "uploadToken": "a1b2c3d4e5f6..."},
+      {"uploadedFileId": 43, "uploadToken": "f6e5d4c3b2a1..."}
   ],
 }
 ```
diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php
@@ -53,6 +53,7 @@
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\IUserSession;
+use OCP\Security\ISecureRandom;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -90,6 +91,7 @@ public function __construct(
 		private readonly UploadedFileMapper $uploadedFileMapper,
 		private readonly IMimeTypeDetector $mimeTypeDetector,
 		private readonly IJobList $jobList,
+		private readonly ISecureRandom $secureRandom,
 	) {
 		parent::__construct($appName, $request);
 		$this->currentUser = $userSession->getUser();
@@ -1428,7 +1430,7 @@ public function newSubmission(int $formId, array $answers, string $shareHash = '
 		$questions = $this->formsService->getQuestions($formId);
 		try {
 			// Is the submission valid
-			$this->submissionService->validateSubmission($questions, $answers, $form->getOwnerId());
+			$this->submissionService->validateSubmission($questions, $answers, $form->getOwnerId(), $formId);
 		} catch (\InvalidArgumentException $e) {
 			throw new OCSBadRequestException($e->getMessage());
 		}
@@ -1524,7 +1526,7 @@ public function updateSubmission(int $formId, int $submissionId, array $answers)
 		$questions = $this->formsService->getQuestions($formId);
 		try {
 			// Is the submission valid
-			$this->submissionService->validateSubmission($questions, $answers, $form->getOwnerId());
+			$this->submissionService->validateSubmission($questions, $answers, $form->getOwnerId(), $formId);
 		} catch (\InvalidArgumentException $e) {
 			throw new OCSBadRequestException($e->getMessage());
 		}
@@ -1773,8 +1775,11 @@ public function uploadFiles(int $formId, int $questionId, string $shareHash = ''
 			$fileName = $folder->getNonExistingName($uploadedFile['name']);
 			$file = $folder->newFile($fileName, file_get_contents($uploadedFile['tmp_name']));
 
+			$uploadToken = $this->secureRandom->generate(32, ISecureRandom::CHAR_ALPHANUMERIC);
 			$uploadedFileEntity = new UploadedFile();
 			$uploadedFileEntity->setFormId($formId);
+			$uploadedFileEntity->setQuestionId($questionId);
+			$uploadedFileEntity->setUploadToken($uploadToken);
 			$uploadedFileEntity->setOriginalFileName($fileName);
 			$uploadedFileEntity->setFileId($file->getId());
 			$uploadedFileEntity->setCreated(time());
@@ -1783,6 +1788,7 @@ public function uploadFiles(int $formId, int $questionId, string $shareHash = ''
 			$response[] = [
 				'uploadedFileId' => $uploadedFileEntity->getId(),
 				'fileName' => $fileName,
+				'uploadToken' => $uploadToken,
 			];
 		}
 
@@ -1797,7 +1803,7 @@ public function uploadFiles(int $formId, int $questionId, string $shareHash = ''
 	 * @param Form $form
 	 * @param int $submissionId
 	 * @param array $question
-	 * @param string[]|array<array{uploadedFileId: string, uploadedFileName: string}> $answerArray
+	 * @param string[]|array<array{uploadedFileId: string, fileName?: string, uploadToken: string}> $answerArray
 	 */
 	private function storeAnswersForQuestion(Form $form, $submissionId, array $question, array $answerArray): void {
 		if ($question['type'] === Constants::ANSWER_TYPE_GRID || $question['type'] === Constants::ANSWER_TYPE_RANKING) {
@@ -1833,7 +1839,12 @@ private function storeAnswersForQuestion(Form $form, $submissionId, array $quest
 					$answerText = str_replace(Constants::QUESTION_EXTRASETTINGS_OTHER_PREFIX, '', $answer);
 				}
 			} elseif ($question['type'] === Constants::ANSWER_TYPE_FILE) {
-				$uploadedFile = $this->uploadedFileMapper->getByUploadedFileId($answer['uploadedFileId']);
+				$uploadedFile = $this->uploadedFileMapper->getForSubmission(
+					(int)$answer['uploadedFileId'],
+					$form->getId(),
+					$question['id'],
+					$answer['uploadToken'] ?? '',
+				);
 				$answerEntity->setFileId($uploadedFile->getFileId());
 
 				$userFolder = $this->rootFolder->getUserFolder($form->getOwnerId());
diff --git a/lib/Db/UploadedFile.php b/lib/Db/UploadedFile.php
@@ -14,6 +14,10 @@
 /**
  * @method int getFormId()
  * @method void setFormId(int $value)
+ * @method ?int getQuestionId()
+ * @method void setQuestionId(?int $value)
+ * @method ?string getUploadToken()
+ * @method void setUploadToken(?string $value)
  * @method string getOriginalFileName()
  * @method void setOriginalFileName(string $value)
  * @method int getFileId()
@@ -23,6 +27,8 @@
  */
 class UploadedFile extends Entity {
 	protected $formId;
+	protected ?int $questionId = null;
+	protected ?string $uploadToken = null;
 	protected $originalFileName;
 	protected $fileId;
 	protected $created;
@@ -32,6 +38,8 @@ class UploadedFile extends Entity {
 	 */
 	public function __construct() {
 		$this->addType('formId', 'integer');
+		$this->addType('questionId', 'integer');
+		$this->addType('uploadToken', 'string');
 		$this->addType('originalFileName', 'string');
 		$this->addType('fileId', 'integer');
 		$this->addType('created', 'integer');
@@ -41,6 +49,7 @@ public function read(): array {
 		return [
 			'id' => $this->getId(),
 			'formId' => $this->getFormId(),
+			'questionId' => $this->getQuestionId(),
 			'originalFileName' => $this->getOriginalFileName(),
 			'fileId' => $this->getFileId(),
 			'created' => $this->getCreated(),
diff --git a/lib/Db/UploadedFileMapper.php b/lib/Db/UploadedFileMapper.php
@@ -55,6 +55,30 @@ public function getByUploadedFileId(string $uploadedFileId): UploadedFile {
 		return $uploadedFile;
 	}
 
+	/**
+	 * find all uploaded files for a given form and question and compare with the given upload token.
+	 *
+	 * @throws DoesNotExistException
+	 */
+	public function getForSubmission(int $uploadedFileId, int $formId, int $questionId, string $uploadToken): UploadedFile {
+		$qb = $this->db->getQueryBuilder();
+
+		$qb->select('*')
+			->from($this->getTableName())
+			->where($qb->expr()->eq('id', $qb->createNamedParameter($uploadedFileId, IQueryBuilder::PARAM_INT)))
+			->andWhere($qb->expr()->eq('form_id', $qb->createNamedParameter($formId, IQueryBuilder::PARAM_INT)))
+			->andWhere($qb->expr()->eq('question_id', $qb->createNamedParameter($questionId, IQueryBuilder::PARAM_INT)));
+
+		$uploadedFile = $this->findEntities($qb)[0] ?? null;
+		if ($uploadedFile === null
+			|| $uploadedFile->getUploadToken() === null
+			|| !hash_equals($uploadedFile->getUploadToken(), $uploadToken)) {
+			throw new DoesNotExistException(sprintf('Uploaded file with id "%d" not found', $uploadedFileId));
+		}
+
+		return $uploadedFile;
+	}
+
 	/**
 	 * @param \DateTimeImmutable $dateTime
 	 * @return UploadedFile[]
diff --git a/lib/Migration/Version050300Date20260713180000.php b/lib/Migration/Version050300Date20260713180000.php
@@ -0,0 +1,56 @@
+<?php
+
+declare(strict_types = 1);
+
+/**
+ * SPDX-FileCopyrightText: 2026 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;
+use Override;
+
+class Version050300Date20260713180000 extends SimpleMigrationStep {
+
+	/**
+	 * @param IOutput $output
+	 * @param Closure(): ISchemaWrapper $schemaClosure
+	 * @param array $options
+	 * @return null|ISchemaWrapper
+	 */
+	#[Override]
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+
+		if (!$schema->hasTable('forms_v2_uploaded_files')) {
+			return null;
+		}
+
+		$table = $schema->getTable('forms_v2_uploaded_files');
+		$changed = false;
+
+		if (!$table->hasColumn('question_id')) {
+			$table->addColumn('question_id', Types::INTEGER, [
+				'notnull' => false,
+			]);
+			$changed = true;
+		}
+
+		if (!$table->hasColumn('upload_token')) {
+			$table->addColumn('upload_token', Types::STRING, [
+				'notnull' => false,
+				'length' => 64,
+			]);
+			$changed = true;
+		}
+
+		return $changed ? $schema : null;
+	}
+}
diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php
@@ -151,7 +151,8 @@
  *
  * @psalm-type FormsUploadedFile = array{
  *   uploadedFileId: int,
- *   fileName: string
+ *   fileName: string,
+ *   uploadToken: string,
  * }
  */
 class ResponseDefinitions {
diff --git a/lib/Service/SubmissionService.php b/lib/Service/SubmissionService.php
@@ -470,9 +470,10 @@ private function escapeCSV(string $value): string {
 	 * @param array $questions Array of the questions of the form
 	 * @param array $answers Array of the submitted answers
 	 * @param string $formOwnerId Owner of the form
+	 * @param int $formId Id of the form being submitted
 	 * @throw \InvalidArgumentException if validation failed
 	 */
-	public function validateSubmission(array $questions, array $answers, string $formOwnerId): void {
+	public function validateSubmission(array $questions, array $answers, string $formOwnerId, int $formId): void {
 		// Check by questions
 		foreach ($questions as $question) {
 			$questionId = $question['id'];
@@ -485,7 +486,7 @@ public function validateSubmission(array $questions, array $answers, string $for
 					if (is_array($value)) {
 						// file type
 						if (isset($value['uploadedFileId'])) {
-							return !empty($value['uploadedFileId']);
+							return !empty($value['uploadedFileId']) && !empty($value['uploadToken']);
 						}
 
 						// Grid questions
@@ -613,8 +614,14 @@ public function validateSubmission(array $questions, array $answers, string $for
 				}
 
 				foreach ($answers[$questionId] as $answer) {
-					$uploadedFile = $this->uploadedFileMapper->findByUploadedFileId($answer['uploadedFileId']);
-					if (!$uploadedFile) {
+					try {
+						$uploadedFile = $this->uploadedFileMapper->getForSubmission(
+							(int)$answer['uploadedFileId'],
+							$formId,
+							$questionId,
+							$answer['uploadToken'] ?? '',
+						);
+					} catch (DoesNotExistException) {
 						throw new \InvalidArgumentException(sprintf('File "%s" for question "%s" not exists anymore. Please delete and re-upload the file.', $answer['fileName'] ?? $answer['uploadedFileId'], $question['text']));
 					}
 
diff --git a/openapi.json b/openapi.json
@@ -703,7 +703,8 @@
                 "type": "object",
                 "required": [
                     "uploadedFileId",
-                    "fileName"
+                    "fileName",
+                    "uploadToken"
                 ],
                 "properties": {
                     "uploadedFileId": {
@@ -712,6 +713,9 @@
                     },
                     "fileName": {
                         "type": "string"
+                    },
+                    "uploadToken": {
+                        "type": "string"
                     }
                 }
             }
diff --git a/tests/Integration/Api/ApiV3Test.php b/tests/Integration/Api/ApiV3Test.php
@@ -780,12 +780,13 @@ public function testDeleteFormWithSubmissions() {
 
 		$uploadedFileData = $this->OcsResponse2Data($uploadedFileResponse);
 		$uploadedFileId = $uploadedFileData[0]['uploadedFileId'];
+		$uploadToken = $uploadedFileData[0]['uploadToken'];
 
 		// Create a submission with the file
 		$resp = $this->http->request('POST', "api/v3/forms/$formId/submissions", [
 			'json' => [
 				'answers' => [
-					$questionId => [['uploadedFileId' => $uploadedFileId]]
+					$questionId => [['uploadedFileId' => $uploadedFileId, 'uploadToken' => $uploadToken]]
 				]
 			]
 		]);
@@ -1403,13 +1404,14 @@ public function testDeleteSubmissionWithFiles() {
 
 		$uploadedFileData = $this->OcsResponse2Data($uploadedFileResponse);
 		$uploadedFileId = $uploadedFileData[0]['uploadedFileId'];
+		$uploadToken = $uploadedFileData[0]['uploadToken'];
 
 		// Create a new submission with the file
 		$resp = $this->http->request('POST', "api/v3/forms/{$this->testForms[0]['id']}/submissions", [
 			'json' => [
 				'answers' => [
 					$this->testForms[0]['questions'][0]['id'] => ['Test Answer'],
-					$this->testForms[0]['questions'][2]['id'] => [['uploadedFileId' => $uploadedFileId]]
+					$this->testForms[0]['questions'][2]['id'] => [['uploadedFileId' => $uploadedFileId, 'uploadToken' => $uploadToken]]
 				]
 			]
 		]);
@@ -1518,6 +1520,7 @@ public function testNewSubmission() {
 
 		$data = $this->OcsResponse2Data($uploadedFileResponse);
 		$uploadedFileId = $data[0]['uploadedFileId'];
+		$uploadToken = $data[0]['uploadToken'];
 
 		$resp = $this->http->request('POST', "api/v3/forms/{$this->testForms[0]['id']}/submissions", [
 			'json' => [
@@ -1526,7 +1529,7 @@ public function testNewSubmission() {
 					$this->testForms[0]['questions'][1]['id'] => [
 						$this->testForms[0]['questions'][1]['options'][0]['id']
 					],
-					$this->testForms[0]['questions'][2]['id'] => [['uploadedFileId' => $uploadedFileId]]
+					$this->testForms[0]['questions'][2]['id'] => [['uploadedFileId' => $uploadedFileId, 'uploadToken' => $uploadToken]]
 				]
 			]
 		]);
diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php
diff --git a/tests/Unit/Service/SubmissionServiceTest.php b/tests/Unit/Service/SubmissionServiceTest.php

Original file line number	Diff line number	Diff line change
`@@ -703,7 +703,8 @@`
`703`	`703`	`"type": "object",`
`704`	`704`	`"required": [`
`705`	`705`	`"uploadedFileId",`
`706`		`- "fileName"`
	`706`	`+ "fileName",`
	`707`	`+ "uploadToken"`
`707`	`708`	`],`
`708`	`709`	`"properties": {`
`709`	`710`	`"uploadedFileId": {`
`@@ -712,6 +713,9 @@`
`712`	`713`	`},`
`713`	`714`	`"fileName": {`
`714`	`715`	`"type": "string"`
	`716`	`+ },`
	`717`	`+ "uploadToken": {`
	`718`	`+ "type": "string"`
`715`	`719`	`}`
`716`	`720`	`}`
`717`	`721`	`}`