From 43e16138580779ef8e64dac24c5a9199b3cd6794 Mon Sep 17 00:00:00 2001 From: Shane Tomlinson <stomlinson@mozilla.com> Date: Thu, 22 Dec 2011 12:59:25 +0000 Subject: [PATCH] wooha. With some caveats, the sign up flow where authentication with the primary is working. * Use WinChan to open a new window to the primary. * Add the idp_auth_complete page for the IdP to redirect back to. * When the window closes, re-try to authenticate the user with the primary. * Add a helper to show an error message. * Add a WinChan mock. * Update unit tests to handle the "need to authenticate with IdP" scenario. * Update compression scripts for WinChan to be included on the main site. --- lib/browserid/views.js | 7 ++ resources/static/pages/page_helpers.js | 25 +++- resources/static/pages/signup.js | 117 +++++++++++------- resources/static/shared/error-messages.js | 7 +- resources/static/shared/helpers.js | 28 ++++- resources/static/test/index.html | 1 + resources/static/test/qunit/mocks/winchan.js | 51 ++++++++ .../qunit/pages/page_helpers_unit_test.js | 10 +- .../test/qunit/pages/signup_unit_test.js | 39 ++++-- .../test/qunit/shared/helpers_unit_test.js | 15 +++ resources/views/idp_auth_complete.ejs | 9 ++ resources/views/layout.ejs | 1 + resources/views/signup.ejs | 2 +- scripts/compress.sh | 2 +- 14 files changed, 252 insertions(+), 62 deletions(-) create mode 100644 resources/static/test/qunit/mocks/winchan.js create mode 100644 resources/views/idp_auth_complete.ejs diff --git a/lib/browserid/views.js b/lib/browserid/views.js index 412609c48..f1d582c46 100644 --- a/lib/browserid/views.js +++ b/lib/browserid/views.js @@ -55,6 +55,13 @@ exports.setup = function(app) { res.render('signup.ejs', {title: 'Sign Up', fullpage: false}); }); + app.get("/idp_auth_complete", function(req, res) { + res.render('idp_auth_complete.ejs', { + title: 'Sign In Complete', + fullpage: false + }); + }); + app.get("/forgot", function(req, res) { res.render('forgot.ejs', {title: 'Forgot Password', fullpage: false, email: req.query.email}); }); diff --git a/resources/static/pages/page_helpers.js b/resources/static/pages/page_helpers.js index 0ea6e2cf1..7f016b399 100644 --- a/resources/static/pages/page_helpers.js +++ b/resources/static/pages/page_helpers.js @@ -87,14 +87,18 @@ BrowserID.PageHelpers = (function() { return decodeURIComponent(results[1].replace(/\+/g, " ")); } + function showFailure(error, info, callback) { + info = $.extend(info || {}, { action: error, dialog: false }); + bid.Screens.error.show("error", info); + $("#errorBackground").stop().fadeIn(); + $("#error").stop().fadeIn(); + + callback && callback(false); + } + function getFailure(error, callback) { return function onFailure(info) { - info = $.extend(info, { action: error, dialog: false }); - bid.Screens.error.show("error", info); - $("#errorBackground").stop().fadeIn(); - $("#error").stop().fadeIn(); - - callback && callback(false); + showFailure(error, info, callback); } } @@ -143,6 +147,15 @@ BrowserID.PageHelpers = (function() { clearStoredEmail: clearStoredEmail, getStoredEmail: getStoredEmail, getParameterByName: getParameterByName, + /** + * shows a failure screen immediately + * @method showFailure + */ + showFailure: showFailure, + /** + * get a function to show an error screen when function is called. + * @method getFailure + */ getFailure: getFailure, replaceInputsWithNotice: replaceInputsWithNotice, replaceFormWithNotice: replaceFormWithNotice, diff --git a/resources/static/pages/signup.js b/resources/static/pages/signup.js index 5f31d21e1..1a1853470 100644 --- a/resources/static/pages/signup.js +++ b/resources/static/pages/signup.js @@ -47,7 +47,7 @@ BrowserID.signUp = (function() { tooltip = BrowserID.Tooltip, ANIMATION_SPEED = 250, storedEmail = pageHelpers, - win = window, + winchan = window.WinChan, verifyEmail, verifyURL; @@ -55,13 +55,16 @@ BrowserID.signUp = (function() { $(selector).fadeIn(ANIMATION_SPEED); } - function verifyWithPrimary(oncomplete) { + function authWithPrimary(oncomplete) { if(!(verifyEmail && verifyURL)) { throw "cannot verify with primary without an email address and URL" } - var url = verifyURL + "?email=" + encodeURIComponent(verifyEmail) + - "&return_to=https://browserid.org/sign_in_complete"; + var url = helpers.toURL(verifyURL, { + email: verifyEmail, + return_to: "https://browserid.org/idp_auth_complete" + }); + // XXX: we should use winchan (and send user to a page that redirects to primary)! // we should: // 1. build a page that we host and we open with winchan @@ -74,10 +77,67 @@ BrowserID.signUp = (function() { // 8. we get notification that the interaction is complete in main page and try to // silently provision again! success means the users is signed up, failure // means there was an auth problem. they can try again? - win.open(url, "_moz_primary_verification", "width=700,height=375"); + var win = winchan.open({ + url: url, + // This is the relay that will be used when the IdP redirects to sign_in_complete + relay_url: "https://browserid.org/relay", + window_features: "width=700,height=375" + }, primaryAuthComplete); oncomplete && oncomplete(); } + function primaryAuthComplete(error, result, oncomplete) { + // XXX once we get our winchan shit figured out, remove the false && + // below. + if(false && error) { + pageHelpers.showFailure(errors.primaryAuthentication, error, oncomplete); + } + else { + // hey ho, the user is authenticated, re-try the submit. + createUser(verifyEmail, oncomplete); + } + } + + function createUser(email, oncomplete) { + function complete(status) { + oncomplete && oncomplete(status); + } + + user.createUser(email, function onComplete(status, info) { + switch(status) { + case "secondary.already_added": + $('#registeredEmail').html(email); + showNotice(".alreadyRegistered"); + complete(false); + break; + case "secondary.verify": + pageHelpers.showEmailSent(complete); + break; + case "secondary.could_not_add": + tooltip.showTooltip("#could_not_add"); + complete(false); + break; + case "primary.already_added": + // XXX Is this status possible? + break; + case "primary.verified": + pageHelpers.replaceFormWithNotice("#congrats", complete.bind(null, true)); + break; + case "primary.verify": + verifyEmail = email; + verifyURL = info.auth; + dom.setInner("#primary_email", email); + pageHelpers.replaceInputsWithNotice("#primary_verify", complete.bind(null, false)); + break; + case "primary.could_not_add": + // XXX Can this happen? + break; + default: + break; + } + }, pageHelpers.getFailure(errors.createUser, complete)); + } + function submit(oncomplete) { var email = helpers.getAndValidateEmail("#email"); @@ -86,39 +146,7 @@ BrowserID.signUp = (function() { } if (email) { - user.createUser(email, function onComplete(status, info) { - switch(status) { - case "secondary.already_added": - $('#registeredEmail').html(email); - showNotice(".alreadyRegistered"); - complete(false); - break; - case "secondary.verify": - pageHelpers.showEmailSent(complete); - break; - case "secondary.could_not_add": - tooltip.showTooltip("#could_not_add"); - complete(false); - break; - case "primary.already_added": - // XXX Is this status possible? - break; - case "primary.verified": - pageHelpers.replaceFormWithNotice("#congrats", complete.bind(null, true)); - break; - case "primary.verify": - verifyEmail = email; - verifyURL = info.auth; - dom.setInner("#primary_email", email); - pageHelpers.replaceInputsWithNotice("#primary_verify", complete.bind(null, false)); - break; - case "primary.could_not_add": - // XXX Can this happen? - break; - default: - break; - } - }, pageHelpers.getFailure(errors.createUser, oncomplete)); + createUser(email, complete); } else { complete(false); @@ -136,8 +164,8 @@ BrowserID.signUp = (function() { function init(config) { config = config || {}; - if(config.window) { - win = config.window; + if(config.winchan) { + winchan = config.winchan; } $("form input[autofocus]").focus(); @@ -147,7 +175,7 @@ BrowserID.signUp = (function() { dom.bindEvent("#email", "keyup", onEmailKeyUp); dom.bindEvent("form", "submit", cancelEvent(submit)); dom.bindEvent("#back", "click", cancelEvent(back)); - dom.bindEvent("#verifyWithPrimary", "click", cancelEvent(verifyWithPrimary)); + dom.bindEvent("#authWithPrimary", "click", cancelEvent(authWithPrimary)); } // BEGIN TESTING API @@ -155,15 +183,16 @@ BrowserID.signUp = (function() { dom.unbindEvent("#email", "keyup"); dom.unbindEvent("form", "submit"); dom.unbindEvent("#back", "click"); - dom.unbindEvent("#verifyWithPrimary", "click"); - win = window; + dom.unbindEvent("#authWithPrimary", "click"); + winchan = window.WinChan; verifyEmail = verifyURL = null; } init.submit = submit; init.reset = reset; init.back = back; - init.verifyWithPrimary = verifyWithPrimary; + init.authWithPrimary = authWithPrimary; + init.primaryAuthComplete = primaryAuthComplete; // END TESTING API return init; diff --git a/resources/static/shared/error-messages.js b/resources/static/shared/error-messages.js index dc444e193..13f8b3078 100644 --- a/resources/static/shared/error-messages.js +++ b/resources/static/shared/error-messages.js @@ -82,8 +82,13 @@ BrowserID.Errors = (function(){ message: "Unfortunately, BrowserID cannot communicate while offline!" }, + primaryAuthentication: { + title: "Authenticating with Identity Provider", + message: "We had trouble communicating with your email provider, please try again!" + }, + provisioningPrimary: { - title: "Provisioning Primary", + title: "Provisioning with Identity Provider", message: "We had trouble communicating with your email provider, please try again!" }, diff --git a/resources/static/shared/helpers.js b/resources/static/shared/helpers.js index 45060aaed..2105a8d3d 100644 --- a/resources/static/shared/helpers.js +++ b/resources/static/shared/helpers.js @@ -66,6 +66,22 @@ return password; } + function toURL(base, params) { + var url = base, + getParams = []; + + for(var key in params) { + getParams.push(key + "=" + encodeURIComponent(params[key])); + } + + if(getParams.length) { + url += "?" + getParams.join("&"); + } + + return url; + } + + extend(helpers, { /** * Extend an object with the properties of another object. Overwrites @@ -90,7 +106,17 @@ * @param {string} target - target containing the password * @return {string} password if password is valid, null otw. */ - getAndValidatePassword: getAndValidatePassword + getAndValidatePassword: getAndValidatePassword, + + /** + * Convert a base URL and an object to a URL with GET parameters. All + * keys/values are converted as <key>=encodeURIComponent(<value>) + * method @toURL + * @param {string} base_url - base url + * @param {object} [params] - object to convert to GET parameters. + * @returns {string} + */ + toURL: toURL }); diff --git a/resources/static/test/index.html b/resources/static/test/index.html index feb6f0daf..b725c20fe 100644 --- a/resources/static/test/index.html +++ b/resources/static/test/index.html @@ -96,6 +96,7 @@ <script type="text/javascript" src="qunit/mocks/templates.js"></script> <script type="text/javascript" src="qunit/mocks/provisioning.js"></script> <script type="text/javascript" src="qunit/mocks/window.js"></script> + <script type="text/javascript" src="qunit/mocks/winchan.js"></script> <script type="text/javascript" src="/shared/javascript-extensions.js"></script> <script type="text/javascript" src="/shared/renderer.js"></script> <script type="text/javascript" src="/shared/class.js"></script> diff --git a/resources/static/test/qunit/mocks/winchan.js b/resources/static/test/qunit/mocks/winchan.js new file mode 100644 index 000000000..2bb3e1491 --- /dev/null +++ b/resources/static/test/qunit/mocks/winchan.js @@ -0,0 +1,51 @@ +/*jshint browsers:true, forin: true, laxbreak: true */ +/*global BrowserID: true */ +/* ***** BEGIN LICENSE BLOCK ***** + * Version: MPL 1.1/GPL 2.0/LGPL 2.1 + * + * The contents of this file are subject to the Mozilla Public License Version + * 1.1 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * http://www.mozilla.org/MPL/ + * + * Software distributed under the License is distributed on an "AS IS" basis, + * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License + * for the specific language governing rights and limitations under the + * License. + * + * The Original Code is Mozilla BrowserID. + * + * The Initial Developer of the Original Code is Mozilla. + * Portions created by the Initial Developer are Copyright (C) 2011 + * the Initial Developer. All Rights Reserved. + * + * Contributor(s): + * + * Alternatively, the contents of this file may be used under the terms of + * either the GNU General Public License Version 2 or later (the "GPL"), or + * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), + * in which case the provisions of the GPL or the LGPL are applicable instead + * of those above. If you wish to allow use of your version of this file only + * under the terms of either the GPL or the LGPL, and not to allow others to + * use your version of this file under the terms of the MPL, indicate your + * decision by deleting the provisions above and replace them with the notice + * and other provisions required by the GPL or the LGPL. If you do not delete + * the provisions above, a recipient may use your version of this file under + * the terms of any one of the MPL, the GPL or the LGPL. + * + * ***** END LICENSE BLOCK ***** */ +BrowserID.Mocks.WinChan = (function() { + "use strict"; + + function WinChan() { }; + + WinChan.prototype = { + open: function(params, callback) { + this.params = params; + this.oncomplete = callback; + } + }; + + return WinChan; + +}()); diff --git a/resources/static/test/qunit/pages/page_helpers_unit_test.js b/resources/static/test/qunit/pages/page_helpers_unit_test.js index 958949b3a..67e8ffaa7 100644 --- a/resources/static/test/qunit/pages/page_helpers_unit_test.js +++ b/resources/static/test/qunit/pages/page_helpers_unit_test.js @@ -39,7 +39,8 @@ var bid = BrowserID, pageHelpers = bid.PageHelpers, - testHelpers = bid.TestHelpers; + testHelpers = bid.TestHelpers, + errors = bid.Errors; module("pages/page_helpers", { setup: function() { @@ -137,6 +138,13 @@ }); }); + asyncTest("showFailure shows a failure screen", function() { + pageHelpers.showFailure({}, errors.offline, function() { + testHelpers.testErrorVisible(); + start(); + }); + }); + }()); diff --git a/resources/static/test/qunit/pages/signup_unit_test.js b/resources/static/test/qunit/pages/signup_unit_test.js index 12c2f17cd..b6fec1620 100644 --- a/resources/static/test/qunit/pages/signup_unit_test.js +++ b/resources/static/test/qunit/pages/signup_unit_test.js @@ -40,18 +40,18 @@ var bid = BrowserID, network = bid.Network, xhr = bid.Mocks.xhr, - WindowMock = bid.Mocks.WindowMock, + WinChanMock = bid.Mocks.WinChan, testHelpers = bid.TestHelpers, provisioning = bid.Mocks.Provisioning, - win; + winchan; module("pages/signup", { setup: function() { testHelpers.setup(); - win = new WindowMock(); + winchan = new WinChanMock(); bid.signUp({ - window: win + winchan: winchan }); }, teardown: function() { @@ -175,16 +175,41 @@ }); }); - asyncTest("verifyWithPrimary opens new tab", function() { + asyncTest("authWithPrimary opens new tab", function() { xhr.useResult("primary"); $("#email").val("unregistered@testuser.com"); bid.signUp.submit(function(status) { - bid.signUp.verifyWithPrimary(function() { - equal(win.open_url, "https://auth_url?email=unregistered%40testuser.com", "user directed to authentication URL"); + bid.signUp.authWithPrimary(function() { + ok(winchan.oncomplete, "winchan set up"); start(); }); }); }); + asyncTest("primaryAuthComplete with error, expect incorrect status", function() { + bid.signUp.primaryAuthComplete("error", "", function(status) { + equal(status, false, "correct status for could not complete"); + testHelpers.testErrorVisible(); + start(); + }); + }); + + asyncTest("primaryAuthComplete with successful authentication, expect correct status and congrats message", function() { + xhr.useResult("primary"); + $("#email").val("unregistered@testuser.com"); + + bid.signUp.submit(function(status) { + bid.signUp.authWithPrimary(function() { + // In real life the user would now be authenticated. + provisioning.setStatus(provisioning.AUTHENTICATED); + bid.signUp.primaryAuthComplete(null, "success", function(status) { + equal(status, true, "correct status"); + equal($("#congrats:visible").length, 1, "success notification is visible"); + start(); + }); + }); + }); + }); + }()); diff --git a/resources/static/test/qunit/shared/helpers_unit_test.js b/resources/static/test/qunit/shared/helpers_unit_test.js index cd570787a..8d00c4683 100644 --- a/resources/static/test/qunit/shared/helpers_unit_test.js +++ b/resources/static/test/qunit/shared/helpers_unit_test.js @@ -106,4 +106,19 @@ strictEqual(password, null, "invalid target returns null"); }); + + test("toURL with no GET parameters", function() { + var url = helpers.toURL("https://browserid.org"); + + equal(url, "https://browserid.org", "correct URL without GET parameters"); + }); + + test("toURL with GET parameters", function() { + var url = helpers.toURL("https://browserid.org", { + email: "testuser@testuser.com", + status: "complete" + }); + + equal(url, "https://browserid.org?email=testuser%40testuser.com&status=complete", "correct URL with GET parameters"); + }); }()); diff --git a/resources/views/idp_auth_complete.ejs b/resources/views/idp_auth_complete.ejs new file mode 100644 index 000000000..55a0d3d8a --- /dev/null +++ b/resources/views/idp_auth_complete.ejs @@ -0,0 +1,9 @@ + <div id="vAlign" class="disply_always"> + This window will now close and account creation will continue. + </div> + + <script type="text/javascript"> + WinChan.onOpen(function(origin, params, complete) { + complete("success"); + }); + </script> diff --git a/resources/views/layout.ejs b/resources/views/layout.ejs index fbfa792ae..72a2c3fc9 100644 --- a/resources/views/layout.ejs +++ b/resources/views/layout.ejs @@ -24,6 +24,7 @@ <script src="/shared/browserid.js" type="text/javascript"></script> <script src="/lib/dom-jquery.js" type="text/javascript"></script> <script src="/lib/jschannel.js" type="text/javascript"></script> + <script src="/lib/winchan.js" type="text/javascript"></script> <script src="/shared/templates.js" type="text/javascript"></script> <script src="/shared/renderer.js" type="text/javascript"></script> diff --git a/resources/views/signup.ejs b/resources/views/signup.ejs index c5642d932..124cbbd8d 100644 --- a/resources/views/signup.ejs +++ b/resources/views/signup.ejs @@ -34,7 +34,7 @@ </p> <p> - <button id="verifyWithPrimary">Verify</button> + <button id="authWithPrimary">Verify</button> </p> </li> </ul> diff --git a/scripts/compress.sh b/scripts/compress.sh index b4d8e22dd..53f0c063c 100755 --- a/scripts/compress.sh +++ b/scripts/compress.sh @@ -60,7 +60,7 @@ cat lib/jquery-1.6.2.min.js lib/winchan.js lib/underscore-min.js lib/vepbundle.j cat css/common.css dialog/css/popup.css dialog/css/m.css > $BUILD_PATH/dialog.uncompressed.css # produce the non interactive frame js -cat lib/jquery-1.6.2.min.js lib/jschannel.js lib/underscore-min.js lib/vepbundle.js shared/javascript-extensions.js shared/browserid.js shared/storage.js shared/network.js shared/user.js communication_iframe/start.js > $BUILD_PATH/communication_iframe.uncompressed.js +cat lib/jquery-1.6.2.min.js lib/jschannel.js lib/winchan.js lib/underscore-min.js lib/vepbundle.js shared/javascript-extensions.js shared/browserid.js shared/storage.js shared/network.js shared/user.js communication_iframe/start.js > $BUILD_PATH/communication_iframe.uncompressed.js echo '' echo '****Building BrowserID.org HTML, CSS, and JS****' -- GitLab