From 92f71d97a4242d6bf46c9214712c9f9d1ce99253 Mon Sep 17 00:00:00 2001 From: harsh mahajan Date: Wed, 21 Oct 2026 17:00:20 +0530 Subject: [PATCH] fix(relationships): link by the key on the written document with read permission 8.0.1 made every nested link to an existing related document need update permission on it. Where the link is only the key on the document being written, a one-way one-to-one, a many-to-one, or a one-to-many child pointing at its parent, nothing on the related document is written, yet linking to a document the caller may only read now fails with "No permissions provided for action 'update'". 7.x linked these with read permission, so read-only lookup collections stopped accepting links after the upgrade. These links need read permission again, on create, update, and in authorizeCreate() and authorizeUpdate(). Links that write the related document or a junction row (many-to-many, two-way one-to-one, one-to-many parent, many-to-one child) still need update permission, a related document sent with attributes to change still needs update permission, and a stored document the caller cannot read is still refused. --- UPGRADE.md | 18 ++++---- src/Database/Hook/Relationships.php | 43 +++++++++---------- tests/e2e/Adapter/Scopes/PermissionTests.php | 4 ++ .../NestedLinkPermissionsTest.php | 11 +++-- .../StagedViewAuthorizationTest.php | 5 ++- 5 files changed, 46 insertions(+), 35 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 79ce574f2..db915035e 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1218,14 +1218,16 @@ method as a no-op, so a hook overrides only what it needs: `deleteDocument()`, so a related document the caller may not read is deleted with the rest or, under `Restrict`, blocks the delete. - Linking an existing related document, at any nesting depth of a create or an update, needs update permission on - that document, for every relationship type; without it the write throws `Utopia\Database\Exception\Authorization` - and nothing is written. In 7.x a many-to-many link needed only read permission on the related document, because - the link is a row in the junction collection rather than a column on the related document, and so did a link by - id from the side that holds the key: a one-way one-to-one, a many-to-one, and a one-to-many child pointing at its - parent. These now need update permission too, whether the value is an id or a document (since 8.0.1; 8.0.0 still - linked them with read permission). A related document the same write creates needs no update permission. - Relinking a document that is already linked needs only read permission, and removing a link needs no update - permission on the unlinked document. + that document when the link writes it or a junction row for it: a many-to-many, a two-way one-to-one, and a + one-to-many parent or many-to-one child listing it. Without it the write throws + `Utopia\Database\Exception\Authorization` and nothing is written. In 7.x a many-to-many link needed only read + permission on the related document, because the link is a row in the junction collection rather than a column on + the related document. A link from the side that holds the key, a one-way one-to-one, a many-to-one, or a + one-to-many child pointing at its parent, writes nothing on the related document and needs only read permission + on it, as in 7.x; a related document sent as a document with attributes to change still needs update permission. + 8.0.1 to 8.3.11 required update permission for these links too. A related document the same write creates needs + no update permission. Relinking a document that is already linked needs only read permission, and removing a link + needs no update permission on the unlinked document. - A stored related document the caller cannot read is refused with `Utopia\Database\Exception\Authorization`, with the read check's reason when the value is its id, and with the update check's reason (or the read check's when the caller may update it) when the value is a document. 7.x left a one-to-one or many-to-one link to it, dropped it diff --git a/src/Database/Hook/Relationships.php b/src/Database/Hook/Relationships.php index e037b0de0..68f19ffb2 100644 --- a/src/Database/Hook/Relationships.php +++ b/src/Database/Hook/Relationships.php @@ -1151,8 +1151,6 @@ public function afterDocumentUpdate(Document $collection, Document $old, Documen if ($related->isEmpty()) { $this->refuseUnreadable($relatedCollection, $value, PermissionType::Read); $document->setAttribute($key, null); - } elseif ($value !== $oldId) { - $this->authorizeLink($relatedCollection, $related); } } elseif ($value instanceof Document) { $relationId = $this->relateDocuments( @@ -1376,8 +1374,6 @@ public function afterDocumentUpdate(Document $collection, Document $old, Documen if ($related->isEmpty()) { $this->refuseUnreadable($relatedCollection, $value, PermissionType::Read); $document->setAttribute($key, null); - } elseif ($value !== $oldId) { - $this->authorizeLink($relatedCollection, $related); } $this->database->purgeCachedDocument($relatedCollection->getId(), $value); } elseif ($value instanceof Document) { @@ -1398,8 +1394,6 @@ public function afterDocumentUpdate(Document $collection, Document $old, Documen $relatedCollection->getId(), $value ); - } elseif ($value->getId() !== $oldId) { - $this->authorizeLink($relatedCollection, $related); } if (! $related->isEmpty() && $related->getAttributes() != $value->getAttributes()) { @@ -2245,7 +2239,7 @@ private function relateDocuments( if ($related->isEmpty()) { $this->refuseUnreadable($relatedCollection, $relation->getId(), PermissionType::Update); - } elseif (! $linked) { + } elseif (! $linked && $this->writesRelated($relationType, $twoWay, $side)) { $this->authorizeLink($relatedCollection, $related); } @@ -2337,7 +2331,7 @@ private function relateDocumentsById( // linking to it links to a document this write creates. $creating = isset($prepared?->preparing[$relatedCollection->getId()][$relationId]) || $this->isRelating($coroutine, $relatedCollection, $relationId); - if ($creating && ($this->checkExist->get() || ! $this->writesById($relationType, $twoWay, $side))) { + if ($creating && ($this->checkExist->get() || ! $this->writesRelated($relationType, $twoWay, $side))) { return; } @@ -2353,7 +2347,7 @@ private function relateDocumentsById( if ($this->checkExist->get()) { return; } - } else { + } elseif ($this->writesRelated($relationType, $twoWay, $side)) { $this->authorizeLink($relatedCollection, $related); } @@ -2390,9 +2384,10 @@ private function relateDocumentsById( } /** - * Whether relating a document by id writes anything; otherwise it only reads the related document. + * Whether linking a related document writes it, or a junction row for it. Otherwise the link is only the key on + * the document being written, so linking reads the related document and needs only read permission on it. */ - private function writesById(RelationshipType $relationType, bool $twoWay, RelationshipSide $side): bool + private function writesRelated(RelationshipType $relationType, bool $twoWay, RelationshipSide $side): bool { return match ($relationType) { RelationshipType::OneToOne => $twoWay, @@ -3382,8 +3377,8 @@ private function linkRelatedDocument(Document $collection, string $id, string $t } /** - * Linking an existing document to another one needs update permission on it, whichever - * relationship type holds the link. + * Linking an existing document needs update permission on it when the link writes it or a junction row for it; + * see writesRelated(). * * @throws AuthorizationException */ @@ -3563,9 +3558,10 @@ private static function toDocument(array $value): Document /** * Check, without writing, that the caller may relate the related documents $document carries the way writing it * relates them, as deep as a write relates them: create on the related collection for a new document, update - * (collection-level, or document-level under document security) to link a stored one or to change one sent as a - * document, and never a stored document the caller cannot read. With $old, the stored document an update - * changes, a related document $old already links is not linked again, so only changing it needs update. + * (collection-level, or document-level under document security) to change one sent as a document or to link a + * stored one where the link writes it or a junction row for it, read to link any other stored one, and never a + * stored document the caller cannot read. With $old, the stored document an update changes, a related document + * $old already links is not linked again, so only changing it needs update. * * With $resolve, every related document is judged as a later write would find it: $resolve(collection, ID) * returns the document as it will be when the write runs, with its permissions and without the caller's read @@ -3612,6 +3608,7 @@ private function authorizeRelated(Document $collection, Document $document, ?Doc } $linked = self::relatedIds($old?->getAttribute($key)); + $writes = $attribute->side === null || $this->writesRelated($relationship->type, $relationship->twoWay, $attribute->side); $value = $document->getAttribute($key); if ($value instanceof Operator) { $value = $value->isArrayOperation() ? $this->applyRelationshipOperator($value, $linked) : []; @@ -3619,30 +3616,31 @@ private function authorizeRelated(Document $collection, Document $document, ?Doc foreach (self::relatedValues($value) as $related) { if (\is_string($related)) { - $this->authorizeLinkById($relatedCollection, $related, $linked, $resolve); + $this->authorizeLinkById($relatedCollection, $related, $linked, $writes, $resolve); continue; } - $this->authorizeRelatedDocument($collection, $relatedCollection, $related, $linked, $writeStack, $resolve); + $this->authorizeRelatedDocument($collection, $relatedCollection, $related, $linked, $writes, $writeStack, $resolve); } } } /** * @param list $linked + * @param bool $writes Whether the link writes the related document or a junction row * @param (Closure(string, string): ?Document)|null $resolve * * @throws AuthorizationException */ - private function authorizeLinkById(Document $relatedCollection, string $id, array $linked, ?Closure $resolve): void + private function authorizeLinkById(Document $relatedCollection, string $id, array $linked, bool $writes, ?Closure $resolve): void { if (\in_array($id, $linked, true)) { return; } $stored = $this->viewRelated($relatedCollection, $id, PermissionType::Read, $resolve, [Query::select([Document::ID])]); - if ($stored->isEmpty()) { + if ($stored->isEmpty() || ! $writes) { return; } @@ -3651,13 +3649,14 @@ private function authorizeLinkById(Document $relatedCollection, string $id, arra /** * @param list $linked + * @param bool $writes Whether the link writes the related document or a junction row * @param list $writeStack * @param (Closure(string, string): ?Document)|null $resolve * * @throws AuthorizationException * @throws RelationshipException */ - private function authorizeRelatedDocument(Document $collection, Document $relatedCollection, Document $related, array $linked, array $writeStack, ?Closure $resolve): void + private function authorizeRelatedDocument(Document $collection, Document $relatedCollection, Document $related, array $linked, bool $writes, array $writeStack, ?Closure $resolve): void { $id = $related->getId(); $stored = $id === '' @@ -3676,7 +3675,7 @@ private function authorizeRelatedDocument(Document $collection, Document $relate } $changes = \array_diff(\array_keys($related->getArrayCopy()), [Document::ID]) !== []; - if (! \in_array($id, $linked, true) || $changes) { + if ($changes || ($writes && ! \in_array($id, $linked, true))) { $this->authorizeLink($relatedCollection, $stored); } diff --git a/tests/e2e/Adapter/Scopes/PermissionTests.php b/tests/e2e/Adapter/Scopes/PermissionTests.php index cf819924a..31a704235 100644 --- a/tests/e2e/Adapter/Scopes/PermissionTests.php +++ b/tests/e2e/Adapter/Scopes/PermissionTests.php @@ -1915,6 +1915,10 @@ public function testNestedRelatedDocumentsFollowTheLinkRule(): void foreach ([RelationshipType::OneToOne, RelationshipType::OneToMany, RelationshipType::ManyToOne, RelationshipType::ManyToMany] as $type) { foreach ($scenarios as $scenario => [$documentSecurity, $permissions, $related, $refusal]) { + // A many-to-one link by ID is only the key on the parent, so it needs read on the related document. + if ($type === RelationshipType::ManyToOne && $related === 'stored' && $refusal === "No permissions provided for action 'update'") { + $refusal = null; + } $parents = 'link_rule_parents_'.\uniqid(); $relatedCollection = 'link_rule_related_'.\uniqid(); $label = $type->value.', '.$scenario; diff --git a/tests/unit/Relationships/NestedLinkPermissionsTest.php b/tests/unit/Relationships/NestedLinkPermissionsTest.php index cbfa3416d..5b1583ad2 100644 --- a/tests/unit/Relationships/NestedLinkPermissionsTest.php +++ b/tests/unit/Relationships/NestedLinkPermissionsTest.php @@ -25,10 +25,11 @@ use Utopia\Database\Validator\Authorization; /** - * A nested write links or changes a stored related document only with update on it (collection-level, or - * document-level under document security), whichever relationship type holds the link and whether the document - * is sent by ID or as a document. A stored document the caller cannot read is refused, never linked, changed or - * taken for a new one. + * A nested write changes a stored related document, or links one where the link writes it or a junction row for + * it, only with update on it (collection-level, or document-level under document security), whether the document + * is sent by ID or as a document. A link by ID that is only the key on the document being written, a one-way + * one-to-one or a many-to-one, needs read. A stored document the caller cannot read is refused, never linked, + * changed or taken for a new one. */ final class NestedLinkPermissionsTest extends TestCase { @@ -89,7 +90,9 @@ public static function cases(): iterable foreach ($adapters as $adapterName => $adapter) { foreach ($writes[$adapterName] as $writeName => $transaction) { foreach (['oneToOne', 'oneToOne one-way', 'oneToMany', 'manyToOne', 'manyToMany'] as $type) { + $keySide = \in_array($type, ['oneToOne one-way', 'manyToOne'], true); foreach ($scenarios as $scenario => [$documentSecurity, $collectionPermissions, $documentPermissions, $caller, $byId, $asDocument]) { + $byId = $keySide && $byId === self::NO_UPDATE ? null : $byId; yield "{$adapterName}, {$writeName}, {$type} by ID, {$scenario}" => [$adapter, $type, $documentSecurity, $collectionPermissions, $documentPermissions, $caller, self::ID, $byId, $transaction]; yield "{$adapterName}, {$writeName}, {$type} as a document, {$scenario}" => [$adapter, $type, $documentSecurity, $collectionPermissions, $documentPermissions, $caller, self::DOCUMENT, $asDocument, $transaction]; } diff --git a/tests/unit/Relationships/StagedViewAuthorizationTest.php b/tests/unit/Relationships/StagedViewAuthorizationTest.php index 1e4adb57d..2e55f04b0 100644 --- a/tests/unit/Relationships/StagedViewAuthorizationTest.php +++ b/tests/unit/Relationships/StagedViewAuthorizationTest.php @@ -89,7 +89,10 @@ public static function cases(): iterable foreach ($adapters as $adapterName => $adapter) { foreach (self::TYPES as $type) { + // A one-way one-to-one or many-to-one link by ID is only the key on the parent, so it needs read. + $keySide = \in_array($type, ['oneToOne one-way', 'manyToOne'], true); foreach ($scenarios as $scenario => [$state, $documentSecurity, $collectionPermissions, $storedPermissions, $stagedPermissions, $byId, $asDocument]) { + $byId = $keySide && $byId === self::NO_UPDATE ? null : $byId; yield "{$adapterName}, {$type} by ID, {$scenario}" => [$adapter, $type, $state, $documentSecurity, $collectionPermissions, $storedPermissions, $stagedPermissions, self::ID, $byId]; yield "{$adapterName}, {$type} as a document, {$scenario}" => [$adapter, $type, $state, $documentSecurity, $collectionPermissions, $storedPermissions, $stagedPermissions, self::DOCUMENT, $asDocument]; } @@ -214,7 +217,7 @@ public function testANestedDocumentIsJudgedAgainstTheLinksTheStagedWritesLeave() $this->seed(new Memory(), 'oneToMany', false, [Permission::create(Role::users()), Permission::read(Role::users()), Permission::update(Role::users())], []); $this->authorization->skip(function (): void { $this->database->createCollection(Collection::create('toys', attributes: [Attribute::string('name', size: 64)], permissions: [Permission::read(Role::users())])); - $this->database->createRelationship('related', Relationship::manyToOne('toys', 'toy', onDelete: RelationshipDeleteAction::SetNull)); + $this->database->createRelationship('related', Relationship::oneToOne('toys', 'toy', twoWay: true, twoWayKey: 'owner', onDelete: RelationshipDeleteAction::SetNull)); $this->database->createDocument('toys', new Document(['$id' => 'toy', 'name' => 'Toy'])); }); $parent = new Document(['$id' => 'parent', 'related' => [['$id' => 'stored', 'toy' => 'toy']]]);