Bug 1244929 - Allow UITour's showFirefoxAccounts command to accept additional URL params. r=MattN

browser/components/uitour/UITour-lib.js

@@ -234,8 +234,21 @@ if (typeof Mozilla == 'undefined') {
     });
   };
 
-  Mozilla.UITour.showFirefoxAccounts = function() {
-    _sendEvent('showFirefoxAccounts');
+  /**
+   * Request the browser open the Firefox Accounts page.
+   *
+   * @param {Object} extraURLCampaignParams - An object containing additional
+   * paramaters for the URL opened by the browser for reasons of promotional
+   * campaign tracking. Each attribute of the object must have a name that
+   * begins with "utm_" and a value that is a string. Both the name and value
+   * must contain only alphanumeric characters, dashes or underscores (meaning
+   * that you are limited to values that don't need encoding, as any such
+   * characters in the name or value will be rejected.)
+   */
+  Mozilla.UITour.showFirefoxAccounts = function(extraURLCampaignParams) {
+    _sendEvent('showFirefoxAccounts', {
+      extraURLCampaignParams: JSON.stringify(extraURLCampaignParams),
+    });
   };
 
   Mozilla.UITour.resetFirefox = function() {

browser/components/uitour/UITour.jsm

@@ -610,8 +610,15 @@ this.UITour = {
       case "showFirefoxAccounts": {
         // 'signup' is the only action that makes sense currently, so we don't
         // accept arbitrary actions just to be safe...
+        let p = new URLSearchParams("action=signup&entrypoint=uitour");
+        // Call our helper to validate extraURLCampaignParams and populate URLSearchParams
+        if (!this._populateCampaignParams(p, data.extraURLCampaignParams)) {
+          log.warn("showFirefoxAccounts: invalid campaign args specified");
+          return false;
+        }
+
         // We want to replace the current tab.
-        browser.loadURI("about:accounts?action=signup&entrypoint=uitour");
+        browser.loadURI("about:accounts?" + p.toString());
         break;
       }
 
@@ -805,6 +812,52 @@ this.UITour = {
     }
   },
 
+  // Given a string that is a JSONified represenation of an object with
+  // additional utm_* URL params that should be appended, validate and append
+  // them to the passed URLSearchParams object. Returns true if the params
+  // were validated and appended, and false if the request should be ignored.
+  _populateCampaignParams: function(urlSearchParams, extraURLCampaignParams) {
+    // We are extra paranoid about what params we allow to be appended.
+    if (typeof extraURLCampaignParams == "undefined") {
+      // no params, so it's all good.
+      return true;
+    }
+    if (typeof extraURLCampaignParams != "string") {
+      log.warn("_populateCampaignParams: extraURLCampaignParams is not a string");
+      return false;
+    }
+    let campaignParams;
+    try {
+      if (extraURLCampaignParams) {
+        campaignParams = JSON.parse(extraURLCampaignParams);
+        if (typeof campaignParams != "object") {
+          log.warn("_populateCampaignParams: extraURLCampaignParams is not a stringified object");
+          return false;
+        }
+      }
+    } catch (ex) {
+      log.warn("_populateCampaignParams: extraURLCampaignParams is not a JSON object");
+      return false;
+    }
+    if (campaignParams) {
+      // The regex that both the name and value of each param must match.
+      let reSimpleString = /^[-_a-zA-Z0-9]*$/;
+      for (let name in campaignParams) {
+        let value = campaignParams[name];
+        if (typeof name != "string" || typeof value != "string" ||
+            !name.startsWith("utm_") ||
+            value.length == 0 ||
+            !reSimpleString.test(name) ||
+            !reSimpleString.test(value)) {
+          log.warn("_populateCampaignParams: invalid campaign param specified");
+          return false;
+        }
+        urlSearchParams.append(name, value);
+      }
+    }
+    return true;
+  },
+
   setTelemetryBucket: function(aPageID) {
     let bucket = BUCKET_NAME + BrowserUITelemetry.BUCKET_SEPARATOR + aPageID;
     BrowserUITelemetry.setBucket(bucket);

browser/components/uitour/test/browser_UITour_sync.js

@@ -22,8 +22,50 @@ add_UITour_task(function* test_checkSyncSetup_enabled() {
 });
 
 // The showFirefoxAccounts API is sync related, so we test that here too...
-add_UITour_task(function* test_firefoxAccounts() {
+add_UITour_task(function* test_firefoxAccountsNoParams() {
   yield gContentAPI.showFirefoxAccounts();
   yield BrowserTestUtils.browserLoaded(gTestTab.linkedBrowser, false,
                                        "about:accounts?action=signup&entrypoint=uitour");
 });
+
+add_UITour_task(function* test_firefoxAccountsValidParams() {
+  yield gContentAPI.showFirefoxAccounts({ utm_foo: "foo", utm_bar: "bar" });
+  yield BrowserTestUtils.browserLoaded(gTestTab.linkedBrowser, false,
+                                       "about:accounts?action=signup&entrypoint=uitour&utm_foo=foo&utm_bar=bar");
+});
+
+// A helper to check the request was ignored due to invalid params.
+function* checkAboutAccountsNotLoaded() {
+  try {
+    yield waitForConditionPromise(() => {
+      return gBrowser.selectedBrowser.currentURI.spec.startsWith("about:accounts");
+    }, "Check if about:accounts opened");
+    ok(false, "No about:accounts tab should have opened");
+  } catch (ex) {
+    ok(true, "No about:accounts tab opened");
+  }
+}
+
+add_UITour_task(function* test_firefoxAccountsNonObject() {
+  // non-string should be rejected.
+  yield gContentAPI.showFirefoxAccounts(99);
+  yield checkAboutAccountsNotLoaded();
+});
+
+add_UITour_task(function* test_firefoxAccountsNonUtmPrefix() {
+  // Any non "utm_" name should should be rejected.
+  yield gContentAPI.showFirefoxAccounts({ utm_foo: "foo", bar: "bar" });
+  yield checkAboutAccountsNotLoaded();
+});
+
+add_UITour_task(function* test_firefoxAccountsNonAlphaName() {
+  // Any "utm_" name which includes non-alpha chars should be rejected.
+  yield gContentAPI.showFirefoxAccounts({ utm_foo: "foo", "utm_bar=": "bar" });
+  yield checkAboutAccountsNotLoaded();
+});
+
+add_UITour_task(function* test_firefoxAccountsNonAlphaValue() {
+  // Any non-alpha value should be rejected.
+  yield gContentAPI.showFirefoxAccounts({ utm_foo: "foo&" });
+  yield checkAboutAccountsNotLoaded();
+});