Finish security todos

This commit is contained in:
Matej Bačo
2026-03-26 15:35:40 +01:00
parent cbfdd27834
commit eb097a037b
5 changed files with 71 additions and 9 deletions
+13 -3
View File
@@ -22,6 +22,7 @@ class Key
protected array $scopes,
protected string $name,
protected bool $expired = false,
protected ?string $expire = null,
protected array $disabledMetrics = [],
protected bool $hostnameOverride = false,
protected bool $bannerDisabled = false,
@@ -71,6 +72,11 @@ class Key
return $this->expired;
}
public function getExpire(): ?string
{
return $this->expire;
}
public function getDisabledMetrics(): array
{
return $this->disabledMetrics;
@@ -176,6 +182,7 @@ class Key
$scopes,
$name,
$expired,
DateTime::addSeconds(new \DateTime(), 86400), // Max possible JWT expiry
$disabledMetrics,
$hostnameOverride,
$bannerDisabled,
@@ -210,7 +217,8 @@ class Key
$role,
$scopes,
$name,
$expired
$expired,
$expire,
);
case API_KEY_ACCOUNT:
$key = $user->find(
@@ -244,7 +252,8 @@ class Key
$role,
$scopes,
$name,
$expired
$expired,
$expire,
);
return $key;
@@ -280,7 +289,8 @@ class Key
$role,
$scopes,
$name,
$expired
$expired,
$expire,
);
return $key;
@@ -2,6 +2,7 @@
namespace Appwrite\Platform\Modules\Project\Http\Project\Keys;
use Appwrite\Auth\Key;
use Appwrite\Event\Event as QueueEvent;
use Appwrite\Extend\Exception;
use Appwrite\Platform\Modules\Compute\Base;
@@ -69,6 +70,7 @@ class Create extends Base
->inject('dbForPlatform')
->inject('project')
->inject('authorization')
->inject('apiKey')
->callback($this->action(...));
}
@@ -85,10 +87,25 @@ class Create extends Base
Database $dbForPlatform,
Document $project,
Authorization $authorization,
?Key $apiKey,
) {
$keyId = ($keyId == 'unique()') ? ID::unique() : $keyId;
// TODO: If authorized as API key, verify scopes and expiry is OK
$isProjectApiKey = $apiKey !== null && !empty($apiKey->getProjectId());
if ($isProjectApiKey) {
if (!empty(\array_diff($scopes ?? [], $apiKey->getScopes()))) {
throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'New API key cannot exceed scopes of currently authenticated API key.');
}
if (\is_null($expire) && !\is_null($apiKey->getExpire())) {
throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'New API key must have expiry set, because currently authenticated API key has an expiry.');
}
if (!\is_null($expire) && $expire > $apiKey->getExpire()) {
throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'New API key expiry must be sooner than currently authenticated API key expiry.');
}
}
$key = new Document([
'$id' => $keyId,
@@ -2,6 +2,7 @@
namespace Appwrite\Platform\Modules\Project\Http\Project\Keys;
use Appwrite\Auth\Key;
use Appwrite\Extend\Exception;
use Appwrite\Platform\Modules\Compute\Base;
use Appwrite\SDK\AuthType;
@@ -53,6 +54,7 @@ class Get extends Base
->inject('dbForPlatform')
->inject('project')
->inject('authorization')
->inject('apiKey')
->callback($this->action(...));
}
@@ -62,6 +64,7 @@ class Get extends Base
Database $dbForPlatform,
Document $project,
Authorization $authorization,
?Key $apiKey,
) {
$key = $authorization->skip(fn () => $dbForPlatform->getDocument('keys', $keyId));
@@ -69,7 +72,11 @@ class Get extends Base
throw new Exception(Exception::KEY_NOT_FOUND);
}
// TODO: If authorized as api key, hide secret of key
$isProjectApiKey = $apiKey !== null && !empty($apiKey->getProjectId());
if ($isProjectApiKey) {
$key->setAttribute('secret', '');
}
$response->dynamic($key, Response::MODEL_KEY);
}
@@ -2,6 +2,7 @@
namespace Appwrite\Platform\Modules\Project\Http\Project\Keys;
use Appwrite\Auth\Key;
use Appwrite\Event\Event as QueueEvent;
use Appwrite\Extend\Exception;
use Appwrite\Platform\Modules\Compute\Base;
@@ -67,6 +68,7 @@ class Update extends Base
->inject('dbForPlatform')
->inject('project')
->inject('authorization')
->inject('apiKey')
->callback($this->action(...));
}
@@ -80,6 +82,7 @@ class Update extends Base
Database $dbForPlatform,
Document $project,
Authorization $authorization,
?Key $apiKey,
) {
$key = $authorization->skip(fn () => $dbForPlatform->getDocument('keys', $keyId));
@@ -87,7 +90,21 @@ class Update extends Base
throw new Exception(Exception::KEY_NOT_FOUND);
}
// TODO: If authorized as API key, verify scopes and expiry is OK
$isProjectApiKey = $apiKey !== null && !empty($apiKey->getProjectId());
if ($isProjectApiKey) {
if (!empty(\array_diff($scopes ?? [], $apiKey->getScopes()))) {
throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'Updated API key cannot exceed scopes of currently authenticated API key.');
}
if (\is_null($expire) && !\is_null($apiKey->getExpire())) {
throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'Updated API key must have expiry set, because currently authenticated API key has an expiry.');
}
if (!\is_null($expire) && $expire > $apiKey->getExpire()) {
throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'Updated API key expiry must be sooner than currently authenticated API key expiry.');
}
}
$updates = new Document([
'name' => $name,
@@ -97,7 +114,7 @@ class Update extends Base
try {
$key = $authorization->skip(fn () => $dbForPlatform->updateDocument('keys', $key->getId(), $updates));
} catch (Duplicate $th) {
} catch (Duplicate) {
throw new Exception(Exception::KEY_ALREADY_EXISTS);
}
@@ -105,7 +122,9 @@ class Update extends Base
$queueForEvents->setParam('keyId', $key->getId());
// TODO: If authorized as api key, hide secret of key
if ($isProjectApiKey) {
$key->setAttribute('secret', '');
}
$response->dynamic($key, Response::MODEL_KEY);
}
@@ -2,6 +2,7 @@
namespace Appwrite\Platform\Modules\Project\Http\Project\Keys;
use Appwrite\Auth\Key;
use Appwrite\Extend\Exception;
use Appwrite\Platform\Modules\Compute\Base;
use Appwrite\SDK\AuthType;
@@ -59,6 +60,7 @@ class XList extends Base
->inject('response')
->inject('dbForPlatform')
->inject('authorization')
->inject('apiKey')
->callback($this->action(...));
}
@@ -72,6 +74,7 @@ class XList extends Base
Response $response,
Database $dbForPlatform,
Authorization $authorization,
?Key $apiKey,
) {
try {
$queries = Query::parseQueries($queries);
@@ -119,7 +122,13 @@ class XList extends Base
throw new Exception(Exception::DATABASE_QUERY_ORDER_NULL, "The order attribute '{$e->getAttribute()}' had a null value. Cursor pagination requires all documents order attribute values are non-null.");
}
// TODO: API keys cannot see secrets
$isProjectApiKey = $apiKey !== null && !empty($apiKey->getProjectId());
if ($isProjectApiKey) {
foreach ($keys as $key) {
$key->setAttribute('secret', '');
}
}
$response->dynamic(new Document([
'keys' => $keys,