Bug 1020156 - Forward URL parameters from about:accounts to the remotely hosted page. r=markh

browser/base/content/aboutaccounts/aboutaccounts.js

@@ -18,6 +18,8 @@ Cu.import("resource://services-sync/util.js");
 const PREF_LAST_FXA_USER = "identity.fxaccounts.lastSignedInUserHash";
 const PREF_SYNC_SHOW_CUSTOMIZATION = "services.sync.ui.showCustomizationDialog";
 
+const ACTION_URL_PARAM = "action";
+
 const OBSERVER_TOPICS = [
   fxAccountsCommon.ONVERIFIED_NOTIFICATION,
   fxAccountsCommon.ONLOGOUT_NOTIFICATION,
@@ -96,7 +98,7 @@ function shouldAllowRelink(acctName) {
 let wrapper = {
   iframe: null,
 
-  init: function (url, entryPoint) {
+  init: function (url, urlParams) {
     let weave = Cc["@mozilla.org/weave/service;1"]
                   .getService(Ci.nsISupports)
                   .wrappedJSObject;
@@ -116,14 +118,14 @@ let wrapper = {
     let iframe = document.getElementById("remote");
     this.iframe = iframe;
     iframe.addEventListener("load", this);
-    try {
-      if (entryPoint) {
-        url += (url.indexOf("?") >= 0 ? "&" : "?") + entryPoint;
-      }
-      iframe.src = url;
-    } catch (e) {
-      error("Couldn't init Firefox Account wrapper: " + e.message);
+
+    // Ideally we'd just merge urlParams with new URL(url).searchParams, but our
+    // URLSearchParams implementation doesn't support iteration (bug 1085284).
+    let urlParamStr = urlParams.toString();
+    if (urlParamStr) {
+      url += (url.contains("?") ? "&" : "?") + urlParamStr;
     }
+    iframe.src = url;
   },
 
   handleEvent: function (evt) {
@@ -295,46 +297,49 @@ function init() {
     if (window.closed) {
       return;
     }
-    // If the url contains an entrypoint query parameter, extract it into a variable
-    // to append it to the accounts URI resource.
-    // Works for the following cases:
-    // - about:accounts?entrypoint="abouthome"
-    // - about:accounts?entrypoint=abouthome&action=signup
-    let entryPointQParam = "entrypoint=";
-    let entryPointPos = window.location.href.indexOf(entryPointQParam);
-    let entryPoint = "";
-    if (entryPointPos >= 0) {
-      entryPoint = window.location.href.substring(entryPointPos).split("&")[0];
-    }
-    if (window.location.href.contains("action=signin")) {
+
+    // Ideally we'd use new URL(document.URL).searchParams, but for about: URIs,
+    // searchParams is empty.
+    let urlParams = new URLSearchParams(document.URL.split("?")[1] || "");
+    let action = urlParams.get(ACTION_URL_PARAM);
+    urlParams.delete(ACTION_URL_PARAM);
+
+    switch (action) {
+    case "signin":
       if (user) {
         // asking to sign-in when already signed in just shows manage.
         show("stage", "manage");
       } else {
         show("remote");
-        wrapper.init(fxAccounts.getAccountsSignInURI(), entryPoint);
+        wrapper.init(fxAccounts.getAccountsSignInURI(), urlParams);
       }
-    } else if (window.location.href.contains("action=signup")) {
+      break;
+    case "signup":
       if (user) {
         // asking to sign-up when already signed in just shows manage.
         show("stage", "manage");
       } else {
         show("remote");
-        wrapper.init(fxAccounts.getAccountsSignUpURI(), entryPoint);
+        wrapper.init(fxAccounts.getAccountsSignUpURI(), urlParams);
       }
-    } else if (window.location.href.contains("action=reauth")) {
+      break;
+    case "reauth":
       // ideally we would only show this when we know the user is in a
       // "must reauthenticate" state - but we don't.
       // As the email address will be included in the URL returned from
       // promiseAccountsForceSigninURI, just always show it.
       fxAccounts.promiseAccountsForceSigninURI().then(url => {
         show("remote");
-        wrapper.init(url, entryPoint);
+        wrapper.init(url, urlParams);
       });
-    } else if (window.location.href.contains("action=migrateToDevEdition") &&
-               user == null) {
-      migrateToDevEdition(user, entryPoint);
-    } else {
+      break;
+    case "migrateToDevEdition":
+      if (user == null) {
+        migrateToDevEdition(user, urlParams);
+        break;
+      }
+      // else, fall through
+    default:
       // No action specified, or migration request when we already have a user.
       if (user) {
         show("stage", "manage");
@@ -343,8 +348,9 @@ function init() {
       } else {
         show("stage", "intro");
         // load the remote frame in the background
-        wrapper.init(fxAccounts.getAccountsSignUpURI(), entryPoint);
+        wrapper.init(fxAccounts.getAccountsSignUpURI(), urlParams);
       }
+      break;
     }
   });
 }
@@ -376,7 +382,7 @@ function show(id, childId) {
 }
 
 // Migrate sync data from the default profile to the dev-edition profile.
-function migrateToDevEdition(user, entryPoint) {
+function migrateToDevEdition(user, urlParams) {
   let migrateSyncCreds = false;
   try {
     migrateSyncCreds = Services.prefs.getBoolPref("identity.fxaccounts.migrateToDevEdition");
@@ -390,13 +396,13 @@ function migrateToDevEdition(user, entryPoint) {
     }).then(() => {
       return fxAccounts.promiseAccountsForceSigninURI().then(url => {
         show("remote");
-        wrapper.init(url, entryPoint);
+        wrapper.init(url, urlParams);
       });
     }).then(null, error => {
       log("Failed to migrate FX Account: " + error);
       show("stage", "intro");
       // load the remote frame in the background
-      wrapper.init(fxAccounts.getAccountsSignUpURI(), entryPoint);
+      wrapper.init(fxAccounts.getAccountsSignUpURI(), urlParams);
     }).then(() => {
       // Reset the pref after migration.
       Services.prefs.setBoolPref("identity.fxaccounts.migrateToDevEdition", false);
@@ -404,7 +410,7 @@ function migrateToDevEdition(user, entryPoint) {
   } else {
     show("stage", "intro");
     // load the remote frame in the background
-    wrapper.init(fxAccounts.getAccountsSignUpURI(), entryPoint);
+    wrapper.init(fxAccounts.getAccountsSignUpURI(), urlParams);
   }
 }
 

browser/base/content/test/general/browser_aboutAccounts.js

@@ -331,6 +331,38 @@ let gTests = [
     is(url, sign_up_url + "&entrypoint=abouthome", "entrypoint=abouthome got the expected URL");
   },
 },
+{
+  desc: "about:accounts URL params should be copied to remote URL params " +
+        "when remote URL has no URL params, except for 'action'",
+  teardown() {
+    gBrowser.removeCurrentTab();
+  },
+  run: function* () {
+    let signupURL = "https://example.com/";
+    setPref("identity.fxaccounts.remote.signup.uri", signupURL);
+    let queryStr = "email=foo%40example.com&foo=bar&baz=quux";
+    let [tab, url] =
+      yield promiseNewTabWithIframeLoadEvent("about:accounts?" + queryStr +
+                                             "&action=action");
+    is(url, signupURL + "?" + queryStr, "URL params are copied to signup URL");
+  },
+},
+{
+  desc: "about:accounts URL params should be copied to remote URL params " +
+        "when remote URL already has some URL params, except for 'action'",
+  teardown() {
+    gBrowser.removeCurrentTab();
+  },
+  run: function* () {
+    let signupURL = "https://example.com/?param";
+    setPref("identity.fxaccounts.remote.signup.uri", signupURL);
+    let queryStr = "email=foo%40example.com&foo=bar&baz=quux";
+    let [tab, url] =
+      yield promiseNewTabWithIframeLoadEvent("about:accounts?" + queryStr +
+                                             "&action=action");
+    is(url, signupURL + "&" + queryStr, "URL params are copied to signup URL");
+  },
+},
 ]; // gTests
 
 function test()