diff --git a/src/mailer.js b/src/mailer.js index b86a05b78..f9f099017 100644 --- a/src/mailer.js +++ b/src/mailer.js @@ -202,7 +202,7 @@ function sendInvite(user, invitor) { var templateData = { user: user, webadminUrl: config.adminOrigin(), - setupLink: config.adminOrigin() + '/api/v1/session/account/setup.html?reset_token=' + user.resetToken, + setupLink: `${config.adminOrigin()}/api/v1/session/account/setup.html?reset_token=${user.resetToken}&email=${user.email}`, invitor: invitor, cloudronName: mailConfig.cloudronName, cloudronAvatarUrl: config.adminOrigin() + '/api/v1/cloudron/avatar' @@ -289,7 +289,7 @@ function passwordReset(user) { var templateData = { user: user, - resetLink: config.adminOrigin() + '/api/v1/session/password/reset.html?reset_token=' + user.resetToken, + resetLink: `${config.adminOrigin()}/api/v1/session/password/reset.html?reset_token=${user.resetToken}&email=${user.email}`, cloudronName: mailConfig.cloudronName, cloudronAvatarUrl: config.adminOrigin() + '/api/v1/cloudron/avatar' }; diff --git a/src/oauth2views/account_setup.ejs b/src/oauth2views/account_setup.ejs index 59045b5ad..8a9ac61d3 100644 --- a/src/oauth2views/account_setup.ejs +++ b/src/oauth2views/account_setup.ejs @@ -29,6 +29,7 @@ app.controller('Controller', ['$scope', function ($scope) {
+

<%= error %>

diff --git a/src/oauth2views/password_reset.ejs b/src/oauth2views/password_reset.ejs index 404a5c36a..39f57897b 100644 --- a/src/oauth2views/password_reset.ejs +++ b/src/oauth2views/password_reset.ejs @@ -26,6 +26,7 @@ app.controller('Controller', [function () {}]); +
diff --git a/src/routes/oauth2.js b/src/routes/oauth2.js index 471237031..25ab4c6e8 100644 --- a/src/routes/oauth2.js +++ b/src/routes/oauth2.js @@ -327,7 +327,7 @@ function passwordResetRequestSite(req, res) { function passwordResetRequest(req, res, next) { assert.strictEqual(typeof req.body, 'object'); - if (typeof req.body.identifier !== 'string') return next(new HttpError(400, 'Missing identifier')); + if (typeof req.body.identifier !== 'string') return next(new HttpError(400, 'Missing identifier')); // email or username debug('passwordResetRequest: email or username %s.', req.body.identifier); @@ -352,6 +352,7 @@ function renderAccountSetupSite(res, req, userObject, error) { error: error, csrf: req.csrfToken(), resetToken: req.query.reset_token || req.body.resetToken, + email: req.query.email || req.body.email, title: 'Password Setup' }); } @@ -359,9 +360,10 @@ function renderAccountSetupSite(res, req, userObject, error) { // -> GET /api/v1/session/account/setup.html function accountSetupSite(req, res) { if (!req.query.reset_token) return sendError(req, res, 'Missing Reset Token'); + if (!req.query.email) return sendError(req, res, 'Missing Email'); - users.getByResetToken(req.query.reset_token, function (error, userObject) { - if (error) return sendError(req, res, 'Invalid Reset Token'); + users.getByResetToken(req.query.email, req.query.reset_token, function (error, userObject) { + if (error) return sendError(req, res, 'Invalid Email or Reset Token'); renderAccountSetupSite(res, req, userObject, ''); }); @@ -371,14 +373,15 @@ function accountSetupSite(req, res) { function accountSetup(req, res, next) { assert.strictEqual(typeof req.body, 'object'); + if (typeof req.body.email !== 'string') return next(new HttpError(400, 'Missing email')); if (typeof req.body.resetToken !== 'string') return next(new HttpError(400, 'Missing resetToken')); if (typeof req.body.password !== 'string') return next(new HttpError(400, 'Missing password')); if (typeof req.body.username !== 'string') return next(new HttpError(400, 'Missing username')); if (typeof req.body.displayName !== 'string') return next(new HttpError(400, 'Missing displayName')); - debug('acountSetup: with token %s.', req.body.resetToken); + debug(`acountSetup: for email ${req.body.email} with token ${req.body.resetToken}`); - users.getByResetToken(req.body.resetToken, function (error, userObject) { + users.getByResetToken(req.body.email, req.body.resetToken, function (error, userObject) { if (error) return sendError(req, res, 'Invalid Reset Token'); var data = _.pick(req.body, 'username', 'displayName'); @@ -405,15 +408,17 @@ function accountSetup(req, res, next) { // -> GET /api/v1/session/password/reset.html function passwordResetSite(req, res, next) { + if (!req.query.email) return next(new HttpError(400, 'Missing email')); if (!req.query.reset_token) return next(new HttpError(400, 'Missing reset_token')); - users.getByResetToken(req.query.reset_token, function (error, user) { - if (error) return next(new HttpError(401, 'Invalid reset_token')); + users.getByResetToken(req.query.email, req.query.reset_token, function (error, user) { + if (error) return next(new HttpError(401, 'Invalid email or reset token')); renderTemplate(res, 'password_reset', { user: user, csrf: req.csrfToken(), resetToken: req.query.reset_token, + email: req.query.email, title: 'Password Reset' }); }); @@ -423,13 +428,14 @@ function passwordResetSite(req, res, next) { function passwordReset(req, res, next) { assert.strictEqual(typeof req.body, 'object'); + if (typeof req.body.email !== 'string') return next(new HttpError(400, 'Missing email')); if (typeof req.body.resetToken !== 'string') return next(new HttpError(400, 'Missing resetToken')); if (typeof req.body.password !== 'string') return next(new HttpError(400, 'Missing password')); - debug('passwordReset: with token %s.', req.body.resetToken); + debug(`passwordReset: for ${req.body.email} with token ${req.body.resetToken}`); - users.getByResetToken(req.body.resetToken, function (error, userObject) { - if (error) return next(new HttpError(401, 'Invalid resetToken')); + users.getByResetToken(req.body.email, req.body.resetToken, function (error, userObject) { + if (error) return next(new HttpError(401, 'Invalid email or resetToken')); if (!userObject.username) return next(new HttpError(401, 'No username set')); diff --git a/src/routes/test/oauth2-test.js b/src/routes/test/oauth2-test.js index e3667fb2c..0dc40a3bd 100644 --- a/src/routes/test/oauth2-test.js +++ b/src/routes/test/oauth2-test.js @@ -1341,9 +1341,19 @@ describe('Password', function () { }); }); - it('setup succeeds', function (done) { + it('setup fails without email', function (done) { superagent.get(SERVER_URL + '/api/v1/session/account/setup.html') .query({ reset_token: USER_0.resetToken }) + .end(function (error, result) { + expect(result.statusCode).to.equal(200); + expect(result.text.indexOf('')).to.not.equal(-1); + done(); + }); + }); + + it('setup succeeds', function (done) { + superagent.get(SERVER_URL + '/api/v1/session/account/setup.html') + .query({ email: USER_0.email, reset_token: USER_0.resetToken }) .end(function (error, result) { expect(result.statusCode).to.equal(200); expect(result.text.indexOf('')).to.not.equal(-1); @@ -1361,7 +1371,16 @@ describe('Password', function () { it('reset fails due to invalid reset_token', function (done) { superagent.get(SERVER_URL + '/api/v1/session/password/reset.html') - .query({ reset_token: hat(256) }) + .query({ email: USER_0.email, reset_token: hat(256) }) + .end(function (error, result) { + expect(result.statusCode).to.equal(401); + done(); + }); + }); + + it('reset fails due to invalid email', function (done) { + superagent.get(SERVER_URL + '/api/v1/session/password/reset.html') + .query({ email: USER_0.email + 'x', reset_token: hat(256) }) .end(function (error, result) { expect(result.statusCode).to.equal(401); done(); @@ -1370,7 +1389,7 @@ describe('Password', function () { it('reset succeeds', function (done) { superagent.get(SERVER_URL + '/api/v1/session/password/reset.html') - .query({ reset_token: USER_0.resetToken }) + .query({ email: USER_0.email, reset_token: USER_0.resetToken }) .end(function (error, result) { expect(result.text.indexOf('')).to.not.equal(-1); expect(result.statusCode).to.equal(200); @@ -1427,7 +1446,7 @@ describe('Password', function () { it('fails due to empty password', function (done) { superagent.post(SERVER_URL + '/api/v1/session/password/reset') - .send({ password: '', resetToken: hat(256) }) + .send({ password: '', email: USER_0.email, resetToken: hat(256) }) .end(function (error, result) { expect(result.statusCode).to.equal(401); done(); @@ -1436,7 +1455,7 @@ describe('Password', function () { it('fails due to empty resetToken', function (done) { superagent.post(SERVER_URL + '/api/v1/session/password/reset') - .send({ password: '', resetToken: '' }) + .send({ password: '', email: USER_0.email, resetToken: '' }) .end(function (error, result) { expect(result.statusCode).to.equal(401); done(); @@ -1445,7 +1464,7 @@ describe('Password', function () { it('fails due to weak password', function (done) { superagent.post(SERVER_URL + '/api/v1/session/password/reset') - .send({ password: 'foobar', resetToken: USER_0.resetToken }) + .send({ password: 'foobar', email: USER_0.email, resetToken: USER_0.resetToken }) .end(function (error, result) { expect(result.statusCode).to.equal(406); done(); @@ -1460,7 +1479,7 @@ describe('Password', function () { .get('/').reply(200, {}); superagent.post(SERVER_URL + '/api/v1/session/password/reset') - .send({ password: 'ASF23$%somepassword', resetToken: USER_0.resetToken }) + .send({ password: '12345678', email: USER_0.email, resetToken: USER_0.resetToken }) .end(function (error, result) { expect(scope.isDone()).to.be.ok(); expect(result.statusCode).to.equal(200); diff --git a/src/test/database-test.js b/src/test/database-test.js index 4c6bc1bb9..dff00a668 100644 --- a/src/test/database-test.js +++ b/src/test/database-test.js @@ -315,8 +315,8 @@ describe('database', function () { }); }); - it('can get by resetToken fails for empty resetToken', function (done) { - userdb.getByResetToken('', function (error, user) { + it('getByResetToken fails for empty resetToken', function (done) { + userdb.getByResetToken(USER_0.email, '', function (error, user) { expect(error).to.be.ok(); expect(error.reason).to.be(DatabaseError.INTERNAL_ERROR); expect(user).to.not.be.ok(); @@ -324,8 +324,17 @@ describe('database', function () { }); }); + it('getByResetToken fails for bad email', function (done) { + userdb.getByResetToken(USER_0.email + 'x', USER_0.resetToken, function (error, user) { + expect(error).to.be.ok(); + expect(error.reason).to.be(DatabaseError.NOT_FOUND); + expect(user).to.not.be.ok(); + done(); + }); + }); + it('can get by resetToken', function (done) { - userdb.getByResetToken(USER_0.resetToken, function (error, user) { + userdb.getByResetToken(USER_0.email, USER_0.resetToken, function (error, user) { expect(error).to.not.be.ok(); expect(user).to.eql(USER_0); done(); diff --git a/src/userdb.js b/src/userdb.js index 74b942fc7..36c37b114 100644 --- a/src/userdb.js +++ b/src/userdb.js @@ -82,13 +82,14 @@ function getOwner(callback) { }); } -function getByResetToken(resetToken, callback) { +function getByResetToken(email, resetToken, callback) { + assert.strictEqual(typeof email, 'string'); assert.strictEqual(typeof resetToken, 'string'); assert.strictEqual(typeof callback, 'function'); if (resetToken.length === 0) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, 'Empty resetToken not allowed')); - database.query('SELECT ' + USERS_FIELDS + ' FROM users WHERE resetToken=?', [ resetToken ], function (error, result) { + database.query('SELECT ' + USERS_FIELDS + ' FROM users WHERE email=? AND resetToken=?', [ email, resetToken ], function (error, result) { if (error) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, error)); if (result.length === 0) return callback(new DatabaseError(DatabaseError.NOT_FOUND)); diff --git a/src/users.js b/src/users.js index 5af018f2b..dc291b5ba 100644 --- a/src/users.js +++ b/src/users.js @@ -337,14 +337,18 @@ function get(userId, callback) { }); } -function getByResetToken(resetToken, callback) { +function getByResetToken(email, resetToken, callback) { + assert.strictEqual(typeof email, 'string'); assert.strictEqual(typeof resetToken, 'string'); assert.strictEqual(typeof callback, 'function'); - var error = validateToken(resetToken); + var error = validateEmail(email); if (error) return callback(error); - userdb.getByResetToken(resetToken, function (error, result) { + error = validateToken(resetToken); + if (error) return callback(error); + + userdb.getByResetToken(email, resetToken, function (error, result) { if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new UsersError(UsersError.NOT_FOUND)); if (error) return callback(new UsersError(UsersError.INTERNAL_ERROR, error));