From 4ff32612c8d0242a7169cc918268817124f94b0f Mon Sep 17 00:00:00 2001 From: Matej Baco Date: Fri, 4 Feb 2022 16:25:54 +0100 Subject: [PATCH] Fixing tests --- app/controllers/api/account.php | 21 ++++++++++++--------- app/controllers/api/projects.php | 2 +- app/controllers/api/teams.php | 2 +- app/controllers/api/users.php | 15 +++++++++++---- tests/unit/Auth/AuthTest.php | 21 +++++++++++++++++---- 5 files changed, 42 insertions(+), 19 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 90fe4dff2..9d7113b27 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -506,7 +506,7 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') 'email' => $email, 'emailVerification' => true, 'status' => true, // Email should already be authenticated by OAuth2 provider - 'password' => Auth::passwordHash(Auth::passwordGenerator()), + 'password' => Auth::passwordHash(Auth::passwordGenerator(), 'bcrypt'), 'passwordUpdate' => 0, 'registration' => \time(), 'reset' => false, @@ -1370,12 +1370,13 @@ App::patch('/v1/account/password') ->label('sdk.response.model', Response::MODEL_USER) ->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) + ->param('hash', 'bcrypt', new WhiteList(['bcrypt', 'scrypt']), 'Hashing algorithm for password. The default value is bcrypt.', true) // We don't allow md5 here on purpose. ->inject('response') ->inject('user') ->inject('dbForProject') ->inject('audits') ->inject('usage') - ->action(function ($password, $oldPassword, $response, $user, $dbForProject, $audits, $usage) { + ->action(function ($password, $oldPassword, $hash, $response, $user, $dbForProject, $audits, $usage) { /** @var Appwrite\Utopia\Response $response */ /** @var Utopia\Database\Document $user */ /** @var Utopia\Database\Database $dbForProject */ @@ -1383,12 +1384,12 @@ App::patch('/v1/account/password') /** @var Appwrite\Stats\Stats $usage */ // Check old password only if its an existing user. - if ($user->getAttribute('passwordUpdate') !== 0 && !Auth::passwordVerify($oldPassword, $user->getAttribute('password'))) { // Double check user password + if ($user->getAttribute('passwordUpdate') !== 0 && !Auth::passwordVerify($oldPassword, $user->getAttribute('password'), $user->getAttribute('hash'))) { // Double check user password throw new Exception('Invalid credentials', 401); } $user = $dbForProject->updateDocument('users', $user->getId(), $user - ->setAttribute('password', Auth::passwordHash($password)) + ->setAttribute('password', Auth::passwordHash($password, $hash)) ->setAttribute('passwordUpdate', \time()) ); @@ -1418,12 +1419,13 @@ App::patch('/v1/account/email') ->label('sdk.response.model', Response::MODEL_USER) ->param('email', '', new Email(), 'User email.') ->param('password', '', new Password(), 'User password. Must be at least 8 chars.') + ->param('hash', 'bcrypt', new WhiteList(['bcrypt', 'scrypt']), 'Hashing algorithm for password. The default value is bcrypt.', true) // We don't allow md5 here on purpose. ->inject('response') ->inject('user') ->inject('dbForProject') ->inject('audits') ->inject('usage') - ->action(function ($email, $password, $response, $user, $dbForProject, $audits, $usage) { + ->action(function ($email, $password, $hash, $response, $user, $dbForProject, $audits, $usage) { /** @var Appwrite\Utopia\Response $response */ /** @var Utopia\Database\Document $user */ /** @var Utopia\Database\Database $dbForProject */ @@ -1434,7 +1436,7 @@ App::patch('/v1/account/email') if ( !$isAnonymousUser && - !Auth::passwordVerify($password, $user->getAttribute('password')) + !Auth::passwordVerify($password, $user->getAttribute('password'), $user->getAttribute('hash')) ) { // Double check user password throw new Exception('Invalid credentials', 401); } @@ -1448,7 +1450,7 @@ App::patch('/v1/account/email') try { $user = $dbForProject->updateDocument('users', $user->getId(), $user - ->setAttribute('password', $isAnonymousUser ? Auth::passwordHash($password) : $user->getAttribute('password', '')) + ->setAttribute('password', $isAnonymousUser ? Auth::passwordHash($password, $hash) : $user->getAttribute('password', '')) ->setAttribute('email', $email) ->setAttribute('emailVerification', false) // After this user needs to confirm mail again ->setAttribute('search', implode(' ', [$user->getId(), $user->getAttribute('name'), $user->getAttribute('email')])) @@ -1975,11 +1977,12 @@ App::put('/v1/account/recovery') ->param('secret', '', new Text(256), 'Valid reset token.') ->param('password', '', new Password(), 'New user password. Must be at least 8 chars.') ->param('passwordAgain', '', new Password(), 'Repeat new user password. Must be at least 8 chars.') + ->param('hash', 'bcrypt', new WhiteList(['bcrypt', 'scrypt']), 'Hashing algorithm for password. The default value is bcrypt.', true) // We don't allow md5 here on purpose. ->inject('response') ->inject('dbForProject') ->inject('audits') ->inject('usage') - ->action(function ($userId, $secret, $password, $passwordAgain, $response, $dbForProject, $audits, $usage) { + ->action(function ($userId, $secret, $password, $passwordAgain, $hash, $response, $dbForProject, $audits, $usage) { /** @var Appwrite\Utopia\Response $response */ /** @var Utopia\Database\Database $dbForProject */ /** @var Appwrite\Event\Event $audits */ @@ -2005,7 +2008,7 @@ App::put('/v1/account/recovery') Authorization::setRole('user:' . $profile->getId()); $profile = $dbForProject->updateDocument('users', $profile->getId(), $profile - ->setAttribute('password', Auth::passwordHash($password)) + ->setAttribute('password', Auth::passwordHash($password, $hash)) ->setAttribute('passwordUpdate', \time()) ->setAttribute('emailVerification', true) ); diff --git a/app/controllers/api/projects.php b/app/controllers/api/projects.php index b65eb5db7..ddd673e9a 100644 --- a/app/controllers/api/projects.php +++ b/app/controllers/api/projects.php @@ -541,7 +541,7 @@ App::delete('/v1/projects/:projectId') /** @var Utopia\Database\Database $dbForConsole */ /** @var Appwrite\Event\Event $deletes */ - if (!Auth::passwordVerify($password, $user->getAttribute('password'))) { // Double check user password + if (!Auth::passwordVerify($password, $user->getAttribute('password'), $user->getAttribute('hash'))) { // Double check user password throw new Exception('Invalid credentials', 401); } diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index 0a5133d6b..810d11dae 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -324,7 +324,7 @@ App::post('/v1/teams/:teamId/memberships') 'email' => $email, 'emailVerification' => false, 'status' => true, - 'password' => Auth::passwordHash(Auth::passwordGenerator()), + 'password' => Auth::passwordHash(Auth::passwordGenerator(), 'bcrypt'), /** * Set the password update time to 0 for users created using * team invite and OAuth to allow password updates without an diff --git a/app/controllers/api/users.php b/app/controllers/api/users.php index e21b2d01f..97993294d 100644 --- a/app/controllers/api/users.php +++ b/app/controllers/api/users.php @@ -38,14 +38,20 @@ App::post('/v1/users') ->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) + ->param('hash', 'bcrypt', new WhiteList(['bcrypt', 'scrypt', 'md5']), 'Hashing algorithm for password. The default value is bcrypt.', true) + ->param('import', false, new Boolean(), 'Are you importing hashed password?', true) ->inject('response') ->inject('dbForProject') ->inject('usage') - ->action(function ($userId, $email, $password, $name, $response, $dbForProject, $usage) { + ->action(function ($userId, $email, $password, $name, $hash, $import, $response, $dbForProject, $usage) { /** @var Appwrite\Utopia\Response $response */ /** @var Utopia\Database\Database $dbForProject */ /** @var Appwrite\Stats\Stats $usage */ + if(!$import && $hash === 'md5') { + throw new Exception('For security reasons, MD5 hashing is only allowed for importing accounts. Please use bcrypt instead.'); + } + $email = \strtolower($email); try { @@ -57,7 +63,7 @@ App::post('/v1/users') 'email' => $email, 'emailVerification' => false, 'status' => true, - 'password' => Auth::passwordHash($password), + 'password' => $import ? $password : Auth::passwordHash($password, $hash), 'passwordUpdate' => \time(), 'registration' => \time(), 'reset' => false, @@ -478,10 +484,11 @@ 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.') + ->param('hash', 'bcrypt', new WhiteList(['bcrypt', 'scrypt']), 'Hashing algorithm for password. The default value is bcrypt.', true) // We don't allow md5 here on purpose. ->inject('response') ->inject('dbForProject') ->inject('audits') - ->action(function ($userId, $password, $response, $dbForProject, $audits) { + ->action(function ($userId, $password, $hash, $response, $dbForProject, $audits) { /** @var Appwrite\Utopia\Response $response */ /** @var Utopia\Database\Database $dbForProject */ /** @var Appwrite\Event\Event $audits */ @@ -493,7 +500,7 @@ App::patch('/v1/users/:userId/password') } $user - ->setAttribute('password', Auth::passwordHash($password)) + ->setAttribute('password', Auth::passwordHash($password, $hash)) ->setAttribute('passwordUpdate', \time()); $user = $dbForProject->updateDocument('users', $user->getId(), $user); diff --git a/tests/unit/Auth/AuthTest.php b/tests/unit/Auth/AuthTest.php index c3b9d0075..584a04a41 100644 --- a/tests/unit/Auth/AuthTest.php +++ b/tests/unit/Auth/AuthTest.php @@ -49,11 +49,24 @@ class AuthTest extends TestCase public function testPassword() { $secret = 'secret'; + + // bcrypt $static = '$2y$08$PDbMtV18J1KOBI9tIYabBuyUwBrtXPGhLxCy9pWP6xkldVOKLrLKy'; - $dynamic = Auth::passwordHash($secret); - - $this->assertEquals(Auth::passwordVerify($secret, $dynamic), true); - $this->assertEquals(Auth::passwordVerify($secret, $static), true); + $dynamic = Auth::passwordHash($secret, 'bcrypt'); + $this->assertEquals(true, Auth::passwordVerify($secret, $static, 'bcrypt')); + $this->assertEquals(true, Auth::passwordVerify($secret, $dynamic, 'bcrypt')); + + // md5 + $static = '5ebe2294ecd0e0f08eab7690d2a6ee69'; + $dynamic = Auth::passwordHash($secret, 'md5'); + $this->assertEquals(true, Auth::passwordVerify($secret, $static, 'md5')); + $this->assertEquals(true, Auth::passwordVerify($secret, $dynamic, 'md5')); + + // scrypt + $static = 'baaf807babd0a518e50edef84ce792e8b0520b3f1c3563504db3562778145c1fd2955f4f5aff398b8d3501f8bfcbdefc8cb51fa58d64cc3c41a6dc3319d83222'; + $dynamic = Auth::passwordHash($secret, 'scrypt'); + $this->assertEquals(true, Auth::passwordVerify($secret, $static, 'scrypt')); + $this->assertEquals(true, Auth::passwordVerify($secret, $dynamic, 'scrypt')); } public function testPasswordGenerator()