From 9cb1cf418cc3b713abf44215b0f0a6cef3b20947 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Fri, 6 Oct 2023 17:02:01 +0100 Subject: [PATCH 1/7] Improve Error Handling and Add more Validation --- app/config/errors.php | 5 ++ app/controllers/api/migrations.php | 118 ++++++++++++++++++++++------- src/Appwrite/Extend/Exception.php | 1 + 3 files changed, 97 insertions(+), 27 deletions(-) diff --git a/app/config/errors.php b/app/config/errors.php index 575a4588b..1f4bda470 100644 --- a/app/config/errors.php +++ b/app/config/errors.php @@ -728,4 +728,9 @@ return [ 'description' => 'Migration is already in progress. You can check the status of the migration in your Appwrite Console\'s "Settings" > "Migrations".', 'code' => 409, ], + Exception::MIGRATION_PROVIDER_ERROR => [ + 'name' => Exception::MIGRATION_PROVIDER_ERROR, + 'description' => 'An error occurred on the provider\'s side. Please try again later.', + 'code' => 424, + ], ]; diff --git a/app/controllers/api/migrations.php b/app/controllers/api/migrations.php index 83888fff2..360e1ae81 100644 --- a/app/controllers/api/migrations.php +++ b/app/controllers/api/migrations.php @@ -207,6 +207,16 @@ App::post('/v1/migrations/firebase') ->inject('user') ->inject('events') ->action(function (array $resources, string $serviceAccount, Response $response, Database $dbForProject, Document $project, Document $user, Event $events) { + $serviceAccountData = json_decode($serviceAccount, true); + + if (empty($serviceAccountData)) { + throw new Exception(Exception::MIGRATION_PROVIDER_ERROR, 'Invalid Service Account JSON'); + } + + if (!isset($serviceAccountData['project_id']) || !isset($serviceAccountData['client_email']) || !isset($serviceAccountData['private_key'])) { + throw new Exception(Exception::MIGRATION_PROVIDER_ERROR, 'Invalid Service Account JSON'); + } + $migration = $dbForProject->createDocument('migrations', new Document([ '$id' => ID::unique(), 'status' => 'pending', @@ -449,15 +459,26 @@ App::get('/v1/migrations/appwrite/report') ->inject('project') ->inject('user') ->action(function (array $resources, string $endpoint, string $projectID, string $key, Response $response) { - try { - $appwrite = new Appwrite($projectID, $endpoint, $key); + $appwrite = new Appwrite($projectID, $endpoint, $key); - $response - ->setStatusCode(Response::STATUS_CODE_OK) - ->dynamic(new Document($appwrite->report($resources)), Response::MODEL_MIGRATION_REPORT); + try { + $report = $appwrite->report($resources); } catch (\Throwable $e) { - throw new Exception(Exception::GENERAL_SERVER_ERROR, 'Source Error: ' . $e->getMessage()); + switch ($e->getCode()) { + case 401: + throw new Exception(Exception::GENERAL_UNAUTHORIZED_SCOPE, 'Source Error: ' . $e->getMessage()); + case 429: + throw new Exception(Exception::GENERAL_RATE_LIMIT_EXCEEDED, 'Source Error: Rate Limit Exceeded, Is your Cloud Provider blocking Appwrite\'s IP?'); + case 500: + throw new Exception(Exception::MIGRATION_PROVIDER_ERROR, 'Source Error: ' . $e->getMessage()); + } + + throw new Exception(Exception::MIGRATION_PROVIDER_ERROR, 'Source Error: ' . $e->getMessage()); } + + $response + ->setStatusCode(Response::STATUS_CODE_OK) + ->dynamic(new Document($report), Response::MODEL_MIGRATION_REPORT); }); App::get('/v1/migrations/firebase/report') @@ -475,15 +496,36 @@ App::get('/v1/migrations/firebase/report') ->param('serviceAccount', '', new Text(65536), 'JSON of the Firebase service account credentials') ->inject('response') ->action(function (array $resources, string $serviceAccount, Response $response) { - try { - $firebase = new Firebase(json_decode($serviceAccount, true)); + $serviceAccount = json_decode($serviceAccount, true); - $response - ->setStatusCode(Response::STATUS_CODE_OK) - ->dynamic(new Document($firebase->report($resources)), Response::MODEL_MIGRATION_REPORT); - } catch (\Exception $e) { - throw new Exception(Exception::GENERAL_SERVER_ERROR, 'Source Error: ' . $e->getMessage()); + if (empty($serviceAccount)) { + throw new Exception(Exception::MIGRATION_PROVIDER_ERROR, 'Invalid Service Account JSON'); } + + if (!isset($serviceAccount['project_id']) || !isset($serviceAccount['client_email']) || !isset($serviceAccount['private_key'])) { + throw new Exception(Exception::MIGRATION_PROVIDER_ERROR, 'Invalid Service Account JSON'); + } + + $firebase = new Firebase($serviceAccount); + + try { + $report = $firebase->report($resources); + } catch (\Throwable $e) { + switch ($e->getCode()) { + case 401: + throw new Exception(Exception::GENERAL_UNAUTHORIZED_SCOPE, 'Source Error: ' . $e->getMessage()); + case 429: + throw new Exception(Exception::GENERAL_RATE_LIMIT_EXCEEDED, 'Source Error: Rate Limit Exceeded, Is your Cloud Provider blocking Appwrite\'s IP?'); + case 500: + throw new Exception(Exception::MIGRATION_PROVIDER_ERROR, 'Source Error: ' . $e->getMessage()); + } + + throw new Exception(Exception::MIGRATION_PROVIDER_ERROR, 'Source Error: ' . $e->getMessage()); + } + + $response + ->setStatusCode(Response::STATUS_CODE_OK) + ->dynamic(new Document($report), Response::MODEL_MIGRATION_REPORT); }); App::get('/v1/migrations/firebase/report/oauth') @@ -869,15 +911,26 @@ App::get('/v1/migrations/supabase/report') ->inject('response') ->inject('dbForProject') ->action(function (array $resources, string $endpoint, string $apiKey, string $databaseHost, string $username, string $password, int $port, Response $response) { - try { - $supabase = new Supabase($endpoint, $apiKey, $databaseHost, 'postgres', $username, $password, $port); + $supabase = new Supabase($endpoint, $apiKey, $databaseHost, 'postgres', $username, $password, $port); - $response - ->setStatusCode(Response::STATUS_CODE_OK) - ->dynamic(new Document($supabase->report($resources)), Response::MODEL_MIGRATION_REPORT); - } catch (\Exception $e) { - throw new Exception(Exception::GENERAL_SERVER_ERROR, 'Source Error: ' . $e->getMessage()); + try { + $report = $supabase->report($resources); + } catch (\Throwable $e) { + switch ($e->getCode()) { + case 401: + throw new Exception(Exception::GENERAL_UNAUTHORIZED_SCOPE, 'Source Error: ' . $e->getMessage()); + case 429: + throw new Exception(Exception::GENERAL_RATE_LIMIT_EXCEEDED, 'Source Error: Rate Limit Exceeded, Is your Cloud Provider blocking Appwrite\'s IP?'); + case 500: + throw new Exception(Exception::MIGRATION_PROVIDER_ERROR, 'Source Error: ' . $e->getMessage()); + } + + throw new Exception(Exception::MIGRATION_PROVIDER_ERROR, 'Source Error: ' . $e->getMessage()); } + + $response + ->setStatusCode(Response::STATUS_CODE_OK) + ->dynamic(new Document($report), Response::MODEL_MIGRATION_REPORT); }); App::get('/v1/migrations/nhost/report') @@ -901,15 +954,26 @@ App::get('/v1/migrations/nhost/report') ->param('port', 5432, new Integer(true), 'Source\'s Database Port.', true) ->inject('response') ->action(function (array $resources, string $subdomain, string $region, string $adminSecret, string $database, string $username, string $password, int $port, Response $response) { - try { - $nhost = new NHost($subdomain, $region, $adminSecret, $database, $username, $password, $port); + $nhost = new NHost($subdomain, $region, $adminSecret, $database, $username, $password, $port); - $response - ->setStatusCode(Response::STATUS_CODE_OK) - ->dynamic(new Document($nhost->report($resources)), Response::MODEL_MIGRATION_REPORT); - } catch (\Exception $e) { - throw new Exception(Exception::GENERAL_SERVER_ERROR, 'Source Error: ' . $e->getMessage()); + try { + $report = $nhost->report($resources); + } catch (\Throwable $e) { + switch ($e->getCode()) { + case 401: + throw new Exception(Exception::GENERAL_UNAUTHORIZED_SCOPE, 'Source Error: ' . $e->getMessage()); + case 429: + throw new Exception(Exception::GENERAL_RATE_LIMIT_EXCEEDED, 'Source Error: Rate Limit Exceeded, Is your Cloud Provider blocking Appwrite\'s IP?'); + case 500: + throw new Exception(Exception::MIGRATION_PROVIDER_ERROR, 'Source Error: ' . $e->getMessage()); + } + + throw new Exception(Exception::MIGRATION_PROVIDER_ERROR, 'Source Error: ' . $e->getMessage()); } + + $response + ->setStatusCode(Response::STATUS_CODE_OK) + ->dynamic(new Document($report), Response::MODEL_MIGRATION_REPORT); }); App::patch('/v1/migrations/:migrationId') diff --git a/src/Appwrite/Extend/Exception.php b/src/Appwrite/Extend/Exception.php index 77dc03e31..b4892f7aa 100644 --- a/src/Appwrite/Extend/Exception.php +++ b/src/Appwrite/Extend/Exception.php @@ -223,6 +223,7 @@ class Exception extends \Exception public const MIGRATION_NOT_FOUND = 'migration_not_found'; public const MIGRATION_ALREADY_EXISTS = 'migration_already_exists'; public const MIGRATION_IN_PROGRESS = 'migration_in_progress'; + public const MIGRATION_PROVIDER_ERROR = 'migration_provider_error'; protected $type = ''; protected $errors = []; From d4b53e50145015f88a6eb1189090aa4f06016808 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Fri, 13 Oct 2023 16:23:20 +0100 Subject: [PATCH 2/7] Run Linter --- app/controllers/api/migrations.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/migrations.php b/app/controllers/api/migrations.php index 360e1ae81..4c4043de9 100644 --- a/app/controllers/api/migrations.php +++ b/app/controllers/api/migrations.php @@ -216,7 +216,7 @@ App::post('/v1/migrations/firebase') if (!isset($serviceAccountData['project_id']) || !isset($serviceAccountData['client_email']) || !isset($serviceAccountData['private_key'])) { throw new Exception(Exception::MIGRATION_PROVIDER_ERROR, 'Invalid Service Account JSON'); } - + $migration = $dbForProject->createDocument('migrations', new Document([ '$id' => ID::unique(), 'status' => 'pending', From 23a1da58fba417dd27a4781d1e91bf81bf7ef770 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Mon, 16 Oct 2023 11:57:17 +0100 Subject: [PATCH 3/7] Update Status Codes --- app/config/errors.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/config/errors.php b/app/config/errors.php index 1f4bda470..e7dc6c446 100644 --- a/app/config/errors.php +++ b/app/config/errors.php @@ -731,6 +731,6 @@ return [ Exception::MIGRATION_PROVIDER_ERROR => [ 'name' => Exception::MIGRATION_PROVIDER_ERROR, 'description' => 'An error occurred on the provider\'s side. Please try again later.', - 'code' => 424, + 'code' => 400, ], ]; From 453ce03881758498da9125829f8505f92a2fcdf7 Mon Sep 17 00:00:00 2001 From: shimon Date: Mon, 23 Oct 2023 00:47:48 +0300 Subject: [PATCH 4/7] fix mod condition --- app/controllers/api/projects.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/projects.php b/app/controllers/api/projects.php index 6f2f4f40e..d8d9f6468 100644 --- a/app/controllers/api/projects.php +++ b/app/controllers/api/projects.php @@ -124,7 +124,7 @@ App::post('/v1/projects') $databaseOverride = App::getEnv('_APP_DATABASE_OVERRIDE', null); $index = array_search($databaseOverride, $databases); - if ($index) { + if ($index !== false) { $database = $databases[$index]; } else { $database = $databases[array_rand($databases)]; @@ -177,7 +177,8 @@ App::post('/v1/projects') $mod = 20; $index = array_search('database_db_fra1_self_hosted_0_0', $databases); if ($index !== false && ($project->getInternalId() % $mod === 0)) { - $project->setAttribute('database', $databases[$index]); + $database = $databases[$index]; + $project->setAttribute('database', $database); $dbForConsole->updateDocument('projects', $project); } From 44cbfb2769485257a023e7dc0788c2b9ade303dd Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Tue, 7 Nov 2023 16:17:14 +0000 Subject: [PATCH 5/7] Add null check to hash --- src/Appwrite/Auth/Validator/PasswordHistory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Appwrite/Auth/Validator/PasswordHistory.php b/src/Appwrite/Auth/Validator/PasswordHistory.php index 8cfabf466..fb73ea6c9 100644 --- a/src/Appwrite/Auth/Validator/PasswordHistory.php +++ b/src/Appwrite/Auth/Validator/PasswordHistory.php @@ -44,7 +44,7 @@ class PasswordHistory extends Password public function isValid($value): bool { foreach ($this->history as $hash) { - if (Auth::passwordVerify($value, $hash, $this->algo, $this->algoOptions)) { + if (!empty($hash) && Auth::passwordVerify($value, $hash, $this->algo, $this->algoOptions)) { return false; } } From dd9d8010c12e3dba256a4446cceff99c37f45a8c Mon Sep 17 00:00:00 2001 From: shimon Date: Wed, 8 Nov 2023 15:20:50 +0200 Subject: [PATCH 6/7] db health api throws an error when ping fails --- app/controllers/api/health.php | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/app/controllers/api/health.php b/app/controllers/api/health.php index 8ca137148..9eb190310 100644 --- a/app/controllers/api/health.php +++ b/app/controllers/api/health.php @@ -87,22 +87,18 @@ App::get('/v1/health/db') 'ping' => \round((\microtime(true) - $checkStart) / 1000) ]); } else { - $output[] = new Document([ - 'name' => $key . " ($database)", - 'status' => 'fail', - 'ping' => \round((\microtime(true) - $checkStart) / 1000) - ]); + $failure[] = $database; } } catch (\Throwable $th) { - $output[] = new Document([ - 'name' => $key . " ($database)", - 'status' => 'fail', - 'ping' => \round((\microtime(true) - $checkStart) / 1000) - ]); + $failure[] = $database; } } } + if (!empty($failure)) { + throw new Exception(Exception::GENERAL_SERVER_ERROR, 'DB failure on: ' . implode(", ", $failure)); + } + $response->dynamic(new Document([ 'statuses' => $output, 'total' => count($output), @@ -536,10 +532,10 @@ App::get('/v1/health/storage/local') foreach ( [ - 'Uploads' => APP_STORAGE_UPLOADS, - 'Cache' => APP_STORAGE_CACHE, - 'Config' => APP_STORAGE_CONFIG, - 'Certs' => APP_STORAGE_CERTIFICATES + 'Uploads' => APP_STORAGE_UPLOADS, + 'Cache' => APP_STORAGE_CACHE, + 'Config' => APP_STORAGE_CONFIG, + 'Certs' => APP_STORAGE_CERTIFICATES ] as $key => $volume ) { $device = new Local($volume); @@ -601,7 +597,7 @@ App::get('/v1/health/anti-virus') }); App::get('/v1/health/stats') // Currently only used internally - ->desc('Get system stats') +->desc('Get system stats') ->groups(['api', 'health']) ->label('scope', 'root') // ->label('sdk.auth', [APP_AUTH_TYPE_KEY]) From 54e5985940a90f53ca2b6b281e9942fb59caf91d Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Wed, 8 Nov 2023 14:35:16 +0000 Subject: [PATCH 7/7] Fix wrong operator for password history This fixes a potential chance for nulls to be introduced into the password history array when $password is null and $passwordHistory != 0 --- app/controllers/api/users.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/users.php b/app/controllers/api/users.php index fcaa2ff64..b9ae42e2d 100644 --- a/app/controllers/api/users.php +++ b/app/controllers/api/users.php @@ -85,7 +85,7 @@ function createUser(string $hash, mixed $hashOptions, string $userId, ?string $e 'status' => true, 'labels' => [], 'password' => $password, - 'passwordHistory' => is_null($password) && $passwordHistory === 0 ? [] : [$password], + 'passwordHistory' => is_null($password) || $passwordHistory === 0 ? [] : [$password], 'passwordUpdate' => (!empty($password)) ? DateTime::now() : null, 'hash' => $hash === 'plaintext' ? Auth::DEFAULT_ALGO : $hash, 'hashOptions' => $hash === 'plaintext' ? Auth::DEFAULT_ALGO_OPTIONS : $hashOptionsObject + ['type' => $hash],