From 5f5d8e5389a3a0b4d1cc2bedd3901c71fe496ee0 Mon Sep 17 00:00:00 2001 From: Brian Warner <warner@lothar.com> Date: Wed, 15 Aug 2012 16:47:19 -0700 Subject: [PATCH] Bug #2307: don't expire existing sessions when adding a secondary address If a persona.org account is initially created with a "primary" address (meaning an address served by a participating IdP, so persona.org is given an assertion from that IdP as proof of ownership), the new account will not have a password associated with it. If you then add a "secondary" address (meaning an address *not* served by a participating IdP, requiring an email challenge to prove ownership), you will have to set up a password when you add the secondary. The establishment of this password should *not* invalidate any sessions that were set up earlier. In Bug #2307, this manifested as the first browser (in which the add-secondary-email operation was started, so it had the old session and was waiting for the operation to complete, polling /wsapi/email_addition_status all the while) receiving a "400 Unauthorized" error when the email challenge link was opened in a second browser (which thus got a new session). The test for this effect lives in tests/primary-then-secondary-test.js, which need the same 2-second delay as password-update-test.js (to make sure that the modified lastPasswordReset time was actually different than the previous value, so the session really would be expired). --- lib/db/json.js | 2 +- lib/db/mysql.js | 2 +- tests/primary-then-secondary-test.js | 39 ++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/lib/db/json.js b/lib/db/json.js index 6c9555b65..39683d7ec 100644 --- a/lib/db/json.js +++ b/lib/db/json.js @@ -310,7 +310,7 @@ exports.completeConfirmEmail = function(secret, cb) { exports.emailToUID(o.email, function(err, uid) { if(err) return cb(err, o.email, o.existing_user); - exports.updatePassword(uid, hash, true, function(err) { + exports.updatePassword(uid, hash, false, function(err) { cb(err || null, o.email, o.existing_user); }); }); diff --git a/lib/db/mysql.js b/lib/db/mysql.js index 4c8f1edf3..ee9f2ac54 100644 --- a/lib/db/mysql.js +++ b/lib/db/mysql.js @@ -397,7 +397,7 @@ exports.completeConfirmEmail = function(secret, cb) { // we're adding or reverifying an email address to an existing user account. add appropriate // entries into email table. if (o.passwd) { - exports.updatePassword(o.existing_user, o.passwd, true, function(err) { + exports.updatePassword(o.existing_user, o.passwd, false, function(err) { if (err) return cb('could not set user\'s password'); addEmailToUser(o.existing_user, o.email, 'secondary', cb); }); diff --git a/tests/primary-then-secondary-test.js b/tests/primary-then-secondary-test.js index 1eb6aaeda..a0de51402 100755 --- a/tests/primary-then-secondary-test.js +++ b/tests/primary-then-secondary-test.js @@ -49,6 +49,7 @@ suite.addBatch({ } }); +var the_assertion; // now let's generate an assertion using this user suite.addBatch({ "generating an assertion": { @@ -60,6 +61,7 @@ suite.addBatch({ }, "and logging in with the assertion succeeds": { topic: function(err, assertion) { + the_assertion = assertion; wsapi.post('/wsapi/auth_with_assertion', { assertion: assertion, ephemeral: true @@ -85,6 +87,32 @@ suite.addBatch({ } }); +// this second session, logged in with just the primary, should *not* be +// invalidated by the addition of a secondary address (and consequent +// establishment of a password) +var context2 = {}; +suite.addBatch({ + "establishing a second session": { + topic: function() { + wsapi.post('/wsapi/auth_with_assertion', { + assertion: the_assertion, + ephemeral: true + }, context2).call(this); + }, + "works as expected": function(err, r) { + assert.strictEqual(JSON.parse(r.body).success, true); + }, + "after waiting for lastPasswordReset's now() to increment": { + topic: function() { + // see password-update-test.js for an explanation of this delay + setTimeout(this.callback, 2000); + }, + "we've waited long enough": function() {} + } + } +}); + + var token; // now we have a new account. let's add a secondary to it suite.addBatch({ @@ -238,6 +266,17 @@ suite.addBatch({ } }); +// and the second session should still be valid +suite.addBatch({ + "second session is still valid": { + topic: wsapi.post('/wsapi/prolong_session', {}, context2), + "works as expected": function(err, r) { + assert.strictEqual(r.code, 200); + assert.strictEqual(r.body, "OK"); + } + } +}); + // shut the server down and cleanup start_stop.addShutdownBatches(suite); -- GitLab