From 8d8f3dd45ece4134f3f1f4612048622911400e62 Mon Sep 17 00:00:00 2001 From: Shane Tomlinson <stomlinson@mozilla.com> Date: Tue, 10 Jan 2012 12:35:46 +0000 Subject: [PATCH] Implementing two level auth in the dialog. * If the user is not authenticated to "password" authentication when they select a secondary email, make them sign in. * Updaate required_email to do its own check for the user's authentication status. * Update the unit tests to make sure we are all kosher. --- .../dialog/controllers/required_email.js | 174 ++++++++++-------- .../static/dialog/resources/state_machine.js | 6 +- .../static/dialog/views/required_email.ejs | 4 +- .../controllers/required_email_unit_test.js | 130 ++++++------- .../static/test/qunit/testHelpers/helpers.js | 2 +- 5 files changed, 160 insertions(+), 156 deletions(-) diff --git a/resources/static/dialog/controllers/required_email.js b/resources/static/dialog/controllers/required_email.js index de8d29496..6dd6bbe1a 100644 --- a/resources/static/dialog/controllers/required_email.js +++ b/resources/static/dialog/controllers/required_email.js @@ -47,14 +47,15 @@ BrowserID.Modules.RequiredEmail = (function() { assertion, cancelEvent = dialogHelpers.cancelEvent, email, - authenticated, - primaryInfo; + auth_level, + primaryInfo, + secondaryAuth; function closePrimaryUser(callback) { this.close("primary_user", _.extend(primaryInfo, { email: email, requiredEmail: true, - add: authenticated + add: !!auth_level })); callback && callback(); @@ -71,15 +72,15 @@ BrowserID.Modules.RequiredEmail = (function() { // With a primary, just go to the primary flow, it'll be taken care of. closePrimaryUser.call(self, callback); } - else if (authenticated) { - // If the user is already authenticated and they own this address, sign - // them in. + else if (auth_level === "password") { + // this is a secondary address and the user is authenticated with their + // password, sign them in. getAssertion(); } else { - // If the user is not already authenticated, but they potentially own - // this address, try and sign them in and generate an assertion if they - // get the password right. + // this is a secondary address, but the user is either not authenticated + // or they are only authenticated at the assertion level. If their + // password is correct, sign them in and get an assertion. var password = helpers.getAndValidatePassword("#password"); if (password) { dialogHelpers.authenticateUser.call(self, email, password, function(authenticated) { @@ -96,13 +97,13 @@ BrowserID.Modules.RequiredEmail = (function() { } function verifyAddress() { - // By being in the verifyAddress, we know that the current user has not + // By being in the verifyAddress, we know that the current user has not // been shown the password box and we have to do a verification of some // sort. This will be either an add email to the current account or a new // registration. var self=this; - if (authenticated) { + if (auth_level) { // If we are veryifying an address and the user is authenticated, it // means that the current user does not have control of the address. // If the address is registered, it means another account has control of @@ -122,7 +123,7 @@ BrowserID.Modules.RequiredEmail = (function() { function cancel() { - this.close("cancel"); + this.close(secondaryAuth ? "cancel_state" : "cancel"); } var RequiredEmail = bid.Modules.PageModule.extend({ @@ -130,102 +131,117 @@ BrowserID.Modules.RequiredEmail = (function() { var self=this; email = options.email || "", - authenticated = options.authenticated || false; - primaryInfo = null; + secondaryAuth = options.secondary_auth; function ready() { options.ready && options.ready(); } - // NOTE: When the app first starts and the user's authentication is - // checked, all email addresses for authenticated users are synced. We - // can be assured by this point that our addresses are up to date. - if (authenticated) { - // if the current user owns the required email, sign in with it - // (without a password). Otherwise, make the user verify the address - // (which shows no password). - var emailInfo = user.getStoredEmailKeypair(email); - if (emailInfo) { - if(emailInfo.type === "secondary" || emailInfo.cert) { - // secondary user or cert is valid, user can sign in normally. - showTemplate({ signin: true }); - ready(); + user.checkAuthentication(function(checked_auth_level) { + auth_level = checked_auth_level; + primaryInfo = null; + + // NOTE: When the app first starts and the user's authentication is + // checked, all email addresses for authenticated users are synced. We + // can be assured by this point that our addresses are up to date. + if (auth_level) { + // if the current user owns the required email, sign in with it + // (without a password). Otherwise, make the user verify the address + // (which shows no password). + var emailInfo = user.getStoredEmailKeypair(email); + if (emailInfo) { + if(emailInfo.type === "secondary") { + // secondary user, show the password field if they are not + // authenticated to the "password" level. + showTemplate({ + signin: true, + password: auth_level !== "password", + secondary_auth: secondaryAuth + }); + ready(); + } + else if(emailInfo.cert) { + // primary user with valid cert, user can sign in normally. + showTemplate({ signin: true }); + ready(); + } + else { + // Uh oh, this is a primary user whose certificate is expired, + // take care of that. + user.addressInfo(email, function(info) { + primaryInfo = info; + if (info.authed) { + // If the user is authed with the IdP, give them the + // opportunity to press "signin" before passing them off + // to the primary user flow + showTemplate({ signin: true }); + } + else { + // User is not authed with IdP, start the verification flow, + // add address to current user. + closePrimaryUser.call(self); + } + ready(); + }, self.getErrorDialog(errors.addressInfo, ready)); + } } else { - // Uh oh, this is a primary user whose certificate is expired, - // take care of that. + // User does not control address, time to verify. user.addressInfo(email, function(info) { - primaryInfo = info; - if (info.authed) { - // If the user is authed with the IdP, give them the - // opportunity to press "signin" before passing them off - // to the primary user flow - showTemplate({ signin: true }); + // authenticated user who does not own primary address, make them + // verify it. + if(info.type === "primary") { + primaryInfo = info; + if (info.authed) { + // If the user is authed with the IdP, give them the + // opportunity to press "signin" before passing them off to + // the primary user flow + showTemplate({ signin: true }); + } + else { + // If user is not authed with IdP, kick them through the + // primary_user flow to get them verified. + closePrimaryUser.call(self); + } } else { - // User is not authed with IdP, start the verification flow, - // add address to current user. - closePrimaryUser.call(self); + showTemplate({ verify: true }); } ready(); }, self.getErrorDialog(errors.addressInfo, ready)); } } else { - // User does not control address, time to verify. user.addressInfo(email, function(info) { - // authenticated user who does not own primary address, make them - // verify it. - if(info.type === "primary") { + if (info.type === "primary") { primaryInfo = info; if (info.authed) { - // If the user is authed with the IdP, give them the - // opportunity to press "signin" before passing them off to - // the primary user flow - showTemplate({ signin: true }); + // If the user is authenticated with their IdP, show the + // sign in button to give the user the chance to abort. + showTemplate({ signin: true, primary: true }); } else { - // If user is not authed with IdP, kick them through the - // primary_user flow to get them verified. + // If the user is not authenticated with their IdP, pass them + // off to the primary user flow. closePrimaryUser.call(self); } } else { - showTemplate({ verify: true }); + // If the current email address is registered but the user is not + // authenticated, make them sign in with it. Otherwise, make them + // verify ownership of the address. + if (info.known) { + showTemplate({ signin: true, password: true }); + } + else { + showTemplate({ verify: true }); + } } ready(); }, self.getErrorDialog(errors.addressInfo, ready)); } - } - else { - user.addressInfo(email, function(info) { - if (info.type === "primary") { - primaryInfo = info; - if (info.authed) { - // If the user is authenticated with their IdP, show the - // sign in button to give the user the chance to abort. - showTemplate({ signin: true, primary: true }); - } - else { - // If the user is not authenticated with their IdP, pass them - // off to the primary user flow. - closePrimaryUser.call(self); - } - } - else { - // If the current email address is registered but the user is not - // authenticated, make them sign in with it. Otherwise, make them - // verify ownership of the address. - if (info.known) { - showTemplate({ signin: true, password: true }); - } - else { - showTemplate({ verify: true }); - } - } - ready(); - }, self.getErrorDialog(errors.addressInfo, ready)); - } + + }, self.getErrorDialog(errors.checkAuthentication, ready)); function showTemplate(options) { options = _.extend({ diff --git a/resources/static/dialog/resources/state_machine.js b/resources/static/dialog/resources/state_machine.js index 142261ce3..18cf9710f 100644 --- a/resources/static/dialog/resources/state_machine.js +++ b/resources/static/dialog/resources/state_machine.js @@ -142,8 +142,7 @@ if (requiredEmail) { startState("doAuthenticateWithRequiredEmail", { - email: requiredEmail, - authenticated: authenticated + email: requiredEmail }); } else if (authenticated) { @@ -235,9 +234,10 @@ else { user.checkAuthentication(function(authentication) { if(authentication === "assertion") { + // user not authenticated, kick them over to the required email + // screen. startState("doAuthenticateWithRequiredEmail", { email: email, - authenticated: false, secondary_auth: true }); } diff --git a/resources/static/dialog/views/required_email.ejs b/resources/static/dialog/views/required_email.ejs index 1f3a188eb..a8a631131 100644 --- a/resources/static/dialog/views/required_email.ejs +++ b/resources/static/dialog/views/required_email.ejs @@ -56,6 +56,8 @@ <button id="verify_address" tabindex="3">verify email</button> <% } %> - <!--button id="cancel" tabindex="4">cancel</button--> + <% if (secondary_auth) { %> + <button id="cancel" tabindex="4">cancel</button> + <% } %> </div> </div> diff --git a/resources/static/test/qunit/controllers/required_email_unit_test.js b/resources/static/test/qunit/controllers/required_email_unit_test.js index ead470e41..c4019fb06 100644 --- a/resources/static/test/qunit/controllers/required_email_unit_test.js +++ b/resources/static/test/qunit/controllers/required_email_unit_test.js @@ -1,5 +1,5 @@ /*jshint browsers:true, forin: true, laxbreak: true */ -/*global test: true, start: true, stop: true, module: true, ok: true, equal: true, BrowserID:true */ +/*global asyncTest: true, test: true, start: true, stop: true, module: true, ok: true, equal: true, BrowserID:true */ /* ***** BEGIN LICENSE BLOCK ***** * Version: MPL 1.1/GPL 2.0/LGPL 2.1 * @@ -109,13 +109,29 @@ start(); } + function testMessageReceived(email, message) { + xhr.setContextInfo("auth_level", "assertion"); + + createController({ + email: email, + ready: function() { + register(message, function(item, info) { + equal(info.email, email, message + " received with correct email"); + start(); + }); + + controller.verifyAddress(); + } + }); + } + + asyncTest("known_secondary: user who is not authenticated - show password form", function() { var email = "registered@testuser.com"; xhr.useResult("known_secondary"); createController({ email: email, - authenticated: false, ready: function() { testSignIn(email, testPasswordSection); } @@ -128,7 +144,6 @@ createController({ email: email, - authenticated: false, ready: function() { testVerify(email); } @@ -138,12 +153,12 @@ asyncTest("primary: user who is authenticated, owns address, cert valid - sees signin screen", function() { var email = "testuser@testuser.com"; + xhr.setContextInfo("auth_level", "assertion"); storage.addEmail(email, { type: "primary", cert: "cert" }); xhr.useResult("primary"); createController({ email: email, - authenticated: true, ready: function() { testSignIn(email); } @@ -160,7 +175,6 @@ createController({ email: email, - authenticated: true, ready: function() { testSignIn(email); } @@ -181,7 +195,6 @@ createController({ email: email, - authenticated: true, ready: function() { equal(msgInfo.email, email, "correct email passed"); start(); @@ -198,7 +211,6 @@ createController({ email: email, - authenticated: true, ready: function() { testSignIn(email); } @@ -218,7 +230,6 @@ createController({ email: email, - authenticated: true, ready: function() { equal(msgInfo.email, email, "correct email passed"); start(); @@ -233,7 +244,6 @@ createController({ email: email, - authenticated: false, ready: function() { testSignIn(email, testNoPasswordSection); } @@ -253,7 +263,6 @@ createController({ email: email, - authenticated: false, ready: function() { equal(msgInfo && msgInfo.email, "unregistered@testuser.com", "correct email address"); start(); @@ -266,7 +275,6 @@ var email = "registered@testuser.com"; createController({ email: email, - authenticated: false, ready: function() { ok(testHelpers.errorVisible(), "Error message is visible"); start(); @@ -274,16 +282,27 @@ }); }); - asyncTest("known_secondary: user who is authenticated, email belongs to user - user sees sign in screen.", function() { - xhr.setContextInfo({ - authenticated: true + asyncTest("known_secondary: assertion authenticated, email belongs to user - user sees sign in screen with password field.", function() { + xhr.setContextInfo("auth_level", "assertion"); + + var email = "registered@testuser.com"; + user.syncEmailKeypair(email, function() { + createController({ + email: email, + ready: function() { + testSignIn(email, testPasswordSection); + } + }); }); + }); + + asyncTest("known_secondary: password authenticated, email belongs to user - user sees sign in screen, no password.", function() { + xhr.setContextInfo("auth_level", "password"); var email = "registered@testuser.com"; user.syncEmailKeypair(email, function() { createController({ email: email, - authenticated: true, ready: function() { testSignIn(email, testNoPasswordSection); } @@ -292,16 +311,13 @@ }); asyncTest("known_secondary: user who is authenticated, email belongs to another user - user sees verify screen", function() { - xhr.setContextInfo({ - authenticated: true - }); + xhr.setContextInfo("auth_level", "password"); var email = "registered@testuser.com"; xhr.useResult("known_secondary"); createController({ email: email, - authenticated: true, ready: function() { // This means the current user is going to take the address from the other // account. @@ -311,16 +327,14 @@ }); asyncTest("unknown_secondary: user who is authenticated, but email unknown - user sees verify screen", function() { - xhr.setContextInfo({ - authenticated: true - }); + xhr.setContextInfo("auth_level", "password"); xhr.useResult("unknown_secondary"); var email = "unregistered@testuser.com"; createController({ email: email, - authenticated: true, + auth_level: "password", ready: function() { testVerify(email); } @@ -329,15 +343,12 @@ asyncTest("secondary: signIn of an authenticated user - generates an assertion, redirects to assertion_generated", function() { - xhr.setContextInfo({ - authenticated: true - }); + xhr.setContextInfo("auth_level", "password"); var email = "registered@testuser.com"; user.syncEmailKeypair(email, function() { createController({ - email: email, - authenticated: true + email: email }); var assertion; @@ -353,16 +364,11 @@ }); asyncTest("secondary: signIn of a non-authenticated user with a good password - generates an assertion, redirects to assertion_generated", function() { - xhr.setContextInfo({ - authenticated: false - }); - var email = "testuser@testuser.com"; xhr.useResult("known_secondary"); createController({ email: email, - authenticated: false, ready: function() { var assertion; register("assertion_generated", function(item, info) { @@ -383,16 +389,11 @@ asyncTest("secondary: signIn of a non-authenticated user with a bad password does not generate an assertion", function() { - xhr.setContextInfo({ - authenticated: false - }); - var email = "registered@testuser.com"; xhr.useResult("known_secondary"); createController({ email: email, - authenticated: false, ready: function() { var assertion; @@ -421,7 +422,6 @@ createController({ email: email, - authenticated: false, ready: function() { var primaryEmail; @@ -438,27 +438,6 @@ }); - function testMessageReceived(email, message) { - var authenticated = true; - - xhr.setContextInfo({ - authenticated: authenticated - }); - - createController({ - email: email, - authenticated: authenticated, - ready: function() { - register(message, function(item, info) { - equal(info.email, email, message + " received with correct email"); - start(); - }); - - controller.verifyAddress(); - } - }); - } - asyncTest("verifyAddress of authenticated user, address belongs to another user - redirects to 'email_staged'", function() { var email = "registered@testuser.com"; xhr.useResult("known_secondary"); @@ -475,16 +454,10 @@ asyncTest("verifyAddress of un-authenticated user, forgot password - redirect to 'forgot_password'", function() { var email = "registered@testuser.com", - authenticated = false, message = "forgot_password"; - xhr.setContextInfo({ - authenticated: authenticated - }); - createController({ email: email, - authenticated: authenticated, ready: function() { register(message, function(item, info) { equal(info.email, email, message + " received with correct email"); @@ -496,18 +469,30 @@ }); }); - asyncTest("cancel raises the cancel message", function() { + asyncTest("cancel normally raises the 'cancel' message", function() { var email = "registered@testuser.com", - message = "cancel", - authenticated = false; + message = "cancel"; + + createController({ + email: email, + ready: function() { + register(message, function(item, info) { + ok(true, message + " received"); + start(); + }); - xhr.setContextInfo({ - authenticated: authenticated + controller.cancel(); + } }); + }); + + asyncTest("cancel with 'secondary_auth' raises the 'cancel_state' message", function() { + var email = "registered@testuser.com", + message = "cancel_state"; createController({ email: email, - authenticated: authenticated, + secondary_auth: true, ready: function() { register(message, function(item, info) { ok(true, message + " received"); @@ -519,5 +504,6 @@ }); }); + }()); diff --git a/resources/static/test/qunit/testHelpers/helpers.js b/resources/static/test/qunit/testHelpers/helpers.js index 9e19c32b9..b309eea01 100644 --- a/resources/static/test/qunit/testHelpers/helpers.js +++ b/resources/static/test/qunit/testHelpers/helpers.js @@ -41,7 +41,7 @@ BrowserID.TestHelpers = { setup: function() { network.setXHR(xhr); - //xhr.setContextInfo("authenticated", false); + xhr.setContextInfo("auth_level", undefined); xhr.useResult("valid"); storage.clear(); -- GitLab