Skip to content

Commit e005389

Browse files
abnegateabnegate
authored
Merge pull request #10405 from appwrite/fix-readonly-on-write
Fix readonly attr stripping on write
2 parents 7f59872 + 1e230a9 commit e005389

13 files changed

Lines changed: 133 additions & 369 deletions

File tree

‎src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Action.php‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,18 @@ public function setHttpPath(string $path): AppwriteAction
2727
$this->context = ROWS;
2828
}
2929

30-
// Use the same helper method to ensure consistency
3130
$contextId = '$' . $this->getCollectionsEventsContext() . 'Id';
32-
$this->removableAttributes = ['$databaseId', $contextId, '$sequence'];
31+
$this->removableAttributes = [
32+
'*' => [
33+
'$sequence',
34+
'$databaseId',
35+
$contextId,
36+
],
37+
'privileged' => [
38+
'$createdAt',
39+
'$updatedAt',
40+
],
41+
];
3342

3443
return parent::setHttpPath($path);
3544
}
@@ -200,11 +209,19 @@ protected function getCollectionsEventsContext(): string
200209
* Remove configured removable attributes from a document.
201210
* Used for relationship path handling to remove API-specific attributes.
202211
*/
203-
protected function removeReadonlyAttributes(Document $document): void
204-
{
205-
foreach ($this->removableAttributes as $attribute) {
206-
$document->removeAttribute($attribute);
212+
protected function removeReadonlyAttributes(
213+
Document|array $document,
214+
bool $privileged = false,
215+
): Document|array {
216+
foreach ($this->removableAttributes['*'] as $attribute) {
217+
unset($document[$attribute]);
218+
}
219+
if (!$privileged) {
220+
foreach ($this->removableAttributes['privileged'] ?? [] as $attribute) {
221+
unset($document[$attribute]);
222+
}
207223
}
224+
return $document;
208225
}
209226

210227
/**

‎src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Bulk/Update.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,7 @@ public function action(string $databaseId, string $collectionId, string|array $d
127127
}
128128
}
129129

130-
// Remove sequence if set
131-
unset($document['$sequence']);
130+
$data = $this->removeReadonlyAttributes($data, privileged: true);
132131

133132
$documents = [];
134133

‎src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Bulk/Upsert.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ public function action(string $databaseId, string $collectionId, array $document
100100
}
101101

102102
foreach ($documents as $key => $document) {
103+
$document = $this->removeReadonlyAttributes($document, privileged: true);
103104
$documents[$key] = new Document($document);
104105
}
105106

‎src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Create.php‎

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ public function action(string $databaseId, string $documentId, string $collectio
254254

255255
$operations = 0;
256256

257-
$checkPermissions = function (Document $collection, Document $document, string $permission) use (&$checkPermissions, $dbForProject, $database, &$operations) {
257+
$checkPermissions = function (Document $collection, Document $document, string $permission) use ($isAPIKey, $isPrivilegedUser, &$checkPermissions, $dbForProject, $database, &$operations) {
258258
$operations++;
259259

260260
$documentSecurity = $collection->getAttribute('documentSecurity', false);
@@ -307,6 +307,8 @@ public function action(string $databaseId, string $documentId, string $collectio
307307
$relation = new Document($relation);
308308
}
309309
if ($relation instanceof Document) {
310+
$relation = $this->removeReadonlyAttributes($relation, $isAPIKey || $isPrivilegedUser);
311+
310312
$current = Authorization::skip(
311313
fn () => $dbForProject->getDocument('database_' . $database->getSequence() . '_collection_' . $relatedCollection->getSequence(), $relation->getId())
312314
);
@@ -318,7 +320,6 @@ public function action(string $databaseId, string $documentId, string $collectio
318320
$relation['$id'] = ID::unique();
319321
}
320322
} else {
321-
$this->removeReadonlyAttributes($relation);
322323
$relation->setAttribute('$collection', $relatedCollection->getId());
323324
$type = Database::PERMISSION_UPDATE;
324325
}
@@ -351,27 +352,12 @@ public function action(string $databaseId, string $documentId, string $collectio
351352
}
352353
}
353354

354-
// Remove sequence if set
355-
unset($document['$sequence']);
356-
357355
// Assign a unique ID if needed, otherwise use the provided ID.
358356
$document['$id'] = $sourceId === 'unique()' ? ID::unique() : $sourceId;
359-
360-
// Allowing to add createdAt and updatedAt timestamps if server side(api key
361-
if (!$isAPIKey && !$isPrivilegedUser) {
362-
if (isset($document['$createdAt'])) {
363-
throw new Exception($this->getInvalidStructureException(), 'Attribute "$createdAt" can not be modified. Please use a server SDK with an API key to modify server attributes.');
364-
}
365-
366-
if (isset($document['$updatedAt'])) {
367-
throw new Exception($this->getInvalidStructureException(), 'Attribute "$updatedAt" can not be modified. Please use a server SDK with an API key to modify server attributes.');
368-
}
369-
}
370-
357+
$document = $this->removeReadonlyAttributes($document, $isAPIKey || $isPrivilegedUser);
371358
$document = new Document($document);
372359
$setPermissions($document, $permissions);
373360
$checkPermissions($collection, $document, Database::PERMISSION_CREATE);
374-
375361
return $document;
376362
}, $documents);
377363

‎src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Delete.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ public function action(string $databaseId, string $collectionId, string $documen
117117
}
118118

119119
$collectionsCache = [];
120+
120121
$this->processDocument(
121122
database: $database,
122123
collection: $collection,

‎src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Update.php‎

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -109,16 +109,6 @@ public function action(string $databaseId, string $collectionId, string $documen
109109
throw new Exception($this->getParentNotFoundException());
110110
}
111111

112-
// Allowing to add createdAt and updatedAt timestamps if server side(api key)
113-
if (!$isAPIKey && !$isPrivilegedUser) {
114-
if (isset($data['$createdAt'])) {
115-
throw new Exception($this->getInvalidStructureException(), 'Attribute "$createdAt" can not be modified. Please use a server SDK with an API key to modify server attributes.');
116-
}
117-
118-
if (isset($data['$updatedAt'])) {
119-
throw new Exception($this->getInvalidStructureException(), 'Attribute "$updatedAt" can not be modified. Please use a server SDK with an API key to modify server attributes.');
120-
}
121-
}
122112
// Read permission should not be required for update
123113
/** @var Document $document */
124114
$document = Authorization::skip(fn () => $dbForProject->getDocument('database_' . $database->getSequence() . '_collection_' . $collection->getSequence(), $documentId));
@@ -159,17 +149,14 @@ public function action(string $databaseId, string $collectionId, string $documen
159149
$permissions = $document->getPermissions() ?? [];
160150
}
161151

162-
// Remove sequence if set
163-
unset($document['$sequence']);
164-
165152
$data['$id'] = $documentId;
166153
$data['$permissions'] = $permissions;
154+
$data = $this->removeReadonlyAttributes($data, $isAPIKey || $isPrivilegedUser);
167155
$newDocument = new Document($data);
168156

169157
$operations = 0;
170158

171-
$setCollection = (function (Document $collection, Document $document) use (&$setCollection, $dbForProject, $database, &$operations) {
172-
159+
$setCollection = (function (Document $collection, Document $document) use ($isAPIKey, $isPrivilegedUser, &$setCollection, $dbForProject, $database, &$operations) {
173160
$operations++;
174161

175162
$relationships = \array_filter(
@@ -208,11 +195,13 @@ public function action(string $databaseId, string $collectionId, string $documen
208195
$relation = new Document($relation);
209196
}
210197
if ($relation instanceof Document) {
198+
$relation = $this->removeReadonlyAttributes($relation, $isAPIKey || $isPrivilegedUser);
199+
211200
$oldDocument = Authorization::skip(fn () => $dbForProject->getDocument(
212201
'database_' . $database->getSequence() . '_collection_' . $relatedCollection->getSequence(),
213202
$relation->getId()
214203
));
215-
$this->removeReadonlyAttributes($relation);
204+
216205
// Attribute $collection is required for Utopia.
217206
$relation->setAttribute(
218207
'$collection',
@@ -242,6 +231,8 @@ public function action(string $databaseId, string $collectionId, string $documen
242231
->addMetric(METRIC_DATABASES_OPERATIONS_WRITES, max($operations, 1))
243232
->addMetric(str_replace('{databaseInternalId}', $database->getSequence(), METRIC_DATABASE_ID_OPERATIONS_WRITES), $operations);
244233

234+
\var_dump($newDocument);
235+
245236
try {
246237
$document = $dbForProject->withRequestTimestamp(
247238
$requestTimestamp,

‎src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Upsert.php‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,8 @@ public function action(string $databaseId, string $collectionId, string $documen
121121
];
122122

123123
$permissions = Permission::aggregate($permissions, $allowedPermissions);
124-
// if no permission, upsert permission from the old document if present (update scenario) else add default permission (create scenario)
124+
125+
// If no permission, upsert permission from the old document if present (update scenario) else add default permission (create scenario)
125126
if (\is_null($permissions)) {
126127
$oldDocument = Authorization::skip(fn () => $dbForProject->getDocument('database_' . $database->getSequence() . '_collection_' . $collection->getSequence(), $documentId));
127128
if ($oldDocument->isEmpty()) {
@@ -157,24 +158,14 @@ public function action(string $databaseId, string $collectionId, string $documen
157158
}
158159
}
159160
}
160-
// Allowing to add createdAt and updatedAt timestamps if server side(api key)
161-
if (!$isAPIKey && !$isPrivilegedUser) {
162-
if (isset($data['$createdAt'])) {
163-
throw new Exception($this->getInvalidStructureException(), 'Attribute "$createdAt" can not be modified. Please use a server SDK with an API key to modify server attributes.');
164-
}
165-
166-
if (isset($data['$updatedAt'])) {
167-
throw new Exception($this->getInvalidStructureException(), 'Attribute "$updatedAt" can not be modified. Please use a server SDK with an API key to modify server attributes.');
168-
}
169-
}
170161

171162
$data['$id'] = $documentId;
172163
$data['$permissions'] = $permissions ?? [];
164+
$data = $this->removeReadonlyAttributes($data, $isAPIKey || $isPrivilegedUser);
173165
$newDocument = new Document($data);
174166
$operations = 0;
175167

176-
$setCollection = (function (Document $collection, Document $document) use (&$setCollection, $dbForProject, $database, &$operations) {
177-
168+
$setCollection = (function (Document $collection, Document $document) use ($isAPIKey, $isPrivilegedUser, &$setCollection, $dbForProject, $database, &$operations) {
178169
$operations++;
179170

180171
$relationships = \array_filter(
@@ -213,11 +204,13 @@ public function action(string $databaseId, string $collectionId, string $documen
213204
$relation = new Document($relation);
214205
}
215206
if ($relation instanceof Document) {
207+
$relation = $this->removeReadonlyAttributes($relation, $isAPIKey || $isPrivilegedUser);
208+
216209
$oldDocument = Authorization::skip(fn () => $dbForProject->getDocument(
217210
'database_' . $database->getSequence() . '_collection_' . $relatedCollection->getSequence(),
218211
$relation->getId()
219212
));
220-
$this->removeReadonlyAttributes($relation);
213+
221214
// Attribute $collection is required for Utopia.
222215
$relation->setAttribute(
223216
'$collection',

‎src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/XList.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ public function action(string $databaseId, string $collectionId, array $queries,
176176
}
177177

178178
// Check which removable attributes are explicitly requested
179-
foreach ($this->removableAttributes as $attribute) {
179+
foreach ($this->removableAttributes['*'] as $attribute) {
180180
if (\in_array($attribute, $values, true)) {
181181
$requestedAttributes[$attribute] = true;
182182
}
@@ -186,7 +186,7 @@ public function action(string $databaseId, string $collectionId, array $queries,
186186
if (!$hasWildcard) {
187187
foreach ($documents as $document) {
188188
// Remove attributes that are not explicitly requested
189-
foreach ($this->removableAttributes as $attribute) {
189+
foreach ($this->removableAttributes['*'] as $attribute) {
190190
if (!isset($requestedAttributes[$attribute])) {
191191
$document->removeAttribute($attribute);
192192
}

‎src/Appwrite/Platform/Workers/Migrations.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,6 @@ protected function updateMigrationDocument(Document $migration, Document $projec
212212
// set the errors back without trace
213213
$clonedMigrationDocument->setAttribute('errors', $errorMessages);
214214

215-
216215
/** Trigger Realtime Events */
217216
$queueForRealtime
218217
->setProject($project)

‎tests/e2e/Services/Databases/Legacy/DatabasesBase.php‎

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1697,6 +1697,7 @@ public function testCreateDocument(array $data): array
16971697
return $data;
16981698
}
16991699

1700+
17001701
/**
17011702
* @depends testCreateIndexes
17021703
*/
@@ -2211,6 +2212,55 @@ public function testUpsertDocument(array $data): void
22112212
$this->assertArrayHasKey('$permissions', $library3['body']);
22122213
$this->assertCount(3, $library3['body']['$permissions']);
22132214
$this->assertNotEmpty($library3['body']['$permissions']);
2215+
2216+
// Readonly attributes are ignored
2217+
$personNoPerm = $this->client->call(Client::METHOD_PUT, '/databases/' . $databaseId . '/collections/' . $person['body']['$id'] . '/documents/' . $newPersonId, array_merge([
2218+
'content-type' => 'application/json',
2219+
'x-appwrite-project' => $this->getProject()['$id'],
2220+
], $this->getHeaders()), [
2221+
'data' => [
2222+
'$id' => 'some-other-id',
2223+
'$collectionId' => 'some-other-collection',
2224+
'$databaseId' => 'some-other-database',
2225+
'$createdAt' => '2024-01-01T00:00:00Z',
2226+
'$updatedAt' => '2024-01-01T00:00:00Z',
2227+
'library' => [
2228+
'$id' => 'library3',
2229+
'libraryName' => 'Library 3',
2230+
'$createdAt' => '2024-01-01T00:00:00Z',
2231+
'$updatedAt' => '2024-01-01T00:00:00Z',
2232+
],
2233+
],
2234+
]);
2235+
2236+
$update = $personNoPerm;
2237+
$update['body']['$id'] = 'random';
2238+
$update['body']['$sequence'] = 123;
2239+
$update['body']['$databaseId'] = 'random';
2240+
$update['body']['$collectionId'] = 'random';
2241+
$update['body']['$createdAt'] = '2024-01-01T00:00:00.000+00:00';
2242+
$update['body']['$updatedAt'] = '2024-01-01T00:00:00.000+00:00';
2243+
2244+
$upserted = $this->client->call(Client::METHOD_PUT, '/databases/' . $databaseId . '/collections/' . $person['body']['$id'] . '/documents/' . $newPersonId, array_merge([
2245+
'content-type' => 'application/json',
2246+
'x-appwrite-project' => $this->getProject()['$id'],
2247+
], $this->getHeaders()), [
2248+
'data' => $update['body']
2249+
]);
2250+
2251+
$this->assertEquals(200, $upserted['headers']['status-code']);
2252+
$this->assertEquals($personNoPerm['body']['$id'], $upserted['body']['$id']);
2253+
$this->assertEquals($personNoPerm['body']['$collectionId'], $upserted['body']['$collectionId']);
2254+
$this->assertEquals($personNoPerm['body']['$databaseId'], $upserted['body']['$databaseId']);
2255+
$this->assertEquals($personNoPerm['body']['$sequence'], $upserted['body']['$sequence']);
2256+
2257+
if ($this->getSide() === 'client') {
2258+
$this->assertEquals($personNoPerm['body']['$createdAt'], $upserted['body']['$createdAt']);
2259+
$this->assertNotEquals('2024-01-01T00:00:00.000+00:00', $upserted['body']['$updatedAt']);
2260+
} else {
2261+
$this->assertEquals('2024-01-01T00:00:00.000+00:00', $upserted['body']['$createdAt']);
2262+
$this->assertEquals('2024-01-01T00:00:00.000+00:00', $upserted['body']['$updatedAt']);
2263+
}
22142264
}
22152265
}
22162266

@@ -3000,6 +3050,37 @@ public function testUpdateDocument(array $data): array
30003050

30013051
$this->assertEquals(200, $response['headers']['status-code']);
30023052

3053+
// Test readonly attributes are ignored
3054+
$response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $databaseId . '/collections/' . $data['moviesId'] . '/documents/' . $id, array_merge([
3055+
'content-type' => 'application/json',
3056+
'x-appwrite-project' => $this->getProject()['$id'],
3057+
'x-appwrite-timestamp' => DateTime::formatTz(DateTime::now()),
3058+
], $this->getHeaders()), [
3059+
'data' => [
3060+
'$id' => 'newId',
3061+
'$sequence' => 9999,
3062+
'$collectionId' => 'newCollectionId',
3063+
'$databaseId' => 'newDatabaseId',
3064+
'$createdAt' => '2024-01-01T00:00:00.000+00:00',
3065+
'$updatedAt' => '2024-01-01T00:00:00.000+00:00',
3066+
'title' => 'Thor: Ragnarok',
3067+
],
3068+
]);
3069+
3070+
$this->assertEquals(200, $response['headers']['status-code']);
3071+
$this->assertEquals($id, $response['body']['$id']);
3072+
$this->assertEquals($data['moviesId'], $response['body']['$collectionId']);
3073+
$this->assertEquals($databaseId, $response['body']['$databaseId']);
3074+
$this->assertNotEquals(9999, $response['body']['$sequence']);
3075+
3076+
if ($this->getSide() === 'client') {
3077+
$this->assertNotEquals('2024-01-01T00:00:00.000+00:00', $response['body']['$createdAt']);
3078+
$this->assertNotEquals('2024-01-01T00:00:00.000+00:00', $response['body']['$updatedAt']);
3079+
} else {
3080+
$this->assertEquals('2024-01-01T00:00:00.000+00:00', $response['body']['$createdAt']);
3081+
$this->assertEquals('2024-01-01T00:00:00.000+00:00', $response['body']['$updatedAt']);
3082+
}
3083+
30033084
return [];
30043085
}
30053086

@@ -4260,7 +4341,9 @@ public function testPersistentCreatedAt(array $data): array
42604341
]
42614342
]);
42624343
if ($this->getSide() === 'client') {
4263-
$this->assertEquals($document['headers']['status-code'], 400);
4344+
$this->assertEquals($document['body']['title'], 'Again Updated Date Test');
4345+
$this->assertNotEquals($document['body']['$createdAt'], DateTime::formatTz('2022-08-01 13:09:23.040'));
4346+
$this->assertNotEquals($document['body']['$updatedAt'], DateTime::formatTz('2022-08-01 13:09:23.050'));
42644347
} else {
42654348
$this->assertEquals($document['body']['title'], 'Again Updated Date Test');
42664349
$this->assertEquals($document['body']['$createdAt'], DateTime::formatTz('2022-08-01 13:09:23.040'));

0 commit comments

Comments
 (0)