diff --git a/lib/wsapi/complete_reset.js b/lib/wsapi/complete_reset.js index 4d3bcfec2b762c39d535d8938f9b36e37e2fecba..49d8b2c58a342ffeb1339aeb3b51738f2489d1c1 100644 --- a/lib/wsapi/complete_reset.js +++ b/lib/wsapi/complete_reset.js @@ -33,8 +33,7 @@ exports.process = function(req, res) { // request // is this the same browser? - if (typeof req.session.pendingReset === 'string' && - req.params.token === req.session.pendingReset) { + if (req.params.token === req.session.pendingReset) { return postAuthentication(); } // is a password provided? diff --git a/lib/wsapi/complete_user_creation.js b/lib/wsapi/complete_user_creation.js index f737e3b8f297bc8873070dcc4b2a2d905b6e9fd4..66955d37849514d1ed8a83351507b8458bc5cc8a 100644 --- a/lib/wsapi/complete_user_creation.js +++ b/lib/wsapi/complete_user_creation.js @@ -38,8 +38,7 @@ exports.process = function(req, res) { // the email address of the attacked. // is this the same browser? - if (typeof req.session.pendingCreation === 'string' && - req.params.token === req.session.pendingCreation) { + if (req.params.token === req.session.pendingCreation) { return postAuthentication(); } // is a password provided? diff --git a/lib/wsapi/email_for_token.js b/lib/wsapi/email_for_token.js index 3aed92174cf1c5b138638959cffea8a223355803..5b28e07c1febb5460fce91db5a3a82e46c7fb90c 100644 --- a/lib/wsapi/email_for_token.js +++ b/lib/wsapi/email_for_token.js @@ -38,29 +38,28 @@ exports.process = function(req, res) { reason: err }); } - } + } function checkMustAuth() { - // must the user authenticate? This is true if they are not authenticated - // as the uid who initiated the verification, and they are not on the same - // browser as the initiator var must_auth = true; - if (uid && req.session.userid === uid) { - must_auth = false; - } - else if (!uid && typeof req.session.pendingCreation === 'string' && - req.params.token === req.session.pendingCreation) { - must_auth = false; - } - else if (typeof req.session.pendingReset === 'string' && - req.params.token === req.session.pendingReset) + // For the following cases, the user must re-authenticate if they're not on the + // same browser. + // 1. they're resetting their password + // 2. they're creating their account + must_auth = + !((req.params.token === req.session.pendingCreation) || + (req.params.token === req.session.pendingReset)); + + // For the following cases, unless the user is on the same browser AND authenticated, + // they must re-provide their password: + // 1. they're re-verifying an email after password reset + // 2. they're confirming a new email they want to add to their account + if (req.params.token === req.session.pendingReverification || + req.params.token === req.session.pendingAddition) { - must_auth = false; + must_auth = !(req.session.userid && req.session.userid === uid); } - // NOTE: for reverification, we require you're authenticated. it's not enough - // to be on the same browser - that path is nonsensical because you must be - // authenticated to initiate a re-verification. res.json({ success: true, diff --git a/lib/wsapi/password_reset_status.js b/lib/wsapi/password_reset_status.js index e82b2f1dfb6d59dec8e88fe3b9b7412ab95ca6a3..67fdc1f9e92ee2d4042c1aaf754cf15f2d94fbe0 100644 --- a/lib/wsapi/password_reset_status.js +++ b/lib/wsapi/password_reset_status.js @@ -23,7 +23,7 @@ exports.process = function(req, res) { // * if we are not authenticated as the owner of the email, we must auth db.isStaged(email, function(err, staged) { if (err) wsapi.databaseDown(res, err); - + if (staged) { return res.json({ status: 'pending' }); } else { diff --git a/tests/forgotten-pass-test.js b/tests/forgotten-pass-test.js index 7e9aae487b38946332a3096323207edc3ad5e329..b13b5ddc14a69ff6f839bbd1e658fe86cc1abf1c 100755 --- a/tests/forgotten-pass-test.js +++ b/tests/forgotten-pass-test.js @@ -25,6 +25,9 @@ start_stop.addStartupBatches(suite); // var 'token' var token = undefined; +// stores wsapi client context +var oldContext; + // create a new account via the api with (first address) suite.addBatch({ "staging an account": { @@ -101,6 +104,52 @@ suite.addBatch({ } }); +// should not require auth to complete +suite.addBatch({ + "given a token, getting an email": { + topic: function() { + wsapi.get('/wsapi/email_for_token', { token: token }).call(this); + }, + "account created": function(err, r) { + assert.equal(r.code, 200); + var body = JSON.parse(r.body); + assert.strictEqual(body.success, true); + assert.strictEqual(body.must_auth, false); + } + } +}); + + +// New context for a second client +suite.addBatch({ + "change context": function () { + oldContext = wsapi.getContext(); + wsapi.setContext({}); + } +}); + +// should require auth to complete for second client +suite.addBatch({ + "given a token, getting an email": { + topic: function() { + wsapi.get('/wsapi/email_for_token', { token: token }).call(this); + }, + "account created": function(err, r) { + assert.equal(r.code, 200); + var body = JSON.parse(r.body); + assert.strictEqual(body.success, true); + assert.strictEqual(body.must_auth, true); + } + } +}); + +// restore context of first client +suite.addBatch({ + "restore context": function () { + wsapi.setContext(oldContext); + } +}); + // confirm second email email address to the account suite.addBatch({ "create second account": { @@ -183,6 +232,7 @@ suite.addBatch({ assert.equal(r.code, 200); var body = JSON.parse(r.body); assert.strictEqual(body.success, true); + assert.strictEqual(body.must_auth, false); } } }); @@ -286,6 +336,67 @@ suite.addBatch({ }, }); +// Test issue #2104: when using a second browser to initiate password reset, first +// browser should be prompted to authenticate + +// New context for a second client +suite.addBatch({ + "change context": function () { + oldContext = wsapi.getContext(); + wsapi.setContext({}); + } +}); + +// Run the "forgot_email" flow with first address. +suite.addBatch({ + "reset password on first account": { + topic: wsapi.post('/wsapi/stage_reset', { + email: 'first@fakeemail.com', + pass: 'secondfakepass', + site:'https://otherfakesite.com' + }), + "works": function(err, r) { + assert.strictEqual(r.code, 200); + } + } +}); + +// wait for the token +suite.addBatch({ + "a token": { + topic: function() { + start_stop.waitForToken(this.callback); + }, + "is obtained": function (t) { + assert.strictEqual(typeof t, 'string'); + token = t; + } + } +}); + +// restore context of first client +suite.addBatch({ + "restore context": function () { + wsapi.setContext(oldContext); + } +}); + +suite.addBatch({ + "given a token, getting an email": { + topic: function() { + wsapi.get('/wsapi/email_for_token', { token: token }).call(this); + }, + "account created": function(err, r) { + assert.equal(r.code, 200); + var body = JSON.parse(r.body); + assert.strictEqual(body.success, true); + assert.strictEqual(body.email, 'first@fakeemail.com'); + assert.strictEqual(body.must_auth, true); + } + } +}); + + // test list emails suite.addBatch({ "list emails API": { diff --git a/tests/lib/wsapi.js b/tests/lib/wsapi.js index 868c86d02756fd53ed624cbe543b403355cb510e..cd35cb64be9a49c0865901ad410e1e9b434c9a32 100644 --- a/tests/lib/wsapi.js +++ b/tests/lib/wsapi.js @@ -42,4 +42,13 @@ exports.getCSRF = function() { return context.session.csrf_token; } return null; -}; \ No newline at end of file +}; + +// allows for multiple clients +exports.setContext = function (cxt) { + context = cxt; +}; + +exports.getContext = function () { + return context; +};