From 8af36cc4ba4ead2bd2ec58d821596c0312505146 Mon Sep 17 00:00:00 2001
From: Shane Tomlinson <stomlinson@mozilla.com>
Date: Wed, 3 Apr 2013 17:36:46 +0100
Subject: [PATCH] Fix doubly escaped RP name on "verify with primary" screen.

Only singly escape the idpName and siteName.

fixes #3173
---
 resources/static/common/js/user.js            |  2 +-
 .../dialog/js/modules/verify_primary_user.js  |  4 +++
 .../dialog/views/verify_primary_user.ejs      | 13 ++++----
 .../dialog/js/modules/verify_primary_user.js  | 31 ++++++++++++++++++-
 4 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/resources/static/common/js/user.js b/resources/static/common/js/user.js
index a030cd3ad..48a2ee803 100644
--- 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
index 4fa60b1b8..72f878da6 100644
--- 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
index 439a3b8a7..552ff0f01 100644
--- 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
index 8a38201ab..9261e8105 100644
--- 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");