Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix: setAction method #375

Merged
merged 2 commits into from
Jan 3, 2024
Merged

Fix: setAction method #375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
00c1553
Refactor auth message and action
Meldiron Jan 3, 2024
c2f3585
Fix test
Meldiron Jan 3, 2024
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
103 changes: 59 additions & 44 deletions src/Database/Database.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use Utopia\Database\Exception\Restricted as RestrictedException;
use Utopia\Database\Exception\Structure as StructureException;
use Utopia\Database\Exception\Query as QueryException;
use Utopia\Database\Exception\Validation;
use Utopia\Database\Helpers\ID;
use Utopia\Database\Helpers\Permission;
use Utopia\Database\Helpers\Role;
Expand Down Expand Up @@ -2255,7 +2256,6 @@ public function getDocument(string $collection, string $id, array $queries = [])

$queries = \array_values($queries);

$validator = $this->authorization->setAction(self::PERMISSION_READ);
$documentSecurity = $collection->getAttribute('documentSecurity', false);
$cacheKey = 'cache-' . $this->getNamespace() . ':' . $collection->getId() . ':' . $id;

Expand All @@ -2269,10 +2269,12 @@ public function getDocument(string $collection, string $id, array $queries = [])
$document = new Document($cache);

if ($collection->getId() !== self::METADATA) {
if (!$validator->isValid([
...$collection->getRead(),
...($documentSecurity ? $document->getRead() : [])
])) {
try {
$this->authorization->validate(self::PERMISSION_READ, [
...$collection->getRead(),
...($documentSecurity ? $document->getRead() : [])
]);
} catch(Validation $err) {
return new Document();
}
}
Expand All @@ -2291,10 +2293,12 @@ public function getDocument(string $collection, string $id, array $queries = [])
$document->setAttribute('$collection', $collection->getId());

if ($collection->getId() !== self::METADATA) {
if (!$validator->isValid([
...$collection->getRead(),
...($documentSecurity ? $document->getRead() : [])
])) {
try {
$this->authorization->validate(self::PERMISSION_READ, [
...$collection->getRead(),
...($documentSecurity ? $document->getRead() : [])
]);
} catch(Validation $err) {
return new Document();
}
}
Expand Down Expand Up @@ -2613,9 +2617,10 @@ public function createDocument(string $collection, Document $document): Document
$collection = $this->silent(fn () => $this->getCollection($collection));

if ($collection->getId() !== self::METADATA) {
$authorization = $this->authorization->setAction(self::PERMISSION_CREATE);
if (!$authorization->isValid($collection->getCreate())) {
throw new AuthorizationException($authorization->getDescription());
try {
$this->authorization->validate(self::PERMISSION_CREATE, $collection->getCreate());
} catch(Validation $err) {
throw new AuthorizationException($err->getMessage());
}
}

Expand Down Expand Up @@ -2963,7 +2968,6 @@ public function updateDocument(string $collection, string $id, Document $documen
return $attribute['type'] === Database::VAR_RELATIONSHIP;
});

$validator = $this->authorization->setAction(self::PERMISSION_UPDATE);
$shouldUpdate = false;

if ($collection->getId() !== self::METADATA) {
Expand Down Expand Up @@ -3050,11 +3054,15 @@ public function updateDocument(string $collection, string $id, Document $documen
}
}

if ($shouldUpdate && !$validator->isValid([
...$collection->getUpdate(),
...($documentSecurity ? $old->getUpdate() : [])
])) {
throw new AuthorizationException($validator->getDescription());
if ($shouldUpdate) {
try {
$this->authorization->validate(self::PERMISSION_UPDATE, [
...$collection->getUpdate(),
...($documentSecurity ? $old->getUpdate() : [])
]);
} catch(Validation $err) {
throw new AuthorizationException($err->getMessage());
}
}
}

Expand Down Expand Up @@ -3496,19 +3504,19 @@ public function increaseDocumentAttribute(string $collection, string $id, string
throw new DatabaseException('Value must be numeric and greater than 0');
}

$validator = $this->authorization->setAction(self::PERMISSION_UPDATE);

$document = $this->authorization->skip(fn () => $this->silent(fn () => $this->getDocument($collection, $id))); // Skip ensures user does not need read permission for this

$collection = $this->silent(fn () => $this->getCollection($collection));

if ($collection->getId() !== self::METADATA) {
$documentSecurity = $collection->getAttribute('documentSecurity', false);
if (!$validator->isValid([
...$collection->getUpdate(),
...($documentSecurity ? $document->getUpdate() : [])
])) {
throw new AuthorizationException($validator->getDescription());
try {
$this->authorization->validate(self::PERMISSION_UPDATE, [
...$collection->getUpdate(),
...($documentSecurity ? $document->getUpdate() : [])
]);
} catch(Validation $err) {
throw new AuthorizationException($err->getMessage());
}
}

Expand Down Expand Up @@ -3563,19 +3571,19 @@ public function decreaseDocumentAttribute(string $collection, string $id, string
throw new DatabaseException('Value must be numeric and greater than 0');
}

$validator = $this->authorization->setAction(self::PERMISSION_UPDATE);

$document = $this->authorization->skip(fn () => $this->silent(fn () => $this->getDocument($collection, $id))); // Skip ensures user does not need read permission for this

$collection = $this->silent(fn () => $this->getCollection($collection));

if ($collection->getId() !== self::METADATA) {
$documentSecurity = $collection->getAttribute('documentSecurity', false);
if (!$validator->isValid([
...$collection->getUpdate(),
...($documentSecurity ? $document->getUpdate() : [])
])) {
throw new AuthorizationException($validator->getDescription());
try {
$this->authorization->validate(self::PERMISSION_UPDATE, [
...$collection->getUpdate(),
...($documentSecurity ? $document->getUpdate() : [])
]);
} catch(Validation $err) {
throw new AuthorizationException($err->getMessage());
}
}

Expand Down Expand Up @@ -3628,15 +3636,15 @@ public function deleteDocument(string $collection, string $id): bool
$document = $this->authorization->skip(fn () => $this->silent(fn () => $this->getDocument($collection, $id))); // Skip ensures user does not need read permission for this
$collection = $this->silent(fn () => $this->getCollection($collection));

$validator = $this->authorization->setAction(self::PERMISSION_DELETE);

if ($collection->getId() !== self::METADATA) {
$documentSecurity = $collection->getAttribute('documentSecurity', false);
if (!$validator->isValid([
...$collection->getDelete(),
...($documentSecurity ? $document->getDelete() : [])
])) {
throw new AuthorizationException($validator->getDescription());
try {
$this->authorization->validate(self::PERMISSION_DELETE, [
...$collection->getDelete(),
...($documentSecurity ? $document->getDelete() : [])
]);
} catch(Validation $err) {
throw new AuthorizationException($err->getMessage());
}
}

Expand Down Expand Up @@ -4089,9 +4097,14 @@ public function find(string $collection, array $queries = [], ?int $timeout = nu
throw new QueryException($validator->getDescription());
}

$authorization = $this->authorization->setAction(self::PERMISSION_READ);
$documentSecurity = $collection->getAttribute('documentSecurity', false);
$skipAuth = $authorization->isValid($collection->getRead());

try {
$this->authorization->validate(self::PERMISSION_READ, $collection->getRead());
$skipAuth = true;
} catch(Validation $err) {
$skipAuth = false;
}

if (!$skipAuth && !$documentSecurity) {
throw new AuthorizationException($validator->getDescription());
Expand Down Expand Up @@ -4261,16 +4274,18 @@ public function count(string $collection, array $queries = [], ?int $max = null)
throw new QueryException($validator->getDescription());
}

$authorization = $this->authorization->setAction(self::PERMISSION_READ);
if ($authorization->isValid($collection->getRead())) {
try {
$this->authorization->validate(self::PERMISSION_READ, $collection->getRead());
$skipAuth = true;
} catch(Validation $err) {
$skipAuth = false;
}

$queries = Query::groupByType($queries)['filters'];
$queries = self::convertQueries($collection, $queries);

$getCount = fn () => $this->adapter->count($collection->getId(), $queries, $max);
$count = $skipAuth ?? false ? $this->authorization->skip($getCount) : $getCount();
$count = $skipAuth ? $this->authorization->skip($getCount) : $getCount();

$this->trigger(self::EVENT_DOCUMENT_COUNT, $count);

Expand Down
7 changes: 7 additions & 0 deletions src/Database/Exception/Validation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

namespace Utopia\Database\Exception;

class Validation extends \Exception
{
}
58 changes: 24 additions & 34 deletions src/Database/Validator/Authorization.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Utopia\Database\Validator;

use Utopia\Database\Exception\Validation;
use Utopia\Http\Validator;

class Authorization extends Validator
Expand All @@ -13,35 +14,11 @@ class Authorization extends Validator
'any' => true
];

/**
* @var string
*/
protected string $action = '';

/**
* @var string
*/
protected string $message = 'Authorization Error';

/**
* @param string $action
*/
public function __construct(string $action = '')
{
$this->action = $action;
}

/**
* @param string $action
* @return self
*/
public function setAction(string $action): self
{
$this->action = $action;

return $this;
}

/**
* Get Description.
*
Expand All @@ -54,37 +31,50 @@ public function getDescription(): string
return $this->message;
}

/*
* Deprecated for this specific validator. Use validate() instead
*/
public function isValid(mixed $permissions): bool
{
try {
$this->validate('unknownAction', $permissions);
return true;
} catch(Validation $error) {
$this->message = $error->getMessage();
return false;
}
}

/**
* Is valid.
* Validation. Replacement of isValid() which throws exception instead
*
* Returns true if valid or false if not.
*
* @param string $action
* @param mixed $permissions
*
* @return bool
* @return void
* @throws Validation
*/
public function isValid($permissions): bool
public function validate(string $action, mixed $permissions): void
{
if (!$this->status) {
return true;
return;
}

if (empty($permissions)) {
$this->message = 'No permissions provided for action \''.$this->action.'\'';
return false;
throw new Validation('No permissions provided for action \''.$action.'\'');
}

$permission = '-';

foreach ($permissions as $permission) {
if (\array_key_exists($permission, $this->roles)) {
return true;
return;
}
}

$this->message = 'Missing "'.$this->action.'" permission for role "'.$permission.'". Only "'.\json_encode($this->getRoles()).'" scopes are allowed and "'.\json_encode($permissions).'" was given.';

return false;
throw new Validation('Missing "'.$action.'" permission for role "'.$permission.'". Only "'.\json_encode($this->getRoles()).'" scopes are allowed and "'.\json_encode($permissions).'" was given.');
}

/**
Expand Down
5 changes: 2 additions & 3 deletions tests/Database/Validator/AuthorizationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace Utopia\Tests\Validator;

use Utopia\Database\Database;
use Utopia\Database\Document;
use Utopia\Database\Helpers\ID;
use Utopia\Database\Helpers\Permission;
Expand Down Expand Up @@ -39,12 +38,12 @@ public function testValues(): void
],
]);

$object = $this->authorization->setAction(Database::PERMISSION_READ);
$object = $this->authorization;

$this->assertEquals($object->isValid($document->getRead()), false);
$this->assertEquals($object->isValid(''), false);
$this->assertEquals($object->isValid([]), false);
$this->assertEquals($object->getDescription(), 'No permissions provided for action \'read\'');
$this->assertEquals($object->getDescription(), 'No permissions provided for action \'unknownAction\'');

$this->authorization->setRole(Role::user('456')->toString());
$this->authorization->setRole(Role::user('123')->toString());
Expand Down