diff --git a/resources/static/dialog/controllers/authenticate.js b/resources/static/dialog/controllers/authenticate.js index de6a81d0bb99adb5657ee96a7d48d696bfe21ce9..a805af2408c42dae48f38909827316a6cae3bc5f 100644 --- a/resources/static/dialog/controllers/authenticate.js +++ b/resources/static/dialog/controllers/authenticate.js @@ -18,7 +18,6 @@ BrowserID.Modules.Authenticate = (function() { dom = bid.DOM, lastEmail = "", addressInfo, - siteTOSPP, hints = ["returning","start","addressInfo"]; function getEmail() { @@ -165,11 +164,12 @@ BrowserID.Modules.Authenticate = (function() { dom.hide(".returning,.start"); - // We have to show the TOS/PP agreement to *all* users here. Users who + // We have to show the TOS/PP agreements to *all* users here. Users who // are already authenticated to their IdP but do not have a Persona // account automatically have an account created with no further - // interaction. - dom.show(".tospp"); + // interaction. To make sure they see the TOS/PP agreement, show it + // here. + dialogHelpers.showRPTosPP.call(self); self.bind("#email", "keyup", emailKeyUp); self.click("#forgotPassword", forgotPassword); diff --git a/resources/static/dialog/controllers/required_email.js b/resources/static/dialog/controllers/required_email.js index 6e5f97e59d25682fbda2022dba60da2a733440e8..2314717b335c4674741637666b3cb6611db081ff 100644 --- a/resources/static/dialog/controllers/required_email.js +++ b/resources/static/dialog/controllers/required_email.js @@ -103,6 +103,7 @@ BrowserID.Modules.RequiredEmail = (function() { email = options.email || ""; secondaryAuth = options.secondary_auth; primaryInfo = null; + var siteTOSPP = options.siteTOSPP; function ready() { options.ready && options.ready(); @@ -212,13 +213,28 @@ BrowserID.Modules.RequiredEmail = (function() { signin: false, password: false, secondary_auth: false, - primary: false, - privacy_url: options.privacyURL || null, - tos_url: options.tosURL || null + primary: false }, templateData); self.renderDialog("required_email", templateData); + // For the first revision, if the required_email template is shown, the + // Persona TOS/PP agreement will always be shown. The RP TOS/PP will + // be shown if there is no email set for the origin (user has not + // visited site before). + // + // This leaves two important holes in coverage for both Persona and the + // RP. #1) New secondary user. The new secondary user flow is + // redirected to the "new_user" flow - eventually landing the user on + // the "set_password" screen. #2) New primary user who is not + // authenticated with their IdP. These users are passed on to the + // "primary_user" flow. In neither case is the Persona TOS/PP + // agreement shown. + var originEmail = user.getOriginEmail(); + if (!originEmail && siteTOSPP) { + dialogHelpers.showRPTosPP.call(self); + } + self.click("#sign_in", signIn); self.click("#verify_address", verifyAddress); self.click("#forgotPassword", forgotPassword); diff --git a/resources/static/dialog/controllers/verify_primary_user.js b/resources/static/dialog/controllers/verify_primary_user.js index e42deec4cdf8d0f7eec840ae4d4b02a301927964..97a6317e2e7c06c2901756b83190bd4c31763763 100644 --- a/resources/static/dialog/controllers/verify_primary_user.js +++ b/resources/static/dialog/controllers/verify_primary_user.js @@ -54,11 +54,6 @@ BrowserID.Modules.VerifyPrimaryUser = (function() { }); self.renderDialog("verify_primary_user", templateData); - if(data.siteTOSPP) { - dialogHelpers.showRPTosPP.call(self); - } - dom.show(".tospp"); - self.click("#cancel", cancel); sc.start.call(self, data); diff --git a/resources/static/dialog/css/popup.css b/resources/static/dialog/css/popup.css index 00f8dc697ce0f0666d29aa9a90fe9f300feef28b..d157ccf00caf91116ef1fa9e0ac15b30b08cb748 100644 --- a/resources/static/dialog/css/popup.css +++ b/resources/static/dialog/css/popup.css @@ -394,17 +394,21 @@ div#required_email { } .tospp { - display: none; font-size: 11px; line-height: 1.2; color: #787878; } -#rp_info .tospp { +#rptospp { + display: none; margin: 10px auto; max-width: 280px; } +.rptospp #rptospp { + display: block; +} + a.emphasize { border-bottom: 1px solid #b8babc; border-radius: 2px; diff --git a/resources/static/dialog/resources/helpers.js b/resources/static/dialog/resources/helpers.js index 7419a230c47410f378c0ad5164eb877de956ebab..ebe8f2d27c5136b125fca6bfb6328bf365d3f3d7 100644 --- a/resources/static/dialog/resources/helpers.js +++ b/resources/static/dialog/resources/helpers.js @@ -135,7 +135,7 @@ } function showRPTosPP() { - dom.show(".rptospp"); + dom.addClass("body", "rptospp"); } helpers.Dialog = helpers.Dialog || {}; diff --git a/resources/static/dialog/resources/state.js b/resources/static/dialog/resources/state.js index ad6947ecf277955e8fce2dea852981e07ed0cfcd..13195aaeaad5525c8940b0243e71dc29e27027ad 100644 --- a/resources/static/dialog/resources/state.js +++ b/resources/static/dialog/resources/state.js @@ -42,7 +42,7 @@ BrowserID.State = (function() { handleState("start", function(msg, info) { self.hostname = info.hostname; - self.siteTOSPP = !!(info.privacyURL && info.tosURL); + self.siteTOSPP = !!(info.privacyPolicy && info.termsOfService); requiredEmail = info.requiredEmail; startAction(false, "doRPInfo", info); @@ -304,9 +304,6 @@ BrowserID.State = (function() { // finally reset, the password_reset message will be raised where we must // await email confirmation. self.resetPasswordEmail = info.email; - info = helpers.extend(info || {}, { - siteTOSPP: self.siteTOSPP - }); startAction(false, "doForgotPassword", info); }); @@ -362,10 +359,6 @@ BrowserID.State = (function() { // either 1) primary_user or 2) email_staged. #1 occurs if the email // address is a primary address, #2 occurs if the address is a secondary // and the verification email has been sent. - info = helpers.extend(info || {}, { - siteTOSPP: self.siteTOSPP - }); - startAction("doAddEmail", info); }); diff --git a/resources/static/dialog/views/authenticate.ejs b/resources/static/dialog/views/authenticate.ejs index 57d0fad239416357fa29b1c28e9b9862eb835a88..238cba3a5980f81e16d25ecf58aa771e59364c5a 100644 --- a/resources/static/dialog/views/authenticate.ejs +++ b/resources/static/dialog/views/authenticate.ejs @@ -59,12 +59,12 @@ <button class="returning" tabindex="3"><%= gettext('sign in') %></button> </p> - <p class="tospp bidtospp"> + <p class="tospp"> <%= format( gettext('By proceeding, you agree to %s\'s <a %s>Terms</a> and <a %s>Privacy Policy</a>.'), [ "Persona", - ' href="https://browserid.org/tos" target="_new"', - ' href="https://browserid.org/privacy" target="_new"', + ' href="https://login.persona.org/tos" target="_new"', + ' href="https://login.persona.org/privacy" target="_new"', ]) %> </p> diff --git a/resources/static/dialog/views/required_email.ejs b/resources/static/dialog/views/required_email.ejs index ee613fa961903ce725a8ebce439c58fce3d8691b..831b4d3d5586d298e54367523f34de6a5a0854cf 100644 --- a/resources/static/dialog/views/required_email.ejs +++ b/resources/static/dialog/views/required_email.ejs @@ -13,8 +13,8 @@ <ul class="inputs"> <li> - <label for="email" class="serif"><%= gettext("Email") %></label> - <input id="required_email" class="sans" type="email" value="<%= email %>" disabled /> + <label for="email" ><%= gettext("Email") %></label> + <input id="required_email" type="email" value="<%= email %>" disabled /> <div id="could_not_add" class="tooltip" for="required_email"> <%= gettext("We just sent an email to that address! If you really want to send another, wait a minute or two and try again.") %> </div> @@ -32,9 +32,9 @@ <% if (password) { %> <li id="password_section"> <a id="forgotPassword" class="right forgot" href="#" tabindex="4"><%= gettext("forgot your password?") %></a> - <label for="password" class="serif"><%= gettext("Password") %></label> + <label for="password" ><%= gettext("Password") %></label> - <input id="password" class="sans" type="password" maxlength="80" tabindex="2" /> + <input id="password" type="password" maxlength="80" tabindex="2" /> <div id="password_required" class="tooltip" for="password"> <%= gettext("The password field is required.") %> @@ -50,14 +50,15 @@ </ul> <div class="submit cf"> - <p class="tospp bidtospp"> + <p class="tospp"> <%= format( gettext('By proceeding, you agree to %s\'s <a %s>Terms</a> and <a %s>Privacy Policy</a>.'), - [ "BrowserID", - ' href="https://browserid.org/tos" target="_new"', - ' href="https://browserid.org/privacy" target="_new"', + [ "Persona", + ' href="https://login.persona.org/tos" target="_new"', + ' href="https://login.persona.org/privacy" target="_new"', ]) %> </p> + <p> <% if (signin) { %> <button id="sign_in" tabindex="3"><%= gettext("sign in") %></button> diff --git a/resources/static/dialog/views/rp_info.ejs b/resources/static/dialog/views/rp_info.ejs index e587fbbe6c94b792fdb0a689483762dcbc66b339..724a3e1925dfbf9a1c5d58d6e8bfc67278d31c44 100644 --- a/resources/static/dialog/views/rp_info.ejs +++ b/resources/static/dialog/views/rp_info.ejs @@ -18,7 +18,7 @@ <% } %> <% if(privacyPolicy && termsOfService) { %> - <p class="rptospp tospp"> + <p id="rptospp" class="tospp"> <%= format(gettext("By proceeding, you agree to %s\'s <a %s>Terms</a> and <a %s>Privacy Policy</a>."), [ siteName || hostname, ' href="' + termsOfService + '" id="rp_tos" target="_blank"', diff --git a/resources/static/dialog/views/verify_primary_user.ejs b/resources/static/dialog/views/verify_primary_user.ejs index d3860675929264cdccb53f538a05fb4d92009fb3..a0cdffb03374e422dc3c0e38f74e8bedfb2d7591 100644 --- a/resources/static/dialog/views/verify_primary_user.ejs +++ b/resources/static/dialog/views/verify_primary_user.ejs @@ -24,14 +24,6 @@ </ul> <div class="submit cf"> - <p class="tospp bidtospp"> - <%= format( - gettext('By proceeding, you agree to %s\'s <a %s>Terms</a> and <a %s>Privacy Policy</a>.'), - [ "BrowserID", - ' href="https://browserid.org/tos" target="_new"', - ' href="https://browserid.org/privacy" target="_new"', - ]) %> - </p> <p> <button id="verifyWithPrimary"><%= gettext("verify") %></button> </p> @@ -48,14 +40,6 @@ </p> <div class="submit cf"> - <p class="tospp bidtospp"> - <%= format( - gettext('By proceeding, you agree to %s\'s <a %s>Terms</a> and <a %s>Privacy Policy</a>.'), - [ "BrowserID", - ' href="https://browserid.org/tos" target="_new"', - ' href="https://browserid.org/privacy" target="_new"', - ]) %> - </p> <p> <button id="verifyWithPrimary"><%= gettext("verify") %></button> <a href="#" id="cancel" class="action"><%= gettext("cancel") %></a> diff --git a/resources/static/shared/modules/page_module.js b/resources/static/shared/modules/page_module.js index f0e0f6854f3825e70434b36eba43014376262f51..492e438f9952cf0b7d3b85e77bfd49ff3252b651 100644 --- a/resources/static/shared/modules/page_module.js +++ b/resources/static/shared/modules/page_module.js @@ -129,7 +129,8 @@ BrowserID.Modules.PageModule = (function() { self.hideWait(); self.hideError(); self.hideDelay(); - dom.hide(".tospp"); + + dom.removeClass("body", "rptospp"); screens.form.show(template, data); dom.focus("input:visible:not(:disabled):eq(0)"); diff --git a/resources/static/test/cases/controllers/required_email.js b/resources/static/test/cases/controllers/required_email.js index 53fbed0c1c6c2290a752ec66f4c88cd2de427581..5f5683748108127bd2e17dc6d3565d91c3e35c3e 100644 --- a/resources/static/test/cases/controllers/required_email.js +++ b/resources/static/test/cases/controllers/required_email.js @@ -95,22 +95,55 @@ } - asyncTest("privacyURL and tosURL specified - show TOS/PP", function() { + asyncTest("siteTOSPP specified, no origin email - show TOS/PP", function() { var email = "registered@testuser.com"; xhr.useResult("known_secondary"); + xhr.setContextInfo("auth_level", "password"); - equal($(".tospp").length, 0, "tospp has not yet been added to the DOM"); createController({ - email: "registered@testuser.com", - privacyURL: "http://testuser.com/priv.html", - tosURL: "http://testuser.com/tos.html", + email: email, + siteTOSPP: true, + ready: function() { + testHelpers.testRPTosPPShown(); + start(); + } + }); + }); + + asyncTest("siteTOSPP specified, origin has email - do not show TOS/PP", function() { + var email = "registered@testuser.com"; + xhr.useResult("known_secondary"); + xhr.setContextInfo("auth_level", "password"); + + storage.addSecondaryEmail(email); + user.setOriginEmail(email); + + createController({ + email: email, + siteTOSPP: true, ready: function() { - equal($(".tospp").length, 1, "tospp has been added to the DOM"); + testHelpers.testRPTosPPNotShown(); start(); } }); }); + asyncTest("siteTOSPP not specified, no origin email - do not show TOS/PP", function() { + var email = "registered@testuser.com"; + xhr.useResult("known_secondary"); + xhr.setContextInfo("auth_level", "password"); + + createController({ + email: email, + siteTOSPP: false, + ready: function() { + testHelpers.testRPTosPPNotShown(); + start(); + } + }); + }); + + asyncTest("known_secondary: user who is not authenticated - show password form", function() { var email = "registered@testuser.com"; xhr.useResult("known_secondary"); diff --git a/resources/static/test/cases/controllers/verify_primary_user.js b/resources/static/test/cases/controllers/verify_primary_user.js index 58573fa4a9a4c37c20d6a66c57eb8fdbfd6bda41..de8da5ad94b8b5f25064f5499011a2ecad185421 100644 --- a/resources/static/test/cases/controllers/verify_primary_user.js +++ b/resources/static/test/cases/controllers/verify_primary_user.js @@ -10,7 +10,6 @@ controller, el, testHelpers = bid.TestHelpers, - testElementExists = testHelpers.testElementExists, WindowMock = bid.Mocks.WindowMock, win, mediator = bid.Mediator; @@ -34,32 +33,6 @@ } }); - test("create siteTOSPP set to true - show TOS/PP", function() { - createController({ - window: win, - add: false, - email: "unregistered@testuser.com", - auth_url: "http://testuser.com/sign_in", - siteTOSPP: true - }); - - // XXX There should be a generalized test for this. - testElementExists(".tospp", "tospp has been added to the DOM"); - }); - - test("create with requiredEmail, privacyURL and tosURL defined - show TOS/PP", function() { - createController({ - window: win, - add: false, - requiredEmail: "unregistered@testuser.com", - email: "unregistered@testuser.com", - auth_url: "http://testuser.com/sign_in", - siteTOSPP: true - }); - - testElementExists(".tospp", "tospp has been added to the DOM"); - }); - asyncTest("submit with `add: false` option opens a new tab with CREATE_EMAIL URL", function() { var messageTriggered = false; createController({ diff --git a/resources/static/test/cases/resources/state.js b/resources/static/test/cases/resources/state.js index 306897840d92b974651d9509910265ff9d2011dd..b5a4ebca8f08eb3149bcd92a2bacde4da10f40f5 100644 --- a/resources/static/test/cases/resources/state.js +++ b/resources/static/test/cases/resources/state.js @@ -137,12 +137,12 @@ test("start - RPInfo always started", function() { mediator.publish("start", { - tosURL: "https://browserid.org/TOS.html", - privacyURL: "https://browserid.org/priv.html" + termsOfService: "https://browserid.org/TOS.html", + privacyPolicy: "https://browserid.org/priv.html" }); - ok(actions.info.doRPInfo.tosURL, "doRPInfo called with tosURL set"); - ok(actions.info.doRPInfo.privacyURL, "doRPInfo called with privacyURL set"); + ok(actions.info.doRPInfo.termsOfService, "doRPInfo called with termsOfService set"); + ok(actions.info.doRPInfo.privacyPolicy, "doRPInfo called with privacyPolicy set"); }); test("user_staged - call doConfirmUser", function() { @@ -269,12 +269,12 @@ }); test("forgot_password - call doForgotPassword with correct options", function() { - mediator.publish("start", { privacyURL: "priv.html", tosURL: "tos.html" }); + mediator.publish("start", { privacyPolicy: "priv.html", termsOfService: "tos.html" }); mediator.publish("forgot_password", { email: TEST_EMAIL, requiredEmail: true }); - testActionStarted("doForgotPassword", { email: TEST_EMAIL, requiredEmail: true, siteTOSPP: true }); + testActionStarted("doForgotPassword", { email: TEST_EMAIL, requiredEmail: true }); }); test("password_reset to user_confirmed - call doUserStaged then doEmailConfirmed", function() { @@ -394,7 +394,7 @@ }); test("authenticate - call doAuthenticate with the correct options", function() { - mediator.publish("start", { privacyURL: "priv.html", tosURL: "tos.html" }); + mediator.publish("start", { privacyPolicy: "priv.html", termsOfService: "tos.html" }); mediator.publish("authenticate", { email: TEST_EMAIL }); testActionStarted("doAuthenticate", { email: TEST_EMAIL, siteTOSPP: true }); @@ -424,9 +424,9 @@ test("add_email - call doAddEmail with correct options", function() { - mediator.publish("start", { privacyURL: "priv.html", tosURL: "tos.html" }); + mediator.publish("start", { privacyPolicy: "priv.html", termsOfService: "tos.html" }); mediator.publish("add_email"); - testActionStarted("doAddEmail", { siteTOSPP: true }); + testActionStarted("doAddEmail"); }); asyncTest("email_chosen with secondary email, user must authenticate - call doAuthenticateWithRequiredEmail", function() { @@ -435,7 +435,7 @@ xhr.setContextInfo("auth_level", "assertion"); - mediator.publish("start", { privacyURL: "priv.html", tosURL: "tos.html" }); + mediator.publish("start", { privacyPolicy: "priv.html", termsOfService: "tos.html" }); mediator.publish("email_chosen", { email: email, complete: function() { @@ -489,8 +489,8 @@ test("null assertion generated - preserve original options in doPickEmail", function() { mediator.publish("start", { hostname: "http://example.com", - privacyURL: "http://example.com/priv.html", - tosURL: "http://example.com/tos.html" + privacyPolicy: "http://example.com/priv.html", + termsOfService: "http://example.com/tos.html" }); mediator.publish("assertion_generated", { assertion: null }); diff --git a/resources/static/test/testHelpers/helpers.js b/resources/static/test/testHelpers/helpers.js index 8b65177da7e36526e587f7e738ca3efa25ed7634..25badaf79e045099ab636f585102088970a65a00 100644 --- a/resources/static/test/testHelpers/helpers.js +++ b/resources/static/test/testHelpers/helpers.js @@ -233,14 +233,26 @@ BrowserID.TestHelpers = (function() { msg || (selector + " has className " + className)); }, + testNotHasClass: function(selector, className, msg) { + ok(!$(selector).hasClass(className), + msg || (selector + " does not have className " + className)); + }, + testElementDoesNotExist: function(selector, msg) { equal($(selector).length, 0, msg || ("element '" + selector + "' does not exist")); }, testElementExists: function(selector, msg) { ok($(selector).length, msg || ("element '" + selector + "' exists")); - } + }, + testRPTosPPShown: function(msg) { + TestHelpers.testHasClass("body", "rptospp", msg || "RP TOS/PP shown"); + }, + + testRPTosPPNotShown: function(msg) { + TestHelpers.testNotHasClass("body", "rptospp", msg || "RP TOS/PP not shown"); + } }; return TestHelpers;