- Notifications
You must be signed in to change notification settings - Fork2
feature/#38 Added reset password endpoint#39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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 | NewvalidationChangePassword function with field checks and hash verification. |
| src/pods/user/validations/index.ts | Exports the newvalidationChangePassword validator. |
| src/pods/user/user.api-model.ts | DefinesChangePasswordParams interface. |
| src/common/custom-error/custom-error.model.ts | AddsInvalidPassword toCustomInternalCodes. |
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The propertysucceded appears misspelled; it should likely besucceeded to match common spelling and avoid mismatches with theValidationInfo interface.
| return{succeded:false,error:{error:CustomInternalCodes.FieldNotInformed}}; | |
| } | |
| constuser=awaituserRepository.getUser(passwordData.id); | |
| if(!user){ | |
| return{succeded:false,error:{error:CustomInternalCodes.UserNotFound}}; | |
| } | |
| constisValidPassword=awaitverifyHash(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}}; | |
| } | |
| constuser=awaituserRepository.getUser(passwordData.id); | |
| if(!user){ | |
| return{succeeded:false,error:{error:CustomInternalCodes.UserNotFound}}; | |
| } | |
| constisValidPassword=awaitverifyHash(passwordData.contraseñaActual,user.contraseña); | |
| if(!isValidPassword){ | |
| return{succeeded:false,error:{error:CustomInternalCodes.InvalidPassword}}; | |
| } | |
| return{succeeded:true}; |
Copilot uses AI. Check for mistakes.
| if (validationResult.succeded) { | ||
| const hashedPassword = await hash(passwordData.nuevaContraseña); | ||
| const user = await userRepository.getUser(passwordData.id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
[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.
Copilot uses AI. Check for mistakes.
| res.sendStatus(204); | ||
| } else { | ||
| const statusCode = (() => { | ||
| switch (validationResult.error?.error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
TheFieldNotInformed case currently falls through to the default status 400; mapping it explicitly to 422 (likeUserNotFound) may better communicate input validation errors.
Copilot uses AI. Check for mistakes.
Closed#38