Password reset #464

Merged
bones7242 merged 3 commits from password-reset into master 2018-06-07 01:40:48 +02:00
2 changed files with 72 additions and 0 deletions

View file

@ -0,0 +1,69 @@
const { handleErrorResponse } = require('../../../utils/errorHandlers.js');
const logger = require('winston');
const db = require('../../../../models');
const { auth: { masterPassword } } = require('../../../../../config/siteConfig.js');
neb-b commented 2018-05-31 20:21:58 +02:00 (Migrated from github.com)
Review

This seems like it could be a security vulnerability?

This seems like it could be a security vulnerability?
bones7242 commented 2018-06-07 01:40:41 +02:00 (Migrated from github.com)
Review

As discussed in standup, the siteConfig will not be kept in version control (it is pulled from www.spee.ch's config which is gitignored). I'm going to merge.

As discussed in standup, the siteConfig will not be kept in version control (it is pulled from www.spee.ch's config which is gitignored). I'm going to merge.
/*
route to update a password
*/
const updateUserPassword = ({ ip, originalUrl, body }, res) => {
let userRecord;
const { userName, oldPassword, newPassword } = body;
if (!masterPassword) {
return res.status(400).json({
success: false,
message: 'no master password set in site config',
});
}
if (!userName || !oldPassword || !newPassword) {
return res.status(400).json({
success: false,
message: 'body should include userName (channel name without the @), oldPassword, & newPassword',
});
}
db.User.findOne({
where: {
userName,
},
})
.then(user => {
userRecord = user;
neb-b commented 2018-05-31 20:14:09 +02:00 (Migrated from github.com)
Review

Why don't you just use user here?

Why don't you just use `user` here?
bones7242 commented 2018-05-31 20:35:32 +02:00 (Migrated from github.com)
Review

I need the user object to be available in the next .then in the chain, so I hoisted it up to the updateUserPassword's scope by defining userRecord there. Is there a better pattern? I need the user object before I can check for a password match, and have to check for a password match before operating on that user object to change the password.

I need the `user` object to be available in the next `.then` in the chain, so I hoisted it up to the `updateUserPassword`'s scope by defining `userRecord` there. Is there a better pattern? I need the `user` object before I can check for a password match, and have to check for a password match before operating on that user object to change the password.
neb-b commented 2018-05-31 23:11:29 +02:00 (Migrated from github.com)
Review

Oh duh.

Oh duh.
if (!userRecord) {
throw new Error('no user found');
}
if (oldPassword === masterPassword) {
logger.debug('master password provided');
return true;
} else {
logger.debug('old password provided');
return userRecord.comparePassword(oldPassword);
}
})
.then(isMatch => {
if (!isMatch) {
throw new Error('Incorrect old password.');
}
logger.debug('Password was a match, updating password');
return userRecord.changePassword(newPassword);
})
.then(() => {
logger.debug('Password successfully updated');
return res.status(200).json({
success: true,
message: 'Password successfully updated',
oldPassword,
newPassword,
});
})
.catch((error) => {
handleErrorResponse(originalUrl, ip, error, res);
});
};
module.exports = updateUserPassword;

View file

@ -12,6 +12,7 @@ const claimPublish = require('../../controllers/api/claim/publish');
const claimResolve = require('../../controllers/api/claim/resolve');
const claimShortId = require('../../controllers/api/claim/shortId');
const fileAvailability = require('../../controllers/api/file/availability');
const userPassword = require('../../controllers/api/user/password');
const multipartMiddleware = require('../utils/multipartMiddleware');
@ -33,4 +34,6 @@ module.exports = (app) => {
app.get('/api/claim/short-id/:longId/:name', claimShortId);
// file routes
app.get('/api/file/availability/:name/:claimId', fileAvailability);
// user routes
app.put('/api/user/password/', userPassword);
};