Merge pull request #536 from utopia-php/fix-upsert-permissions

Make sure auth is skipped when appropriate for upsert

Author: abnegate

composer.lock

@@ -1050,24 +1050,29 @@ protected function getSQLIndexType(string $type): string
      *
      * @param string $collection
      * @param array<string> $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)}
+        )";
     }
 
     /**

src/Database/Adapter/SQL.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<Document>
      * @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', '$permissions'])],
+                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));