github/overleaf
1
0
Fork 0
mirror of https://github.com/overleaf/overleaf.git synced 2024-11-07 20:31:06 -05:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #1944 from overleaf/em-password-reset

Browse source 
Store the email address in the password reset token data

GitOrigin-RevId: 9aa2eaff49de9ac88258cb996202934dab71cc0a
This commit is contained in:
Eric Mc Sween 2019-07-04 08:40:12 -04:00 committed by sharelatex
parent 02a4289422
commit a31090daab
4 changed files with 169 additions and 225 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

92
services/web/app/src/Features/PasswordReset/PasswordResetController.js
View file

@ -1,17 +1,3 @@
/* eslint-disable
camelcase,
handle-callback-err,
max-len,
no-useless-escape,
*/
// TODO: This file was created by bulk-decaffeinate.
// Fix any style issues and re-enable lint.
/*
* decaffeinate suggestions:
* DS102: Remove unnecessary code created because of implicit returns
* DS207: Consider shorter variations of null checks
* Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
*/
const PasswordResetHandler = require('./PasswordResetHandler')
const RateLimiter = require('../../infrastructure/RateLimiter')
const AuthenticationController = require('../Authentication/AuthenticationController')
@ -25,10 +11,10 @@ const Settings = require('settings-sharelatex')
module.exports = {
renderRequestResetForm(req, res) {
logger.log('rendering request reset form')
return res.render('user/passwordReset', { title: 'reset_password' })
res.render('user/passwordReset', { title: 'reset_password' })
},
requestReset(req, res) {
requestReset(req, res, next) {
const email = req.body.email.trim().toLowerCase()
const opts = {
endpointName: 'password_reset_rate_limit',
@ -36,36 +22,39 @@ module.exports = {
subjectName: req.ip,
throttle: 6
}
return RateLimiter.addCount(opts, function(err, canContinue) {
RateLimiter.addCount(opts, function(err, canContinue) {
if (err != null) {
return next(err)
}
if (!canContinue) {
return res.send(429, {
message: req.i18n.translate('rate_limit_hit_wait')
})
}
return PasswordResetHandler.generateAndEmailResetToken(email, function(
PasswordResetHandler.generateAndEmailResetToken(email, function(
err,
status
) {
if (err != null) {
return res.send(500, {
message: err != null ? err.message : undefined
res.send(500, {
message: err.message
})
} else if (status === 'primary') {
return res.send(200, {
res.send(200, {
message: { text: req.i18n.translate('password_reset_email_sent') }
})
} else if (status === 'secondary') {
return res.send(404, {
res.send(404, {
message: req.i18n.translate('secondary_email_password_reset')
})
} else if (status === 'sharelatex') {
return res.send(404, {
message: `<a href=\"${
res.send(404, {
message: `<a href="${
Settings.accountMerge.sharelatexHost
}/user/password/reset\">${req.i18n.translate('reset_from_sl')}</a>`
}/user/password/reset">${req.i18n.translate('reset_from_sl')}</a>`
})
} else {
return res.send(404, {
res.send(404, {
message: req.i18n.translate('cant_find_email')
})
}
@ -81,7 +70,7 @@ module.exports = {
if (req.session.resetToken == null) {
return res.redirect('/user/password/reset')
}
return res.render('user/setPassword', {
res.render('user/setPassword', {
title: 'set_password',
passwordResetToken: req.session.resetToken
})
@ -90,55 +79,48 @@ module.exports = {
setNewUserPassword(req, res, next) {
const { passwordResetToken, password } = req.body
if (
password == null ||
password.length === 0 ||
passwordResetToken == null ||
passwordResetToken.length === 0 ||
AuthenticationManager.validatePassword(
password != null ? password.trim() : undefined
) != null
!password ||
!passwordResetToken ||
AuthenticationManager.validatePassword(password.trim()) != null
) {
return res.sendStatus(400)
}
delete req.session.resetToken
return PasswordResetHandler.setNewUserPassword(
passwordResetToken != null ? passwordResetToken.trim() : undefined,
password != null ? password.trim() : undefined,
function(err, found, user_id) {
PasswordResetHandler.setNewUserPassword(
passwordResetToken.trim(),
password.trim(),
function(err, found, userId) {
if (err && err.name && err.name === 'NotFoundError') {
return res.status(404).send('NotFoundError')
res.status(404).send('NotFoundError')
} else if (err && err.name && err.name === 'NotInV2Error') {
return res.status(403).send('NotInV2Error')
res.status(403).send('NotInV2Error')
} else if (err && err.name && err.name === 'SLInV2Error') {
return res.status(403).send('SLInV2Error')
res.status(403).send('SLInV2Error')
} else if (err && err.statusCode && err.statusCode === 500) {
return res.status(500)
res.status(500)
} else if (err && !err.statusCode) {
return res.status(500)
res.status(500)
} else if (found) {
if (user_id == null) {
if (userId == null) {
return res.sendStatus(200)
} // will not exist for v1-only users
return UserSessionsManager.revokeAllUserSessions(
{ _id: user_id },
UserSessionsManager.revokeAllUserSessions(
{ _id: userId },
[],
function(err) {
if (err != null) {
return next(err)
}
return UserUpdater.removeReconfirmFlag(user_id, function(err) {
UserUpdater.removeReconfirmFlag(userId, function(err) {
if (err != null) {
return next(err)
}
if (req.body.login_after) {
return UserGetter.getUser(user_id, { email: 1 }, function(
err,
user
) {
UserGetter.getUser(userId, { email: 1 }, function(err, user) {
if (err != null) {
return next(err)
}
return AuthenticationController.afterLoginSessionSetup(
AuthenticationController.afterLoginSessionSetup(
req,
user,
function(err) {
@ -149,7 +131,7 @@ module.exports = {
)
return next(err)
}
return res.json({
res.json({
redir:
AuthenticationController._getRedirectFromSession(
req
@ -159,13 +141,13 @@ module.exports = {
)
})
} else {
return res.sendStatus(200)
res.sendStatus(200)
}
})
}
)
} else {
return res.sendStatus(404)
res.sendStatus(404)
}
}
)

167
services/web/app/src/Features/PasswordReset/PasswordResetHandler.js
View file

@ -1,20 +1,4 @@
/* eslint-disable
camelcase,
handle-callback-err,
max-len,
no-unused-vars,
*/
// TODO: This file was created by bulk-decaffeinate.
// Fix any style issues and re-enable lint.
/*
* decaffeinate suggestions:
* DS102: Remove unnecessary code created because of implicit returns
* DS207: Consider shorter variations of null checks
* Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
*/
let PasswordResetHandler
const settings = require('settings-sharelatex')
const async = require('async')
const UserGetter = require('../User/UserGetter')
const OneTimeTokenHandler = require('../Security/OneTimeTokenHandler')
const EmailHandler = require('../Email/EmailHandler')
@ -22,23 +6,17 @@ const AuthenticationManager = require('../Authentication/AuthenticationManager')
const logger = require('logger-sharelatex')
const V1Api = require('../V1/V1Api')
module.exports = PasswordResetHandler = {
const PasswordResetHandler = {
generateAndEmailResetToken(email, callback) {
if (callback == null) {
callback = function(error, status) {}
}
return PasswordResetHandler._getPasswordResetData(email, function(
PasswordResetHandler._getPasswordResetData(email, function(
error,
exists,
data
) {
if (error != null) {
return callback(error, null)
callback(error)
} else if (exists) {
return OneTimeTokenHandler.getNewToken('password', data, function(
err,
token
) {
OneTimeTokenHandler.getNewToken('password', data, function(err, token) {
if (err) {
return callback(err)
}
@ -50,27 +28,28 @@ module.exports = PasswordResetHandler = {
email
)}`
}
return EmailHandler.sendEmail(
EmailHandler.sendEmail(
'passwordResetRequested',
emailOptions,
function(error) {
if (error != null) {
return callback(error)
}
return callback(null, 'primary')
callback(null, 'primary')
}
)
})
} else {
return UserGetter.getUserByAnyEmail(email, function(err, user) {
UserGetter.getUserByAnyEmail(email, function(err, user) {
if (err != null) {
return callback(err)
}
if (!user) {
return callback(error, null)
} else if (
(user.overleaf != null ? user.overleaf.id : undefined) == null
) {
return callback(error, 'sharelatex')
callback(null, null)
} else if (user.overleaf == null || user.overleaf.id == null) {
callback(null, 'sharelatex')
} else {
return callback(error, 'secondary')
callback(null, 'secondary')
}
})
}
@ -78,72 +57,60 @@ module.exports = PasswordResetHandler = {
},
setNewUserPassword(token, password, callback) {
if (callback == null) {
callback = function(error, found, user_id) {}
}
return OneTimeTokenHandler.getValueFromTokenAndExpire(
'password',
token,
function(err, data) {
if (err) {
return callback(err)
}
if (data == null) {
return callback(null, false, null)
}
if (typeof data === 'string') {
// Backwards compatible with old format.
// Tokens expire after 1h, so this can be removed soon after deploy.
// Possibly we should keep this until we do an onsite release too.
data = { user_id: data }
}
if (data.user_id != null) {
return AuthenticationManager.setUserPassword(
data.user_id,
password,
function(err, reset) {
if (err) {
return callback(err)
}
return callback(null, reset, data.user_id)
}
)
} else if (data.v1_user_id != null) {
return AuthenticationManager.setUserPasswordInV1(
data.v1_user_id,
password,
function(error, reset) {
if (error != null) {
return callback(error)
}
return UserGetter.getUser(
{ 'overleaf.id': data.v1_user_id },
{ _id: 1 },
function(error, user) {
if (error != null) {
return callback(error)
}
return callback(
null,
reset,
user != null ? user._id : undefined
)
}
)
}
)
}
OneTimeTokenHandler.getValueFromTokenAndExpire('password', token, function(
err,
data
) {
if (err) {
return callback(err)
}
)
if (data == null) {
return callback(null, false, null)
}
if (typeof data === 'string') {
// Backwards compatible with old format.
// Tokens expire after 1h, so this can be removed soon after deploy.
// Possibly we should keep this until we do an onsite release too.
data = { user_id: data }
}
if (data.user_id != null) {
AuthenticationManager.setUserPassword(data.user_id, password, function(
err,
reset
) {
if (err) {
return callback(err)
}
callback(null, reset, data.user_id)
})
} else if (data.v1_user_id != null) {
AuthenticationManager.setUserPasswordInV1(
data.v1_user_id,
password,
function(error, reset) {
if (error != null) {
return callback(error)
}
UserGetter.getUser(
{ 'overleaf.id': data.v1_user_id },
{ _id: 1 },
function(error, user) {
if (error != null) {
return callback(error)
}
callback(null, reset, user != null ? user._id : undefined)
}
)
}
)
}
})
},
_getPasswordResetData(email, callback) {
if (callback == null) {
callback = function(error, exists, data) {}
}
if (settings.overleaf != null) {
// Overleaf v2
return V1Api.request(
V1Api.request(
{
url: '/api/v1/sharelatex/user_emails',
qs: {
@ -156,15 +123,15 @@ module.exports = PasswordResetHandler = {
return callback(error)
}
if (response.statusCode === 404) {
return callback(null, false)
callback(null, false)
} else {
return callback(null, true, { v1_user_id: body.user_id })
callback(null, true, { v1_user_id: body.user_id, email: email })
}
}
)
} else {
// ShareLaTeX
return UserGetter.getUserByMainEmail(email, function(err, user) {
UserGetter.getUserByMainEmail(email, function(err, user) {
if (err) {
return callback(err)
}
@ -172,8 +139,10 @@ module.exports = PasswordResetHandler = {
logger.err({ email }, 'user could not be found for password reset')
return callback(null, false)
}
return callback(null, true, { user_id: user._id })
callback(null, true, { user_id: user._id, email: email })
})
}
}
}
module.exports = PasswordResetHandler

15
services/web/app/src/Features/PasswordReset/PasswordResetRouter.js
View file

@ -1,13 +1,3 @@
/* eslint-disable
max-len,
*/
// TODO: This file was created by bulk-decaffeinate.
// Fix any style issues and re-enable lint.
/*
* decaffeinate suggestions:
* DS102: Remove unnecessary code created because of implicit returns
* Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
*/
const PasswordResetController = require('./PasswordResetController')
const AuthenticationController = require('../Authentication/AuthenticationController')
@ -30,9 +20,6 @@ module.exports = {
)
AuthenticationController.addEndpointToLoginWhitelist('/user/password/set')
return webRouter.post(
'/user/reconfirm',
PasswordResetController.requestReset
)
webRouter.post('/user/reconfirm', PasswordResetController.requestReset)
}
}

120
services/web/test/unit/src/PasswordReset/PasswordResetHandlerTests.js
View file

@ -1,26 +1,11 @@
/* eslint-disable
handle-callback-err,
max-len,
no-return-assign,
no-unused-vars,
*/
// TODO: This file was created by bulk-decaffeinate.
// Fix any style issues and re-enable lint.
/*
* decaffeinate suggestions:
* DS102: Remove unnecessary code created because of implicit returns
* Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
*/
const should = require('chai').should()
const SandboxedModule = require('sandboxed-module')
const assert = require('assert')
const path = require('path')
const sinon = require('sinon')
const modulePath = path.join(
__dirname,
'../../../../app/src/Features/PasswordReset/PasswordResetHandler'
)
const { expect } = require('chai')
describe('PasswordResetHandler', function() {
beforeEach(function() {
@ -57,9 +42,10 @@ describe('PasswordResetHandler', function() {
})
this.token = '12312321i'
this.user_id = 'user_id_here'
this.user = { email: (this.email = 'bob@bob.com') }
this.email = 'bob@bob.com'
this.user = { _id: this.user_id, email: this.email }
this.password = 'my great secret password'
return (this.callback = sinon.stub())
this.callback = sinon.stub()
})
describe('generateAndEmailResetToken', function() {
@ -68,11 +54,14 @@ describe('PasswordResetHandler', function() {
this.UserGetter.getUserByMainEmail.callsArgWith(1)
this.UserGetter.getUserByAnyEmail.callsArgWith(1)
this.OneTimeTokenHandler.getNewToken.yields()
return this.PasswordResetHandler.generateAndEmailResetToken(
this.PasswordResetHandler.generateAndEmailResetToken(
this.user.email,
(err, status) => {
if (err) {
return done(err)
}
should.equal(status, null)
return done()
done()
}
)
})
@ -81,10 +70,20 @@ describe('PasswordResetHandler', function() {
this.UserGetter.getUserByMainEmail.callsArgWith(1, null, this.user)
this.OneTimeTokenHandler.getNewToken.yields(null, this.token)
this.EmailHandler.sendEmail.callsArgWith(2)
return this.PasswordResetHandler.generateAndEmailResetToken(
this.PasswordResetHandler.generateAndEmailResetToken(
this.user.email,
(err, status) => {
if (err) {
return done(err)
}
this.EmailHandler.sendEmail.called.should.equal(true)
this.OneTimeTokenHandler.getNewToken.should.have.been.calledWith(
'password',
{
user_id: this.user_id,
email: this.email
}
)
status.should.equal('primary')
const args = this.EmailHandler.sendEmail.args[0]
args[0].should.equal('passwordResetRequested')
@ -93,7 +92,7 @@ describe('PasswordResetHandler', function() {
this.token
}&email=${encodeURIComponent(this.user.email)}`
)
return done()
done()
}
)
})
@ -103,19 +102,28 @@ describe('PasswordResetHandler', function() {
this.UserGetter.getUserByMainEmail.callsArgWith(1, null, this.user)
this.UserGetter.getUserByAnyEmail.callsArgWith(1)
this.OneTimeTokenHandler.getNewToken.yields()
return this.PasswordResetHandler.generateAndEmailResetToken(
this.PasswordResetHandler.generateAndEmailResetToken(
this.user.email,
(err, status) => {
if (err) {
return done(err)
}
should.equal(status, null)
return done()
done()
}
)
})
it('should set the password token data to the user id and email', function() {
this.UserGetter.getUserByMainEmail.callsArgWith(1, null, this.user)
this.OneTimeTokenHandler.getNewToken.yields(null, this.token)
this.EmailHandler.sendEmail.callsArgWith(2)
})
})
describe('when in overleaf', function() {
beforeEach(function() {
return (this.settings.overleaf = true)
this.settings.overleaf = true
})
describe('when the email exists', function() {
@ -123,14 +131,14 @@ describe('PasswordResetHandler', function() {
this.V1Api.request.yields(null, {}, { user_id: 42 })
this.OneTimeTokenHandler.getNewToken.yields(null, this.token)
this.EmailHandler.sendEmail.yields()
return this.PasswordResetHandler.generateAndEmailResetToken(
this.PasswordResetHandler.generateAndEmailResetToken(
this.email,
this.callback
)
})
it('should call the v1 api for the user', function() {
return this.V1Api.request
this.V1Api.request
.calledWith({
url: '/api/v1/sharelatex/user_emails',
qs: {
@ -142,18 +150,20 @@ describe('PasswordResetHandler', function() {
})
it('should set the password token data to the user id and email', function() {
return this.OneTimeTokenHandler.getNewToken
.calledWith('password', {
v1_user_id: 42
})
.should.equal(true)
this.OneTimeTokenHandler.getNewToken.should.have.been.calledWith(
'password',
{
v1_user_id: 42,
email: this.email
}
)
})
it('should send an email with the token', function() {
this.EmailHandler.sendEmail.called.should.equal(true)
const args = this.EmailHandler.sendEmail.args[0]
args[0].should.equal('passwordResetRequested')
return args[1].setNewPasswordUrl.should.equal(
args[1].setNewPasswordUrl.should.equal(
`${this.settings.siteUrl}/user/password/set?passwordResetToken=${
this.token
}&email=${encodeURIComponent(this.user.email)}`
@ -161,7 +171,7 @@ describe('PasswordResetHandler', function() {
})
it('should return status == true', function() {
return this.callback.calledWith(null, 'primary').should.equal(true)
this.callback.calledWith(null, 'primary').should.equal(true)
})
})
@ -171,22 +181,22 @@ describe('PasswordResetHandler', function() {
.stub()
.yields(null, { statusCode: 404 }, {})
this.UserGetter.getUserByAnyEmail.callsArgWith(1)
return this.PasswordResetHandler.generateAndEmailResetToken(
this.PasswordResetHandler.generateAndEmailResetToken(
this.email,
this.callback
)
})
it('should not set the password token data', function() {
return this.OneTimeTokenHandler.getNewToken.called.should.equal(false)
this.OneTimeTokenHandler.getNewToken.called.should.equal(false)
})
it('should send an email with the token', function() {
return this.EmailHandler.sendEmail.called.should.equal(false)
this.EmailHandler.sendEmail.called.should.equal(false)
})
it('should return status == null', function() {
return this.callback.calledWith(null, null).should.equal(true)
this.callback.calledWith(null, null).should.equal(true)
})
})
@ -196,22 +206,22 @@ describe('PasswordResetHandler', function() {
.stub()
.yields(null, { statusCode: 404 }, {})
this.UserGetter.getUserByAnyEmail.callsArgWith(1, null, this.user)
return this.PasswordResetHandler.generateAndEmailResetToken(
this.PasswordResetHandler.generateAndEmailResetToken(
this.email,
this.callback
)
})
it('should not set the password token data', function() {
return this.OneTimeTokenHandler.getNewToken.called.should.equal(false)
this.OneTimeTokenHandler.getNewToken.called.should.equal(false)
})
it('should not send an email with the token', function() {
return this.EmailHandler.sendEmail.called.should.equal(false)
this.EmailHandler.sendEmail.called.should.equal(false)
})
it('should return status == sharelatex', function() {
return this.callback.calledWith(null, 'sharelatex').should.equal(true)
this.callback.calledWith(null, 'sharelatex').should.equal(true)
})
})
@ -222,22 +232,22 @@ describe('PasswordResetHandler', function() {
.yields(null, { statusCode: 404 }, {})
this.user.overleaf = { id: 101 }
this.UserGetter.getUserByAnyEmail.callsArgWith(1, null, this.user)
return this.PasswordResetHandler.generateAndEmailResetToken(
this.PasswordResetHandler.generateAndEmailResetToken(
this.email,
this.callback
)
})
it('should not set the password token data', function() {
return this.OneTimeTokenHandler.getNewToken.called.should.equal(false)
this.OneTimeTokenHandler.getNewToken.called.should.equal(false)
})
it('should not send an email with the token', function() {
return this.EmailHandler.sendEmail.called.should.equal(false)
this.EmailHandler.sendEmail.called.should.equal(false)
})
it('should return status == secondary', function() {
return this.callback.calledWith(null, 'secondary').should.equal(true)
this.callback.calledWith(null, 'secondary').should.equal(true)
})
})
})
@ -247,7 +257,7 @@ describe('PasswordResetHandler', function() {
describe('when no data is found', function() {
beforeEach(function() {
this.OneTimeTokenHandler.getValueFromTokenAndExpire.yields(null, null)
return this.PasswordResetHandler.setNewUserPassword(
this.PasswordResetHandler.setNewUserPassword(
this.token,
this.password,
this.callback
@ -255,7 +265,7 @@ describe('PasswordResetHandler', function() {
})
it('should return exists == false', function() {
return this.callback.calledWith(null, false).should.equal(true)
this.callback.calledWith(null, false).should.equal(true)
})
})
@ -270,7 +280,7 @@ describe('PasswordResetHandler', function() {
null,
this.user_id
)
return this.PasswordResetHandler.setNewUserPassword(
this.PasswordResetHandler.setNewUserPassword(
this.token,
this.password,
this.callback
@ -278,15 +288,13 @@ describe('PasswordResetHandler', function() {
})
it('should call setUserPasswordInV2', function() {
return this.AuthenticationManager.setUserPassword
this.AuthenticationManager.setUserPassword
.calledWith(this.user_id, this.password)
.should.equal(true)
})
it('should reset == true and the user_id', function() {
return this.callback
.calledWith(null, true, this.user_id)
.should.equal(true)
this.callback.calledWith(null, true, this.user_id).should.equal(true)
})
})
@ -300,7 +308,7 @@ describe('PasswordResetHandler', function() {
this.OneTimeTokenHandler.getValueFromTokenAndExpire.yields(null, {
user_id: this.user_id
})
return this.PasswordResetHandler.setNewUserPassword(
this.PasswordResetHandler.setNewUserPassword(
this.token,
this.password,
this.callback
@ -308,15 +316,13 @@ describe('PasswordResetHandler', function() {
})
it('should call setUserPasswordInV2', function() {
return this.AuthenticationManager.setUserPassword
this.AuthenticationManager.setUserPassword
.calledWith(this.user_id, this.password)
.should.equal(true)
})
it('should reset == true and the user_id', function() {
return this.callback
.calledWith(null, true, this.user_id)
.should.equal(true)
this.callback.calledWith(null, true, this.user_id).should.equal(true)
})
})
})