Fix doubly escaped RP name on "verify with primary" screen.

Only singly escape the idpName and siteName. fixes mozilla#3173
callahad · Apr 3, 2013 · 8af36cc · 8af36cc
1 parent 5402de1
commit 8af36cc
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 8 deletions.
diff --git a/resources/static/common/js/user.js b/resources/static/common/js/user.js
@@ -1072,7 +1072,7 @@ BrowserID.User = (function() {
             User.isUserAuthenticatedToPrimary(normalizedEmail, info,
                 function(authed) {
               info.authed = authed;
-              info.idpName = getIdPName(info);
+              info.idpName = _.escape(getIdPName(info));
               complete(info);
             }, onFailure);
           }, onFailure);

diff --git a/resources/static/dialog/js/modules/verify_primary_user.js b/resources/static/dialog/js/modules/verify_primary_user.js
@@ -58,6 +58,10 @@ BrowserID.Modules.VerifyPrimaryUser = (function() {
       email = data.email;
       auth_url = data.auth_url;
 
+      // major assumption:
+      // both siteName and idpName are escaped before they make it here.
+      // siteName is escaped in dialog/js/modules/dialog.js
+      // idpName is escaped in common/js/user.js->addressInfo
       user.addressInfo(email, function onSuccess(info) {
         if (showsPrimaryTransition(info.state)) {
           self.renderForm("verify_primary_user", {

diff --git a/resources/static/dialog/views/verify_primary_user.ejs b/resources/static/dialog/views/verify_primary_user.ejs
@@ -4,26 +4,27 @@
 
 <div class="cf form_section">
 	<h3>
-    <%= format(gettext("%s makes this easy"), [idpName]) %>
+    <%- format(gettext("%s makes this easy"), [idpName]) %>
   </h3>
 
   <p class="description">
     <% if (transition_to_primary) { %>
-      <%=format(gettext("Your %(emailProvider)s account has been upgraded to work even better with Persona. We will redirect you to %(emailProvider)s."), {
+      <%- format(gettext("Your %(emailProvider)s account has been upgraded to work even better with Persona. We will redirect you to %(emailProvider)s."), {
         emailProvider: idpName
       })%>
     <% } else { %>
-      <%= format(gettext("Persona lets you use your %(emailProvider)s account to sign into sites like %(aWebsite)s."),
+      <%- format(gettext("Persona lets you use your %(emailProvider)s account to sign into sites like %(aWebsite)s."),
                { emailProvider: idpName, aWebsite: siteName }) %>
     <% } %>
   </p>
-  <p>
-    <%= format(gettext("Once you verify your account there, you will be signed in to %(aWebsite)s."),
+
+  <p id="postVerify">
+    <%- format(gettext("Once you verify your account there, you will be signed in to %(aWebsite)s."),
                {aWebsite : siteName}) %>
   </p>
 
   <p class="submit cf buttonrow">
-      <button id="verifyWithPrimary"><%= format(gettext("sign in with %s"), [idpName]) %></button>
+      <button id="verifyWithPrimary"><%- format(gettext("sign in with %s"), [idpName]) %></button>
       <a href="#" id="cancel" class="emphasize right"><%= gettext("Use a different email address") %></a>
   </p>
 

diff --git a/resources/static/test/cases/dialog/js/modules/verify_primary_user.js b/resources/static/test/cases/dialog/js/modules/verify_primary_user.js
@@ -20,7 +20,7 @@
     controller.start(config);
   }
 
-  module("controllers/verify_primary_user", {
+  module("dialog/js/modules/verify_primary_user", {
     setup: function() {
       testHelpers.setup();
       win = new WindowMock();
@@ -35,6 +35,35 @@
     }
   });
 
+  asyncTest("siteName and idpName are only escaped once", function() {
+    xhr.useResult("primaryUnknown");
+
+    var messageTriggered = false;
+
+    // siteName and idpName are escaped when they come into the system. The
+    // values do not need to be escaped again. See issue #3173
+    var siteName = _.escape("a / b");
+    var idpName = _.escape("idp / idp++");
+
+    createController({
+      siteName: siteName,
+      idpName: idpName,
+      window: win,
+      add: false,
+      email: "unregistered@testuser.com",
+      auth_url: "http://testuser.com/sign_in",
+      ready: function ready() {
+        var description = $(".description").html();
+        // If there is double escaping going on, the indexOfs will all fail.
+        equal(description.indexOf(_.escape(idpName)), -1);
+        equal(description.indexOf(_.escape(siteName)), -1);
+        equal($("#postVerify").html().indexOf(_.escape(siteName)), -1);
+        start();
+      }
+    });
+
+  });
+
   asyncTest("submit with `add: false` option opens a new tab with proper URL (updated for sessionStorage)", function() {
 
     xhr.useResult("primaryUnknown");