Skip to content
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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

#167706805 Enables an Already Logged In User To Change Their Password #35

Merged
merged 1 commit into from
Aug 9, 2019
Merged

#167706805 Enables an Already Logged In User To Change Their Password #35

merged 1 commit into from
Aug 9, 2019

Conversation

Adekoreday
Copy link
Contributor

@Adekoreday Adekoreday commented Aug 8, 2019 •
edited
Loading

What does this PR do?

This PR enables signed-in user to change their password

Description of Task proposed in this pull request?

A user controller responsible for updating the user's password and validations for password

How should this be manually tested (Quality Assurance)?

npm install
npm start:dev
patch localhost:5000/api/v1/users/changePassword
add the following in the request body

{
	"password": "mypassword",
	"confirmPassword": "mypassword"
}

What are the relevant pivotal tracker stories?

What I have learned working on this feature:

  • I learnt how to reset user password and validate user data

Screenshots:

Screenshot 2019-08-08 at 8 38 19 PM

Screenshot 2019-08-08 at 8 39 26 PM

@Adekoreday Adekoreday added pr:wip PR is still a work in progress type:feature A new feature labels Aug 8, 2019
@Adekoreday Adekoreday changed the title #167706805 Enables an Already Logged In User To Change Their Password in user can change their password #167706805 Enables an Already Logged In User To Change Their Password Aug 8, 2019
@Adekoreday Adekoreday force-pushed the ft-logged-in-user-reset-password-167706805 branch 2 times, most recently from af996cb to 1d1063d Compare August 8, 2019 22:57
@Adekoreday Adekoreday requested review from chialuka, henryade and abdulfatai360 and removed request for henryade August 8, 2019 23:18
@Adekoreday Adekoreday added pr:review This is PR is currently under review by team pr:wip PR is still a work in progress and removed pr:wip PR is still a work in progress pr:review This is PR is currently under review by team labels Aug 8, 2019
@Adekoreday Adekoreday force-pushed the ft-logged-in-user-reset-password-167706805 branch from 1d1063d to c18a397 Compare August 9, 2019 00:45
@Adekoreday Adekoreday added pr:ready This PR is ready to be merged pr:review This is PR is currently under review by team and removed pr:wip PR is still a work in progress pr:ready This PR is ready to be merged labels Aug 9, 2019
@Adekoreday Adekoreday force-pushed the ft-logged-in-user-reset-password-167706805 branch from c18a397 to 716052f Compare August 9, 2019 08:03
hound[bot]
hound bot reviewed Aug 9, 2019
View reviewed changes
test/users/index.js Show resolved Hide resolved
test/users/index.js Show resolved Hide resolved
@Adekoreday Adekoreday force-pushed the ft-logged-in-user-reset-password-167706805 branch 2 times, most recently from 653d912 to 193c038 Compare August 9, 2019 08:28
@Adekoreday Adekoreday added pr:wip PR is still a work in progress and removed pr:review This is PR is currently under review by team labels Aug 9, 2019
@Adekoreday Adekoreday force-pushed the ft-logged-in-user-reset-password-167706805 branch from 193c038 to f69ad60 Compare August 9, 2019 08:42
@Adekoreday Adekoreday added pr:review This is PR is currently under review by team and removed pr:wip PR is still a work in progress labels Aug 9, 2019
dinobi
dinobi reviewed Aug 9, 2019
View reviewed changes
server/controllers/Users.js
* @param {Object} res express response object
* @returns {JSON} JSON object with details of new user
*/
static async changePassword(req, res) {
Copy link
Contributor

@dinobi dinobi Aug 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is the check to ensure password and confirm_password match?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was handled in the middleware server/middlewares/passwordValidation.js

dinobi
dinobi reviewed Aug 9, 2019
View reviewed changes
server/routes/user.js Outdated

route.post('/create', validateUserSignup, Users.create);
route.get('/verifyEmail/:token', verifyToken, Users.verifyUserEmail);

route.patch(
'/changepassword',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why lower P?

dinobi
dinobi reviewed Aug 9, 2019
View reviewed changes
server/docs/authors-haven-api.yml Outdated
@@ -364,6 +364,34 @@ paths:
description: Success. Email verification was successful
500:
description: Internal server errorcomponents

/api/v1/users/changepassword:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update this

@Adekoreday Adekoreday force-pushed the ft-logged-in-user-reset-password-167706805 branch from f69ad60 to 46bf460 Compare August 9, 2019 10:53
@Adekoreday Adekoreday temporarily deployed to authors-haven-development August 9, 2019 11:29 Inactive
chialuka
chialuka previously approved these changes Aug 9, 2019
View reviewed changes
server/controllers/Users.js Outdated
static async changePassword(req, res) {
const { password } = req.body;
const { id } = req.user.dataValues;
await User.update({ password }, { where: { id } });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you should hash this password before saving it to the database. This way, when the user tries to login, the bcrypt.compare() function will be able to do a successful comparison.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I have hashed it thanks

@Adekoreday Adekoreday force-pushed the ft-logged-in-user-reset-password-167706805 branch from 46bf460 to fb4cadf Compare August 9, 2019 11:55
@chialuka chialuka self-requested a review August 9, 2019 12:07
chialuka
chialuka previously approved these changes Aug 9, 2019
View reviewed changes
ayodejiAA
ayodejiAA previously approved these changes Aug 9, 2019
View reviewed changes
@Adekoreday Adekoreday dismissed stale reviews from ayodejiAA and chialuka via ff03d37 August 9, 2019 12:45
@Adekoreday Adekoreday force-pushed the ft-logged-in-user-reset-password-167706805 branch from fb4cadf to ff03d37 Compare August 9, 2019 12:45
@chialuka chialuka self-requested a review August 9, 2019 12:56
chialuka
chialuka previously approved these changes Aug 9, 2019
View reviewed changes
henryade
henryade previously approved these changes Aug 9, 2019
View reviewed changes
ayodejiAA
ayodejiAA previously approved these changes Aug 9, 2019
View reviewed changes
feature(change Password):Logged in user can change their password
1823c9f 
Enables an already logged In user to reset their password

[Delivers #167706805]
@Adekoreday Adekoreday dismissed stale reviews from ayodejiAA, henryade, and chialuka via 1823c9f August 9, 2019 13:17
@Adekoreday Adekoreday force-pushed the ft-logged-in-user-reset-password-167706805 branch from ff03d37 to 1823c9f Compare August 9, 2019 13:17
@dinobi dinobi added pr:ready This PR is ready to be merged and removed pr:review This is PR is currently under review by team labels Aug 9, 2019
@dinobi dinobi merged commit 67d98c7 into develop Aug 9, 2019
ayodejiAA pushed a commit that referenced this pull request Aug 10, 2019
@Adekoreday @ayodejiAA
feature(change Password):Logged in user can change their password (#35)
5cca55e 
Enables an already logged In user to reset their password

[Delivers #167706805]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hound Hound Hound left review comments

@dinobi dinobi dinobi approved these changes

@chialuka chialuka chialuka left review comments

@ayodejiAA ayodejiAA ayodejiAA left review comments

@henryade henryade henryade left review comments

@abdulfatai360 abdulfatai360 Awaiting requested review from abdulfatai360

Assignees
No one assigned
Labels
pr:ready This PR is ready to be merged type:feature A new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Adekoreday @dinobi @chialuka @ayodejiAA @henryade