From bc14ded655a0b0c6d121cae7d19d976c726d7e96 Mon Sep 17 00:00:00 2001
From: Lloyd Hilaiel <lloyd@hilaiel.com>
Date: Tue, 3 Jul 2012 10:55:36 +0100
Subject: [PATCH] implement complete_reset, stage_reset and refactor database
code to support this addition, add tests
---
lib/db.js | 4 +-
lib/db/json.js | 95 ++++++++++-----
lib/db/mysql.js | 115 ++++++++++++------
lib/wsapi/complete_email_addition.js | 2 +-
lib/wsapi/complete_reset.js | 86 +++++++++++++
lib/wsapi/complete_user_creation.js | 2 +-
lib/wsapi/stage_reset.js | 104 ++++++++++++++++
tests/db-test.js | 4 +-
...n-email-test.js => forgotten-pass-test.js} | 45 ++++---
tests/verify-in-different-browser-test.js | 2 +-
10 files changed, 373 insertions(+), 86 deletions(-)
create mode 100644 lib/wsapi/complete_reset.js
create mode 100644 lib/wsapi/stage_reset.js
rename tests/{forgotten-email-test.js => forgotten-pass-test.js} (88%)
diff --git a/lib/db.js b/lib/db.js
index 1f08f6e99..27826a0fb 100644
--- a/lib/db.js
+++ b/lib/db.js
@@ -101,7 +101,9 @@ exports.onReady = function(f) {
[
'stageUser',
'stageEmail',
- 'gotVerificationSecret',
+ 'completeCreateUser',
+ 'completeAddEmail',
+ 'completePasswordReset',
'removeEmail',
'cancelAccount',
'updatePassword',
diff --git a/lib/db/json.js b/lib/db/json.js
index 34cb13b3e..9d57297a1 100644
--- a/lib/db/json.js
+++ b/lib/db/json.js
@@ -263,7 +263,7 @@ exports.verificationSecretForEmail = function(email, cb) {
}, 0);
};
-exports.gotVerificationSecret = function(secret, cb) {
+function getAndDeleteRowForSecret(secret, cb) {
sync();
if (!db.staged.hasOwnProperty(secret)) return cb("unknown secret");
@@ -272,7 +272,42 @@ exports.gotVerificationSecret = function(secret, cb) {
delete db.staged[secret];
delete db.stagedEmails[o.email];
flush();
- if (o.type === 'add_account') {
+
+ process.nextTick(function() { cb(null, o); });
+}
+
+exports.completeAddEmail = function(secret, cb) {
+ getAndDeleteRowForSecret(secret, function(err, o) {
+ exports.emailKnown(o.email, function(err, known) {
+ function addIt() {
+ addEmailToAccount(o.existing_user, o.email, 'secondary', function(e) {
+ var hash = o.passwd;
+ if(e || hash === null) return cb(e, o.email, o.existing_user);
+
+ // a hash was specified, update the password for the user
+ exports.emailToUID(o.email, function(err, uid) {
+ if(err) return cb(err, o.email, o.existing_user);
+
+ exports.updatePassword(uid, hash, function(err) {
+ cb(err || null, o.email, o.existing_user);
+ });
+ });
+ });
+ }
+ if (known) {
+ removeEmailNoCheck(o.email, function (err) {
+ if (err) cb(err);
+ else addIt();
+ });
+ } else {
+ addIt();
+ }
+ });
+ });
+}
+
+exports.completeCreateUser = function(secret, cb) {
+ getAndDeleteRowForSecret(secret, function(err, o) {
exports.emailKnown(o.email, function(err, known) {
function createAccount() {
var emailVal = {};
@@ -303,35 +338,39 @@ exports.gotVerificationSecret = function(secret, cb) {
createAccount();
}
});
- } else if (o.type === 'add_email') {
- exports.emailKnown(o.email, function(err, known) {
- function addIt() {
- addEmailToAccount(o.existing_user, o.email, 'secondary', function(e) {
- var hash = o.passwd;
- if(e || hash === null) return cb(e, o.email, o.existing_user);
-
- // a hash was specified, update the password for the user
- exports.emailToUID(o.email, function(err, uid) {
- if(err) return cb(err, o.email, o.existing_user);
+ });
+};
- exports.updatePassword(uid, hash, function(err) {
- cb(err || null, o.email, o.existing_user);
- });
- });
- });
- }
- if (known) {
- removeEmailNoCheck(o.email, function (err) {
- if (err) cb(err);
- else addIt();
+exports.completePasswordReset = function(secret, cb) {
+ getAndDeleteRowForSecret(secret, function(err, o) {
+ exports.emailKnown(o.email, function(err, known) {
+ if (err) return cb(err);
+
+ exports.emailToUID(o.email, function(err, uid) {
+ if (err) return cb(err);
+
+ // if for some reason the email is associated with a different user now than when
+ // the action was initiated, error out.
+ if (uid !== o.existing_user) {
+ return cb("cannot update password, data inconsistency");
+ }
+
+ sync();
+ // flip the verification bit on all emails for the user other than the one just verified
+ var emails = jsel.match(":has(.id:expr(x=?)) > .emails", [ uid ], db.users)[0];
+
+ Object.keys(emails).forEach(function(email) {
+ if (email != o.email && emails[email].type === 'secondary') {
+ emails[email].verified = false;
+ }
});
- } else {
- addIt();
- }
+ flush();
+
+ // update the password!
+ exports.updatePassword(uid, o.passwd, cb);
+ });
});
- } else {
- cb("internal error");
- }
+ });
};
exports.addPrimaryEmailToAccount = function(userID, emailToAdd, cb) {
diff --git a/lib/db/mysql.js b/lib/db/mysql.js
index b1461c304..dcd1cadb2 100644
--- a/lib/db/mysql.js
+++ b/lib/db/mysql.js
@@ -15,6 +15,7 @@
* | string passwd | \- |*int user |
* +---------------+ |*string address|
* | enum type |
+ * | bool verified |
* +---------------+
*
*
@@ -332,7 +333,7 @@ function addEmailToUser(userID, email, type, cb) {
});
}
-exports.gotVerificationSecret = function(secret, cb) {
+function getAndDeleteRowForSecret(secret, cb) {
client.query(
"SELECT * FROM staged WHERE secret = ?", [ secret ],
function(err, rows) {
@@ -342,45 +343,87 @@ exports.gotVerificationSecret = function(secret, cb) {
} else if (rows.length === 0) {
cb("unknown secret");
} else {
- var o = rows[0];
-
// delete the record
client.query("DELETE LOW_PRIORITY FROM staged WHERE secret = ?", [ secret ]);
+ cb(null, rows[0]);
+ }
+ });
+}
- if (o.new_acct) {
- var hash = o.passwd;
- // we're creating a new account, add appropriate entries into user and email tables.
- client.query(
- "INSERT INTO user(passwd) VALUES(?)",
- [ hash ],
- function(err, info) {
- if (err) return cb(err);
- addEmailToUser(info.insertId, o.email, 'secondary', cb);
- });
- } else {
- // ensure the expected existing_user field is populated, which it must always be when
- // new_acct is false
- if (typeof o.existing_user !== 'number') {
- return cb("data inconsistency, no numeric existing user associated with staged email address");
- }
+exports.completeCreateUser = function(secret, cb) {
+ getAndDeleteRowForSecret(secret, function(err, o) {
+ if (err) return cb(err);
+
+ if (!o.new_acct) return cb("this verification link is not for a new account");
- // we're adding an email address to an existing user account. add appropriate entries into
- // email table
- var hash = o.passwd;
- var uid = o.existing_user;
- if (hash) {
- exports.updatePassword(uid, hash, function(err) {
- if (err) return cb('could not set user\'s password');
- addEmailToUser(uid, o.email, 'secondary', cb);
- });
- } else {
- addEmailToUser(uid, o.email, 'secondary', cb);
- }
- }
- };
+ // we're creating a new account, add appropriate entries into user and email tables.
+ client.query(
+ "INSERT INTO user(passwd) VALUES(?)",
+ [ o.passwd ],
+ function(err, info) {
+ if (err) return cb(err);
+ addEmailToUser(info.insertId, o.email, 'secondary', cb);
+ });
+ });
+};
+
+exports.completeAddEmail = function(secret, cb) {
+ getAndDeleteRowForSecret(secret, function(err, o) {
+ if (err) return cb(err);
+
+ if (o.new_acct) return cb("this verification link is not for an email addition");
+
+ // ensure the expected existing_user field is populated, which it must always be when
+ // new_acct is false
+ if (typeof o.existing_user !== 'number') {
+ return cb("data inconsistency, no numeric existing user associated with staged email address");
}
- );
-}
+
+ // we're adding an email address to an existing user account. add appropriate entries into
+ // email table
+ if (o.passwd) {
+ exports.updatePassword(o.existing_user, o.passwd, function(err) {
+ if (err) return cb('could not set user\'s password');
+ addEmailToUser(o.existing_user, o.email, 'secondary', cb);
+ });
+ } else {
+ addEmailToUser(o.existing_user, o.email, 'secondary', cb);
+ }
+ });
+};
+
+exports.completePasswordReset = function(secret, cb) {
+ getAndDeleteRowForSecret(secret, function(err, o) {
+ if (err) return cb(err);
+
+ if (o.new_acct || !o.passwd || !o.existing_user) {
+ return cb("this verification link is not for a password reset");
+ }
+
+ // verify that the email still exists in the database, and the the user with whom it is
+ // associated is the same as the user in the database
+ exports.emailToUID(o.email, function(err, uid) {
+ if (err) return cb(err);
+
+ // if for some reason the email is associated with a different user now than when
+ // the action was initiated, error out.
+ if (uid !== o.existing_user) {
+ return cb("cannot update password, data inconsistency");
+ }
+
+ // flip the verification bit on all emails for the user other than the one just verified
+ client.query(
+ 'UPDATE email SET verified = FALSE WHERE user = ? AND type = ? AND address != ?',
+ [ uid, 'secondary', o.email ],
+ function(err) {
+ if (err) return cb(err);
+
+ // update the password!
+ exports.updatePassword(uid, o.passwd, cb);
+ });
+ });
+ });
+};
exports.addPrimaryEmailToAccount = function(uid, emailToAdd, cb) {
// we're adding an email address to an existing user account. add appropriate entries into
@@ -481,7 +524,7 @@ exports.listEmails = function(uid, cb) {
for (var i = 0; i < rows.length; i++) {
var o = { type: rows[i].type };
if (o.type === 'secondary') {
- o.verified = rows[i].verified;
+ o.verified = rows[i].verified ? true : false;
}
emails[rows[i].address] = o;
}
diff --git a/lib/wsapi/complete_email_addition.js b/lib/wsapi/complete_email_addition.js
index 53ddb3625..f903a28ad 100644
--- a/lib/wsapi/complete_email_addition.js
+++ b/lib/wsapi/complete_email_addition.js
@@ -47,7 +47,7 @@ exports.process = function(req, res) {
}
function postAuthentication() {
- db.gotVerificationSecret(req.body.token, function(e, email, uid) {
+ db.completeAddEmail(req.body.token, function(e, email, uid) {
if (e) {
logger.warn("couldn't complete email verification: " + e);
wsapi.databaseDown(res, e);
diff --git a/lib/wsapi/complete_reset.js b/lib/wsapi/complete_reset.js
new file mode 100644
index 000000000..32ea867c2
--- /dev/null
+++ b/lib/wsapi/complete_reset.js
@@ -0,0 +1,86 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+const
+db = require('../db.js'),
+logger = require('../logging.js').logger,
+wsapi = require('../wsapi.js'),
+bcrypt = require('../bcrypt.js'),
+httputils = require('../httputils.js');
+
+exports.method = 'post';
+exports.writes_db = true;
+exports.authed = false;
+// NOTE: this API also takes a 'pass' parameter which is required
+// when a user has a null password (only primaries on their acct)
+exports.args = ['token'];
+exports.i18n = true;
+
+exports.process = function(req, res) {
+ // in order to complete a password reset, one of the following must be true:
+ //
+ // 1. you are using the same browser to complete the email verification as you
+ // used to start it
+ // 2. you have provided the password chosen by the initiator of the verification
+ // request
+
+ // is this the same browser?
+ if (typeof req.session.pendingReset === 'string' &&
+ req.body.token === req.session.pendingReset) {
+ return postAuthentication();
+ }
+ // is a password provided?
+ else if (typeof req.body.pass === 'string') {
+ return db.authForVerificationSecret(req.body.token, function(err, hash) {
+ if (err) {
+ logger.warn("couldn't get password for verification secret: " + err);
+ return wsapi.databaseDown(res, err);
+ }
+
+ bcrypt.compare(req.body.pass, hash, function (err, success) {
+ if (err) {
+ logger.warn("max load hit, failing on auth request with 503: " + err);
+ return httputils.serviceUnavailable(res, "server is too busy");
+ } else if (!success) {
+ return httputils.authRequired(res, "password mismatch");
+ } else {
+ return postAuthentication();
+ }
+ });
+ });
+ } else {
+ return httputils.authRequired(res, 'Provide your password');
+ }
+
+ function postAuthentication() {
+ db.haveVerificationSecret(req.body.token, function(err, known) {
+ if (err) return wsapi.databaseDown(res, err);
+
+ if (!known) {
+ // clear the pendingCreation token from the session if we find no such
+ // token in the database
+ delete req.session.pendingCreation;
+ return res.json({ success: false} );
+ }
+
+ db.completePasswordReset(req.body.token, function(err, email, uid) {
+ if (err) {
+ logger.warn("couldn't complete email verification: " + err);
+ wsapi.databaseDown(res, err);
+ } else {
+ // clear the pendingCreation token from the session once we
+ // successfully complete user creation
+ delete req.session.pendingCreation;
+
+ // At this point, the user is either on the same browser with a token from
+ // their email address, OR they've provided their account password. It's
+ // safe to grant them an authenticated session.
+ wsapi.authenticateSession(req.session, uid, 'password',
+ config.get('ephemeral_session_duration_ms'));
+ res.json({ success: true });
+ }
+ });
+ });
+ }
+};
diff --git a/lib/wsapi/complete_user_creation.js b/lib/wsapi/complete_user_creation.js
index 7a65ec488..5e253d38a 100644
--- a/lib/wsapi/complete_user_creation.js
+++ b/lib/wsapi/complete_user_creation.js
@@ -68,7 +68,7 @@ exports.process = function(req, res) {
return res.json({ success: false} );
}
- db.gotVerificationSecret(req.body.token, function(err, email, uid) {
+ db.completeCreateUser(req.body.token, function(err, email, uid) {
if (err) {
logger.warn("couldn't complete email verification: " + err);
wsapi.databaseDown(res, err);
diff --git a/lib/wsapi/stage_reset.js b/lib/wsapi/stage_reset.js
new file mode 100644
index 000000000..ccc0a4eca
--- /dev/null
+++ b/lib/wsapi/stage_reset.js
@@ -0,0 +1,104 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+const
+db = require('../db.js'),
+wsapi = require('../wsapi.js'),
+httputils = require('../httputils'),
+logger = require('../logging.js').logger,
+email = require('../email.js'),
+sanitize = require('../sanitize'),
+config = require('../configuration');
+
+/* First half of account creation. Stages a user account for creation.
+ * this involves creating a secret url that must be delivered to the
+ * user via their claimed email address. Upon timeout expiry OR clickthrough
+ * the staged user account transitions to a valid user account
+ */
+
+exports.method = 'post';
+exports.writes_db = true;
+exports.authed = false;
+exports.args = ['email','site','pass'];
+exports.i18n = true;
+
+exports.process = function(req, res) {
+ // a password *must* be supplied to this call iff the user's password
+ // is currently NULL - this would occur in the case where this is the
+ // first secondary address to be added to an account
+
+ // validate
+ try {
+ sanitize(req.body.email).isEmail();
+ sanitize(req.body.site).isOrigin();
+ } catch(e) {
+ var msg = "invalid arguments: " + e;
+ logger.warn("bad request received: " + msg);
+ return httputils.badRequest(res, msg);
+ }
+
+ var err = wsapi.checkPassword(req.body.pass);
+ if (err) {
+ logger.warn("invalid password received: " + err);
+ return httputils.badRequest(res, err);
+ }
+
+ db.lastStaged(req.body.email, function (err, last) {
+ if (err) return wsapi.databaseDown(res, err);
+
+ if (last && (new Date() - last) < config.get('min_time_between_emails_ms')) {
+ logger.warn('throttling request to stage email address ' + req.body.email + ', only ' +
+ ((new Date() - last) / 1000.0) + "s elapsed");
+ return httputils.throttled(res, "Too many emails sent to that address, try again later.");
+ }
+
+ db.emailToUID(req.body.email, function(err, uid) {
+ if (err) {
+ logger.info("reset password fails: " + err);
+ return res.json({ success: false });
+ }
+
+ if (!uid) {
+ return res.json({
+ reason: "No such email address.",
+ success: false
+ });
+ }
+
+ // staging a user logs you out.
+ wsapi.clearAuthenticatedUser(req.session);
+
+ // now bcrypt the password
+ wsapi.bcryptPassword(req.body.pass, function (err, hash) {
+ if (err) {
+ if (err.indexOf('exceeded') != -1) {
+ logger.warn("max load hit, failing on auth request with 503: " + err);
+ return httputils.serviceUnavailable(res, "server is too busy");
+ }
+ logger.error("can't bcrypt: " + err);
+ return res.json({ success: false });
+ }
+
+ // on failure stageEmail may throw
+ try {
+ db.stageEmail(uid, req.body.email, hash, function(err, secret) {
+ if (err) return wsapi.databaseDown(res, err);
+
+ var langContext = wsapi.langContext(req);
+
+ // store the email being added in session data
+ req.session.pendingReset = secret;
+
+ res.json({ success: true });
+ // let's now kick out a verification email!
+ email.sendForgotPasswordEmail(req.body.email, req.body.site, secret, langContext);
+ });
+ } catch(e) {
+ // we should differentiate tween' 400 and 500 here.
+ httputils.badRequest(res, e.toString());
+ }
+ });
+ });
+ });
+};
diff --git a/tests/db-test.js b/tests/db-test.js
index 6c8ec1ea5..d17038d8b 100755
--- a/tests/db-test.js
+++ b/tests/db-test.js
@@ -125,7 +125,7 @@ suite.addBatch({
suite.addBatch({
"upon receipt of a secret": {
topic: function() {
- db.gotVerificationSecret(secret, this.callback);
+ db.completeCreateUser(secret, this.callback);
},
"gotVerificationSecret completes without error": function (err, r) {
assert.isNull(err);
@@ -215,7 +215,7 @@ suite.addBatch({
"makes it visible via isStaged": function(sekret, r) { assert.isTrue(r); },
"lets you verify it": {
topic: function(secret, r) {
- db.gotVerificationSecret(secret, this.callback);
+ db.completeAddEmail(secret, this.callback);
},
"successfully": function(err, r) {
assert.isNull(err);
diff --git a/tests/forgotten-email-test.js b/tests/forgotten-pass-test.js
similarity index 88%
rename from tests/forgotten-email-test.js
rename to tests/forgotten-pass-test.js
index c2e6aa382..a9ca24042 100755
--- a/tests/forgotten-email-test.js
+++ b/tests/forgotten-pass-test.js
@@ -132,11 +132,10 @@ suite.addBatch({
}
});
-// Run the "forgot_email" flow with first address. This is really
-// just re-registering the user.
+// Run the "forgot_email" flow with first address.
suite.addBatch({
"re-stage first account": {
- topic: wsapi.post('/wsapi/stage_user', {
+ topic: wsapi.post('/wsapi/stage_reset', {
email: 'first@fakeemail.com',
pass: 'secondfakepass',
site:'https://otherfakesite.com'
@@ -187,9 +186,9 @@ suite.addBatch({
// now let's complete the re-registration of first email address
suite.addBatch({
- "re-create first email address": {
+ "complete password reset": {
topic: function() {
- wsapi.post('/wsapi/complete_user_creation', { token: token }).call(this);
+ wsapi.post('/wsapi/complete_reset', { token: token }).call(this);
},
"account created": function(err, r) {
assert.equal(r.code, 200);
@@ -198,8 +197,7 @@ suite.addBatch({
}
});
-// now we should be able to sign into the first email address with the first
-// password, and all other combinations should fail
+// now we should be able to sign in using any email address
suite.addBatch({
"first email, first pass bad": {
topic: wsapi.post('/wsapi/authenticate_user', {
@@ -221,20 +219,14 @@ suite.addBatch({
assert.strictEqual(JSON.parse(r.body).success, true);
}
},
- "logout": {
- topic: wsapi.post('/wsapi/logout', {}),
- "should work": function(err, r) {
- assert.strictEqual(JSON.parse(r.body).success, true);
- }
- },
- "second email, first pass good": {
+ "second email, first pass bad": {
topic: wsapi.post('/wsapi/authenticate_user', {
email: 'second@fakeemail.com',
pass: 'firstfakepass',
ephemeral: false
}),
"should work": function(err, r) {
- assert.strictEqual(JSON.parse(r.body).success, true);
+ assert.strictEqual(JSON.parse(r.body).success, false);
}
},
"second email, second pass bad": {
@@ -244,11 +236,32 @@ suite.addBatch({
ephemeral: false
}),
"shouldn' work": function(err, r) {
- assert.strictEqual(JSON.parse(r.body).success, false);
+ assert.strictEqual(JSON.parse(r.body).success, true);
}
},
});
+// test list emails
+suite.addBatch({
+ "list emails API": {
+ topic: wsapi.get('/wsapi/list_emails', {}),
+ "succeeds with HTTP 200" : function(err, r) {
+ assert.strictEqual(r.code, 200);
+ },
+ "returns an object with proper bits set": function(err, r) {
+ r = JSON.parse(r.body);
+ assert.strictEqual(r['second@fakeemail.com'].verified, false);
+ assert.strictEqual(r['first@fakeemail.com'].verified, true);
+ }
+ }
+});
+
+
+// XXX: test that verification of unverified emails fails
+
+// XXX: test that we can verify the remaining email ok
+
+
start_stop.addShutdownBatches(suite);
// run or export the suite.
diff --git a/tests/verify-in-different-browser-test.js b/tests/verify-in-different-browser-test.js
index b5a5a0fbb..b221322a8 100755
--- a/tests/verify-in-different-browser-test.js
+++ b/tests/verify-in-different-browser-test.js
@@ -254,7 +254,7 @@ suite.addBatch({
},
"but succeeds": {
topic: function() {
- wsapi.post('/wsapi/complete_email_addition', {
+ wsapi.post('/wsapi/complete_user_creation', {
token: this._token,
pass: TEST_PASS
}).call(this);
--
GitLab