From 5182c6c534c05a61b36a76b9c1335fe77d246a80 Mon Sep 17 00:00:00 2001
From: Shane Tomlinson <stomlinson@mozilla.com>
Date: Wed, 16 Nov 2011 23:42:35 +0000
Subject: [PATCH] Adding the ability to press "cancel" in the verify email
dialog.
* Cancel causes the user to go back to the previous screen.
* Cancel causes the network polling to stop.
close #587
---
.../checkregistration_controller.js | 45 ++++++---
.../dialog/controllers/dialog_controller.js | 22 ++++-
resources/static/shared/user.js | 26 +++++-
.../checkregistration_controller_unit_test.js | 27 ++++--
.../test/qunit/pages/browserid_unit_test.js | 2 +-
resources/static/test/qunit/qunit.js | 2 +
.../test/qunit/shared/user_unit_test.js | 91 ++++++++++++++-----
7 files changed, 161 insertions(+), 54 deletions(-)
diff --git a/resources/static/dialog/controllers/checkregistration_controller.js b/resources/static/dialog/controllers/checkregistration_controller.js
index f5aa94968..c6269895c 100644
--- a/resources/static/dialog/controllers/checkregistration_controller.js
+++ b/resources/static/dialog/controllers/checkregistration_controller.js
@@ -37,36 +37,51 @@
(function() {
"use strict";
- var user = BrowserID.User;
+ var bid = BrowserID,
+ user = bid.User,
+ dom = bid.DOM,
+ errors = bid.Errors;
PageController.extend("Checkregistration", {}, {
- init: function(el, options) {
- var me=this;
- me._super(el, {
- waitTemplate: "confirmemail",
- waitVars: {
+ start: function(options) {
+ var self=this;
+ self.renderWait("confirmemail", {
email: options.email
- }
});
- me.email = options.email;
- me.verifier = options.verifier;
- me.verificationMessage = options.verificationMessage;
+ self.email = options.email;
+ self.verifier = options.verifier;
+ self.verificationMessage = options.verificationMessage;
+
+ dom.bindEvent("#back", "click", self.cancel.bind(self));
+
+ self._super();
+ },
+
+ stop: function() {
+ dom.unbindEvent("#back", "click");
+
+ this._super();
},
startCheck: function() {
- var me=this;
- user[me.verifier](me.email, function(status) {
+ var self=this;
+ user[self.verifier](self.email, function(status) {
if (status === "complete") {
user.syncEmails(function() {
- me.close(me.verificationMessage);
+ self.close(self.verificationMessage);
});
}
else if (status === "mustAuth") {
- me.close("auth", { email: me.email });
+ self.close("auth", { email: self.email });
}
- }, me.getErrorDialog(BrowserID.Errors.registration));
+ }, self.getErrorDialog(errors.registration));
},
+ cancel: function() {
+ var self=this;
+ self.close("cancel_" + self.verificationMessage);
+ }
+
});
diff --git a/resources/static/dialog/controllers/dialog_controller.js b/resources/static/dialog/controllers/dialog_controller.js
index daf337755..7bffc9bb4 100644
--- a/resources/static/dialog/controllers/dialog_controller.js
+++ b/resources/static/dialog/controllers/dialog_controller.js
@@ -145,6 +145,11 @@
self.doEmailConfirmed();
});
+ subscribe("cancel_user_confirmed", function() {
+ user.cancelUserValidation();
+ self.returnFromStageCancel();
+ });
+
subscribe("authenticated", function(msg, info) {
//self.doEmailSelected(info.email);
// XXX benadida, lloyd - swap these two if you want to experiment with
@@ -173,6 +178,11 @@
self.doEmailConfirmed();
});
+ subscribe("cancel_email_confirmed", function() {
+ user.cancelEmailValidation();
+ self.returnFromStageCancel();
+ });
+
subscribe("notme", function() {
self.doNotMe();
});
@@ -229,6 +239,7 @@
doPickEmail: function() {
var self=this;
+ self.returnFromStageCancel = self.doPickEmail.bind(self);
self.element.pickemail({
// XXX ideal is to get rid of this and have a User function
// that takes care of getting email addresses AND the last used email
@@ -239,11 +250,18 @@
},
doAuthenticate: function(info) {
- this.element.authenticate(info);
+ var self = this;
+
+ // Save this off in case the user forgets their password or goes to add
+ // a new password but has to cancel.
+ self.returnFromStageCancel = self.doAuthenticate.bind(self, info);
+ self.element.authenticate(info);
},
doAuthenticateWithRequiredEmail: function(info) {
- this.element.requiredemail(info);
+ var self=this;
+ self.returnFromStageCancel = self.doAuthenticateWithRequiredEmail.bind(self, info);
+ self.element.requiredemail(info);
},
doForgotPassword: function(email) {
diff --git a/resources/static/shared/user.js b/resources/static/shared/user.js
index 937ab1e68..6c9a19077 100644
--- a/resources/static/shared/user.js
+++ b/resources/static/shared/user.js
@@ -41,7 +41,7 @@ BrowserID.User = (function() {
var jwk, jwt, vep, jwcert, origin,
network = BrowserID.Network,
storage = BrowserID.Storage,
- User;
+ User, pollTimeout;
function prepareDeps() {
if (!jwk) {
@@ -117,7 +117,7 @@ BrowserID.User = (function() {
}
}
else if (status === 'pending') {
- setTimeout(poll, 3000);
+ pollTimeout = setTimeout(poll, 3000);
}
else if (onFailure) {
onFailure(status);
@@ -128,6 +128,12 @@ BrowserID.User = (function() {
poll();
}
+ function cancelRegistrationPoll() {
+ if (pollTimeout) {
+ clearTimeout(pollTimeout);
+ pollTimeout = null;
+ }
+ }
/**
* Certify an identity with the server, persist it to storage if the server
@@ -250,6 +256,14 @@ BrowserID.User = (function() {
registrationPoll(network.checkUserRegistration, email, onSuccess, onFailure);
},
+ /**
+ * Cancel the waitForUserValidation poll
+ * @method cancelUserValidation
+ */
+ cancelUserValidation: function() {
+ cancelRegistrationPoll();
+ },
+
/**
* Verify a user
* @method verifyUser
@@ -505,6 +519,14 @@ BrowserID.User = (function() {
registrationPoll(network.checkEmailRegistration, email, onSuccess, onFailure);
},
+ /**
+ * Cancel the waitForEmailValidation poll
+ * @method cancelEmailValidation
+ */
+ cancelEmailValidation: function() {
+ cancelRegistrationPoll();
+ },
+
/**
* Verify a users email address given by the token
* @method verifyEmail
diff --git a/resources/static/test/qunit/controllers/checkregistration_controller_unit_test.js b/resources/static/test/qunit/controllers/checkregistration_controller_unit_test.js
index b763784e9..ee15db686 100644
--- a/resources/static/test/qunit/controllers/checkregistration_controller_unit_test.js
+++ b/resources/static/test/qunit/controllers/checkregistration_controller_unit_test.js
@@ -56,10 +56,6 @@ steal.plugins("jquery").then("/dialog/controllers/page_controller", "/dialog/con
}
}
- function reset() {
- unsubscribeAll();
- }
-
function initController(verifier, message) {
el = $("body");
controller = el.checkregistration({
@@ -71,8 +67,9 @@ steal.plugins("jquery").then("/dialog/controllers/page_controller", "/dialog/con
module("controllers/checkregistration_controller", {
setup: function() {
+ xhr.useResult("valid");
network.setXHR(xhr);
- reset();
+ $("#error").hide();
},
teardown: function() {
@@ -83,7 +80,8 @@ steal.plugins("jquery").then("/dialog/controllers/page_controller", "/dialog/con
controller.destroy();
} catch(e) {}
}
- reset();
+ unsubscribeAll();
+ $("#error").hide();
}
});
@@ -114,21 +112,30 @@ steal.plugins("jquery").then("/dialog/controllers/page_controller", "/dialog/con
});
test("user validation with XHR error", function() {
- $("#error").hide();
-
xhr.useResult("ajaxError");
initController("waitForUserValidation", "user_verified");
subscribe("user_verified", function() {
ok(false, "on XHR error, should not complete");
- start();
});
controller.startCheck();
setTimeout(function() {
ok($("#error").is(":visible"), "Error message is visible");
start();
- }, 100);
+ }, 1000);
+
+ stop();
+ });
+
+ test("cancel raises cancel_user_verified", function() {
+ initController("waitForUserValidation", "user_verified");
+ subscribe("cancel_user_verified", function() {
+ ok(true, "on cancel, cancel_user_verified is triggered");
+ start();
+ });
+ controller.startCheck();
+ controller.cancel();
stop();
});
diff --git a/resources/static/test/qunit/pages/browserid_unit_test.js b/resources/static/test/qunit/pages/browserid_unit_test.js
index 7a5184845..1ba1e5c1a 100644
--- a/resources/static/test/qunit/pages/browserid_unit_test.js
+++ b/resources/static/test/qunit/pages/browserid_unit_test.js
@@ -34,7 +34,7 @@
* the terms of any one of the MPL, the GPL or the LGPL.
*
* ***** END LICENSE BLOCK ***** */
-steal.then("/pages/page_helpers", "/pages/browserid", function() {
+steal.then(function() {
"use strict";
module("/pages/browserid");
diff --git a/resources/static/test/qunit/qunit.js b/resources/static/test/qunit/qunit.js
index f414ec307..d129414f8 100644
--- a/resources/static/test/qunit/qunit.js
+++ b/resources/static/test/qunit/qunit.js
@@ -25,6 +25,8 @@ steal.plugins(
"/dialog/controllers/page_controller",
"/dialog/controllers/required_email_controller",
+ "/pages/page_helpers",
+
"pages/browserid_unit_test",
"pages/page_helpers_unit_test",
diff --git a/resources/static/test/qunit/shared/user_unit_test.js b/resources/static/test/qunit/shared/user_unit_test.js
index 66d46bd73..8d13450db 100644
--- a/resources/static/test/qunit/shared/user_unit_test.js
+++ b/resources/static/test/qunit/shared/user_unit_test.js
@@ -39,9 +39,11 @@ var jwt = require("./jwt");
var jwcert = require("./jwcert");
steal.then(function() {
- var lib = BrowserID.User,
- storage = BrowserID.Storage,
- xhr = BrowserID.Mocks.xhr,
+ var bid = BrowserID,
+ lib = bid.User,
+ storage = bid.Storage,
+ network = bid.Network,
+ xhr = bid.Mocks.xhr,
testOrigin = "testOrigin";
// I generated these locally, they are used nowhere else.
@@ -51,7 +53,7 @@ steal.then(function() {
var random_cert = "eyJhbGciOiJSUzEyOCJ9.eyJpc3MiOiJpc3N1ZXIuY29tIiwiZXhwIjoxMzE2Njk1MzY3NzA3LCJwdWJsaWMta2V5Ijp7ImFsZ29yaXRobSI6IlJTIiwibiI6IjU2MDYzMDI4MDcwNDMyOTgyMzIyMDg3NDE4MTc2ODc2NzQ4MDcyMDM1NDgyODk4MzM0ODExMzY4NDA4NTI1NTk2MTk4MjUyNTE5MjY3MTA4MTMyNjA0MTk4MDA0NzkyODQ5MDc3ODY4OTUxOTA2MTcwODEyNTQwNzEzOTgyOTU0NjUzODEwNTM5OTQ5Mzg0NzEyNzczMzkwMjAwNzkxOTQ5NTY1OTAzNDM5NTIxNDI0OTA5NTc2ODMyNDE4ODkwODE5MjA0MzU0NzI5MjE3MjA3MzYwMTA1OTA2MDM5MDIzMjk5NTYxMzc0MDk4OTQyNzg5OTk2NzgwMTAyMDczMDcxNzYwODUyODQxMDY4OTg5ODYwNDAzNDMxNzM3NDgwMTgyNzI1ODUzODk5NzMzNzA2MDY5IiwiZSI6IjY1NTM3In0sInByaW5jaXBhbCI6eyJlbWFpbCI6InRlc3R1c2VyQHRlc3R1c2VyLmNvbSJ9fQ.aVIO470S_DkcaddQgFUXciGwq2F_MTdYOJtVnEYShni7I6mqBwK3fkdWShPEgLFWUSlVUtcy61FkDnq2G-6ikSx1fUZY7iBeSCOKYlh6Kj9v43JX-uhctRSB2pI17g09EUtvmb845EHUJuoowdBLmLa4DSTdZE-h4xUQ9MsY7Ik";
- function testAssertion(assertion) {
+ function testAssertion(assertion, cb) {
equal(typeof assertion, "string", "An assertion was correctly generated");
// Decode the assertion to a bundle.
@@ -71,30 +73,32 @@ steal.then(function() {
var expires = tok.expires.getTime();
ok(typeof expires === "number" && !isNaN(expires), "expiration date is valid");
- var nowPlus2Mins = new Date().getTime() + (2 * 60 * 1000);
- // expiration date must be within 5 seconds of 2 minutes from now - see
- // issue 433 (https://github.com/mozilla/browserid/issues/433)
- ok(((nowPlus2Mins - 5000) < expires) && (expires < (nowPlus2Mins + 5000)), "expiration date must be within 5 seconds of 2 minutes from now");
-
- equal(typeof tok.cryptoSegment, "string", "cryptoSegment exists");
- equal(typeof tok.headerSegment, "string", "headerSegment exists");
- equal(typeof tok.payloadSegment, "string", "payloadSegment exists");
- /*
- // What are these supposed to be?
- ok(tok.issuer, "issuer?");
- ok(tok.payload, "payload?");
- */
+ // this should be based on server time, not local time.
+ network.serverTime(function(time) {
+ var nowPlus2Mins = time.getTime() + (2 * 60 * 1000);
+
+ // expiration date must be within 5 seconds of 2 minutes from now - see
+ // issue 433 (https://github.com/mozilla/browserid/issues/433)
+ var diff = Math.abs(expires - nowPlus2Mins);
+ ok(diff < 5000, "expiration date must be within 5 seconds of 2 minutes from now: " + diff);
+
+ equal(typeof tok.cryptoSegment, "string", "cryptoSegment exists");
+ equal(typeof tok.headerSegment, "string", "headerSegment exists");
+ equal(typeof tok.payloadSegment, "string", "payloadSegment exists");
+
+ if(cb) cb();
+ });
}
module("shared/user", {
setup: function() {
- BrowserID.Network.setXHR(xhr);
+ network.setXHR(xhr);
xhr.useResult("valid");
lib.clearStoredEmailKeypairs();
lib.setOrigin(testOrigin);
},
teardown: function() {
- BrowserID.Network.setXHR($);
+ network.setXHR($);
}
});
@@ -226,7 +230,7 @@ steal.then(function() {
start();
}, function(status) {
- ok(storage.getStagedOnBehalfOf(), "staged on behalf of is cleared when validation completes");
+ ok(storage.getStagedOnBehalfOf(), "staged on behalf of is not cleared for noRegistration response");
ok(status, "noRegistration", "noRegistration response causes failure");
start();
});
@@ -234,6 +238,7 @@ steal.then(function() {
stop();
});
+
test("waitForUserValidation with XHR failure", function() {
xhr.useResult("ajaxError");
@@ -250,6 +255,25 @@ steal.then(function() {
stop();
});
+ test("cancelUserValidation: ~1 second", function() {
+ xhr.useResult("pending");
+
+ storage.setStagedOnBehalfOf(testOrigin);
+ lib.waitForUserValidation("registered@testuser.com", function(status) {
+ ok(false, "not expecting success")
+ }, function(status) {
+ ok(false, "not expecting failure");
+ });
+
+ setTimeout(function() {
+ lib.cancelUserValidation();
+ ok(storage.getStagedOnBehalfOf(), "staged on behalf of is not cleared when validation cancelled");
+ start();
+ }, 1000);
+
+ stop();
+ });
+
test("verifyUser with a good token", function() {
storage.setStagedOnBehalfOf(testOrigin);
@@ -625,6 +649,25 @@ steal.then(function() {
});
+ test("cancelEmailValidation: ~1 second", function() {
+ xhr.useResult("pending");
+
+ storage.setStagedOnBehalfOf(testOrigin);
+ lib.waitForEmailValidation("registered@testuser.com", function(status) {
+ ok(false, "not expecting success")
+ }, function(status) {
+ ok(false, "not expecting failure");
+ });
+
+ setTimeout(function() {
+ lib.cancelUserValidation();
+ ok(storage.getStagedOnBehalfOf(), "staged on behalf of is not cleared when validation cancelled");
+ start();
+ }, 1000);
+
+ stop();
+ });
+
test("verifyEmail with a good token", function() {
storage.setStagedOnBehalfOf(testOrigin);
lib.verifyEmail("token", function onSuccess(info) {
@@ -850,10 +893,10 @@ steal.then(function() {
test("getAssertion with known email that has key", function() {
lib.setOrigin(testOrigin);
+ lib.removeEmail("testuser@testuser.com");
lib.syncEmailKeypair("testuser@testuser.com", function() {
lib.getAssertion("testuser@testuser.com", function onSuccess(assertion) {
- testAssertion(assertion);
- start();
+ testAssertion(assertion, start);
}, failure("getAssertion failure"));
}, failure("syncEmailKeypair failure"));
@@ -863,10 +906,10 @@ steal.then(function() {
test("getAssertion with known email that does not have a key", function() {
lib.setOrigin(testOrigin);
+ lib.removeEmail("testuser@testuser.com");
storage.addEmail("testuser@testuser.com", {});
lib.getAssertion("testuser@testuser.com", function onSuccess(assertion) {
- testAssertion(assertion);
- start();
+ testAssertion(assertion, start);
}, failure("getAssertion failure"));
stop();
--
GitLab