From 0b6c76a84ec9eb9568cb03183704eb26382733b2 Mon Sep 17 00:00:00 2001 From: fogelito Date: Sun, 14 Jun 2026 09:28:39 +0300 Subject: [PATCH 1/7] uid change --- src/Database/Adapter/MariaDB.php | 44 ++++++++++++++++++++++++++++---- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 599da878e..f6b05b43b 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -980,11 +980,22 @@ public function updateDocument(Document $collection, string $id, Document $docum $attributes['_createdAt'] = $document->getCreatedAt(); $attributes['_updatedAt'] = $document->getUpdatedAt(); $attributes['_permissions'] = json_encode($document->getPermissions()); + $attributes['_uid'] = $document->getId(); $name = $this->filter($collection); $columns = ''; if (!$skipPermissions) { + /** + * Permission rows in the _perms table are keyed by the document's + * UID (the _document column). When the UID changes, the existing + * rows must be re-pointed to the new UID before the diff below is + * applied, otherwise the old rows are orphaned and unchanged + * permissions are lost for the new UID. + */ + $newUid = $document->offsetExists('$id') ? $document->getId() : $id; + $uidChanged = $newUid !== $id; + $sql = " SELECT _type, _permission FROM {$this->getSQLTable($name . '_perms')} @@ -998,7 +1009,8 @@ public function updateDocument(Document $collection, string $id, Document $docum * Get current permissions from the database */ $sqlPermissions = $this->getPDO()->prepare($sql); - $sqlPermissions->bindValue(':_uid', $document->getId()); + + $sqlPermissions->bindValue(':_uid', $id); if ($this->sharedTables) { $sqlPermissions->bindValue(':_tenant', $this->tenant); @@ -1041,6 +1053,26 @@ public function updateDocument(Document $collection, string $id, Document $docum } } + /** + * Query to re-point existing permissions to the new UID + */ + if ($uidChanged) { + $sql = " + UPDATE {$this->getSQLTable($name . '_perms')} + SET _document = :_newUid + WHERE _document = :_uid + {$this->getTenantQuery($collection)} + "; + + $stmtRepointPermissions = $this->getPDO()->prepare($sql); + $stmtRepointPermissions->bindValue(':_uid', $id); + $stmtRepointPermissions->bindValue(':_newUid', $newUid); + + if ($this->sharedTables) { + $stmtRepointPermissions->bindValue(':_tenant', $this->tenant); + } + } + /** * Query to remove permissions */ @@ -1071,7 +1103,7 @@ public function updateDocument(Document $collection, string $id, Document $docum $removeQuery = $this->trigger(Database::EVENT_PERMISSIONS_DELETE, $removeQuery); $stmtRemovePermissions = $this->getPDO()->prepare($removeQuery); - $stmtRemovePermissions->bindValue(':_uid', $document->getId()); + $stmtRemovePermissions->bindValue(':_uid', $newUid); if ($this->sharedTables) { $stmtRemovePermissions->bindValue(':_tenant', $this->tenant); @@ -1119,7 +1151,7 @@ public function updateDocument(Document $collection, string $id, Document $docum $stmtAddPermissions = $this->getPDO()->prepare($sql); - $stmtAddPermissions->bindValue(":_uid", $document->getId()); + $stmtAddPermissions->bindValue(":_uid", $newUid); if ($this->sharedTables) { $stmtAddPermissions->bindValue(":_tenant", $this->tenant); @@ -1168,7 +1200,7 @@ public function updateDocument(Document $collection, string $id, Document $docum $sql = " UPDATE {$this->getSQLTable($name)} - SET {$columns} _uid = :_newUid + SET " . \rtrim($columns, ',') . " WHERE _id=:_sequence {$this->getTenantQuery($collection)} "; @@ -1178,7 +1210,6 @@ public function updateDocument(Document $collection, string $id, Document $docum $stmt = $this->getPDO()->prepare($sql); $stmt->bindValue(':_sequence', $document->getSequence()); - $stmt->bindValue(':_newUid', $document->getId()); if ($this->sharedTables) { $stmt->bindValue(':_tenant', $this->tenant); @@ -1209,6 +1240,9 @@ public function updateDocument(Document $collection, string $id, Document $docum $stmt->execute(); + if (isset($stmtRepointPermissions)) { + $stmtRepointPermissions->execute(); + } if (isset($stmtRemovePermissions)) { $stmtRemovePermissions->execute(); } From 20dd6e4f371febc3fb0bf372da399b8425da42df Mon Sep 17 00:00:00 2001 From: fogelito Date: Sun, 14 Jun 2026 09:30:01 +0300 Subject: [PATCH 2/7] Database.php --- src/Database/Database.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Database/Database.php b/src/Database/Database.php index 37b903d13..80bbb9dc6 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -6183,6 +6183,14 @@ public function updateDocument(string $collection, string $id, Document $documen } $document = new Document($document); + // A UID change re-keys the permission rows (_perms._document) to the + // new UID, so the permission block must run even when the permission + // set itself is unchanged. Otherwise the old rows are orphaned and the + // new UID is left with no permissions. + if ($document->getId() !== $id) { + $skipPermissionsUpdate = false; + } + $attributes = $collection->getAttribute('attributes', []); $relationships = \array_filter($attributes, function ($attribute) { From 9898ed1293e9c42ff44b561abd9f75255bb6597d Mon Sep 17 00:00:00 2001 From: fogelito Date: Sun, 14 Jun 2026 10:01:14 +0300 Subject: [PATCH 3/7] Add tests --- tests/e2e/Adapter/Scopes/DocumentTests.php | 104 +++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index 23cc3b623..5b935e4bc 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -1572,6 +1572,110 @@ public function testCreateDocumentDefaults(): void $database->deleteCollection('defaults'); } + /** + * When a document's UID changes on update, its permission rows in the + * collection's _perms table must follow the new UID. Otherwise the old + * rows are orphaned and the renamed document is left with no permissions, + * even when the permission set itself was not changed. + */ + public function testUpdateDocumentChangeIdMigratesPermissions(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + $auth = $database->getAuthorization(); + + $collection = 'update_change_id_perms'; + + // documentSecurity with no collection-level permissions: reads are + // governed purely by the document's rows in the _perms table. + $database->createCollection($collection, permissions: [], documentSecurity: true); + $this->assertEquals(true, $database->createAttribute($collection, 'name', Database::VAR_STRING, 128, false)); + + // Create a document whose read permission is scoped to a single role, + // so that find() must consult the _perms table to return it. + $document = $auth->skip(fn () => $database->createDocument($collection, new Document([ + '$id' => 'old_id', + 'name' => 'test', + '$permissions' => [ + Permission::read(Role::user('alice')), + Permission::update(Role::user('alice')), + Permission::delete(Role::user('alice')), + ], + ]))); + $this->assertEquals('old_id', $document->getId()); + + // Sanity: as alice the document is visible via the _perms table. + $auth->addRole(Role::user('alice')->toString()); + $this->assertCount(1, $database->find($collection)); + + // Rename the document WITHOUT changing its permission set. + $renamed = $auth->skip(fn () => $database->updateDocument($collection, 'old_id', new Document(\array_merge( + $document->getArrayCopy(), + ['$id' => 'new_id'], + )))); + $this->assertEquals('new_id', $renamed->getId()); + + // The old UID must no longer resolve to a document. + $this->assertTrue($auth->skip(fn () => $database->getDocument($collection, 'old_id'))->isEmpty()); + + // The new UID must exist and keep its permissions on the main row. + $newDoc = $auth->skip(fn () => $database->getDocument($collection, 'new_id')); + $this->assertFalse($newDoc->isEmpty()); + $this->assertContains(Permission::read(Role::user('alice')), $newDoc->getPermissions()); + + // The crucial check: the permission rows must have migrated to the new + // UID in the _perms table. As alice, find() (which joins _perms) must + // still return exactly the renamed document. With orphaned rows under + // the old UID this returns 0. + $found = $database->find($collection); + $this->assertCount(1, $found); + $this->assertEquals('new_id', $found[0]->getId()); + + /** + * Second scenario: change the UID AND the permission set in the same + * update. Drop alice's access and grant bob instead. The removed rows + * must be gone, the added rows must land under the new UID, and nothing + * may be left orphaned under the old UID. + */ + $rekeyed = $auth->skip(fn () => $database->updateDocument($collection, 'new_id', new Document(\array_merge( + $newDoc->getArrayCopy(), + [ + '$id' => 'final_id', + '$permissions' => [ + Permission::read(Role::user('bob')), + Permission::update(Role::user('bob')), + Permission::delete(Role::user('bob')), + ], + ], + )))); + $this->assertEquals('final_id', $rekeyed->getId()); + + // The old UID must no longer resolve to a document. + $this->assertTrue($auth->skip(fn () => $database->getDocument($collection, 'new_id'))->isEmpty()); + + // The main row must reflect the new permission set. + $finalDoc = $auth->skip(fn () => $database->getDocument($collection, 'final_id')); + $this->assertFalse($finalDoc->isEmpty()); + $this->assertContains(Permission::read(Role::user('bob')), $finalDoc->getPermissions()); + $this->assertNotContains(Permission::read(Role::user('alice')), $finalDoc->getPermissions()); + + // alice's permission rows were removed: as alice nothing is returned. + $this->assertCount(0, $database->find($collection)); + + // bob's permission rows landed under the new UID: as bob the renamed + // document is returned via the _perms join. + $auth->addRole(Role::user('bob')->toString()); + $foundAsBob = $database->find($collection); + $this->assertCount(1, $foundAsBob); + $this->assertEquals('final_id', $foundAsBob[0]->getId()); + + $auth->removeRole(Role::user('alice')->toString()); + $auth->removeRole(Role::user('bob')->toString()); + + $auth->skip(fn () => $database->deleteCollection($collection)); + } + public function testIncreaseDecrease(): Document { /** @var Database $database */ From b9d2a24d20ae4e9a8df0499b760723da58705249 Mon Sep 17 00:00:00 2001 From: fogelito Date: Sun, 14 Jun 2026 10:08:07 +0300 Subject: [PATCH 4/7] Uid change --- src/Database/Database.php | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 80bbb9dc6..b12cd6434 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -6170,6 +6170,12 @@ public function updateDocument(string $collection, string $id, Document $documen $skipPermissionsUpdate = ($originalPermissions === $currentPermissions); } + + // UID change + if ($document->offsetExists('$id') && $document->getId() !== $id) { + $skipPermissionsUpdate = false; + } + $createdAt = $document->getCreatedAt(); $document = \array_merge($old->getArrayCopy(), $document->getArrayCopy()); @@ -6183,14 +6189,6 @@ public function updateDocument(string $collection, string $id, Document $documen } $document = new Document($document); - // A UID change re-keys the permission rows (_perms._document) to the - // new UID, so the permission block must run even when the permission - // set itself is unchanged. Otherwise the old rows are orphaned and the - // new UID is left with no permissions. - if ($document->getId() !== $id) { - $skipPermissionsUpdate = false; - } - $attributes = $collection->getAttribute('attributes', []); $relationships = \array_filter($attributes, function ($attribute) { From 083161ed7d4e4b7fdeaadb1c788a7bad013f846d Mon Sep 17 00:00:00 2001 From: fogelito Date: Sun, 14 Jun 2026 14:47:56 +0300 Subject: [PATCH 5/7] try finally --- tests/e2e/Adapter/Scopes/DocumentTests.php | 168 +++++++++++---------- 1 file changed, 85 insertions(+), 83 deletions(-) diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index 5b935e4bc..6f0d78da9 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -1587,93 +1587,95 @@ public function testUpdateDocumentChangeIdMigratesPermissions(): void $collection = 'update_change_id_perms'; - // documentSecurity with no collection-level permissions: reads are - // governed purely by the document's rows in the _perms table. - $database->createCollection($collection, permissions: [], documentSecurity: true); - $this->assertEquals(true, $database->createAttribute($collection, 'name', Database::VAR_STRING, 128, false)); - - // Create a document whose read permission is scoped to a single role, - // so that find() must consult the _perms table to return it. - $document = $auth->skip(fn () => $database->createDocument($collection, new Document([ - '$id' => 'old_id', - 'name' => 'test', - '$permissions' => [ - Permission::read(Role::user('alice')), - Permission::update(Role::user('alice')), - Permission::delete(Role::user('alice')), - ], - ]))); - $this->assertEquals('old_id', $document->getId()); - - // Sanity: as alice the document is visible via the _perms table. - $auth->addRole(Role::user('alice')->toString()); - $this->assertCount(1, $database->find($collection)); - - // Rename the document WITHOUT changing its permission set. - $renamed = $auth->skip(fn () => $database->updateDocument($collection, 'old_id', new Document(\array_merge( - $document->getArrayCopy(), - ['$id' => 'new_id'], - )))); - $this->assertEquals('new_id', $renamed->getId()); - - // The old UID must no longer resolve to a document. - $this->assertTrue($auth->skip(fn () => $database->getDocument($collection, 'old_id'))->isEmpty()); - - // The new UID must exist and keep its permissions on the main row. - $newDoc = $auth->skip(fn () => $database->getDocument($collection, 'new_id')); - $this->assertFalse($newDoc->isEmpty()); - $this->assertContains(Permission::read(Role::user('alice')), $newDoc->getPermissions()); - - // The crucial check: the permission rows must have migrated to the new - // UID in the _perms table. As alice, find() (which joins _perms) must - // still return exactly the renamed document. With orphaned rows under - // the old UID this returns 0. - $found = $database->find($collection); - $this->assertCount(1, $found); - $this->assertEquals('new_id', $found[0]->getId()); - - /** - * Second scenario: change the UID AND the permission set in the same - * update. Drop alice's access and grant bob instead. The removed rows - * must be gone, the added rows must land under the new UID, and nothing - * may be left orphaned under the old UID. - */ - $rekeyed = $auth->skip(fn () => $database->updateDocument($collection, 'new_id', new Document(\array_merge( - $newDoc->getArrayCopy(), - [ - '$id' => 'final_id', + try { + // documentSecurity with no collection-level permissions: reads are + // governed purely by the document's rows in the _perms table. + $database->createCollection($collection, permissions: [], documentSecurity: true); + $this->assertEquals(true, $database->createAttribute($collection, 'name', Database::VAR_STRING, 128, false)); + + // Create a document whose read permission is scoped to a single role, + // so that find() must consult the _perms table to return it. + $document = $auth->skip(fn () => $database->createDocument($collection, new Document([ + '$id' => 'old_id', + 'name' => 'test', '$permissions' => [ - Permission::read(Role::user('bob')), - Permission::update(Role::user('bob')), - Permission::delete(Role::user('bob')), + Permission::read(Role::user('alice')), + Permission::update(Role::user('alice')), + Permission::delete(Role::user('alice')), ], - ], - )))); - $this->assertEquals('final_id', $rekeyed->getId()); - - // The old UID must no longer resolve to a document. - $this->assertTrue($auth->skip(fn () => $database->getDocument($collection, 'new_id'))->isEmpty()); + ]))); + $this->assertEquals('old_id', $document->getId()); + + // Sanity: as alice the document is visible via the _perms table. + $auth->addRole(Role::user('alice')->toString()); + $this->assertCount(1, $database->find($collection)); + + // Rename the document WITHOUT changing its permission set. + $renamed = $auth->skip(fn () => $database->updateDocument($collection, 'old_id', new Document(\array_merge( + $document->getArrayCopy(), + ['$id' => 'new_id'], + )))); + $this->assertEquals('new_id', $renamed->getId()); + + // The old UID must no longer resolve to a document. + $this->assertTrue($auth->skip(fn () => $database->getDocument($collection, 'old_id'))->isEmpty()); + + // The new UID must exist and keep its permissions on the main row. + $newDoc = $auth->skip(fn () => $database->getDocument($collection, 'new_id')); + $this->assertFalse($newDoc->isEmpty()); + $this->assertContains(Permission::read(Role::user('alice')), $newDoc->getPermissions()); + + // The crucial check: the permission rows must have migrated to the new + // UID in the _perms table. As alice, find() (which joins _perms) must + // still return exactly the renamed document. With orphaned rows under + // the old UID this returns 0. + $found = $database->find($collection); + $this->assertCount(1, $found); + $this->assertEquals('new_id', $found[0]->getId()); - // The main row must reflect the new permission set. - $finalDoc = $auth->skip(fn () => $database->getDocument($collection, 'final_id')); - $this->assertFalse($finalDoc->isEmpty()); - $this->assertContains(Permission::read(Role::user('bob')), $finalDoc->getPermissions()); - $this->assertNotContains(Permission::read(Role::user('alice')), $finalDoc->getPermissions()); - - // alice's permission rows were removed: as alice nothing is returned. - $this->assertCount(0, $database->find($collection)); - - // bob's permission rows landed under the new UID: as bob the renamed - // document is returned via the _perms join. - $auth->addRole(Role::user('bob')->toString()); - $foundAsBob = $database->find($collection); - $this->assertCount(1, $foundAsBob); - $this->assertEquals('final_id', $foundAsBob[0]->getId()); - - $auth->removeRole(Role::user('alice')->toString()); - $auth->removeRole(Role::user('bob')->toString()); + /** + * Second scenario: change the UID AND the permission set in the same + * update. Drop alice's access and grant bob instead. The removed rows + * must be gone, the added rows must land under the new UID, and nothing + * may be left orphaned under the old UID. + */ + $rekeyed = $auth->skip(fn () => $database->updateDocument($collection, 'new_id', new Document(\array_merge( + $newDoc->getArrayCopy(), + [ + '$id' => 'final_id', + '$permissions' => [ + Permission::read(Role::user('bob')), + Permission::update(Role::user('bob')), + Permission::delete(Role::user('bob')), + ], + ], + )))); + $this->assertEquals('final_id', $rekeyed->getId()); + + // The old UID must no longer resolve to a document. + $this->assertTrue($auth->skip(fn () => $database->getDocument($collection, 'new_id'))->isEmpty()); + + // The main row must reflect the new permission set. + $finalDoc = $auth->skip(fn () => $database->getDocument($collection, 'final_id')); + $this->assertFalse($finalDoc->isEmpty()); + $this->assertContains(Permission::read(Role::user('bob')), $finalDoc->getPermissions()); + $this->assertNotContains(Permission::read(Role::user('alice')), $finalDoc->getPermissions()); + + // alice's permission rows were removed: as alice nothing is returned. + $this->assertCount(0, $database->find($collection)); + + // bob's permission rows landed under the new UID: as bob the renamed + // document is returned via the _perms join. + $auth->addRole(Role::user('bob')->toString()); + $foundAsBob = $database->find($collection); + $this->assertCount(1, $foundAsBob); + $this->assertEquals('final_id', $foundAsBob[0]->getId()); + } finally { + $auth->removeRole(Role::user('alice')->toString()); + $auth->removeRole(Role::user('bob')->toString()); - $auth->skip(fn () => $database->deleteCollection($collection)); + $auth->skip(fn () => $database->deleteCollection($collection)); + } } public function testIncreaseDecrease(): Document From ab0996dcfd41fa3b87a5dd6bee9e09a384cb2d88 Mon Sep 17 00:00:00 2001 From: fogelito Date: Sun, 14 Jun 2026 14:52:15 +0300 Subject: [PATCH 6/7] repoint perms --- src/Database/Adapter/MariaDB.php | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index f6b05b43b..66a7c508b 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -986,13 +986,6 @@ public function updateDocument(Document $collection, string $id, Document $docum $columns = ''; if (!$skipPermissions) { - /** - * Permission rows in the _perms table are keyed by the document's - * UID (the _document column). When the UID changes, the existing - * rows must be re-pointed to the new UID before the diff below is - * applied, otherwise the old rows are orphaned and unchanged - * permissions are lost for the new UID. - */ $newUid = $document->offsetExists('$id') ? $document->getId() : $id; $uidChanged = $newUid !== $id; @@ -1006,7 +999,7 @@ public function updateDocument(Document $collection, string $id, Document $docum $sql = $this->trigger(Database::EVENT_PERMISSIONS_READ, $sql); /** - * Get current permissions from the database + * Get current permissions */ $sqlPermissions = $this->getPDO()->prepare($sql); From 49d63d6e43063adbe638eef0bfaed3007a9a3919 Mon Sep 17 00:00:00 2001 From: fogelito Date: Sun, 14 Jun 2026 14:52:53 +0300 Subject: [PATCH 7/7] Get current permissions from the database --- src/Database/Adapter/MariaDB.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 66a7c508b..62b01b5cf 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -999,7 +999,7 @@ public function updateDocument(Document $collection, string $id, Document $docum $sql = $this->trigger(Database::EVENT_PERMISSIONS_READ, $sql); /** - * Get current permissions + * Get current permissions from the database */ $sqlPermissions = $this->getPDO()->prepare($sql);