From a890d1e0d30851741392e6e7f14acffe685d28e0 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 26 Sep 2018 16:28:15 +0200 Subject: Check current password on server side --- .../my-account-change-password.component.html | 10 +++---- .../my-account-change-password.component.ts | 33 +++++++++------------ client/src/app/shared/users/user.service.ts | 3 +- server/controllers/api/users/me.ts | 2 +- server/middlewares/validators/users.ts | 21 +++++++++++-- server/tests/api/check-params/users.ts | 34 +++++++++++++++++++++- server/tests/api/users/users.ts | 33 ++++++++++++++++++--- server/tests/utils/users/users.ts | 2 ++ 8 files changed, 105 insertions(+), 33 deletions(-) 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 @@
{{ error }}
-
+ -
- {{ formErrors['old-password'] }} +
+ {{ formErrors['current-password'] }}
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 -- cgit v1.2.3