From: Chocobozzz <me@florianbigard.com>
Date: Wed, 26 Sep 2018 14:28:15 +0000 (+0200)
Subject: Check current password on server side
X-Git-Tag: v1.0.0-beta.16~36
X-Git-Url: https://git.immae.eu/?a=commitdiff_plain;h=a890d1e0d30851741392e6e7f14acffe685d28e0;p=github%2FChocobozzz%2FPeerTube.git

Check current password on server side
---

diff --git a/client/src/app/+my-account/my-account-settings/my-account-change-password/my-account-change-password.component.html b/client/src/app/+my-account/my-account-settings/my-account-change-password/my-account-change-password.component.html
index 00b6d20fa..ae797d1bc 100644
--- a/client/src/app/+my-account/my-account-settings/my-account-change-password/my-account-change-password.component.html
+++ b/client/src/app/+my-account/my-account-settings/my-account-change-password/my-account-change-password.component.html
@@ -1,14 +1,14 @@
 <div *ngIf="error" class="alert alert-danger">{{ error }}</div>
 
-<form role="form" (ngSubmit)="checkPassword()" [formGroup]="form">
+<form role="form" (ngSubmit)="changePassword()" [formGroup]="form">
 
   <label i18n for="new-password">Change password</label>
   <input
-    type="password" id="old-password" i18n-placeholder placeholder="Old password"
-    formControlName="old-password" [ngClass]="{ 'input-error': formErrors['old-password'] }"
+    type="password" id="current-password" i18n-placeholder placeholder="Current password"
+    formControlName="current-password" [ngClass]="{ 'input-error': formErrors['current-password'] }"
   >
-  <div *ngIf="formErrors['old-password']" class="form-error">
-    {{ formErrors['old-password'] }}
+  <div *ngIf="formErrors['current-password']" class="form-error">
+    {{ formErrors['current-password'] }}
   </div>
 
   <input
diff --git a/client/src/app/+my-account/my-account-settings/my-account-change-password/my-account-change-password.component.ts b/client/src/app/+my-account/my-account-settings/my-account-change-password/my-account-change-password.component.ts
index da0021df9..e5343b33d 100644
--- a/client/src/app/+my-account/my-account-settings/my-account-change-password/my-account-change-password.component.ts
+++ b/client/src/app/+my-account/my-account-settings/my-account-change-password/my-account-change-password.component.ts
@@ -30,7 +30,7 @@ export class MyAccountChangePasswordComponent extends FormReactive implements On
 
   ngOnInit () {
     this.buildForm({
-      'old-password': this.userValidatorsService.USER_PASSWORD,
+      'current-password': this.userValidatorsService.USER_PASSWORD,
       'new-password': this.userValidatorsService.USER_PASSWORD,
       'new-confirmed-password': this.userValidatorsService.USER_CONFIRM_PASSWORD
     })
@@ -44,31 +44,26 @@ export class MyAccountChangePasswordComponent extends FormReactive implements On
                           .subscribe(() => confirmPasswordControl.setErrors({ matchPassword: true }))
   }
 
-  checkPassword () {
-    this.error = null
-    const oldPassword = this.form.value[ 'old-password' ]
+  changePassword () {
+    const currentPassword = this.form.value[ 'current-password' ]
+    const newPassword = this.form.value[ 'new-password' ]
 
-    // compare old password
-    this.authService.login(this.user.account.name, oldPassword)
-      .subscribe(
-        () => this.changePassword(),
-        err => {
-          if (err.message.indexOf('credentials are invalid') !== -1) this.error = this.i18n('Incorrect old password.')
-          else this.error = err.message
-        }
-      )
-
-  }
-
-  private changePassword () {
-    this.userService.changePassword(this.form.value[ 'new-password' ]).subscribe(
+    this.userService.changePassword(currentPassword, newPassword).subscribe(
       () => {
         this.notificationsService.success(this.i18n('Success'), this.i18n('Password updated.'))
 
         this.form.reset()
+        this.error = null
       },
 
-      err => this.error = err.message
+      err => {
+        if (err.status === 401) {
+          this.error = this.i18n('You current password is invalid.')
+          return
+        }
+
+        this.error = err.message
+      }
     )
   }
 }
diff --git a/client/src/app/shared/users/user.service.ts b/client/src/app/shared/users/user.service.ts
index fad5b0980..bd5cd45d4 100644
--- a/client/src/app/shared/users/user.service.ts
+++ b/client/src/app/shared/users/user.service.ts
@@ -17,9 +17,10 @@ export class UserService {
   ) {
   }
 
-  changePassword (newPassword: string) {
+  changePassword (currentPassword: string, newPassword: string) {
     const url = UserService.BASE_USERS_URL + 'me'
     const body: UserUpdateMe = {
+      currentPassword,
       password: newPassword
     }
 
diff --git a/server/controllers/api/users/me.ts b/server/controllers/api/users/me.ts
index ff3a87b7f..591ec6b25 100644
--- a/server/controllers/api/users/me.ts
+++ b/server/controllers/api/users/me.ts
@@ -87,7 +87,7 @@ meRouter.get('/me/videos/:videoId/rating',
 
 meRouter.put('/me',
   authenticate,
-  usersUpdateMeValidator,
+  asyncMiddleware(usersUpdateMeValidator),
   asyncRetryTransactionMiddleware(updateMe)
 )
 
diff --git a/server/middlewares/validators/users.ts b/server/middlewares/validators/users.ts
index d3ba1ae23..61297120a 100644
--- a/server/middlewares/validators/users.ts
+++ b/server/middlewares/validators/users.ts
@@ -22,6 +22,7 @@ import { Redis } from '../../lib/redis'
 import { UserModel } from '../../models/account/user'
 import { areValidationErrors } from './utils'
 import { ActorModel } from '../../models/activitypub/actor'
+import { comparePassword } from '../../helpers/peertube-crypto'
 
 const usersAddValidator = [
   body('username').custom(isUserUsernameValid).withMessage('Should have a valid username (lowercase alphanumeric characters)'),
@@ -137,15 +138,31 @@ const usersUpdateValidator = [
 const usersUpdateMeValidator = [
   body('displayName').optional().custom(isUserDisplayNameValid).withMessage('Should have a valid display name'),
   body('description').optional().custom(isUserDescriptionValid).withMessage('Should have a valid description'),
+  body('currentPassword').optional().custom(isUserPasswordValid).withMessage('Should have a valid current password'),
   body('password').optional().custom(isUserPasswordValid).withMessage('Should have a valid password'),
   body('email').optional().isEmail().withMessage('Should have a valid email attribute'),
   body('nsfwPolicy').optional().custom(isUserNSFWPolicyValid).withMessage('Should have a valid display Not Safe For Work policy'),
   body('autoPlayVideo').optional().custom(isUserAutoPlayVideoValid).withMessage('Should have a valid automatically plays video attribute'),
 
-  (req: express.Request, res: express.Response, next: express.NextFunction) => {
-    // TODO: Add old password verification
+  async (req: express.Request, res: express.Response, next: express.NextFunction) => {
     logger.debug('Checking usersUpdateMe parameters', { parameters: omit(req.body, 'password') })
 
+    if (req.body.password) {
+      if (!req.body.currentPassword) {
+        return res.status(400)
+                  .send({ error: 'currentPassword parameter is missing.' })
+                  .end()
+      }
+
+      const user: UserModel = res.locals.oauth.token.User
+
+      if (await user.isPasswordMatch(req.body.currentPassword) !== true) {
+        return res.status(401)
+                  .send({ error: 'currentPassword is invalid.' })
+                  .end()
+      }
+    }
+
     if (areValidationErrors(req, res)) return
 
     return next()
diff --git a/server/tests/api/check-params/users.ts b/server/tests/api/check-params/users.ts
index 95903c8a5..cbfa0c137 100644
--- a/server/tests/api/check-params/users.ts
+++ b/server/tests/api/check-params/users.ts
@@ -254,6 +254,7 @@ describe('Test users API validators', function () {
 
     it('Should fail with a too small password', async function () {
       const fields = {
+        currentPassword: 'my super password',
         password: 'bla'
       }
 
@@ -262,12 +263,31 @@ describe('Test users API validators', function () {
 
     it('Should fail with a too long password', async function () {
       const fields = {
+        currentPassword: 'my super password',
         password: 'super'.repeat(61)
       }
 
       await makePutBodyRequest({ url: server.url, path: path + 'me', token: userAccessToken, fields })
     })
 
+    it('Should fail without the current password', async function () {
+      const fields = {
+        currentPassword: 'my super password',
+        password: 'super'.repeat(61)
+      }
+
+      await makePutBodyRequest({ url: server.url, path: path + 'me', token: userAccessToken, fields })
+    })
+
+    it('Should fail with an invalid current password', async function () {
+      const fields = {
+        currentPassword: 'my super password fail',
+        password: 'super'.repeat(61)
+      }
+
+      await makePutBodyRequest({ url: server.url, path: path + 'me', token: userAccessToken, fields, statusCodeExpected: 401 })
+    })
+
     it('Should fail with an invalid NSFW policy attribute', async function () {
       const fields = {
         nsfwPolicy: 'hello'
@@ -286,6 +306,7 @@ describe('Test users API validators', function () {
 
     it('Should fail with an non authenticated user', async function () {
       const fields = {
+        currentPassword: 'my super password',
         password: 'my super password'
       }
 
@@ -300,8 +321,9 @@ describe('Test users API validators', function () {
       await makePutBodyRequest({ url: server.url, path: path + 'me', token: userAccessToken, fields })
     })
 
-    it('Should succeed with the correct params', async function () {
+    it('Should succeed to change password with the correct params', async function () {
       const fields = {
+        currentPassword: 'my super password',
         password: 'my super password',
         nsfwPolicy: 'blur',
         autoPlayVideo: false,
@@ -310,6 +332,16 @@ describe('Test users API validators', function () {
 
       await makePutBodyRequest({ url: server.url, path: path + 'me', token: userAccessToken, fields, statusCodeExpected: 204 })
     })
+
+    it('Should succeed without password change with the correct params', async function () {
+      const fields = {
+        nsfwPolicy: 'blur',
+        autoPlayVideo: false,
+        email: 'super_email@example.com'
+      }
+
+      await makePutBodyRequest({ url: server.url, path: path + 'me', token: userAccessToken, fields, statusCodeExpected: 204 })
+    })
   })
 
   describe('When updating my avatar', function () {
diff --git a/server/tests/api/users/users.ts b/server/tests/api/users/users.ts
index c0dd587ee..8b9c6b455 100644
--- a/server/tests/api/users/users.ts
+++ b/server/tests/api/users/users.ts
@@ -4,10 +4,34 @@ import * as chai from 'chai'
 import 'mocha'
 import { User, UserRole } from '../../../../shared/index'
 import {
-  createUser, flushTests, getBlacklistedVideosList, getMyUserInformation, getMyUserVideoQuotaUsed, getMyUserVideoRating,
-  getUserInformation, getUsersList, getUsersListPaginationAndSort, getVideosList, killallServers, login, makePutBodyRequest, rateVideo,
-  registerUser, removeUser, removeVideo, runServer, ServerInfo, testImage, updateMyAvatar, updateMyUser, updateUser, uploadVideo, userLogin,
-  deleteMe, blockUser, unblockUser, updateCustomSubConfig
+  blockUser,
+  createUser,
+  deleteMe,
+  flushTests,
+  getBlacklistedVideosList,
+  getMyUserInformation,
+  getMyUserVideoQuotaUsed,
+  getMyUserVideoRating,
+  getUserInformation,
+  getUsersList,
+  getUsersListPaginationAndSort,
+  getVideosList,
+  killallServers,
+  login,
+  makePutBodyRequest,
+  rateVideo,
+  registerUser,
+  removeUser,
+  removeVideo,
+  runServer,
+  ServerInfo,
+  testImage,
+  unblockUser,
+  updateMyAvatar,
+  updateMyUser,
+  updateUser,
+  uploadVideo,
+  userLogin
 } from '../../utils/index'
 import { follow } from '../../utils/server/follows'
 import { setAccessTokensToServers } from '../../utils/users/login'
@@ -302,6 +326,7 @@ describe('Test users', function () {
     await updateMyUser({
       url: server.url,
       accessToken: accessTokenUser,
+      currentPassword: 'super password',
       newPassword: 'new password'
     })
     user.password = 'new password'
diff --git a/server/tests/utils/users/users.ts b/server/tests/utils/users/users.ts
index cd1b07701..41d8ce265 100644
--- a/server/tests/utils/users/users.ts
+++ b/server/tests/utils/users/users.ts
@@ -162,6 +162,7 @@ function unblockUser (url: string, userId: number | string, accessToken: string,
 function updateMyUser (options: {
   url: string
   accessToken: string,
+  currentPassword?: string,
   newPassword?: string,
   nsfwPolicy?: NSFWPolicyType,
   email?: string,
@@ -172,6 +173,7 @@ function updateMyUser (options: {
   const path = '/api/v1/users/me'
 
   const toSend = {}
+  if (options.currentPassword !== undefined && options.currentPassword !== null) toSend['currentPassword'] = options.currentPassword
   if (options.newPassword !== undefined && options.newPassword !== null) toSend['password'] = options.newPassword
   if (options.nsfwPolicy !== undefined && options.nsfwPolicy !== null) toSend['nsfwPolicy'] = options.nsfwPolicy
   if (options.autoPlayVideo !== undefined && options.autoPlayVideo !== null) toSend['autoPlayVideo'] = options.autoPlayVideo