diff --git a/backend/openapi/components/requestBodies/internal/change-password-request.yaml b/backend/openapi/components/requestBodies/internal/change-password-request.yaml index fe06946..f46603c 100644 --- a/backend/openapi/components/requestBodies/internal/change-password-request.yaml +++ b/backend/openapi/components/requestBodies/internal/change-password-request.yaml @@ -5,7 +5,12 @@ content: schema: type: object properties: + currentPassword: + type: string + description: The current password of the user. + example: "currentPassword123" newPassword: type: string + minLength: 5 description: The new password for the user. example: "newSecurePassword123" diff --git a/backend/openapi/components/responses/internal/error-invalid-password.yaml b/backend/openapi/components/responses/internal/error-invalid-password.yaml new file mode 100644 index 0000000..a2464f1 --- /dev/null +++ b/backend/openapi/components/responses/internal/error-invalid-password.yaml @@ -0,0 +1,8 @@ +description: Bad Request — The current password is invalid +content: + application/json: + schema: + $ref: '../../schemas/error.yaml' + example: + error: 'Change Password Error' + message: 'Invalid current password' diff --git a/backend/openapi/paths/internal/users.yaml b/backend/openapi/paths/internal/users.yaml index 00d7f1d..24af572 100644 --- a/backend/openapi/paths/internal/users.yaml +++ b/backend/openapi/paths/internal/users.yaml @@ -28,6 +28,8 @@ responses: '200': $ref: '../../components/responses/internal/success-change-password.yaml' + '400': + $ref: '../../components/responses/internal/error-invalid-password.yaml' '401': $ref: '../../components/responses/unauthorized-error.yaml' '422': diff --git a/backend/src/controllers/user.controller.ts b/backend/src/controllers/user.controller.ts index fc44faf..f6717b1 100644 --- a/backend/src/controllers/user.controller.ts +++ b/backend/src/controllers/user.controller.ts @@ -24,11 +24,11 @@ export const changePassword = async (req: Request, res: Response) => { return rejectRequestFromMeetError(res, error); } - const { newPassword } = req.body as { newPassword: string }; + const { currentPassword, newPassword } = req.body as { currentPassword: string; newPassword: string }; try { const userService = container.get(UserService); - await userService.changePassword(user.username, newPassword); + await userService.changePassword(user.username, currentPassword, newPassword); return res.status(200).json({ message: 'Password changed successfully' }); } catch (error) { handleError(res, error, 'changing password'); diff --git a/backend/src/middlewares/request-validators/user-validator.middleware.ts b/backend/src/middlewares/request-validators/user-validator.middleware.ts index be3db47..37167cb 100644 --- a/backend/src/middlewares/request-validators/user-validator.middleware.ts +++ b/backend/src/middlewares/request-validators/user-validator.middleware.ts @@ -3,7 +3,8 @@ import { z } from 'zod'; import { rejectUnprocessableRequest } from '../../models/error.model.js'; const ChangePasswordRequestSchema = z.object({ - newPassword: z.string().min(4, 'New password must be at least 4 characters long') + currentPassword: z.string(), + newPassword: z.string().min(5, 'New password must be at least 5 characters long') }); export const validateChangePasswordRequest = (req: Request, res: Response, next: NextFunction) => { diff --git a/backend/src/models/error.model.ts b/backend/src/models/error.model.ts index 8905b13..5fc0f4d 100644 --- a/backend/src/models/error.model.ts +++ b/backend/src/models/error.model.ts @@ -72,6 +72,10 @@ export const errorInvalidCredentials = (): OpenViduMeetError => { return new OpenViduMeetError('Login Error', 'Invalid username or password', 404); }; +export const errorInvalidPassword = (): OpenViduMeetError => { + return new OpenViduMeetError('Change Password Error', 'Invalid current password', 400); +}; + export const errorUnauthorized = (): OpenViduMeetError => { return new OpenViduMeetError('Authentication Error', 'Unauthorized', 401); }; diff --git a/backend/src/services/user.service.ts b/backend/src/services/user.service.ts index 1cce99e..f0f462e 100644 --- a/backend/src/services/user.service.ts +++ b/backend/src/services/user.service.ts @@ -2,7 +2,7 @@ import { User, UserDTO, UserRole } from '@typings-ce'; import { inject, injectable } from 'inversify'; import INTERNAL_CONFIG from '../config/internal-config.js'; import { PasswordHelper } from '../helpers/password.helper.js'; -import { internalError } from '../models/error.model.js'; +import { errorInvalidPassword, internalError } from '../models/error.model.js'; import { MeetStorageService } from './index.js'; @injectable() @@ -29,13 +29,19 @@ export class UserService { }; } - async changePassword(username: string, newPassword: string) { + async changePassword(username: string, currentPassword: string, newPassword: string) { const user = await this.storageService.getUser(username); if (!user) { throw internalError(`getting user ${username} for password change`); } + const isCurrentPasswordValid = await PasswordHelper.verifyPassword(currentPassword, user.passwordHash); + + if (!isCurrentPasswordValid) { + throw errorInvalidPassword(); + } + user.passwordHash = await PasswordHelper.hashPassword(newPassword); await this.storageService.saveUser(user); } diff --git a/backend/tests/helpers/request-helpers.ts b/backend/tests/helpers/request-helpers.ts index 846f912..c3c4417 100644 --- a/backend/tests/helpers/request-helpers.ts +++ b/backend/tests/helpers/request-helpers.ts @@ -190,13 +190,13 @@ export const getProfile = async (cookie: string) => { .send(); }; -export const changePassword = async (newPassword: string, cookie: string) => { +export const changePassword = async (currentPassword: string, newPassword: string, cookie: string) => { checkAppIsRunning(); return await request(app) .post(`${INTERNAL_CONFIG.INTERNAL_API_BASE_PATH_V1}/users/change-password`) .set('Cookie', cookie) - .send({ newPassword }); + .send({ currentPassword, newPassword }); }; export const createRoom = async (options: MeetRoomOptions = {}): Promise => { diff --git a/backend/tests/integration/api/security/user-security.test.ts b/backend/tests/integration/api/security/user-security.test.ts index c4c93e3..419e67a 100644 --- a/backend/tests/integration/api/security/user-security.test.ts +++ b/backend/tests/integration/api/security/user-security.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeAll, describe, expect, it } from '@jest/globals'; +import { beforeAll, describe, expect, it } from '@jest/globals'; import { Express } from 'express'; import request from 'supertest'; import INTERNAL_CONFIG from '../../../../src/config/internal-config.js'; @@ -34,6 +34,7 @@ describe('User API Security Tests', () => { describe('Change Password Tests', () => { const changePasswordRequest = { + currentPassword: MEET_ADMIN_SECRET, newPassword: 'newpassword123' }; @@ -43,17 +44,15 @@ describe('User API Security Tests', () => { adminCookie = await loginUser(); }); - afterEach(async () => { - // Reset password - await changePassword(MEET_ADMIN_SECRET, adminCookie); - }); - it('should succeed when user is authenticated as admin', async () => { const response = await request(app) .post(`${USERS_PATH}/change-password`) .set('Cookie', adminCookie) .send(changePasswordRequest); expect(response.status).toBe(200); + + // Reset password + await changePassword(changePasswordRequest.newPassword, MEET_ADMIN_SECRET, adminCookie); }); it('should fail when user is not authenticated', async () => { diff --git a/backend/tests/integration/api/users/change-password.test.ts b/backend/tests/integration/api/users/change-password.test.ts index a3a9dc6..f19a814 100644 --- a/backend/tests/integration/api/users/change-password.test.ts +++ b/backend/tests/integration/api/users/change-password.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeAll, describe, expect, it } from '@jest/globals'; +import { beforeAll, describe, expect, it } from '@jest/globals'; import { MEET_ADMIN_SECRET } from '../../../../src/environment.js'; import { expectValidationError } from '../../../helpers/assertion-helpers.js'; import { changePassword, loginUser, startTestServer } from '../../../helpers/request-helpers.js'; @@ -11,21 +11,26 @@ describe('Users API Tests', () => { adminCookie = await loginUser(); }); - afterEach(async () => { - // Reset password - await changePassword(MEET_ADMIN_SECRET, adminCookie); - }); - describe('Change Password Tests', () => { it('should successfully change password', async () => { - const response = await changePassword('newpassword123', adminCookie); + const newPassword = 'newpassword123'; + const response = await changePassword(MEET_ADMIN_SECRET, newPassword, adminCookie); expect(response.status).toBe(200); expect(response.body).toHaveProperty('message', 'Password changed successfully'); + + // Reset password + await changePassword(newPassword, MEET_ADMIN_SECRET, adminCookie); }); - it('should fail when new password is not 4 characters long', async () => { - const response = await changePassword('123', adminCookie); - expectValidationError(response, 'newPassword', 'New password must be at least 4 characters long'); + it('should fail when current password is incorrect', async () => { + const response = await changePassword('wrongpassword', 'newpassword123', adminCookie); + expect(response.status).toBe(400); + expect(response.body).toHaveProperty('message', 'Invalid current password'); + }); + + it('should fail when new password is not 5 characters long', async () => { + const response = await changePassword(MEET_ADMIN_SECRET, '1234', adminCookie); + expectValidationError(response, 'newPassword', 'New password must be at least 5 characters long'); }); }); });