From f0c10acbb445a0eede176c7b659b3784c9f67028 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 29 Aug 2025 19:30:59 +1200 Subject: [PATCH] Fix readonly attr stripping on write --- .../Collections/Documents/Action.php | 9 +-- .../Collections/Documents/Bulk/Update.php | 3 +- .../Collections/Documents/Bulk/Upsert.php | 1 + .../Collections/Documents/Create.php | 9 ++- .../Collections/Documents/Update.php | 11 +-- .../Collections/Documents/Upsert.php | 5 +- .../Databases/Legacy/DatabasesBase.php | 69 +++++++++++++++++++ 7 files changed, 90 insertions(+), 17 deletions(-) diff --git a/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Action.php b/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Action.php index 78df15b0c1..e077545b11 100644 --- a/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Action.php +++ b/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Action.php @@ -27,9 +27,8 @@ abstract class Action extends AppwriteAction $this->context = ROWS; } - // Use the same helper method to ensure consistency $contextId = '$' . $this->getCollectionsEventsContext() . 'Id'; - $this->removableAttributes = ['$databaseId', $contextId, '$sequence']; + $this->removableAttributes = ['$sequence', '$databaseId', $contextId]; return parent::setHttpPath($path); } @@ -200,11 +199,13 @@ abstract class Action extends AppwriteAction * Remove configured removable attributes from a document. * Used for relationship path handling to remove API-specific attributes. */ - protected function removeReadonlyAttributes(Document $document): void + protected function removeReadonlyAttributes(Document|array $document): Document|array { foreach ($this->removableAttributes as $attribute) { - $document->removeAttribute($attribute); + \var_dump('Removing attribute: ' . $attribute); + unset($document[$attribute]); } + return $document; } /** diff --git a/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Bulk/Update.php b/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Bulk/Update.php index a9f9c3f76d..226303c657 100644 --- a/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Bulk/Update.php +++ b/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Bulk/Update.php @@ -127,8 +127,7 @@ class Update extends Action } } - // Remove sequence if set - unset($document['$sequence']); + $data = $this->removeReadonlyAttributes($data); $documents = []; diff --git a/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Bulk/Upsert.php b/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Bulk/Upsert.php index a6f27637e3..6b9a81cd69 100644 --- a/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Bulk/Upsert.php +++ b/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Bulk/Upsert.php @@ -100,6 +100,7 @@ class Upsert extends Action } foreach ($documents as $key => $document) { + $document = $this->removeReadonlyAttributes($document); $documents[$key] = new Document($document); } diff --git a/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Create.php b/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Create.php index 04c90c4ec1..4bd0c6cdee 100644 --- a/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Create.php +++ b/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Create.php @@ -298,6 +298,8 @@ class Create extends Action ); foreach ($relations as &$relation) { + $relation = $this->removeReadonlyAttributes($relation); + if ( \is_array($relation) && \array_values($relation) !== $relation @@ -318,7 +320,6 @@ class Create extends Action $relation['$id'] = ID::unique(); } } else { - $this->removeReadonlyAttributes($relation); $relation->setAttribute('$collection', $relatedCollection->getId()); $type = Database::PERMISSION_UPDATE; } @@ -351,9 +352,6 @@ class Create extends Action } } - // Remove sequence if set - unset($document['$sequence']); - // Assign a unique ID if needed, otherwise use the provided ID. $document['$id'] = $sourceId === 'unique()' ? ID::unique() : $sourceId; @@ -368,10 +366,11 @@ class Create extends Action } } + $document = $this->removeReadonlyAttributes($document); + $document = new Document($document); $setPermissions($document, $permissions); $checkPermissions($collection, $document, Database::PERMISSION_CREATE); - return $document; }, $documents); diff --git a/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Update.php b/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Update.php index 334bcb8448..fd86fa995c 100644 --- a/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Update.php +++ b/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Update.php @@ -159,17 +159,14 @@ class Update extends Action $permissions = $document->getPermissions() ?? []; } - // Remove sequence if set - unset($document['$sequence']); - $data['$id'] = $documentId; $data['$permissions'] = $permissions; + $data = $this->removeReadonlyAttributes($data); $newDocument = new Document($data); $operations = 0; $setCollection = (function (Document $collection, Document $document) use (&$setCollection, $dbForProject, $database, &$operations) { - $operations++; $relationships = \array_filter( @@ -198,6 +195,8 @@ class Update extends Action ); foreach ($relations as &$relation) { + $relation = $this->removeReadonlyAttributes($relation); + // If the relation is an array it can be either update or create a child document. if ( \is_array($relation) @@ -212,7 +211,7 @@ class Update extends Action 'database_' . $database->getSequence() . '_collection_' . $relatedCollection->getSequence(), $relation->getId() )); - $this->removeReadonlyAttributes($relation); + // Attribute $collection is required for Utopia. $relation->setAttribute( '$collection', @@ -242,6 +241,8 @@ class Update extends Action ->addMetric(METRIC_DATABASES_OPERATIONS_WRITES, max($operations, 1)) ->addMetric(str_replace('{databaseInternalId}', $database->getSequence(), METRIC_DATABASE_ID_OPERATIONS_WRITES), $operations); + \var_dump($newDocument); + try { $document = $dbForProject->withRequestTimestamp( $requestTimestamp, diff --git a/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Upsert.php b/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Upsert.php index 6027a20c41..622b43db2a 100644 --- a/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Upsert.php +++ b/src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Upsert.php @@ -170,6 +170,7 @@ class Upsert extends Action $data['$id'] = $documentId; $data['$permissions'] = $permissions ?? []; + $data = $this->removeReadonlyAttributes($data); $newDocument = new Document($data); $operations = 0; @@ -203,6 +204,8 @@ class Upsert extends Action ); foreach ($relations as &$relation) { + $relation = $this->removeReadonlyAttributes($relation); + // If the relation is an array it can be either update or create a child document. if ( \is_array($relation) @@ -217,7 +220,7 @@ class Upsert extends Action 'database_' . $database->getSequence() . '_collection_' . $relatedCollection->getSequence(), $relation->getId() )); - $this->removeReadonlyAttributes($relation); + // Attribute $collection is required for Utopia. $relation->setAttribute( '$collection', diff --git a/tests/e2e/Services/Databases/Legacy/DatabasesBase.php b/tests/e2e/Services/Databases/Legacy/DatabasesBase.php index bd71272537..0df0a2f5af 100644 --- a/tests/e2e/Services/Databases/Legacy/DatabasesBase.php +++ b/tests/e2e/Services/Databases/Legacy/DatabasesBase.php @@ -1697,6 +1697,7 @@ trait DatabasesBase return $data; } + /** * @depends testCreateIndexes */ @@ -2211,6 +2212,49 @@ trait DatabasesBase $this->assertArrayHasKey('$permissions', $library3['body']); $this->assertCount(3, $library3['body']['$permissions']); $this->assertNotEmpty($library3['body']['$permissions']); + + // Readonly attributes are ignored + $personNoPerm = $this->client->call(Client::METHOD_PUT, '/databases/' . $databaseId . '/collections/' . $person['body']['$id'] . '/documents/' . $newPersonId, array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'data' => [ + '$id' => 'some-other-id', + '$collectionId' => 'some-other-collection', + '$databaseId' => 'some-other-database', + '$createdAt' => '2024-01-01T00:00:00Z', + '$updatedAt' => '2024-01-01T00:00:00Z', + 'library' => [ + '$id' => 'library3', + 'libraryName' => 'Library 3', + '$createdAt' => '2024-01-01T00:00:00Z', + '$updatedAt' => '2024-01-01T00:00:00Z', + ], + ], + ]); + + $update = $personNoPerm; + $update['body']['$id'] = 'random'; + $update['body']['$sequence'] = 123; + $update['body']['$databaseId'] = 'random'; + $update['body']['$collectionId'] = 'random'; + $update['body']['$createdAt'] = '2024-01-01T00:00:00Z'; + $update['body']['$updatedAt'] = '2024-01-01T00:00:00Z'; + + $upserted = $this->client->call(Client::METHOD_PUT, '/databases/' . $databaseId . '/collections/' . $person['body']['$id'] . '/documents/' . $newPersonId, array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'data' => $update['body'] + ]); + + $this->assertEquals(200, $upserted['headers']['status-code']); + $this->assertEquals($personNoPerm['body']['$id'], $upserted['body']['$id']); + $this->assertEquals($personNoPerm['body']['$collectionId'], $upserted['body']['$collectionId']); + $this->assertEquals($personNoPerm['body']['$databaseId'], $upserted['body']['$databaseId']); + $this->assertEquals($personNoPerm['body']['$sequence'], $upserted['body']['$sequence']); + $this->assertEquals($personNoPerm['body']['$createdAt'], $upserted['body']['$createdAt']); + $this->assertNotEquals('2024-01-01T00:00:00Z', $upserted['body']['$updatedAt']); } } @@ -3000,6 +3044,31 @@ trait DatabasesBase $this->assertEquals(200, $response['headers']['status-code']); + // Test readonly attributes are ignored + $response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $databaseId . '/collections/' . $data['moviesId'] . '/documents/' . $id, array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-timestamp' => DateTime::formatTz(DateTime::now()), + ], $this->getHeaders()), [ + 'data' => [ + '$id' => 'newId', + '$sequence' => 9999, + '$collectionId' => 'newCollectionId', + '$databaseId' => 'newDatabaseId', + '$createdAt' => '2024-01-01T00:00:00+00:00', + '$updatedAt' => '2024-01-01T00:00:00+00:00', + 'title' => 'Thor: Ragnarok', + ], + ]); + + $this->assertEquals(200, $response['headers']['status-code']); + $this->assertEquals($id, $response['body']['$id']); + $this->assertEquals($data['moviesId'], $response['body']['$collectionId']); + $this->assertEquals($databaseId, $response['body']['$databaseId']); + $this->assertNotEquals('2024-01-01T00:00:00+00:00', $response['body']['$createdAt']); + $this->assertNotEquals('2024-01-01T00:00:00+00:00', $response['body']['$updatedAt']); + $this->assertNotEquals(9999, $response['body']['$sequence']); + return []; }