Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds a new change-password endpoint that validates the old password, hashes the new one, and updates the user record.
- Introduces
validationChangePasswordto enforce required fields and verify the current password. - Exposes the validator in the validations index and integrates it into the REST API.
- Extends the API model with
ChangePasswordParamsand adds a new internal error code.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pods/user/validations/password-validation.ts | New validationChangePassword function with field checks and hash verification. |
| src/pods/user/validations/index.ts | Exports the new validationChangePassword validator. |
| src/pods/user/user.api-model.ts | Defines ChangePasswordParams interface. |
| src/common/custom-error/custom-error.model.ts | Adds InvalidPassword to CustomInternalCodes. |
| src/pods/user/user.rest-api.ts | Adds /change-password route, applies validation, hashing, and status mapping. |
Comments suppressed due to low confidence (2)
src/pods/user/user.api-model.ts:33
- [nitpick] Using Spanish identifiers with accents (e.g.,
contraseñaActual) may cause encoding or consistency issues; consider using English names likecurrentPassword.
contraseñaActual: string;
src/pods/user/user.rest-api.ts:120
- The new
/change-passwordendpoint and its validation logic lack accompanying tests; consider adding unit and integration tests to cover the success and error flows.
.post('/change-password', async (req, res, next) => {
| return { succeded: false, error: { error: CustomInternalCodes.FieldNotInformed } }; | ||
| } | ||
|
|
||
| const user = await userRepository.getUser(passwordData.id); | ||
| if (!user) { | ||
| return { succeded: false, error: { error: CustomInternalCodes.UserNotFound } }; | ||
| } | ||
|
|
||
| const isValidPassword = await verifyHash(passwordData.contraseñaActual, user.contraseña); | ||
| if (!isValidPassword) { | ||
| return { succeded: false, error: { error: CustomInternalCodes.InvalidPassword } }; | ||
| } | ||
|
|
||
| return { succeded: true }; |
There was a problem hiding this comment.
The property succeded appears misspelled; it should likely be succeeded to match common spelling and avoid mismatches with the ValidationInfo interface.
| return { succeded: false, error: { error: CustomInternalCodes.FieldNotInformed } }; | |
| } | |
| const user = await userRepository.getUser(passwordData.id); | |
| if (!user) { | |
| return { succeded: false, error: { error: CustomInternalCodes.UserNotFound } }; | |
| } | |
| const isValidPassword = await verifyHash(passwordData.contraseñaActual, user.contraseña); | |
| if (!isValidPassword) { | |
| return { succeded: false, error: { error: CustomInternalCodes.InvalidPassword } }; | |
| } | |
| return { succeded: true }; | |
| return { succeeded: false, error: { error: CustomInternalCodes.FieldNotInformed } }; | |
| } | |
| const user = await userRepository.getUser(passwordData.id); | |
| if (!user) { | |
| return { succeeded: false, error: { error: CustomInternalCodes.UserNotFound } }; | |
| } | |
| const isValidPassword = await verifyHash(passwordData.contraseñaActual, user.contraseña); | |
| if (!isValidPassword) { | |
| return { succeeded: false, error: { error: CustomInternalCodes.InvalidPassword } }; | |
| } | |
| return { succeeded: true }; |
|
|
||
| if (validationResult.succeded) { | ||
| const hashedPassword = await hash(passwordData.nuevaContraseña); | ||
| const user = await userRepository.getUser(passwordData.id); |
There was a problem hiding this comment.
[nitpick] This call fetches the user again after validation already retrieved it; you could return the user from the validation step to reduce an extra database query.
| res.sendStatus(204); | ||
| } else { | ||
| const statusCode = (() => { | ||
| switch (validationResult.error?.error) { |
There was a problem hiding this comment.
The FieldNotInformed case currently falls through to the default status 400; mapping it explicitly to 422 (like UserNotFound) may better communicate input validation errors.
Closed #38