diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 294ae817bc..e6d2cf00ef 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -509,7 +509,7 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') 'emailVerification' => true, 'status' => Auth::USER_STATUS_ACTIVATED, // Email should already be authenticated by OAuth2 provider 'password' => Auth::passwordHash(Auth::passwordGenerator()), - 'passwordUpdate' => \time(), + 'passwordUpdate' => 0, 'registration' => \time(), 'reset' => false, 'name' => $name, @@ -1007,7 +1007,7 @@ App::patch('/v1/account/password') ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) ->label('sdk.response.model', Response::MODEL_USER) ->param('password', '', new Password(), 'New user password. Must be between 6 to 32 chars.') - ->param('oldPassword', '', new Password(), 'Old user password. Must be between 6 to 32 chars.') + ->param('oldPassword', '', new Password(), 'Old user password. Must be between 6 to 32 chars.', true) ->inject('response') ->inject('user') ->inject('projectDB') @@ -1018,12 +1018,14 @@ App::patch('/v1/account/password') /** @var Appwrite\Database\Database $projectDB */ /** @var Appwrite\Event\Event $audits */ - if (!Auth::passwordVerify($oldPassword, $user->getAttribute('password'))) { // Double check user password + // Check old password only if its an existing user. + if ($user->getAttribute('passwordUpdate') !== 0 && !Auth::passwordVerify($oldPassword, $user->getAttribute('password'))) { // Double check user password throw new Exception('Invalid credentials', 401); } $user = $projectDB->updateDocument(\array_merge($user->getArrayCopy(), [ 'password' => Auth::passwordHash($password), + 'passwordUpdate' => \time(), ])); if (false === $user) { diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index 9fcb511fee..d19a15efcb 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -341,7 +341,12 @@ App::post('/v1/teams/:teamId/memberships') 'emailVerification' => false, 'status' => Auth::USER_STATUS_UNACTIVATED, 'password' => Auth::passwordHash(Auth::passwordGenerator()), - 'passwordUpdate' => \time(), + /** + * Set the password update time to 0 for users created using + * team invite and OAuth to allow password updates without an + * old password + */ + 'passwordUpdate' => 0, 'registration' => \time(), 'reset' => false, 'name' => $name, @@ -580,7 +585,7 @@ App::patch('/v1/teams/:teamId/memberships/:membershipId/status') } if ($userId != $membership->getAttribute('userId')) { - throw new Exception('Invite not belong to current user ('.$user->getAttribute('email').')', 401); + throw new Exception('Invite does not belong to current user ('.$user->getAttribute('email').')', 401); } if (empty($user->getId())) { @@ -594,7 +599,7 @@ App::patch('/v1/teams/:teamId/memberships/:membershipId/status') } if ($membership->getAttribute('userId') !== $user->getId()) { - throw new Exception('Invite not belong to current user ('.$user->getAttribute('email').')', 401); + throw new Exception('Invite does not belong to current user ('.$user->getAttribute('email').')', 401); } $membership // Attach user to team diff --git a/docs/references/account/update-password.md b/docs/references/account/update-password.md index b37b70cef9..c682030a10 100644 --- a/docs/references/account/update-password.md +++ b/docs/references/account/update-password.md @@ -1 +1 @@ -Update currently logged in user password. For validation, user is required to pass in the new password, and the old password. +Update currently logged in user password. For validation, user is required to pass in the new password, and the old password. For users created with OAuth and Team Invites, oldPassword is optional. \ No newline at end of file diff --git a/src/Appwrite/Utopia/Response/Model/User.php b/src/Appwrite/Utopia/Response/Model/User.php index 2df8bf7432..00353724cd 100644 --- a/src/Appwrite/Utopia/Response/Model/User.php +++ b/src/Appwrite/Utopia/Response/Model/User.php @@ -34,6 +34,12 @@ class User extends Model 'default' => 0, 'example' => 0, ]) + ->addRule('passwordUpdate', [ + 'type' => self::TYPE_INTEGER, + 'description' => 'Unix timestamp of the most recent password update', + 'default' => 0, + 'example' => 1592981250, + ]) ->addRule('email', [ 'type' => self::TYPE_STRING, 'description' => 'User email address.', diff --git a/tests/e2e/Services/Account/AccountBase.php b/tests/e2e/Services/Account/AccountBase.php index 38d985ab06..46b9f127d0 100644 --- a/tests/e2e/Services/Account/AccountBase.php +++ b/tests/e2e/Services/Account/AccountBase.php @@ -509,6 +509,33 @@ trait AccountBase $this->assertEquals($response['headers']['status-code'], 400); + /** + * Existing user tries to update password by passing wrong old password -> SHOULD FAIL + */ + $response = $this->client->call(Client::METHOD_PATCH, '/account/password', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'cookie' => 'a_session_'.$this->getProject()['$id'].'=' . $session, + ]), [ + 'password' => 'new-password', + 'oldPassword' => $password, + ]); + $this->assertEquals($response['headers']['status-code'], 401); + + /** + * Existing user tries to update password without passing old password -> SHOULD FAIL + */ + $response = $this->client->call(Client::METHOD_PATCH, '/account/password', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'cookie' => 'a_session_'.$this->getProject()['$id'].'=' . $session, + ]), [ + 'password' => 'new-password' + ]); + $this->assertEquals($response['headers']['status-code'], 401); + $data['password'] = 'new-password'; return $data; diff --git a/tests/e2e/Services/Account/AccountCustomClientTest.php b/tests/e2e/Services/Account/AccountCustomClientTest.php index bc274996f9..6c4de08fde 100644 --- a/tests/e2e/Services/Account/AccountCustomClientTest.php +++ b/tests/e2e/Services/Account/AccountCustomClientTest.php @@ -272,11 +272,10 @@ class AccountCustomClientTest extends Scope 'x-appwrite-project' => $this->getProject()['$id'], 'cookie' => 'a_session_'.$this->getProject()['$id'].'=' . $session, ]), [ - 'password' => 'new-password', 'oldPassword' => '', ]); - $this->assertEquals($response['headers']['status-code'], 400); + $this->assertEquals(400, $response['headers']['status-code']); return $session; } diff --git a/tests/e2e/Services/Teams/TeamsBaseClient.php b/tests/e2e/Services/Teams/TeamsBaseClient.php index e41e5531ff..9df83fcd9b 100644 --- a/tests/e2e/Services/Teams/TeamsBaseClient.php +++ b/tests/e2e/Services/Teams/TeamsBaseClient.php @@ -43,6 +43,7 @@ trait TeamsBaseClient $teamUid = $data['teamUid'] ?? ''; $teamName = $data['teamName'] ?? ''; $email = uniqid().'friend@localhost.test'; + $name = 'Friend User'; /** * Test for SUCCESS @@ -52,7 +53,7 @@ trait TeamsBaseClient 'x-appwrite-project' => $this->getProject()['$id'], ], $this->getHeaders()), [ 'email' => $email, - 'name' => 'Friend User', + 'name' => $name, 'roles' => ['admin', 'editor'], 'url' => 'http://localhost:5000/join-us#title' ]); @@ -68,7 +69,7 @@ trait TeamsBaseClient $lastEmail = $this->getLastEmail(); $this->assertEquals($email, $lastEmail['to'][0]['address']); - $this->assertEquals('Friend User', $lastEmail['to'][0]['name']); + $this->assertEquals($name, $lastEmail['to'][0]['name']); $this->assertEquals('Invitation to '.$teamName.' Team at '.$this->getProject()['name'], $lastEmail['subject']); $secret = substr($lastEmail['text'], strpos($lastEmail['text'], '&secret=', 0) + 8, 256); @@ -83,7 +84,7 @@ trait TeamsBaseClient 'x-appwrite-project' => $this->getProject()['$id'], ], $this->getHeaders()), [ 'email' => 'dasdkaskdjaskdjasjkd', - 'name' => 'Friend User', + 'name' => $name, 'roles' => ['admin', 'editor'], 'url' => 'http://localhost:5000/join-us#title' ]); @@ -95,7 +96,7 @@ trait TeamsBaseClient 'x-appwrite-project' => $this->getProject()['$id'], ], $this->getHeaders()), [ 'email' => $email, - 'name' => 'Friend User', + 'name' => $name, 'roles' => 'bad string', 'url' => 'http://localhost:5000/join-us#title' ]); @@ -107,7 +108,7 @@ trait TeamsBaseClient 'x-appwrite-project' => $this->getProject()['$id'], ], $this->getHeaders()), [ 'email' => $email, - 'name' => 'Friend User', + 'name' => $name, 'roles' => ['admin', 'editor'], 'url' => 'http://example.com/join-us#title' // bad url ]); @@ -119,6 +120,8 @@ trait TeamsBaseClient 'secret' => $secret, 'membershipUid' => $membershipUid, 'userUid' => $userUid, + 'email' => $email, + 'name' => $name ]; } @@ -131,6 +134,8 @@ trait TeamsBaseClient $secret = $data['secret'] ?? ''; $membershipUid = $data['membershipUid'] ?? ''; $userUid = $data['userUid'] ?? ''; + $email = $data['email'] ?? ''; + $name = $data['name'] ?? ''; /** * Test for SUCCESS @@ -151,6 +156,61 @@ trait TeamsBaseClient $this->assertCount(2, $response['body']['roles']); $this->assertIsInt($response['body']['joined']); $this->assertEquals(true, $response['body']['confirm']); + $session = $this->client->parseCookie((string)$response['headers']['set-cookie'])['a_session_'.$this->getProject()['$id']]; + + /** + * New User tries to update password without old password -> SHOULD PASS + */ + $response = $this->client->call(Client::METHOD_PATCH, '/account/password', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'cookie' => 'a_session_'.$this->getProject()['$id'].'=' . $session, + ]), [ + 'password' => 'new-password' + ]); + + $this->assertEquals($response['headers']['status-code'], 200); + $this->assertIsArray($response['body']); + $this->assertNotEmpty($response['body']); + $this->assertNotEmpty($response['body']['$id']); + $this->assertIsNumeric($response['body']['registration']); + $this->assertEquals($response['body']['email'], $email); + $this->assertEquals($response['body']['name'], $name); + + /** + * New User again tries to update password with ONLY new password -> SHOULD FAIL + */ + $response = $this->client->call(Client::METHOD_PATCH, '/account/password', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'cookie' => 'a_session_'.$this->getProject()['$id'].'=' . $session, + ]), [ + 'password' => 'new-password', + ]); + $this->assertEquals(401, $response['headers']['status-code']); + + /** + * New User tries to update password by passing both old and new password -> SHOULD PASS + */ + $response = $this->client->call(Client::METHOD_PATCH, '/account/password', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'cookie' => 'a_session_'.$this->getProject()['$id'].'=' . $session, + ]), [ + 'password' => 'newer-password', + 'oldPassword' => 'new-password' + ]); + + $this->assertEquals($response['headers']['status-code'], 200); + $this->assertIsArray($response['body']); + $this->assertNotEmpty($response['body']); + $this->assertNotEmpty($response['body']['$id']); + $this->assertIsNumeric($response['body']['registration']); + $this->assertEquals($response['body']['email'], $email); + $this->assertEquals($response['body']['name'], $name); /** * Test for FAILURE