From 7f82cf7a12b0c617e5d75822ddb1f8027f08ddf3 Mon Sep 17 00:00:00 2001 From: fogelito Date: Sun, 11 Oct 2026 16:44:24 +0300 Subject: [PATCH 01/29] Collection level permissions --- src/Database/Database.php | 45 +++ src/Database/Document.php | 13 +- src/Database/Permission.php | 91 +++++- src/Database/Trait/Collections.php | 11 +- src/Database/Trait/Documents.php | 15 +- src/Database/Validator/Permissions.php | 43 ++- tests/e2e/Adapter/Base.php | 2 + .../Adapter/Scopes/ColumnPermissionTests.php | 285 ++++++++++++++++++ tests/unit/DocumentTest.php | 33 ++ tests/unit/PermissionTest.php | 76 +++++ tests/unit/Validator/PermissionsTest.php | 49 +++ 11 files changed, 639 insertions(+), 24 deletions(-) create mode 100644 tests/e2e/Adapter/Scopes/ColumnPermissionTests.php diff --git a/src/Database/Database.php b/src/Database/Database.php index aa4f3fd5d..1308e1aa0 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -257,6 +257,9 @@ class Database /** @var Value|null */ private ?Value $validation = null; + /** @var Value|null */ + private ?Value $foreignColumnPermissions = null; + /** @var Value|null */ private ?Value $datePreservation = null; @@ -1312,6 +1315,32 @@ public function skipValidation(callable $callback): mixed return $this->withValidation(false, $callback); } + /** + * Run the callback accepting column-scoped permissions on documents whose columns belong + * to another collection. + * + * A document's permissions are normally judged against its own collection's columns, and a + * column-scoped one is refused because there is nothing here it could correctly name. That + * is wrong for a document that *records* another collection's permissions rather than + * carrying its own -- a collection definition does this, and so does any row a caller keeps + * a collection's settings in. Those permissions are enforced where they belong, on the + * collection they describe, and judging them here would only compare them to the wrong + * schema. + * + * Scoped to the callback rather than set once, so the exemption covers the one write that + * needs it instead of every write that follows. Nothing is enforced inside: this says the + * permissions are a record, not a rule. + * + * @template T + * + * @param callable(): T $callback + * @return T + */ + public function withForeignColumnPermissions(callable $callback): mixed + { + return $this->foreignColumnPermissions()->with(true, $callback); + } + /** * Register a hook into the database pipeline. * @@ -1650,6 +1679,22 @@ private function validation(): Value return $this->validation ??= new Value(true); } + /** + * @return Value + */ + private function foreignColumnPermissions(): Value + { + return $this->foreignColumnPermissions ??= new Value(false); + } + + /** + * Whether a document's permissions may name a column belonging to another collection. + */ + public function hasForeignColumnPermissions(): bool + { + return $this->foreignColumnPermissions()->get(); + } + /** * @return Value */ diff --git a/src/Database/Document.php b/src/Database/Document.php index 798511983..9b02d6e4d 100644 --- a/src/Database/Document.php +++ b/src/Database/Document.php @@ -299,7 +299,18 @@ private function parsePermissions(): array continue; } $type = \trim(\substr($permission, 0, $open)); - $roles[$type][] = \str_replace([')', '"', ' '], '', \substr($permission, $open + 1)); + $role = \substr($permission, $open + 1); + + // A column-scoped permission is read("role", "column"). Only the role belongs here: + // this list answers who holds a permission, and the column narrows what that + // permission reaches, not who holds it. Leaving the column in would produce a role + // of "any,name" that matches nobody. + $separator = \strpos($role, '", "'); + if ($separator !== false) { + $role = \substr($role, 0, $separator + 1); + } + + $roles[$type][] = \str_replace([')', '"', ' '], '', $role); } return $this->parsedPermissions = [ diff --git a/src/Database/Permission.php b/src/Database/Permission.php index 8ded25a20..0059c5b3f 100644 --- a/src/Database/Permission.php +++ b/src/Database/Permission.php @@ -10,6 +10,15 @@ */ class Permission { + /** + * A permission naming no column covers every column. + * + * The empty string rather than null: a permission is parsed from and rendered back to + * a string, and an absent column has to round-trip through that without becoming the + * word "null" or a column of that name. + */ + public const string ALL_COLUMNS = ''; + private Role $role; /** @@ -28,12 +37,14 @@ class Permission * @param string $role The role name * @param string $identifier The role identifier * @param string $dimension The role dimension + * @param string $column The column this permission is scoped to, or ALL_COLUMNS for the whole row */ public function __construct( private string $permission, string $role, string $identifier = '', string $dimension = '', + private string $column = self::ALL_COLUMNS, ) { $this->role = new Role($role, $identifier, $dimension); } @@ -45,7 +56,27 @@ public function __construct( */ public function toString(): string { - return $this->permission.'("'.$this->role->toString().'")'; + if ($this->column === self::ALL_COLUMNS) { + return $this->permission.'("'.$this->role->toString().'")'; + } + + return $this->permission.'("'.$this->role->toString().'", "'.$this->column.'")'; + } + + /** + * The column this permission is scoped to, or ALL_COLUMNS when it covers the whole row. + */ + public function getColumn(): string + { + return $this->column; + } + + /** + * Does this permission cover every column rather than naming one? + */ + public function isForAllColumns(): bool + { + return $this->column === self::ALL_COLUMNS; } /** @@ -109,6 +140,23 @@ public static function parse(string $permission): self throw new DatabaseException('Invalid permission type: "'.$permission.'".'); } $fullRole = \str_replace('")', '', $permissionParts[1]); + + // A column-scoped permission is read("role", "column"). Split the column off here so + // every branch below parses a bare role exactly as it did before -- the role grammar + // (identifier, dimension) is unchanged by the column, and keeping them separate stops + // the two from having to know about each other. + $column = self::ALL_COLUMNS; + $separator = \strpos($fullRole, '", "'); + + if ($separator !== false) { + $column = \substr($fullRole, $separator + 4); + $fullRole = \substr($fullRole, 0, $separator); + + if ($column === '') { + throw new DatabaseException('Column must not be empty'); + } + } + $roleParts = \explode(':', $fullRole); $role = $roleParts[0]; @@ -116,13 +164,13 @@ public static function parse(string $permission): self $hasDimension = \str_contains($fullRole, '/'); if (! $hasIdentifier && ! $hasDimension) { - return new self($permission, $role); + return new self($permission, $role, column: $column); } if ($hasIdentifier && ! $hasDimension) { $identifier = $roleParts[1]; - return new self($permission, $role, $identifier); + return new self($permission, $role, $identifier, column: $column); } if (! $hasIdentifier) { @@ -138,7 +186,7 @@ public static function parse(string $permission): self throw new DatabaseException('Dimension must not be empty'); } - return new self($permission, $role, '', $dimension); + return new self($permission, $role, '', $dimension, $column); } // Has both identifier and dimension @@ -154,7 +202,7 @@ public static function parse(string $permission): self throw new DatabaseException('Dimension must not be empty'); } - return new self($permission, $role, $identifier, $dimension); + return new self($permission, $role, $identifier, $dimension, $column); } /** @@ -196,7 +244,8 @@ public static function aggregate(?array $permissions, array $allowed = [Permissi $subType, $permission->getRole(), $permission->getIdentifier(), - $permission->getDimension() + $permission->getDimension(), + $permission->getColumn() ))->toString(); } } @@ -209,15 +258,17 @@ public static function aggregate(?array $permissions, array $allowed = [Permissi * Create a read permission string from the given Role. * * @param Role $role The role to grant read permission to + * @param string $column Scope the permission to one column, or ALL_COLUMNS for the whole row * @return string The formatted permission string */ - public static function read(Role $role): string + public static function read(Role $role, string $column = self::ALL_COLUMNS): string { $permission = new self( PermissionType::Read->value, $role->getRole(), $role->getIdentifier(), - $role->getDimension() + $role->getDimension(), + $column ); return $permission->toString(); @@ -227,15 +278,17 @@ public static function read(Role $role): string * Create a create permission string from the given Role. * * @param Role $role The role to grant create permission to + * @param string $column Scope the permission to one column, or ALL_COLUMNS for the whole row * @return string The formatted permission string */ - public static function create(Role $role): string + public static function create(Role $role, string $column = self::ALL_COLUMNS): string { $permission = new self( PermissionType::Create->value, $role->getRole(), $role->getIdentifier(), - $role->getDimension() + $role->getDimension(), + $column ); return $permission->toString(); @@ -245,15 +298,17 @@ public static function create(Role $role): string * Create an update permission string from the given Role. * * @param Role $role The role to grant update permission to + * @param string $column Scope the permission to one column, or ALL_COLUMNS for the whole row * @return string The formatted permission string */ - public static function update(Role $role): string + public static function update(Role $role, string $column = self::ALL_COLUMNS): string { $permission = new self( PermissionType::Update->value, $role->getRole(), $role->getIdentifier(), - $role->getDimension() + $role->getDimension(), + $column ); return $permission->toString(); @@ -263,15 +318,17 @@ public static function update(Role $role): string * Create a delete permission string from the given Role. * * @param Role $role The role to grant delete permission to + * @param string $column Scope the permission to one column, or ALL_COLUMNS for the whole row * @return string The formatted permission string */ - public static function delete(Role $role): string + public static function delete(Role $role, string $column = self::ALL_COLUMNS): string { $permission = new self( PermissionType::Delete->value, $role->getRole(), $role->getIdentifier(), - $role->getDimension() + $role->getDimension(), + $column ); return $permission->toString(); @@ -281,15 +338,17 @@ public static function delete(Role $role): string * Create a write permission string from the given Role. * * @param Role $role The role to grant write permission to + * @param string $column Scope the permission to one column, or ALL_COLUMNS for the whole row * @return string The formatted permission string */ - public static function write(Role $role): string + public static function write(Role $role, string $column = self::ALL_COLUMNS): string { $permission = new self( PermissionType::Write->value, $role->getRole(), $role->getIdentifier(), - $role->getDimension() + $role->getDimension(), + $column ); return $permission->toString(); diff --git a/src/Database/Trait/Collections.php b/src/Database/Trait/Collections.php index ee06d09b5..8af929a64 100644 --- a/src/Database/Trait/Collections.php +++ b/src/Database/Trait/Collections.php @@ -55,7 +55,7 @@ public function createCollection(Collection $collection): Collection } if ($this->validation()->get()) { - $validator = new Permissions(); + $validator = new Permissions(columns: \array_map(fn (Attribute $attribute) => $attribute->key, $attributes)); if (! $validator->isValid($permissions)) { throw new DatabaseException($validator->getDescription()); } @@ -160,15 +160,18 @@ public function createCollection(Collection $collection): Collection */ public function updateCollection(string $collection, CollectionUpdate $update): Collection { + $definition = $this->silent(fn () => $this->getOwnCollection($collection)); + + // After the read, not before: a column-scoped permission is judged against this + // collection's columns, so there is nothing to judge it with until the definition + // is in hand. if ($update->permissions !== null && $this->validation()->get()) { - $validator = new Permissions(); + $validator = new Permissions(columns: \array_map(fn (Attribute $attribute) => $attribute->key, $definition->attributes())); if (! $validator->isValid($update->permissions)) { throw new DatabaseException($validator->getDescription()); } } - $definition = $this->silent(fn () => $this->getOwnCollection($collection)); - if ($update->permissions === null && $update->documentSecurity === null) { return $definition; } diff --git a/src/Database/Trait/Documents.php b/src/Database/Trait/Documents.php index 827538f44..6a3bc2cc6 100644 --- a/src/Database/Trait/Documents.php +++ b/src/Database/Trait/Documents.php @@ -1229,8 +1229,19 @@ private function prepareDocument(Document $collection, Document $document): Docu $document = $this->encode($collection, $document); - if ($this->validation()->get()) { - $validator = new Permissions(); + // A metadata document is a collection definition, whose permissions name that + // collection's columns rather than _metadata's, and createCollection() and + // updateCollection() have already judged them against the right list. + // withForeignColumnPermissions() says the same of any other document a caller keeps a + // collection's settings in. Both are records of permissions enforced elsewhere, so + // judging them here would only ever compare them to the wrong schema. + $recordsForeignPermissions = $collection->getId() === Database::METADATA + || $this->hasForeignColumnPermissions(); + + if ($this->validation()->get() && ! $recordsForeignPermissions) { + // No column list on purpose: column permissions are set on the collection, so a + // document naming one is refused rather than stored and never enforced. + $validator = new Permissions(columns: null); if (! $validator->isValid($document->getPermissions())) { throw new DatabaseException($validator->getDescription()); } diff --git a/src/Database/Validator/Permissions.php b/src/Database/Validator/Permissions.php index bcb96f493..7de411106 100644 --- a/src/Database/Validator/Permissions.php +++ b/src/Database/Validator/Permissions.php @@ -20,14 +20,28 @@ class Permissions extends Roles #[\Override] protected int $length; + /** + * Columns a permission in this set is allowed to name. + * + * @var array|null + */ + private ?array $columns; + /** * @param int $length maximum amount of permissions. 0 means unlimited. * @param array $allowed allowed permissions. Defaults to all available. + * @param array|null $columns the collection's column keys. A permission may name one + * of these and no other. Null means this set may not name + * a column at all, which is the case for every permission + * outside a collection's own -- a document's permissions + * carry no column, and nothing else has a column list to + * judge one against. */ - public function __construct(int $length = 0, array $allowed = [PermissionType::Create, PermissionType::Read, PermissionType::Update, PermissionType::Delete, PermissionType::Write]) + public function __construct(int $length = 0, array $allowed = [PermissionType::Create, PermissionType::Read, PermissionType::Update, PermissionType::Delete, PermissionType::Write], ?array $columns = null) { $this->length = $length; $this->allowed = \array_map(fn (PermissionType $p) => $p->value, $allowed); + $this->columns = $columns; } #[\Override] @@ -98,6 +112,33 @@ public function isValid(mixed $value): bool if (! $this->isValidRole($role, $identifier, $dimension)) { return false; } + + if (! $permission->isForAllColumns() && ! $this->isValidColumn($permission)) { + return false; + } + } + + return true; + } + + /** + * A permission may name a column only where columns are meaningful, and only one that exists. + * + * Reported by the key the caller wrote. Nothing has translated it yet at this point, and an + * error naming something the caller did not type is worse than no error at all. + */ + private function isValidColumn(Permission $permission): bool + { + if ($this->columns === null) { + $this->message = 'Permission "'.$permission->toString().'" is scoped to a column. Column-scoped permissions are set on the collection, not here.'; + + return false; + } + + if (! \in_array($permission->getColumn(), $this->columns, true)) { + $this->message = 'Column "'.$permission->getColumn().'" does not exist.'; + + return false; } return true; diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 7e8058f9c..d6080f81d 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -7,6 +7,7 @@ use Tests\E2E\Adapter\Scopes\AggregationTests; use Tests\E2E\Adapter\Scopes\AttributeTests; use Tests\E2E\Adapter\Scopes\CollectionTests; +use Tests\E2E\Adapter\Scopes\ColumnPermissionTests; use Tests\E2E\Adapter\Scopes\CustomDocumentTypeTests; use Tests\E2E\Adapter\Scopes\DatabaseTests; use Tests\E2E\Adapter\Scopes\DocumentTests; @@ -45,6 +46,7 @@ abstract class Base extends TestCase use AggregationTests; use AttributeTests; use CollectionTests; + use ColumnPermissionTests; use CustomDocumentTypeTests; use DatabaseTests; use DocumentTests; diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php new file mode 100644 index 000000000..fe2943709 --- /dev/null +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -0,0 +1,285 @@ +getDatabase(); + + $database->createCollection(Collection::create( + id: 'columnGrants', + attributes: [ + Attribute::string(key: 'name', size: 64), + Attribute::integer(key: 'salary'), + ], + permissions: [ + Permission::read(Role::any(), 'name'), + Permission::read(Role::team('hr'), 'salary'), + Permission::create(Role::any()), + ], + )); + + $stored = $database->getCollection('columnGrants'); + + $this->assertContains('read("any", "name")', $stored->getPermissions()); + $this->assertContains('read("team:hr", "salary")', $stored->getPermissions()); + $this->assertContains('create("any")', $stored->getPermissions()); + } + + public function testACollectionPermissionMayNotNameAColumnThatDoesNotExist(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + try { + $database->createCollection(Collection::create( + id: 'columnGrantsUnknown', + attributes: [Attribute::string(key: 'name', size: 64)], + permissions: [Permission::read(Role::any(), 'nope')], + )); + $this->fail('Expected createCollection() to reject a permission naming an unknown column'); + } catch (DatabaseException $e) { + $this->assertStringContainsString('Column "nope" does not exist.', $e->getMessage()); + } + } + + public function testUpdatingACollectionJudgesAColumnGrantAgainstItsOwnColumns(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + $database->createCollection(Collection::create( + id: 'columnGrantsUpdate', + attributes: [ + Attribute::string(key: 'name', size: 64), + Attribute::integer(key: 'salary'), + ], + permissions: [Permission::create(Role::any())], + )); + + $database->updateCollection('columnGrantsUpdate', new CollectionUpdate(permissions: [ + Permission::read(Role::any(), 'name'), + Permission::create(Role::any()), + ])); + + $this->assertContains( + 'read("any", "name")', + $database->getCollection('columnGrantsUpdate')->getPermissions(), + ); + + try { + $database->updateCollection('columnGrantsUpdate', new CollectionUpdate(permissions: [ + Permission::read(Role::any(), 'nope'), + ])); + $this->fail('Expected updateCollection() to reject a permission naming an unknown column'); + } catch (DatabaseException $e) { + $this->assertStringContainsString('Column "nope" does not exist.', $e->getMessage()); + } + + // The rejected update left the collection as it was. + $this->assertContains( + 'read("any", "name")', + $database->getCollection('columnGrantsUpdate')->getPermissions(), + ); + } + + public function testADocumentPermissionMayNotNameAColumn(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + $database->createCollection(Collection::create( + id: 'columnGrantsDocument', + attributes: [ + Attribute::string(key: 'name', size: 64), + Attribute::integer(key: 'salary'), + ], + permissions: [Permission::create(Role::any())], + documentSecurity: true, + )); + + // Column permissions belong to the collection. A document saying otherwise is + // refused rather than stored and ignored, which would read as a restriction that + // silently does nothing. + try { + $database->createDocument('columnGrantsDocument', new Document([ + '$id' => 'row', + 'name' => 'Ada', + 'salary' => 120000, + '$permissions' => [Permission::read(Role::any(), 'salary')], + ])); + $this->fail('Expected createDocument() to reject a column-scoped permission'); + } catch (DatabaseException $e) { + $this->assertStringContainsString('is scoped to a column', $e->getMessage()); + } + + // The rejected write left nothing behind. + $this->assertTrue($database->getDocument('columnGrantsDocument', 'row')->isEmpty()); + } + + public function testADocumentMayRecordAnotherCollectionsColumnPermissions(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + // Stands in for a row a caller keeps another collection's settings in -- Appwrite's + // table list does this. Its own columns are nothing like the ones its permissions name. + $database->createCollection(Collection::create( + id: 'columnGrantsRecords', + attributes: [ + Attribute::string(key: 'tableId', size: 64), + Attribute::string(key: 'label', size: 64), + ], + permissions: [Permission::create(Role::any())], + documentSecurity: true, + )); + + $recorded = [ + Permission::read(Role::any(), 'name'), + Permission::read(Role::team('hr'), 'salary'), + ]; + + try { + $database->createDocument('columnGrantsRecords', new Document([ + '$id' => 'row', + 'tableId' => 'employees', + 'label' => 'Employees', + '$permissions' => $recorded, + ])); + $this->fail('Expected createDocument() to reject columns belonging to another collection'); + } catch (DatabaseException $e) { + $this->assertStringContainsString('is scoped to a column', $e->getMessage()); + } + + $database->withForeignColumnPermissions(fn () => $database->createDocument('columnGrantsRecords', new Document([ + '$id' => 'row', + 'tableId' => 'employees', + 'label' => 'Employees', + '$permissions' => $recorded, + ]))); + + $stored = $database->getDocument('columnGrantsRecords', 'row'); + + $this->assertSame('employees', $stored->getAttribute('tableId')); + $this->assertContains('read("any", "name")', $stored->getPermissions()); + $this->assertContains('read("team:hr", "salary")', $stored->getPermissions()); + + // Scoped to the callback: the next write is judged as strictly as the first. + $this->assertFalse($database->hasForeignColumnPermissions()); + } + + public function testARecordedColumnGrantStillDecidesRowVisibilityByRoleAlone(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + $authorization = $database->getAuthorization(); + + // Appwrite's table list: each row records one table's permissions, column grants and + // all. Whether a caller may see the row is a question about the role alone, so a column + // narrows what the permission reaches without narrowing who holds it. + $database->createCollection(Collection::create( + id: 'columnGrantsListing', + attributes: [Attribute::string(key: 'tableId', size: 64)], + permissions: [Permission::create(Role::any())], + documentSecurity: true, + )); + + $database->withForeignColumnPermissions(function () use ($database) { + $database->createDocument('columnGrantsListing', new Document([ + '$id' => 'employees', + 'tableId' => 'employees', + '$permissions' => [ + Permission::read(Role::any(), 'name'), + Permission::read(Role::team('hr'), 'salary'), + ], + ])); + $database->createDocument('columnGrantsListing', new Document([ + '$id' => 'invoices', + 'tableId' => 'invoices', + '$permissions' => [Permission::read(Role::team('finance'))], + ])); + }); + + $listed = function (array $roles) use ($database, $authorization): array { + $previous = $authorization->getRoles(); + $authorization->cleanRoles(); + foreach ($roles as $role) { + $authorization->addRole($role); + } + + try { + $ids = \array_map( + static fn (Document $document): string => $document->getId(), + $database->find('columnGrantsListing'), + ); + } finally { + $authorization->cleanRoles(); + foreach ($previous as $role) { + $authorization->addRole($role); + } + } + + \sort($ids); + + return $ids; + }; + + $this->assertSame( + ['employees'], + $listed([Role::any()->toString()]), + 'A grant scoped to a column still admits its role to the row', + ); + $this->assertSame( + ['invoices'], + $listed([Role::team('finance')->toString()]), + 'A role named by no grant on a row does not see it', + ); + } + + public function testADocumentPermissionNamingNoColumnIsUnaffected(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + $database->createCollection(Collection::create( + id: 'columnGrantsPlain', + attributes: [Attribute::string(key: 'name', size: 64)], + permissions: [Permission::create(Role::any())], + documentSecurity: true, + )); + + $database->createDocument('columnGrantsPlain', new Document([ + '$id' => 'row', + 'name' => 'Ada', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::user('alice')), + ], + ])); + + $stored = $database->getDocument('columnGrantsPlain', 'row'); + + $this->assertSame('Ada', $stored->getAttribute('name')); + $this->assertContains('read("any")', $stored->getPermissions()); + $this->assertContains('update("user:alice")', $stored->getPermissions()); + } +} diff --git a/tests/unit/DocumentTest.php b/tests/unit/DocumentTest.php index 705ca47a8..735a22629 100644 --- a/tests/unit/DocumentTest.php +++ b/tests/unit/DocumentTest.php @@ -541,6 +541,39 @@ public function testArrayAccessPermissionWriteRefreshesRoles(): void $this->assertSame([Permission::read(Role::any())], $document->getPermissions()); } + /** + * The role list answers who holds a permission. A column narrows what the permission + * reaches, not who holds it, so it is left out of the roles and the grant still admits + * its role to the row. + */ + public function testAColumnScopedPermissionContributesOnlyItsRole(): void + { + $document = new Document([Document::PERMISSIONS => [ + Permission::read(Role::any(), 'name'), + Permission::read(Role::team('hr'), 'salary'), + Permission::update(Role::user('alice'), 'salary'), + ]]); + + $this->assertSame(['any', 'team:hr'], $document->getPermissionsByType(PermissionType::Read)); + $this->assertSame(['user:alice'], $document->getPermissionsByType(PermissionType::Update)); + + // The permissions themselves keep the column they were written with. + $this->assertContains('read("any", "name")', $document->getPermissions()); + } + + /** + * Two grants on the same role, each narrowed to a different column, name that role once. + */ + public function testTwoColumnGrantsOnOneRoleYieldOneRole(): void + { + $document = new Document([Document::PERMISSIONS => [ + Permission::read(Role::any(), 'name'), + Permission::read(Role::any(), 'salary'), + ]]); + + $this->assertSame(['any'], $document->getPermissionsByType(PermissionType::Read)); + } + public function testUnsetPermissionsClearsRoles(): void { $document = new Document([Document::PERMISSIONS => [Permission::read(Role::any())]]); diff --git a/tests/unit/PermissionTest.php b/tests/unit/PermissionTest.php index 047f7fecf..72b919a0b 100644 --- a/tests/unit/PermissionTest.php +++ b/tests/unit/PermissionTest.php @@ -3,6 +3,7 @@ namespace Tests\Unit; use PHPUnit\Framework\TestCase; +use Utopia\Database\Exception as DatabaseException; use Utopia\Database\Id; use Utopia\Database\Permission; use Utopia\Database\PermissionType; @@ -329,4 +330,79 @@ public function test_aggregation(): void 'delete("user:123")', ], $parsed); } + + /** + * A permission may name one column, and then covers that column alone. Naming none + * covers every column, which is what every permission has always meant. + * + * @throws \Exception + */ + public function test_column_scope(): void + { + $permission = Permission::parse('read("any", "salary")'); + $this->assertEquals('read', $permission->getPermission()); + $this->assertEquals('any', $permission->getRole()); + $this->assertEquals('salary', $permission->getColumn()); + $this->assertFalse($permission->isForAllColumns()); + $this->assertEquals('read("any", "salary")', $permission->toString()); + + $permission = Permission::parse('read("any")'); + $this->assertEquals(Permission::ALL_COLUMNS, $permission->getColumn()); + $this->assertTrue($permission->isForAllColumns()); + + // The column does not disturb the role grammar either side of it. + $permission = Permission::parse('update("team:123/admin", "salary")'); + $this->assertEquals('team', $permission->getRole()); + $this->assertEquals('123', $permission->getIdentifier()); + $this->assertEquals('admin', $permission->getDimension()); + $this->assertEquals('salary', $permission->getColumn()); + $this->assertEquals('update("team:123/admin", "salary")', $permission->toString()); + + $permission = Permission::parse('read("user:123", "name")'); + $this->assertEquals('user', $permission->getRole()); + $this->assertEquals('123', $permission->getIdentifier()); + $this->assertEmpty($permission->getDimension()); + $this->assertEquals('name', $permission->getColumn()); + } + + /** + * @throws \Exception + */ + public function test_column_must_not_be_empty(): void + { + $this->expectException(DatabaseException::class); + $this->expectExceptionMessage('Column must not be empty'); + + Permission::parse('read("any", "")'); + } + + /** + * Every builder takes the column, so a caller never has to assemble the string by hand. + */ + public function test_column_builders(): void + { + $this->assertEquals('read("any", "name")', Permission::read(Role::any(), 'name')); + $this->assertEquals('create("any", "name")', Permission::create(Role::any(), 'name')); + $this->assertEquals('update("any", "name")', Permission::update(Role::any(), 'name')); + $this->assertEquals('delete("any", "name")', Permission::delete(Role::any(), 'name')); + $this->assertEquals('write("any", "name")', Permission::write(Role::any(), 'name')); + + $this->assertEquals('read("any")', Permission::read(Role::any())); + } + + /** + * Expanding write() keeps the column: the aggregate is the same grant, narrowed the same way. + * + * @throws \Exception + */ + public function test_aggregation_keeps_the_column(): void + { + $parsed = Permission::aggregate([Permission::write(Role::any(), 'salary')]); + + $this->assertEquals([ + 'create("any", "salary")', + 'update("any", "salary")', + 'delete("any", "salary")', + ], $parsed); + } } diff --git a/tests/unit/Validator/PermissionsTest.php b/tests/unit/Validator/PermissionsTest.php index c60d85c96..bd1f5da70 100644 --- a/tests/unit/Validator/PermissionsTest.php +++ b/tests/unit/Validator/PermissionsTest.php @@ -356,4 +356,53 @@ public function test_custom_type_rejected_as_in_7_4(): void $this->assertFalse($object->isValid(['execute("any")'])); $this->assertSame('Permission "execute("any")" is not allowed. Must be one of: create, read, update, delete, write.', $object->getDescription()); } + + /** + * A column list is the collection's own. Only then may a permission name a column, + * and only one that is on the list. + */ + public function test_column_scope_against_a_collection(): void + { + $object = new Permissions(columns: ['name', 'salary']); + + $this->assertTrue($object->isValid([Permission::read(Role::any(), 'name')])); + $this->assertTrue($object->isValid([Permission::read(Role::team('hr'), 'salary')])); + + // A permission naming no column still covers every column. + $this->assertTrue($object->isValid([Permission::read(Role::any())])); + + $this->assertFalse($object->isValid([Permission::read(Role::any(), 'nope')])); + $this->assertSame('Column "nope" does not exist.', $object->getDescription()); + } + + /** + * Without a column list there is nothing to judge a column against, so naming one is + * refused rather than stored and never enforced. This is every set but a collection's own. + */ + public function test_column_scope_refused_without_a_column_list(): void + { + $object = new Permissions(); + + $this->assertFalse($object->isValid([Permission::read(Role::any(), 'name')])); + $this->assertSame( + 'Permission "read("any", "name")" is scoped to a column. Column-scoped permissions are set on the collection, not here.', + $object->getDescription(), + ); + + $this->assertTrue($object->isValid([Permission::read(Role::any())])); + } + + /** + * An empty column list is a collection with no columns: no permission may name one, + * and that is not the same as having no list at all. + */ + public function test_column_scope_against_a_collection_without_columns(): void + { + $object = new Permissions(columns: []); + + $this->assertFalse($object->isValid([Permission::read(Role::any(), 'name')])); + $this->assertSame('Column "name" does not exist.', $object->getDescription()); + + $this->assertTrue($object->isValid([Permission::read(Role::any())])); + } } From 96303adcc47dfe99e60d97a4e764259f217b2db8 Mon Sep 17 00:00:00 2001 From: fogelito Date: Mon, 12 Oct 2026 14:43:36 +0300 Subject: [PATCH 02/29] declaredPermissions --- src/Database/Collection.php | 99 ++++++- src/Database/Database.php | 49 +--- src/Database/Permission.php | 52 ++++ src/Database/Trait/Collections.php | 2 +- src/Database/Trait/Documents.php | 19 +- tests/e2e/Adapter/Base.php | 40 +-- .../Adapter/Scopes/ColumnPermissionTests.php | 254 +++++++++++------- .../Collections/CollectionDefinitionTest.php | 3 +- tests/unit/PermissionTest.php | 53 ++++ 9 files changed, 391 insertions(+), 180 deletions(-) diff --git a/src/Database/Collection.php b/src/Database/Collection.php index 593c9a696..ac9e52ea6 100644 --- a/src/Database/Collection.php +++ b/src/Database/Collection.php @@ -2,6 +2,7 @@ namespace Utopia\Database; +use Utopia\Database\Exception as DatabaseException; use Utopia\Database\Exception\Index as IndexException; use Utopia\Database\Exception\Relationship as RelationshipException; use Utopia\Database\Exception\Structure as StructureException; @@ -16,7 +17,22 @@ class Collection extends Document public const string DOCUMENT_SECURITY = 'documentSecurity'; - private const array CORE_KEYS = [self::ATTRIBUTES, self::INDEXES, self::DOCUMENT_SECURITY]; + /** + * The collection's permissions as they were written, kept only when one of them names a column. + * + * $permissions holds the same set with every column dropped, because that is the form every + * read gate matches against -- the `_perms` side table, Postgres' JSONB containment, Mongo's + * `$in`. Keeping the column there would make a collection granted `read("any", "name")` + * invisible to `any`, since no gate would find the `read("any")` it looks for. + * + * So this field is the collection's permissions and $permissions is a derived index of them. + * Read them through getPermissions(); nothing outside a gate should read $permissions directly. + * Empty whenever no permission names a column, which is almost every collection, and then + * $permissions is already the verbatim set. + */ + public const string COLUMN_PERMISSIONS = 'columnPermissions'; + + private const array CORE_KEYS = [self::ATTRIBUTES, self::INDEXES, self::DOCUMENT_SECURITY, self::COLUMN_PERMISSIONS]; /** @var list|null */ private ?array $attributeModels = null; @@ -80,7 +96,7 @@ public static function create( ]; if ($permissions !== null) { - $data[self::PERMISSIONS] = $permissions; + $data = \array_merge($data, self::splitPermissions($permissions)); } return new self(\array_merge($data, $metadata)); @@ -124,6 +140,44 @@ public static function fromDocument(Document $document): self return $document instanceof self ? $document : self::fromArray($document->getArrayCopy()); } + /** + * Replace the permission set, writing the gate and the verbatim list together. + * + * The only place the two fields are set, so they cannot drift: a caller that writes + * $permissions alone would leave a stale verbatim list behind it, and getPermissions() + * would keep answering with the permissions the collection used to have. + * + * @param list $permissions + * + * @throws DatabaseException + */ + public function setPermissions(array $permissions): self + { + foreach (self::splitPermissions($permissions) as $key => $value) { + $this->setAttribute($key, $value); + } + + return $this; + } + + /** + * @param list $permissions + * @return array> + * + * @throws DatabaseException + */ + private static function splitPermissions(array $permissions): array + { + $scoped = Permission::hasColumns($permissions); + + return [ + self::PERMISSIONS => $scoped ? Permission::withoutColumns($permissions) : $permissions, + // Cleared rather than left alone, so dropping the last column-scoped grant from a + // collection also drops the list that used to describe it. + self::COLUMN_PERMISSIONS => $scoped ? $permissions : [], + ]; + } + public function toDocument(): Document { $document = new Document(); @@ -265,11 +319,48 @@ public function documentSecurity(): bool } /** - * @return list|null + * The collection's permissions as they were written, columns included. + * + * getPermissions() answers with what is stored, which is the role-only projection of these: + * every read gate matches that form, so a column stored there would hide the collection from + * the role it was granted to. Ask here for the grants themselves -- validation, enforcement + * and anything reporting them to a caller -- and ask getPermissions() for what the gate sees. + * + * @return list|null null when the collection declares no permissions + * + * @throws StructureException when the stored grants are not a list of permission strings */ public function declaredPermissions(): ?array { - return $this->offsetExists(self::PERMISSIONS) ? $this->getPermissions() : null; + if (! $this->offsetExists(self::PERMISSIONS) && ! $this->offsetExists(self::COLUMN_PERMISSIONS)) { + return null; + } + + $verbatim = $this->getAttribute(self::COLUMN_PERMISSIONS, []); + + // A definition read straight from storage reaches here before the json filter has run. + if (\is_string($verbatim) && $verbatim !== '') { + $verbatim = \json_decode($verbatim, true); + } + + if ($verbatim === null || $verbatim === '' || $verbatim === []) { + return $this->getPermissions(); + } + + // Never fall back on a value that is present but unreadable. $permissions holds the same + // grants widened to the whole row, so answering with it would quietly report more access + // than the collection was given. + if (! \is_array($verbatim)) { + throw new StructureException('Collection '.self::COLUMN_PERMISSIONS.' must be a list of permission strings'); + } + + foreach ($verbatim as $permission) { + if (! \is_string($permission)) { + throw new StructureException('Collection '.self::COLUMN_PERMISSIONS.' must be a list of permission strings'); + } + } + + return \array_values($verbatim); } #[\Override] diff --git a/src/Database/Database.php b/src/Database/Database.php index 1308e1aa0..de5344b52 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -182,6 +182,8 @@ class Database private const string COLLECTION_DOCUMENT_SECURITY = Collection::DOCUMENT_SECURITY; + private const string COLLECTION_COLUMN_PERMISSIONS = Collection::COLUMN_PERMISSIONS; + private const string INDEX_ATTRIBUTES = 'attributes'; /** @@ -199,6 +201,7 @@ class Database self::COLLECTION_ATTRIBUTES => true, self::COLLECTION_INDEXES => true, self::COLLECTION_DOCUMENT_SECURITY => true, + self::COLLECTION_COLUMN_PERMISSIONS => true, ]; protected Adapter $adapter; @@ -257,9 +260,6 @@ class Database /** @var Value|null */ private ?Value $validation = null; - /** @var Value|null */ - private ?Value $foreignColumnPermissions = null; - /** @var Value|null */ private ?Value $datePreservation = null; @@ -1315,32 +1315,6 @@ public function skipValidation(callable $callback): mixed return $this->withValidation(false, $callback); } - /** - * Run the callback accepting column-scoped permissions on documents whose columns belong - * to another collection. - * - * A document's permissions are normally judged against its own collection's columns, and a - * column-scoped one is refused because there is nothing here it could correctly name. That - * is wrong for a document that *records* another collection's permissions rather than - * carrying its own -- a collection definition does this, and so does any row a caller keeps - * a collection's settings in. Those permissions are enforced where they belong, on the - * collection they describe, and judging them here would only compare them to the wrong - * schema. - * - * Scoped to the callback rather than set once, so the exemption covers the one write that - * needs it instead of every write that follows. Nothing is enforced inside: this says the - * permissions are a record, not a rule. - * - * @template T - * - * @param callable(): T $callback - * @return T - */ - public function withForeignColumnPermissions(callable $callback): mixed - { - return $this->foreignColumnPermissions()->with(true, $callback); - } - /** * Register a hook into the database pipeline. * @@ -1679,22 +1653,6 @@ private function validation(): Value return $this->validation ??= new Value(true); } - /** - * @return Value - */ - private function foreignColumnPermissions(): Value - { - return $this->foreignColumnPermissions ??= new Value(false); - } - - /** - * Whether a document's permissions may name a column belonging to another collection. - */ - public function hasForeignColumnPermissions(): bool - { - return $this->foreignColumnPermissions()->get(); - } - /** * @return Value */ @@ -2613,6 +2571,7 @@ private static function sharedDefinition(): Collection Attribute::string(self::COLLECTION_ATTRIBUTES, 1_000_000, filters: [Filter::Json]), Attribute::string(self::COLLECTION_INDEXES, 1_000_000, filters: [Filter::Json]), Attribute::boolean(self::COLLECTION_DOCUMENT_SECURITY, required: true), + Attribute::string(self::COLLECTION_COLUMN_PERMISSIONS, 1_000_000, filters: [Filter::Json]), ], documentSecurity: false, metadata: [Document::COLLECTION => self::METADATA], diff --git a/src/Database/Permission.php b/src/Database/Permission.php index 0059c5b3f..eac48fbdb 100644 --- a/src/Database/Permission.php +++ b/src/Database/Permission.php @@ -254,6 +254,58 @@ public static function aggregate(?array $permissions, array $allowed = [Permissi return \array_values(\array_unique($mutated)); } + /** + * Does any of these permissions name a column? + * + * @param list $permissions + * + * @throws DatabaseException + */ + public static function hasColumns(array $permissions): bool + { + foreach ($permissions as $permission) { + if (! self::parse($permission)->isForAllColumns()) { + return true; + } + } + + return false; + } + + /** + * The same permissions with every column dropped, deduplicated. + * + * Whether a caller may reach a row is a question about the role alone: a column narrows what + * a permission reaches, not who holds it. This is the form a read gate matches against -- + * a side table of roles, a JSONB containment, a Mongo `$in` -- so the column is kept only + * where the scope is enforced and never where access is decided. A gate handed the column + * would be comparing `read("any", "name")` against `read("any")` and finding nothing. + * + * @param list $permissions + * @return list + * + * @throws DatabaseException + */ + public static function withoutColumns(array $permissions): array + { + $stripped = []; + + foreach ($permissions as $permission) { + $parsed = self::parse($permission); + + $stripped[] = $parsed->isForAllColumns() + ? $permission + : (new self( + $parsed->getPermission(), + $parsed->getRole(), + $parsed->getIdentifier(), + $parsed->getDimension() + ))->toString(); + } + + return \array_values(\array_unique($stripped)); + } + /** * Create a read permission string from the given Role. * diff --git a/src/Database/Trait/Collections.php b/src/Database/Trait/Collections.php index 8af929a64..23f966d82 100644 --- a/src/Database/Trait/Collections.php +++ b/src/Database/Trait/Collections.php @@ -177,7 +177,7 @@ public function updateCollection(string $collection, CollectionUpdate $update): } if ($update->permissions !== null) { - $definition->setAttribute(Document::PERMISSIONS, $update->permissions); + $definition->setPermissions($update->permissions); } if ($update->documentSecurity !== null) { diff --git a/src/Database/Trait/Documents.php b/src/Database/Trait/Documents.php index 6a3bc2cc6..f8e6fcd52 100644 --- a/src/Database/Trait/Documents.php +++ b/src/Database/Trait/Documents.php @@ -1229,18 +1229,13 @@ private function prepareDocument(Document $collection, Document $document): Docu $document = $this->encode($collection, $document); - // A metadata document is a collection definition, whose permissions name that - // collection's columns rather than _metadata's, and createCollection() and - // updateCollection() have already judged them against the right list. - // withForeignColumnPermissions() says the same of any other document a caller keeps a - // collection's settings in. Both are records of permissions enforced elsewhere, so - // judging them here would only ever compare them to the wrong schema. - $recordsForeignPermissions = $collection->getId() === Database::METADATA - || $this->hasForeignColumnPermissions(); - - if ($this->validation()->get() && ! $recordsForeignPermissions) { - // No column list on purpose: column permissions are set on the collection, so a - // document naming one is refused rather than stored and never enforced. + if ($this->validation()->get()) { + // No column list on purpose, and no exemption: a column-scoped permission belongs to + // a collection, and a row naming one is refused rather than stored somewhere nothing + // enforces it. It is also what every read gate matches against -- a side table of + // roles, a JSONB containment, a Mongo `$in` -- so a column stored here would hide the + // row from the role it was granted to. + // $validator = new Permissions(columns: null); if (! $validator->isValid($document->getPermissions())) { throw new DatabaseException($validator->getDescription()); diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index d6080f81d..02dbdbe5b 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -43,26 +43,28 @@ abstract class Base extends TestCase { - use AggregationTests; - use AttributeTests; - use CollectionTests; use ColumnPermissionTests; - use CustomDocumentTypeTests; - use DatabaseTests; - use DocumentTests; - use GeneralTests; - use IndexTests; - use JoinComboTests; - use JoinTests; - use MetadataCacheTests; - use ObjectAttributeTests; - use OperatorTests; - use PermissionTests; - use RelationshipTests; - use SchemaReconciliationTests; - use SchemalessTests; - use SpatialTests; - use VectorTests; + +// use AggregationTests; +// use AttributeTests; +// use CollectionTests; +// use ColumnPermissionTests; +// use CustomDocumentTypeTests; +// use DatabaseTests; +// use DocumentTests; +// use GeneralTests; +// use IndexTests; +// use JoinComboTests; +// use JoinTests; +// use MetadataCacheTests; +// use ObjectAttributeTests; +// use OperatorTests; +// use PermissionTests; +// use RelationshipTests; +// use SchemaReconciliationTests; +// use SchemalessTests; +// use SpatialTests; +// use VectorTests; /** * @var array diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php index fe2943709..55bf017ad 100644 --- a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -9,6 +9,7 @@ use Utopia\Database\Document; use Utopia\Database\Exception as DatabaseException; use Utopia\Database\Permission; +use Utopia\Database\PermissionType; use Utopia\Database\Role; /** @@ -40,9 +41,144 @@ public function testACollectionPermissionMayNameOneOfItsColumns(): void $stored = $database->getCollection('columnGrants'); - $this->assertContains('read("any", "name")', $stored->getPermissions()); - $this->assertContains('read("team:hr", "salary")', $stored->getPermissions()); - $this->assertContains('create("any")', $stored->getPermissions()); + $this->assertContains('read("any", "name")', $stored->declaredPermissions()); + $this->assertContains('read("team:hr", "salary")', $stored->declaredPermissions()); + $this->assertContains('create("any")', $stored->declaredPermissions()); + } + + /** + * Regression: a column-scoped grant must not hide the collection from the role it names. + * + * Listing applies the permission gate, and two adapters answer it from the row's own + * permission list rather than a side table of roles -- Postgres by JSONB containment, + * Mongo by an exact `$in`. Both look for `read("any")`, so a stored `read("any", "name")` + * matched nothing and the collection vanished for the very role it was granted to, while + * getCollection() -- which carries no gate -- still returned it. + */ + public function testACollectionWithAColumnScopedGrantIsStillListed(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + $authorization = $database->getAuthorization(); + + $database->createCollection(Collection::create( + id: 'columnGrantsListed', + attributes: [Attribute::string(key: 'name', size: 64)], + permissions: [Permission::read(Role::any(), 'name')], + )); + + $previous = $authorization->getRoles(); + $authorization->cleanRoles(); + $authorization->addRole(Role::any()->toString()); + + try { + $ids = \array_map( + static fn (Collection $collection): string => $collection->getId(), + $database->listCollections(100), + ); + } finally { + $authorization->cleanRoles(); + foreach ($previous as $role) { + $authorization->addRole($role); + } + } + + $this->assertContains('columnGrantsListed', $ids); + } + + /** + * The grants are stored as they were written; only the gate is narrowed to roles. + */ + public function testAColumnScopedGrantIsReadBackAsItWasWritten(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + $database->createCollection(Collection::create( + id: 'columnGrantsVerbatim', + attributes: [ + Attribute::string(key: 'name', size: 64), + Attribute::integer(key: 'salary'), + ], + permissions: [ + Permission::read(Role::any(), 'name'), + Permission::read(Role::team('hr'), 'salary'), + Permission::create(Role::any()), + ], + )); + + $stored = $database->getCollection('columnGrantsVerbatim'); + + $this->assertSame([ + 'read("any", "name")', + 'read("team:hr", "salary")', + 'create("any")', + ], $stored->declaredPermissions()); + + // The gate keeps the roles and drops the columns, which is what every adapter matches. + $this->assertSame(['any', 'team:hr'], $stored->getPermissionsByType(PermissionType::Read)); + + // And the column the gates read holds the projection, not the grants. Postgres matches + // it by JSONB containment and Mongo by an exact `$in`, so a column stored here would + // hide the collection from the role it names while getCollection() still returned it. + $this->assertSame([ + 'read("any")', + 'read("team:hr")', + 'create("any")', + ], $stored->getPermissions()); + } + + /** + * A collection naming no column stores what it has always stored: nothing is duplicated + * for the collections that do not use the feature. + */ + public function testACollectionWithoutColumnGrantsStoresNothingExtra(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + $database->createCollection(Collection::create( + id: 'columnGrantsAbsent', + attributes: [Attribute::string(key: 'name', size: 64)], + permissions: [Permission::read(Role::any()), Permission::create(Role::any())], + )); + + $stored = $database->getCollection('columnGrantsAbsent'); + + $this->assertSame(['read("any")', 'create("any")'], $stored->declaredPermissions()); + $this->assertSame([], $stored->getAttribute(Collection::COLUMN_PERMISSIONS)); + } + + /** + * Dropping the last column-scoped grant also drops the list that used to describe it, + * so a later read does not answer with permissions the collection no longer has. + */ + public function testRemovingTheLastColumnGrantClearsTheStoredGrants(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + $collection = $database->createCollection(Collection::create( + id: 'columnGrantsCleared', + attributes: [Attribute::string(key: 'name', size: 64)], + permissions: [Permission::read(Role::any(), 'name'), Permission::create(Role::any())], + )); + + $this->assertSame(['read("any", "name")', 'create("any")'], $collection->declaredPermissions()); + $this->assertSame(['read("any")', 'create("any")'], $collection->getPermissions()); + + $collection = $database->updateCollection('columnGrantsCleared', new CollectionUpdate(permissions: [ + Permission::read(Role::any()), + Permission::create(Role::any()), + ])); + + $this->assertSame(['read("any")', 'create("any")'], $collection->declaredPermissions()); + $this->assertSame(['read("any")', 'create("any")'], $collection->getPermissions()); + + $stored = $database->getCollection('columnGrantsCleared'); + + $this->assertSame([], $stored->getAttribute(Collection::COLUMN_PERMISSIONS)); + $this->assertSame(['read("any")', 'create("any")'], $stored->declaredPermissions()); } public function testACollectionPermissionMayNotNameAColumnThatDoesNotExist(): void @@ -83,7 +219,7 @@ public function testUpdatingACollectionJudgesAColumnGrantAgainstItsOwnColumns(): $this->assertContains( 'read("any", "name")', - $database->getCollection('columnGrantsUpdate')->getPermissions(), + $database->getCollection('columnGrantsUpdate')->declaredPermissions(), ); try { @@ -98,7 +234,7 @@ public function testUpdatingACollectionJudgesAColumnGrantAgainstItsOwnColumns(): // The rejected update left the collection as it was. $this->assertContains( 'read("any", "name")', - $database->getCollection('columnGrantsUpdate')->getPermissions(), + $database->getCollection('columnGrantsUpdate')->declaredPermissions(), ); } @@ -135,14 +271,20 @@ public function testADocumentPermissionMayNotNameAColumn(): void // The rejected write left nothing behind. $this->assertTrue($database->getDocument('columnGrantsDocument', 'row')->isEmpty()); } - - public function testADocumentMayRecordAnotherCollectionsColumnPermissions(): void + /** + * A row that records another collection's grants is refused like any other. + * + * Appwrite keeps one of these per table, and the grants it records name that table's + * columns rather than the row's. There is no waiver for it: a column stored in a row's + * permissions is enforced nowhere, and the read gate -- which matches roles, not grants -- + * would hide the row from the role it names. The caller splits them instead, keeping the + * verbatim grants in a column of their own and the roles in the permissions. + */ + public function testADocumentMayNotRecordAnotherCollectionsColumnPermissions(): void { /** @var Database $database */ $database = $this->getDatabase(); - // Stands in for a row a caller keeps another collection's settings in -- Appwrite's - // table list does this. Its own columns are nothing like the ones its permissions name. $database->createCollection(Collection::create( id: 'columnGrantsRecords', attributes: [ @@ -153,106 +295,22 @@ public function testADocumentMayRecordAnotherCollectionsColumnPermissions(): voi documentSecurity: true, )); - $recorded = [ - Permission::read(Role::any(), 'name'), - Permission::read(Role::team('hr'), 'salary'), - ]; - try { $database->createDocument('columnGrantsRecords', new Document([ '$id' => 'row', 'tableId' => 'employees', 'label' => 'Employees', - '$permissions' => $recorded, - ])); - $this->fail('Expected createDocument() to reject columns belonging to another collection'); - } catch (DatabaseException $e) { - $this->assertStringContainsString('is scoped to a column', $e->getMessage()); - } - - $database->withForeignColumnPermissions(fn () => $database->createDocument('columnGrantsRecords', new Document([ - '$id' => 'row', - 'tableId' => 'employees', - 'label' => 'Employees', - '$permissions' => $recorded, - ]))); - - $stored = $database->getDocument('columnGrantsRecords', 'row'); - - $this->assertSame('employees', $stored->getAttribute('tableId')); - $this->assertContains('read("any", "name")', $stored->getPermissions()); - $this->assertContains('read("team:hr", "salary")', $stored->getPermissions()); - - // Scoped to the callback: the next write is judged as strictly as the first. - $this->assertFalse($database->hasForeignColumnPermissions()); - } - - public function testARecordedColumnGrantStillDecidesRowVisibilityByRoleAlone(): void - { - /** @var Database $database */ - $database = $this->getDatabase(); - $authorization = $database->getAuthorization(); - - // Appwrite's table list: each row records one table's permissions, column grants and - // all. Whether a caller may see the row is a question about the role alone, so a column - // narrows what the permission reaches without narrowing who holds it. - $database->createCollection(Collection::create( - id: 'columnGrantsListing', - attributes: [Attribute::string(key: 'tableId', size: 64)], - permissions: [Permission::create(Role::any())], - documentSecurity: true, - )); - - $database->withForeignColumnPermissions(function () use ($database) { - $database->createDocument('columnGrantsListing', new Document([ - '$id' => 'employees', - 'tableId' => 'employees', '$permissions' => [ Permission::read(Role::any(), 'name'), Permission::read(Role::team('hr'), 'salary'), ], ])); - $database->createDocument('columnGrantsListing', new Document([ - '$id' => 'invoices', - 'tableId' => 'invoices', - '$permissions' => [Permission::read(Role::team('finance'))], - ])); - }); - - $listed = function (array $roles) use ($database, $authorization): array { - $previous = $authorization->getRoles(); - $authorization->cleanRoles(); - foreach ($roles as $role) { - $authorization->addRole($role); - } - - try { - $ids = \array_map( - static fn (Document $document): string => $document->getId(), - $database->find('columnGrantsListing'), - ); - } finally { - $authorization->cleanRoles(); - foreach ($previous as $role) { - $authorization->addRole($role); - } - } - - \sort($ids); - - return $ids; - }; + $this->fail('Expected createDocument() to reject columns belonging to another collection'); + } catch (DatabaseException $e) { + $this->assertStringContainsString('is scoped to a column', $e->getMessage()); + } - $this->assertSame( - ['employees'], - $listed([Role::any()->toString()]), - 'A grant scoped to a column still admits its role to the row', - ); - $this->assertSame( - ['invoices'], - $listed([Role::team('finance')->toString()]), - 'A role named by no grant on a row does not see it', - ); + $this->assertTrue($database->getDocument('columnGrantsRecords', 'row')->isEmpty()); } public function testADocumentPermissionNamingNoColumnIsUnaffected(): void diff --git a/tests/unit/Collections/CollectionDefinitionTest.php b/tests/unit/Collections/CollectionDefinitionTest.php index 11f21d619..d2b5b9432 100644 --- a/tests/unit/Collections/CollectionDefinitionTest.php +++ b/tests/unit/Collections/CollectionDefinitionTest.php @@ -36,6 +36,7 @@ public function testTheMetadataAttributesKeepTheirStoredShape(): void 'attributes' => [ColumnType::String, 1_000_000, false, ['json']], 'indexes' => [ColumnType::String, 1_000_000, false, ['json']], 'documentSecurity' => [ColumnType::Boolean, null, true, []], + 'columnPermissions' => [ColumnType::String, 1_000_000, false, ['json']], ], $attributes); } @@ -48,7 +49,7 @@ public function testEveryCallReturnsAnIndependentCopy(): void $second = Database::collectionDefinition(); $this->assertSame('collections', $second->name()); - $this->assertCount(4, $second->attributes()); + $this->assertCount(5, $second->attributes()); } public function testGetCollectionReturnsTheDefinitionForTheMetadataCollection(): void diff --git a/tests/unit/PermissionTest.php b/tests/unit/PermissionTest.php index 72b919a0b..7533a0a04 100644 --- a/tests/unit/PermissionTest.php +++ b/tests/unit/PermissionTest.php @@ -405,4 +405,57 @@ public function test_aggregation_keeps_the_column(): void 'delete("any", "salary")', ], $parsed); } + + /** + * The form a read gate matches against: every grant with its column dropped. + * + * @throws \Exception + */ + public function test_without_columns(): void + { + $this->assertSame( + ['read("any")', 'read("team:hr")', 'create("any")'], + Permission::withoutColumns([ + 'read("any", "name")', + 'read("team:hr", "salary")', + 'create("any")', + ]), + ); + } + + /** + * Two grants narrowing the same role to different columns admit that role once. + * + * @throws \Exception + */ + public function test_without_columns_deduplicates(): void + { + $this->assertSame( + ['read("any")'], + Permission::withoutColumns(['read("any", "name")', 'read("any", "salary")']), + ); + } + + /** + * A set naming no column is its own projection, so a collection that does not use the + * feature stores exactly what it stores today. + * + * @throws \Exception + */ + public function test_without_columns_leaves_unscoped_permissions_alone(): void + { + $permissions = ['read("any")', 'update("user:alice")', 'create("users")']; + + $this->assertSame($permissions, Permission::withoutColumns($permissions)); + } + + /** + * @throws \Exception + */ + public function test_has_columns(): void + { + $this->assertTrue(Permission::hasColumns(['create("any")', 'read("any", "name")'])); + $this->assertFalse(Permission::hasColumns(['create("any")', 'read("any")'])); + $this->assertFalse(Permission::hasColumns([])); + } } From 93dfdc5ff75d376b987bca4b7543eb05e15b0412 Mon Sep 17 00:00:00 2001 From: fogelito Date: Mon, 12 Oct 2026 14:44:57 +0300 Subject: [PATCH 03/29] Base --- tests/e2e/Adapter/Base.php | 40 ++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 02dbdbe5b..d6080f81d 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -43,28 +43,26 @@ abstract class Base extends TestCase { + use AggregationTests; + use AttributeTests; + use CollectionTests; use ColumnPermissionTests; - -// use AggregationTests; -// use AttributeTests; -// use CollectionTests; -// use ColumnPermissionTests; -// use CustomDocumentTypeTests; -// use DatabaseTests; -// use DocumentTests; -// use GeneralTests; -// use IndexTests; -// use JoinComboTests; -// use JoinTests; -// use MetadataCacheTests; -// use ObjectAttributeTests; -// use OperatorTests; -// use PermissionTests; -// use RelationshipTests; -// use SchemaReconciliationTests; -// use SchemalessTests; -// use SpatialTests; -// use VectorTests; + use CustomDocumentTypeTests; + use DatabaseTests; + use DocumentTests; + use GeneralTests; + use IndexTests; + use JoinComboTests; + use JoinTests; + use MetadataCacheTests; + use ObjectAttributeTests; + use OperatorTests; + use PermissionTests; + use RelationshipTests; + use SchemaReconciliationTests; + use SchemalessTests; + use SpatialTests; + use VectorTests; /** * @var array From d79830f96238678115cf14f86e9ca936cccdd71f Mon Sep 17 00:00:00 2001 From: fogelito Date: Mon, 12 Oct 2026 16:42:04 +0300 Subject: [PATCH 04/29] Validate COLUMN_PERMISSIONS --- src/Database/Trait/Documents.php | 23 ++++++++++++++++++ tests/e2e/Adapter/Base.php | 40 +++++++++++++++++--------------- 2 files changed, 44 insertions(+), 19 deletions(-) diff --git a/src/Database/Trait/Documents.php b/src/Database/Trait/Documents.php index f8e6fcd52..801d14d19 100644 --- a/src/Database/Trait/Documents.php +++ b/src/Database/Trait/Documents.php @@ -1227,6 +1227,29 @@ private function prepareDocument(Document $collection, Document $document): Docu } } + // Before encode(), which turns both of these into JSON strings. + // + // A collection definition records the grants it was given, columns and all, beside the + // role-only projection of them that goes in $permissions. The rule below never sees them + // -- by the time they reach a row they are not in $permissions any more -- so they are + // judged here, against the columns the definition itself declares. Without this, a write + // that reaches past createCollection() could record a grant on a column that does not + // exist, and nothing would ever say so. + if ($this->validation()->get() && $collection->getId() === Database::METADATA) { + $columns = []; + foreach ($document->getAttribute(Collection::ATTRIBUTES, []) as $attribute) { + $key = $attribute instanceof Document ? $attribute->getAttribute('key') : ($attribute['key'] ?? null); + if (\is_string($key) && $key !== '') { + $columns[] = $key; + } + } + + $validator = new Permissions(columns: $columns); + if (! $validator->isValid($document->getAttribute(Collection::COLUMN_PERMISSIONS, []))) { + throw new DatabaseException($validator->getDescription()); + } + } + $document = $this->encode($collection, $document); if ($this->validation()->get()) { diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index d6080f81d..02dbdbe5b 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -43,26 +43,28 @@ abstract class Base extends TestCase { - use AggregationTests; - use AttributeTests; - use CollectionTests; use ColumnPermissionTests; - use CustomDocumentTypeTests; - use DatabaseTests; - use DocumentTests; - use GeneralTests; - use IndexTests; - use JoinComboTests; - use JoinTests; - use MetadataCacheTests; - use ObjectAttributeTests; - use OperatorTests; - use PermissionTests; - use RelationshipTests; - use SchemaReconciliationTests; - use SchemalessTests; - use SpatialTests; - use VectorTests; + +// use AggregationTests; +// use AttributeTests; +// use CollectionTests; +// use ColumnPermissionTests; +// use CustomDocumentTypeTests; +// use DatabaseTests; +// use DocumentTests; +// use GeneralTests; +// use IndexTests; +// use JoinComboTests; +// use JoinTests; +// use MetadataCacheTests; +// use ObjectAttributeTests; +// use OperatorTests; +// use PermissionTests; +// use RelationshipTests; +// use SchemaReconciliationTests; +// use SchemalessTests; +// use SpatialTests; +// use VectorTests; /** * @var array From 7c635b12ab749cd6ad9d98542f60c4ce278cc89c Mon Sep 17 00:00:00 2001 From: fogelito Date: Mon, 12 Oct 2026 16:42:38 +0300 Subject: [PATCH 05/29] Base --- tests/e2e/Adapter/Base.php | 40 ++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 02dbdbe5b..d6080f81d 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -43,28 +43,26 @@ abstract class Base extends TestCase { + use AggregationTests; + use AttributeTests; + use CollectionTests; use ColumnPermissionTests; - -// use AggregationTests; -// use AttributeTests; -// use CollectionTests; -// use ColumnPermissionTests; -// use CustomDocumentTypeTests; -// use DatabaseTests; -// use DocumentTests; -// use GeneralTests; -// use IndexTests; -// use JoinComboTests; -// use JoinTests; -// use MetadataCacheTests; -// use ObjectAttributeTests; -// use OperatorTests; -// use PermissionTests; -// use RelationshipTests; -// use SchemaReconciliationTests; -// use SchemalessTests; -// use SpatialTests; -// use VectorTests; + use CustomDocumentTypeTests; + use DatabaseTests; + use DocumentTests; + use GeneralTests; + use IndexTests; + use JoinComboTests; + use JoinTests; + use MetadataCacheTests; + use ObjectAttributeTests; + use OperatorTests; + use PermissionTests; + use RelationshipTests; + use SchemaReconciliationTests; + use SchemalessTests; + use SpatialTests; + use VectorTests; /** * @var array From 0ee30b6ced6f25cce9347341fdc0b13beea3c121 Mon Sep 17 00:00:00 2001 From: fogelito Date: Mon, 12 Oct 2026 16:59:50 +0300 Subject: [PATCH 06/29] codeQL fix --- src/Database/Trait/Documents.php | 11 ++++++-- .../Adapter/Scopes/ColumnPermissionTests.php | 25 +++++++++++-------- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/Database/Trait/Documents.php b/src/Database/Trait/Documents.php index 801d14d19..e330e383f 100644 --- a/src/Database/Trait/Documents.php +++ b/src/Database/Trait/Documents.php @@ -1236,9 +1236,16 @@ private function prepareDocument(Document $collection, Document $document): Docu // that reaches past createCollection() could record a grant on a column that does not // exist, and nothing would ever say so. if ($this->validation()->get() && $collection->getId() === Database::METADATA) { + $declared = $document->getAttribute(Collection::ATTRIBUTES, []); $columns = []; - foreach ($document->getAttribute(Collection::ATTRIBUTES, []) as $attribute) { - $key = $attribute instanceof Document ? $attribute->getAttribute('key') : ($attribute['key'] ?? null); + + foreach (\is_array($declared) ? $declared : [] as $attribute) { + $key = match (true) { + $attribute instanceof Document => $attribute->getAttribute('key'), + \is_array($attribute) => $attribute['key'] ?? null, + default => null, + }; + if (\is_string($key) && $key !== '') { $columns[] = $key; } diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php index 55bf017ad..5281bc10c 100644 --- a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -41,9 +41,12 @@ public function testACollectionPermissionMayNameOneOfItsColumns(): void $stored = $database->getCollection('columnGrants'); - $this->assertContains('read("any", "name")', $stored->declaredPermissions()); - $this->assertContains('read("team:hr", "salary")', $stored->declaredPermissions()); - $this->assertContains('create("any")', $stored->declaredPermissions()); + $permissions = $stored->declaredPermissions(); + + $this->assertNotNull($permissions); + $this->assertContains('read("any", "name")', $permissions); + $this->assertContains('read("team:hr", "salary")', $permissions); + $this->assertContains('create("any")', $permissions); } /** @@ -217,10 +220,10 @@ public function testUpdatingACollectionJudgesAColumnGrantAgainstItsOwnColumns(): Permission::create(Role::any()), ])); - $this->assertContains( - 'read("any", "name")', - $database->getCollection('columnGrantsUpdate')->declaredPermissions(), - ); + $updated = $database->getCollection('columnGrantsUpdate')->declaredPermissions(); + + $this->assertNotNull($updated); + $this->assertContains('read("any", "name")', $updated); try { $database->updateCollection('columnGrantsUpdate', new CollectionUpdate(permissions: [ @@ -232,10 +235,10 @@ public function testUpdatingACollectionJudgesAColumnGrantAgainstItsOwnColumns(): } // The rejected update left the collection as it was. - $this->assertContains( - 'read("any", "name")', - $database->getCollection('columnGrantsUpdate')->declaredPermissions(), - ); + $updated = $database->getCollection('columnGrantsUpdate')->declaredPermissions(); + + $this->assertNotNull($updated); + $this->assertContains('read("any", "name")', $updated); } public function testADocumentPermissionMayNotNameAColumn(): void From 7a1668780cde6ca3f43b090c3d894cd97bb5c821 Mon Sep 17 00:00:00 2001 From: fogelito Date: Tue, 13 Oct 2026 08:31:51 +0300 Subject: [PATCH 07/29] change declaredPermissions to columnPermissions --- src/Database/Collection.php | 2 +- src/Database/Database.php | 2 +- src/Database/Trait/Collections.php | 2 +- .../Adapter/Scopes/ColumnPermissionTests.php | 110 ++++++++++++++++-- tests/unit/Model/CollectionTest.php | 14 +-- 5 files changed, 112 insertions(+), 18 deletions(-) diff --git a/src/Database/Collection.php b/src/Database/Collection.php index ac9e52ea6..0e6b74602 100644 --- a/src/Database/Collection.php +++ b/src/Database/Collection.php @@ -330,7 +330,7 @@ public function documentSecurity(): bool * * @throws StructureException when the stored grants are not a list of permission strings */ - public function declaredPermissions(): ?array + public function columnPermissions(): ?array { if (! $this->offsetExists(self::PERMISSIONS) && ! $this->offsetExists(self::COLUMN_PERMISSIONS)) { return null; diff --git a/src/Database/Database.php b/src/Database/Database.php index de5344b52..e46ff4d32 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -2571,7 +2571,7 @@ private static function sharedDefinition(): Collection Attribute::string(self::COLLECTION_ATTRIBUTES, 1_000_000, filters: [Filter::Json]), Attribute::string(self::COLLECTION_INDEXES, 1_000_000, filters: [Filter::Json]), Attribute::boolean(self::COLLECTION_DOCUMENT_SECURITY, required: true), - Attribute::string(self::COLLECTION_COLUMN_PERMISSIONS, 1_000_000, filters: [Filter::Json]), + Attribute::string(self::COLLECTION_COLUMN_PERMISSIONS, 1_000_000, default: [], filters: [Filter::Json]), ], documentSecurity: false, metadata: [Document::COLLECTION => self::METADATA], diff --git a/src/Database/Trait/Collections.php b/src/Database/Trait/Collections.php index 23f966d82..78ffb7509 100644 --- a/src/Database/Trait/Collections.php +++ b/src/Database/Trait/Collections.php @@ -47,7 +47,7 @@ public function createCollection(Collection $collection): Collection $id = $collection->getId(); $attributes = \array_map(self::normalise(...), $collection->attributes()); - $permissions = $collection->declaredPermissions() ?? [Permission::create(Role::any())]; + $permissions = $collection->columnPermissions() ?? [Permission::create(Role::any())]; $typeValidator = $this->typeValidator(); foreach ($attributes as $attribute) { diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php index 5281bc10c..e20493aef 100644 --- a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -41,7 +41,7 @@ public function testACollectionPermissionMayNameOneOfItsColumns(): void $stored = $database->getCollection('columnGrants'); - $permissions = $stored->declaredPermissions(); + $permissions = $stored->columnPermissions(); $this->assertNotNull($permissions); $this->assertContains('read("any", "name")', $permissions); @@ -116,7 +116,7 @@ public function testAColumnScopedGrantIsReadBackAsItWasWritten(): void 'read("any", "name")', 'read("team:hr", "salary")', 'create("any")', - ], $stored->declaredPermissions()); + ], $stored->columnPermissions()); // The gate keeps the roles and drops the columns, which is what every adapter matches. $this->assertSame(['any', 'team:hr'], $stored->getPermissionsByType(PermissionType::Read)); @@ -148,7 +148,7 @@ public function testACollectionWithoutColumnGrantsStoresNothingExtra(): void $stored = $database->getCollection('columnGrantsAbsent'); - $this->assertSame(['read("any")', 'create("any")'], $stored->declaredPermissions()); + $this->assertSame(['read("any")', 'create("any")'], $stored->columnPermissions()); $this->assertSame([], $stored->getAttribute(Collection::COLUMN_PERMISSIONS)); } @@ -167,7 +167,7 @@ public function testRemovingTheLastColumnGrantClearsTheStoredGrants(): void permissions: [Permission::read(Role::any(), 'name'), Permission::create(Role::any())], )); - $this->assertSame(['read("any", "name")', 'create("any")'], $collection->declaredPermissions()); + $this->assertSame(['read("any", "name")', 'create("any")'], $collection->columnPermissions()); $this->assertSame(['read("any")', 'create("any")'], $collection->getPermissions()); $collection = $database->updateCollection('columnGrantsCleared', new CollectionUpdate(permissions: [ @@ -175,13 +175,13 @@ public function testRemovingTheLastColumnGrantClearsTheStoredGrants(): void Permission::create(Role::any()), ])); - $this->assertSame(['read("any")', 'create("any")'], $collection->declaredPermissions()); + $this->assertSame(['read("any")', 'create("any")'], $collection->columnPermissions()); $this->assertSame(['read("any")', 'create("any")'], $collection->getPermissions()); $stored = $database->getCollection('columnGrantsCleared'); $this->assertSame([], $stored->getAttribute(Collection::COLUMN_PERMISSIONS)); - $this->assertSame(['read("any")', 'create("any")'], $stored->declaredPermissions()); + $this->assertSame(['read("any")', 'create("any")'], $stored->columnPermissions()); } public function testACollectionPermissionMayNotNameAColumnThatDoesNotExist(): void @@ -220,7 +220,7 @@ public function testUpdatingACollectionJudgesAColumnGrantAgainstItsOwnColumns(): Permission::create(Role::any()), ])); - $updated = $database->getCollection('columnGrantsUpdate')->declaredPermissions(); + $updated = $database->getCollection('columnGrantsUpdate')->columnPermissions(); $this->assertNotNull($updated); $this->assertContains('read("any", "name")', $updated); @@ -235,7 +235,7 @@ public function testUpdatingACollectionJudgesAColumnGrantAgainstItsOwnColumns(): } // The rejected update left the collection as it was. - $updated = $database->getCollection('columnGrantsUpdate')->declaredPermissions(); + $updated = $database->getCollection('columnGrantsUpdate')->columnPermissions(); $this->assertNotNull($updated); $this->assertContains('read("any", "name")', $updated); @@ -316,6 +316,100 @@ public function testADocumentMayNotRecordAnotherCollectionsColumnPermissions(): $this->assertTrue($database->getDocument('columnGrantsRecords', 'row')->isEmpty()); } + /** + * The shape a consumer keeping a list of its collections has to use. + * + * This mirrors Appwrite: a `database_N` collection whose rows each describe one table, + * carrying that table's grants so the console can show and edit them. + * Those grants name the table's columns, not the row's, so they cannot live in the row's + * permissions: the library refuses them there, and every read gate matches the role form + * anyway, so a column stored in a row's permissions would hide the row from the role it + * names. The consumer splits them -- roles gate the row, the grants ride in a column of + * their own -- which is what the library does for a collection definition, by hand. + */ + public function testAppwriteCollectionsSimulation(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + $authorization = $database->getAuthorization(); + + // The table itself, under the id Appwrite gives it. Its grants name its own columns. + $database->createCollection(Collection::create( + id: 'database_1_collection_1', + attributes: [ + Attribute::string(key: 'name', size: 64), + Attribute::integer(key: 'salary'), + ], + permissions: [ + Permission::read(Role::any(), 'name'), + Permission::read(Role::team('hr'), 'salary'), + ], + )); + + $grants = $database->getCollection('database_1_collection_1')->columnPermissions(); + $this->assertNotNull($grants); + + // Appwrite's list of the database's tables, one row per table. Its columns -- tableId, + // grants -- are nothing like the ones those grants name. + $database->createCollection(Collection::create( + id: 'database_1', + attributes: [ + Attribute::string(key: 'tableId', size: 64), + Attribute::string(key: 'grants', size: 4096, array: true), + ], + permissions: [Permission::create(Role::any())], + documentSecurity: true, + )); + + // Putting them in the row's permissions is refused. + try { + $database->createDocument('database_1', new Document([ + '$id' => 'wrong', + 'tableId' => 'database_1_collection_1', + 'grants' => $grants, + '$permissions' => $grants, + ])); + $this->fail('Expected createDocument() to reject grants naming another collection\'s columns'); + } catch (DatabaseException $e) { + $this->assertStringContainsString('is scoped to a column', $e->getMessage()); + } + + // Splitting them is not. + $database->createDocument('database_1', new Document([ + '$id' => 'employees', + 'tableId' => 'database_1_collection_1', + 'grants' => $grants, + '$permissions' => Permission::withoutColumns($grants), + ])); + + $stored = $database->getDocument('database_1', 'employees'); + + // The grants come back as the table has them, ready to report to a caller... + $this->assertSame($grants, $stored->getAttribute('grants')); + + // ...while the row answers the gate with roles alone. + $this->assertSame(['any', 'team:hr'], $stored->getPermissionsByType(PermissionType::Read)); + + // So a role holding nothing but a column-scoped grant still finds the row it describes. + $previous = $authorization->getRoles(); + $authorization->cleanRoles(); + $authorization->addRole(Role::team('hr')->toString()); + + try { + $ids = \array_map( + static fn (Document $document): string => $document->getId(), + $database->find('database_1'), + ); + } finally { + $authorization->cleanRoles(); + foreach ($previous as $role) { + $authorization->addRole($role); + } + } + + $this->assertSame(['employees'], $ids); + } + public function testADocumentPermissionNamingNoColumnIsUnaffected(): void { /** @var Database $database */ diff --git a/tests/unit/Model/CollectionTest.php b/tests/unit/Model/CollectionTest.php index e1f2c7597..356d70da8 100644 --- a/tests/unit/Model/CollectionTest.php +++ b/tests/unit/Model/CollectionTest.php @@ -44,7 +44,7 @@ public function testCreateStoresEveryField(): void $this->assertSame([ColumnType::String, ColumnType::Integer], \array_map(static fn (Attribute $attribute): ColumnType => $attribute->type, $collection->attributes())); $this->assertSame(['by_title'], \array_map(static fn (Index $index): string => $index->key, $collection->indexes())); $this->assertSame([OrderDirection::Asc], $collection->indexes()[0]->orders); - $this->assertSame(['read("any")'], $collection->declaredPermissions()); + $this->assertSame(['read("any")'], $collection->columnPermissions()); $this->assertFalse($collection->documentSecurity()); $this->assertFalse($collection->getAttribute('documentSecurity')); } @@ -56,7 +56,7 @@ public function testCreateDefaults(): void $this->assertSame('books', $collection->name()); $this->assertSame([], $collection->attributes()); $this->assertSame([], $collection->indexes()); - $this->assertNull($collection->declaredPermissions()); + $this->assertNull($collection->columnPermissions()); $this->assertTrue($collection->documentSecurity()); $this->assertTrue($collection->getAttribute('documentSecurity')); } @@ -78,7 +78,7 @@ public function testCreateStoresAttributesAndIndexesAsDocuments(): void public function testCreateWithNoPermissionsDeclaresAnEmptyList(): void { - $this->assertSame([], Collection::create('books', permissions: [])->declaredPermissions()); + $this->assertSame([], Collection::create('books', permissions: [])->columnPermissions()); } public function testCreateKeepsMetadataInStorage(): void @@ -106,7 +106,7 @@ public function testFromArrayKeepsExtrasAndNormalisesTheCoreKeys(): void $this->assertSame('books', $collection->name()); $this->assertTrue($collection->documentSecurity()); $this->assertTrue($collection->getAttribute('documentSecurity')); - $this->assertNull($collection->declaredPermissions()); + $this->assertNull($collection->columnPermissions()); $this->assertSame('fiction', $collection->getAttribute('category')); $this->assertSame('title', $collection->attributes()[0]->key); $this->assertSame(ColumnType::String, $collection->attributes()[0]->type); @@ -136,7 +136,7 @@ public function testFromArrayDropsPermissionsThatAreNotAList(): void { $collection = Collection::fromArray(['$id' => 'books', '$permissions' => 'read("any")']); - $this->assertNull($collection->declaredPermissions()); + $this->assertNull($collection->columnPermissions()); } public function testFromArrayRejectsInvalidPermissionEntries(): void @@ -205,11 +205,11 @@ public function testDeclaredPermissionsFollowStorage(): void $collection = Collection::create('books'); $collection->setAttribute('$permissions', [Permission::read(Role::any())]); - $this->assertSame(['read("any")'], $collection->declaredPermissions()); + $this->assertSame(['read("any")'], $collection->columnPermissions()); $collection->removeAttribute('$permissions'); - $this->assertNull($collection->declaredPermissions()); + $this->assertNull($collection->columnPermissions()); } public function testDocumentSecurityFollowsStorage(): void From 714adcac3dfa39c4f837e9fc87aea537fdfaade0 Mon Sep 17 00:00:00 2001 From: fogelito Date: Tue, 13 Oct 2026 12:40:11 +0300 Subject: [PATCH 08/29] Add withColumnPermissions --- src/Database/Database.php | 42 ++++++++++ src/Database/Trait/Collections.php | 11 +++ .../Adapter/Scopes/ColumnPermissionTests.php | 80 +++++++++++++++---- 3 files changed, 117 insertions(+), 16 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index e46ff4d32..f14123ea2 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -260,6 +260,9 @@ class Database /** @var Value|null */ private ?Value $validation = null; + /** @var Value|null */ + private ?Value $columnPermissions = null; + /** @var Value|null */ private ?Value $datePreservation = null; @@ -1302,6 +1305,29 @@ public function withValidation(bool $validation, callable $callback): mixed return $this->validation()->with($validation, $callback); } + /** + * Run the callback accepting collection permissions that name a column. + * + * Off by default, and deliberately so until the column is enforced. A grant naming a column + * is recorded, and the role it names is admitted to the row -- which is what every gate + * matches -- but nothing yet limits that role to the column. So a grant written to narrow + * access currently widens it, and the only safe thing to do with a feature in that state is + * to make it unreachable by accident. + * + * Scoped to the callback rather than stored, because this gates writing such a grant, not + * holding one: a collection created through here keeps its grant, and keeps widening, until + * masking and query refusal land and the gate comes off. + * + * @template T + * + * @param callable(): T $callback + * @return T + */ + public function withColumnPermissions(callable $callback): mixed + { + return $this->columnPermissions()->with(true, $callback); + } + /** * Run the callback without validation for the calling coroutine and the coroutines it starts. * @@ -1653,6 +1679,22 @@ private function validation(): Value return $this->validation ??= new Value(true); } + /** + * @return Value + */ + private function columnPermissions(): Value + { + return $this->columnPermissions ??= new Value(false); + } + + /** + * Whether a collection's permissions may name one of its columns. + */ + public function hasColumnPermissions(): bool + { + return $this->columnPermissions()->get(); + } + /** * @return Value */ diff --git a/src/Database/Trait/Collections.php b/src/Database/Trait/Collections.php index 78ffb7509..9023d7923 100644 --- a/src/Database/Trait/Collections.php +++ b/src/Database/Trait/Collections.php @@ -54,6 +54,13 @@ public function createCollection(Collection $collection): Collection $typeValidator->checkType($attribute); } + // Off until the column is enforced: the role a scoped grant names is admitted to the + // row, and nothing yet holds it to the column, so such a grant widens access instead of + // narrowing it. Refused rather than recorded, so no one meets that by accident. + if (! $this->hasColumnPermissions() && Permission::hasColumns($permissions)) { + throw new DatabaseException('Column-scoped permissions are not enabled'); + } + if ($this->validation()->get()) { $validator = new Permissions(columns: \array_map(fn (Attribute $attribute) => $attribute->key, $attributes)); if (! $validator->isValid($permissions)) { @@ -165,6 +172,10 @@ public function updateCollection(string $collection, CollectionUpdate $update): // After the read, not before: a column-scoped permission is judged against this // collection's columns, so there is nothing to judge it with until the definition // is in hand. + if ($update->permissions !== null && ! $this->hasColumnPermissions() && Permission::hasColumns($update->permissions)) { + throw new DatabaseException('Column-scoped permissions are not enabled'); + } + if ($update->permissions !== null && $this->validation()->get()) { $validator = new Permissions(columns: \array_map(fn (Attribute $attribute) => $attribute->key, $definition->attributes())); if (! $validator->isValid($update->permissions)) { diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php index e20493aef..b0f7cad3e 100644 --- a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -26,7 +26,7 @@ public function testACollectionPermissionMayNameOneOfItsColumns(): void /** @var Database $database */ $database = $this->getDatabase(); - $database->createCollection(Collection::create( + $database->withColumnPermissions(fn () => $database->createCollection(Collection::create( id: 'columnGrants', attributes: [ Attribute::string(key: 'name', size: 64), @@ -37,7 +37,7 @@ public function testACollectionPermissionMayNameOneOfItsColumns(): void Permission::read(Role::team('hr'), 'salary'), Permission::create(Role::any()), ], - )); + ))); $stored = $database->getCollection('columnGrants'); @@ -64,11 +64,11 @@ public function testACollectionWithAColumnScopedGrantIsStillListed(): void $database = $this->getDatabase(); $authorization = $database->getAuthorization(); - $database->createCollection(Collection::create( + $database->withColumnPermissions(fn () => $database->createCollection(Collection::create( id: 'columnGrantsListed', attributes: [Attribute::string(key: 'name', size: 64)], permissions: [Permission::read(Role::any(), 'name')], - )); + ))); $previous = $authorization->getRoles(); $authorization->cleanRoles(); @@ -97,7 +97,7 @@ public function testAColumnScopedGrantIsReadBackAsItWasWritten(): void /** @var Database $database */ $database = $this->getDatabase(); - $database->createCollection(Collection::create( + $database->withColumnPermissions(fn () => $database->createCollection(Collection::create( id: 'columnGrantsVerbatim', attributes: [ Attribute::string(key: 'name', size: 64), @@ -108,7 +108,7 @@ public function testAColumnScopedGrantIsReadBackAsItWasWritten(): void Permission::read(Role::team('hr'), 'salary'), Permission::create(Role::any()), ], - )); + ))); $stored = $database->getCollection('columnGrantsVerbatim'); @@ -161,11 +161,11 @@ public function testRemovingTheLastColumnGrantClearsTheStoredGrants(): void /** @var Database $database */ $database = $this->getDatabase(); - $collection = $database->createCollection(Collection::create( + $collection = $database->withColumnPermissions(fn () => $database->createCollection(Collection::create( id: 'columnGrantsCleared', attributes: [Attribute::string(key: 'name', size: 64)], permissions: [Permission::read(Role::any(), 'name'), Permission::create(Role::any())], - )); + ))); $this->assertSame(['read("any", "name")', 'create("any")'], $collection->columnPermissions()); $this->assertSame(['read("any")', 'create("any")'], $collection->getPermissions()); @@ -184,17 +184,65 @@ public function testRemovingTheLastColumnGrantClearsTheStoredGrants(): void $this->assertSame(['read("any")', 'create("any")'], $stored->columnPermissions()); } - public function testACollectionPermissionMayNotNameAColumnThatDoesNotExist(): void + /** + * The feature is off until the column is enforced. + * + * A scoped grant admits its role to the row -- that is what every gate matches -- and + * nothing yet holds that role to the column, so the grant widens access rather than + * narrowing it. Refused by default so no one writes one by accident; withColumnPermissions() + * is the deliberate way in, and goes away once masking and query refusal land. + */ + public function testAColumnScopedGrantIsRefusedUnlessAskedFor(): void { /** @var Database $database */ $database = $this->getDatabase(); try { $database->createCollection(Collection::create( + id: 'columnGrantsGated', + attributes: [Attribute::string(key: 'name', size: 64)], + permissions: [Permission::read(Role::any(), 'name')], + )); + $this->fail('Expected createCollection() to refuse a column-scoped grant'); + } catch (DatabaseException $e) { + $this->assertStringContainsString('Column-scoped permissions are not enabled', $e->getMessage()); + } + + $this->assertNull($database->findCollection('columnGrantsGated')); + + // An update may not introduce one either. + $database->createCollection(Collection::create( + id: 'columnGrantsGatedUpdate', + attributes: [Attribute::string(key: 'name', size: 64)], + permissions: [Permission::read(Role::any())], + )); + + try { + $database->updateCollection('columnGrantsGatedUpdate', new CollectionUpdate(permissions: [ + Permission::read(Role::any(), 'name'), + ])); + $this->fail('Expected updateCollection() to refuse a column-scoped grant'); + } catch (DatabaseException $e) { + $this->assertStringContainsString('Column-scoped permissions are not enabled', $e->getMessage()); + } + + $this->assertSame( + ['read("any")'], + $database->getCollection('columnGrantsGatedUpdate')->columnPermissions(), + ); + } + + public function testACollectionPermissionMayNotNameAColumnThatDoesNotExist(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + try { + $database->withColumnPermissions(fn () => $database->createCollection(Collection::create( id: 'columnGrantsUnknown', attributes: [Attribute::string(key: 'name', size: 64)], permissions: [Permission::read(Role::any(), 'nope')], - )); + ))); $this->fail('Expected createCollection() to reject a permission naming an unknown column'); } catch (DatabaseException $e) { $this->assertStringContainsString('Column "nope" does not exist.', $e->getMessage()); @@ -215,10 +263,10 @@ public function testUpdatingACollectionJudgesAColumnGrantAgainstItsOwnColumns(): permissions: [Permission::create(Role::any())], )); - $database->updateCollection('columnGrantsUpdate', new CollectionUpdate(permissions: [ + $database->withColumnPermissions(fn () => $database->updateCollection('columnGrantsUpdate', new CollectionUpdate(permissions: [ Permission::read(Role::any(), 'name'), Permission::create(Role::any()), - ])); + ]))); $updated = $database->getCollection('columnGrantsUpdate')->columnPermissions(); @@ -226,9 +274,9 @@ public function testUpdatingACollectionJudgesAColumnGrantAgainstItsOwnColumns(): $this->assertContains('read("any", "name")', $updated); try { - $database->updateCollection('columnGrantsUpdate', new CollectionUpdate(permissions: [ + $database->withColumnPermissions(fn () => $database->updateCollection('columnGrantsUpdate', new CollectionUpdate(permissions: [ Permission::read(Role::any(), 'nope'), - ])); + ]))); $this->fail('Expected updateCollection() to reject a permission naming an unknown column'); } catch (DatabaseException $e) { $this->assertStringContainsString('Column "nope" does not exist.', $e->getMessage()); @@ -334,7 +382,7 @@ public function testAppwriteCollectionsSimulation(): void $authorization = $database->getAuthorization(); // The table itself, under the id Appwrite gives it. Its grants name its own columns. - $database->createCollection(Collection::create( + $database->withColumnPermissions(fn () => $database->createCollection(Collection::create( id: 'database_1_collection_1', attributes: [ Attribute::string(key: 'name', size: 64), @@ -344,7 +392,7 @@ public function testAppwriteCollectionsSimulation(): void Permission::read(Role::any(), 'name'), Permission::read(Role::team('hr'), 'salary'), ], - )); + ))); $grants = $database->getCollection('database_1_collection_1')->columnPermissions(); $this->assertNotNull($grants); From 17602fcf6391a98589b157fed0d1d1b68cf42bf8 Mon Sep 17 00:00:00 2001 From: fogelito Date: Tue, 13 Oct 2026 14:16:08 +0300 Subject: [PATCH 09/29] grants --- src/Database/Collection.php | 2 +- src/Database/Permission.php | 27 ++++++++--- src/Database/Trait/Collections.php | 2 +- .../Adapter/Scopes/ColumnPermissionTests.php | 45 ++++++++++--------- tests/unit/Model/CollectionTest.php | 14 +++--- tests/unit/PermissionTest.php | 45 +++++++++++++++++++ 6 files changed, 100 insertions(+), 35 deletions(-) diff --git a/src/Database/Collection.php b/src/Database/Collection.php index 0e6b74602..09cd86243 100644 --- a/src/Database/Collection.php +++ b/src/Database/Collection.php @@ -330,7 +330,7 @@ public function documentSecurity(): bool * * @throws StructureException when the stored grants are not a list of permission strings */ - public function columnPermissions(): ?array + public function grants(): ?array { if (! $this->offsetExists(self::PERMISSIONS) && ! $this->offsetExists(self::COLUMN_PERMISSIONS)) { return null; diff --git a/src/Database/Permission.php b/src/Database/Permission.php index eac48fbdb..af7a2e37c 100644 --- a/src/Database/Permission.php +++ b/src/Database/Permission.php @@ -145,16 +145,33 @@ public static function parse(string $permission): self // every branch below parses a bare role exactly as it did before -- the role grammar // (identifier, dimension) is unchanged by the column, and keeping them separate stops // the two from having to know about each other. + // + // The role ends at its closing quote, whatever spacing follows the comma. Matching the + // canonical '", "' alone would read read("any","name") as the role any","name, which is + // a role that matches nobody -- and these strings come from callers, not only from + // toString(). $column = self::ALL_COLUMNS; - $separator = \strpos($fullRole, '", "'); + $quote = \strpos($fullRole, '"'); - if ($separator !== false) { - $column = \substr($fullRole, $separator + 4); - $fullRole = \substr($fullRole, 0, $separator); + if ($quote !== false) { + $rest = \ltrim(\substr($fullRole, $quote + 1)); - if ($column === '') { + if (! \str_starts_with($rest, ',')) { + throw new DatabaseException('Invalid permission: "'.$permission.'".'); + } + + $rest = \ltrim(\substr($rest, 1)); + + if (! \str_starts_with($rest, '"')) { + throw new DatabaseException('Invalid permission: "'.$permission.'".'); + } + + if ($rest === '"') { throw new DatabaseException('Column must not be empty'); } + + $column = \substr($rest, 1); + $fullRole = \substr($fullRole, 0, $quote); } $roleParts = \explode(':', $fullRole); diff --git a/src/Database/Trait/Collections.php b/src/Database/Trait/Collections.php index 9023d7923..3502b7413 100644 --- a/src/Database/Trait/Collections.php +++ b/src/Database/Trait/Collections.php @@ -47,7 +47,7 @@ public function createCollection(Collection $collection): Collection $id = $collection->getId(); $attributes = \array_map(self::normalise(...), $collection->attributes()); - $permissions = $collection->columnPermissions() ?? [Permission::create(Role::any())]; + $permissions = $collection->grants() ?? [Permission::create(Role::any())]; $typeValidator = $this->typeValidator(); foreach ($attributes as $attribute) { diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php index b0f7cad3e..748c81263 100644 --- a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -8,6 +8,7 @@ use Utopia\Database\Database; use Utopia\Database\Document; use Utopia\Database\Exception as DatabaseException; +use Utopia\Database\Filter; use Utopia\Database\Permission; use Utopia\Database\PermissionType; use Utopia\Database\Role; @@ -41,7 +42,7 @@ public function testACollectionPermissionMayNameOneOfItsColumns(): void $stored = $database->getCollection('columnGrants'); - $permissions = $stored->columnPermissions(); + $permissions = $stored->grants(); $this->assertNotNull($permissions); $this->assertContains('read("any", "name")', $permissions); @@ -116,7 +117,7 @@ public function testAColumnScopedGrantIsReadBackAsItWasWritten(): void 'read("any", "name")', 'read("team:hr", "salary")', 'create("any")', - ], $stored->columnPermissions()); + ], $stored->grants()); // The gate keeps the roles and drops the columns, which is what every adapter matches. $this->assertSame(['any', 'team:hr'], $stored->getPermissionsByType(PermissionType::Read)); @@ -148,7 +149,7 @@ public function testACollectionWithoutColumnGrantsStoresNothingExtra(): void $stored = $database->getCollection('columnGrantsAbsent'); - $this->assertSame(['read("any")', 'create("any")'], $stored->columnPermissions()); + $this->assertSame(['read("any")', 'create("any")'], $stored->grants()); $this->assertSame([], $stored->getAttribute(Collection::COLUMN_PERMISSIONS)); } @@ -167,7 +168,7 @@ public function testRemovingTheLastColumnGrantClearsTheStoredGrants(): void permissions: [Permission::read(Role::any(), 'name'), Permission::create(Role::any())], ))); - $this->assertSame(['read("any", "name")', 'create("any")'], $collection->columnPermissions()); + $this->assertSame(['read("any", "name")', 'create("any")'], $collection->grants()); $this->assertSame(['read("any")', 'create("any")'], $collection->getPermissions()); $collection = $database->updateCollection('columnGrantsCleared', new CollectionUpdate(permissions: [ @@ -175,13 +176,13 @@ public function testRemovingTheLastColumnGrantClearsTheStoredGrants(): void Permission::create(Role::any()), ])); - $this->assertSame(['read("any")', 'create("any")'], $collection->columnPermissions()); + $this->assertSame(['read("any")', 'create("any")'], $collection->grants()); $this->assertSame(['read("any")', 'create("any")'], $collection->getPermissions()); $stored = $database->getCollection('columnGrantsCleared'); $this->assertSame([], $stored->getAttribute(Collection::COLUMN_PERMISSIONS)); - $this->assertSame(['read("any")', 'create("any")'], $stored->columnPermissions()); + $this->assertSame(['read("any")', 'create("any")'], $stored->grants()); } /** @@ -228,7 +229,7 @@ public function testAColumnScopedGrantIsRefusedUnlessAskedFor(): void $this->assertSame( ['read("any")'], - $database->getCollection('columnGrantsGatedUpdate')->columnPermissions(), + $database->getCollection('columnGrantsGatedUpdate')->grants(), ); } @@ -268,7 +269,7 @@ public function testUpdatingACollectionJudgesAColumnGrantAgainstItsOwnColumns(): Permission::create(Role::any()), ]))); - $updated = $database->getCollection('columnGrantsUpdate')->columnPermissions(); + $updated = $database->getCollection('columnGrantsUpdate')->grants(); $this->assertNotNull($updated); $this->assertContains('read("any", "name")', $updated); @@ -283,7 +284,7 @@ public function testUpdatingACollectionJudgesAColumnGrantAgainstItsOwnColumns(): } // The rejected update left the collection as it was. - $updated = $database->getCollection('columnGrantsUpdate')->columnPermissions(); + $updated = $database->getCollection('columnGrantsUpdate')->grants(); $this->assertNotNull($updated); $this->assertContains('read("any", "name")', $updated); @@ -394,16 +395,17 @@ public function testAppwriteCollectionsSimulation(): void ], ))); - $grants = $database->getCollection('database_1_collection_1')->columnPermissions(); + $grants = $database->getCollection('database_1_collection_1')->grants(); $this->assertNotNull($grants); - // Appwrite's list of the database's tables, one row per table. Its columns -- tableId, - // grants -- are nothing like the ones those grants name. + // Appwrite's list of the database's tables, one row per table, shaped like the real + // thing: databaseId beside a columnPermissions column. Neither resembles the columns + // the grants it records name. $database->createCollection(Collection::create( id: 'database_1', attributes: [ - Attribute::string(key: 'tableId', size: 64), - Attribute::string(key: 'grants', size: 4096, array: true), + Attribute::string(key: 'databaseId', size: Database::LENGTH_KEY), + Attribute::string(key: 'columnPermissions', size: 1_000_000, filters: [Filter::Json]), ], permissions: [Permission::create(Role::any())], documentSecurity: true, @@ -413,8 +415,8 @@ public function testAppwriteCollectionsSimulation(): void try { $database->createDocument('database_1', new Document([ '$id' => 'wrong', - 'tableId' => 'database_1_collection_1', - 'grants' => $grants, + 'databaseId' => 'database_1', + 'columnPermissions' => $grants, '$permissions' => $grants, ])); $this->fail('Expected createDocument() to reject grants naming another collection\'s columns'); @@ -423,17 +425,18 @@ public function testAppwriteCollectionsSimulation(): void } // Splitting them is not. + // The row's id is the table's id, as Appwrite writes it. $database->createDocument('database_1', new Document([ - '$id' => 'employees', - 'tableId' => 'database_1_collection_1', - 'grants' => $grants, + '$id' => 'database_1_collection_1', + 'databaseId' => 'database_1', + 'columnPermissions' => $grants, '$permissions' => Permission::withoutColumns($grants), ])); - $stored = $database->getDocument('database_1', 'employees'); + $stored = $database->getDocument('database_1', 'database_1_collection_1'); // The grants come back as the table has them, ready to report to a caller... - $this->assertSame($grants, $stored->getAttribute('grants')); + $this->assertSame($grants, $stored->getAttribute('columnPermissions')); // ...while the row answers the gate with roles alone. $this->assertSame(['any', 'team:hr'], $stored->getPermissionsByType(PermissionType::Read)); diff --git a/tests/unit/Model/CollectionTest.php b/tests/unit/Model/CollectionTest.php index 356d70da8..bb61e9226 100644 --- a/tests/unit/Model/CollectionTest.php +++ b/tests/unit/Model/CollectionTest.php @@ -44,7 +44,7 @@ public function testCreateStoresEveryField(): void $this->assertSame([ColumnType::String, ColumnType::Integer], \array_map(static fn (Attribute $attribute): ColumnType => $attribute->type, $collection->attributes())); $this->assertSame(['by_title'], \array_map(static fn (Index $index): string => $index->key, $collection->indexes())); $this->assertSame([OrderDirection::Asc], $collection->indexes()[0]->orders); - $this->assertSame(['read("any")'], $collection->columnPermissions()); + $this->assertSame(['read("any")'], $collection->grants()); $this->assertFalse($collection->documentSecurity()); $this->assertFalse($collection->getAttribute('documentSecurity')); } @@ -56,7 +56,7 @@ public function testCreateDefaults(): void $this->assertSame('books', $collection->name()); $this->assertSame([], $collection->attributes()); $this->assertSame([], $collection->indexes()); - $this->assertNull($collection->columnPermissions()); + $this->assertNull($collection->grants()); $this->assertTrue($collection->documentSecurity()); $this->assertTrue($collection->getAttribute('documentSecurity')); } @@ -78,7 +78,7 @@ public function testCreateStoresAttributesAndIndexesAsDocuments(): void public function testCreateWithNoPermissionsDeclaresAnEmptyList(): void { - $this->assertSame([], Collection::create('books', permissions: [])->columnPermissions()); + $this->assertSame([], Collection::create('books', permissions: [])->grants()); } public function testCreateKeepsMetadataInStorage(): void @@ -106,7 +106,7 @@ public function testFromArrayKeepsExtrasAndNormalisesTheCoreKeys(): void $this->assertSame('books', $collection->name()); $this->assertTrue($collection->documentSecurity()); $this->assertTrue($collection->getAttribute('documentSecurity')); - $this->assertNull($collection->columnPermissions()); + $this->assertNull($collection->grants()); $this->assertSame('fiction', $collection->getAttribute('category')); $this->assertSame('title', $collection->attributes()[0]->key); $this->assertSame(ColumnType::String, $collection->attributes()[0]->type); @@ -136,7 +136,7 @@ public function testFromArrayDropsPermissionsThatAreNotAList(): void { $collection = Collection::fromArray(['$id' => 'books', '$permissions' => 'read("any")']); - $this->assertNull($collection->columnPermissions()); + $this->assertNull($collection->grants()); } public function testFromArrayRejectsInvalidPermissionEntries(): void @@ -205,11 +205,11 @@ public function testDeclaredPermissionsFollowStorage(): void $collection = Collection::create('books'); $collection->setAttribute('$permissions', [Permission::read(Role::any())]); - $this->assertSame(['read("any")'], $collection->columnPermissions()); + $this->assertSame(['read("any")'], $collection->grants()); $collection->removeAttribute('$permissions'); - $this->assertNull($collection->columnPermissions()); + $this->assertNull($collection->grants()); } public function testDocumentSecurityFollowsStorage(): void diff --git a/tests/unit/PermissionTest.php b/tests/unit/PermissionTest.php index 7533a0a04..1a2d8ba9b 100644 --- a/tests/unit/PermissionTest.php +++ b/tests/unit/PermissionTest.php @@ -2,6 +2,7 @@ namespace Tests\Unit; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Utopia\Database\Exception as DatabaseException; use Utopia\Database\Id; @@ -365,6 +366,50 @@ public function test_column_scope(): void $this->assertEquals('name', $permission->getColumn()); } + /** + * The role ends at its closing quote, whatever spacing the caller put after the comma. + * + * These strings arrive from API clients, not only from toString(). Matching the canonical + * spacing alone read read("any","name") as the role any","name -- a role matching nobody, + * reported as "Role "any","name" is not allowed" for input that is perfectly reasonable. + * + * @throws \Exception + */ + #[DataProvider('spacings')] + public function test_column_tolerates_spacing(string $permission): void + { + $parsed = Permission::parse($permission); + + $this->assertEquals('any', $parsed->getRole()); + $this->assertEquals('name', $parsed->getColumn()); + } + + /** + * @return array + */ + public static function spacings(): array + { + return [ + 'canonical' => ['read("any", "name")'], + 'no space' => ['read("any","name")'], + 'space before the comma' => ['read("any" , "name")'], + 'two spaces after' => ['read("any", "name")'], + ]; + } + + /** + * A second argument that is not a quoted column is malformed, not a role with a quote in it. + * + * @throws \Exception + */ + public function test_a_malformed_second_argument_is_rejected(): void + { + $this->expectException(DatabaseException::class); + $this->expectExceptionMessage('Invalid permission'); + + Permission::parse('read("any" "name")'); + } + /** * @throws \Exception */ From 7a7c1f50454448a9142d6efc61546d075c3354f7 Mon Sep 17 00:00:00 2001 From: fogelito Date: Tue, 13 Oct 2026 14:44:51 +0300 Subject: [PATCH 10/29] assertRowPermissions skip validation --- src/Database/Trait/Documents.php | 54 +++++++++++--- .../Adapter/Scopes/ColumnPermissionTests.php | 74 ++++++++++++++++++- 2 files changed, 115 insertions(+), 13 deletions(-) diff --git a/src/Database/Trait/Documents.php b/src/Database/Trait/Documents.php index e330e383f..5bcab8d4f 100644 --- a/src/Database/Trait/Documents.php +++ b/src/Database/Trait/Documents.php @@ -1189,6 +1189,35 @@ private function assertCreateTenancy(string $collection): void * @throws DatabaseException * @throws StructureException */ + /** + * A row's permissions may not name a column. + * + * No column list on purpose, and no exemption: a column-scoped permission belongs to a + * collection, and a row naming one would be stored where nothing enforces it. It is also + * what every read gate matches against -- the `_perms` side table, Postgres' JSONB + * containment, Mongo's `$in` -- so a column stored here hides the row from the role it was + * granted to, while getDocument() still returns it. + * + * Every path that writes permissions calls this. It used to live inline in + * prepareDocument(), which only createDocument() reaches, so the rule held on one entry + * point out of six and a batch create, an update or an upsert stored the grant happily. + * + * @throws DatabaseException + * @throws StructureException + */ + private function assertRowPermissions(Document $document): void + { + if (! $this->validation()->get()) { + return; + } + + $validator = new Permissions(columns: null); + + if (! $validator->isValid($document->getPermissions())) { + throw new DatabaseException($validator->getDescription()); + } + } + private function prepareDocument(Document $collection, Document $document): Document { if ($collection->getId() !== self::METADATA) { @@ -1259,18 +1288,7 @@ private function prepareDocument(Document $collection, Document $document): Docu $document = $this->encode($collection, $document); - if ($this->validation()->get()) { - // No column list on purpose, and no exemption: a column-scoped permission belongs to - // a collection, and a row naming one is refused rather than stored somewhere nothing - // enforces it. It is also what every read gate matches against -- a side table of - // roles, a JSONB containment, a Mongo `$in` -- so a column stored here would hide the - // row from the role it was granted to. - // - $validator = new Permissions(columns: null); - if (! $validator->isValid($document->getPermissions())) { - throw new DatabaseException($validator->getDescription()); - } - } + $this->assertRowPermissions($document); if ($this->validation()->get()) { $structure = new Structure( @@ -1333,6 +1351,10 @@ public function createDocuments( } } + foreach ($documents as $document) { + $this->assertRowPermissions($document); + } + $time = DateTime::now(); $modified = 0; $hasRelationships = self::relationshipAttributes($collection) !== []; @@ -1460,6 +1482,8 @@ public function updateDocument(string $collection, string $id, Document $documen throw new DatabaseException('Must define $id attribute'); } + $this->assertRowPermissions($document); + $collection = $this->silent(fn () => $this->getCollection($collection)); $newUpdatedAt = $document->getUpdatedAt(); $hasOperators = false; @@ -1780,6 +1804,8 @@ public function updateDocuments( $this->rejectJoins($queries, 'Join queries are not supported for bulk updates'); + $this->assertRowPermissions($updates); + if ($updates->isEmpty()) { return 0; } @@ -2101,6 +2127,10 @@ private function upsert( throw new DatabaseException('Adapter does not support upserts'); } + foreach ($documents as $document) { + $this->assertRowPermissions($document); + } + if ( $this->adapter->hasSharedTables() && ! $this->adapter->isTenantPerDocument() diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php index 748c81263..053a694cb 100644 --- a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -458,7 +458,79 @@ public function testAppwriteCollectionsSimulation(): void } } - $this->assertSame(['employees'], $ids); + $this->assertSame(['database_1_collection_1'], $ids); + } + + /** + * Every write path refuses it, not just createDocument(). + * + * The rule lived inline in prepareDocument() at first, which only createDocument() reaches, + * so a batch create, an update or an upsert stored the grant happily -- and a row holding a + * column in its permissions is hidden from the role it names by the gates that match that + * field directly. One case per entry point, so the rule cannot quietly shrink back to one. + */ + public function testEveryWritePathRefusesAColumnScopedPermission(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + $database->createCollection(Collection::create( + id: 'columnGrantsWrites', + attributes: [Attribute::string(key: 'name', size: 64)], + permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any()), + Permission::update(Role::any()), + ], + documentSecurity: true, + )); + + $database->createDocument('columnGrantsWrites', new Document([ + '$id' => 'row', + 'name' => 'Ada', + '$permissions' => [Permission::read(Role::any()), Permission::update(Role::any())], + ])); + + $scoped = [Permission::read(Role::any(), 'name')]; + + $writes = [ + 'createDocument' => fn () => $database->createDocument('columnGrantsWrites', new Document([ + '$id' => 'created', 'name' => 'x', '$permissions' => $scoped, + ])), + 'createDocuments' => fn () => $database->createDocuments('columnGrantsWrites', [new Document([ + '$id' => 'batched', 'name' => 'x', '$permissions' => $scoped, + ])]), + 'updateDocument' => fn () => $database->updateDocument('columnGrantsWrites', 'row', new Document([ + '$permissions' => $scoped, + ])), + 'updateDocuments' => fn () => $database->updateDocuments('columnGrantsWrites', new Document([ + '$permissions' => $scoped, + ])), + 'upsertDocument' => fn () => $database->upsertDocument('columnGrantsWrites', new Document([ + '$id' => 'upserted', 'name' => 'x', '$permissions' => $scoped, + ])), + 'upsertDocuments' => fn () => $database->upsertDocuments('columnGrantsWrites', [new Document([ + '$id' => 'upserted', 'name' => 'x', '$permissions' => $scoped, + ])]), + ]; + + foreach ($writes as $path => $write) { + try { + $write(); + $this->fail($path.'() stored a column-scoped permission on a row'); + } catch (DatabaseException $e) { + $this->assertStringContainsString('is scoped to a column', $e->getMessage(), $path); + } + } + + // None of them left anything behind, and the row it tried to rewrite is untouched. + $this->assertSame( + ['read("any")', 'update("any")'], + $database->getDocument('columnGrantsWrites', 'row')->getPermissions(), + ); + $this->assertTrue($database->getDocument('columnGrantsWrites', 'created')->isEmpty()); + $this->assertTrue($database->getDocument('columnGrantsWrites', 'batched')->isEmpty()); + $this->assertTrue($database->getDocument('columnGrantsWrites', 'upserted')->isEmpty()); } public function testADocumentPermissionNamingNoColumnIsUnaffected(): void From da2e5617835cbcf356aa4a1f3d35ea1e2f1f7fbc Mon Sep 17 00:00:00 2001 From: fogelito Date: Tue, 13 Oct 2026 15:04:19 +0300 Subject: [PATCH 11/29] Snapshot --- src/Database/Database.php | 4 ++- src/Database/State/Snapshot.php | 2 ++ .../Adapter/Scopes/ColumnPermissionTests.php | 31 +++++++++++++++++++ tests/unit/State/CoroutineStateTest.php | 1 + 4 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index f14123ea2..d45c5291b 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -1583,6 +1583,7 @@ public function snapshot(): Snapshot preserveDates: $this->datePreservation()->get(), preserveSequence: $this->sequencePreservation()->get(), ignoreDuplicates: $this->ignoringDuplicates()->get(), + columnPermissions: $this->columnPermissions()->get(), requestTimestamp: $this->requestTimestamp()->get(), ); } @@ -1625,7 +1626,8 @@ public function withSnapshot(Snapshot $snapshot, callable $callback): mixed private function withToggles(Snapshot $snapshot, callable $callback): mixed { $timestamped = fn (): mixed => $this->requestTimestamp()->with($snapshot->requestTimestamp, $callback); - $deduplicated = fn (): mixed => $this->ignoringDuplicates()->with($snapshot->ignoreDuplicates, $timestamped); + $scoped = fn (): mixed => $this->columnPermissions()->with($snapshot->columnPermissions, $timestamped); + $deduplicated = fn (): mixed => $this->ignoringDuplicates()->with($snapshot->ignoreDuplicates, $scoped); $sequenced = fn (): mixed => $this->sequencePreservation()->with($snapshot->preserveSequence, $deduplicated); $dated = fn (): mixed => $this->datePreservation()->with($snapshot->preserveDates, $sequenced); $validated = fn (): mixed => $this->validation()->with($snapshot->validation, $dated); diff --git a/src/Database/State/Snapshot.php b/src/Database/State/Snapshot.php index 7e644abb1..6c7d977da 100644 --- a/src/Database/State/Snapshot.php +++ b/src/Database/State/Snapshot.php @@ -26,6 +26,7 @@ * @param bool $preserveDates Whether writes keep the dates they are given * @param bool $preserveSequence Whether writes keep the sequences they are given * @param bool $ignoreDuplicates Whether creating a document that exists is skipped instead of failing + * @param bool $columnPermissions Whether a collection's permissions may name one of its columns * @param DateTime|null $requestTimestamp The time an update conflicts after */ public function __construct( @@ -43,6 +44,7 @@ public function __construct( public bool $preserveDates, public bool $preserveSequence, public bool $ignoreDuplicates, + public bool $columnPermissions, public ?DateTime $requestTimestamp, ) { } diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php index 053a694cb..de0384947 100644 --- a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -233,6 +233,37 @@ public function testAColumnScopedGrantIsRefusedUnlessAskedFor(): void ); } + /** + * The toggle travels in a snapshot, like every other scoped flag. + * + * A snapshot captured inside the opt-in has to restore it, or a caller replaying that state + * elsewhere -- another coroutine, a queued job -- is refused despite having captured the + * permission to do exactly this. + */ + public function testTheColumnPermissionsToggleSurvivesASnapshot(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + $snapshot = $database->withColumnPermissions(fn () => $database->snapshot()); + + // The scope has closed, so the toggle is off again... + $this->assertFalse($database->hasColumnPermissions()); + + // ...but the snapshot still carries it. + $database->withSnapshot($snapshot, fn () => $database->createCollection(Collection::create( + id: 'columnGrantsSnapshot', + attributes: [Attribute::string(key: 'name', size: 64)], + permissions: [Permission::read(Role::any(), 'name')], + ))); + + $this->assertContains( + 'read("any", "name")', + $database->getCollection('columnGrantsSnapshot')->grants() ?? [], + ); + $this->assertFalse($database->hasColumnPermissions()); + } + public function testACollectionPermissionMayNotNameAColumnThatDoesNotExist(): void { /** @var Database $database */ diff --git a/tests/unit/State/CoroutineStateTest.php b/tests/unit/State/CoroutineStateTest.php index 5e89d68c8..938b41bee 100644 --- a/tests/unit/State/CoroutineStateTest.php +++ b/tests/unit/State/CoroutineStateTest.php @@ -257,6 +257,7 @@ private function snapshot(bool $population): Snapshot preserveDates: false, preserveSequence: false, ignoreDuplicates: false, + columnPermissions: false, requestTimestamp: null, ); } From 01f0fe12d05f28961dfa8d955e270269a730832d Mon Sep 17 00:00:00 2001 From: fogelito Date: Tue, 13 Oct 2026 15:17:30 +0300 Subject: [PATCH 12/29] Fix lint --- tests/e2e/Adapter/Base.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index e26b0e436..5f466e351 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -7,8 +7,8 @@ use Tests\E2E\Adapter\Scopes\AggregationTests; use Tests\E2E\Adapter\Scopes\AttributeTests; use Tests\E2E\Adapter\Scopes\CollectionTests; -use Tests\E2E\Adapter\Scopes\CursorTests; use Tests\E2E\Adapter\Scopes\ColumnPermissionTests; +use Tests\E2E\Adapter\Scopes\CursorTests; use Tests\E2E\Adapter\Scopes\CustomDocumentTypeTests; use Tests\E2E\Adapter\Scopes\DatabaseTests; use Tests\E2E\Adapter\Scopes\DocumentTests; From d97d0fb935eeaac2bcc0e6e96d8b9685167cc630 Mon Sep 17 00:00:00 2001 From: fogelito Date: Tue, 13 Oct 2026 15:22:32 +0300 Subject: [PATCH 13/29] Fix withToggles --- src/Database/Database.php | 4 ++-- tests/unit/CollectionDefinitionTest.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 1202f79bc..d80e56d32 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -1728,13 +1728,13 @@ private function withToggles(Snapshot $snapshot, callable $callback): mixed $rounded = fn (): mixed => $this->fractionRounding()->with($snapshot->roundFractions, $callback); $checked = fn (): mixed => $this->grantChecking()->with($snapshot->checkGrants, $rounded); $timestamped = fn (): mixed => $this->requestTimestamp()->with($snapshot->requestTimestamp, $checked); - $deduplicated = fn (): mixed => $this->ignoringDuplicates()->with($snapshot->ignoreDuplicates, $timestamped); + $scoped = fn (): mixed => $this->columnPermissions()->with($snapshot->columnPermissions, $timestamped); + $deduplicated = fn (): mixed => $this->ignoringDuplicates()->with($snapshot->ignoreDuplicates, $scoped); $sequenced = fn (): mixed => $this->sequencePreservation()->with($snapshot->preserveSequence, $deduplicated); $dated = fn (): mixed => $this->datePreservation()->with($snapshot->preserveDates, $sequenced); $validated = fn (): mixed => $this->validation()->with($snapshot->validation, $dated); $excluded = fn (): mixed => $this->filterExclusions()->with($snapshot->disabledFilters, $validated); $filtered = fn (): mixed => $this->filtering()->with($snapshot->filters, $excluded); - $scoped = fn (): mixed => $this->columnPermissions()->with($snapshot->columnPermissions, $timestamped); return $this->adapter->withTenant($snapshot->tenant, $filtered); } diff --git a/tests/unit/CollectionDefinitionTest.php b/tests/unit/CollectionDefinitionTest.php index 615b11ea5..53eb1b4f1 100644 --- a/tests/unit/CollectionDefinitionTest.php +++ b/tests/unit/CollectionDefinitionTest.php @@ -30,7 +30,7 @@ public function testCollectionDefinitionKeys(): void $this->assertSame(Database::METADATA, $definition->getId()); $this->assertSame(Database::METADATA, $definition->getAttribute(Document::COLLECTION)); $this->assertSame('collections', $definition->name()); - $this->assertSame(['name', 'attributes', 'indexes', 'documentSecurity'], $this->keys($definition)); + $this->assertSame(['name', 'attributes', 'indexes', 'documentSecurity', 'columnPermissions'], $this->keys($definition)); } public function testCollectionDefinitionAttributeTypes(): void From f1bdecd2719d24601e21ca872394d81a03749a2a Mon Sep 17 00:00:00 2001 From: fogelito Date: Tue, 13 Oct 2026 15:38:11 +0300 Subject: [PATCH 14/29] Fixes --- src/Database/Collection.php | 6 +++- src/Database/Mirror.php | 14 ++++++++++ .../Adapter/Scopes/ColumnPermissionTests.php | 28 +++++++++++++++---- 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/Database/Collection.php b/src/Database/Collection.php index 09cd86243..5d7170bb6 100644 --- a/src/Database/Collection.php +++ b/src/Database/Collection.php @@ -354,13 +354,17 @@ public function grants(): ?array throw new StructureException('Collection '.self::COLUMN_PERMISSIONS.' must be a list of permission strings'); } + $grants = []; + foreach ($verbatim as $permission) { if (! \is_string($permission)) { throw new StructureException('Collection '.self::COLUMN_PERMISSIONS.' must be a list of permission strings'); } + + $grants[] = $permission; } - return \array_values($verbatim); + return $grants; } #[\Override] diff --git a/src/Database/Mirror.php b/src/Database/Mirror.php index f54529a94..45896258e 100644 --- a/src/Database/Mirror.php +++ b/src/Database/Mirror.php @@ -520,6 +520,20 @@ public function withValidation(bool $validation, callable $callback): mixed return parent::withValidation($validation, fn (): mixed => $this->source->withValidation($validation, $scoped)); } + /** + * Opened on the mirror, its source and its destination alike: a collection the mirror + * replicates is written to both, and the second write would be refused if only the first + * had asked for column-scoped permissions. + */ + #[\Override] + public function withColumnPermissions(callable $callback): mixed + { + $destination = $this->destination; + $scoped = $destination === null ? $callback : fn (): mixed => $destination->withColumnPermissions($callback); + + return parent::withColumnPermissions(fn (): mixed => $this->source->withColumnPermissions($scoped)); + } + /** * Opens the scope on the mirror, its source and its destination, so the writes the mirror replicates before * returning use the tenant too; the replications it queues carry it (see replicate()). diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php index de0384947..6e0860643 100644 --- a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -2,6 +2,7 @@ namespace Tests\E2E\Adapter\Scopes; +use Utopia\Database\Adapter\Feature; use Utopia\Database\Attribute; use Utopia\Database\Collection; use Utopia\Database\CollectionUpdate; @@ -9,6 +10,7 @@ use Utopia\Database\Document; use Utopia\Database\Exception as DatabaseException; use Utopia\Database\Filter; +use Utopia\Database\Mirror; use Utopia\Database\Permission; use Utopia\Database\PermissionType; use Utopia\Database\Role; @@ -245,6 +247,12 @@ public function testTheColumnPermissionsToggleSurvivesASnapshot(): void /** @var Database $database */ $database = $this->getDatabase(); + if ($database instanceof Mirror) { + // True of every toggle, not this one: replicate() propagates a snapshot explicitly + // where it needs to, so there is nothing here for this test to pin. + $this->markTestSkipped('Mirror does not override withSnapshot(), so a restored snapshot reaches the wrapper and not its source or destination.'); + } + $snapshot = $database->withColumnPermissions(fn () => $database->snapshot()); // The scope has closed, so the toggle is off again... @@ -537,13 +545,18 @@ public function testEveryWritePathRefusesAColumnScopedPermission(): void 'updateDocuments' => fn () => $database->updateDocuments('columnGrantsWrites', new Document([ '$permissions' => $scoped, ])), - 'upsertDocument' => fn () => $database->upsertDocument('columnGrantsWrites', new Document([ + ]; + + // The upsert paths share one guard, and an adapter without upserts refuses the call + // before reaching it. + if ($database->getAdapter()->hasFeature(Feature\Upserts::class)) { + $writes['upsertDocument'] = fn () => $database->upsertDocument('columnGrantsWrites', new Document([ '$id' => 'upserted', 'name' => 'x', '$permissions' => $scoped, - ])), - 'upsertDocuments' => fn () => $database->upsertDocuments('columnGrantsWrites', [new Document([ + ])); + $writes['upsertDocuments'] = fn () => $database->upsertDocuments('columnGrantsWrites', [new Document([ '$id' => 'upserted', 'name' => 'x', '$permissions' => $scoped, - ])]), - ]; + ])]); + } foreach ($writes as $path => $write) { try { @@ -561,7 +574,10 @@ public function testEveryWritePathRefusesAColumnScopedPermission(): void ); $this->assertTrue($database->getDocument('columnGrantsWrites', 'created')->isEmpty()); $this->assertTrue($database->getDocument('columnGrantsWrites', 'batched')->isEmpty()); - $this->assertTrue($database->getDocument('columnGrantsWrites', 'upserted')->isEmpty()); + + if ($database->getAdapter()->hasFeature(Feature\Upserts::class)) { + $this->assertTrue($database->getDocument('columnGrantsWrites', 'upserted')->isEmpty()); + } } public function testADocumentPermissionNamingNoColumnIsUnaffected(): void From d1b2ccf0be9026c637d7d51d4b86e1f81fb6959b Mon Sep 17 00:00:00 2001 From: fogelito Date: Tue, 13 Oct 2026 15:45:22 +0300 Subject: [PATCH 15/29] fix unit --- tests/unit/Attributes/CheckAttributeTest.php | 31 ++++++++++++++++--- .../Documents/ResolvedCollectionReadTest.php | 4 +-- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/tests/unit/Attributes/CheckAttributeTest.php b/tests/unit/Attributes/CheckAttributeTest.php index 91940f6a5..0b0496286 100644 --- a/tests/unit/Attributes/CheckAttributeTest.php +++ b/tests/unit/Attributes/CheckAttributeTest.php @@ -18,18 +18,41 @@ final class CheckAttributeTest extends TestCase { public function testAnAttributeWithinTheLimitPasses(): void { - $database = $this->database([Attribute::string('first', 16), Attribute::string('second', 16), Attribute::string('third', 16)]); + $database = $this->database($this->attributes(self::limit() - 1)); - $this->assertTrue($database->checkAttribute('items', Attribute::string('fourth', 16))); + $this->assertTrue($database->checkAttribute('items', Attribute::string('last', 16))); } public function testAnAttributeOverTheLimitIsRefused(): void { - $database = $this->database([Attribute::string('first', 16), Attribute::string('second', 16), Attribute::string('third', 16), Attribute::string('fourth', 16)]); + $database = $this->database($this->attributes(self::limit())); $this->expectException(LimitException::class); - $database->checkAttribute('items', Attribute::string('fifth', 16)); + $database->checkAttribute('items', Attribute::string('overTheLimit', 16)); + } + + /** + * How many attributes the adapter below allows, which it derives from the metadata + * definition -- so a fixture counting them by hand goes stale the moment that grows. + */ + private static function limit(): int + { + return \count(Database::collectionDefinition()->attributes()); + } + + /** + * @return list + */ + private function attributes(int $count): array + { + $attributes = []; + + for ($i = 0; $i < $count; $i++) { + $attributes[] = Attribute::string('attribute'.$i, 16); + } + + return $attributes; } public function testCheckingDoesNotStoreTheAttribute(): void diff --git a/tests/unit/Documents/ResolvedCollectionReadTest.php b/tests/unit/Documents/ResolvedCollectionReadTest.php index a4443e163..d07b115f0 100644 --- a/tests/unit/Documents/ResolvedCollectionReadTest.php +++ b/tests/unit/Documents/ResolvedCollectionReadTest.php @@ -292,7 +292,7 @@ public function testADecoratorMutatingTheMetadataDefinitionLeavesTheNextReadInta $definition = Database::collectionDefinition(); $this->assertSame('collections', $definition->getAttribute(Collection::NAME)); - $this->assertCount(4, $definition->attributes()); + $this->assertCount(5, $definition->attributes()); $this->assertSame(['name', 'count'], \array_map( static fn (Attribute $attribute): string => $attribute->key, $database->getCollection(self::COLLECTION)->attributes(), @@ -348,7 +348,7 @@ public function testADecoratorAddedDuringAMetadataReadLeavesTheSharedDefinitionI $this->assertSame(self::COLLECTION, $stored->getId()); $definition = Database::collectionDefinition(); $this->assertSame('collections', $definition->getAttribute(Collection::NAME), 'The decorator changed the shared metadata definition'); - $this->assertCount(4, $definition->attributes()); + $this->assertCount(5, $definition->attributes()); } private function metadataMutatingDecorator(): Decorator From be06c5ca2c700007d6bb12c2ec801c70739e5a17 Mon Sep 17 00:00:00 2001 From: fogelito Date: Wed, 14 Oct 2026 08:22:25 +0300 Subject: [PATCH 16/29] Revert declaredPermissions & validation --- src/Database/Collection.php | 2 +- src/Database/Permission.php | 6 ++- src/Database/Trait/Collections.php | 2 +- src/Database/Validator/Permissions.php | 22 ++++++++++ .../Adapter/Scopes/ColumnPermissionTests.php | 22 +++++----- tests/unit/Model/CollectionTest.php | 14 +++---- tests/unit/PermissionTest.php | 25 +++++++++++- tests/unit/Validator/PermissionsTest.php | 40 +++++++++++++++++++ 8 files changed, 110 insertions(+), 23 deletions(-) diff --git a/src/Database/Collection.php b/src/Database/Collection.php index 5d7170bb6..17e29d7f4 100644 --- a/src/Database/Collection.php +++ b/src/Database/Collection.php @@ -330,7 +330,7 @@ public function documentSecurity(): bool * * @throws StructureException when the stored grants are not a list of permission strings */ - public function grants(): ?array + public function declaredPermissions(): ?array { if (! $this->offsetExists(self::PERMISSIONS) && ! $this->offsetExists(self::COLUMN_PERMISSIONS)) { return null; diff --git a/src/Database/Permission.php b/src/Database/Permission.php index af7a2e37c..7ea539eff 100644 --- a/src/Database/Permission.php +++ b/src/Database/Permission.php @@ -167,7 +167,11 @@ public static function parse(string $permission): self } if ($rest === '"') { - throw new DatabaseException('Column must not be empty'); + throw new DatabaseException('Column must not be empty. Omit the argument to grant every column.'); + } + + if ($rest === '"*') { + throw new DatabaseException('Wildcard column "*" is not supported. Omit the argument to grant every column.'); } $column = \substr($rest, 1); diff --git a/src/Database/Trait/Collections.php b/src/Database/Trait/Collections.php index 3502b7413..82e79e8ca 100644 --- a/src/Database/Trait/Collections.php +++ b/src/Database/Trait/Collections.php @@ -47,7 +47,7 @@ public function createCollection(Collection $collection): Collection $id = $collection->getId(); $attributes = \array_map(self::normalise(...), $collection->attributes()); - $permissions = $collection->grants() ?? [Permission::create(Role::any())]; + $permissions = $collection->declaredPermissions() ?? [Permission::create(Role::any())]; $typeValidator = $this->typeValidator(); foreach ($attributes as $attribute) { diff --git a/src/Database/Validator/Permissions.php b/src/Database/Validator/Permissions.php index 7de411106..668d6d40d 100644 --- a/src/Database/Validator/Permissions.php +++ b/src/Database/Validator/Permissions.php @@ -27,6 +27,8 @@ class Permissions extends Roles */ private ?array $columns; + private Key $key; + /** * @param int $length maximum amount of permissions. 0 means unlimited. * @param array $allowed allowed permissions. Defaults to all available. @@ -42,6 +44,7 @@ public function __construct(int $length = 0, array $allowed = [PermissionType::C $this->length = $length; $this->allowed = \array_map(fn (PermissionType $p) => $p->value, $allowed); $this->columns = $columns; + $this->key = new Key(); } #[\Override] @@ -129,12 +132,31 @@ public function isValid(mixed $value): bool */ private function isValidColumn(Permission $permission): bool { + $type = $permission->getPermission(); + + // Before anything about where the permission lives: delete removes the whole row, so + // narrowing it to one column says nothing, and write aggregates to create, update and + // delete, so it inherits the same objection. + if (\in_array($type, [PermissionType::Delete->value, PermissionType::Write->value], true)) { + $this->message = 'Permission "'.$type.'" cannot be scoped to a column, it applies to the whole row.'; + + return false; + } + if ($this->columns === null) { $this->message = 'Permission "'.$permission->toString().'" is scoped to a column. Column-scoped permissions are set on the collection, not here.'; return false; } + // Judged before existence, so a column that could never be a key is told why rather + // than being reported as one the collection happens not to have. + if (! $this->key->isValid($permission->getColumn())) { + $this->message = 'Column "'.$permission->getColumn().'" is not a valid column key.'; + + return false; + } + if (! \in_array($permission->getColumn(), $this->columns, true)) { $this->message = 'Column "'.$permission->getColumn().'" does not exist.'; diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php index 6e0860643..e46e265ee 100644 --- a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -44,7 +44,7 @@ public function testACollectionPermissionMayNameOneOfItsColumns(): void $stored = $database->getCollection('columnGrants'); - $permissions = $stored->grants(); + $permissions = $stored->declaredPermissions(); $this->assertNotNull($permissions); $this->assertContains('read("any", "name")', $permissions); @@ -119,7 +119,7 @@ public function testAColumnScopedGrantIsReadBackAsItWasWritten(): void 'read("any", "name")', 'read("team:hr", "salary")', 'create("any")', - ], $stored->grants()); + ], $stored->declaredPermissions()); // The gate keeps the roles and drops the columns, which is what every adapter matches. $this->assertSame(['any', 'team:hr'], $stored->getPermissionsByType(PermissionType::Read)); @@ -151,7 +151,7 @@ public function testACollectionWithoutColumnGrantsStoresNothingExtra(): void $stored = $database->getCollection('columnGrantsAbsent'); - $this->assertSame(['read("any")', 'create("any")'], $stored->grants()); + $this->assertSame(['read("any")', 'create("any")'], $stored->declaredPermissions()); $this->assertSame([], $stored->getAttribute(Collection::COLUMN_PERMISSIONS)); } @@ -170,7 +170,7 @@ public function testRemovingTheLastColumnGrantClearsTheStoredGrants(): void permissions: [Permission::read(Role::any(), 'name'), Permission::create(Role::any())], ))); - $this->assertSame(['read("any", "name")', 'create("any")'], $collection->grants()); + $this->assertSame(['read("any", "name")', 'create("any")'], $collection->declaredPermissions()); $this->assertSame(['read("any")', 'create("any")'], $collection->getPermissions()); $collection = $database->updateCollection('columnGrantsCleared', new CollectionUpdate(permissions: [ @@ -178,13 +178,13 @@ public function testRemovingTheLastColumnGrantClearsTheStoredGrants(): void Permission::create(Role::any()), ])); - $this->assertSame(['read("any")', 'create("any")'], $collection->grants()); + $this->assertSame(['read("any")', 'create("any")'], $collection->declaredPermissions()); $this->assertSame(['read("any")', 'create("any")'], $collection->getPermissions()); $stored = $database->getCollection('columnGrantsCleared'); $this->assertSame([], $stored->getAttribute(Collection::COLUMN_PERMISSIONS)); - $this->assertSame(['read("any")', 'create("any")'], $stored->grants()); + $this->assertSame(['read("any")', 'create("any")'], $stored->declaredPermissions()); } /** @@ -231,7 +231,7 @@ public function testAColumnScopedGrantIsRefusedUnlessAskedFor(): void $this->assertSame( ['read("any")'], - $database->getCollection('columnGrantsGatedUpdate')->grants(), + $database->getCollection('columnGrantsGatedUpdate')->declaredPermissions(), ); } @@ -267,7 +267,7 @@ public function testTheColumnPermissionsToggleSurvivesASnapshot(): void $this->assertContains( 'read("any", "name")', - $database->getCollection('columnGrantsSnapshot')->grants() ?? [], + $database->getCollection('columnGrantsSnapshot')->declaredPermissions() ?? [], ); $this->assertFalse($database->hasColumnPermissions()); } @@ -308,7 +308,7 @@ public function testUpdatingACollectionJudgesAColumnGrantAgainstItsOwnColumns(): Permission::create(Role::any()), ]))); - $updated = $database->getCollection('columnGrantsUpdate')->grants(); + $updated = $database->getCollection('columnGrantsUpdate')->declaredPermissions(); $this->assertNotNull($updated); $this->assertContains('read("any", "name")', $updated); @@ -323,7 +323,7 @@ public function testUpdatingACollectionJudgesAColumnGrantAgainstItsOwnColumns(): } // The rejected update left the collection as it was. - $updated = $database->getCollection('columnGrantsUpdate')->grants(); + $updated = $database->getCollection('columnGrantsUpdate')->declaredPermissions(); $this->assertNotNull($updated); $this->assertContains('read("any", "name")', $updated); @@ -434,7 +434,7 @@ public function testAppwriteCollectionsSimulation(): void ], ))); - $grants = $database->getCollection('database_1_collection_1')->grants(); + $grants = $database->getCollection('database_1_collection_1')->declaredPermissions(); $this->assertNotNull($grants); // Appwrite's list of the database's tables, one row per table, shaped like the real diff --git a/tests/unit/Model/CollectionTest.php b/tests/unit/Model/CollectionTest.php index bb61e9226..e1f2c7597 100644 --- a/tests/unit/Model/CollectionTest.php +++ b/tests/unit/Model/CollectionTest.php @@ -44,7 +44,7 @@ public function testCreateStoresEveryField(): void $this->assertSame([ColumnType::String, ColumnType::Integer], \array_map(static fn (Attribute $attribute): ColumnType => $attribute->type, $collection->attributes())); $this->assertSame(['by_title'], \array_map(static fn (Index $index): string => $index->key, $collection->indexes())); $this->assertSame([OrderDirection::Asc], $collection->indexes()[0]->orders); - $this->assertSame(['read("any")'], $collection->grants()); + $this->assertSame(['read("any")'], $collection->declaredPermissions()); $this->assertFalse($collection->documentSecurity()); $this->assertFalse($collection->getAttribute('documentSecurity')); } @@ -56,7 +56,7 @@ public function testCreateDefaults(): void $this->assertSame('books', $collection->name()); $this->assertSame([], $collection->attributes()); $this->assertSame([], $collection->indexes()); - $this->assertNull($collection->grants()); + $this->assertNull($collection->declaredPermissions()); $this->assertTrue($collection->documentSecurity()); $this->assertTrue($collection->getAttribute('documentSecurity')); } @@ -78,7 +78,7 @@ public function testCreateStoresAttributesAndIndexesAsDocuments(): void public function testCreateWithNoPermissionsDeclaresAnEmptyList(): void { - $this->assertSame([], Collection::create('books', permissions: [])->grants()); + $this->assertSame([], Collection::create('books', permissions: [])->declaredPermissions()); } public function testCreateKeepsMetadataInStorage(): void @@ -106,7 +106,7 @@ public function testFromArrayKeepsExtrasAndNormalisesTheCoreKeys(): void $this->assertSame('books', $collection->name()); $this->assertTrue($collection->documentSecurity()); $this->assertTrue($collection->getAttribute('documentSecurity')); - $this->assertNull($collection->grants()); + $this->assertNull($collection->declaredPermissions()); $this->assertSame('fiction', $collection->getAttribute('category')); $this->assertSame('title', $collection->attributes()[0]->key); $this->assertSame(ColumnType::String, $collection->attributes()[0]->type); @@ -136,7 +136,7 @@ public function testFromArrayDropsPermissionsThatAreNotAList(): void { $collection = Collection::fromArray(['$id' => 'books', '$permissions' => 'read("any")']); - $this->assertNull($collection->grants()); + $this->assertNull($collection->declaredPermissions()); } public function testFromArrayRejectsInvalidPermissionEntries(): void @@ -205,11 +205,11 @@ public function testDeclaredPermissionsFollowStorage(): void $collection = Collection::create('books'); $collection->setAttribute('$permissions', [Permission::read(Role::any())]); - $this->assertSame(['read("any")'], $collection->grants()); + $this->assertSame(['read("any")'], $collection->declaredPermissions()); $collection->removeAttribute('$permissions'); - $this->assertNull($collection->grants()); + $this->assertNull($collection->declaredPermissions()); } public function testDocumentSecurityFollowsStorage(): void diff --git a/tests/unit/PermissionTest.php b/tests/unit/PermissionTest.php index 1a2d8ba9b..b9171da13 100644 --- a/tests/unit/PermissionTest.php +++ b/tests/unit/PermissionTest.php @@ -429,10 +429,13 @@ public function test_column_builders(): void $this->assertEquals('read("any", "name")', Permission::read(Role::any(), 'name')); $this->assertEquals('create("any", "name")', Permission::create(Role::any(), 'name')); $this->assertEquals('update("any", "name")', Permission::update(Role::any(), 'name')); - $this->assertEquals('delete("any", "name")', Permission::delete(Role::any(), 'name')); - $this->assertEquals('write("any", "name")', Permission::write(Role::any(), 'name')); $this->assertEquals('read("any")', Permission::read(Role::any())); + + // delete() and write() take the argument too -- a builder renders what it is handed -- + // but the validator refuses both, because a column cannot narrow a whole-row delete. + $this->assertEquals('delete("any", "name")', Permission::delete(Role::any(), 'name')); + $this->assertEquals('write("any", "name")', Permission::write(Role::any(), 'name')); } /** @@ -440,6 +443,13 @@ public function test_column_builders(): void * * @throws \Exception */ + /** + * Expanding a grant keeps its column. Shown through write(), which is the only type that + * expands -- the validator refuses a scoped write, so this is about the transform, not + * about a set anyone can store. + * + * @throws \Exception + */ public function test_aggregation_keeps_the_column(): void { $parsed = Permission::aggregate([Permission::write(Role::any(), 'salary')]); @@ -451,6 +461,17 @@ public function test_aggregation_keeps_the_column(): void ], $parsed); } + /** + * @throws \Exception + */ + public function test_a_wildcard_column_is_rejected(): void + { + $this->expectException(DatabaseException::class); + $this->expectExceptionMessage('Wildcard column "*" is not supported'); + + Permission::parse('read("any", "*")'); + } + /** * The form a read gate matches against: every grant with its column dropped. * diff --git a/tests/unit/Validator/PermissionsTest.php b/tests/unit/Validator/PermissionsTest.php index bd1f5da70..230beb424 100644 --- a/tests/unit/Validator/PermissionsTest.php +++ b/tests/unit/Validator/PermissionsTest.php @@ -405,4 +405,44 @@ public function test_column_scope_against_a_collection_without_columns(): void $this->assertTrue($object->isValid([Permission::read(Role::any())])); } + + /** + * Delete removes the whole row, so narrowing it to one column says nothing. Write + * aggregates to create, update and delete, so it inherits the objection. + */ + public function test_column_scope_rejected_for_delete_and_write(): void + { + $object = new Permissions(columns: ['name']); + + $this->assertFalse($object->isValid([Permission::delete(Role::any(), 'name')])); + $this->assertSame( + 'Permission "delete" cannot be scoped to a column, it applies to the whole row.', + $object->getDescription(), + ); + + $this->assertFalse($object->isValid([Permission::write(Role::any(), 'name')])); + $this->assertSame( + 'Permission "write" cannot be scoped to a column, it applies to the whole row.', + $object->getDescription(), + ); + + // The three that can be narrowed still are. + $this->assertTrue($object->isValid([ + Permission::read(Role::any(), 'name'), + Permission::create(Role::any(), 'name'), + Permission::update(Role::any(), 'name'), + ]), $object->getDescription()); + } + + /** + * A column that could never be a key is told why, rather than reported as one the + * collection happens not to have. + */ + public function test_column_scope_rejects_an_invalid_key(): void + { + $object = new Permissions(columns: ['name']); + + $this->assertFalse($object->isValid(['read("any", "_internal")'])); + $this->assertSame('Column "_internal" is not a valid column key.', $object->getDescription()); + } } From 69f0c53ec07a44a4f8f7127592e8d4b0ee47f992 Mon Sep 17 00:00:00 2001 From: fogelito Date: Wed, 14 Oct 2026 14:55:11 +0300 Subject: [PATCH 17/29] Selecting Column Permission --- src/Database/Trait/Documents.php | 141 ++++++++++++++++++ .../Adapter/Scopes/ColumnPermissionTests.php | 65 ++++++++ 2 files changed, 206 insertions(+) diff --git a/src/Database/Trait/Documents.php b/src/Database/Trait/Documents.php index 1efd53e99..0f40bf9d8 100644 --- a/src/Database/Trait/Documents.php +++ b/src/Database/Trait/Documents.php @@ -42,10 +42,12 @@ use Utopia\Database\Hook\Preparation; use Utopia\Database\Id; use Utopia\Database\Operator; +use Utopia\Database\Permission; use Utopia\Database\PermissionType; use Utopia\Database\Query; use Utopia\Database\RelationshipSide; use Utopia\Database\RelationshipType; +use Utopia\Database\Role; use Utopia\Database\Validator\Authorization\Input; use Utopia\Database\Validator\BigInt; use Utopia\Database\Validator\PartialStructure; @@ -1348,6 +1350,138 @@ private function assertCreateTenancy(string $collection): void * @throws DatabaseException * @throws StructureException */ + /** + * The columns the caller may reach on this collection, or null when nothing narrows them. + * + * Grants only ever add: a grant naming no column reaches every column, one naming a column + * reaches that column alone, and a caller's reach is the union over the grants whose role + * they hold. Nothing is restricted by default and nothing a grant says takes access away -- + * a column is simply unreachable until something grants it. + * + * Null means "do not narrow", which covers the ordinary collection where no grant names a + * column at all, and the caller who holds a whole-row grant. A document's own permissions + * never name a column, so a row-level grant is whole-row and reaches everything. + * + * @return list|null + * + * @throws DatabaseException + * @throws StructureException + */ + private function grantedColumns(Document $collection, PermissionType $type): ?array + { + $definition = Collection::fromDocument($collection); + + // The overwhelming majority of collections name no column anywhere, and pay one + // array check rather than a parse of every permission on every read. + if ($definition->getAttribute(Collection::COLUMN_PERMISSIONS, []) === []) { + return null; + } + + $columns = []; + $narrowed = false; + + foreach ($definition->declaredPermissions() ?? [] as $permission) { + $parsed = Permission::parse($permission); + + if ($parsed->getPermission() !== $type->value) { + continue; + } + + $role = (new Role($parsed->getRole(), $parsed->getIdentifier(), $parsed->getDimension()))->toString(); + + if (! $this->authorization->isValid(new Input($type, [$role]))) { + continue; + } + + if ($parsed->isForAllColumns()) { + return null; + } + + $narrowed = true; + $columns[] = $parsed->getColumn(); + } + + return $narrowed ? \array_values(\array_unique($columns)) : null; + } + + /** + * Refuse a query that names a column the caller may not read. + * + * Masking the result is not enough on its own: a filter on a column answers questions about + * it whether or not the value comes back, and Query::greaterThan() turns that into a binary + * search. Ordering leaks the order, and an aggregate leaks the aggregate. So anything that + * names a column is settled here, before the adapter sees it. + * + * @param array $queries + * + * @throws AuthorizationException + * @throws DatabaseException + * @throws StructureException + */ + private function assertQueryColumns(Document $collection, array $queries): void + { + if (! $this->authorization->getStatus()) { + return; + } + + $granted = $this->grantedColumns($collection, PermissionType::Read); + + if ($granted === null) { + return; + } + + foreach (self::columnsNamedBy($queries) as $column) { + if (! \in_array($column, $granted, true)) { + throw new AuthorizationException('Missing "read" permission for column "'.$column.'".'); + } + } + } + + /** + * Every column a set of queries names, nested ones included. + * + * @param array $queries + * @return list + */ + private static function columnsNamedBy(array $queries): array + { + $columns = []; + + foreach ($queries as $query) { + if (! $query instanceof Query) { + continue; + } + + if ($query->getMethod() === Method::Select) { + foreach ($query->getValues() as $value) { + // '*' names nothing in particular -- it asks for whatever the caller is + // entitled to, which is what a read with no select asks for too. Naming a + // column is what gets refused; asking for the lot is not. + if (\is_string($value) && $value !== '' && $value !== '*' && ! \str_starts_with($value, '$')) { + $columns[] = $value; + } + } + + continue; + } + + $attribute = $query->getAttribute(); + + if ($attribute !== '' && ! \str_starts_with($attribute, '$')) { + $columns[] = $attribute; + } + + // and/or carry their operands as values + $nested = \array_filter($query->getValues(), static fn (mixed $value): bool => $value instanceof Query); + + foreach (self::columnsNamedBy(\array_values($nested)) as $column) { + $columns[] = $column; + } + } + + return \array_values(\array_unique($columns)); + } + private function assertRowPermissions(Document $document): void { if (! $this->validation()->get()) { @@ -4079,6 +4213,8 @@ private function fetch(Collection $collection, array $queries, PermissionType $f $relationships = self::relationshipAttributes($collection); + $this->assertQueryColumns($collection, $queries); + $parsed = Query::groupByType($queries); $filters = $parsed->filters; $selects = $parsed->selections; @@ -4431,6 +4567,8 @@ public function count(string $collection, array $queries = [], ?int $max = null) $collection = $this->silent(fn () => $this->getCollection($collection)); + $this->assertQueryColumns($collection, $queries); + $this->checkQueryTypes($queries); $queries = self::resolveOneToManyJoins($collection, $queries); @@ -4488,6 +4626,9 @@ public function sum(string $collection, string $attribute, array $queries = [], $collection = $this->silent(fn () => $this->getCollection($collection)); + // The aggregated column is read too, so it is named alongside the queries. + $this->assertQueryColumns($collection, [...$queries, Query::sum($attribute)]); + $this->checkQueryTypes($queries); $queries = self::resolveOneToManyJoins($collection, $queries); diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php index e46e265ee..f867bb93c 100644 --- a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -9,10 +9,12 @@ use Utopia\Database\Database; use Utopia\Database\Document; use Utopia\Database\Exception as DatabaseException; +use Utopia\Database\Exception\Authorization as AuthorizationException; use Utopia\Database\Filter; use Utopia\Database\Mirror; use Utopia\Database\Permission; use Utopia\Database\PermissionType; +use Utopia\Database\Query; use Utopia\Database\Role; /** @@ -580,6 +582,69 @@ public function testEveryWritePathRefusesAColumnScopedPermission(): void } } + /** + * Asking for everything is not the same as naming a column you may not read. + * + * select(['*']) and a read with no select are the same request -- "whatever I am entitled + * to" -- and neither names a column, so neither is refused; the caller is given what their + * grants reach. Naming the column is what gets refused, even alongside one they may read, + * because a query that names a column answers questions about it whether or not the value + * comes back. + */ + public function testSelectingEverythingIsNotRefusedButNamingAnUngrantedColumnIs(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + $authorization = $database->getAuthorization(); + + $database->withColumnPermissions(fn () => $database->createCollection(Collection::create( + id: 'columnGrantsSelect', + attributes: [ + Attribute::string(key: 'name', size: 64), + Attribute::integer(key: 'salary'), + ], + permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any(), 'name'), + Permission::read(Role::team('hr'), 'salary'), + ], + ))); + + $authorization->skip(fn () => $database->createDocument('columnGrantsSelect', new Document([ + '$id' => 'e1', + 'name' => 'Ada', + 'salary' => 120000, + ]))); + + $previous = $authorization->getRoles(); + $authorization->cleanRoles(); + $authorization->addRole(Role::any()->toString()); + + try { + // Asking for the lot, however it is spelled. + $this->assertCount(1, $database->find('columnGrantsSelect', [Query::select(['*'])])); + $this->assertCount(1, $database->find('columnGrantsSelect')); + + // Naming only what the grant reaches. + $this->assertCount(1, $database->find('columnGrantsSelect', [Query::select(['name'])])); + + // Naming one it does not, even beside one it does. + foreach ([['salary'], ['name', 'salary']] as $selection) { + try { + $database->find('columnGrantsSelect', [Query::select($selection)]); + $this->fail('Expected select('.\implode(', ', $selection).') to be refused'); + } catch (AuthorizationException $e) { + $this->assertStringContainsString('Missing "read" permission for column "salary"', $e->getMessage()); + } + } + } finally { + $authorization->cleanRoles(); + foreach ($previous as $role) { + $authorization->addRole($role); + } + } + } + public function testADocumentPermissionNamingNoColumnIsUnaffected(): void { /** @var Database $database */ From 43240435e186fe2ec875adb8c6e747ffad918105 Mon Sep 17 00:00:00 2001 From: fogelito Date: Wed, 14 Oct 2026 15:45:16 +0300 Subject: [PATCH 18/29] fix WritableColumns --- src/Database/Trait/Documents.php | 51 +++++- .../Adapter/Scopes/ColumnPermissionTests.php | 145 +++++++++++++++++- 2 files changed, 190 insertions(+), 6 deletions(-) diff --git a/src/Database/Trait/Documents.php b/src/Database/Trait/Documents.php index 1e6196c40..7eb352b53 100644 --- a/src/Database/Trait/Documents.php +++ b/src/Database/Trait/Documents.php @@ -48,8 +48,8 @@ use Utopia\Database\Query; use Utopia\Database\RelationshipSide; use Utopia\Database\RelationshipType; -use Utopia\Database\Rounding; use Utopia\Database\Role; +use Utopia\Database\Rounding; use Utopia\Database\Validator\Authorization\Input; use Utopia\Database\Validator\BigInt; use Utopia\Database\Validator\PartialStructure; @@ -1485,6 +1485,41 @@ private function grantedColumns(Document $collection, PermissionType $type): ?ar return $narrowed ? \array_values(\array_unique($columns)) : null; } + /** + * Refuse a write that touches a column the caller may not change. + * + * The mirror of the read gate, and the simpler half: there is no query surface, only the + * columns the document carries. A caller granted one column may change that column and no + * other, and the write is refused whole rather than applied in part -- a half-applied update + * is harder to reason about than one that did not happen. + * + * @throws AuthorizationException + * @throws DatabaseException + * @throws StructureException + */ + private function assertWritableColumns(Document $collection, Document $document): void + { + if (! $this->authorization->getStatus()) { + return; + } + + $granted = $this->grantedColumns($collection, PermissionType::Update); + + if ($granted === null) { + return; + } + + foreach (\array_keys($document->getArrayCopy()) as $key) { + if (\str_starts_with($key, '$')) { + continue; + } + + if (! \in_array($key, $granted, true)) { + throw new AuthorizationException('Missing "update" permission for column "'.$key.'".'); + } + } + } + /** * Refuse a query that names a column the caller may not read. * @@ -1529,10 +1564,6 @@ private static function columnsNamedBy(array $queries): array $columns = []; foreach ($queries as $query) { - if (! $query instanceof Query) { - continue; - } - if ($query->getMethod() === Method::Select) { foreach ($query->getValues() as $value) { // '*' names nothing in particular -- it asks for whatever the caller is @@ -1850,6 +1881,9 @@ public function updateDocument(string $collection, string $id, Document $documen $this->assertRowPermissions($document); $collection = $this->silent(fn () => $this->getCollection($collection)); + + $this->assertWritableColumns($collection, $document); + $preparation = $this->nestedPreparation(lock: true); $newUpdatedAt = $document->getUpdatedAt(); $hasOperators = false; @@ -2174,6 +2208,7 @@ public function updateDocuments( $this->rejectJoins($queries, 'Join queries are not supported for bulk updates'); $this->assertRowPermissions($updates); + $this->assertWritableColumns($this->silent(fn () => $this->getCollection($collection)), $updates); if ($updates->isEmpty()) { return 0; @@ -4944,6 +4979,12 @@ private function pages( public function aggregate(string $collection, array $queries): array { $definition = $this->silent(fn () => $this->getCollection($collection)); + + // min, max, avg and the rest arrive as queries naming the column they aggregate, so the + // same check covers them: an aggregate over a column you cannot read discloses it as + // surely as reading it. + $this->assertQueryColumns($definition, $queries); + $queries = self::aliasAggregates($this->rewriteQueries(Event::DocumentAggregate, $definition, $queries)); $rows = []; diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php index f867bb93c..e343dbdfd 100644 --- a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -5,7 +5,7 @@ use Utopia\Database\Adapter\Feature; use Utopia\Database\Attribute; use Utopia\Database\Collection; -use Utopia\Database\CollectionUpdate; +use Utopia\Database\Collection\Update as CollectionUpdate; use Utopia\Database\Database; use Utopia\Database\Document; use Utopia\Database\Exception as DatabaseException; @@ -16,6 +16,7 @@ use Utopia\Database\PermissionType; use Utopia\Database\Query; use Utopia\Database\Role; +use Utopia\Database\Validator\Authorization; /** * Column permissions are set on a collection, never on a document. @@ -645,6 +646,148 @@ public function testSelectingEverythingIsNotRefusedButNamingAnUngrantedColumnIs( } } + /** + * A query naming a column the caller cannot read is refused, wherever it names it. + * + * Returning the rows would answer the question the filter asks whether or not the value + * comes back -- Query::greaterThan() turns that into a binary search -- and returning none + * is no better, since an empty result is a signal too and a wrong answer besides. Ordering + * leaks the order, and an aggregate leaks the aggregate. + */ + public function testAQueryNamingAnUngrantedColumnIsRefused(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + $authorization = $database->getAuthorization(); + + $database->withColumnPermissions(fn () => $database->createCollection(Collection::create( + id: 'columnGrantsQueries', + attributes: [ + Attribute::string(key: 'name', size: 64), + Attribute::integer(key: 'salary'), + ], + permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any(), 'name'), + Permission::read(Role::team('hr'), 'salary'), + ], + ))); + + $authorization->skip(fn () => $database->createDocument('columnGrantsQueries', new Document([ + '$id' => 'e1', + 'name' => 'Ada', + 'salary' => 120000, + ]))); + + $refused = [ + 'filter' => fn () => $database->find('columnGrantsQueries', [Query::equal('salary', [120000])]), + 'range filter' => fn () => $database->find('columnGrantsQueries', [Query::greaterThan('salary', 1)]), + 'order' => fn () => $database->find('columnGrantsQueries', [Query::orderDesc('salary')]), + 'select' => fn () => $database->find('columnGrantsQueries', [Query::select(['salary'])]), + 'count' => fn () => $database->count('columnGrantsQueries', [Query::equal('salary', [120000])]), + 'sum' => fn () => $database->sum('columnGrantsQueries', 'salary'), + 'aggregate' => fn () => $database->aggregate('columnGrantsQueries', [Query::max('salary')]), + ]; + + $this->asRole($authorization, Role::any()->toString(), function () use ($refused, $database): void { + foreach ($refused as $what => $run) { + try { + $run(); + $this->fail($what.' on an ungranted column was allowed'); + } catch (AuthorizationException $e) { + $this->assertStringContainsString('Missing "read" permission for column "salary"', $e->getMessage(), $what); + } + } + + // The column the grant does reach is queryable as usual. + $this->assertCount(1, $database->find('columnGrantsQueries', [Query::equal('name', ['Ada'])])); + }); + + // And it is per role, not per column: hr reaches salary and not name. + $this->asRole($authorization, Role::team('hr')->toString(), function () use ($database): void { + $this->assertSame(120000, $database->sum('columnGrantsQueries', 'salary')); + + try { + $database->find('columnGrantsQueries', [Query::equal('name', ['Ada'])]); + $this->fail('hr filtered on a column it was not granted'); + } catch (AuthorizationException $e) { + $this->assertStringContainsString('Missing "read" permission for column "name"', $e->getMessage()); + } + }); + } + + /** + * A write touching a column the caller may not change is refused, and refused whole. + * + * Applying the part they are allowed and dropping the rest would leave a document neither + * the caller nor the collection asked for. + */ + public function testAWriteTouchingAnUngrantedColumnIsRefused(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + $authorization = $database->getAuthorization(); + + $database->withColumnPermissions(fn () => $database->createCollection(Collection::create( + id: 'columnGrantsWritesScoped', + attributes: [ + Attribute::string(key: 'name', size: 64), + Attribute::integer(key: 'salary'), + ], + permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any()), + Permission::update(Role::any(), 'name'), + Permission::update(Role::team('hr'), 'salary'), + ], + ))); + + $authorization->skip(fn () => $database->createDocument('columnGrantsWritesScoped', new Document([ + '$id' => 'e1', + 'name' => 'Ada', + 'salary' => 120000, + ]))); + + $this->asRole($authorization, Role::any()->toString(), function () use ($database): void { + $this->assertSame('Grace', $database->updateDocument('columnGrantsWritesScoped', 'e1', new Document([ + 'name' => 'Grace', + ]))->getAttribute('name')); + + foreach (['salary alone' => ['salary' => 1], 'both at once' => ['name' => 'X', 'salary' => 2]] as $what => $data) { + try { + $database->updateDocument('columnGrantsWritesScoped', 'e1', new Document($data)); + $this->fail($what.' was allowed'); + } catch (AuthorizationException $e) { + $this->assertStringContainsString('Missing "update" permission for column "salary"', $e->getMessage(), $what); + } + } + }); + + // Refused whole: the granted half of "both at once" was not applied either. + $stored = $authorization->skip(fn () => $database->getDocument('columnGrantsWritesScoped', 'e1')); + $this->assertSame('Grace', $stored->getAttribute('name')); + $this->assertSame(120000, $stored->getAttribute('salary')); + } + + /** + * Run the callback holding one role and nothing else. + */ + private function asRole(Authorization $authorization, string $role, callable $callback): void + { + $previous = $authorization->getRoles(); + $authorization->cleanRoles(); + $authorization->addRole($role); + + try { + $callback(); + } finally { + $authorization->cleanRoles(); + foreach ($previous as $held) { + $authorization->addRole($held); + } + } + } + public function testADocumentPermissionNamingNoColumnIsUnaffected(): void { /** @var Database $database */ From 397034a225e937704e4359d4f3cdbd1ba1f7bef5 Mon Sep 17 00:00:00 2001 From: fogelito Date: Thu, 15 Oct 2026 12:56:05 +0300 Subject: [PATCH 19/29] maskColumns --- src/Database/Trait/Documents.php | 87 +++++-- .../Adapter/Scopes/ColumnPermissionTests.php | 219 +++++++++++++++++- 2 files changed, 292 insertions(+), 14 deletions(-) diff --git a/src/Database/Trait/Documents.php b/src/Database/Trait/Documents.php index 7eb352b53..c8fad145c 100644 --- a/src/Database/Trait/Documents.php +++ b/src/Database/Trait/Documents.php @@ -521,7 +521,11 @@ public function getDocument(string $collection, string $id, array $queries = [], ? self::sharedDefinition() : $this->silent(fn () => $this->getCollection($collection)); - return $this->readDocument($definition, $id, $this->rewriteQueries(Event::DocumentRead, $definition, $queries), $forUpdate); + $document = $this->readDocument($definition, $id, $this->rewriteQueries(Event::DocumentRead, $definition, $queries), $forUpdate); + + $this->maskColumns($definition, $document); + + return $document; } /** @@ -1268,6 +1272,8 @@ public function createDocument(string $collection, Document $document): Document $this->dispatch(new Event\Document\Created($collection->getId(), $document), $listeners); } + $this->maskColumns($collection, $document); + return $document; } @@ -1485,6 +1491,41 @@ private function grantedColumns(Document $collection, PermissionType $type): ?ar return $narrowed ? \array_values(\array_unique($columns)) : null; } + /** + * Remove the columns the caller was not granted from a document on its way back. + * + * The other half of the gate: a query naming an ungranted column is refused, and a read + * naming nothing is answered with what the caller may see. Asking for everything -- a bare + * read, or select('*') -- is not refusing, so it is shaped rather than thrown. + * + * The key is removed rather than nulled: null would be indistinguishable from a column that + * genuinely holds null, which tells the caller something untrue. System attributes are left + * alone; they are not columns and the row gate already decided them. + * + * @throws DatabaseException + * @throws StructureException + */ + private function maskColumns(Document $collection, Document $document): void + { + if (! $this->authorization->getStatus() || $document->isEmpty()) { + return; + } + + $granted = $this->grantedColumns($collection, PermissionType::Read); + + if ($granted === null) { + return; + } + + foreach (\array_keys($document->getArrayCopy()) as $key) { + if (\str_starts_with($key, '$') || \in_array($key, $granted, true)) { + continue; + } + + $document->removeAttribute($key); + } + } + /** * Refuse a write that touches a column the caller may not change. * @@ -1556,7 +1597,7 @@ private function assertQueryColumns(Document $collection, array $queries): void /** * Every column a set of queries names, nested ones included. * - * @param array $queries + * @param array $queries * @return list */ private static function columnsNamedBy(array $queries): array @@ -1564,6 +1605,10 @@ private static function columnsNamedBy(array $queries): array $columns = []; foreach ($queries as $query) { + if (! $query instanceof Query) { + continue; + } + if ($query->getMethod() === Method::Select) { foreach ($query->getValues() as $value) { // '*' names nothing in particular -- it asks for whatever the caller is @@ -1584,9 +1629,7 @@ private static function columnsNamedBy(array $queries): array } // and/or carry their operands as values - $nested = \array_filter($query->getValues(), static fn (mixed $value): bool => $value instanceof Query); - - foreach (self::columnsNamedBy(\array_values($nested)) as $column) { + foreach (self::columnsNamedBy($query->getValues()) as $column) { $columns[] = $column; } } @@ -1845,6 +1888,7 @@ function () use ($collection, $chunk): array { foreach ($batch as $document) { if ($onNext !== null) { + $this->maskColumns($collection, $document); $onNext($document, null); } @@ -2170,6 +2214,8 @@ public function updateDocument(string $collection, string $id, Document $documen $this->dispatch(new Event\Document\Updated($collection->getId(), $document), $listeners); } + $this->maskColumns($collection, $document); + return $document; } @@ -2422,6 +2468,8 @@ public function updateDocuments( foreach ($batch as $index => $doc) { if ($onNext !== null) { + $this->maskColumns($collection, $doc); + $this->maskColumns($collection, $old[$index]); $onNext($doc, $old[$index]); } $modified++; @@ -2848,9 +2896,11 @@ function () use ($collection, $increase, $chunk): array { if (! $old->isEmpty()) { $old = $this->castAfterDocument($collection, $old); + $this->maskColumns($collection, $old); } if ($onNext !== null) { + $this->maskColumns($collection, $doc); $onNext($doc, $old->isEmpty() ? null : $old); } } @@ -3040,6 +3090,8 @@ public function increaseDocumentAttribute( $this->dispatch(new Event\Document\Increased($collection->getId(), $document, $attribute), $listeners); } + $this->maskColumns($collection, $document); + return $document; } @@ -3171,6 +3223,8 @@ public function decreaseDocumentAttribute( $this->dispatch(new Event\Document\Decreased($collection->getId(), $document, $attribute), $listeners); } + $this->maskColumns($collection, $document); + return $document; } @@ -3433,6 +3487,7 @@ public function deleteDocuments( foreach ($batch as $document) { if ($onNext !== null) { + $this->maskColumns($collection, $document); $onNext($document, clone $document); } $modified++; @@ -4230,7 +4285,13 @@ public function find(string $collection, array $queries = [], PermissionType $fo { $definition = $this->silent(fn () => $this->getCollection($collection)); - return $this->fetch($definition, $this->rewriteQueries(Event::DocumentFind, $definition, $queries), $forPermission, false); + $documents = $this->fetch($definition, $this->rewriteQueries(Event::DocumentFind, $definition, $queries), $forPermission, false); + + foreach ($documents as $document) { + $this->maskColumns($definition, $document); + } + + return $documents; } /** @@ -4266,6 +4327,10 @@ private function fetch(Collection $collection, array $queries, PermissionType $f $relationships = self::relationshipAttributes($collection); + // Covers aggregate() too, which reads through here: min, max and the rest arrive as + // queries naming the column they aggregate, and an aggregate over a column you cannot + // read discloses it as surely as reading it. After checkQueryTypes() above, so a + // malformed query is still reported as malformed rather than reaching this. $this->assertQueryColumns($collection, $queries); $parsed = Query::groupByType($queries); @@ -4598,9 +4663,9 @@ public function count(string $collection, array $queries = [], ?int $max = null) $collection = $this->silent(fn () => $this->getCollection($collection)); $queries = $this->rewriteQueries(Event::DocumentCount, $collection, $queries); + $this->checkQueryTypes($queries); $this->assertQueryColumns($collection, $queries); - $this->checkQueryTypes($queries); $queries = self::resolveOneToManyJoins($collection, $queries); $joinedCollections = null; @@ -4658,10 +4723,11 @@ public function sum(string $collection, string $attribute, array $queries = [], $collection = $this->silent(fn () => $this->getCollection($collection)); $queries = $this->rewriteQueries(Event::DocumentSum, $collection, $queries); + $this->checkQueryTypes($queries); + // The aggregated column is read too, so it is named alongside the queries. $this->assertQueryColumns($collection, [...$queries, Query::sum($attribute)]); - $this->checkQueryTypes($queries); $queries = self::resolveOneToManyJoins($collection, $queries); $joinedCollections = null; @@ -4980,11 +5046,6 @@ public function aggregate(string $collection, array $queries): array { $definition = $this->silent(fn () => $this->getCollection($collection)); - // min, max, avg and the rest arrive as queries naming the column they aggregate, so the - // same check covers them: an aggregate over a column you cannot read discloses it as - // surely as reading it. - $this->assertQueryColumns($definition, $queries); - $queries = self::aliasAggregates($this->rewriteQueries(Event::DocumentAggregate, $definition, $queries)); $rows = []; diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php index e343dbdfd..c601c8c6b 100644 --- a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -4,6 +4,7 @@ use Utopia\Database\Adapter\Feature; use Utopia\Database\Attribute; +use Utopia\Database\Capability; use Utopia\Database\Collection; use Utopia\Database\Collection\Update as CollectionUpdate; use Utopia\Database\Database; @@ -686,9 +687,14 @@ public function testAQueryNamingAnUngrantedColumnIsRefused(): void 'select' => fn () => $database->find('columnGrantsQueries', [Query::select(['salary'])]), 'count' => fn () => $database->count('columnGrantsQueries', [Query::equal('salary', [120000])]), 'sum' => fn () => $database->sum('columnGrantsQueries', 'salary'), - 'aggregate' => fn () => $database->aggregate('columnGrantsQueries', [Query::max('salary')]), ]; + // An adapter without aggregates rejects max() as a method before the column is reached, + // which is the right order: an unsupported query is not a permission question. + if ($database->getAdapter()->supports(Capability::Aggregations)) { + $refused['aggregate'] = fn () => $database->aggregate('columnGrantsQueries', [Query::max('salary')]); + } + $this->asRole($authorization, Role::any()->toString(), function () use ($refused, $database): void { foreach ($refused as $what => $run) { try { @@ -788,6 +794,217 @@ private function asRole(Authorization $authorization, string $role, callable $ca } } + /** + * A read that names nothing comes back holding what the caller was granted. + * + * The mirror of the refusal: naming an ungranted column throws, asking for everything does + * not, so the answer is shaped instead. The key is removed rather than nulled, since null + * would be indistinguishable from a column that genuinely holds null. + */ + public function testAReadComesBackHoldingOnlyTheGrantedColumns(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + $authorization = $database->getAuthorization(); + + $database->withColumnPermissions(fn () => $database->createCollection(Collection::create( + id: 'columnGrantsMasked', + attributes: [ + Attribute::string(key: 'name', size: 64), + Attribute::integer(key: 'salary'), + ], + permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any(), 'name'), + Permission::read(Role::team('hr'), 'salary'), + Permission::read(Role::team('exec')), + ], + ))); + + $authorization->skip(fn () => $database->createDocument('columnGrantsMasked', new Document([ + '$id' => 'e1', + 'name' => 'Ada', + 'salary' => 120000, + ]))); + + $columns = static fn (Document $document): array => \array_keys(\array_intersect_key( + $document->getArrayCopy(), + ['name' => true, 'salary' => true], + )); + + // Granted one column: the other is gone, not nulled. + $this->asRole($authorization, Role::any()->toString(), function () use ($database, $columns): void { + $stored = $database->getDocument('columnGrantsMasked', 'e1'); + + $this->assertSame(['name'], $columns($stored)); + $this->assertSame('Ada', $stored->getAttribute('name')); + $this->assertFalse($stored->offsetExists('salary'), 'salary came back for a role granted only name'); + + // find() and select('*') shape the same way, and the system attributes survive. + $this->assertSame(['name'], $columns($database->find('columnGrantsMasked')[0])); + $this->assertSame(['name'], $columns($database->find('columnGrantsMasked', [Query::select(['*'])])[0])); + $this->assertSame('e1', $database->find('columnGrantsMasked')[0]->getId()); + }); + + // A different role, a different column. + $this->asRole($authorization, Role::team('hr')->toString(), function () use ($database, $columns): void { + $this->assertSame(['salary'], $columns($database->getDocument('columnGrantsMasked', 'e1'))); + }); + + // A whole-row grant names no column, so nothing is withheld from it. + $this->asRole($authorization, Role::team('exec')->toString(), function () use ($database, $columns): void { + $this->assertSame(['name', 'salary'], $columns($database->getDocument('columnGrantsMasked', 'e1'))); + }); + + // Internal reads are not shaped, or the library would lose columns it needs to write. + $this->assertSame( + ['name', 'salary'], + $columns($authorization->skip(fn () => $database->getDocument('columnGrantsMasked', 'e1'))), + ); + } + + /** + * A write hands its result back the same way a read does. + * + * An update reads the row to merge the patch onto it, so its response carries every column + * the row holds -- including the ones the caller may not read. The write itself is judged + * separately: a role may be allowed to change a column it cannot see, and then the change + * lands while the answer still withholds it. + */ + public function testAWriteComesBackHoldingOnlyTheGrantedColumns(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + $authorization = $database->getAuthorization(); + + $database->withColumnPermissions(fn () => $database->createCollection(Collection::create( + id: 'columnGrantsWritten', + attributes: [ + Attribute::string(key: 'name', size: 64), + Attribute::integer(key: 'salary'), + ], + permissions: [ + Permission::create(Role::any()), + Permission::update(Role::any()), + Permission::read(Role::any(), 'name'), + Permission::update(Role::team('exec')), + Permission::read(Role::team('exec')), + ], + ))); + + $columns = static fn (Document $document): array => \array_keys(\array_intersect_key( + $document->getArrayCopy(), + ['name' => true, 'salary' => true], + )); + + $this->asRole($authorization, Role::any()->toString(), function () use ($database, $columns): void { + $created = $database->createDocument('columnGrantsWritten', new Document([ + '$id' => 'w1', + 'name' => 'Ada', + 'salary' => 120000, + ])); + $this->assertSame(['name'], $columns($created), 'createDocument() handed back an ungranted column'); + + $updated = $database->updateDocument('columnGrantsWritten', 'w1', new Document([ + 'name' => 'Ada Lovelace', + ])); + $this->assertSame(['name'], $columns($updated), 'updateDocument() handed back an ungranted column'); + $this->assertSame('Ada Lovelace', $updated->getAttribute('name')); + + if ($database->getAdapter()->hasFeature(Feature\Upserts::class)) { + $upserted = $database->upsertDocument('columnGrantsWritten', new Document([ + '$id' => 'w1', + 'name' => 'Ada King', + ])); + $this->assertSame(['name'], $columns($upserted), 'upsertDocument() handed back an ungranted column'); + } + + // A counter may be bumped by a role that cannot read it, and the answer still withholds it. + $increased = $database->increaseDocumentAttribute('columnGrantsWritten', 'w1', 'salary', 1000); + $this->assertSame(['name'], $columns($increased), 'increaseDocumentAttribute() handed back an ungranted column'); + + $decreased = $database->decreaseDocumentAttribute('columnGrantsWritten', 'w1', 'salary', 500); + $this->assertSame(['name'], $columns($decreased), 'decreaseDocumentAttribute() handed back an ungranted column'); + }); + + // Shaping the answer is not editing the row: every write above landed in full. + $stored = $authorization->skip(fn () => $database->getDocument('columnGrantsWritten', 'w1')); + $this->assertSame(['name', 'salary'], $columns($stored)); + $this->assertSame(120500, $stored->getAttribute('salary')); + + // A whole-row grant names no column, so nothing is withheld from it. + $this->asRole($authorization, Role::team('exec')->toString(), function () use ($database, $columns): void { + $this->assertSame( + ['name', 'salary'], + $columns($database->updateDocument('columnGrantsWritten', 'w1', new Document(['name' => 'Ada']))), + ); + }); + } + + /** + * A batch write reports each row through onNext, which is the same way out. + * + * Both documents it is handed are shaped: the one just written and the one it replaced, + * since the previous row is read from storage in full. + */ + public function testABatchWriteHandsBackOnlyTheGrantedColumns(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + $authorization = $database->getAuthorization(); + + $database->withColumnPermissions(fn () => $database->createCollection(Collection::create( + id: 'columnGrantsBatched', + attributes: [ + Attribute::string(key: 'name', size: 64), + Attribute::integer(key: 'salary'), + ], + permissions: [ + Permission::create(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + Permission::read(Role::any(), 'name'), + ], + ))); + + $columns = static fn (Document $document): array => \array_keys(\array_intersect_key( + $document->getArrayCopy(), + ['name' => true, 'salary' => true], + )); + + $this->asRole($authorization, Role::any()->toString(), function () use ($database, $columns): void { + $seen = []; + $collect = function (Document $document, ?Document $previous) use (&$seen, $columns): void { + $seen[] = $columns($document); + if ($previous !== null) { + $seen[] = $columns($previous); + } + }; + + $database->createDocuments('columnGrantsBatched', [ + new Document(['$id' => 'b1', 'name' => 'Ada', 'salary' => 120000]), + new Document(['$id' => 'b2', 'name' => 'Grace', 'salary' => 130000]), + ], onNext: $collect); + $this->assertSame([['name'], ['name']], $seen, 'createDocuments() reported an ungranted column'); + + $seen = []; + $database->updateDocuments('columnGrantsBatched', new Document(['name' => 'Redacted']), onNext: $collect); + $this->assertSame([['name'], ['name'], ['name'], ['name']], $seen, 'updateDocuments() reported an ungranted column'); + + if ($database->getAdapter()->hasFeature(Feature\Upserts::class)) { + $seen = []; + $database->upsertDocuments('columnGrantsBatched', [ + new Document(['$id' => 'b1', 'name' => 'Ada', 'salary' => 1]), + ], onNext: $collect); + $this->assertSame([['name'], ['name']], $seen, 'upsertDocuments() reported an ungranted column'); + } + + $seen = []; + $database->deleteDocuments('columnGrantsBatched', onNext: $collect); + $this->assertSame([['name'], ['name'], ['name'], ['name']], $seen, 'deleteDocuments() reported an ungranted column'); + }); + } + public function testADocumentPermissionNamingNoColumnIsUnaffected(): void { /** @var Database $database */ From 2f121974057ff13390b2fcd1616ebb35abf1c630 Mon Sep 17 00:00:00 2001 From: fogelito Date: Thu, 15 Oct 2026 13:44:43 +0300 Subject: [PATCH 20/29] reject Joins perms --- src/Database/Trait/Documents.php | 75 +++++++ .../Adapter/Scopes/ColumnPermissionTests.php | 199 ++++++++++++++++-- 2 files changed, 252 insertions(+), 22 deletions(-) diff --git a/src/Database/Trait/Documents.php b/src/Database/Trait/Documents.php index 7a2ce2b38..08d556b9a 100644 --- a/src/Database/Trait/Documents.php +++ b/src/Database/Trait/Documents.php @@ -1494,6 +1494,76 @@ private function grantedColumns(Document $collection, PermissionType $type): ?ar return $narrowed ? \array_values(\array_unique($columns)) : null; } + /** + * Refuse a join while a column restriction applies to either side of it. + * + * A join names columns in a second collection under an alias, and returns them beside the + * base row as "alias.column". Neither this gate nor maskColumns() resolves that alias: the + * gate would read the joined names against the base collection's grants, and the mask only + * knows the base collection, so the joined columns would come back unshaped. That cuts both + * ways -- a restricted base must not reach into an open collection, and an open base must + * not be used to read a restricted one -- so the join is refused if either side narrows for + * this role. A role holding a whole-row grant on both sides is unrestricted and joins as it + * always did. + * + * @param array $queries + * + * @throws QueryException + * @throws DatabaseException + * @throws StructureException + */ + private function rejectRestrictedJoins(Document $collection, array $queries): void + { + $joined = self::joinedBy($queries); + + if ($joined === []) { + return; + } + + $message = 'Join queries are not supported while a column-scoped permission applies'; + + if ($this->grantedColumns($collection, PermissionType::Read) !== null) { + throw new QueryException($message); + } + + foreach ($joined as $id) { + // A join naming no such collection is validation's to report, not this gate's. + $definition = $this->silent(fn (): ?Collection => $this->findCollection($id)); + + if ($definition !== null && $this->grantedColumns($definition, PermissionType::Read) !== null) { + throw new QueryException($message); + } + } + } + + /** + * Every collection a set of queries joins, nested joins included. + * + * @param array $queries + * @return list + */ + private static function joinedBy(array $queries): array + { + $ids = []; + + foreach ($queries as $query) { + if (! $query instanceof Query) { + continue; + } + + if ($query->getMethod()->isJoin() && $query->getAttribute() !== '') { + $ids[] = $query->getAttribute(); + } + + // A nested join is a join among its parent's values. + foreach (self::joinedBy($query->getValues()) as $id) { + $ids[] = $id; + } + } + + return \array_values(\array_unique($ids)); + } + /** * Remove the columns the caller was not granted from a document on its way back. * @@ -1584,6 +1654,11 @@ private function assertQueryColumns(Document $collection, array $queries): void return; } + // Before the base collection's own grant is consulted: a join is refused for a + // restriction on either side, and an unrestricted base is exactly the case where one + // can be used to read a restricted collection. + $this->rejectRestrictedJoins($collection, $queries); + $granted = $this->grantedColumns($collection, PermissionType::Read); if ($granted === null) { diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php index c601c8c6b..76e83c0d6 100644 --- a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -11,6 +11,7 @@ use Utopia\Database\Document; use Utopia\Database\Exception as DatabaseException; use Utopia\Database\Exception\Authorization as AuthorizationException; +use Utopia\Database\Exception\Query as QueryException; use Utopia\Database\Filter; use Utopia\Database\Mirror; use Utopia\Database\Permission; @@ -623,9 +624,16 @@ public function testSelectingEverythingIsNotRefusedButNamingAnUngrantedColumnIs( $authorization->addRole(Role::any()->toString()); try { - // Asking for the lot, however it is spelled. - $this->assertCount(1, $database->find('columnGrantsSelect', [Query::select(['*'])])); - $this->assertCount(1, $database->find('columnGrantsSelect')); + // Asking for the lot, however it is spelled: not refused, but answered with only + // what the grant reaches. Refusing '*' would make a plain read unusable, so the + // answer is shaped instead -- the other half of the gate. + foreach ([[Query::select(['*'])], []] as $queries) { + $rows = $database->find('columnGrantsSelect', $queries); + + $this->assertCount(1, $rows); + $this->assertSame('Ada', $rows[0]->getAttribute('name')); + $this->assertFalse($rows[0]->offsetExists('salary'), 'select(*) returned an ungranted column'); + } // Naming only what the grant reaches. $this->assertCount(1, $database->find('columnGrantsSelect', [Query::select(['name'])])); @@ -972,36 +980,183 @@ public function testABatchWriteHandsBackOnlyTheGrantedColumns(): void ['name' => true, 'salary' => true], )); - $this->asRole($authorization, Role::any()->toString(), function () use ($database, $columns): void { + // Each write gets its own collector, so what one reported cannot carry into the next. + $shapesFrom = static function (callable $write) use ($columns): array { + /** @var list> $seen */ $seen = []; - $collect = function (Document $document, ?Document $previous) use (&$seen, $columns): void { + + $write(static function (Document $document, ?Document $previous) use (&$seen, $columns): void { $seen[] = $columns($document); + if ($previous !== null) { $seen[] = $columns($previous); } - }; + }); - $database->createDocuments('columnGrantsBatched', [ - new Document(['$id' => 'b1', 'name' => 'Ada', 'salary' => 120000]), - new Document(['$id' => 'b2', 'name' => 'Grace', 'salary' => 130000]), - ], onNext: $collect); - $this->assertSame([['name'], ['name']], $seen, 'createDocuments() reported an ungranted column'); + return $seen; + }; - $seen = []; - $database->updateDocuments('columnGrantsBatched', new Document(['name' => 'Redacted']), onNext: $collect); - $this->assertSame([['name'], ['name'], ['name'], ['name']], $seen, 'updateDocuments() reported an ungranted column'); + $this->asRole($authorization, Role::any()->toString(), function () use ($database, $shapesFrom): void { + $this->assertSame( + [['name'], ['name']], + $shapesFrom(static fn (callable $onNext) => $database->createDocuments('columnGrantsBatched', [ + new Document(['$id' => 'b1', 'name' => 'Ada', 'salary' => 120000]), + new Document(['$id' => 'b2', 'name' => 'Grace', 'salary' => 130000]), + ], onNext: $onNext)), + 'createDocuments() reported an ungranted column', + ); + + $this->assertSame( + [['name'], ['name'], ['name'], ['name']], + $shapesFrom(static fn (callable $onNext) => $database->updateDocuments( + 'columnGrantsBatched', + new Document(['name' => 'Redacted']), + onNext: $onNext, + )), + 'updateDocuments() reported an ungranted column', + ); if ($database->getAdapter()->hasFeature(Feature\Upserts::class)) { - $seen = []; - $database->upsertDocuments('columnGrantsBatched', [ - new Document(['$id' => 'b1', 'name' => 'Ada', 'salary' => 1]), - ], onNext: $collect); - $this->assertSame([['name'], ['name']], $seen, 'upsertDocuments() reported an ungranted column'); + $this->assertSame( + [['name'], ['name']], + $shapesFrom(static fn (callable $onNext) => $database->upsertDocuments('columnGrantsBatched', [ + new Document(['$id' => 'b1', 'name' => 'Ada', 'salary' => 1]), + ], onNext: $onNext)), + 'upsertDocuments() reported an ungranted column', + ); } - $seen = []; - $database->deleteDocuments('columnGrantsBatched', onNext: $collect); - $this->assertSame([['name'], ['name'], ['name'], ['name']], $seen, 'deleteDocuments() reported an ungranted column'); + $this->assertSame( + [['name'], ['name'], ['name'], ['name']], + $shapesFrom(static fn (callable $onNext) => $database->deleteDocuments('columnGrantsBatched', onNext: $onNext)), + 'deleteDocuments() reported an ungranted column', + ); + }); + } + + /** + * A role under a column restriction may not join. + * + * A join names columns in a second collection under an alias, and returns them beside the + * base row. The gate reads those names against the base collection and the mask only knows + * the base collection, so a join would be judged against the wrong schema and its columns + * would come back unshaped. Refusing is the honest answer until both halves follow aliases. + * + * A whole-row grant is unrestricted and joins exactly as it always did. + */ + public function testARestrictedRoleMayNotJoin(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + if (! $database->getAdapter()->supports(Capability::Joins)) { + $this->markTestSkipped('Adapter does not support joins'); + } + + $authorization = $database->getAuthorization(); + + $database->createCollection(Collection::create( + id: 'columnGrantsJoined', + attributes: [ + Attribute::string(key: 'mainId', size: 64), + Attribute::string(key: 'note', size: 64) + ], + permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any()), + // asRole() replaces the role list, so exec holds nothing that "any" would cover. + Permission::read(Role::team('exec')), + ], + )); + + $database->withColumnPermissions(fn () => $database->createCollection(Collection::create( + id: 'columnGrantsJoinBase', + attributes: [ + Attribute::string(key: 'name', size: 64), + Attribute::integer(key: 'salary'), + ], + permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any(), 'name'), + Permission::read(Role::team('exec')), + ], + ))); + + $authorization->skip(function () use ($database): void { + $database->createDocument('columnGrantsJoinBase', new Document(['$id' => 'm1', 'name' => 'Ada', 'salary' => 120000])); + $database->createDocument('columnGrantsJoined', new Document(['$id' => 's1', 'mainId' => 'm1', 'note' => 'hi'])); + }); + + $this->asRole($authorization, Role::any()->toString(), function () use ($database): void { + try { + $database->find('columnGrantsJoinBase', [ + Query::join('columnGrantsJoined', 'entry', [Query::on('$id', 'mainId')]), + ]); + $this->fail('a column-restricted role was allowed to join'); + } catch (QueryException $e) { + $this->assertStringContainsString('Join queries are not supported', $e->getMessage()); + } + + // The refusal does not mention a column, because the join names none of the base ones. + try { + $database->find('columnGrantsJoinBase', [ + Query::join('columnGrantsJoined', 'entry', [Query::on('$id', 'mainId')]), + Query::equal('entry.note', ['hi']), + ]); + $this->fail('a column-restricted role was allowed to join and filter the joined collection'); + } catch (QueryException $e) { + $this->assertStringNotContainsString('columnGrantsJoined', $e->getMessage()); + } + }); + + // A whole-row grant is not restricted, so joins keep working for it. + $this->asRole($authorization, Role::team('exec')->toString(), function () use ($database): void { + $rows = $database->find('columnGrantsJoinBase', [ + Query::join('columnGrantsJoined', 'entry', [Query::on('$id', 'mainId')]), + ]); + + $this->assertCount(1, $rows); + $this->assertSame('Ada', $rows[0]->getAttribute('name')); + $this->assertSame(120000, $rows[0]->getAttribute('salary')); + }); + + // The other direction: an open collection must not be used as a way into a restricted + // one. The base names no column, so nothing about it is refused -- the join is. + $database->createCollection(Collection::create( + id: 'columnGrantsJoinOpen', + attributes: [Attribute::string(key: 'baseId', size: 64), Attribute::string(key: 'label', size: 64)], + permissions: [Permission::create(Role::any()), Permission::read(Role::any())], + )); + + $authorization->skip(fn () => $database->createDocument('columnGrantsJoinOpen', new Document([ + '$id' => 'o1', + 'baseId' => 'm1', + 'label' => 'row', + ]))); + + $this->asRole($authorization, Role::any()->toString(), function () use ($database): void { + try { + $database->find('columnGrantsJoinOpen', [ + Query::join('columnGrantsJoinBase', 'secret', [Query::on('baseId', '$id')]), + ]); + $this->fail('an open collection was joined into a column-restricted one'); + } catch (QueryException $e) { + $this->assertStringContainsString('Join queries are not supported', $e->getMessage()); + } + + // And filtering on the restricted column through the alias is refused with it. + try { + $database->find('columnGrantsJoinOpen', [ + Query::join('columnGrantsJoinBase', 'secret', [Query::on('baseId', '$id')]), + Query::equal('secret.salary', [120000]), + ]); + $this->fail('a restricted column was filtered through a join alias'); + } catch (QueryException $e) { + $this->assertStringContainsString('Join queries are not supported', $e->getMessage()); + } + + // Without the join the open collection reads as it always did. + $this->assertCount(1, $database->find('columnGrantsJoinOpen')); }); } From b7d619db207d01c1af6f21ad54d4c78c757fbdf1 Mon Sep 17 00:00:00 2001 From: fogelito Date: Thu, 15 Oct 2026 14:02:54 +0300 Subject: [PATCH 21/29] Change log --- CHANGELOG.md | 61 +++++++++++++ UPGRADE.md | 76 ++++++++++++++++ src/Database/Trait/Documents.php | 37 ++++---- .../Adapter/Scopes/ColumnPermissionTests.php | 90 +++++++++++++++++++ 4 files changed, 243 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76f7a6e33..b27ab2f20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,66 @@ # Changelog +## 8.2.0 + +8.2.0 lets a collection's permission name one of its columns, so `read("any", "name")` grants that column alone. It +is off until asked for and changes nothing for a collection that uses none; see +[Upgrading from 8.1.4 to 8.2.0](UPGRADE.md#upgrading-from-814-to-820) for the one schema change it needs. + +Grants only ever add. A grant naming no column reaches every column, as every grant always has; one naming a column +reaches that column alone; and a caller reaches the union over the grants whose role they hold. Nothing is restricted +by default and no grant takes access away -- a column is unreachable until something grants it, and a role holding +one whole-row grant is never narrowed by a column-scoped grant beside it. + +### Added + +- `Permission::read()`, `create()` and `update()` take an optional column: `Permission::read(Role::any(), 'name')` + renders `read("any", "name")`, and `Permission::parse()` reads it back. `delete()` and `write()` reject a column, + because a delete removes the whole row and a write aggregates to create, update and delete. The column must be a + column of the collection; `"*"` and an empty column are refused, since omitting the argument already means every + column. +- `Permission::getColumn(): string` and `Permission::isForAllColumns(): bool` report what one parsed permission + names. `Permission::hasColumns(array $permissions): bool` and `Permission::withoutColumns(array $permissions): + array` answer for a list. `Permission::aggregate()` carries a column through the permissions it expands. +- `Database::withColumnPermissions(callable $callback): mixed` runs `$callback` with column-scoped permissions + allowed, and `Database::hasColumnPermissions(): bool` reports whether they are. `createCollection()` and + `updateCollection()` throw `Exception` with `Column-scoped permissions are not enabled` for a permission naming a + column outside the scope, so a caller opts in once where it means to. The scope is captured by `getSnapshot()` and + restored by `withToggles()`, and `Mirror::withColumnPermissions()` opens it on the mirror, its source and its + destination. +- `Collection::declaredPermissions(): ?array` returns a collection's permissions as they were written, column-scoped + ones included. `getPermissions()` keeps returning what the read gate matches, which is the same grants with every + column dropped. Use `declaredPermissions()` to report or replay a collection's grants and `getPermissions()` for + the gate. +- `_metadata` holds a `columnPermissions` column, a JSON list of the collection's permissions as they were written, + stored only when one of them names a column and cleared when the last one is dropped. `Collection::setPermissions()` + writes it and `$permissions` together, so the two cannot drift. + +### Changed + +- A read answers with the columns the caller was granted. A query naming a column the caller may not read is refused + with `Exception\Authorization` (`Missing "read" permission for column "salary"`) -- filters, ordering, `select()`, + `count()`, `sum()` and the aggregates of `aggregate()` alike, since an aggregate over a column discloses it as + surely as reading it. Asking for everything is not naming anything, so a bare read and `select(['*'])` are answered + rather than refused: the columns the caller was not granted are removed from the document, keys and all, by + `getDocument()`, `find()`, `findOne()`, `cursor()`, every write that returns a document, and the documents a bulk + write reports through `onNext`. A related document is shaped by the grants of its own collection. The key is + removed rather than nulled, because a null would be indistinguishable from a column that genuinely holds null. +- A write touching a column the caller may not change is refused whole, with `Exception\Authorization` + (`Missing "update" permission for column "salary"`). Applying the granted part and dropping the rest would store a + document neither the caller nor the collection asked for. +- A join is refused with `Exception\Query` (`Join queries are not supported while a column-scoped permission + applies`) when a column restriction applies to the base collection or to any collection it joins, nested joins + included. A join returns the joined columns under an alias, which neither the query gate nor the masking resolves, + so it would judge them against the wrong collection's grants and return them unshaped. A role holding a whole-row + grant on every collection in the join is not narrowed and joins as before. +- A row's permissions may not name a column, on every path that writes them: `createDocument()`, + `createDocuments()`, `updateDocument()`, `updateDocuments()`, `upsertDocument()` and `upsertDocuments()`. Column + permissions belong to a collection; stored on a row they would sit where nothing enforces them, and a column in + that list is what every read gate matches against -- the `_perms` side table, PostgreSQL's JSONB containment, + MongoDB's `$in` -- so the row would be hidden from the very role it was granted to. +- Enforcement does not depend on the scope. `withColumnPermissions()` governs only whether a column-scoped grant may + be written; once a collection holds one, every read and write is gated and shaped by it. + ## 8.1.4 8.1.4 fixes `deleteDocuments()` deleting documents that stopped matching its queries after they were found. It needs diff --git a/UPGRADE.md b/UPGRADE.md index 79ce574f2..5c1e66d09 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,3 +1,79 @@ +# Upgrading from 8.1.4 to 8.2.0 + +8.2.0 adds column-scoped collection permissions. It needs no code change and no adapter change, and a database that +uses none behaves as in 8.1.4. It does need one column added to the metadata table; see +[CHANGELOG.md](CHANGELOG.md#820) for the feature itself. + +- [The `columnPermissions` column](#the-columnpermissions-column) +- [Reading a collection's permissions](#reading-a-collections-permissions) +- [Granting a column](#granting-a-column) +- [Joins](#joins) + +## The `columnPermissions` column + +`_metadata` holds a new `columnPermissions` column: a JSON list, defaulting to `[]`. The metadata table is described +in code rather than by a row of its own, so `createAttribute()` cannot add it and an existing database needs the +column created directly, before any code of 8.2.0 reads the table. Create it nullable, or with `[]` as its default, +so the collections already stored keep working unchanged. + +On a host with shared tables the column belongs to the shared table, so add it once per table, not once per tenant. +Adding it while a tenant is set gives its row that tenant and leaves the column missing for every other tenant on +the host. + +Running 8.2.0 before the column exists degrades in one direction only, so an upgrade that cannot add it in the same +step is not left broken: reading is unaffected, and changing the schema fails until it is there. + +| | Without the column | +|---|---| +| `getCollection()`, `getDocument()`, `find()` and the rest of the reads | work, since they select every column rather than naming them | +| `createDocument()` and the other document writes | work | +| `createCollection()` | fails with `Failed to create collection metadata for '': Attribute not found` | +| `updateCollection()` | fails with `Exception\NotFound` (`Attribute not found`) | + +Add the column first and deploy second. Deploying first leaves an application that serves reads and document writes +but cannot create or change a collection until the column is added, at which point it recovers with no restart. + +## Reading a collection's permissions + +`Collection::getPermissions()` keeps its meaning: the grants as the read gate matches them, which is every grant with +its column dropped. That is what the gate must see -- the `_perms` side table, PostgreSQL's JSONB containment and +MongoDB's `$in` all match the stored form, so a column left in it would hide the collection from the role it was +granted to. + +Code that reports a collection's permissions back to a caller, or replays them onto another collection, wants +`Collection::declaredPermissions()` instead, which returns them as they were written. For a collection with no +column-scoped grant the two return the same list, so this only matters once a column is granted. + +## Granting a column + +A column-scoped grant is refused unless the call asks for one: + +```php +$database->withColumnPermissions(fn () => $database->createCollection(Collection::create( + id: 'employees', + attributes: [Attribute::string(key: 'name', size: 64), Attribute::integer(key: 'salary')], + permissions: [ + Permission::read(Role::any(), 'name'), + Permission::read(Role::team('hr')), + ], +))); +``` + +Grants add rather than restrict: `any` reaches `name` alone, `hr` holds a whole-row grant and reaches everything. A +caller reaches the union over the grants whose role they hold, so a whole-row grant is never narrowed by a +column-scoped one beside it. + +Once a collection holds such a grant it is enforced on every read and write, with or without the scope: a query +naming an ungranted column throws `Exception\Authorization`, a write touching one is refused whole, and a read that +names nothing comes back holding only the granted columns. A row's own permissions may never name a column. + +## Joins + +A join is refused with `Exception\Query` while a column restriction applies to the base collection or to any +collection it joins. A join returns the joined columns under an alias, which the query gate and the masking do not +resolve, so allowing it would judge those columns against the wrong collection's grants and return them unshaped. A +role holding a whole-row grant on every collection in the join is unrestricted and joins as in 8.1.4. + # Upgrading from 8.1.3 to 8.1.4 8.1.4 is a patch release and needs no code change. `Database::deleteDocuments()` keeps its signature and now deletes diff --git a/src/Database/Trait/Documents.php b/src/Database/Trait/Documents.php index 08d556b9a..2ca2f3dbe 100644 --- a/src/Database/Trait/Documents.php +++ b/src/Database/Trait/Documents.php @@ -1419,27 +1419,6 @@ private function assertCreateTenancy(string $collection): void } } - /** - * @throws AuthorizationException - * @throws DatabaseException - * @throws StructureException - */ - /** - * A row's permissions may not name a column. - * - * No column list on purpose, and no exemption: a column-scoped permission belongs to a - * collection, and a row naming one would be stored where nothing enforces it. It is also - * what every read gate matches against -- the `_perms` side table, Postgres' JSONB - * containment, Mongo's `$in` -- so a column stored here hides the row from the role it was - * granted to, while getDocument() still returns it. - * - * Every path that writes permissions calls this. It used to live inline in - * prepareDocument(), which only createDocument() reaches, so the rule held on one entry - * point out of six and a batch create, an update or an upsert stored the grant happily. - * - * @throws DatabaseException - * @throws StructureException - */ /** * The columns the caller may reach on this collection, or null when nothing narrows them. * @@ -1715,6 +1694,22 @@ private static function columnsNamedBy(array $queries): array return \array_values(\array_unique($columns)); } + /** + * A row's permissions may not name a column. + * + * No column list on purpose, and no exemption: a column-scoped permission belongs to a + * collection, and a row naming one would be stored where nothing enforces it. It is also + * what every read gate matches against -- the `_perms` side table, Postgres' JSONB + * containment, Mongo's `$in` -- so a column stored here hides the row from the role it was + * granted to, while getDocument() still returns it. + * + * Every path that writes permissions calls this. It used to live inline in + * prepareDocument(), which only createDocument() reaches, so the rule held on one entry + * point out of six and a batch create, an update or an upsert stored the grant happily. + * + * @throws DatabaseException + * @throws StructureException + */ private function assertRowPermissions(Document $document): void { if (! $this->validation()->get()) { diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php index 76e83c0d6..ef83ecc0c 100644 --- a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -17,6 +17,7 @@ use Utopia\Database\Permission; use Utopia\Database\PermissionType; use Utopia\Database\Query; +use Utopia\Database\Relationship; use Utopia\Database\Role; use Utopia\Database\Validator\Authorization; @@ -1160,6 +1161,95 @@ public function testARestrictedRoleMayNotJoin(): void }); } + /** + * A related document is shaped by its own collection's grants. + * + * Population reads the related collection through the same find() and getDocument() the + * caller would, so the mask reaches it and the grants that apply are the related + * collection's own, not the parent's. Pinned because a population that ever read rows + * directly from the adapter would hand back everything and nothing else would notice. + */ + public function testARelatedDocumentIsMaskedByItsOwnCollectionsGrants(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + if (! $database->getAdapter()->hasFeature(Feature\Relationships::class)) { + $this->markTestSkipped('Adapter does not support relationships'); + } + + $authorization = $database->getAuthorization(); + + // The parent names no column, so nothing of its own is withheld. + $database->createCollection(Collection::create( + id: 'columnGrantsParent', + attributes: [Attribute::string(key: 'label', size: 64)], + permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any()), + // asRole() replaces the role list, so exec holds nothing that "any" would cover. + Permission::read(Role::team('exec')), + ], + )); + + $database->withColumnPermissions(fn () => $database->createCollection(Collection::create( + id: 'columnGrantsRelated', + attributes: [ + Attribute::string(key: 'name', size: 64), + Attribute::integer(key: 'salary'), + ], + permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any(), 'name'), + Permission::read(Role::team('exec')), + ], + ))); + + $database->createRelationship( + 'columnGrantsParent', + Relationship::oneToOne(relatedCollection: 'columnGrantsRelated', key: 'related'), + ); + + $authorization->skip(function () use ($database): void { + $database->createDocument('columnGrantsRelated', new Document([ + '$id' => 'r1', + 'name' => 'Ada', + 'salary' => 120000, + ])); + $database->createDocument('columnGrantsParent', new Document([ + '$id' => 'p1', + 'label' => 'row', + 'related' => 'r1', + ])); + }); + + $this->asRole($authorization, Role::any()->toString(), function () use ($database): void { + foreach ([ + 'getDocument' => $database->getDocument('columnGrantsParent', 'p1'), + 'find' => $database->find('columnGrantsParent')[0], + ] as $what => $parent) { + // The parent is untouched: it carries no column grant of its own. + $this->assertSame('row', $parent->getAttribute('label'), $what); + + $related = $parent->getAttribute('related'); + $this->assertInstanceOf(Document::class, $related, $what); + $this->assertSame('Ada', $related->getAttribute('name'), $what); + $this->assertFalse( + $related->offsetExists('salary'), + $what.'() returned an ungranted column of the related collection', + ); + } + }); + + // A whole-row grant on the related collection withholds nothing from it. + $this->asRole($authorization, Role::team('exec')->toString(), function () use ($database): void { + $related = $database->getDocument('columnGrantsParent', 'p1')->getAttribute('related'); + + $this->assertInstanceOf(Document::class, $related); + $this->assertSame(120000, $related->getAttribute('salary')); + }); + } + public function testADocumentPermissionNamingNoColumnIsUnaffected(): void { /** @var Database $database */ From f30991d59ba191628b69628d00e23702bcc0bd05 Mon Sep 17 00:00:00 2001 From: fogelito Date: Thu, 15 Oct 2026 14:29:16 +0300 Subject: [PATCH 22/29] Unit tests + write columns scopes --- CHANGELOG.md | 61 ------------ UPGRADE.md | 76 -------------- src/Database/Trait/Documents.php | 42 ++++++-- src/Database/Validator/Permissions.php | 12 ++- .../Adapter/Scopes/ColumnPermissionTests.php | 98 +++++++++++++++++++ tests/unit/PermissionTest.php | 7 +- tests/unit/Validator/PermissionsTest.php | 35 ++++--- 7 files changed, 161 insertions(+), 170 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b27ab2f20..76f7a6e33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,66 +1,5 @@ # Changelog -## 8.2.0 - -8.2.0 lets a collection's permission name one of its columns, so `read("any", "name")` grants that column alone. It -is off until asked for and changes nothing for a collection that uses none; see -[Upgrading from 8.1.4 to 8.2.0](UPGRADE.md#upgrading-from-814-to-820) for the one schema change it needs. - -Grants only ever add. A grant naming no column reaches every column, as every grant always has; one naming a column -reaches that column alone; and a caller reaches the union over the grants whose role they hold. Nothing is restricted -by default and no grant takes access away -- a column is unreachable until something grants it, and a role holding -one whole-row grant is never narrowed by a column-scoped grant beside it. - -### Added - -- `Permission::read()`, `create()` and `update()` take an optional column: `Permission::read(Role::any(), 'name')` - renders `read("any", "name")`, and `Permission::parse()` reads it back. `delete()` and `write()` reject a column, - because a delete removes the whole row and a write aggregates to create, update and delete. The column must be a - column of the collection; `"*"` and an empty column are refused, since omitting the argument already means every - column. -- `Permission::getColumn(): string` and `Permission::isForAllColumns(): bool` report what one parsed permission - names. `Permission::hasColumns(array $permissions): bool` and `Permission::withoutColumns(array $permissions): - array` answer for a list. `Permission::aggregate()` carries a column through the permissions it expands. -- `Database::withColumnPermissions(callable $callback): mixed` runs `$callback` with column-scoped permissions - allowed, and `Database::hasColumnPermissions(): bool` reports whether they are. `createCollection()` and - `updateCollection()` throw `Exception` with `Column-scoped permissions are not enabled` for a permission naming a - column outside the scope, so a caller opts in once where it means to. The scope is captured by `getSnapshot()` and - restored by `withToggles()`, and `Mirror::withColumnPermissions()` opens it on the mirror, its source and its - destination. -- `Collection::declaredPermissions(): ?array` returns a collection's permissions as they were written, column-scoped - ones included. `getPermissions()` keeps returning what the read gate matches, which is the same grants with every - column dropped. Use `declaredPermissions()` to report or replay a collection's grants and `getPermissions()` for - the gate. -- `_metadata` holds a `columnPermissions` column, a JSON list of the collection's permissions as they were written, - stored only when one of them names a column and cleared when the last one is dropped. `Collection::setPermissions()` - writes it and `$permissions` together, so the two cannot drift. - -### Changed - -- A read answers with the columns the caller was granted. A query naming a column the caller may not read is refused - with `Exception\Authorization` (`Missing "read" permission for column "salary"`) -- filters, ordering, `select()`, - `count()`, `sum()` and the aggregates of `aggregate()` alike, since an aggregate over a column discloses it as - surely as reading it. Asking for everything is not naming anything, so a bare read and `select(['*'])` are answered - rather than refused: the columns the caller was not granted are removed from the document, keys and all, by - `getDocument()`, `find()`, `findOne()`, `cursor()`, every write that returns a document, and the documents a bulk - write reports through `onNext`. A related document is shaped by the grants of its own collection. The key is - removed rather than nulled, because a null would be indistinguishable from a column that genuinely holds null. -- A write touching a column the caller may not change is refused whole, with `Exception\Authorization` - (`Missing "update" permission for column "salary"`). Applying the granted part and dropping the rest would store a - document neither the caller nor the collection asked for. -- A join is refused with `Exception\Query` (`Join queries are not supported while a column-scoped permission - applies`) when a column restriction applies to the base collection or to any collection it joins, nested joins - included. A join returns the joined columns under an alias, which neither the query gate nor the masking resolves, - so it would judge them against the wrong collection's grants and return them unshaped. A role holding a whole-row - grant on every collection in the join is not narrowed and joins as before. -- A row's permissions may not name a column, on every path that writes them: `createDocument()`, - `createDocuments()`, `updateDocument()`, `updateDocuments()`, `upsertDocument()` and `upsertDocuments()`. Column - permissions belong to a collection; stored on a row they would sit where nothing enforces them, and a column in - that list is what every read gate matches against -- the `_perms` side table, PostgreSQL's JSONB containment, - MongoDB's `$in` -- so the row would be hidden from the very role it was granted to. -- Enforcement does not depend on the scope. `withColumnPermissions()` governs only whether a column-scoped grant may - be written; once a collection holds one, every read and write is gated and shaped by it. - ## 8.1.4 8.1.4 fixes `deleteDocuments()` deleting documents that stopped matching its queries after they were found. It needs diff --git a/UPGRADE.md b/UPGRADE.md index 5c1e66d09..79ce574f2 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,79 +1,3 @@ -# Upgrading from 8.1.4 to 8.2.0 - -8.2.0 adds column-scoped collection permissions. It needs no code change and no adapter change, and a database that -uses none behaves as in 8.1.4. It does need one column added to the metadata table; see -[CHANGELOG.md](CHANGELOG.md#820) for the feature itself. - -- [The `columnPermissions` column](#the-columnpermissions-column) -- [Reading a collection's permissions](#reading-a-collections-permissions) -- [Granting a column](#granting-a-column) -- [Joins](#joins) - -## The `columnPermissions` column - -`_metadata` holds a new `columnPermissions` column: a JSON list, defaulting to `[]`. The metadata table is described -in code rather than by a row of its own, so `createAttribute()` cannot add it and an existing database needs the -column created directly, before any code of 8.2.0 reads the table. Create it nullable, or with `[]` as its default, -so the collections already stored keep working unchanged. - -On a host with shared tables the column belongs to the shared table, so add it once per table, not once per tenant. -Adding it while a tenant is set gives its row that tenant and leaves the column missing for every other tenant on -the host. - -Running 8.2.0 before the column exists degrades in one direction only, so an upgrade that cannot add it in the same -step is not left broken: reading is unaffected, and changing the schema fails until it is there. - -| | Without the column | -|---|---| -| `getCollection()`, `getDocument()`, `find()` and the rest of the reads | work, since they select every column rather than naming them | -| `createDocument()` and the other document writes | work | -| `createCollection()` | fails with `Failed to create collection metadata for '': Attribute not found` | -| `updateCollection()` | fails with `Exception\NotFound` (`Attribute not found`) | - -Add the column first and deploy second. Deploying first leaves an application that serves reads and document writes -but cannot create or change a collection until the column is added, at which point it recovers with no restart. - -## Reading a collection's permissions - -`Collection::getPermissions()` keeps its meaning: the grants as the read gate matches them, which is every grant with -its column dropped. That is what the gate must see -- the `_perms` side table, PostgreSQL's JSONB containment and -MongoDB's `$in` all match the stored form, so a column left in it would hide the collection from the role it was -granted to. - -Code that reports a collection's permissions back to a caller, or replays them onto another collection, wants -`Collection::declaredPermissions()` instead, which returns them as they were written. For a collection with no -column-scoped grant the two return the same list, so this only matters once a column is granted. - -## Granting a column - -A column-scoped grant is refused unless the call asks for one: - -```php -$database->withColumnPermissions(fn () => $database->createCollection(Collection::create( - id: 'employees', - attributes: [Attribute::string(key: 'name', size: 64), Attribute::integer(key: 'salary')], - permissions: [ - Permission::read(Role::any(), 'name'), - Permission::read(Role::team('hr')), - ], -))); -``` - -Grants add rather than restrict: `any` reaches `name` alone, `hr` holds a whole-row grant and reaches everything. A -caller reaches the union over the grants whose role they hold, so a whole-row grant is never narrowed by a -column-scoped one beside it. - -Once a collection holds such a grant it is enforced on every read and write, with or without the scope: a query -naming an ungranted column throws `Exception\Authorization`, a write touching one is refused whole, and a read that -names nothing comes back holding only the granted columns. A row's own permissions may never name a column. - -## Joins - -A join is refused with `Exception\Query` while a column restriction applies to the base collection or to any -collection it joins. A join returns the joined columns under an alias, which the query gate and the masking do not -resolve, so allowing it would judge those columns against the wrong collection's grants and return them unshaped. A -role holding a whole-row grant on every collection in the join is unrestricted and joins as in 8.1.4. - # Upgrading from 8.1.3 to 8.1.4 8.1.4 is a patch release and needs no code change. `Database::deleteDocuments()` keeps its signature and now deletes diff --git a/src/Database/Trait/Documents.php b/src/Database/Trait/Documents.php index 2ca2f3dbe..44abb61cf 100644 --- a/src/Database/Trait/Documents.php +++ b/src/Database/Trait/Documents.php @@ -1491,7 +1491,7 @@ private function grantedColumns(Document $collection, PermissionType $type): ?ar * @throws DatabaseException * @throws StructureException */ - private function rejectRestrictedJoins(Document $collection, array $queries): void + private function rejectRestrictedJoins(Document $collection, array $queries, ?array &$joinedCollections): void { $joined = self::joinedBy($queries); @@ -1501,13 +1501,19 @@ private function rejectRestrictedJoins(Document $collection, array $queries): vo $message = 'Join queries are not supported while a column-scoped permission applies'; + // Before resolving anything: a restricted base refuses the join whatever it joins. if ($this->grantedColumns($collection, PermissionType::Read) !== null) { throw new QueryException($message); } + // The read's own map, filled in here when this gate is the first to need it and reused + // by the rest of the read, so a joined collection is still resolved once per read. + $joinedCollections ??= $this->resolveJoinedCollections($queries); + foreach ($joined as $id) { - // A join naming no such collection is validation's to report, not this gate's. - $definition = $this->silent(fn (): ?Collection => $this->findCollection($id)); + // Absent from the map only for a nested join, which it does not walk. A join naming + // no such collection is validation's to report, not this gate's. + $definition = $joinedCollections[$id] ?? $this->silent(fn (): ?Collection => $this->findCollection($id)); if ($definition !== null && $this->grantedColumns($definition, PermissionType::Read) !== null) { throw new QueryException($message); @@ -1627,7 +1633,7 @@ private function assertWritableColumns(Document $collection, Document $document) * @throws DatabaseException * @throws StructureException */ - private function assertQueryColumns(Document $collection, array $queries): void + private function assertQueryColumns(Document $collection, array $queries, ?array &$joinedCollections = null): void { if (! $this->authorization->getStatus()) { return; @@ -1636,7 +1642,7 @@ private function assertQueryColumns(Document $collection, array $queries): void // Before the base collection's own grant is consulted: a join is refused for a // restriction on either side, and an unrestricted base is exactly the case where one // can be used to read a restricted collection. - $this->rejectRestrictedJoins($collection, $queries); + $this->rejectRestrictedJoins($collection, $queries, $joinedCollections); $granted = $this->grantedColumns($collection, PermissionType::Read); @@ -2709,6 +2715,15 @@ private function upsert( $old = $existing[$this->upsertKey($document)] ?? new Document(); $document = $this->removeUnknownAttributes($collection, $document); + + // An upsert onto a stored row is an update, and is gated as updateDocument() is. + // Before keepStored() merges the row in, so the columns judged are the ones the + // caller wrote rather than every column the row happens to hold. An upsert that + // creates carries no stored row and is covered by the create grant instead. + if (! $old->isEmpty()) { + $this->assertWritableColumns($collection, $document); + } + $unchanged = $this->roundOperators($collection, $document); if (! $old->isEmpty()) { self::keepStored($document, $old, $unchanged); @@ -3075,6 +3090,9 @@ public function increaseDocumentAttribute( $this->assertPositiveChange($value); $collection = $this->silent(fn () => $this->getCollection($collection)); + + // The one column the counter changes, judged as any other write of it would be. + $this->assertWritableColumns($collection, new Document([$attribute => null])); $numericAttribute = $this->numericAttribute($collection, $attribute); if ($this->isDeclaredInteger($numericAttribute ?? $this->declaredAttribute($collection, $attribute))) { @@ -3208,6 +3226,9 @@ public function decreaseDocumentAttribute( $collection = $this->silent(fn () => $this->getCollection($collection)); + // The one column the counter changes, judged as any other write of it would be. + $this->assertWritableColumns($collection, new Document([$attribute => null])); + $numericAttribute = $this->numericAttribute($collection, $attribute); if ($this->isDeclaredInteger($numericAttribute ?? $this->declaredAttribute($collection, $attribute))) { @@ -4601,7 +4622,7 @@ private function fetch(Collection $collection, array $queries, PermissionType $f // queries naming the column they aggregate, and an aggregate over a column you cannot // read discloses it as surely as reading it. After checkQueryTypes() above, so a // malformed query is still reported as malformed rather than reaching this. - $this->assertQueryColumns($collection, $queries); + $this->assertQueryColumns($collection, $queries, $joinedCollectionsById); $parsed = Query::groupByType($queries); $filters = $parsed->filters; @@ -4935,7 +4956,6 @@ public function count(string $collection, array $queries = [], ?int $max = null) $queries = $this->rewriteQueries(Event::DocumentCount, $collection, $queries); $this->checkQueryTypes($queries); - $this->assertQueryColumns($collection, $queries); $queries = self::resolveOneToManyJoins($collection, $queries); @@ -4946,6 +4966,8 @@ public function count(string $collection, array $queries = [], ?int $max = null) $this->validateDocumentsQueries($collection, $queries, $joinedCollections); } + $this->assertQueryColumns($collection, $queries, $joinedCollections); + $documentSecurity = $collection->getAttribute('documentSecurity', false); $collectionGranted = $this->authorization->isValid(new Input(PermissionType::Read, $collection->getPermissionsByType(PermissionType::Read))); @@ -4996,9 +5018,6 @@ public function sum(string $collection, string $attribute, array $queries = [], $this->checkQueryTypes($queries); - // The aggregated column is read too, so it is named alongside the queries. - $this->assertQueryColumns($collection, [...$queries, Query::sum($attribute)]); - $queries = self::resolveOneToManyJoins($collection, $queries); $joinedCollections = null; @@ -5011,6 +5030,9 @@ public function sum(string $collection, string $attribute, array $queries = [], $this->validateSumAttribute($collection, $attribute, $queries, $joinedCollections); } + // The aggregated column is read too, so it is named alongside the queries. + $this->assertQueryColumns($collection, [...$queries, Query::sum($attribute)], $joinedCollections); + if (! \str_contains($attribute, '.') && ! $this->declaresSumAttribute($collection, $attribute)) { $joinedCollections ??= $this->resolveJoinedCollections($queries); $attribute = $this->resolveSumAttribute($collection, $attribute, $queries, $joinedCollections); diff --git a/src/Database/Validator/Permissions.php b/src/Database/Validator/Permissions.php index 668d6d40d..1381c51d6 100644 --- a/src/Database/Validator/Permissions.php +++ b/src/Database/Validator/Permissions.php @@ -134,10 +134,14 @@ private function isValidColumn(Permission $permission): bool { $type = $permission->getPermission(); - // Before anything about where the permission lives: delete removes the whole row, so - // narrowing it to one column says nothing, and write aggregates to create, update and - // delete, so it inherits the same objection. - if (\in_array($type, [PermissionType::Delete->value, PermissionType::Write->value], true)) { + // Before anything about where the permission lives. Only read and update answer a + // question about one column. A create brings a whole row into being, so narrowing it to + // one column would have to mean something about the columns it leaves unset, which is + // not a thing a grant can say; a delete removes the whole row, so narrowing it says + // nothing at all; and write aggregates to create, update and delete, so it inherits + // both objections. Refused rather than accepted and ignored: a grant that reads as a + // restriction and enforces none is worse than no grant. + if (\in_array($type, [PermissionType::Create->value, PermissionType::Delete->value, PermissionType::Write->value], true)) { $this->message = 'Permission "'.$type.'" cannot be scoped to a column, it applies to the whole row.'; return false; diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php index ef83ecc0c..ab4bcb18d 100644 --- a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -278,6 +278,35 @@ public function testTheColumnPermissionsToggleSurvivesASnapshot(): void $this->assertFalse($database->hasColumnPermissions()); } + /** + * Only read and update may name a column, and createCollection() enforces it. + * + * A create brings a whole row into being and a delete removes one, so neither is narrowed + * by naming a column, and write aggregates to all three. The rule is the validator's; this + * pins that the collection write actually runs it. + */ + public function testACollectionPermissionMayNotScopeCreateDeleteOrWrite(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + foreach (['create', 'delete', 'write'] as $type) { + try { + $database->withColumnPermissions(fn () => $database->createCollection(Collection::create( + id: 'columnGrantsWholeRow'.$type, + attributes: [Attribute::string(key: 'name', size: 64)], + permissions: [Permission::$type(Role::any(), 'name')], + ))); + $this->fail('Expected createCollection() to reject a column-scoped "'.$type.'"'); + } catch (DatabaseException $e) { + $this->assertStringContainsString( + 'Permission "'.$type.'" cannot be scoped to a column, it applies to the whole row.', + $e->getMessage(), + ); + } + } + } + public function testACollectionPermissionMayNotNameAColumnThatDoesNotExist(): void { /** @var Database $database */ @@ -1250,6 +1279,75 @@ public function testARelatedDocumentIsMaskedByItsOwnCollectionsGrants(): void }); } + /** + * Every write path refuses a column the caller may not change, not just updateDocument(). + * + * An upsert onto a stored row is an update, and a counter changes one column like any other + * write of it, so each is judged the same way. Pinned per path because the gate is a call at + * each of them rather than one choke point: a new write path added without it would be a + * hole nothing else notices. + */ + public function testEveryWritePathRefusesAnUngrantedColumn(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + $authorization = $database->getAuthorization(); + + $database->withColumnPermissions(fn () => $database->createCollection(Collection::create( + id: 'columnGrantsWriteGate', + attributes: [ + Attribute::string(key: 'name', size: 64), + Attribute::integer(key: 'salary'), + ], + permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any()), + Permission::update(Role::any(), 'name'), + ], + ))); + + $authorization->skip(fn () => $database->createDocument('columnGrantsWriteGate', new Document([ + '$id' => 'g1', + 'name' => 'Ada', + 'salary' => 100, + ]))); + + $refused = [ + 'updateDocument' => fn () => $database->updateDocument('columnGrantsWriteGate', 'g1', new Document(['salary' => 1])), + 'updateDocuments' => fn () => $database->updateDocuments('columnGrantsWriteGate', new Document(['salary' => 2])), + 'increaseDocumentAttribute' => fn () => $database->increaseDocumentAttribute('columnGrantsWriteGate', 'g1', 'salary', 7), + 'decreaseDocumentAttribute' => fn () => $database->decreaseDocumentAttribute('columnGrantsWriteGate', 'g1', 'salary', 7), + ]; + + if ($database->getAdapter()->hasFeature(Feature\Upserts::class)) { + $refused['upsertDocument'] = fn () => $database->upsertDocument('columnGrantsWriteGate', new Document(['$id' => 'g1', 'salary' => 5])); + $refused['upsertDocuments'] = fn () => $database->upsertDocuments('columnGrantsWriteGate', [new Document(['$id' => 'g1', 'salary' => 6])]); + } + + $this->asRole($authorization, Role::any()->toString(), function () use ($refused, $database): void { + foreach ($refused as $what => $run) { + try { + $run(); + $this->fail($what.' changed a column the caller was not granted'); + } catch (AuthorizationException $e) { + $this->assertStringContainsString('Missing "update" permission for column "salary"', $e->getMessage(), $what); + } + } + + // The granted column is writable through the same paths. + $this->assertSame( + 'Ada Lovelace', + $database->updateDocument('columnGrantsWriteGate', 'g1', new Document(['name' => 'Ada Lovelace']))->getAttribute('name'), + ); + }); + + // Nothing the refusals touched was stored. + $stored = $authorization->skip(fn () => $database->getDocument('columnGrantsWriteGate', 'g1')); + + $this->assertSame(100, $stored->getAttribute('salary')); + $this->assertSame('Ada Lovelace', $stored->getAttribute('name')); + } + public function testADocumentPermissionNamingNoColumnIsUnaffected(): void { /** @var Database $database */ diff --git a/tests/unit/PermissionTest.php b/tests/unit/PermissionTest.php index b9171da13..33cbc450a 100644 --- a/tests/unit/PermissionTest.php +++ b/tests/unit/PermissionTest.php @@ -427,13 +427,14 @@ public function test_column_must_not_be_empty(): void public function test_column_builders(): void { $this->assertEquals('read("any", "name")', Permission::read(Role::any(), 'name')); - $this->assertEquals('create("any", "name")', Permission::create(Role::any(), 'name')); $this->assertEquals('update("any", "name")', Permission::update(Role::any(), 'name')); $this->assertEquals('read("any")', Permission::read(Role::any())); - // delete() and write() take the argument too -- a builder renders what it is handed -- - // but the validator refuses both, because a column cannot narrow a whole-row delete. + // create(), delete() and write() take the argument too -- a builder renders what it is + // handed -- but the validator refuses all three, because a column narrows neither a row + // brought into being nor one removed. + $this->assertEquals('create("any", "name")', Permission::create(Role::any(), 'name')); $this->assertEquals('delete("any", "name")', Permission::delete(Role::any(), 'name')); $this->assertEquals('write("any", "name")', Permission::write(Role::any(), 'name')); } diff --git a/tests/unit/Validator/PermissionsTest.php b/tests/unit/Validator/PermissionsTest.php index 230beb424..308d99c66 100644 --- a/tests/unit/Validator/PermissionsTest.php +++ b/tests/unit/Validator/PermissionsTest.php @@ -407,31 +407,34 @@ public function test_column_scope_against_a_collection_without_columns(): void } /** - * Delete removes the whole row, so narrowing it to one column says nothing. Write - * aggregates to create, update and delete, so it inherits the objection. + * Only read and update answer a question about one column. + * + * A create brings a whole row into being and a delete removes one, so neither is narrowed + * by naming a column, and write aggregates to all three. Refused rather than accepted and + * ignored, so a grant never reads as a restriction it does not enforce. */ - public function test_column_scope_rejected_for_delete_and_write(): void + public function test_column_scope_rejected_for_create_delete_and_write(): void { $object = new Permissions(columns: ['name']); - $this->assertFalse($object->isValid([Permission::delete(Role::any(), 'name')])); - $this->assertSame( - 'Permission "delete" cannot be scoped to a column, it applies to the whole row.', - $object->getDescription(), - ); - - $this->assertFalse($object->isValid([Permission::write(Role::any(), 'name')])); - $this->assertSame( - 'Permission "write" cannot be scoped to a column, it applies to the whole row.', - $object->getDescription(), - ); + foreach (['create', 'delete', 'write'] as $type) { + $this->assertFalse($object->isValid([Permission::$type(Role::any(), 'name')]), $type); + $this->assertSame( + 'Permission "'.$type.'" cannot be scoped to a column, it applies to the whole row.', + $object->getDescription(), + ); + } - // The three that can be narrowed still are. + // The two that can be narrowed still are, and all three still grant the whole row. $this->assertTrue($object->isValid([ Permission::read(Role::any(), 'name'), - Permission::create(Role::any(), 'name'), Permission::update(Role::any(), 'name'), ]), $object->getDescription()); + + $this->assertTrue($object->isValid([ + Permission::create(Role::any()), + Permission::delete(Role::any()), + ]), $object->getDescription()); } /** From 87337bf5c7d6812b34b8e83398df374d3b58d1a3 Mon Sep 17 00:00:00 2001 From: fogelito Date: Thu, 15 Oct 2026 14:53:41 +0300 Subject: [PATCH 23/29] Unit tests + lint --- src/Database/Trait/Documents.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Database/Trait/Documents.php b/src/Database/Trait/Documents.php index 44abb61cf..8daea32c0 100644 --- a/src/Database/Trait/Documents.php +++ b/src/Database/Trait/Documents.php @@ -1485,7 +1485,8 @@ private function grantedColumns(Document $collection, PermissionType $type): ?ar * this role. A role holding a whole-row grant on both sides is unrestricted and joins as it * always did. * - * @param array $queries + * @param array $queries + * @param array|null $joinedCollections The collection each join names, by its id * * @throws QueryException * @throws DatabaseException @@ -1499,6 +1500,13 @@ private function rejectRestrictedJoins(Document $collection, array $queries, ?ar return; } + // An adapter that cannot join at all refuses the join further down, for a better + // reason than this one. Stepping aside leaves that error to it, and a join that can + // never run leaks nothing in the meantime. + if (! $this->adapter->supports(Capability::Joins)) { + return; + } + $message = 'Join queries are not supported while a column-scoped permission applies'; // Before resolving anything: a restricted base refuses the join whatever it joins. @@ -1628,6 +1636,7 @@ private function assertWritableColumns(Document $collection, Document $document) * names a column is settled here, before the adapter sees it. * * @param array $queries + * @param array|null $joinedCollections The collection each join names, by its id * * @throws AuthorizationException * @throws DatabaseException From 398d235cd0bc73cd364851ba53f34c02e88f702e Mon Sep 17 00:00:00 2001 From: fogelito Date: Sun, 18 Oct 2026 09:49:07 +0300 Subject: [PATCH 24/29] Rename column-scoped updater --- src/Database/Permission.php | 63 +++++++ src/Database/Trait/Attributes.php | 9 + src/Database/Trait/Documents.php | 8 + .../Adapter/Scopes/ColumnPermissionTests.php | 163 ++++++++++++++++++ 4 files changed, 243 insertions(+) diff --git a/src/Database/Permission.php b/src/Database/Permission.php index 7ea539eff..237d4a741 100644 --- a/src/Database/Permission.php +++ b/src/Database/Permission.php @@ -293,6 +293,69 @@ public static function hasColumns(array $permissions): bool return false; } + /** + * The same permissions with every grant on $old pointed at $new. + * + * A grant names a column by its key, so renaming the column would otherwise leave the grant + * naming one that no longer exists: it would reach nothing, and the collection would hold a + * grant createCollection() refuses. Following the rename keeps the grant on the column the + * caller granted, under whatever it is now called. + * + * @param list $permissions + * @return list + * + * @throws DatabaseException + */ + public static function renameColumn(array $permissions, string $old, string $new): array + { + $renamed = []; + + foreach ($permissions as $permission) { + $parsed = self::parse($permission); + + $renamed[] = $parsed->isForAllColumns() || $parsed->getColumn() !== $old + ? $permission + : (new self( + $parsed->getPermission(), + $parsed->getRole(), + $parsed->getIdentifier(), + $parsed->getDimension(), + $new + ))->toString(); + } + + return \array_values(\array_unique($renamed)); + } + + /** + * The same permissions without the grants naming $column. + * + * Dropping the column leaves nothing for a grant on it to reach, and a grant left behind + * would name a column the collection no longer has. Grants naming no column are untouched: + * they cover the whole row, which still exists. + * + * @param list $permissions + * @return list + * + * @throws DatabaseException + */ + public static function withoutColumn(array $permissions, string $column): array + { + $kept = []; + + foreach ($permissions as $permission) { + $parsed = self::parse($permission); + + if (! $parsed->isForAllColumns() && $parsed->getColumn() === $column) { + continue; + } + + $kept[] = $permission; + } + + return \array_values(\array_unique($kept)); + } + /** * The same permissions with every column dropped, deduplicated. * diff --git a/src/Database/Trait/Attributes.php b/src/Database/Trait/Attributes.php index 3b27fd3ca..7c032a6cf 100644 --- a/src/Database/Trait/Attributes.php +++ b/src/Database/Trait/Attributes.php @@ -23,6 +23,7 @@ use Utopia\Database\Exception\Refused as RefusedException; use Utopia\Database\Exception\Structure as StructureException; use Utopia\Database\Index; +use Utopia\Database\Permission; use Utopia\Database\Schema; use Utopia\Database\SetType; use Utopia\Database\Validator\AttributeDefinition; @@ -376,6 +377,10 @@ public function deleteAttribute(string $collection, string $key): void $this->writeAttributes($definition, \array_values($attributes)); $this->writeIndexes($definition, self::withoutIndexedAttribute($indexes, $key)); + // Nothing is left for a grant on this column to reach, and one left behind would name + // a column the collection no longer has -- which createCollection() refuses outright. + $definition->setPermissions(Permission::withoutColumn($definition->declaredPermissions() ?? [], $key)); + $deletedInSchema = $this->deleteAttributeFromSchema($definition->getId(), $key); $this->updateMetadata( @@ -449,6 +454,10 @@ public function renameAttribute(string $collection, string $old, string $new): v $this->writeAttributes($definition, $attributes); $this->writeIndexes($definition, self::renameIndexedAttribute($indexes, $old, $new)); + // A grant names its column by key, so it follows the rename with it. setPermissions() + // writes the gate and the verbatim list together, so the two cannot drift. + $definition->setPermissions(Permission::renameColumn($definition->declaredPermissions() ?? [], $old, $new)); + $this->updateMetadata( collection: $definition, rollbackOperation: fn () => $this->adapter->renameAttribute($definition->getId(), $new, $old), diff --git a/src/Database/Trait/Documents.php b/src/Database/Trait/Documents.php index b0168270d..1b2d0b528 100644 --- a/src/Database/Trait/Documents.php +++ b/src/Database/Trait/Documents.php @@ -1692,6 +1692,14 @@ private function assertWritableColumns(Document $collection, Document $document) return; } + // The row's own permissions are not a column, and rewriting them is a whole-row act: + // a caller who may change one column could otherwise grant itself delete and destroy + // the row, turning a grant over one column into power over all of it. The other system + // attributes are not writable this way, so skipping them stays correct. + if ($document->offsetExists(Document::PERMISSIONS)) { + throw new AuthorizationException('Missing "update" permission for the row\'s permissions, which a column-scoped grant does not cover.'); + } + foreach (\array_keys($document->getArrayCopy()) as $key) { if (\str_starts_with($key, '$')) { continue; diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php index ab4bcb18d..d5471c1ff 100644 --- a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -1348,6 +1348,169 @@ public function testEveryWritePathRefusesAnUngrantedColumn(): void $this->assertSame('Ada Lovelace', $stored->getAttribute('name')); } + /** + * A column-scoped updater may not rewrite the row's permissions. + * + * The row's permissions are not a column, and rewriting them is a whole-row act. Left + * open, a caller granted one column could grant itself delete through them and destroy the + * row, turning a grant over one column into power over all of it. The mask would still + * hold -- it reads the collection's grants, not the row's -- so the escalation is over the + * row rather than its columns, which makes it no less an escalation. + */ + public function testAColumnScopedUpdaterMayNotRewriteTheRowsPermissions(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + $authorization = $database->getAuthorization(); + + $database->withColumnPermissions(fn () => $database->createCollection(Collection::create( + id: 'columnGrantsEscalation', + attributes: [ + Attribute::string(key: 'name', size: 64), + Attribute::integer(key: 'salary'), + ], + permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any(), 'name'), + Permission::update(Role::any(), 'name'), + Permission::read(Role::team('exec')), + Permission::update(Role::team('exec')), + ], + documentSecurity: true, + ))); + + $stored = [ + Permission::read(Role::any()), + Permission::update(Role::any()), + ]; + + $authorization->skip(fn () => $database->createDocument('columnGrantsEscalation', new Document([ + '$id' => 'row', + 'name' => 'Ada', + 'salary' => 120000, + '$permissions' => $stored, + ]))); + + $this->asRole($authorization, Role::any()->toString(), function () use ($database): void { + // Granting itself delete is the escalation the refusal exists for. + try { + $database->updateDocument('columnGrantsEscalation', 'row', new Document([ + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + ])); + $this->fail('a column-scoped updater rewrote the row\'s permissions'); + } catch (AuthorizationException $e) { + $this->assertStringContainsString('Missing "update" permission for the row', $e->getMessage()); + } + + // Refused even alongside a column it is allowed to change, since the write is + // refused whole rather than applied in part. + try { + $database->updateDocument('columnGrantsEscalation', 'row', new Document([ + 'name' => 'Ada Lovelace', + '$permissions' => [Permission::read(Role::any())], + ])); + $this->fail('a column-scoped updater rewrote permissions beside a granted column'); + } catch (AuthorizationException $e) { + $this->assertStringContainsString('Missing "update" permission for the row', $e->getMessage()); + } + + // The granted column alone still updates. + $this->assertSame( + 'Ada L', + $database->updateDocument('columnGrantsEscalation', 'row', new Document(['name' => 'Ada L']))->getAttribute('name'), + ); + }); + + // Nothing was escalated: the row keeps the permissions it was created with. + $after = $authorization->skip(fn () => $database->getDocument('columnGrantsEscalation', 'row')); + + $this->assertSame($stored, $after->getPermissions()); + + // A whole-row grant is not column-scoped, so it rewrites permissions as it always did. + $this->asRole($authorization, Role::team('exec')->toString(), function () use ($database): void { + $updated = $database->updateDocument('columnGrantsEscalation', 'row', new Document([ + '$permissions' => [Permission::read(Role::any()), Permission::update(Role::team('exec'))], + ])); + + $this->assertContains('update("team:exec")', $updated->getPermissions()); + }); + } + + /** + * A grant follows the column it names, and goes when the column goes. + * + * A grant names its column by key, so a rename would otherwise leave it pointing at a key + * nothing has and a delete would leave it naming a column the collection no longer holds. + * Either way the collection ends up with a grant createCollection() refuses outright, and + * the role silently loses the access it was given. Both are kept consistent instead. + */ + public function testAColumnGrantFollowsARenameAndGoesWithADelete(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + $authorization = $database->getAuthorization(); + + $database->withColumnPermissions(fn () => $database->createCollection(Collection::create( + id: 'columnGrantsRenamed', + attributes: [ + Attribute::string(key: 'name', size: 64), + Attribute::integer(key: 'salary'), + ], + permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any(), 'name'), + Permission::update(Role::any(), 'name'), + Permission::read(Role::team('hr'), 'salary'), + ], + ))); + + $authorization->skip(fn () => $database->createDocument('columnGrantsRenamed', new Document([ + '$id' => 'r1', + 'name' => 'Ada', + 'salary' => 120000, + ]))); + + $database->renameAttribute('columnGrantsRenamed', 'name', 'fullName'); + + $declared = $database->getCollection('columnGrantsRenamed')->declaredPermissions() ?? []; + + $this->assertContains('read("any", "fullName")', $declared, 'the read grant did not follow the rename'); + $this->assertContains('update("any", "fullName")', $declared, 'the update grant did not follow the rename'); + $this->assertNotContains('read("any", "name")', $declared); + + // A grant on another column is untouched, and so is one naming no column. + $this->assertContains('read("team:hr", "salary")', $declared); + $this->assertContains('create("any")', $declared); + + // The grant still reaches the column it was given, under its new name. + $this->asRole($authorization, Role::any()->toString(), function () use ($database): void { + $stored = $database->getDocument('columnGrantsRenamed', 'r1'); + + $this->assertSame('Ada', $stored->getAttribute('fullName')); + $this->assertFalse($stored->offsetExists('salary')); + + // And it is writable, which a grant naming a missing column could not be. + $this->assertSame( + 'Ada L', + $database->updateDocument('columnGrantsRenamed', 'r1', new Document(['fullName' => 'Ada L']))->getAttribute('fullName'), + ); + }); + + // Deleting the column takes its grants with it, leaving the rest alone. + $database->deleteAttribute('columnGrantsRenamed', 'fullName'); + + $declared = $database->getCollection('columnGrantsRenamed')->declaredPermissions() ?? []; + + $this->assertNotContains('read("any", "fullName")', $declared, 'a grant outlived the column it named'); + $this->assertNotContains('update("any", "fullName")', $declared, 'a grant outlived the column it named'); + $this->assertContains('read("team:hr", "salary")', $declared); + $this->assertContains('create("any")', $declared); + } + public function testADocumentPermissionNamingNoColumnIsUnaffected(): void { /** @var Database $database */ From e2048724bbff657142337c890137602a1c72312e Mon Sep 17 00:00:00 2001 From: fogelito Date: Sun, 18 Oct 2026 11:30:14 +0300 Subject: [PATCH 25/29] StructureException for permissions validation errors --- src/Database/Trait/Collections.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Database/Trait/Collections.php b/src/Database/Trait/Collections.php index eecb31290..b72cb62c9 100644 --- a/src/Database/Trait/Collections.php +++ b/src/Database/Trait/Collections.php @@ -64,7 +64,7 @@ public function createCollection(Collection $collection): Collection if ($this->validation()->get()) { $validator = new Permissions(columns: \array_map(fn (Attribute $attribute) => $attribute->key, $attributes)); if (! $validator->isValid($permissions)) { - throw new DatabaseException($validator->getDescription()); + throw new StructureException($validator->getDescription()); } } @@ -179,7 +179,7 @@ public function updateCollection(string $collection, Update $update): Collection if ($update->permissions !== null && $this->validation()->get()) { $validator = new Permissions(columns: \array_map(fn (Attribute $attribute) => $attribute->key, $definition->attributes())); if (! $validator->isValid($update->permissions)) { - throw new DatabaseException($validator->getDescription()); + throw new StructureException($validator->getDescription()); } } From 83aa68050a66708e4debb01bbbed82e1be162318 Mon Sep 17 00:00:00 2001 From: fogelito Date: Sun, 18 Oct 2026 14:48:19 +0300 Subject: [PATCH 26/29] Relationships delete attribute --- src/Database/Trait/Relationships.php | 11 ++++ .../Adapter/Scopes/ColumnPermissionTests.php | 61 +++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/src/Database/Trait/Relationships.php b/src/Database/Trait/Relationships.php index 20296afaa..9f4134a70 100644 --- a/src/Database/Trait/Relationships.php +++ b/src/Database/Trait/Relationships.php @@ -20,6 +20,7 @@ use Utopia\Database\Exception\Refused as RefusedException; use Utopia\Database\Exception\Structure as StructureException; use Utopia\Database\Index; +use Utopia\Database\Permission; use Utopia\Database\Relationship; use Utopia\Database\Relationship\Update as RelationshipUpdate; use Utopia\Database\RelationshipSide; @@ -493,6 +494,16 @@ public function deleteRelationship(string $collection, string $key): void $collection->setAttribute(self::COLLECTION_ATTRIBUTES, $collectionAttributes); $relatedCollection->setAttribute(self::COLLECTION_ATTRIBUTES, $relatedCollectionAttributes); + // A relationship is an attribute, so a permission may name it as its column. Both + // sides lose their key here, so both lose the grants that named it -- a grant left + // behind would name a column the collection no longer has, which createCollection() + // refuses outright. The same rule deleteAttribute() follows for an ordinary column. + $collection->setPermissions(Permission::withoutColumn($collection->declaredPermissions() ?? [], $key)); + + if ($twoWayKey !== '') { + $relatedCollection->setPermissions(Permission::withoutColumn($relatedCollection->declaredPermissions() ?? [], $twoWayKey)); + } + try { $shouldRollback = $this->deleteRelationshipFromSchema($collection->getId(), $relationship, $side); } catch (Throwable $error) { diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php index d5471c1ff..64392e324 100644 --- a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -1511,6 +1511,67 @@ public function testAColumnGrantFollowsARenameAndGoesWithADelete(): void $this->assertContains('create("any")', $declared); } + /** + * A relationship is an attribute, so a grant may name it -- and goes with it. + * + * Both sides lose their key when the relationship is deleted, so both lose the grants that + * named it. Left behind, such a grant would name a column the collection no longer has, + * which createCollection() refuses outright, and it would reach nothing in the meantime. + */ + public function testAGrantOnARelationshipGoesWithIt(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + if (! $database->getAdapter()->hasFeature(Feature\Relationships::class)) { + $this->markTestSkipped('Adapter does not support relationships'); + } + + foreach (['columnGrantsRelLeft', 'columnGrantsRelRight'] as $id) { + $database->createCollection(Collection::create( + id: $id, + attributes: [Attribute::string(key: 'name', size: 64)], + permissions: [Permission::create(Role::any()), Permission::read(Role::any())], + )); + } + + $database->createRelationship('columnGrantsRelLeft', Relationship::oneToOne( + relatedCollection: 'columnGrantsRelRight', + key: 'link', + twoWay: true, + twoWayKey: 'backlink', + )); + + $database->withColumnPermissions(function () use ($database): void { + $database->updateCollection('columnGrantsRelLeft', new CollectionUpdate(permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any(), 'link'), + Permission::read(Role::any(), 'name'), + ])); + $database->updateCollection('columnGrantsRelRight', new CollectionUpdate(permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any(), 'backlink'), + Permission::read(Role::any(), 'name'), + ])); + }); + + $this->assertContains('read("any", "link")', $database->getCollection('columnGrantsRelLeft')->declaredPermissions() ?? []); + $this->assertContains('read("any", "backlink")', $database->getCollection('columnGrantsRelRight')->declaredPermissions() ?? []); + + $database->deleteRelationship('columnGrantsRelLeft', 'link'); + + $left = $database->getCollection('columnGrantsRelLeft')->declaredPermissions() ?? []; + $right = $database->getCollection('columnGrantsRelRight')->declaredPermissions() ?? []; + + $this->assertNotContains('read("any", "link")', $left, 'a grant outlived the relationship it named'); + $this->assertNotContains('read("any", "backlink")', $right, 'a grant outlived the two-way key it named'); + + // Grants on the columns that remain are untouched, on both sides. + $this->assertContains('read("any", "name")', $left); + $this->assertContains('read("any", "name")', $right); + $this->assertContains('create("any")', $left); + } + public function testADocumentPermissionNamingNoColumnIsUnaffected(): void { /** @var Database $database */ From d692ca7b514b7f4edee579fd30fdb167a2ac2db8 Mon Sep 17 00:00:00 2001 From: fogelito Date: Sun, 18 Oct 2026 15:04:35 +0300 Subject: [PATCH 27/29] replaceAttribute setPermissions --- src/Database/Trait/Attributes.php | 5 +++++ src/Database/Trait/Relationships.php | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/src/Database/Trait/Attributes.php b/src/Database/Trait/Attributes.php index 7c032a6cf..f82a9cc58 100644 --- a/src/Database/Trait/Attributes.php +++ b/src/Database/Trait/Attributes.php @@ -234,6 +234,11 @@ public function updateAttribute(string $collection, string $key, Update $update) $this->writeAttributes($definition, $attributes); if ($renaming) { $this->writeIndexes($definition, $indexes); + + // A grant names its column by key, so it follows the rename as the indexes above + // do. Left alone it would name a key nothing has: it would reach nothing, and the + // collection would hold a grant createCollection() refuses. + $definition->setPermissions(Permission::renameColumn($definition->declaredPermissions() ?? [], $key, $newKey)); } if ( diff --git a/src/Database/Trait/Relationships.php b/src/Database/Trait/Relationships.php index 9f4134a70..5c6c0efde 100644 --- a/src/Database/Trait/Relationships.php +++ b/src/Database/Trait/Relationships.php @@ -788,6 +788,14 @@ private function replaceAttribute(string $collection, string $key, Attribute $at } $definition->setAttribute(self::COLLECTION_ATTRIBUTES, \array_map(static fn (Attribute $stored): Document => $stored->toDocument(), $attributes)); + + // The one place a relationship's key changes on either side, so the one place a grant + // naming it has to follow. Left alone it would name a key nothing has -- unreachable, + // and a grant createCollection() refuses. deleteRelationship() drops them the same way + // when the key goes entirely. + if ($attribute->key !== $key) { + $definition->setPermissions(Permission::renameColumn($definition->declaredPermissions() ?? [], $key, $attribute->key)); + } $this->updateMetadata( collection: $definition, rollbackOperation: null, From 0b1709dd77612462f4b406b136f3a15719c95497 Mon Sep 17 00:00:00 2001 From: fogelito Date: Sun, 18 Oct 2026 15:08:57 +0300 Subject: [PATCH 28/29] Relationships tests --- .../Adapter/Scopes/ColumnPermissionTests.php | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php index 64392e324..36d0a6042 100644 --- a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -4,6 +4,7 @@ use Utopia\Database\Adapter\Feature; use Utopia\Database\Attribute; +use Utopia\Database\Attribute\Update as AttributeUpdate; use Utopia\Database\Capability; use Utopia\Database\Collection; use Utopia\Database\Collection\Update as CollectionUpdate; @@ -18,6 +19,7 @@ use Utopia\Database\PermissionType; use Utopia\Database\Query; use Utopia\Database\Relationship; +use Utopia\Database\Relationship\Update as RelationshipUpdate; use Utopia\Database\Role; use Utopia\Database\Validator\Authorization; @@ -1572,6 +1574,100 @@ public function testAGrantOnARelationshipGoesWithIt(): void $this->assertContains('create("any")', $left); } + /** + * A grant follows a rename made through updateAttribute(), which is the rename a consumer + * reaches: renameAttribute() is the dedicated call, but an attribute update carrying a new + * key renames too, and that is the one an API exposes. + * + * Indexes already follow such a rename. A grant that did not would name a key nothing has: + * unreachable, and a grant createCollection() refuses. + */ + public function testAColumnGrantFollowsAnUpdateThatRenamesIt(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + $database->withColumnPermissions(fn () => $database->createCollection(Collection::create( + id: 'columnGrantsUpdateRenamed', + attributes: [ + Attribute::string(key: 'name', size: 64), + Attribute::integer(key: 'salary'), + ], + permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any(), 'name'), + Permission::update(Role::any(), 'name'), + Permission::read(Role::team('hr'), 'salary'), + ], + ))); + + $database->updateAttribute('columnGrantsUpdateRenamed', 'name', new AttributeUpdate(key: 'fullName')); + + $declared = $database->getCollection('columnGrantsUpdateRenamed')->declaredPermissions() ?? []; + + $this->assertContains('read("any", "fullName")', $declared, 'the read grant did not follow the rename'); + $this->assertContains('update("any", "fullName")', $declared, 'the update grant did not follow the rename'); + $this->assertNotContains('read("any", "name")', $declared); + + // Grants on other columns, and grants naming none, are untouched. + $this->assertContains('read("team:hr", "salary")', $declared); + $this->assertContains('create("any")', $declared); + } + + /** + * And a grant on a relationship follows a rename of its key, on either side. + */ + public function testAGrantOnARelationshipFollowsARename(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + if (! $database->getAdapter()->hasFeature(Feature\Relationships::class)) { + $this->markTestSkipped('Adapter does not support relationships'); + } + + foreach (['columnGrantsRenRelLeft', 'columnGrantsRenRelRight'] as $id) { + $database->createCollection(Collection::create( + id: $id, + attributes: [Attribute::string(key: 'name', size: 64)], + permissions: [Permission::create(Role::any()), Permission::read(Role::any())], + )); + } + + $database->createRelationship('columnGrantsRenRelLeft', Relationship::oneToOne( + relatedCollection: 'columnGrantsRenRelRight', + key: 'link', + twoWay: true, + twoWayKey: 'backlink', + )); + + $database->withColumnPermissions(function () use ($database): void { + $database->updateCollection('columnGrantsRenRelLeft', new CollectionUpdate(permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any(), 'link'), + ])); + $database->updateCollection('columnGrantsRenRelRight', new CollectionUpdate(permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any(), 'backlink'), + ])); + }); + + $database->updateRelationship( + collection: 'columnGrantsRenRelLeft', + key: 'link', + update: new RelationshipUpdate(key: 'connection', twoWayKey: 'backConnection'), + ); + + $left = $database->getCollection('columnGrantsRenRelLeft')->declaredPermissions() ?? []; + $right = $database->getCollection('columnGrantsRenRelRight')->declaredPermissions() ?? []; + + $this->assertContains('read("any", "connection")', $left, 'the grant did not follow the relationship rename'); + $this->assertNotContains('read("any", "link")', $left); + + $this->assertContains('read("any", "backConnection")', $right, 'the grant did not follow the two-way key rename'); + $this->assertNotContains('read("any", "backlink")', $right); + } + public function testADocumentPermissionNamingNoColumnIsUnaffected(): void { /** @var Database $database */ From af607ce55bef45271f227bdc4bc6c945500b03b9 Mon Sep 17 00:00:00 2001 From: fogelito Date: Sun, 18 Oct 2026 15:32:48 +0300 Subject: [PATCH 29/29] Fix Mongo test --- tests/e2e/Adapter/MongoDBTest.php | 25 ++++++++++++++++--- .../Adapter/Scopes/ColumnPermissionTests.php | 20 ++++++++++++--- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/tests/e2e/Adapter/MongoDBTest.php b/tests/e2e/Adapter/MongoDBTest.php index 15886f82d..f47eb13a6 100644 --- a/tests/e2e/Adapter/MongoDBTest.php +++ b/tests/e2e/Adapter/MongoDBTest.php @@ -217,10 +217,27 @@ public function testListCollectionsReturnsOnlyReadableDefinitions(): void $database->createCollection(Collection::create(id: 'adminDefinition', permissions: [Permission::read(Role::user('admin'))])); $database->createCollection(Collection::create(id: 'unlistedDefinition', permissions: [Permission::create(Role::any())])); - $listed = fn (): array => \array_values(\array_intersect( - $definitions, - \array_map(fn (Collection $collection) => $collection->getId(), $database->listCollections(100)), - )); + // Paged rather than a single page: the listing has no order, so it answers with the + // oldest collections first, and these three are created last in a database every + // other test has already added to. A first page would stop finding them the moment + // the suite grows past it, which says nothing about who may read them. + $listed = function () use ($database, $definitions): array { + $ids = []; + + for ($offset = 0; ; $offset += 100) { + $page = $database->listCollections(100, $offset); + + foreach ($page as $collection) { + $ids[] = $collection->getId(); + } + + if (\count($page) < 100) { + break; + } + } + + return \array_values(\array_intersect($definitions, $ids)); + }; $queries = [Query::equal('$id', $definitions)]; $this->actAs('bob', function () use ($database, $listed, $queries): void { diff --git a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php index 36d0a6042..513a33565 100644 --- a/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php +++ b/tests/e2e/Adapter/Scopes/ColumnPermissionTests.php @@ -86,10 +86,22 @@ public function testACollectionWithAColumnScopedGrantIsStillListed(): void $authorization->addRole(Role::any()->toString()); try { - $ids = \array_map( - static fn (Collection $collection): string => $collection->getId(), - $database->listCollections(100), - ); + // Paged: the listing has no order, so it answers with the oldest collections + // first and this one is created last. A single page would stop finding it once + // the suite grows past it, which says nothing about the grant under test. + $ids = []; + + for ($offset = 0; ; $offset += 100) { + $page = $database->listCollections(100, $offset); + + foreach ($page as $collection) { + $ids[] = $collection->getId(); + } + + if (\count($page) < 100) { + break; + } + } } finally { $authorization->cleanRoles(); foreach ($previous as $role) {