From 1ff8ce2382b2dbe60ade9ef8e99769cfb89ef5c0 Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Mon, 26 Dec 2022 05:24:26 +0000 Subject: [PATCH] check against passwords dictionary --- app/controllers/api/account.php | 12 ++++++------ app/controllers/api/users.php | 19 +++++++++++++++++-- app/init.php | 6 +++--- src/Appwrite/Extend/Exception.php | 1 + 4 files changed, 27 insertions(+), 11 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 43c8fc9cb4..2b7877b640 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -67,13 +67,13 @@ App::post('/v1/account') ->param('email', '', new Email(), 'User email.') ->param('password', '', new Password(), 'User password. Must be at least 8 chars.') ->param('name', '', new Text(128), 'User name. Max length: 128 chars.', true) - ->inject('passwordsdb') + ->inject('passwordsDB') ->inject('request') ->inject('response') ->inject('project') ->inject('dbForProject') ->inject('events') - ->action(function (string $userId, string $email, string $password, string $name, string $passwordsdb, Request $request, Response $response, Document $project, Database $dbForProject, Event $events) { + ->action(function (string $userId, string $email, string $password, string $name, string $passwordsDB, Request $request, Response $response, Document $project, Database $dbForProject, Event $events) { $email = \strtolower($email); if ('console' === $project->getId()) { @@ -99,7 +99,7 @@ App::post('/v1/account') } } - if(str_contains($passwordsdb, $password)) { + if(str_contains($passwordsDB, $password)) { throw new Exception(Exception::USER_PASSWORD_IN_DICTIONARY, 'The password is among the common passwords in dictionary.', 403); @@ -1520,20 +1520,20 @@ App::patch('/v1/account/password') ->label('sdk.response.model', Response::MODEL_ACCOUNT) ->param('password', '', new Password(), 'New user password. Must be at least 8 chars.') ->param('oldPassword', '', new Password(), 'Current user password. Must be at least 8 chars.', true) - ->inject('passwordsdb') + ->inject('passwordsDB') ->inject('response') ->inject('user') ->inject('project') ->inject('dbForProject') ->inject('events') - ->action(function (string $password, string $oldPassword, string $passwordsdb, Response $response, Document $user, Document $project, Database $dbForProject, Event $events) { + ->action(function (string $password, string $oldPassword, string $passwordsDB, Response $response, Document $user, Document $project, Database $dbForProject, Event $events) { // Check old password only if its an existing user. if (!empty($user->getAttribute('passwordUpdate')) && !Auth::passwordVerify($oldPassword, $user->getAttribute('password'), $user->getAttribute('hash'), $user->getAttribute('hashOptions'))) { // Double check user password throw new Exception(Exception::USER_INVALID_CREDENTIALS); } - if(str_contains($passwordsdb, $password)) { + if(str_contains($passwordsDB, $password)) { throw new Exception(Exception::USER_PASSWORD_IN_DICTIONARY, 'The password is among the common passwords in dictionary.', 403); diff --git a/app/controllers/api/users.php b/app/controllers/api/users.php index 54e132f085..2b8ed631b2 100644 --- a/app/controllers/api/users.php +++ b/app/controllers/api/users.php @@ -107,11 +107,19 @@ App::post('/v1/users') ->param('phone', null, new Phone(), 'Phone number. Format this number with a leading \'+\' and a country code, e.g., +16175551212.', true) ->param('password', null, new Password(), 'Plain text user password. Must be at least 8 chars.', true) ->param('name', '', new Text(128), 'User name. Max length: 128 chars.', true) + ->inject('passwordsDB') ->inject('response') ->inject('project') ->inject('dbForProject') ->inject('events') - ->action(function (string $userId, ?string $email, ?string $phone, ?string $password, string $name, Response $response, Document $project, Database $dbForProject, Event $events) { + ->action(function (string $userId, ?string $email, ?string $phone, ?string $password, string $name, string $passwordsDB, Response $response, Document $project, Database $dbForProject, Event $events) { + + if(str_contains($passwordsDB, $password)) { + throw new Exception(Exception::USER_PASSWORD_IN_DICTIONARY, + 'The password is among the common passwords in dictionary.', + 403); + } + $user = createUser('plaintext', '{}', $userId, $email, $password, $phone, $name, $project, $dbForProject, $events); $response @@ -792,11 +800,12 @@ App::patch('/v1/users/:userId/password') ->label('sdk.response.model', Response::MODEL_USER) ->param('userId', '', new UID(), 'User ID.') ->param('password', '', new Password(), 'New user password. Must be at least 8 chars.') + ->inject('passwordsDB') ->inject('response') ->inject('project') ->inject('dbForProject') ->inject('events') - ->action(function (string $userId, string $password, Response $response, Document $project, Database $dbForProject, Event $events) { + ->action(function (string $userId, string $password, string $passwordsDB, Response $response, Document $project, Database $dbForProject, Event $events) { $user = $dbForProject->getDocument('users', $userId); @@ -804,6 +813,12 @@ App::patch('/v1/users/:userId/password') throw new Exception(Exception::USER_NOT_FOUND); } + if(str_contains($passwordsDB, $password)) { + throw new Exception(Exception::USER_PASSWORD_IN_DICTIONARY, + 'The password is among the common passwords in dictionary.', + 403); + } + $newPassword = Auth::passwordHash($password, Auth::DEFAULT_ALGO, Auth::DEFAULT_ALGO_OPTIONS); $historyLimit = $project->getAttribute('auths', [])['passwordHistory'] ?? 0; diff --git a/app/init.php b/app/init.php index c02dffff95..5762085eae 100644 --- a/app/init.php +++ b/app/init.php @@ -605,7 +605,7 @@ $register->set('smtp', function () { $register->set('geodb', function () { return new Reader(__DIR__ . '/assets/dbip/dbip-country-lite-2022-06.mmdb'); }); -$register->set('passwordsdb', function () { +$register->set('passwordsDB', function () { return \file_get_contents(__DIR__ . '/assets/security/10k-common-passwords'); }); $register->set('db', function () { @@ -1025,9 +1025,9 @@ App::setResource('geodb', function ($register) { return $register->get('geodb'); }, ['register']); -App::setResource('passwordsdb', function ($register) { +App::setResource('passwordsDB', function ($register) { /** @var Utopia\Registry\Registry $register */ - return $register->get('passwordsdb'); + return $register->get('passwordsDB'); }, ['register']); App::setResource('sms', function () { diff --git a/src/Appwrite/Extend/Exception.php b/src/Appwrite/Extend/Exception.php index 3940ad6da9..2af560c18f 100644 --- a/src/Appwrite/Extend/Exception.php +++ b/src/Appwrite/Extend/Exception.php @@ -65,6 +65,7 @@ class Exception extends \Exception public const USER_SESSION_ALREADY_EXISTS = 'user_session_already_exists'; public const USER_NOT_FOUND = 'user_not_found'; public const USER_PASSWORD_RECENTLY_USED = 'password_recently_used'; + public const USER_PASSWORD_IN_DICTIONARY = 'password_in_dictionary'; public const USER_EMAIL_ALREADY_EXISTS = 'user_email_already_exists'; public const USER_PASSWORD_MISMATCH = 'user_password_mismatch'; public const USER_SESSION_NOT_FOUND = 'user_session_not_found';