From d644d5084a84a79f985093e64b817d402495b099 Mon Sep 17 00:00:00 2001
From: Shane Tomlinson <stomlinson@mozilla.com>
Date: Thu, 24 May 2012 11:27:14 +0100
Subject: [PATCH] Make the in dialog set-password flow work with requiredEmail.
* In set_password.ejs, only show the "cancel" button if not setting the initial password for a required email.
* Update required_email.js to handle the set-password in dialog flow.
* Clean up required_email.js so that it is easier to follow.
* Rename the "add_email_submit_with_secondary" message to "stage_email" - much clearer.
* Show a message whenever resetting the account password.
issue #1638
issue #1580
---
.../dialog/controllers/required_email.js | 80 +++++++++++++------
.../static/dialog/controllers/set_password.js | 3 +-
resources/static/dialog/resources/helpers.js | 2 +-
resources/static/dialog/resources/state.js | 11 ++-
.../static/dialog/views/set_password.ejs | 16 ++--
.../test/cases/controllers/add_email.js | 32 ++++----
.../test/cases/controllers/required_email.js | 56 ++++++++++---
.../test/cases/controllers/set_password.js | 10 ++-
.../static/test/cases/resources/helpers.js | 5 +-
.../static/test/cases/resources/state.js | 53 ++++++++----
10 files changed, 190 insertions(+), 78 deletions(-)
diff --git a/resources/static/dialog/controllers/required_email.js b/resources/static/dialog/controllers/required_email.js
index 67505cbb6..6e5f97e59 100644
--- a/resources/static/dialog/controllers/required_email.js
+++ b/resources/static/dialog/controllers/required_email.js
@@ -6,8 +6,7 @@
BrowserID.Modules.RequiredEmail = (function() {
"use strict";
- var ANIMATION_TIME = 250,
- bid = BrowserID,
+ var bid = BrowserID,
user = bid.User,
errors = bid.Errors,
helpers = bid.Helpers,
@@ -80,7 +79,7 @@ BrowserID.Modules.RequiredEmail = (function() {
dialogHelpers.addEmail.call(self, email);
}
else {
- dialogHelpers.createUser.call(self, email);
+ self.close("new_user", { email: email });
}
}
@@ -118,9 +117,8 @@ BrowserID.Modules.RequiredEmail = (function() {
// the user is not authenticated, all addresses are wiped, meaning
// a user could not be looking at stale data and/or authenticate as
// somebody else.
- var emailInfo = user.getStoredEmailKeypair(email);
- //alert(auth_level + ' ' + JSON.stringify(emailInfo) + JSON.stringify(options));
- if(emailInfo && emailInfo.type === "secondary") {
+ var storedEmailInfo = user.getStoredEmailKeypair(email);
+ if(storedEmailInfo && storedEmailInfo.type === "secondary") {
// secondary user, show the password field if they are not
// authenticated to the "password" level.
showTemplate({
@@ -130,46 +128,78 @@ BrowserID.Modules.RequiredEmail = (function() {
});
ready();
}
- else if(emailInfo && emailInfo.cert) {
+ else if(storedEmailInfo && storedEmailInfo.type === "primary" && storedEmailInfo.cert) {
// primary user with valid cert, user can sign in normally.
- primaryInfo = emailInfo;
+ primaryInfo = storedEmailInfo;
showTemplate({ signin: true, primary: true });
ready();
}
else {
// At this point, there are several possibilities:
- // 1) Authenticated primary user who has valid cert.
- // 2) Authenticated primary user who has an expired cert.
- // 3) Authenticated user who does not control address.
- // 4) Unauthenticated user.
+ // 1) Authenticated primary user who has an expired cert.
+ // 2) Authenticated user who does not control address.
+ // 3) Unauthenticated user.
user.addressInfo(email, function(info) {
if(info.type === "primary") primaryInfo = info;
- if (info.authed) {
+ if (info.type === "primary" && info.authed) {
// this is a primary user who is authenticated with their IdP.
// We know the user has control of this address, give them
// a chance to hit "sign in" before we kick them off to the
// primary flow account.
showTemplate({ signin: true, primary: true });
}
- else if(emailInfo || info.type === "primary") {
- // If there is emailInfo, this is a primary user who has
- // control of the address, but whose cert is expired and the user
- // is not authenticated with their IdP.
- // OR
+ else if(info.type === "primary" && !info.authed) {
// User who does not control a primary address.
+
+ // Kick the user down the primary user flow. User creation and
+ // addition will be taken care of there.
closePrimaryUser.call(self);
- } else if(auth_level || !info.known) {
- // user is authenticated, but does not control address
- // OR
- // address is unknown, make the user verify.
+ }
+ else if(info.type === "secondary" && auth_level === "password") {
+ // address is a secondary that the user does not control.
+
+ // user is authenticated to the password level but does not
+ // control the address, user is adding a secondary address to
+ // their account. Being authenticated to the password level
+ // means the account already has a password, the set_password
+ // step is not necessary. Show the confirmation screen before
+ // the verification starts.
showTemplate({ verify: true });
}
- else {
- // We've made it all this way. It is a user who is not logged in
- // at all and the address is known. Make the user log in.
+ else if(info.type === "secondary" && auth_level === "assertion") {
+ // address is a secondary that the user does not control. At
+ // this point, we need to know whether the account has a password
+ // or not.
+
+ // If the account does not have a password, kick the user down
+ // the stage_email flow which will ask to set a password.
+ // If the account does have a password, show the user
+ // a confirmation screen before starting the verification. When
+ // the user confirms ownership of the address, they may be asked
+ // for their password and their authentication credentials will
+ // be upgraded to "password" status.
+ user.passwordNeededToAddSecondaryEmail(function(passwordNeeded) {
+ if(passwordNeeded) {
+ self.publish("stage_email", { email: email });
+ }
+ else {
+ showTemplate({ verify: true });
+ }
+ });
+ }
+ else if(info.type === "secondary" && info.known) {
+ // address is a known secondary but the user is not logged in.
+
+ // Make the user log in.
showTemplate({ signin: true, password: true });
}
+ else {
+ // address is an unknown secondary. User is not logged in.
+
+ // Create an account. User will have to set their password.
+ self.close("new_user", { email: email });
+ }
ready();
}, self.getErrorDialog(errors.addressInfo, ready));
}
diff --git a/resources/static/dialog/controllers/set_password.js b/resources/static/dialog/controllers/set_password.js
index 308cee328..7bc6848c5 100644
--- a/resources/static/dialog/controllers/set_password.js
+++ b/resources/static/dialog/controllers/set_password.js
@@ -35,7 +35,8 @@ BrowserID.Modules.SetPassword = (function() {
options = options || {};
self.renderDialog("set_password", {
- password_reset: !!options.password_reset
+ password_reset: !!options.password_reset,
+ cancelable: options.cancelable !== false
});
self.click("#cancel", cancel);
diff --git a/resources/static/dialog/resources/helpers.js b/resources/static/dialog/resources/helpers.js
index e7a09abe5..c72534073 100644
--- a/resources/static/dialog/resources/helpers.js
+++ b/resources/static/dialog/resources/helpers.js
@@ -104,7 +104,7 @@
complete(callback, true);
}
else {
- self.publish("add_email_submit_with_secondary", { email: email });
+ self.publish("stage_email", { email: email });
complete(callback, true);
}
}, self.getErrorDialog(errors.addressInfo, callback));
diff --git a/resources/static/dialog/resources/state.js b/resources/static/dialog/resources/state.js
index 6ef339ce5..8a016d6f5 100644
--- a/resources/static/dialog/resources/state.js
+++ b/resources/static/dialog/resources/state.js
@@ -94,6 +94,10 @@ BrowserID.State = (function() {
handleState("new_user", function(msg, info) {
self.newUserEmail = info.email;
+
+ // cancel is disabled if the user is doing the initial password set
+ // for a requiredEmail.
+ info.cancelable = !requiredEmail;
startAction(false, "doSetPassword", info);
});
@@ -347,15 +351,20 @@ BrowserID.State = (function() {
startAction("doAddEmail", info);
});
- handleState("add_email_submit_with_secondary", function(msg, info) {
+ handleState("stage_email", function(msg, info) {
user.passwordNeededToAddSecondaryEmail(function(passwordNeeded) {
if(passwordNeeded) {
self.addEmailEmail = info.email;
+ // cancel is disabled if the user is doing the initial password set
+ // for a requiredEmail.
+ info.cancelable = !requiredEmail;
startAction(false, "doSetPassword", info);
}
else {
startAction(false, "doStageEmail", info);
}
+
+ complete(info.complete);
});
});
diff --git a/resources/static/dialog/views/set_password.ejs b/resources/static/dialog/views/set_password.ejs
index a07b5d873..750764e20 100644
--- a/resources/static/dialog/views/set_password.ejs
+++ b/resources/static/dialog/views/set_password.ejs
@@ -6,11 +6,13 @@
<div class="form_section" id="set_password">
<ul class="inputs">
- <% if(!password_reset) { %>
- <li>
- <%= gettext("Next, choose a new password you'll use when you sign in with BrowserID.") %>
- </li>
- <% } %>
+ <li>
+ <% if(password_reset || !cancelable) { %>
+ <%= gettext("Choose a new password you'll use when you sign in with BrowserID.") %>
+ <% } else { %>
+ <%= gettext("Next, choose a new password you'll use when you sign in with BrowserID.") %>
+ <% } %>
+ </li>
<li>
@@ -49,6 +51,8 @@
<button tabindex="1" id="<%= password_reset ? "password_reset" : "verify_user" %>">
<%= password_reset ? gettext('reset password') : gettext('verify email') %>
</button>
- <a id="cancel" class="action" href="#"><%= gettext('cancel') %></a>
+ <% if(cancelable) { %>
+ <a id="cancel" class="action" href="#"><%= gettext('cancel') %></a>
+ <% } %>
</div>
</div>
diff --git a/resources/static/test/cases/controllers/add_email.js b/resources/static/test/cases/controllers/add_email.js
index b490b519d..6d635e822 100644
--- a/resources/static/test/cases/controllers/add_email.js
+++ b/resources/static/test/cases/controllers/add_email.js
@@ -56,7 +56,7 @@
ok($("#newEmail").val(), "testuser@testuser.com", "email prepopulated");
});
- asyncTest("addEmail with first valid unknown secondary email - trigger add_email_submit_with_secondary", function() {
+ asyncTest("addEmail with first valid unknown secondary email - trigger stage_email", function() {
createController();
xhr.useResult("unknown_secondary");
@@ -64,15 +64,15 @@
$("#newEmail").val("unregistered@testuser.com");
- register("add_email_submit_with_secondary", function(msg, info) {
- equal(info.email, "unregistered@testuser.com", "add_email_submit_with_secondary called with correct email");
+ register("stage_email", function(msg, info) {
+ equal(info.email, "unregistered@testuser.com", "stage_email called with correct email");
start();
});
controller.addEmail();
});
- asyncTest("addEmail with second valid unknown secondary email - trigger add_email_submit_with_secondary", function() {
+ asyncTest("addEmail with second valid unknown secondary email - trigger stage_email", function() {
createController();
xhr.useResult("unknown_secondary");
@@ -80,8 +80,8 @@
$("#newEmail").val("unregistered@testuser.com");
- register("add_email_submit_with_secondary", function(msg, info) {
- equal(info.email, "unregistered@testuser.com", "add_email_submit_with_secondary called with correct email");
+ register("stage_email", function(msg, info) {
+ equal(info.email, "unregistered@testuser.com", "stage_email called with correct email");
start();
});
@@ -89,13 +89,13 @@
controller.addEmail();
});
- asyncTest("addEmail with valid unknown secondary email with leading/trailing whitespace - allows address, triggers add_email_submit_with_secondary", function() {
+ asyncTest("addEmail with valid unknown secondary email with leading/trailing whitespace - allows address, triggers stage_email", function() {
createController();
xhr.useResult("unknown_secondary");
$("#newEmail").val(" unregistered@testuser.com ");
- register("add_email_submit_with_secondary", function(msg, info) {
- equal(info.email, "unregistered@testuser.com", "add_email_submit_with_secondary called with correct email");
+ register("stage_email", function(msg, info) {
+ equal(info.email, "unregistered@testuser.com", "stage_email called with correct email");
start();
});
controller.addEmail();
@@ -106,12 +106,12 @@
$("#newEmail").val("unregistered");
var handlerCalled = false;
- register("add_email_submit_with_secondary", function(msg, info) {
+ register("stage_email", function(msg, info) {
handlerCalled = true;
- ok(false, "add_email_submit_with_secondary should not be called on invalid email");
+ ok(false, "stage_email should not be called on invalid email");
});
controller.addEmail(function() {
- equal(handlerCalled, false, "the add_email_submit_with_secondary handler should have never been called");
+ equal(handlerCalled, false, "the stage_email handler should have never been called");
start();
});
});
@@ -121,8 +121,8 @@
$("#newEmail").val("registered@testuser.com");
- register("add_email_submit_with_secondary", function(msg, info) {
- ok(false, "unexpected add_email_submit_with_secondary message");
+ register("stage_email", function(msg, info) {
+ ok(false, "unexpected stage_email message");
});
// simulate the email being already added.
@@ -142,8 +142,8 @@
xhr.useResult("known_secondary");
$("#newEmail").val("registered@testuser.com");
- register("add_email_submit_with_secondary", function(msg, info) {
- equal(info.email, "registered@testuser.com", "add_email_submit_with_secondary called with correct email");
+ register("stage_email", function(msg, info) {
+ equal(info.email, "registered@testuser.com", "stage_email called with correct email");
start();
});
controller.addEmail();
diff --git a/resources/static/test/cases/controllers/required_email.js b/resources/static/test/cases/controllers/required_email.js
index 5251469d8..53fbed0c1 100644
--- a/resources/static/test/cases/controllers/required_email.js
+++ b/resources/static/test/cases/controllers/required_email.js
@@ -123,15 +123,17 @@
});
});
- asyncTest("unknown_secondary: user who is not authenticated - user must verify", function() {
+ asyncTest("unknown_secondary: user who is not authenticated - kick over to new_user flow", function() {
var email = "unregistered@testuser.com";
xhr.useResult("unknown_secondary");
+ register("new_user", function(item, info) {
+ equal(info.email, email, "correct email");
+ start();
+ });
+
createController({
- email: email,
- ready: function() {
- testVerify(email);
- }
+ email: email
});
});
@@ -311,7 +313,7 @@
});
});
- asyncTest("unknown_secondary: user who is authenticated, but email unknown - user sees verify screen", function() {
+ asyncTest("unknown_secondary: user who is authenticated to password level - user sees verify screen", function() {
xhr.setContextInfo("auth_level", "password");
xhr.useResult("unknown_secondary");
@@ -326,6 +328,40 @@
});
});
+ asyncTest("unknown_secondary: user who is authenticated to assertion level, account already has password - user sees verify screen", function() {
+ xhr.setContextInfo("auth_level", "assertion");
+ xhr.useResult("unknown_secondary");
+
+ storage.addEmail("testuser@testuser.com", { type: "secondary" });
+
+ var email = "unregistered@testuser.com";
+
+ createController({
+ email: email,
+ auth_level: "assertion",
+ ready: function() {
+ testVerify(email);
+ }
+ });
+ });
+
+ asyncTest("unknown_secondary: user who is authenticated to assertion level, account needs password - stage_email triggered", function() {
+ xhr.setContextInfo("auth_level", "assertion");
+ xhr.useResult("unknown_secondary");
+
+ var email = "unregistered@testuser.com";
+
+ register("stage_email", function(msg, info) {
+ testHelpers.testObjectValuesEqual(info, { email: email });
+ start();
+ });
+
+ createController({
+ email: email,
+ auth_level: "assertion"
+ });
+ });
+
asyncTest("secondary: signIn of an authenticated user - generates an assertion, redirects to assertion_generated", function() {
xhr.setContextInfo("auth_level", "password");
@@ -423,18 +459,18 @@
});
- asyncTest("verifyAddress of authenticated user, secondary address belongs to another user - redirects to 'add_email_submit_with_secondary'", function() {
+ asyncTest("verifyAddress of authenticated user, secondary address belongs to another user - redirects to 'stage_email'", function() {
var email = "registered@testuser.com";
xhr.useResult("known_secondary");
- testMessageReceived(email, "add_email_submit_with_secondary");
+ testMessageReceived(email, "stage_email");
});
- asyncTest("verifyAddress of authenticated user, unknown address - redirects to 'add_email_submit_with_secondary'", function() {
+ asyncTest("verifyAddress of authenticated user, unknown address - redirects to 'stage_email'", function() {
var email = "unregistered@testuser.com";
xhr.useResult("unknown_secondary");
- testMessageReceived(email, "add_email_submit_with_secondary");
+ testMessageReceived(email, "stage_email");
});
asyncTest("verifyAddress of un-authenticated user, forgot password - redirect to 'forgot_password'", function() {
diff --git a/resources/static/test/cases/controllers/set_password.js b/resources/static/test/cases/controllers/set_password.js
index bfdda1c0b..a1ae3b3c6 100644
--- a/resources/static/test/cases/controllers/set_password.js
+++ b/resources/static/test/cases/controllers/set_password.js
@@ -31,9 +31,10 @@
});
- test("create with no options - show template, user must verify email", function() {
+ test("create with no options - show template, user must verify email, can cancel", function() {
ok($("#set_password").length, "set_password template added");
equal($("#verify_user").length, 1, "correct button shown");
+ equal($("#cancel").length, 1, "cancel button shown");
});
test("create with password_reset option - show template, show reset password button", function() {
@@ -41,6 +42,13 @@
createController({ password_reset: true });
ok($("#set_password").length, "set_password template added");
equal($("#password_reset").length, 1, "correct button shown");
+ equal($("#cancel").length, 1, "cancel button shown");
+ });
+
+ test("create with cancelable=false option - cancel button not shown", function() {
+ controller.destroy();
+ createController({ cancelable: false });
+ equal($("#cancel").length, 0, "cancel button not shown");
});
asyncTest("submit with good password/vpassword - password_set message raised", function() {
diff --git a/resources/static/test/cases/resources/helpers.js b/resources/static/test/cases/resources/helpers.js
index e7b65da58..7bd85e996 100644
--- a/resources/static/test/cases/resources/helpers.js
+++ b/resources/static/test/cases/resources/helpers.js
@@ -145,12 +145,11 @@
});
});
- asyncTest("addEmail with secondary email - trigger add_email_submit_with_secondary", function() {
+ asyncTest("addEmail with secondary email - trigger stage_email", function() {
xhr.useResult("unknown_secondary");
- expectedMessage("add_email_submit_with_secondary", {
+ expectedMessage("stage_email", {
email: "unregistered@testuser.com"
});
-
dialogHelpers.addEmail.call(controllerMock, "unregistered@testuser.com", function(success) {
equal(success, true, "success status");
start();
diff --git a/resources/static/test/cases/resources/state.js b/resources/static/test/cases/resources/state.js
index 16b634350..1addb9623 100644
--- a/resources/static/test/cases/resources/state.js
+++ b/resources/static/test/cases/resources/state.js
@@ -78,10 +78,23 @@
equal(error, "start: controller must be specified", "creating a state machine without a controller fails");
});
- test("new_user - call doSetPassword with correct email", function() {
+ test("new_user - call doSetPassword with correct email, cancelable set to true", function() {
mediator.publish("new_user", { email: TEST_EMAIL });
- equal(actions.info.doSetPassword.email, TEST_EMAIL, "correct email sent to doSetPassword");
+ testHelpers.testObjectValuesEqual(actions.info.doSetPassword, {
+ email: TEST_EMAIL,
+ cancelable: true
+ });
+ });
+
+ test("new_user with requiredEmail - call doSetPassword with correct email, cancelable set to false", function() {
+ mediator.publish("start", { requiredEmail: TEST_EMAIL });
+ mediator.publish("new_user", { email: TEST_EMAIL });
+
+ testHelpers.testObjectValuesEqual(actions.info.doSetPassword, {
+ email: TEST_EMAIL,
+ cancelable: false
+ });
});
test("cancel new user password_set flow - go back to the authentication screen", function() {
@@ -101,7 +114,7 @@
});
test("password_set for add secondary email - call doStageEmail with correct email", function() {
- mediator.publish("add_email_submit_with_secondary", { email: TEST_EMAIL });
+ mediator.publish("stage_email", { email: TEST_EMAIL });
mediator.publish("password_set");
equal(actions.info.doStageEmail.email, TEST_EMAIL, "correct email sent to doStageEmail");
@@ -464,28 +477,27 @@
});
test("add_email - call doAddEmail", function() {
- mediator.publish("add_email", {
- complete: function() {
- equal(actions.called.doAddEmail, true, "doAddEmail called");
- start();
- }
- });
+ mediator.publish("add_email");
+
+ equal(actions.called.doAddEmail, true, "doAddEmail called");
});
- test("add_email_submit_with_secondary - first secondary email - call doSetPassword", function() {
- mediator.publish("add_email", {
+ asyncTest("stage_email - first secondary email - call doSetPassword with cancelable=true", function() {
+ mediator.publish("stage_email", {
complete: function() {
- equal(actions.called.doSetPassword, true, "doSetPassword called");
+ testHelpers.testObjectValuesEqual(actions.info.doSetPassword, {
+ cancelable: true
+ });
start();
}
});
});
- test("add_email_submit_with_secondary - second secondary email - call doStageEmail", function() {
+ asyncTest("stage_email - second secondary email - call doStageEmail", function() {
storage.addSecondaryEmail("testuser@testuser.com");
- mediator.publish("add_email", {
+ mediator.publish("stage_email", {
complete: function() {
equal(actions.called.doStageEmail, true, "doStageEmail called");
start();
@@ -493,5 +505,18 @@
});
});
+ asyncTest("stage_email first secondary requiredEmail - call doSetPassword with cancelable=false", function() {
+ mediator.publish("start", { requiredEmail: TEST_EMAIL });
+ mediator.publish("stage_email", {
+ complete: function() {
+ testHelpers.testObjectValuesEqual(actions.info.doSetPassword, {
+ cancelable: false
+ });
+ start();
+ }
+ });
+ });
+
+
}());
--
GitLab