From 0defa9c830c074d5485ef113cb1f826a8f243a7a Mon Sep 17 00:00:00 2001 From: Christian Hartmann Date: Tue, 16 Jun 2026 14:01:52 +0000 Subject: [PATCH] fix: disallow update accessEnum directly Signed-off-by: GitHub --- lib/Controller/ApiController.php | 2 +- tests/Integration/Api/ApiV3Test.php | 18 ++++++++++++ tests/Unit/Controller/ApiControllerTest.php | 32 +++++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 2494afbab..09a40d241 100644 --- a/lib/Controller/ApiController.php +++ b/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)) { diff --git a/tests/Integration/Api/ApiV3Test.php b/tests/Integration/Api/ApiV3Test.php index c90ef7bb0..36e98f028 100644 --- a/tests/Integration/Api/ApiV3Test.php +++ b/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); diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php index 49f0d2ad3..79d908675 100644 --- a/tests/Unit/Controller/ApiControllerTest.php +++ b/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'],