diff --git a/ChangeLog b/ChangeLog index cb72ff6b1ac02c05ffa8702c3f6e487392232287..d859a4af8449740a16e7b233a7dec5141bf90639 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,21 @@ -train-2012.04.25 (in progress): +train-2012.05.09 (in progress): * +train-2012.04.27: + * Observer API updated, still experimental. + * A more responsive dialog design that scales to different screen sizes: #1101, #1317 + * Improved consistency of links: #702, #1453 + * Test improvements: #1246, #1437, #1488, #1464 + * Allow underscores in email address domain and site origin: #1454 + * Fixes to per site last-used-email state maintenence: #968 + * Never ask a user if this "is your computer" in a session that involves email verificatino: #1446 + * Remove placeholder text from change password inputs: #1461 + * General cleanup: #1449, #1396 + * Logging improvements: #1383 + * IE visual improvements for error screens: #1485, #1390, #1496 + * Improved checks for disabled cookies: #1418, #1484 + * Fix bug where if user pauses for 2 minutes on "is this your computer" the generated assertion is invalid: #1460 + train-2012.04.11: * New BrowserID "Observer" API implemented in experimental status: #912 * Implement variable length sessions and explicit user confirmation to improve saftey on public terminals/shared computers: #884 @@ -17,6 +32,12 @@ train-2012.04.11: * developers link now points to MDN: #1397 * fix issues that were introduced while implementing the above features: #1349, #1348, #1354, #1357, #1374, #1399, #1400, #1408, #1395, #1406, #1405, #1390, #1391 * (hotfix 2012.04.12) return 400 rather than 500 for invalid params to stage_user or stage_email: #1429 + * (hotfix 2012.04.12) fix broken string, "is this your computer" was broken into two fragments: #1425 + * (hotfix 2012.04.16) fix API regression that would cause javascript error when .get() invoked without second arg: #1442 + * (hotfix 2012.04.16) update load_gen to new server apis that require an `ephemeral` argument: #1436 + * (hotfix 2012.04.17) fix broken reset password flow - button was non-responsive in dialog: #1440 + * (hotfix 2012.04.17) mitigate errors seen when adding a secondary email to an acct with only primary emails: #1445 + * (hotfix 2012.04.18) fix error where under certain conditions user could see an error immediately after authenticating: #1449 train-2012.03.28: * work towards better user messaging for when cookies are disabled: #1167, #1302 diff --git a/lib/static_resources.js b/lib/static_resources.js index ff7654e2687ff3804034a424dadeea743927d08f..8f88114a98a55ebb9388d63900121d624762dca8 100644 --- a/lib/static_resources.js +++ b/lib/static_resources.js @@ -97,7 +97,7 @@ var dialog_js = und.flatten([ '/dialog/controllers/verify_primary_user.js', '/dialog/controllers/provision_primary_user.js', '/dialog/controllers/primary_user_provisioned.js', - '/dialog/controllers/email_chosen.js', + '/dialog/controllers/generate_assertion.js', '/dialog/controllers/is_this_your_computer.js', '/dialog/start.js' diff --git a/lib/wsapi/complete_email_addition.js b/lib/wsapi/complete_email_addition.js index 1359b49c379efab649d942f988f37f8c466c7dfa..6e7dd2a4df4ce2116142fe5603d848ae43f9cccf 100644 --- a/lib/wsapi/complete_email_addition.js +++ b/lib/wsapi/complete_email_addition.js @@ -41,27 +41,32 @@ exports.process = function(req, res) { }); } - db.gotVerificationSecret(req.body.token, req.body.pass, function(e, email, uid) { + // got verification secret's second paramter is a password. That password + // will only be used on new account creation. Because we know this is not + // a new account, we don't provide it. + db.gotVerificationSecret(req.body.token, "", function(e, email, uid) { if (e) { logger.warn("couldn't complete email verification: " + e); wsapi.databaseDown(res, e); } else { // now do we need to set the password? if (r.needs_password && req.body.pass) { + // requiring the client to wait until the bcrypt process is complete here + // exacerbates race conditions in front-end code. We'll return success early, + // here, then update the password after the fact. The worst thing that could + // happen is that password update could fail (due to extreme load), and the + // user will have to reset their password. + wsapi.authenticateSession(req.session, uid, 'password'); + res.json({ success: true }); + wsapi.bcryptPassword(req.body.pass, function(err, hash) { if (err) { logger.warn("couldn't bcrypt password during email verification: " + err); - return res.json({ success: false }); + return; } db.updatePassword(uid, hash, function(err) { if (err) { logger.warn("couldn't update password during email verification: " + err); - wsapi.databaseDown(res, err); - } else { - // XXX: what if our software 503s? User doesn't get a password set and - // cannot change it. - wsapi.authenticateSession(req.session, uid, 'password'); - res.json({ success: !err }); } }); }); diff --git a/resources/static/dialog/controllers/actions.js b/resources/static/dialog/controllers/actions.js index 2a6ea48508b470b26395f356663510167d5e4110..08239b6befc50e5b56547823020dd07a5b7ef5a1 100644 --- a/resources/static/dialog/controllers/actions.js +++ b/resources/static/dialog/controllers/actions.js @@ -105,16 +105,6 @@ BrowserID.Modules.Actions = (function() { startRegCheckService.call(this, info, "waitForEmailValidation", "email_confirmed"); }, - doEmailConfirmed: function(info) { - var self=this; - // yay! now we need to produce an assertion. - user.getAssertion(info.email, user.getOrigin(), function(assertion) { - self.publish("assertion_generated", { - assertion: assertion - }); - }, self.getErrorDialog(errors.getAssertion)); - }, - doAssertionGenerated: function(info) { // Clear onerror before the call to onsuccess - the code to onsuccess // calls window.close, which would trigger the onerror callback if we @@ -160,8 +150,8 @@ BrowserID.Modules.Actions = (function() { startService("is_this_your_computer", info); }, - doEmailChosen: function(info) { - startService("email_chosen", info); + doGenerateAssertion: function(info) { + startService("generate_assertion", info); } }); diff --git a/resources/static/dialog/controllers/check_registration.js b/resources/static/dialog/controllers/check_registration.js index 7e4e90ab8e5de2856bfc2c87e518ef28cace42c1..3389f3f9e0ca7b47bb22e796e3c8135c91af9ce0 100644 --- a/resources/static/dialog/controllers/check_registration.js +++ b/resources/static/dialog/controllers/check_registration.js @@ -40,7 +40,10 @@ BrowserID.Modules.CheckRegistration = (function() { }); } else if (status === "mustAuth") { - self.close("authenticate", { email: self.email }); + user.addressInfo(self.email, function(info) { + self.close("authenticate", info); + }); + oncomplete && oncomplete(); } }, self.getErrorDialog(errors.registration, oncomplete)); diff --git a/resources/static/dialog/controllers/dialog.js b/resources/static/dialog/controllers/dialog.js index 9d5c2693bbeeacea1e703d51b0403967feff70b3..1d3e57b872a4fc526677fe7189401bfc9a688f71 100644 --- a/resources/static/dialog/controllers/dialog.js +++ b/resources/static/dialog/controllers/dialog.js @@ -86,7 +86,7 @@ BrowserID.Modules.Dialog = (function() { if (/^http/.test(url)) u = URLParse(url); else if (/^\//.test(url)) u = URLParse(origin + url); else throw "relative urls not allowed: (" + url + ")"; - return u.validate().normalize().toString(); + return encodeURI(u.validate().normalize().toString()); } var Dialog = bid.Modules.PageModule.extend({ diff --git a/resources/static/dialog/controllers/email_chosen.js b/resources/static/dialog/controllers/generate_assertion.js similarity index 84% rename from resources/static/dialog/controllers/email_chosen.js rename to resources/static/dialog/controllers/generate_assertion.js index 8ce4747938702165bd391cb447fbc4176fa9e3fa..eef1b2d0d3ccef38dddb34f94355064ee49c3bf3 100644 --- a/resources/static/dialog/controllers/email_chosen.js +++ b/resources/static/dialog/controllers/generate_assertion.js @@ -3,7 +3,7 @@ /* 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/. */ -BrowserID.Modules.EmailChosen = (function() { +BrowserID.Modules.GenerateAssertion = (function() { "use strict"; var bid = BrowserID, @@ -12,7 +12,7 @@ BrowserID.Modules.EmailChosen = (function() { user = bid.User, storage = bid.Storage; - var EmailChosen = bid.Modules.PageModule.extend({ + var GenerateAssertion = bid.Modules.PageModule.extend({ start: function(options) { var email = options.email, self=this; @@ -28,9 +28,9 @@ BrowserID.Modules.EmailChosen = (function() { } }); - sc = EmailChosen.sc; + sc = GenerateAssertion.sc; - return EmailChosen; + return GenerateAssertion; }()); diff --git a/resources/static/dialog/css/popup.css b/resources/static/dialog/css/popup.css index a8c40097bc5cecd227972408a8d0ba270819f241..4d456ea6718a94ede2032010cf41faa37779c234 100644 --- a/resources/static/dialog/css/popup.css +++ b/resources/static/dialog/css/popup.css @@ -13,7 +13,13 @@ h2 { header, footer { position: absolute; + /* The *padding is a fix for IE6 and IE7 showing scroll bars in the + * unsupported dialog. Since IE6 and IE7 do not support box-sizing: + * border-box, the left and right padding cause these versions of IE to + * overflow the dialog box. + */ padding: 20px; + *padding: 20px 0; z-index: 2; } @@ -109,8 +115,14 @@ section > .contents { right: 0; top: 61px; bottom: 61px; - /* Fix for IE6 not displaying the unsupported dialog correctly */ - _width: 100%; + /* Fix for IE6 not displaying the unsupported dialog correctly. IE6 by + * default sets the height and width of the element to 0 meaning nothing + * shows up on the screen. + * Note, these are magic numbers that depend on the width and height of the + * dialog. The height also depends on the height of the header and footer. + */ + _width: 682px; + _height: 250px; } #wait, #error, #delay { diff --git a/resources/static/dialog/resources/state.js b/resources/static/dialog/resources/state.js index 1d8bcfdbceb2c2f6124112342c0d0ff39bda75be..a96d5b8881da56ca8efda4538b355de817b3cf7b 100644 --- a/resources/static/dialog/resources/state.js +++ b/resources/static/dialog/resources/state.js @@ -23,7 +23,7 @@ BrowserID.State = (function() { handleState = self.subscribe.bind(self), redirectToState = mediator.publish.bind(mediator), startAction = function(save, msg, options) { - if(typeof save !== "boolean") { + if (typeof save !== "boolean") { options = msg; msg = save; save = true; @@ -46,7 +46,7 @@ BrowserID.State = (function() { // Invalid format startAction("doError", "invalid_required_email", {email: requiredEmail}); } - else if(info.email && info.type === "primary") { + else if (info.email && info.type === "primary") { primaryVerificationInfo = info; redirectToState("primary_user", info); } @@ -101,7 +101,7 @@ BrowserID.State = (function() { handleState("user_confirmed", function() { self.email = self.stagedEmail; - startAction("doEmailConfirmed", { email: self.stagedEmail} ); + redirectToState("email_chosen", { email: self.stagedEmail} ); }); handleState("primary_user", function(msg, info) { @@ -109,7 +109,7 @@ BrowserID.State = (function() { email = info.email; var idInfo = storage.getEmail(email); - if(idInfo && idInfo.cert) { + if (idInfo && idInfo.cert) { redirectToState("primary_user_ready", info); } else { @@ -123,6 +123,10 @@ BrowserID.State = (function() { handleState("primary_user_provisioned", function(msg, info) { info = info || {}; info.add = !!addPrimaryUser; + // The user is is authenticated with their IdP. Two possibilities exist + // for the email - 1) create a new account or 2) add address to the + // existing account. If the user is authenticated with BrowserID, #2 + // will happen. If not, #1. startAction("doPrimaryUserProvisioned", info); }); @@ -135,12 +139,12 @@ BrowserID.State = (function() { tosURL: self.tosURL }); - if(primaryVerificationInfo) { + if (primaryVerificationInfo) { primaryVerificationInfo = null; - if(requiredEmail) { + if (requiredEmail) { startAction("doCannotVerifyRequiredPrimary", info); } - else if(info.add) { + else if (info.add) { // Add the pick_email in case the user cancels the add_email screen. // The user needs something to go "back" to. redirectToState("pick_email"); @@ -186,22 +190,26 @@ BrowserID.State = (function() { complete(info.complete); } - if(idInfo) { - if(idInfo.type === "primary") { - if(idInfo.cert) { - startAction("doEmailChosen", info); + if (idInfo) { + if (idInfo.type === "primary") { + if (idInfo.cert) { + // Email is a primary and the cert is available - the user can log + // in without authenticating with the IdP. All invalid/expired + // certs are assumed to have been checked and removed by this + // point. + redirectToState("email_valid_and_ready", info); } else { - // If the email is a primary, and their cert is not available, - // throw the user down the primary flow. - // Doing so will catch cases where the primary certificate is expired + // If the email is a primary and the cert is not available, + // throw the user down the primary flow. The primary flow will + // catch cases where the primary certificate is expired // and the user must re-verify with their IdP. redirectToState("primary_user", info); } } else { user.checkAuthentication(function(authentication) { - if(authentication === "assertion") { + if (authentication === "assertion") { // user must authenticate with their password, kick them over to // the required email screen to enter the password. startAction("doAuthenticateWithRequiredEmail", { @@ -212,7 +220,7 @@ BrowserID.State = (function() { }); } else { - startAction("doEmailChosen", info); + redirectToState("email_valid_and_ready", info); } oncomplete(); }, oncomplete); @@ -223,59 +231,86 @@ BrowserID.State = (function() { } }); - handleState("notme", function() { - startAction("doNotMe"); - }); - - handleState("logged_out", function() { - redirectToState("authenticate"); + handleState("email_valid_and_ready", function(msg, info) { + // this state is only called after all checking is done on the email + // address. For secondaries, this means the email has been validated and + // the user is authenticated to the password level. For primaries, this + // means the user is authenticated with their IdP and the certificate for + // the address is valid. An assertion can be generated, but first we + // may have to check whether the user owns the computer. + user.shouldAskIfUsersComputer(function(shouldAsk) { + if (shouldAsk) { + redirectToState("is_this_your_computer", info); + } + else { + redirectToState("generate_assertion", info); + } + }); }); - handleState("authenticated", function(msg, info) { - redirectToState("email_chosen", info); + handleState("is_this_your_computer", function(msg, info) { + // We have to confirm the user's computer ownership status. Save off + // the selected email info for when the user_computer_status_set is + // complete so that the user can continue the flow with the correct + // email address. + self.chosenEmailInfo = info; + startAction("doIsThisYourComputer", info); }); - handleState("forgot_password", function(msg, info) { - // forgot password initiates the forgotten password flow. - startAction(false, "doForgotPassword", info); + handleState("user_computer_status_set", function(msg, info) { + // User's status has been confirmed, an assertion can safely be + // generated as there are no more delays introduced by user interaction. + // Use the email address that was stored in the call to + // "is_this_your_computer". + var emailInfo = self.chosenEmailInfo; + self.chosenEmailInfo = null; + redirectToState("generate_assertion", emailInfo); }); - handleState("reset_password", function(msg, info) { - // reset password says the password has been reset, now waiting for - // confirmation. - startAction(false, "doResetPassword", info); + handleState("generate_assertion", function(msg, info) { + startAction("doGenerateAssertion", info); }); handleState("assertion_generated", function(msg, info) { self.success = true; if (info.assertion !== null) { - user.shouldAskIfUsersComputer(function(shouldAsk) { - if (shouldAsk) { - // We have to confirm the user's status - self.assertion_info = info; - redirectToState("is_this_your_computer", info); - } - else { - storage.setLoggedIn(user.getOrigin(), self.email); - startAction("doAssertionGenerated", { assertion: info.assertion, email: self.email }); - } - }); + // XXX TODO - move the setLoggedIn to the getAssertion perhaps? + storage.setLoggedIn(user.getOrigin(), self.email); + startAction("doAssertionGenerated", { assertion: info.assertion, email: self.email }); } else { redirectToState("pick_email"); } }); - handleState("is_this_your_computer", function(msg, info) { - startAction("doIsThisYourComputer", info); + handleState("notme", function() { + startAction("doNotMe"); }); - handleState("user_computer_status_set", function(msg, info) { - // User's status has been confirmed, redirect them back to the - // assertion_generated state with the stored assertion_info - var assertion_info = self.assertion_info; - self.assertion_info = null; - redirectToState("assertion_generated", assertion_info); + handleState("logged_out", function() { + redirectToState("authenticate"); + }); + + handleState("authenticated", function(msg, info) { + redirectToState("email_chosen", info); + }); + + handleState("forgot_password", function(msg, info) { + // forgot password initiates the forgotten password flow. + startAction(false, "doForgotPassword", info); + }); + + handleState("reset_password", function(msg, info) { + info = info || {}; + // reset_password says the user has confirmed that they want to + // reset their password. doResetPassword will attempt to invoke + // the create_user wsapi. If the wsapi call is successful, + // the user will be shown the "go verify your account" message. + + // We have to save the staged email address here for when the user + // verifies their account and user_confirmed is called. + self.stagedEmail = info.email; + startAction(false, "doResetPassword", info); }); handleState("add_email", function(msg, info) { @@ -294,7 +329,7 @@ BrowserID.State = (function() { }); handleState("email_confirmed", function() { - startAction("doEmailConfirmed", { email: self.stagedEmail} ); + redirectToState("email_chosen", { email: self.stagedEmail} ); }); handleState("cancel_state", function(msg, info) { diff --git a/resources/static/dialog/start.js b/resources/static/dialog/start.js index 970034d1afe9fea098c09969146267db1c8ac527..45e04d4fb15c4d2f7c590eed8a4a213e9b78bfbe 100644 --- a/resources/static/dialog/start.js +++ b/resources/static/dialog/start.js @@ -30,7 +30,7 @@ moduleManager.register("verify_primary_user", modules.VerifyPrimaryUser); moduleManager.register("provision_primary_user", modules.ProvisionPrimaryUser); moduleManager.register("primary_user_provisioned", modules.PrimaryUserProvisioned); - moduleManager.register("email_chosen", modules.EmailChosen); + moduleManager.register("generate_assertion", modules.GenerateAssertion); moduleManager.register("xhr_delay", modules.XHRDelay); moduleManager.register("xhr_disable_form", modules.XHRDisableForm); diff --git a/resources/static/dialog/views/is_this_your_computer.ejs b/resources/static/dialog/views/is_this_your_computer.ejs index 2ab5bd1789788cf4abb72f2860898db25bd4982c..d592e7714619c878d42a04f96b45c2c55499e40e 100644 --- a/resources/static/dialog/views/is_this_your_computer.ejs +++ b/resources/static/dialog/views/is_this_your_computer.ejs @@ -12,8 +12,7 @@ <p> <button class="this_is_not_my_computer negative" tabindex="3"><%= gettext('no') %></button> - <%= gettext('If you\'re at a public computer such as a library or internet cafe, we\'ll ask you ' + - 'for your password again in an hour.') %> + <%= gettext('If you\'re at a public computer such as a library or internet cafe, we\'ll ask you for your password again in an hour.') %> </p> </div> diff --git a/resources/static/include_js/include.js b/resources/static/include_js/include.js index 567529fa2d3f785f38d4eaaa71f16adf20e614f7..06c655aebd8df128581cc07f464d2e35421861ed 100644 --- a/resources/static/include_js/include.js +++ b/resources/static/include_js/include.js @@ -958,10 +958,16 @@ } } - var commChan; + var commChan, + browserSupported = BrowserSupport.isSupported(); // this is for calls that are non-interactive function _open_hidden_iframe() { + // If this is an unsupported browser, do not even attempt to add the + // IFRAME as doing so will cause an exception to be thrown in IE6 and IE7 + // from within the communication_iframe. + if(!browserSupported) return; + try { if (!commChan) { var doc = window.document; @@ -996,8 +1002,8 @@ }); } } catch(e) { - // channel building failed! this is probably an unsupported browser. let's ignore - // the error and allow higher level code to handle user messaging. + // channel building failed! let's ignore the error and allow higher + // level code to handle user messaging. commChan = undefined; } } @@ -1122,8 +1128,8 @@ logout: function(callback) { // allocate iframe if it is not allocated _open_hidden_iframe(); - // send logout message - commChan.notify({ method: 'logout' }); + // send logout message if the commChan exists + if (commChan) commChan.notify({ method: 'logout' }); if (typeof callback === 'function') setTimeout(callback, 0); }, // get an assertion diff --git a/resources/static/shared/network.js b/resources/static/shared/network.js index 656dcfa139855458ab2e9f5df3c1b6726fc40159..2f7049e6ac98739b1f0ce8c2311fa0226cca8d40 100644 --- a/resources/static/shared/network.js +++ b/resources/static/shared/network.js @@ -258,6 +258,11 @@ BrowserID.Network = (function() { url: "/wsapi/user_creation_status?email=" + encodeURIComponent(email), success: function(status, textStatus, jqXHR) { if (status.status === 'complete' && status.userid) { + // The user at this point can ONLY be logged in with password + // authentication. Once the registration is complete, that means + // the server has updated the user's cookies and the user is + // officially authenticated. + auth_status = 'password'; setUserID(status.userid); } complete(onComplete, status.status); diff --git a/resources/static/shared/storage.js b/resources/static/shared/storage.js index 6e131ba49703237af34702405d9635bc510cc78f..c1a6ec6f5426d2e3d4c01687571b6d92c04f09f5 100644 --- a/resources/static/shared/storage.js +++ b/resources/static/shared/storage.js @@ -12,8 +12,9 @@ BrowserID.Storage = (function() { } catch(e) { // Fx with cookies disabled will except while trying to access - // localStorage. Because of this, and because the new API requires access - // to localStorage + // localStorage. IE6/IE7 will just plain blow up because they have no + // notion of localStorage. Because of this, and because the new API + // requires access to localStorage, create a fake one with removeItem. storage = { removeItem: function(key) { this[key] = null; diff --git a/resources/static/test/cases/controllers/actions.js b/resources/static/test/cases/controllers/actions.js index c670633e8a71193cb9443778085c5702d2f4aac9..0e0881e2748005970320b4f1d6ac170276af0a04 100644 --- a/resources/static/test/cases/controllers/actions.js +++ b/resources/static/test/cases/controllers/actions.js @@ -10,13 +10,30 @@ user = bid.User, controller, el, - testHelpers = bid.TestHelpers; + testHelpers = bid.TestHelpers, + TEST_EMAIL = "testuser@testuser.com"; function createController(config) { controller = BrowserID.Modules.Actions.create(); controller.start(config); } + function testActionStartsModule(actionName, actionOptions, expectedModule) { + createController({ + ready: function() { + var error; + try { + controller[actionName](actionOptions); + } catch(e) { + error = e; + } + + equal(error, "module not registered for " + expectedModule, "correct service started"); + start(); + } + }); + } + module("controllers/actions", { setup: function() { testHelpers.setup(); @@ -55,35 +72,13 @@ }); asyncTest("doProvisionPrimaryUser - start the provision_primary_user service", function() { - createController({ - ready: function() { - var error; - try { - controller.doProvisionPrimaryUser({email: "testuser@testuser.com"}); - } catch(e) { - error = e; - } - - equal(error, "module not registered for provision_primary_user", "correct service started"); - start(); - } - }); + testActionStartsModule("doProvisionPrimaryUser", {email: TEST_EMAIL}, + "provision_primary_user"); }); asyncTest("doVerifyPrimaryUser - start the verify_primary_user service", function() { - createController({ - ready: function() { - var error; - try { - controller.doVerifyPrimaryUser(); - } catch(e) { - error = e; - } - - equal(error, "module not registered for verify_primary_user", "correct service started"); - start(); - } - }); + testActionStartsModule("doVerifyPrimaryUser", {}, + "verify_primary_user"); }); asyncTest("doCannotVerifyRequiredPrimary - show the error screen", function() { @@ -99,83 +94,23 @@ }); asyncTest("doPrimaryUserProvisioned - start the primary_user_verified service", function() { - createController({ - ready: function() { - var error; - try { - controller.doPrimaryUserProvisioned(); - } catch(e) { - error = e; - } - - equal(error, "module not registered for primary_user_provisioned", "correct service started"); - start(); - } - }); - }); - - asyncTest("doEmailChosen - start the email_chosen service", function() { - createController({ - ready: function() { - var error; - try { - controller.doEmailChosen({email: "testuser@testuser.com"}); - } catch(e) { - error = e; - } - - equal(error, "module not registered for email_chosen", "correct service started"); - start(); - } - }); + testActionStartsModule("doPrimaryUserProvisioned", {}, + "primary_user_provisioned"); }); asyncTest("doConfirmUser - start the check_registration service", function() { - createController({ - ready: function() { - var error; - try { - controller.doConfirmUser({email: "testuser@testuser.com"}); - } catch(e) { - error = e; - } - - equal(error, "module not registered for check_registration", "correct service started"); - start(); - } - }); + testActionStartsModule("doConfirmUser", {email: TEST_EMAIL}, + "check_registration"); }); asyncTest("doConfirmEmail - start the check_registration service", function() { - createController({ - ready: function() { - var error; - try { - controller.doConfirmEmail({email: "testuser@testuser.com"}); - } catch(e) { - error = e; - } - - equal(error, "module not registered for check_registration", "correct service started"); - start(); - } - }); - + testActionStartsModule("doConfirmEmail", {email: TEST_EMAIL}, + "check_registration"); }); - asyncTest("doEmailConfirmed - generate an assertion for the email", function() { - createController({ - ready: function() { - testHelpers.register("assertion_generated", function(msg, info) { - ok(info.assertion, "assertion generated"); - start(); - }); - - user.syncEmailKeypair("testuser@testuser.com", function() { - controller.doEmailConfirmed({email: "testuser@testuser.com"}); - }); - } - }); + asyncTest("doGenerateAssertion - start the generate_assertion service", function() { + testActionStartsModule('doGenerateAssertion', { email: TEST_EMAIL }, "generate_assertion"); }); + }()); diff --git a/resources/static/test/cases/controllers/check_registration.js b/resources/static/test/cases/controllers/check_registration.js index 8619cfa0a11c0b392b7cf7529ffdd3d12ac5aea3..03a01bf0a23fed05da0e3de64fab2eb43843e9eb 100644 --- a/resources/static/test/cases/controllers/check_registration.js +++ b/resources/static/test/cases/controllers/check_registration.js @@ -48,10 +48,18 @@ controller.startCheck(); } - asyncTest("user validation with mustAuth result", function() { + asyncTest("user validation with mustAuth result - callback with email, type and known set to true", function() { xhr.useResult("mustAuth"); - - testVerifiedUserEvent("authenticate", "User Must Auth"); + createController("waitForUserValidation"); + register("authenticate", function(msg, info) { + // we want the email, type and known all sent back to the caller so that + // this information does not need to be queried again. + equal(info.email, "registered@testuser.com", "correct email"); + ok(info.type, "type sent with info"); + ok(info.known, "email is known"); + start(); + }); + controller.startCheck(); }); asyncTest("user validation with pending->complete result ~3 seconds", function() { diff --git a/resources/static/test/cases/controllers/email_chosen.js b/resources/static/test/cases/controllers/generate_assertion.js similarity index 95% rename from resources/static/test/cases/controllers/email_chosen.js rename to resources/static/test/cases/controllers/generate_assertion.js index 83398112bfb1ef125dde8d22854494eb6fa3d4b4..436cffe82c87df0ec7a1c26ef8de935ddfdb5d62 100644 --- a/resources/static/test/cases/controllers/email_chosen.js +++ b/resources/static/test/cases/controllers/generate_assertion.js @@ -17,7 +17,7 @@ config = config || {}; config.ready = complete; - controller = BrowserID.Modules.EmailChosen.create(); + controller = BrowserID.Modules.GenerateAssertion.create(); controller.start(config); } diff --git a/resources/static/test/cases/resources/state.js b/resources/static/test/cases/resources/state.js index 1c32995418eb58c6e2c785a5530d5a8bd791f23b..83e6c7cf6164e71f77b0befcd479e5a3754928ef 100644 --- a/resources/static/test/cases/resources/state.js +++ b/resources/static/test/cases/resources/state.js @@ -93,10 +93,21 @@ equal(actions.info.doConfirmUser.required, true, "doConfirmUser called with required flag"); }); - test("user_confirmed - call doEmailConfirmed", function() { - mediator.publish("user_confirmed"); + test("user_confirmed - redirect to email_chosen", function() { + mediator.subscribe("email_chosen", function(msg, info) { + equal(info.email, TEST_EMAIL, "correct email passed"); + start(); + }); - ok(actions.called.doEmailConfirmed, "user was confirmed"); + // simulate the flow of a user being staged through to confirmation. Since + // we are not actually doing the middle bits and saving off a cert for the + // email address, we get an invalid email exception thrown. + mediator.publish("user_staged", { email: TEST_EMAIL }); + try { + mediator.publish("user_confirmed"); + } catch(e) { + equal(e.toString(), "invalid email", "expected failure"); + } }); test("email_staged - call doConfirmEmail", function() { @@ -112,10 +123,13 @@ equal(actions.info.doConfirmEmail.required, true, "doConfirmEmail called with required flag"); }); - test("primary_user with already provisioned primary user - call doEmailChosen", function() { + asyncTest("primary_user with already provisioned primary user - redirect to primary_user_ready", function() { storage.addEmail(TEST_EMAIL, { type: "primary", cert: "cert" }); + mediator.subscribe("primary_user_ready", function(msg, info) { + equal(info.email, TEST_EMAIL, "primary_user_ready triggered with correct email"); + start(); + }); mediator.publish("primary_user", { email: TEST_EMAIL }); - ok(actions.called.doEmailChosen, "doEmailChosen called"); }); test("primary_user with unprovisioned primary user - call doProvisionPrimaryUser", function() { @@ -198,21 +212,37 @@ equal(actions.info.doForgotPassword.requiredEmail, true, "correct requiredEmail passed"); }); - test("reset_password - call doResetPassword", function() { - // XXX how is this different from forgot_password? + test("reset_password to user_confirmed - call doResetPassword then doEmailConfirmed", function() { + // reset_password indicates the user has verified that they want to reset + // their password. mediator.publish("reset_password", { email: TEST_EMAIL }); equal(actions.info.doResetPassword.email, TEST_EMAIL, "reset password with the correct email"); + + // At this point the user should be displayed the "go confirm your address" + // screen. + + // user_confirmed means the user has confirmed their email and the dialog + // has received the "complete" message from /wsapi/user_creation_status. + try { + mediator.publish("user_confirmed"); + } catch(e) { + // Exception is expected because as part of the user confirmation + // process, before user_confirmed is called, email addresses are synced. + // Addresses are not synced in this test. + equal(e.toString(), "invalid email", "expected failure"); + } }); + test("cancel reset_password flow - go two steps back", function() { // we want to skip the "verify" screen of reset password and instead go two // screens back. Do do this, we are simulating the steps necessary to get // to the reset_password flow. mediator.publish("authenticate"); mediator.publish("forgot_password", undefined, { email: TEST_EMAIL }); - mediator.publish("reset_password"); + mediator.publish("reset_password", { email: TEST_EMAIL }); actions.info.doAuthenticate = {}; mediator.publish("cancel_state"); equal(actions.info.doAuthenticate.email, TEST_EMAIL, "authenticate called with the correct email"); @@ -228,7 +258,20 @@ }); }); - asyncTest("assertion_generated with assertion, need to ask user whether it's their computer - redirect to is_this_your_computer", function() { + test("assertion_generated with assertion - doAssertionGenerated called", function() { + setContextInfo("password"); + storage.addEmail(TEST_EMAIL, {}); + mediator.publish("assertion_generated", { + assertion: "assertion" + }); + + equal(actions.info.doAssertionGenerated.assertion, "assertion", + "doAssertionGenerated called with assertion"); + }); + + + + asyncTest("email_valid_and_ready, need to ask user whether it's their computer - redirect to is_this_your_computer", function() { setContextInfo("password"); storage.usersComputer.forceAsk(network.userid()); mediator.subscribe("is_this_your_computer", function() { @@ -236,31 +279,37 @@ start(); }); - mediator.publish("assertion_generated", { + mediator.publish("email_valid_and_ready", { assertion: "assertion" }); }); - test("assertion_generated with assertion, do not ask user whether it's their computer - doAssertionGenerated called", function() { + test("email_valid_and_ready, do not need to ask user whether it's their computer - redirect to email_ready", function() { setContextInfo("password"); // First, set up the context info for the email. storage.addEmail(TEST_EMAIL, {}); - mediator.publish("email_chosen", { email: TEST_EMAIL }); - mediator.publish("assertion_generated", { - assertion: "assertion" + mediator.subscribe("email_ready", function() { + ok(true, "redirect to email_ready"); + start(); }); - - equal(actions.info.doAssertionGenerated.assertion, "assertion", - "doAssertionGenerated called with assertion"); - equal(actions.info.doAssertionGenerated.email, TEST_EMAIL, - "doAssertionGenerated called with email"); + mediator.publish("email_valid_and_ready", { email: TEST_EMAIL }); }); test("email_confirmed", function() { - mediator.publish("email_confirmed"); - - ok(actions.called.doEmailConfirmed, "user has confirmed the email"); + mediator.subscribe("email_chosen", function(msg, info) { + equal(info.email, TEST_EMAIL, "correct email passed"); + start(); + }); + mediator.publish("email_staged", { email: TEST_EMAIL }); + // simulate the flow of a user being staged through to confirmation. Since + // we are not actually doing the middle bits and saving off a cert for the + // email address, we get an invalid email exception thrown. + try { + mediator.publish("email_confirmed"); + } catch(e) { + equal(e.toString(), "invalid email", "expected failure"); + } }); test("cancel_state goes back to previous state if available", function() { @@ -347,17 +396,17 @@ }); }); - asyncTest("email_chosen with secondary email, user authenticated to secondary - call doEmailChosen", function() { - var email = TEST_EMAIL; - storage.addEmail(email, { type: "secondary" }); + asyncTest("email_chosen with secondary email, user authenticated to secondary - redirect to email_valid_and_ready", function() { + storage.addEmail(TEST_EMAIL, { type: "secondary" }); xhr.setContextInfo("auth_level", "password"); + mediator.subscribe("email_valid_and_ready", function(msg, info) { + equal(info.email, TEST_EMAIL, "correctly redirected to email_valid_and_ready with correct email"); + start(); + }); + mediator.publish("email_chosen", { - email: email, - complete: function() { - equal(actions.called.doEmailChosen, true, "doEmailChosen called"); - start(); - } + email: TEST_EMAIL }); }); diff --git a/resources/static/test/cases/shared/network.js b/resources/static/test/cases/shared/network.js index d2fc7508315a4b6e96d6aa39b4c9783566ebd813..7d50cfaff4865df05562ee6e20236ec4079b8cc4 100644 --- a/resources/static/test/cases/shared/network.js +++ b/resources/static/test/cases/shared/network.js @@ -212,21 +212,51 @@ failureCheck(network.createUser, "validuser", "origin"); }); - asyncTest("checkUserRegistration with pending email", function() { + asyncTest("checkUserRegistration returns pending - pending status, user is not logged in", function() { transport.useResult("pending"); - network.checkUserRegistration("registered@testuser.com", function(status) { - equal(status, "pending"); - start(); + // To properly check the user registration status, we first have to + // simulate the first checkAuth or else network has no context from which + // to work. + network.checkAuth(function(auth_status) { + equal(!!auth_status, false, "user not yet authenticated"); + network.checkUserRegistration("registered@testuser.com", function(status) { + equal(status, "pending"); + network.checkAuth(function(auth_status) { + equal(!!auth_status, false, "user not yet authenticated"); + start(); + }, testHelpers.unexpectedFailure); + }, testHelpers.unexpectedFailure); + }, testHelpers.unexpectedFailure); + }); + + asyncTest("checkUserRegistration returns mustAuth - mustAuth status, user is not logged in", function() { + transport.useResult("mustAuth"); + + network.checkAuth(function(auth_status) { + equal(!!auth_status, false, "user not yet authenticated"); + network.checkUserRegistration("registered@testuser.com", function(status) { + equal(status, "mustAuth"); + network.checkAuth(function(auth_status) { + equal(!!auth_status, false, "user not yet authenticated"); + start(); + }, testHelpers.unexpectedFailure); + }, testHelpers.unexpectedFailure); }, testHelpers.unexpectedFailure); }); - asyncTest("checkUserRegistration with complete email", function() { + asyncTest("checkUserRegistration returns complete - complete status, user is logged in", function() { transport.useResult("complete"); - network.checkUserRegistration("registered@testuser.com", function(status) { - equal(status, "complete"); - start(); + network.checkAuth(function(auth_status) { + equal(!!auth_status, false, "user not yet authenticated"); + network.checkUserRegistration("registered@testuser.com", function(status) { + equal(status, "complete"); + network.checkAuth(function(auth_status) { + equal(auth_status, "password", "user authenticated after checkUserRegistration returns complete"); + start(); + }, testHelpers.unexpectedFailure); + }, testHelpers.unexpectedFailure); }, testHelpers.unexpectedFailure); }); diff --git a/resources/static/test/mocks/xhr.js b/resources/static/test/mocks/xhr.js index 14c08098bdb87b1653e03db4ce9e85cc62811596..17aa6c9851933562e057afbd92c26cb68fa1d4af 100644 --- a/resources/static/test/mocks/xhr.js +++ b/resources/static/test/mocks/xhr.js @@ -110,6 +110,7 @@ BrowserID.Mocks.xhr = (function() { "get /wsapi/address_info?email=unregistered%40testuser.com primary": { type: "primary", auth: "https://auth_url", prov: "https://prov_url" }, "get /wsapi/address_info?email=testuser%40testuser.com unknown_secondary": { type: "secondary", known: false }, "get /wsapi/address_info?email=testuser%40testuser.com known_secondary": { type: "secondary", known: true }, + "get /wsapi/address_info?email=registered%40testuser.com mustAuth": { type: "secondary", known: true }, "get /wsapi/address_info?email=testuser%40testuser.com primary": { type: "primary", auth: "https://auth_url", prov: "https://prov_url" }, "get /wsapi/address_info?email=testuser%40testuser.com ajaxError": undefined, "post /wsapi/add_email_with_assertion invalid": { success: false }, diff --git a/resources/views/test.ejs b/resources/views/test.ejs index dc0bbea64df28b06d94f65f432e8bd552e9c395b..a1fb2cd58698ae93864018bbfaeee0935584d76c 100644 --- a/resources/views/test.ejs +++ b/resources/views/test.ejs @@ -122,7 +122,7 @@ <script src="/dialog/controllers/forgot_password.js"></script> <script src="/dialog/controllers/required_email.js"></script> <script src="/dialog/controllers/verify_primary_user.js"></script> - <script src="/dialog/controllers/email_chosen.js"></script> + <script src="/dialog/controllers/generate_assertion.js"></script> <script src="/dialog/controllers/provision_primary_user.js"></script> <script src="/dialog/controllers/primary_user_provisioned.js"></script> <script src="/dialog/controllers/is_this_your_computer.js"></script> @@ -181,7 +181,7 @@ <script src="cases/controllers/forgot_password.js"></script> <script src="cases/controllers/required_email.js"></script> <script src="cases/controllers/verify_primary_user.js"></script> - <script src="cases/controllers/email_chosen.js"></script> + <script src="cases/controllers/generate_assertion.js"></script> <script src="cases/controllers/provision_primary_user.js"></script> <script src="cases/controllers/primary_user_provisioned.js"></script> <script src="cases/controllers/is_this_your_computer.js"></script> diff --git a/scripts/browserid.spec b/scripts/browserid.spec index 1e657bbbdc2c46358c2d12bee05773efcc44520d..7a9f52d4595f848ab5d5713b02352b5d3ae849dd 100644 --- a/scripts/browserid.spec +++ b/scripts/browserid.spec @@ -1,7 +1,7 @@ %define _rootdir /opt/browserid Name: browserid-server -Version: 0.2012.04.25 +Version: 0.2012.05.09 Release: 1%{?dist}_%{svnrev} Summary: BrowserID server Packager: Pete Fritchman <petef@mozilla.com> diff --git a/tests/primary-then-secondary-test.js b/tests/primary-then-secondary-test.js index bfb72c4b064f3bfd937f25348442d3d07fb11abf..11104396191720812da44e195cb6b2020aea0751 100755 --- a/tests/primary-then-secondary-test.js +++ b/tests/primary-then-secondary-test.js @@ -141,6 +141,24 @@ suite.addBatch({ }); +// after a small delay, we can authenticate with our password +suite.addBatch({ + "after a small delay": { + topic: function() { setTimeout(this.callback, 1500); }, + "authenticating with our newly set password" : { + topic: wsapi.post('/wsapi/authenticate_user', { + email: TEST_EMAIL, + pass: TEST_PASS, + ephemeral: false + }), + "works": function(err, r) { + assert.strictEqual(r.code, 200); + } + } + } +}); + + // adding a second secondary will not let us set the password suite.addBatch({ "add a new email address to our account": { @@ -159,7 +177,7 @@ suite.addBatch({ this._token = t; assert.strictEqual(typeof t, 'string'); }, - "and to complete": { + "and to complete": { topic: function(t) { wsapi.get('/wsapi/email_for_token', { token: t