Bug 945363 - Ensure that FXA RPs provide an onlogout handler. r=ferjm

dom/identity/DOMIdentity.jsm

@@ -336,7 +336,15 @@ this.DOMIdentity = {
 
   _unwatch: function DOMIdentity_unwatch(message, targetMM) {
     log("DOMIDentity__unwatch: " + message.id);
-    this.getService(message).RP.unwatch(message.id, targetMM);
+    // If watch failed for some reason (e.g., exception thrown because RP did
+    // not have the right callbacks, we don't want unwatch to throw, because it
+    // will break the process of releasing the page's resources and leak
+    // memory.
+    try {
+      this.getService(message).RP.unwatch(message.id, targetMM);
+    } catch(ex) {
+      log("ERROR: can't unwatch " + message.id + ": " + ex);
+    }
   },
 
   _request: function DOMIdentity__request(message) {

dom/identity/nsDOMIdentity.js

@@ -101,7 +101,7 @@ nsDOMIdentity.prototype = {
    * Relying Party (RP) APIs
    */
 
-  watch: function nsDOMIdentity_watch(aOptions) {
+  watch: function nsDOMIdentity_watch(aOptions = {}) {
     if (this._rpWatcher) {
       // For the initial release of Firefox Accounts, we support callers who
       // invoke watch() either for Firefox Accounts, or Persona, but not both.
@@ -111,33 +111,7 @@ nsDOMIdentity.prototype = {
       throw new Error("navigator.id.watch was already called");
     }
 
-    if (!aOptions || typeof(aOptions) !== "object") {
-      throw new Error("options argument to watch is required");
-    }
-
-    // The relying party (RP) provides callbacks on watch().
-    //
-    // In the future, BrowserID will probably only require an onlogin()
-    // callback [1], lifting the requirement that BrowserID handle logged-in
-    // state management for RPs.  See
-    // https://github.com/mozilla/id-specs/blob/greenfield/browserid/api-rp.md
-    //
-    // However, Firefox Accounts will almost certainly require RPs to provide
-    // onlogout(), onready(), and possibly an onerror() callback.
-    // XXX Bug 945278
-    //
-    // To accomodate the more and less lenient uses of the API, we will simply
-    // be strict about checking for onlogin here.
-    if (typeof(aOptions["onlogin"]) != "function") {
-      throw new Error("onlogin() callback is required.");
-    }
-
-    // Optional callbacks
-    for (let cb of ["onready", "onerror", "onlogout"]) {
-      if (aOptions[cb] && typeof(aOptions[cb]) != "function") {
-        throw new Error(cb + " must be a function");
-      }
-    }
+    assertCorrectCallbacks(aOptions);
 
     let message = this.DOMIdentityMessage(aOptions);
 
@@ -797,7 +771,38 @@ nsDOMIdentityInternal.prototype = {
     interfaces: [],
     classDescription: "Identity DOM Implementation"
   })
-
 };
 
+function assertCorrectCallbacks(aOptions) {
+  // The relying party (RP) provides callbacks on watch().
+  //
+  // In the future, BrowserID will probably only require an onlogin()
+  // callback, lifting the requirement that BrowserID handle logged-in
+  // state management for RPs.  See
+  // https://github.com/mozilla/id-specs/blob/greenfield/browserid/api-rp.md
+  //
+  // However, Firefox Accounts requires callers to provide onlogout(),
+  // onready(), and also supports an onerror() callback.
+
+  let requiredCallbacks = ["onlogin"];
+  let optionalCallbacks = ["onlogout", "onready", "onerror"];
+
+  if (aOptions.wantIssuer == "firefox-accounts") {
+    requiredCallbacks = ["onlogin", "onlogout", "onready"];
+    optionalCallbacks = ["onerror"];
+  }
+
+  for (let cbName of requiredCallbacks) {
+    if (typeof(aOptions[cbName]) != "function") {
+      throw new Error(cbName + " callback is required.");
+    }
+  }
+
+  for (let cbName of optionalCallbacks) {
+    if (aOptions[cbName] && typeof(aOptions[cbName]) != "function") {
+      throw new Error(cbName + " must be a function");
+    }
+  }
+}
+
 this.NSGetFactory = XPCOMUtils.generateNSGetFactory([nsDOMIdentityInternal]);

dom/identity/tests/mochitest/chrome.ini

@@ -1,9 +1,16 @@
 [DEFAULT]
 
 support-files=
+  file_browserid_rp_ok.html
+  file_browserid_rp_noOnlogin.html
   file_declareAudience.html
+  file_fxa_rp_ok.html
+  file_fxa_rp_noOnready.html
+  file_fxa_rp_noOnlogin.html
+  file_fxa_rp_noOnlogout.html
   file_syntheticEvents.html
 
 [test_declareAudience.html]
+[test_rpHasValidCallbacks.html]
 [test_syntheticEvents.html]
 

dom/identity/tests/mochitest/file_browserid_rp_noOnlogin.html

@@ -0,0 +1,44 @@
+<!--
+  * This Source Code Form is subject to the terms of the Mozilla Public
+  * License, v. 2.0. If a copy of the MPL was not distributed with this
+  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+ -->
+<!DOCTYPE html>
+<html>
+  <!--
+  RPs have the correct callbacks for BrowserID or Firefox Accounts
+  https://bugzilla.mozilla.org/show_bug.cgi?id=945363
+  -->
+<head>
+  <meta charset="utf-8">
+  <title>Test app for bug 945363</title>
+</head>
+
+<body>
+<script type="application/javascript;version=1.7">
+
+  // This is a bad RP.  It does not provide an onlogin callback.
+  // nsDOMIdentity will throw an exception when we call watch().
+
+  function postMessage(message) {
+    SpecialPowers.wrap(window).parent
+      .postMessage(JSON.stringify(message), "*");
+  }
+
+  let error = false;
+
+  window.addEventListener('load', function onLoad(event) {
+    window.removeEventListener('load', onLoad);
+    try {
+      navigator.mozId.watch({ });
+    } catch(ex) {
+      error = true;
+    }
+    postMessage({error: error});
+  }, false);
+
+
+
+</script>
+</body>
+</html>

dom/identity/tests/mochitest/file_browserid_rp_ok.html

@@ -0,0 +1,43 @@
+<!--
+  * This Source Code Form is subject to the terms of the Mozilla Public
+  * License, v. 2.0. If a copy of the MPL was not distributed with this
+  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+ -->
+<!DOCTYPE html>
+<html>
+  <!--
+  RPs have the correct callbacks for BrowserID or Firefox Accounts
+  https://bugzilla.mozilla.org/show_bug.cgi?id=945363
+  -->
+<head>
+  <meta charset="utf-8">
+  <title>Test app for bug 945363</title>
+</head>
+
+<body>
+<script type="application/javascript;version=1.7">
+
+  // This is a happy RP.  It has the expected callbacks for BrowserID.
+
+  function postMessage(message) {
+    SpecialPowers.wrap(window).parent
+      .postMessage(JSON.stringify(message), "*");
+  }
+
+  let error = false;
+
+  window.addEventListener('load', function onLoad(event) {
+    window.removeEventListener('load', onLoad);
+    try {
+      navigator.mozId.watch({
+        onlogin: function() {},
+      });
+    } catch(ex) {
+      error = true;
+    }
+    postMessage({error: error});
+  }, false);
+
+</script>
+</body>
+</html>

dom/identity/tests/mochitest/file_fxa_rp_noOnlogin.html

@@ -0,0 +1,46 @@
+<!--
+  * This Source Code Form is subject to the terms of the Mozilla Public
+  * License, v. 2.0. If a copy of the MPL was not distributed with this
+  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+ -->
+<!DOCTYPE html>
+<html>
+  <!--
+  RPs have the correct callbacks for BrowserID or Firefox Accounts
+  https://bugzilla.mozilla.org/show_bug.cgi?id=945363
+  -->
+<head>
+  <meta charset="utf-8">
+  <title>Test app for bug 945363</title>
+</head>
+
+<body>
+<script type="application/javascript;version=1.7">
+
+  // This is a bad Firefox Accounts RP.  It has no onlogin callback.
+  // nsDOMIdentity will throw an exception.
+
+  function postMessage(message) {
+    SpecialPowers.wrap(window).parent
+      .postMessage(JSON.stringify(message), "*");
+  }
+
+  let error = false;
+
+  window.addEventListener('load', function onLoad(event) {
+    window.removeEventListener('load', onLoad);
+    try {
+      navigator.mozId.watch({
+        wantIssuer: "firefox-accounts",
+        onready: function() {},
+        onlogout: function() {},
+      });
+    } catch(ex) {
+      error = true;
+    }
+    postMessage({error: error});
+  }, false);
+
+</script>
+</body>
+</html>

dom/identity/tests/mochitest/file_fxa_rp_noOnlogout.html

@@ -0,0 +1,46 @@
+<!--
+  * This Source Code Form is subject to the terms of the Mozilla Public
+  * License, v. 2.0. If a copy of the MPL was not distributed with this
+  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+ -->
+<!DOCTYPE html>
+<html>
+  <!--
+  RPs have the correct callbacks for BrowserID or Firefox Accounts
+  https://bugzilla.mozilla.org/show_bug.cgi?id=945363
+  -->
+<head>
+  <meta charset="utf-8">
+  <title>Test app for bug 945363</title>
+</head>
+
+<body>
+<script type="application/javascript;version=1.7">
+
+  // This is a bad Firefox Accounts RP.  It has no onlogout callback.
+  // nsDOMIdentity will throw an exception.
+
+  function postMessage(message) {
+    SpecialPowers.wrap(window).parent
+      .postMessage(JSON.stringify(message), "*");
+  }
+
+  let error = false;
+
+  window.addEventListener('load', function onLoad(event) {
+    window.removeEventListener('load', onLoad);
+    try {
+      navigator.mozId.watch({
+        wantIssuer: "firefox-accounts",
+        onlogin: function() {},
+        onready: function() {},
+      });
+    } catch(ex) {
+      error = true;
+    }
+    postMessage({error: error});
+  }, false);
+
+</script>
+</body>
+</html>

dom/identity/tests/mochitest/file_fxa_rp_noOnready.html

@@ -0,0 +1,46 @@
+<!--
+  * This Source Code Form is subject to the terms of the Mozilla Public
+  * License, v. 2.0. If a copy of the MPL was not distributed with this
+  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+ -->
+<!DOCTYPE html>
+<html>
+  <!--
+  RPs have the correct callbacks for BrowserID or Firefox Accounts
+  https://bugzilla.mozilla.org/show_bug.cgi?id=945363
+  -->
+<head>
+  <meta charset="utf-8">
+  <title>Test app for bug 945363</title>
+</head>
+
+<body>
+<script type="application/javascript;version=1.7">
+
+  // This is a bad Firefox Accounts RP.  It has no onready callback.
+  // nsDOMIdentity will throw an exception.
+
+  function postMessage(message) {
+    SpecialPowers.wrap(window).parent
+      .postMessage(JSON.stringify(message), "*");
+  }
+
+  let error = false;
+
+  window.addEventListener('load', function onLoad(event) {
+    window.removeEventListener('load', onLoad);
+    try {
+      navigator.mozId.watch({
+        wantIssuer: "firefox-accounts",
+        onlogin: function() {},
+        onlogout: function() {},
+      });
+    } catch(ex) {
+      error = true;
+    }
+    postMessage({error: error});
+  }, false);
+
+</script>
+</body>
+</html>

dom/identity/tests/mochitest/file_fxa_rp_ok.html

@@ -0,0 +1,46 @@
+<!--
+  * This Source Code Form is subject to the terms of the Mozilla Public
+  * License, v. 2.0. If a copy of the MPL was not distributed with this
+  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+ -->
+<!DOCTYPE html>
+<html>
+  <!--
+  RPs have the correct callbacks for BrowserID or Firefox Accounts
+  https://bugzilla.mozilla.org/show_bug.cgi?id=945363
+  -->
+<head>
+  <meta charset="utf-8">
+  <title>Test app for bug 945363</title>
+</head>
+
+<body>
+<script type="application/javascript;version=1.7">
+
+  // This is a happy RP.  It has the expected callbacks for Firefox Accounts.
+
+  function postMessage(message) {
+    SpecialPowers.wrap(window).parent
+      .postMessage(JSON.stringify(message), "*");
+  }
+
+  let error = false;
+
+  window.addEventListener('load', function onLoad(event) {
+    window.removeEventListener('load', onLoad);
+    try {
+      navigator.mozId.watch({
+        wantIssuer: "firefox-accounts",
+        onready: function() {},
+        onlogin: function() {},
+        onlogout: function() {},
+      });
+    } catch(ex) {
+      error = true;
+    }
+    postMessage({error: error});
+  }, false);
+
+</script>
+</body>
+</html>

dom/identity/tests/mochitest/test_rpHasValidCallbacks.html

@@ -0,0 +1,95 @@
+<!--
+  * This Source Code Form is subject to the terms of the Mozilla Public
+  * License, v. 2.0. If a copy of the MPL was not distributed with this
+  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+ -->
+<!DOCTYPE html>
+<html>
+  <!--
+  https://bugzilla.mozilla.org/show_bug.cgi?id=945363
+  -->
+<head>
+  <meta charset="utf-8">
+  <title>BrowserID and Firefox Accounts RPs provide requried callbacks - Bug 945363</title>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"/>
+</head>
+
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=945363">Mozilla Bug 945363</a>
+<p id="display"></p>
+<div id="content">
+
+</div>
+<pre id="test">
+<script type="application/javascript;version=1.7">
+
+/** Test for Bug 945363 **/
+
+const BASE_URL = "http://mochi.test:8888/chrome/dom/identity/tests/mochitest/";
+
+SimpleTest.waitForExplicitFinish();
+
+// Candidate RPs, and whether we expect them to experience an error.
+// Will will load each of these in turn into an iframe.  The candiates
+// will invoke navigator.mozId.watch().  If they do not provide the
+// correct arguments to watch(), nsDOMIdentity will throw an exception.
+let candidates = [
+  [BASE_URL + "file_browserid_rp_ok.html",          false],
+  [BASE_URL + "file_browserid_rp_noOnlogin.html",   true ],
+  [BASE_URL + "file_fxa_rp_ok.html",                false],
+  [BASE_URL + "file_fxa_rp_noOnlogin.html",         true ],
+  [BASE_URL + "file_fxa_rp_noOnlogout.html",        true ],
+  [BASE_URL + "file_fxa_rp_noOnready.html",         true ],
+];
+
+let checkedCount = 0;
+let checksTodo = candidates.length;
+
+// Each iframe will postMessage to us, telling us whether they caught
+// an exception or not when calling watch().
+window.addEventListener('message', function onMessage(event) {
+  let message = JSON.parse(event.data);
+  let [uri, expectedError] = candidates[checkedCount];
+
+  is(message.error, expectedError, "Unexpected error result from " + uri);
+
+  if (++checkedCount < checksTodo) {
+    testRunner.next();
+  } else {
+    window.removeEventListener('message', onMessage);
+    SimpleTest.finish();
+  }
+}, false);
+
+let content = document.getElementById('content');
+function runTest() {
+  for (let [uri, _] of candidates) {
+
+    let iframe = document.createElement('iframe');
+    iframe.setAttribute('mozbrowser', 'true');
+    iframe.src = uri;
+
+    content.appendChild(iframe);
+
+    yield undefined;
+  }
+}
+
+let testRunner = runTest();
+
+// Enable the identity systems and use verbose logging
+SpecialPowers.pushPrefEnv({'set': [
+    ['dom.identity.enabled', true],               // navigator.mozId
+    ['identity.fxaccounts.enabled', true],        // fx accounts
+    ['dom.identity.syntheticEventsOk', true],     // so we can call request()
+    ['toolkit.identity.debug', true],             // verbose identity logging
+    ['browser.dom.window.dump.enabled', true],
+  ]},
+  function () { testRunner.next(); }
+);
+
+</script>
+</pre>
+</body>
+</html>

toolkit/identity/tests/unit/head_identity.js

@@ -213,8 +213,8 @@ function setup_provisioning(identity, afterSetupCallback, doneProvisioningCallba
         doneProvisioningCallback(err);
     },
     sandbox: {
-  // Emulate the free() method on the iframe sandbox
-  free: function() {}
+      // Emulate the free() method on the iframe sandbox
+      free: function() {}
     }
   };
 

toolkit/identity/tests/unit/test_firefox_accounts.js

@@ -15,18 +15,23 @@ XPCOMUtils.defineLazyModuleGetter(this, "FirefoxAccounts",
 do_get_profile();
 
 function MockFXAManager() {
-  this.signedIn = true;
+  this.signedInUser = true;
 }
 MockFXAManager.prototype = {
   getAssertion: function(audience) {
-    let result = this.signedIn ? TEST_ASSERTION : null;
+    let result = this.signedInUser ? TEST_ASSERTION : null;
     return Promise.resolve(result);
   },
 
   signOut: function() {
-    this.signedIn = false;
+    this.signedInUser = false;
     return Promise.resolve(null);
   },
+
+  signIn: function(user) {
+    this.signedInUser = user;
+    return Promise.resolve(user);
+  },
 }
 
 let originalManager = FirefoxAccounts.fxAccountsManager;
@@ -36,6 +41,14 @@ do_register_cleanup(() => {
   FirefoxAccounts.fxAccountsManager = originalManager;
 });
 
+function withNobodySignedIn() {
+  return FirefoxAccounts.fxAccountsManager.signOut();
+}
+
+function withSomebodySignedIn() {
+  return FirefoxAccounts.fxAccountsManager.signIn('Pertelote');
+}
+
 function test_overall() {
   do_check_neq(FirefoxAccounts, null);
   run_next_test();
@@ -44,10 +57,12 @@ function test_overall() {
 function test_mock() {
   do_test_pending();
 
-  FirefoxAccounts.fxAccountsManager.getAssertion().then(assertion => {
-    do_check_eq(assertion, TEST_ASSERTION);
-    do_test_finished();
-    run_next_test();
+  withSomebodySignedIn().then(() => {
+    FirefoxAccounts.fxAccountsManager.getAssertion().then(assertion => {
+      do_check_eq(assertion, TEST_ASSERTION);
+      do_test_finished();
+      run_next_test();
+    });
   });
 }
 
@@ -70,15 +85,15 @@ function test_watch_signed_in() {
     }
   });
 
-  FirefoxAccounts.RP.watch(mockedRP);
+  withSomebodySignedIn().then(() => {
+    FirefoxAccounts.RP.watch(mockedRP);
+  });
 }
 
 function test_watch_signed_out() {
   do_test_pending();
 
   let received = [];
-  let signedInState = FirefoxAccounts.fxAccountsManager.signedIn;
-  FirefoxAccounts.fxAccountsManager.signedIn = false;
 
   let mockedRP = mock_fxa_rp(null, TEST_URL, function(method) {
     received.push(method);
@@ -89,14 +104,14 @@ function test_watch_signed_out() {
       do_check_eq(received[0], "logout");
       do_check_eq(received[1], "ready");
 
-      // restore initial state
-      FirefoxAccounts.fxAccountsManager.signedIn = signedInState;
       do_test_finished();
       run_next_test();
     }
   });
 
-  FirefoxAccounts.RP.watch(mockedRP);
+  withNobodySignedIn().then(() => {
+    FirefoxAccounts.RP.watch(mockedRP);
+  });
 }
 
 function test_request() {
@@ -104,10 +119,6 @@ function test_request() {
 
   let received = [];
 
-  // initially signed out
-  let signedInState = FirefoxAccounts.fxAccountsManager.signedIn;
-  FirefoxAccounts.fxAccountsManager.signedIn = false;
-
   let mockedRP = mock_fxa_rp(null, TEST_URL, function(method, data) {
     received.push([method, data]);
 
@@ -117,23 +128,24 @@ function test_request() {
       do_check_eq(received[1][0], "ready");
 
       // Pretend request() showed ux and the user signed in
-      FirefoxAccounts.fxAccountsManager.signedIn = true;
-      FirefoxAccounts.RP.request(mockedRP.id);
+      withSomebodySignedIn().then(() => {
+        FirefoxAccounts.RP.request(mockedRP.id);
+      });
     }
 
     if (received.length === 3) {
       do_check_eq(received[2][0], "login");
       do_check_eq(received[2][1], TEST_ASSERTION);
 
-      // restore initial state
-      FirefoxAccounts.fxAccountsManager.signedIn = signedInState;
       do_test_finished();
       run_next_test();
     }
   });
 
-  // First, call watch()
-  FirefoxAccounts.RP.watch(mockedRP);
+  // First, call watch() with nobody signed in
+  withNobodySignedIn().then(() => {
+    FirefoxAccounts.RP.watch(mockedRP);
+  });
 }
 
 function test_logout() {
@@ -160,7 +172,9 @@ function test_logout() {
   });
 
   // First, call watch()
-  FirefoxAccounts.RP.watch(mockedRP);
+  withSomebodySignedIn().then(() => {
+    FirefoxAccounts.RP.watch(mockedRP);
+  });
 }
 
 function test_error() {
@@ -171,11 +185,9 @@ function test_error() {
   // Mock the fxAccountsManager so that getAssertion rejects its promise and
   // triggers our onerror handler.  (This is the method that's used internally
   // by FirefoxAccounts.RP.request().)
-  let originalManager = FirefoxAccounts.fxAccountsManager;
-  FirefoxAccounts.RP.fxAccountsManager = {
-    getAssertion: function(audience) {
-      return Promise.reject(new Error("barf!"));
-    }
+  let originalGetAssertion = FirefoxAccounts.fxAccountsManager.getAssertion;
+  FirefoxAccounts.fxAccountsManager.getAssertion = function(audience) {
+    return Promise.reject(new Error("barf!"));
   };
 
   let mockedRP = mock_fxa_rp(null, TEST_URL, function(method, message) {
@@ -185,14 +197,16 @@ function test_error() {
     do_check_true(/barf/.test(message));
 
     // Put things back the way they were
-    FirefoxAccounts.fxAccountsManager = originalManager;
+    FirefoxAccounts.fxAccountsManager.getAssertion = originalGetAssertion;
 
     do_test_finished();
     run_next_test();
   });
 
   // First, call watch()
-  FirefoxAccounts.RP.watch(mockedRP);
+  withSomebodySignedIn().then(() => {
+    FirefoxAccounts.RP.watch(mockedRP);
+  });
 }
 
 function test_child_process_shutdown() {
@@ -230,7 +244,9 @@ function test_child_process_shutdown() {
   });
 
   mockedRP._mm = "my message manager";
-  FirefoxAccounts.RP.watch(mockedRP);
+  withSomebodySignedIn().then(() => {
+    FirefoxAccounts.RP.watch(mockedRP);
+  });
 
   // fake a dom window context
   DOMIdentity.newContext(mockedRP, mockedRP._mm);