Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 21 additions & 22 deletions src/Database/Hook/Relationships.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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) {
Expand All @@ -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()) {
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}

Expand All @@ -2353,7 +2347,7 @@ private function relateDocumentsById(
if ($this->checkExist->get()) {
return;
}
} else {
} elseif ($this->writesRelated($relationType, $twoWay, $side)) {
$this->authorizeLink($relatedCollection, $related);
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -3612,37 +3608,39 @@ 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) : [];
}

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<string> $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;
}

Expand All @@ -3651,13 +3649,14 @@ private function authorizeLinkById(Document $relatedCollection, string $id, arra

/**
* @param list<string> $linked
* @param bool $writes Whether the link writes the related document or a junction row
* @param list<string> $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 === ''
Expand All @@ -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);
}

Expand Down
4 changes: 4 additions & 0 deletions tests/e2e/Adapter/Scopes/PermissionTests.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 7 additions & 4 deletions tests/unit/Relationships/NestedLinkPermissionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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];
}
Expand Down
5 changes: 4 additions & 1 deletion tests/unit/Relationships/StagedViewAuthorizationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
Expand Down Expand Up @@ -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']]]);
Expand Down
Loading