Merge pull request #1120 from appwrite/feat-update-team-membership-password

Allow external users to update password without an old password

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) {

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

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.

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.',

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;

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;
     }

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