diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 2cf9770fc..56cf2ab92 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -514,7 +514,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, @@ -682,7 +682,7 @@ App::post('/v1/account/sessions/anonymous') 'emailVerification' => false, 'status' => Auth::USER_STATUS_UNACTIVATED, 'password' => null, - 'passwordUpdate' => \time(), + 'passwordUpdate' => 0, 'registration' => \time(), 'reset' => false, 'name' => null @@ -1012,7 +1012,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') @@ -1023,12 +1023,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 910427da1..49aaf3a00 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -341,7 +341,17 @@ 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 + * with a team invite. This will allow us to distinguish b/w + * new and old appwrite users. + * if (passwordUpdate == 0) { + * // new appwrite user created with oauth/anonymous/team invite + * } else { + * // existing appwrite user + * } + */ + 'passwordUpdate' => 0, 'registration' => \time(), 'reset' => false, 'name' => $name, @@ -580,7 +590,7 @@ App::patch('/v1/teams/:teamId/memberships/:inviteId/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 +604,7 @@ App::patch('/v1/teams/:teamId/memberships/:inviteId/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/src/Appwrite/Utopia/Response/Model/User.php b/src/Appwrite/Utopia/Response/Model/User.php index 2df8bf743..00353724c 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 38d985ab0..46b9f127d 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 bc274996f..ddad5d23f 100644 --- a/tests/e2e/Services/Account/AccountCustomClientTest.php +++ b/tests/e2e/Services/Account/AccountCustomClientTest.php @@ -264,7 +264,7 @@ class AccountCustomClientTest extends Scope public function testUpdateAnonymousAccountPassword($session) { /** - * Test for FAILURE + * Anonymous account updates password without old password -> SHOULD PASS */ $response = $this->client->call(Client::METHOD_PATCH, '/account/password', array_merge([ 'origin' => 'http://localhost', @@ -276,7 +276,44 @@ class AccountCustomClientTest extends Scope 'oldPassword' => '', ]); - $this->assertEquals($response['headers']['status-code'], 400); + $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']); + + /** + * Anonymous account tries to update password again without 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); + + /** + * Anonymous account updates password with new and 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' => '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']); return $session; } diff --git a/tests/e2e/Services/Teams/TeamsBaseClient.php b/tests/e2e/Services/Teams/TeamsBaseClient.php index 72fc99d88..042a95c42 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, 'inviteUid' => $inviteUid, 'userUid' => $userUid, + 'email' => $email, + 'name' => $name ]; } @@ -131,6 +134,8 @@ trait TeamsBaseClient $secret = $data['secret'] ?? ''; $inviteUid = $data['inviteUid'] ?? ''; $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