diff --git a/example/rp/index.html b/example/rp/index.html index 50f5808a8da6400b14728ef5d541909b156e29a3..bef670ebd017a23aec6d333a0f25362f3a7f8b33 100644 --- a/example/rp/index.html +++ b/example/rp/index.html @@ -193,7 +193,7 @@ $(document).ready(function() { }); }); - $(".specify button.logout").click(navigator.id.logout); + $(".specify button.logout").click(function() { navigator.id.logout() }); $(".session button.update_session").click(function() { storage.loggedInUser = $.trim($('#loggedInUser').val()); diff --git a/resources/static/common/js/network.js b/resources/static/common/js/network.js index 190b3396df94d5d4be4e61abeff2789576075b88..ce04a07a463b422d96ca3d46051ab15307af24b7 100644 --- a/resources/static/common/js/network.js +++ b/resources/static/common/js/network.js @@ -101,15 +101,8 @@ BrowserID.Network = (function() { } function handleAddressVerifyCheckResponse(onComplete, status, textStatus, jqXHR) { - if (status.status === 'complete') { - // 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'; - - if (status.userid) setUserID(status.userid); - } + if (status.status === 'complete' && status.userid) + setUserID(status.userid); complete(onComplete, status.status); } diff --git a/resources/static/common/js/user.js b/resources/static/common/js/user.js index babc592290d3ce01c4e62e3c36aad20757d842eb..c5433499a1e4f4a4ae765dc7d0a8ddb6f7fbfaeb 100644 --- a/resources/static/common/js/user.js +++ b/resources/static/common/js/user.js @@ -17,7 +17,9 @@ BrowserID.User = (function() { addressCache = {}, primaryAuthCache = {}, complete = bid.Helpers.complete, - registrationComplete = false; + registrationComplete = false, + POLL_DURATION = 3000, + pollDuration = POLL_DURATION; function prepareDeps() { if (!jwcrypto) { @@ -114,6 +116,12 @@ BrowserID.User = (function() { complete(onComplete, status); } + function markAddressVerified(email) { + var idInfo = storage.getEmail(email) || {}; + idInfo.verified = true; + storage.addSecondaryEmail(email, idInfo); + } + function completeAddressVerification(completeFunc, token, password, onComplete, onFailure) { User.tokenInfo(token, function(info) { var invalidInfo = { valid: false }; @@ -123,17 +131,10 @@ BrowserID.User = (function() { if (valid) { result = _.extend({ valid: valid }, info); - var email = info.email, - idInfo = storage.getEmail(email); - // Now that the address is verified, its verified bit has to be // updated as well or else the user will be forced to verify the // address again. - if (idInfo) { - idInfo.verified = true; - storage.addEmail(email, idInfo); - } - + markAddressVerified(info.email); storage.setReturnTo(""); } @@ -146,7 +147,6 @@ BrowserID.User = (function() { } - function addressVerificationPoll(checkFunc, email, onSuccess, onFailure) { function poll() { checkFunc(email, function(status) { @@ -162,6 +162,11 @@ BrowserID.User = (function() { // data. storage.setReturnTo(""); + // Now that the address is verified, its verified bit has to be + // updated as well or else the user will be forced to verify the + // address again. + markAddressVerified(email); + // To avoid too many address_info requests, returns from each // address_info request are cached. If the user is doing // a addressVerificationPoll, it means the user was registering the address @@ -179,12 +184,22 @@ BrowserID.User = (function() { // they just completed a registration. registrationComplete = true; - if (onSuccess) { - onSuccess(status); + if (status === "complete") { + // If the response is complete but the user is not authenticated + // to the password level, the user *must* authenticate or else + // they will see an error when they try to certify a cert. Users + // who have entered their password in this dialog session will be + // automatically authenticated in modules/check_registration.js, + // all others will have to enter their password. See issue #2088. + network.checkAuth(function(authLevel) { + if (authLevel !== "password") status = "mustAuth"; + complete(onSuccess, status); + }, onFailure); } + else complete(onSuccess, status); } else if (status === 'pending') { - pollTimeout = setTimeout(poll, 3000); + pollTimeout = setTimeout(poll, pollDuration); } else if (onFailure) { onFailure(status); @@ -274,12 +289,18 @@ BrowserID.User = (function() { provisioning = config.provisioning; } + // BEGIN TESTING API + if (config.pollDuration) { + pollDuration = config.pollDuration; + } + // END TESTING API }, reset: function() { provisioning = BrowserID.Provisioning; User.resetCaches(); registrationComplete = false; + pollDuration = POLL_DURATION; }, resetCaches: function() { diff --git a/resources/static/dialog/js/misc/state.js b/resources/static/dialog/js/misc/state.js index b9f1f7db346016bf81d7b1a735d7f2348112adab..2f87712883cee7008236b049cbb3987a091a2c22 100644 --- a/resources/static/dialog/js/misc/state.js +++ b/resources/static/dialog/js/misc/state.js @@ -49,6 +49,10 @@ BrowserID.State = (function() { // screen. var actionInfo = { email: info.email, + // password is used to authenticate the user if the verification poll + // wsapi comes back with "mustAuth" or the user is currently + // authenticated to the "assertion" level. See issue #2088 + password: self.stagedPassword, siteName: self.siteName }; @@ -56,6 +60,25 @@ BrowserID.State = (function() { startAction(actionName, actionInfo); } + function handleEmailConfirmed(msg, info) { + self.email = self.stagedEmail; + + if (info.mustAuth) { + // If the mustAuth flag comes in, the user has to authenticate. + // This is not a cancelable authentication. mustAuth is set + // after a user verifies an address but is not authenticated + // to the password level. + redirectToState("authenticate_specified_email", { + email: self.stagedEmail, + mustAuth: info.mustAuth, + cancelable: !info.mustAuth + }); + } + else { + redirectToState("email_chosen", { email: self.stagedEmail }); + } + } + handleState("start", function(msg, info) { self.hostname = info.hostname; @@ -120,6 +143,22 @@ BrowserID.State = (function() { startAction("doAuthenticate", info); }); + handleState("authenticate_specified_email", function(msg, info) { + // user must authenticate with their password, kick them over to + // the required email screen to enter the password. + startAction("doAuthenticateWithRequiredEmail", { + email: info.email, + secondary_auth: true, + cancelable: ("cancelable" in info) ? info.cancelable : true, + // This is a user is already authenticated to the assertion + // level who has chosen a secondary email address from the + // pick_email screen. They would have been shown the + // siteTOSPP there. + siteTOSPP: false + }); + complete(info.complete); + }); + handleState("new_user", function(msg, info) { self.newUserEmail = info.email; @@ -156,6 +195,11 @@ BrowserID.State = (function() { */ info = _.extend({ email: self.newUserEmail || self.addEmailEmail || self.resetPasswordEmail }, info); + // stagedPassword is used to authenticate a user if the verification poll + // comes back with "mustAuth" or the user is not currently authenticated + // to the "password" level. See issue #2088 + self.stagedPassword = info.password; + if(self.newUserEmail) { self.newUserEmail = null; startAction(false, "doStageUser", info); @@ -172,15 +216,9 @@ BrowserID.State = (function() { handleState("user_staged", handleEmailStaged.curry("doConfirmUser")); - handleState("user_confirmed", function() { - self.email = self.stagedEmail; - redirectToState("email_chosen", { email: self.stagedEmail} ); - }); + handleState("user_confirmed", handleEmailConfirmed); - handleState("staged_address_confirmed", function() { - self.email = self.stagedEmail; - redirectToState("email_chosen", { email: self.stagedEmail} ); - }); + handleState("staged_address_confirmed", handleEmailConfirmed); handleState("primary_user", function(msg, info) { addPrimaryUser = !!info.add; @@ -320,21 +358,12 @@ BrowserID.State = (function() { if (authentication === "assertion") { // user must authenticate with their password, kick them over to // the required email screen to enter the password. - startAction("doAuthenticateWithRequiredEmail", { - email: email, - secondary_auth: true, - - // This is a user is already authenticated to the assertion - // level who has chosen a secondary email address from the - // pick_email screen. They would have been shown the - // siteTOSPP there. - siteTOSPP: false - }); + redirectToState("authenticate_specified_email", info); } else { redirectToState("email_valid_and_ready", info); + oncomplete(); } - oncomplete(); }, oncomplete); } }); @@ -472,9 +501,7 @@ BrowserID.State = (function() { handleState("email_staged", handleEmailStaged.curry("doConfirmEmail")); - handleState("email_confirmed", function() { - redirectToState("email_chosen", { email: self.stagedEmail } ); - }); + handleState("email_confirmed", handleEmailConfirmed); handleState("cancel_state", function(msg, info) { cancelState(info); diff --git a/resources/static/dialog/js/modules/check_registration.js b/resources/static/dialog/js/modules/check_registration.js index 29094a1dcfd4bc7749ed50db4616f3d4baaa46c3..f782037da457ad4b4a40527bf98d68c71c40dd04 100644 --- a/resources/static/dialog/js/modules/check_registration.js +++ b/resources/static/dialog/js/modules/check_registration.js @@ -41,7 +41,7 @@ BrowserID.Modules.CheckRegistration = (function() { if (status === "complete") { // TODO - move the syncEmails somewhere else, perhaps into user.js user.syncEmails(function() { - self.close(self.verificationMessage); + self.close(self.verificationMessage, { mustAuth: false }); oncomplete && oncomplete(); }); } @@ -49,22 +49,22 @@ BrowserID.Modules.CheckRegistration = (function() { // if we have a password (because it was just chosen in dialog), // then we can authenticate the user and proceed if (self.password) { + // XXX Move all of this authentication stuff into user.js. This + // high level shouldn't have to worry about this stuff. user.authenticate(self.email, self.password, function (authenticated) { if (authenticated) { user.syncEmails(function() { - self.close(self.verificationMessage); + self.close(self.verificationMessage, { mustAuth: false }); oncomplete && oncomplete(); }); } else { - user.addressInfo(self.email, function(info) { - self.close("authenticate", info); - }); + // unable to log the user in, make them authenticate manually. + self.close(self.verificationMessage, { mustAuth: true }); } }); } else { - user.addressInfo(self.email, function(info) { - self.close("authenticate", info); - }); + // no password to log the user in, make them authenticate manually. + self.close(self.verificationMessage, { mustAuth: true }); } oncomplete && oncomplete(); diff --git a/resources/static/dialog/js/modules/required_email.js b/resources/static/dialog/js/modules/required_email.js index 09d184e5d4d47baccc32527dd25063a1aeefde5e..e950984c3514bb3fc2a5f9c1ef11f97ca1d4c7e6 100644 --- a/resources/static/dialog/js/modules/required_email.js +++ b/resources/static/dialog/js/modules/required_email.js @@ -124,7 +124,8 @@ BrowserID.Modules.RequiredEmail = (function() { showTemplate({ signin: true, password: auth_level !== "password", - secondary_auth: secondaryAuth + secondary_auth: secondaryAuth, + cancelable: options.cancelable }); ready(); } @@ -219,7 +220,8 @@ BrowserID.Modules.RequiredEmail = (function() { password: false, secondary_auth: false, primary: false, - personaTOSPP: false + personaTOSPP: false, + cancelable: true }, templateData); self.renderDialog("required_email", templateData); diff --git a/resources/static/dialog/views/required_email.ejs b/resources/static/dialog/views/required_email.ejs index 0bb323a27b3e7478b621349e3b0375a6e69226ee..500327fbfee2c46573f4bf473542d07848b2d8b5 100644 --- a/resources/static/dialog/views/required_email.ejs +++ b/resources/static/dialog/views/required_email.ejs @@ -57,7 +57,7 @@ <button id="verify_address" tabindex="3"><%= gettext("verify email") %></button> <% } %> - <% if (secondary_auth) { %> + <% if (cancelable && secondary_auth) { %> <a href="#" id="cancel" class="action" tabindex="4"><%= gettext("cancel") %></a> <% } %> </p> diff --git a/resources/static/test/cases/common/js/network.js b/resources/static/test/cases/common/js/network.js index 7000cf007a253c8277d90095f023326bef838fa4..fc153cacbf2c62429521c6f4c23e8fb99762756a 100644 --- a/resources/static/test/cases/common/js/network.js +++ b/resources/static/test/cases/common/js/network.js @@ -55,10 +55,7 @@ transport.useResult("complete"); network[funcName]("registered@testuser.com", function(status) { equal(status, "complete"); - network.checkAuth(function(auth_level) { - equal(auth_level, "password", "user can only be authenticated to password level after verification is complete"); - start(); - }); + start(); }, testHelpers.unexpectedFailure); }); } @@ -257,53 +254,11 @@ failureCheck(network.createUser, "validuser", "password", "origin"); }); - asyncTest("checkUserRegistration returns pending - pending status, user is not logged in", function() { - transport.useResult("pending"); - - // 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 pending - pending status, user is not logged in", testVerificationPending.curry("checkUserRegistration")); - asyncTest("checkUserRegistration returns mustAuth - mustAuth status, user is not logged in", function() { - transport.useResult("mustAuth"); + asyncTest("checkUserRegistration returns mustAuth - mustAuth status, user is not logged in", testVerificationMustAuth.curry("checkUserRegistration")); - 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 returns complete - complete status, user is logged in", function() { - transport.useResult("complete"); - - 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); - }); + asyncTest("checkUserRegistration returns complete - complete status, user is logged in", testVerificationComplete.curry("checkUserRegistration")); asyncTest("checkUserRegistration with XHR failure", function() { failureCheck(network.checkUserRegistration, "registered@testuser.com"); diff --git a/resources/static/test/cases/common/js/user.js b/resources/static/test/cases/common/js/user.js index f39ed418c20b87b8fd73f881e1f965cfcfb44aeb..46f7a2376315a1cc18af304fd5f0d8b429cd48b0 100644 --- a/resources/static/test/cases/common/js/user.js +++ b/resources/static/test/cases/common/js/user.js @@ -320,13 +320,16 @@ ); }); - asyncTest("waitForUserValidation with `complete` response", function() { + asyncTest("waitForUserValidation with complete from backend, user not authed - `mustAuth` response", function() { storage.setReturnTo(testOrigin); + xhr.setContextInfo("auth_level", false); xhr.useResult("complete"); lib.waitForUserValidation("registered@testuser.com", function(status) { - equal(status, "complete", "complete response expected"); + equal(status, "mustAuth", "mustAuth response expected"); + + testHelpers.testEmailMarkedVerified("registered@testuser.com"); ok(!storage.getReturnTo(), "staged on behalf of is cleared when validation completes"); start(); @@ -341,6 +344,8 @@ lib.waitForUserValidation("registered@testuser.com", function(status) { equal(status, "mustAuth", "mustAuth response expected"); + testHelpers.testEmailMarkedVerified("registered@testuser.com"); + ok(!storage.getReturnTo(), "staged on behalf of is cleared when validation completes"); start(); }, testHelpers.unexpectedXHRFailure); @@ -844,12 +849,28 @@ }); - asyncTest("waitForEmailValidation `complete` response", function() { + asyncTest("waitForEmailValidation with `complete` backend response, user authenticated to assertion level - expect 'mustAuth'", function() { + storage.setReturnTo(testOrigin); + xhr.setContextInfo("auth_level", "assertion"); + + xhr.useResult("complete"); + lib.waitForEmailValidation("registered@testuser.com", function(status) { + ok(!storage.getReturnTo(), "staged on behalf of is cleared when validation completes"); + testHelpers.testEmailMarkedVerified("registered@testuser.com"); + equal(status, "mustAuth", "mustAuth response expected"); + start(); + }, testHelpers.unexpectedXHRFailure); + }); + + + asyncTest("waitForEmailValidation with `complete` backend response, user authenticated to password level - expect 'complete'", function() { storage.setReturnTo(testOrigin); + xhr.setContextInfo("auth_level", "password"); xhr.useResult("complete"); lib.waitForEmailValidation("registered@testuser.com", function(status) { ok(!storage.getReturnTo(), "staged on behalf of is cleared when validation completes"); + testHelpers.testEmailMarkedVerified("registered@testuser.com"); equal(status, "complete", "complete response expected"); start(); }, testHelpers.unexpectedXHRFailure); @@ -861,6 +882,7 @@ lib.waitForEmailValidation("registered@testuser.com", function(status) { ok(!storage.getReturnTo(), "staged on behalf of is cleared when validation completes"); + testHelpers.testEmailMarkedVerified("registered@testuser.com"); equal(status, "mustAuth", "mustAuth response expected"); start(); }, testHelpers.unexpectedXHRFailure); diff --git a/resources/static/test/cases/dialog/js/misc/state.js b/resources/static/test/cases/dialog/js/misc/state.js index eaec7deb5cc6a0dd5583b7448d34a5969fe930f8..8fda485c5018cb0fd0febbd4133a6ffb4f118d91 100644 --- a/resources/static/test/cases/dialog/js/misc/state.js +++ b/resources/static/test/cases/dialog/js/misc/state.js @@ -568,6 +568,33 @@ mediator.publish("window_unload"); }); + function testAuthenticateSpecifiedEmail(specified, expected) { + var options = { + email: TEST_EMAIL, + complete: function() { + testActionStarted("doAuthenticateWithRequiredEmail", { + cancelable: expected + }); + start(); + } + }; + + if (typeof specified !== "undefined") options.cancelable = specified; + + mediator.publish("authenticate_specified_email", options); + } + + asyncTest("authenticate_specified_email with false specified - call doAuthenticateWithRequiredEmail using specified cancelable", function() { + testAuthenticateSpecifiedEmail(false, false); + }); + + asyncTest("authenticate_specified_email with true specified - call doAuthenticateWithRequiredEmail using specified cancelable", function() { + testAuthenticateSpecifiedEmail(true, true); + }); + + asyncTest("authenticate_specified_email without cancelable - call doAuthenticateWithRequiredEmail, cancelable defaults to true", function() { + testAuthenticateSpecifiedEmail(undefined, true); + }); }()); diff --git a/resources/static/test/cases/dialog/js/modules/check_registration.js b/resources/static/test/cases/dialog/js/modules/check_registration.js index 29c84236e7e2f0c9cf52801cfd678c04882145dc..e7499d31eb145d214e40743adbb3858c9d820a2b 100644 --- a/resources/static/test/cases/dialog/js/modules/check_registration.js +++ b/resources/static/test/cases/dialog/js/modules/check_registration.js @@ -8,15 +8,17 @@ var controller, bid = BrowserID, + user = bid.User, xhr = bid.Mocks.xhr, network = bid.Network, testHelpers = bid.TestHelpers, register = testHelpers.register; - function createController(verifier, message, required) { + function createController(verifier, message, required, password) { controller = bid.Modules.CheckRegistration.create(); controller.start({ email: "registered@testuser.com", + password: password, verifier: verifier, verificationMessage: message, required: required, @@ -40,37 +42,52 @@ } }); - function testVerifiedUserEvent(event_name, message) { - createController("waitForUserValidation", event_name); - register(event_name, function() { - ok(true, message); + function testVerifiedUserEvent(event_name, message, password) { + createController("waitForUserValidation", event_name, false, password); + register(event_name, function(msg, info) { + equal(info.mustAuth, false, "user does not need to verify"); start(); }); controller.startCheck(); } - asyncTest("user validation with mustAuth result - callback with email, type and known set to true", function() { - xhr.useResult("mustAuth"); - 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"); + function testMustAuthUserEvent(event_name, message) { + createController("waitForUserValidation", event_name); + register(event_name, function(msg, info) { + equal(info.mustAuth, true, "user needs to verify"); start(); }); controller.startCheck(); + } + + asyncTest("user validation with mustAuth result - userVerified with mustAuth: true", function() { + xhr.useResult("mustAuth"); + testMustAuthUserEvent("user_verified"); + }); + + asyncTest("user validation with pending->complete with auth_level = assertion, no authentication info given - user_verified with mustAuth triggered", function() { + user.init({ pollDuration: 100 }); + xhr.useResult("pending"); + xhr.setContextInfo("auth_level", "assertion"); + testMustAuthUserEvent("user_verified"); + + // use setTimeout to simulate a delay in the user opening the email. + setTimeout(function() { + xhr.useResult("complete"); + }, 50); }); - asyncTest("user validation with pending->complete result ~3 seconds", function() { + asyncTest("user validation with pending->complete with auth_level = password - user_verified triggered", function() { + user.init({ pollDuration: 100 }); xhr.useResult("pending"); + xhr.setContextInfo("auth_level", "password"); + + testVerifiedUserEvent("user_verified"); - testVerifiedUserEvent("user_verified", "User verified"); // use setTimeout to simulate a delay in the user opening the email. setTimeout(function() { xhr.useResult("complete"); - }, 500); + }, 50); }); asyncTest("user validation with XHR error - show error message", function() { diff --git a/resources/static/test/testHelpers/helpers.js b/resources/static/test/testHelpers/helpers.js index 55035dbfb4511d44ed59a3f7ea6689261827a031..3f2acbbab02a6d914c0daff71fb5bb1604f15fd9 100644 --- a/resources/static/test/testHelpers/helpers.js +++ b/resources/static/test/testHelpers/helpers.js @@ -1,5 +1,5 @@ -/*jshint browsers: true laxbreak: true, expr: true */ -/*global BrowserID: true, ok: true, equal: true, start: true */ +/*jshint browser: true laxbreak: true, expr: true */ +/*global BrowserID: true, ok: true, equal: true, start: true, deepEqual: true, notEqual: true */ /* 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 @@ -291,6 +291,12 @@ BrowserID.TestHelpers = (function() { testRPTosPPNotShown: function(msg) { TestHelpers.testNotHasClass("body", "rptospp", msg || "RP TOS/PP not shown"); + }, + + testEmailMarkedVerified: function(email, msg) { + var emailInfo = storage.getEmail(email); + equal(emailInfo && emailInfo.verified, true, + "verified bit set for " + email); } };