diff --git a/lib/db/json.js b/lib/db/json.js
index e49615c1fb4f5c2f495c9246cb8bcb4dbb695684..376a2a791cae9a968949a202772df5c2193d552f 100644
--- a/lib/db/json.js
+++ b/lib/db/json.js
@@ -236,7 +236,7 @@ exports.emailForVerificationSecret = function(secret, cb) {
process.nextTick(function() {
sync();
if (!db.staged[secret]) return cb("no such secret");
- cb(null, db.staged[secret].email, db.staged[secret].existing_user);
+ cb(null, db.staged[secret].email, db.staged[secret].existing_user, db.staged[secret].passwd);
});
};
diff --git a/lib/db/mysql.js b/lib/db/mysql.js
index b2e123ae6667e7c150963f627f0af30ba9c491bc..8c2d805e63fed01488e4b6f82b561198c98369c1 100644
--- a/lib/db/mysql.js
+++ b/lib/db/mysql.js
@@ -265,14 +265,14 @@ exports.haveVerificationSecret = function(secret, cb) {
exports.emailForVerificationSecret = function(secret, cb) {
client.query(
- "SELECT email, existing_user FROM staged WHERE secret = ?", [ secret ],
+ "SELECT email, existing_user, passwd FROM staged WHERE secret = ?", [ secret ],
function(err, rows) {
if (err) return cb("database unavailable");
// if the record was not found, fail out
if (!rows || rows.length != 1) return cb("no such secret");
- cb(null, rows[0].email, rows[0].existing_user);
+ cb(null, rows[0].email, rows[0].existing_user, rows[0].passwd);
});
};
@@ -291,7 +291,7 @@ exports.authForVerificationSecret = function(secret, cb) {
if (o.passwd) return cb(null, o.passwd, o.existing_user);
// otherwise, let's get the passwd from the user record
- if (!o.existing_user) cb("no password for user");
+ if (!o.existing_user) return cb("no password for user");
exports.checkAuth(o.existing_user, function(err, hash) {
cb(err, hash, o.existing_user);
@@ -337,8 +337,9 @@ exports.gotVerificationSecret = function(secret, cb) {
if (err) {
logUnexpectedError(err);
cb(err);
- } else if (rows.length === 0) cb("unknown secret");
- else {
+ } else if (rows.length === 0) {
+ cb("unknown secret");
+ } else {
var o = rows[0];
// delete the record
diff --git a/lib/wsapi/complete_email_addition.js b/lib/wsapi/complete_email_addition.js
index fcff7281387b49d78fb53a05ad4b74187dd36db2..7756fcf5b9cc8bb06cc14c7ba7768901854c6cad 100644
--- a/lib/wsapi/complete_email_addition.js
+++ b/lib/wsapi/complete_email_addition.js
@@ -21,13 +21,40 @@ exports.process = function(req, res) {
//
// 1. you must already be authenticated as the user who initiated the verification
// 2. you must provide the password of the initiator.
- //
+
+ // TRANSITIONAL CODE COMMENT
+ // for issue 1000 we moved initial password selection to the browserid dialog (from
+ // the verification page). Rolling out this change causes some temporal pain.
+ // Outstannding verification links sent before the change was deployed will have
+ // email addition requests that require passwords without passwords in the stage table.
+ // When the verification page is loaded for
+ // these links, we prompt the user for a password. That password is sent up with
+ // the request. this code and comment should all be purged after the new code
+ // has been in production for 2 weeks.
+
+ var transitionalPassword = null;
+
+ // END TRANSITIONAL CODE COMMENT
+
+
db.authForVerificationSecret(req.body.token, function(err, initiator_hash, initiator_uid) {
if (err) {
logger.info("unknown verification secret: " + err);
return wsapi.databaseDown(res, err);
}
+ // TRANSITIONAL CODE
+ if (!initiator_hash) {
+ if (!req.body.pass) return httputils.authRequired(res, "password required");
+ var err = wsapi.checkPassword(req.body.pass);
+ if (err) {
+ logger.warn("invalid password received: " + err);
+ return httputils.badRequest(res, err);
+ }
+ transitionalPassword = req.body.pass;
+ postAuthentication();
+ } else
+ // END TRANSITIONAL CODE
if (req.session.userid === initiator_uid) {
postAuthentication();
} else if (typeof req.body.pass === 'string') {
@@ -53,6 +80,23 @@ exports.process = function(req, res) {
} else {
wsapi.authenticateSession(req.session, uid, 'password');
res.json({ success: true });
+
+ // TRANSITIONAL CODE
+ if (transitionalPassword) {
+ wsapi.bcryptPassword(transitionalPassword, function(err, hash) {
+ if (err) {
+ logger.warn("couldn't bcrypt pass for old verification link: " + err);
+ return;
+ }
+
+ db.updatePassword(uid, hash, function(err) {
+ if (err) {
+ logger.warn("couldn't bcrypt pass for old verification link: " + err);
+ }
+ });
+ });
+ }
+ // END TRANSITIONAL CODE
}
});
};
diff --git a/lib/wsapi/complete_user_creation.js b/lib/wsapi/complete_user_creation.js
index e507e0f95e1396268a4f180100f00191ee469656..5b87c3ec9af756562a37076fb88a2e9f09713f99 100644
--- a/lib/wsapi/complete_user_creation.js
+++ b/lib/wsapi/complete_user_creation.js
@@ -28,18 +28,36 @@ exports.process = function(req, res) {
// and then control a browserid account that they can use to prove they own
// the email address of the attacked.
+ // TRANSITIONAL CODE COMMENT
+ // for issue 1000 we moved initial password selection to the browserid dialog (from
+ // the verification page). Rolling out this change causes some temporal pain.
+ // Outstannding verification links sent before the change was deployed will have
+ // new user requests without passwords. When the verification page is loaded for
+ // these links, we prompt the user for a password. That password is sent up with
+ // the request. this code and comment should all be purged after the new code
+ // has been in production for 2 weeks.
+ // END TRANSITIONAL CODE COMMENT
+
// is this the same browser?
if (typeof req.session.pendingCreation === 'string' &&
req.body.token === req.session.pendingCreation) {
- postAuthentication();
+ return postAuthentication();
}
// is a password provided?
else if (typeof req.body.pass === 'string') {
return db.authForVerificationSecret(req.body.token, function(err, hash) {
+ // TRANSITIONAL CODE
+ // if hash is null, no password was provided during verification and
+ // this is an old-style verification. We accept the password and will
+ // update it after the verification is complete.
+ if (err == 'no password for user' || !hash) return postAuthentication();
+ // END TRANSITIONAL CODE
+
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);
@@ -47,7 +65,7 @@ exports.process = function(req, res) {
} else if (!success) {
return httputils.authRequired(res, "password mismatch");
} else {
- postAuthentication();
+ return postAuthentication();
}
});
});
@@ -65,19 +83,58 @@ exports.process = function(req, res) {
if (!known) return res.json({ success: false} );
- db.gotVerificationSecret(req.body.token, function(err, email, uid) {
- if (err) {
- logger.warn("couldn't complete email verification: " + err);
- wsapi.databaseDown(res, err);
- } else {
- // FIXME: not sure if we want to do this (ba)
- // at this point the user has set a password associated with an email address
- // that they've verified. We create an authenticated session.
- wsapi.authenticateSession(req.session, uid, 'password',
- config.get('ephemeral_session_duration_ms'));
- res.json({ success: true });
+ // TRANSITIONAL CODE
+ // user is authorized (1 or 2 above) OR user has no password set, in which
+ // case for a short time we'll accept the password provided with the verification
+ // link, and set it as theirs.
+ var transitionalPassword = null;
+
+ db.authForVerificationSecret(req.body.token, function(err, hash) {
+ if (err == 'no password for user' || !hash) {
+ if (!req.body.pass) return httputils.authRequired(res, "password required");
+ err = wsapi.checkPassword(req.body.pass);
+ if (err) {
+ logger.warn("invalid password received: " + err);
+ return httputils.badRequest(res, err);
+ }
+ transitionalPassword = req.body.pass;
}
+ completeCreation();
});
+ // END TRANSITIONAL CODE
+
+ function completeCreation() {
+ db.gotVerificationSecret(req.body.token, function(err, email, uid) {
+ if (err) {
+ logger.warn("couldn't complete email verification: " + err);
+ wsapi.databaseDown(res, err);
+ } else {
+ // FIXME: not sure if we want to do this (ba)
+ // at this point the user has set a password associated with an email address
+ // that they've verified. We create an authenticated session.
+ wsapi.authenticateSession(req.session, uid, 'password',
+ config.get('ephemeral_session_duration_ms'));
+ res.json({ success: true });
+
+ // TRANSITIONAL CODE
+ if (transitionalPassword) {
+ wsapi.bcryptPassword(transitionalPassword, function(err, hash) {
+ if (err) {
+ logger.warn("couldn't bcrypt pass for old verification link: " + err);
+ return;
+ }
+
+ db.updatePassword(uid, hash, function(err) {
+ if (err) {
+ logger.warn("couldn't bcrypt pass for old verification link: " + err);
+ }
+ });
+ });
+ }
+ // END TRANSITIONAL CODE
+ }
+ });
+ }
});
}
};
diff --git a/lib/wsapi/email_for_token.js b/lib/wsapi/email_for_token.js
index f492bcff595e0978c7f96e24f31b287fdceb8851..ac0a5d9543514ac4cb74c1ab3986eded0ad63421 100644
--- a/lib/wsapi/email_for_token.js
+++ b/lib/wsapi/email_for_token.js
@@ -19,7 +19,7 @@ exports.args = ['token'];
exports.i18n = false;
exports.process = function(req, res) {
- db.emailForVerificationSecret(req.query.token, function(err, email, uid) {
+ db.emailForVerificationSecret(req.query.token, function(err, email, uid, hash) {
if (err) {
if (err === 'database unavailable') {
httputils.serviceUnavailable(res, err);
@@ -30,24 +30,64 @@ exports.process = function(req, res) {
});
}
} else {
- // 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;
+ 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;
+ if (uid && req.session.userid === uid) {
+ must_auth = false;
+ }
+ else if (!uid && typeof req.session.pendingCreation === 'string' &&
+ req.query.token === req.session.pendingCreation) {
+ must_auth = false;
+ }
+
+ res.json({
+ success: true,
+ email: email,
+ must_auth: must_auth
+ });
+ }
+
+ // backwards compatibility - issue #1592
+ // if there is no password in the user record, and no password in the staged
+ // table, then we require a password be fetched from the user upon verification.
+ // these checks are temporary and should disappear in 1 trains time.
+ function needsPassword() {
+ // no password is set neither in the user table nor in the staged record.
+ // the user must pick a password
+ res.json({
+ success: true,
+ email: email,
+ needs_password: true
+ });
}
- else if (!uid && typeof req.session.pendingCreation === 'string' &&
- req.query.token === req.session.pendingCreation) {
- must_auth = false;
+
+ if (!hash) {
+ if (!uid) {
+ needsPassword();
+ } else {
+ db.checkAuth(uid, function(err, hash) {
+ if (err) {
+ return res.json({
+ success: false,
+ reason: err
+ });
+ }
+
+ if (!hash) {
+ needsPassword();
+ } else {
+ checkMustAuth();
+ }
+ });
+ }
+ } else {
+ checkMustAuth();
}
- res.json({
- success: true,
- email: email,
- must_auth: must_auth
- });
}
});
};
diff --git a/resources/static/css/style.css b/resources/static/css/style.css
index 86b0b219f6f3b529e0ba677130ce4f0d0271ad76..802295daca104265ab0c2bf4feb716cb39357683 100644
--- a/resources/static/css/style.css
+++ b/resources/static/css/style.css
@@ -669,7 +669,7 @@ h1 {
margin-bottom: 10px;
}
-.siteinfo, #congrats, .password_entry, .enter_password .hint, #unknown_secondary, #primary_verify, .verify_primary .submit {
+.siteinfo, #congrats, .password_entry, #verify_password, .enter_password .hint, #unknown_secondary, #primary_verify, .verify_primary .submit {
display: none;
}
@@ -677,7 +677,7 @@ h1 {
float: left;
}
-.enter_password .password_entry, .known_secondary .password_entry,
+.enter_password .password_entry, .enter_verify_password #verify_password, .known_secondary .password_entry,
.unknown_secondary #unknown_secondary, .verify_primary #verify_primary {
display: block;
}
diff --git a/resources/static/pages/verify_secondary_address.js b/resources/static/pages/verify_secondary_address.js
index 2ccfb0c804ec90c672114c6235fcbefee8b3e0df..607a5621c80d4b477de7ea572c4008b007d6a30a 100644
--- a/resources/static/pages/verify_secondary_address.js
+++ b/resources/static/pages/verify_secondary_address.js
@@ -17,6 +17,7 @@ BrowserID.verifySecondaryAddress = (function() {
validation = bid.Validation,
token,
sc,
+ needsPassword,
mustAuth,
verifyFunction;
@@ -36,7 +37,11 @@ BrowserID.verifySecondaryAddress = (function() {
function submit(oncomplete) {
var pass = dom.getInner("#password") || undefined,
- valid = !mustAuth || validation.password(pass);
+ vpass = dom.getInner("#vpassword") || undefined,
+ valid = (!needsPassword ||
+ validation.passwordAndValidationPassword(pass, vpass))
+ && (!mustAuth ||
+ validation.password(pass));
if (valid) {
user[verifyFunction](token, pass, function(info) {
@@ -56,13 +61,25 @@ BrowserID.verifySecondaryAddress = (function() {
if(info) {
showRegistrationInfo(info);
+ needsPassword = info.needs_password;
mustAuth = info.must_auth;
- if (mustAuth) {
+ if (needsPassword) {
+ // This is a fix for legacy users who started the user creation
+ // process without setting their password in the dialog. If the user
+ // needs a password, they must set it now. Once all legacy users are
+ // verified or their links invalidated, this flow can be removed.
+ dom.addClass("body", "enter_password");
+ dom.addClass("body", "enter_verify_password");
+ complete(oncomplete, true);
+ }
+ else if (mustAuth) {
+ // These are users who have set their passwords inside of the dialog.
dom.addClass("body", "enter_password");
complete(oncomplete, true);
}
else {
+ // These are users who do not have to set their passwords at all.
submit(oncomplete);
}
}
diff --git a/resources/static/test/cases/pages/verify_secondary_address.js b/resources/static/test/cases/pages/verify_secondary_address.js
index a770237db60734b31e39a6d9f82553bb917b9bc5..d7eacaa574bb8d489c7a5897d0913c2060d43a6b 100644
--- a/resources/static/test/cases/pages/verify_secondary_address.js
+++ b/resources/static/test/cases/pages/verify_secondary_address.js
@@ -116,6 +116,7 @@
xhr.useResult("mustAuth");
createController(config, function() {
xhr.useResult("valid");
+ testHasClass("body", "enter_password");
controller.submit(function(status) {
equal(status, true, "correct status");
testHasClass("body", "complete");
@@ -134,4 +135,88 @@
});
});
+ asyncTest("must set password, successful login", function() {
+ xhr.useResult("needsPassword");
+ createController(config, function() {
+ xhr.useResult("valid");
+
+ $("#password").val("password");
+ $("#vpassword").val("password");
+
+ testHasClass("body", "enter_password");
+ testHasClass("body", "enter_verify_password");
+
+ controller.submit(function(status) {
+ equal(status, true, "correct status");
+ testHasClass("body", "complete");
+ start();
+ });
+ });
+ });
+
+ asyncTest("must set password, too short a password", function() {
+ xhr.useResult("needsPassword");
+ createController(config, function() {
+ xhr.useResult("valid");
+
+ $("#password").val("pass");
+ $("#vpassword").val("pass");
+
+ controller.submit(function(status) {
+ equal(status, false, "correct status");
+ testHelpers.testTooltipVisible();
+ start();
+ });
+ });
+ });
+
+ asyncTest("must set password, too long a password", function() {
+ xhr.useResult("needsPassword");
+ createController(config, function() {
+ xhr.useResult("valid");
+
+ var pass = testHelpers.generateString(81);
+ $("#password").val(pass);
+ $("#vpassword").val(pass);
+
+ controller.submit(function(status) {
+ equal(status, false, "correct status");
+ testHelpers.testTooltipVisible();
+ start();
+ });
+ });
+ });
+
+ asyncTest("must set password, missing verification password", function() {
+ xhr.useResult("needsPassword");
+ createController(config, function() {
+ xhr.useResult("valid");
+
+ $("#password").val("password");
+ $("#vpassword").val("");
+
+ controller.submit(function(status) {
+ equal(status, false, "correct status");
+ testHelpers.testTooltipVisible();
+ start();
+ });
+ });
+ });
+
+ asyncTest("must set password, mismatched passwords", function() {
+ xhr.useResult("needsPassword");
+ createController(config, function() {
+ xhr.useResult("valid");
+
+ $("#password").val("password");
+ $("#vpassword").val("password1");
+
+ controller.submit(function(status) {
+ equal(status, false, "correct status");
+ testHelpers.testTooltipVisible();
+ start();
+ });
+ });
+ });
+
}());
diff --git a/resources/static/test/mocks/xhr.js b/resources/static/test/mocks/xhr.js
index 8b505f997d4680903d922e8de6e2a90d78f08cc1..12c6122513905fee03db42473b6173355cc1869e 100644
--- a/resources/static/test/mocks/xhr.js
+++ b/resources/static/test/mocks/xhr.js
@@ -34,6 +34,7 @@ BrowserID.Mocks.xhr = (function() {
"get /wsapi/session_context contextAjaxError": undefined,
"get /wsapi/email_for_token?token=token valid": { email: "testuser@testuser.com" },
"get /wsapi/email_for_token?token=token mustAuth": { email: "testuser@testuser.com", must_auth: true },
+ "get /wsapi/email_for_token?token=token needsPassword": { email: "testuser@testuser.com", needs_password: true },
"get /wsapi/email_for_token?token=token invalid": { success: false },
"post /wsapi/authenticate_user valid": { success: true, userid: 1 },
"post /wsapi/authenticate_user invalid": { success: false },
diff --git a/resources/views/add_email_address.ejs b/resources/views/add_email_address.ejs
index fa6fbd891b20f14c23532b9497da17adf3127e16..45b711cec7d8d7b53a9da83b7f2c1f933de87ca4 100644
--- a/resources/views/add_email_address.ejs
+++ b/resources/views/add_email_address.ejs
@@ -31,6 +31,20 @@
<%= gettext('Password must be between 8 and 80 characters long.') %>
</div>
</li>
+
+ <li class="password_entry" id="verify_password">
+ <label class="serif" for="vpassword"><%= gettext('Verify Password') %></label>
+ <input class="sans" id="vpassword" placeholder="<%= gettext('Repeat Password') %>" type="password" maxlength="80">
+
+ <div id="vpassword_required" class="tooltip" for="vpassword">
+ <%= gettext('Verification password is required.') %>
+ </div>
+
+ <div class="tooltip" id="passwords_no_match" for="vpassword">
+ <%= gettext ('Passwords do not match.') %>
+ </div>
+
+ </li>
</ul>
<div class="submit cf password_entry">
diff --git a/resources/views/verify_email_address.ejs b/resources/views/verify_email_address.ejs
index a73c80ead95230830d13bc8f218e5e84e878c314..1f275d3fe8c3362f98b6360c58484d401e2a9e9c 100644
--- a/resources/views/verify_email_address.ejs
+++ b/resources/views/verify_email_address.ejs
@@ -18,6 +18,7 @@
<label class="serif" for="email"><%= gettext('Email Address') %></label>
<input class="youraddress sans" id="email" placeholder="<%= gettext('Your Email') %>" type="email" value="" disabled="disabled" maxlength="254" />
</li>
+
<li class="password_entry">
<label class="serif" for="password"><%= gettext('Password') %></label>
<input class="sans" id="password" placeholder="<%= gettext('Your Password') %>" type="password" autofocus maxlength=80 />
@@ -30,6 +31,20 @@
<%= gettext('Password must be between 8 and 80 characters long.') %>
</div>
</li>
+
+ <li class="password_entry" id="verify_password">
+ <label class="serif" for="vpassword"><%= gettext('Verify Password') %></label>
+ <input class="sans" id="vpassword" placeholder="<%= gettext('Repeat Password') %>" type="password" maxlength="80">
+
+ <div id="vpassword_required" class="tooltip" for="vpassword">
+ <%= gettext('Verification password is required.') %>
+ </div>
+
+ <div class="tooltip" id="passwords_no_match" for="vpassword">
+ <%= gettext ('Passwords do not match.') %>
+ </div>
+
+ </li>
</ul>
<div class="submit cf password_entry">