Merge pull request #3426 from nextcloud/fix/disallow-update-accessEnum

fix: disallow update accessEnum directly

Author: Chartman123

lib/Controller/ApiController.php

@@ -1875,7 +1875,7 @@ private function storeAnswersForQuestion(Form $form, $submissionId, array $quest
 	 */
 	private function checkForbiddenKeys(array $keyValuePairs): void {
 		$forbiddenKeys = [
-			'id', 'hash', 'ownerId', 'created', 'lastUpdated', 'lockedBy', 'lockedUntil'
+			'id', 'hash', 'ownerId', 'created', 'lastUpdated', 'lockedBy', 'lockedUntil', 'accessEnum'
 		];
 		foreach ($forbiddenKeys as $key) {
 			if (array_key_exists($key, $keyValuePairs)) {

tests/Integration/Api/ApiV3Test.php

@@ -701,6 +701,24 @@ public function testUpdateFormProperties(array $expected): void {
 		$this->testGetFullForm($expected);
 	}
 
+	public function testUpdateFormRejectsForbiddenKeys(): void {
+		$before = $this->OcsResponse2Data($this->http->request('GET', "api/v3/forms/{$this->testForms[0]['id']}"));
+
+		$resp = $this->http->request('PATCH', "api/v3/forms/{$this->testForms[0]['id']}", [
+			'json' => [
+				'keyValuePairs' => [
+					'id' => 999,
+				],
+			],
+			'http_errors' => false,
+		]);
+
+		$this->assertEquals(403, $resp->getStatusCode());
+
+		$after = $this->OcsResponse2Data($this->http->request('GET', "api/v3/forms/{$this->testForms[0]['id']}"));
+		$this->assertEquals($before, $after);
+	}
+
 	public function testDeleteForm() {
 		$resp = $this->http->request('DELETE', "api/v3/forms/{$this->testForms[0]['id']}");
 		$data = $this->OcsResponse2Data($resp);

tests/Unit/Controller/ApiControllerTest.php

@@ -1174,6 +1174,38 @@ public function testUpdateFormAllowsValidConfirmationEmailQuestionId(): void {
 		$this->assertSame(7, $form->getConfirmationEmailQuestionId());
 	}
 
+	public static function dataUpdateFormRejectsForbiddenKeys(): array {
+		return [
+			'id' => [['id' => 2]],
+			'hash' => [['hash' => 'new-hash']],
+			'created' => [['created' => 123456]],
+			'lastUpdated' => [['lastUpdated' => 123456]],
+			'lockedBy' => [['lockedBy' => 'anotherUser']],
+			'lockedUntil' => [['lockedUntil' => 123456]],
+			'accessEnum' => [['accessEnum' => 2]],
+		];
+	}
+
+	/**
+	 * @dataProvider dataUpdateFormRejectsForbiddenKeys
+	 */
+	public function testUpdateFormRejectsForbiddenKeys(array $keyValuePairs): void {
+		$form = new Form();
+		$form->setId(1);
+		$form->setOwnerId('currentUser');
+		$form->setHash('hash');
+
+		$this->formsService
+			->method('getFormIfAllowed')
+			->with(1, Constants::PERMISSION_EDIT)
+			->willReturn($form);
+
+		$this->formMapper->expects($this->never())->method('update');
+
+		$this->expectException(OCSForbiddenException::class);
+		$this->apiController->updateForm(1, $keyValuePairs);
+	}
+
 	public static function dataUpdateFormRejectsInvalidConfirmationEmailQuestionId(): array {
 		return [
 			'invalid-question-id' => ['Invalid question ID'],