From 9325a456f730d1cafd3c82929af5fb8d37ee3b15 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 13 Mar 2025 20:42:26 +1300 Subject: [PATCH 1/4] Make sure auth is skipped when appropriate for upsert --- composer.lock | 48 ++++++++++++++++---------------- src/Database/Adapter/SQL.php | 27 ++++++++++-------- src/Database/Database.php | 53 ++++++++++++++++++++++++++---------- 3 files changed, 78 insertions(+), 50 deletions(-) diff --git a/composer.lock b/composer.lock index 9f1d43f57..0f01edf61 100644 --- a/composer.lock +++ b/composer.lock @@ -594,16 +594,16 @@ }, { "name": "open-telemetry/exporter-otlp", - "version": "1.2.0", + "version": "1.2.1", "source": { "type": "git", "url": "https://github.com/opentelemetry-php/exporter-otlp.git", - "reference": "243d9657c44a06f740cf384f486afe954c2b725f" + "reference": "b7580440b7481a98da97aceabeb46e1b276c8747" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/opentelemetry-php/exporter-otlp/zipball/243d9657c44a06f740cf384f486afe954c2b725f", - "reference": "243d9657c44a06f740cf384f486afe954c2b725f", + "url": "https://api.github.com/repos/opentelemetry-php/exporter-otlp/zipball/b7580440b7481a98da97aceabeb46e1b276c8747", + "reference": "b7580440b7481a98da97aceabeb46e1b276c8747", "shasum": "" }, "require": { @@ -654,7 +654,7 @@ "issues": "https://github.com/open-telemetry/opentelemetry-php/issues", "source": "https://github.com/open-telemetry/opentelemetry-php" }, - "time": "2025-01-08T23:50:03+00:00" + "time": "2025-03-06T23:21:56+00:00" }, { "name": "open-telemetry/gen-otlp-protobuf", @@ -2086,16 +2086,16 @@ }, { "name": "utopia-php/framework", - "version": "0.33.17", + "version": "0.33.19", "source": { "type": "git", "url": "https://github.com/utopia-php/http.git", - "reference": "73fac6fbce9f56282dba4e52a58cf836ec434644" + "reference": "64c7b7bb8a8595ffe875fa8d4b7705684dbf46c0" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/http/zipball/73fac6fbce9f56282dba4e52a58cf836ec434644", - "reference": "73fac6fbce9f56282dba4e52a58cf836ec434644", + "url": "https://api.github.com/repos/utopia-php/http/zipball/64c7b7bb8a8595ffe875fa8d4b7705684dbf46c0", + "reference": "64c7b7bb8a8595ffe875fa8d4b7705684dbf46c0", "shasum": "" }, "require": { @@ -2127,9 +2127,9 @@ ], "support": { "issues": "https://github.com/utopia-php/http/issues", - "source": "https://github.com/utopia-php/http/tree/0.33.17" + "source": "https://github.com/utopia-php/http/tree/0.33.19" }, - "time": "2025-02-24T17:35:48+00:00" + "time": "2025-03-06T11:37:49+00:00" }, { "name": "utopia-php/mongo", @@ -2378,16 +2378,16 @@ }, { "name": "laravel/pint", - "version": "v1.21.0", + "version": "v1.21.1", "source": { "type": "git", "url": "https://github.com/laravel/pint.git", - "reference": "531fa0871fbde719c51b12afa3a443b8f4e4b425" + "reference": "c44bffbb2334e90fba560933c45948fa4a3f3e86" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/laravel/pint/zipball/531fa0871fbde719c51b12afa3a443b8f4e4b425", - "reference": "531fa0871fbde719c51b12afa3a443b8f4e4b425", + "url": "https://api.github.com/repos/laravel/pint/zipball/c44bffbb2334e90fba560933c45948fa4a3f3e86", + "reference": "c44bffbb2334e90fba560933c45948fa4a3f3e86", "shasum": "" }, "require": { @@ -2398,9 +2398,9 @@ "php": "^8.2.0" }, "require-dev": { - "friendsofphp/php-cs-fixer": "^3.68.5", - "illuminate/view": "^11.42.0", - "larastan/larastan": "^3.0.4", + "friendsofphp/php-cs-fixer": "^3.70.2", + "illuminate/view": "^11.44.1", + "larastan/larastan": "^3.1.0", "laravel-zero/framework": "^11.36.1", "mockery/mockery": "^1.6.12", "nunomaduro/termwind": "^2.3", @@ -2440,7 +2440,7 @@ "issues": "https://github.com/laravel/pint/issues", "source": "https://github.com/laravel/pint" }, - "time": "2025-02-18T03:18:57+00:00" + "time": "2025-03-11T03:22:21+00:00" }, { "name": "myclabs/deep-copy", @@ -2712,16 +2712,16 @@ }, { "name": "phpstan/phpstan", - "version": "1.12.20", + "version": "1.12.21", "source": { "type": "git", "url": "https://github.com/phpstan/phpstan.git", - "reference": "3240b1972042c7f73cf1045e879ea5bd5f761bb7" + "reference": "14276fdef70575106a3392a4ed553c06a984df28" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpstan/phpstan/zipball/3240b1972042c7f73cf1045e879ea5bd5f761bb7", - "reference": "3240b1972042c7f73cf1045e879ea5bd5f761bb7", + "url": "https://api.github.com/repos/phpstan/phpstan/zipball/14276fdef70575106a3392a4ed553c06a984df28", + "reference": "14276fdef70575106a3392a4ed553c06a984df28", "shasum": "" }, "require": { @@ -2766,7 +2766,7 @@ "type": "github" } ], - "time": "2025-03-05T13:37:43+00:00" + "time": "2025-03-09T09:24:50+00:00" }, { "name": "phpunit/php-code-coverage", diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index f03140cf8..eafa917d6 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -1050,24 +1050,29 @@ protected function getSQLIndexType(string $type): string * * @param string $collection * @param array $roles + * @param string $type * @return string - * @throws Exception + * @throws DatabaseException */ - protected function getSQLPermissionsCondition(string $collection, array $roles, string $type = Database::PERMISSION_READ): string - { - if (!in_array($type, Database::PERMISSIONS)) { + protected function getSQLPermissionsCondition( + string $collection, + array $roles, + string $type = Database::PERMISSION_READ + ): string { + if (!\in_array($type, Database::PERMISSIONS)) { throw new DatabaseException('Unknown permission type: ' . $type); } - $roles = array_map(fn (string $role) => $this->getPDO()->quote($role), $roles); + $roles = \array_map(fn ($role) => $this->getPDO()->quote($role), $roles); + $roles = \implode(', ', $roles); return "table_main._uid IN ( - SELECT _document - FROM {$this->getSQLTable($collection . '_perms')} - WHERE _permission IN (" . implode(', ', $roles) . ") - AND _type = '{$type}' - {$this->getTenantQuery($collection)} - )"; + SELECT _document + FROM {$this->getSQLTable($collection . '_perms')} + WHERE _permission IN ({$roles}) + AND _type = '{$type}' + {$this->getTenantQuery($collection)} + )"; } /** diff --git a/src/Database/Database.php b/src/Database/Database.php index 445afaa5c..d22987d4c 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -3477,7 +3477,7 @@ public function createDocuments( $stack = []; foreach (\array_chunk($documents, $batchSize) as $chunk) { - $stack = array_merge($stack, $this->adapter->createDocuments($collection->getId(), $chunk)); + \array_push($stack, ...$this->adapter->createDocuments($collection->getId(), $chunk)); } return $stack; @@ -4592,6 +4592,7 @@ private function getJunctionCollection(Document $collection, Document $relatedCo * @param int $batchSize * @return array * @throws StructureException + * @throws \Throwable */ public function createOrUpdateDocuments( string $collection, @@ -4629,23 +4630,37 @@ public function createOrUpdateDocumentsWithIncrease( } $batchSize = \min(Database::INSERT_BATCH_SIZE, \max(1, $batchSize)); - $collection = $this->silent(fn () => $this->getCollection($collection)); - + $documentSecurity = $collection->getAttribute('documentSecurity', false); $time = DateTime::now(); foreach ($documents as $key => $document) { - $old = Authorization::skip(fn () => $this->silent(fn () => $this->getDocument($collection->getId(), $document->getId()))); - - if (!$old->isEmpty()) { - $validator = new Authorization(self::PERMISSION_UPDATE); + $old = Authorization::skip(fn () => $this->silent(fn () => $this->getDocument( + $collection->getId(), + $document->getId(), + [Query::select(['$internalId'])], + forUpdate: true + ))); + + // If old is empty, check if user has create permission on the collection + // If old is not empty, check if user has update permission on the collection + // If old is not empty AND documentSecurity is enabled, we need to check if user has update permission on the collection or document + + $validator = new Authorization( + $old->isEmpty() ? + self::PERMISSION_CREATE : + self::PERMISSION_UPDATE + ); - if (!$validator->isValid([ - ...$collection->getUpdate(), - ...($collection->getAttribute('documentSecurity') ? $old->getUpdate() : []) - ])) { + if ($old->isEmpty()) { + if (!$validator->isValid($collection->getCreate())) { throw new AuthorizationException($validator->getDescription()); } + } elseif (!$validator->isValid([ + ...$collection->getUpdate(), + ...($documentSecurity ? $old->getUpdate() : []) + ])) { + throw new AuthorizationException($validator->getDescription()); } $createdAt = $document->getCreatedAt(); @@ -4680,7 +4695,14 @@ public function createOrUpdateDocumentsWithIncrease( $stack = []; foreach (\array_chunk($documents, $batchSize) as $chunk) { - $stack = array_merge($stack, $this->adapter->createOrUpdateDocuments($collection->getId(), $attribute, $chunk)); + \array_push( + $stack, + ...Authorization::skip(fn () => $this->adapter->createOrUpdateDocuments( + $collection->getId(), + $attribute, + $chunk + )) + ); } return $stack; @@ -5347,6 +5369,7 @@ private function deleteCascade(Document $collection, Document $relatedCollection * @throws AuthorizationException * @throws DatabaseException * @throws RestrictedException + * @throws \Throwable */ public function deleteDocuments(string $collection, array $queries = [], int $batchSize = self::DELETE_BATCH_SIZE): array { @@ -5461,12 +5484,12 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba } foreach (\array_chunk($documents, $batchSize) as $chunk) { - $callback = fn () => $this->adapter->deleteDocuments( + $getResults = fn () => $this->adapter->deleteDocuments( $collection->getId(), array_map(fn ($document) => $document->getId(), $chunk) ); - $skipAuth ? $authorization->skip($callback) : $callback(); + $skipAuth ? $authorization->skip($getResults) : $getResults(); } return $documents; @@ -5568,7 +5591,7 @@ public function find(string $collection, array $queries = [], string $forPermiss } } - $authorization = new Authorization(self::PERMISSION_READ); + $authorization = new Authorization($forPermission); $documentSecurity = $collection->getAttribute('documentSecurity', false); $skipAuth = $authorization->isValid($collection->getPermissionsByType($forPermission)); From a4d4adbb96b0ce195854c4605d2639bfc36b0640 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 13 Mar 2025 21:25:32 +1300 Subject: [PATCH 2/4] Fix missing permissions select --- src/Database/Database.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index d22987d4c..71d381933 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -4638,7 +4638,7 @@ public function createOrUpdateDocumentsWithIncrease( $old = Authorization::skip(fn () => $this->silent(fn () => $this->getDocument( $collection->getId(), $document->getId(), - [Query::select(['$internalId'])], + [Query::select(['$internalId', '$permissions'])], forUpdate: true ))); From ed83731ce3e3020c7868a81d548e10f4d1ee7511 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 13 Mar 2025 22:07:49 +1300 Subject: [PATCH 3/4] Only select id, internal ID and permissions for prefetch loop --- src/Database/Database.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 71d381933..6bb4ef10b 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -5431,7 +5431,8 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba } $new = [ - Query::limit($batchSize) + Query::limit($batchSize), + Query::select(['$id', '$internalId', '$permissions']), ]; if (! empty($lastDocument)) { From c9630ffc30721184d6292db24a0cf81c2966d592 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 13 Mar 2025 22:12:22 +1300 Subject: [PATCH 4/4] Revert "Only select id, internal ID and permissions for prefetch loop" This reverts commit ed83731ce3e3020c7868a81d548e10f4d1ee7511. --- src/Database/Database.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 6bb4ef10b..71d381933 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -5431,8 +5431,7 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba } $new = [ - Query::limit($batchSize), - Query::select(['$id', '$internalId', '$permissions']), + Query::limit($batchSize) ]; if (! empty($lastDocument)) {