Bug 1047667 - Unregister logged in user from the Loop server upon logout. r=jaws
☠☠ backed out by a660e08ac3b2 ☠ ☠
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Thu, 18 Sep 2014 13:53:44 -0700
changeset 206142 43b24197d25adaf2030e7b5ce1ea5fa0bdfbc820
parent 206141 5710731f09e99074e79ed0c4420b2598dd42f535
child 206143 87b3c243b7644be4942b2d2dbab17f561b03209c
push id27514
push usercbook@mozilla.com
push dateFri, 19 Sep 2014 12:24:09 +0000
treeherdermozilla-central@3475e6a1665a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs1047667
milestone35.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1047667 - Unregister logged in user from the Loop server upon logout. r=jaws
browser/components/loop/MozLoopAPI.jsm
browser/components/loop/MozLoopService.jsm
browser/components/loop/content/js/panel.js
browser/components/loop/content/js/panel.jsx
browser/components/loop/test/mochitest/browser_fxa_login.js
browser/components/loop/test/mochitest/head.js
browser/components/loop/test/mochitest/loop_fxa.sjs
--- a/browser/components/loop/MozLoopAPI.jsm
+++ b/browser/components/loop/MozLoopAPI.jsm
@@ -421,16 +421,24 @@ function injectLoopAPI(targetWindow) {
     logInToFxA: {
       enumerable: true,
       writable: true,
       value: function() {
         return MozLoopService.logInToFxA();
       }
     },
 
+    logOutFromFxA: {
+      enumerable: true,
+      writable: true,
+      value: function() {
+        return MozLoopService.logOutFromFxA();
+      }
+    },
+
     /**
      * Copies passed string onto the system clipboard.
      *
      * @param {String} str The string to copy
      */
     copyString: {
       enumerable: true,
       writable: true,
--- a/browser/components/loop/MozLoopService.jsm
+++ b/browser/components/loop/MozLoopService.jsm
@@ -68,17 +68,16 @@ XPCOMUtils.defineLazyServiceGetter(this,
 
 
 // The current deferred for the registration process. This is set if in progress
 // or the registration was successful. This is null if a registration attempt was
 // unsuccessful.
 let gRegisteredDeferred = null;
 let gPushHandler = null;
 let gHawkClient = null;
-let gRegisteredLoopServer = false;
 let gLocalizedStrings =  null;
 let gInitializeTimer = null;
 let gFxAOAuthClientPromise = null;
 let gFxAOAuthClient = null;
 let gFxAOAuthTokenData = null;
 let gFxAOAuthProfile = null;
 let gErrors = new Map();
 
@@ -287,16 +286,30 @@ let MozLoopServiceInternal = {
         gRegisteredDeferred.reject("session-token-wrong-size");
         gRegisteredDeferred = null;
         return false;
       }
     }
     return true;
   },
 
+
+  /**
+   * Clear the loop session token so we don't use it for Hawk Requests anymore.
+   *
+   * This should normally be used after unregistering with the server so it can
+   * clean up session state first.
+   *
+   * @param {LOOP_SESSION_TYPE} sessionType The type of session to use for the request.
+   *                                        One of the LOOP_SESSION_TYPE members.
+   */
+  clearSessionToken: function(sessionType) {
+    Services.prefs.clearUserPref(this.getSessionTokenPrefName(sessionType));
+  },
+
   /**
    * Callback from MozLoopPushHandler - The push server has been registered
    * and has given us a push url.
    *
    * @param {String} pushUrl The push url given by the push server.
    */
   onPushRegistered: function(err, pushUrl) {
     if (err) {
@@ -309,17 +322,17 @@ let MozLoopServiceInternal = {
       // storeSessionToken could have rejected and nulled the promise if the token was malformed.
       if (!gRegisteredDeferred) {
         return;
       }
       gRegisteredDeferred.resolve();
       // No need to clear the promise here, everything was good, so we don't need
       // to re-register.
     }, (error) => {
-      Cu.reportError("Failed to register with Loop server: " + error.errno);
+      console.error("Failed to register with Loop server: ", error);
       gRegisteredDeferred.reject(error.errno);
       gRegisteredDeferred = null;
     });
   },
 
   /**
    * Registers with the Loop server either as a guest or a FxA user.
    *
@@ -344,31 +357,61 @@ let MozLoopServiceInternal = {
         if (error.code === 401 && error.errno === INVALID_AUTH_TOKEN) {
           if (this.urlExpiryTimeIsInFuture()) {
             // XXX Should this be reported to the user is a visible manner?
             Cu.reportError("Loop session token is invalid, all previously "
                            + "generated urls will no longer work.");
           }
 
           // Authorization failed, invalid token, we need to try again with a new token.
-          Services.prefs.clearUserPref(this.getSessionTokenPrefName(sessionType));
+          this.clearSessionToken(sessionType);
           if (retry) {
             return this.registerWithLoopServer(sessionType, pushUrl, false);
           }
         }
 
         // XXX Bubble the precise details up to the UI somehow (bug 1013248).
-        Cu.reportError("Failed to register with the loop server. error: " + error);
+        console.error("Failed to register with the loop server. Error: ", error);
         this.setError("registration", error);
         throw error;
       }
     );
   },
 
   /**
+   * Unregisters from the Loop server either as a guest or a FxA user.
+   *
+   * This is normally only wanted for FxA users as we normally want to keep the
+   * guest session with the device.
+   *
+   * @param {LOOP_SESSION_TYPE} sessionType The type of session e.g. guest or FxA
+   * @param {String} pushURL The push URL previously given by the push server.
+   *                         This may not be necessary to unregister in the future.
+   * @return {Promise} resolving when the unregistration request finishes
+   */
+  unregisterFromLoopServer: function(sessionType, pushURL) {
+    let unregisterURL = "/registration?simplePushURL=" + encodeURIComponent(pushURL);
+    return this.hawkRequest(sessionType, unregisterURL, "DELETE")
+      .then(() => {
+        MozLoopServiceInternal.clearSessionToken(sessionType);
+      },
+      error => {
+        // Always clear the registration token regardless of whether the server acknowledges the logout.
+        MozLoopServiceInternal.clearSessionToken(sessionType);
+        if (error.code === 401 && error.errno === INVALID_AUTH_TOKEN) {
+          // Authorization failed, invalid token. This is fine since it may mean we already logged out.
+          return;
+        }
+
+        console.error("Failed to unregister with the loop server. Error: ", error);
+        throw error;
+      });
+  },
+
+  /**
    * Callback from MozLoopPushHandler - A push notification has been received from
    * the server.
    *
    * @param {String} version The version information from the server.
    */
   onHandleNotification: function(version) {
     if (this.doNotDisturb) {
       return;
@@ -1034,16 +1077,40 @@ this.MozLoopService = {
     }).catch(error => {
       gFxAOAuthTokenData = null;
       gFxAOAuthProfile = null;
       throw error;
     });
   },
 
   /**
+   * Logs the user out from FxA.
+   *
+   * Gracefully handles if the user is already logged out.
+   *
+   * @return {Promise} that resolves when the FxA logout flow is complete.
+   */
+  logOutFromFxA: Task.async(function*() {
+    yield MozLoopServiceInternal.unregisterFromLoopServer(LOOP_SESSION_TYPE.FXA,
+                                                          gPushHandler.pushUrl);
+
+    gFxAOAuthTokenData = null;
+    gFxAOAuthProfile = null;
+
+    // Reset the client since the initial promiseFxAOAuthParameters() call is
+    // what creates a new session.
+    gFxAOAuthClient = null;
+    gFxAOAuthClientPromise = null;
+
+    // clearError calls notifyStatusChanged so should be done last when the
+    // state is clean.
+    MozLoopServiceInternal.clearError("registration");
+  }),
+
+  /**
    * Performs a hawk based request to the loop server.
    *
    * @param {LOOP_SESSION_TYPE} sessionType The type of session to use for the request.
    *                                        One of the LOOP_SESSION_TYPE members.
    * @param {String} path The path to make the request to.
    * @param {String} method The request method, e.g. 'POST', 'GET'.
    * @param {Object} payloadObj An object which is converted to JSON and
    *                            transmitted with the request.
--- a/browser/components/loop/content/js/panel.js
+++ b/browser/components/loop/content/js/panel.js
@@ -216,17 +216,16 @@ loop.panel = (function(_, mozL10n) {
     },
 
     handleClickAccountEntry: function() {
       // XXX to be implemented
     },
 
     handleClickAuthEntry: function() {
       if (this._isSignedIn()) {
-        // XXX to be implemented - bug 979845
         navigator.mozLoop.logOutFromFxA();
       } else {
         navigator.mozLoop.logInToFxA();
       }
     },
 
     _isSignedIn: function() {
       return !!navigator.mozLoop.userProfile;
--- a/browser/components/loop/content/js/panel.jsx
+++ b/browser/components/loop/content/js/panel.jsx
@@ -216,17 +216,16 @@ loop.panel = (function(_, mozL10n) {
     },
 
     handleClickAccountEntry: function() {
       // XXX to be implemented
     },
 
     handleClickAuthEntry: function() {
       if (this._isSignedIn()) {
-        // XXX to be implemented - bug 979845
         navigator.mozLoop.logOutFromFxA();
       } else {
         navigator.mozLoop.logInToFxA();
       }
     },
 
     _isSignedIn: function() {
       return !!navigator.mozLoop.userProfile;
--- a/browser/components/loop/test/mochitest/browser_fxa_login.js
+++ b/browser/components/loop/test/mochitest/browser_fxa_login.js
@@ -245,25 +245,35 @@ add_task(function* basicAuthorizationAnd
   ise(tokenData.token_type, "bearer", "Check token_type");
 
   is(MozLoopService.userProfile.email, "test@example.com", "email should exist in the profile data");
   is(MozLoopService.userProfile.uid, "1234abcd", "uid should exist in the profile data");
   is(visibleEmail.textContent, "test@example.com", "the email should be correct on the panel");
   is(loopButton.getAttribute("state"), "active", "state of loop button should be active when logged in");
 
   let registrationResponse = yield promiseOAuthGetRegistration(BASE_URL);
-  ise(registrationResponse.response.simplePushURL, "https://localhost/pushUrl/fxa", "Check registered push URL");
+  ise(registrationResponse.response.simplePushURL, "https://localhost/pushUrl/fxa",
+      "Check registered push URL");
 
   let loopPanel = document.getElementById("loop-notification-panel");
   loopPanel.hidePopup();
   statusChangedPromise = promiseObserverNotified("loop-status-changed");
   yield loadLoopPanel({loopURL: BASE_URL, stayOnline: true});
   yield statusChangedPromise;
   is(loopButton.getAttribute("state"), "", "state of loop button should return to empty after panel is opened");
   loopPanel.hidePopup();
+
+  info("logout");
+  yield MozLoopService.logOutFromFxA();
+  checkLoggedOutState();
+  registrationResponse = yield promiseOAuthGetRegistration(BASE_URL);
+  ise(registrationResponse.response, null,
+      "Check registration was deleted on the server");
+  is(visibleEmail.textContent, "Guest", "Guest should be displayed on the panel again after logout");
+  is(MozLoopService.userProfile, null, "userProfile should be null after logout");
 });
 
 add_task(function* loginWithParams401() {
   resetFxA();
   let params = {
     client_id: "client_id",
     content_uri: BASE_URL + "/content",
     oauth_uri: BASE_URL + "/oauth",
@@ -279,16 +289,62 @@ add_task(function* loginWithParams401() 
     ok(false, "Promise should have rejected");
   },
   error => {
     ise(error.code, 401, "Check error code");
     ise(gFxAOAuthTokenData, null, "Check there is no saved token data");
   });
 });
 
+add_task(function* logoutWithIncorrectPushURL() {
+  resetFxA();
+  let pushURL = "http://www.example.com/";
+  mockPushHandler.pushUrl = pushURL;
+
+  // Create a fake FxA hawk session token
+  const fxASessionPref = MozLoopServiceInternal.getSessionTokenPrefName(LOOP_SESSION_TYPE.FXA);
+  Services.prefs.setCharPref(fxASessionPref, "X".repeat(HAWK_TOKEN_LENGTH));
+
+  yield MozLoopServiceInternal.registerWithLoopServer(LOOP_SESSION_TYPE.FXA, pushURL);
+  let registrationResponse = yield promiseOAuthGetRegistration(BASE_URL);
+  ise(registrationResponse.response.simplePushURL, pushURL, "Check registered push URL");
+  mockPushHandler.pushUrl = "http://www.example.com/invalid";
+  let caught = false;
+  yield MozLoopService.logOutFromFxA().catch((error) => {
+    caught = true;
+  });
+  ok(caught, "Should have caught an error logging out with a mismatched push URL");
+  checkLoggedOutState();
+  registrationResponse = yield promiseOAuthGetRegistration(BASE_URL);
+  ise(registrationResponse.response.simplePushURL, pushURL, "Check registered push URL wasn't deleted");
+});
+
+add_task(function* logoutWithNoPushURL() {
+  resetFxA();
+  let pushURL = "http://www.example.com/";
+  mockPushHandler.pushUrl = pushURL;
+
+  // Create a fake FxA hawk session token
+  const fxASessionPref = MozLoopServiceInternal.getSessionTokenPrefName(LOOP_SESSION_TYPE.FXA);
+  Services.prefs.setCharPref(fxASessionPref, "X".repeat(HAWK_TOKEN_LENGTH));
+
+  yield MozLoopServiceInternal.registerWithLoopServer(LOOP_SESSION_TYPE.FXA, pushURL);
+  let registrationResponse = yield promiseOAuthGetRegistration(BASE_URL);
+  ise(registrationResponse.response.simplePushURL, pushURL, "Check registered push URL");
+  mockPushHandler.pushUrl = null;
+  let caught = false;
+  yield MozLoopService.logOutFromFxA().catch((error) => {
+    caught = true;
+  });
+  ok(caught, "Should have caught an error logging out without a push URL");
+  checkLoggedOutState();
+  registrationResponse = yield promiseOAuthGetRegistration(BASE_URL);
+  ise(registrationResponse.response.simplePushURL, pushURL, "Check registered push URL wasn't deleted");
+});
+
 add_task(function* loginWithRegistration401() {
   resetFxA();
   let params = {
     client_id: "client_id",
     content_uri: BASE_URL + "/content",
     oauth_uri: BASE_URL + "/oauth",
     profile_uri: BASE_URL + "/profile",
     state: "state",
--- a/browser/components/loop/test/mochitest/head.js
+++ b/browser/components/loop/test/mochitest/head.js
@@ -114,16 +114,27 @@ function resetFxA() {
   Services.prefs.clearUserPref(fxASessionPref);
 }
 
 function setInternalLoopGlobal(aName, aValue) {
   let global = Cu.import("resource:///modules/loop/MozLoopService.jsm", {});
   global[aName] = aValue;
 }
 
+function checkLoggedOutState() {
+  let global = Cu.import("resource:///modules/loop/MozLoopService.jsm", {});
+  ise(global.gFxAOAuthClientPromise, null, "gFxAOAuthClientPromise should be cleared");
+  ise(global.gFxAOAuthProfile, null, "gFxAOAuthProfile should be cleared");
+  ise(global.gFxAOAuthClient, null, "gFxAOAuthClient should be cleared");
+  ise(global.gFxAOAuthTokenData, null, "gFxAOAuthTokenData should be cleared");
+  const fxASessionPref = MozLoopServiceInternal.getSessionTokenPrefName(LOOP_SESSION_TYPE.FXA);
+  ise(Services.prefs.getPrefType(fxASessionPref), Services.prefs.PREF_INVALID,
+      "FxA hawk session should be cleared anyways");
+}
+
 function promiseDeletedOAuthParams(baseURL) {
   let deferred = Promise.defer();
   let xhr = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"].
               createInstance(Ci.nsIXMLHttpRequest);
   xhr.open("DELETE", baseURL + "/setup_params", true);
   xhr.addEventListener("load", () => deferred.resolve(xhr));
   xhr.addEventListener("error", deferred.reject);
   xhr.send();
--- a/browser/components/loop/test/mochitest/loop_fxa.sjs
+++ b/browser/components/loop/test/mochitest/loop_fxa.sjs
@@ -6,38 +6,44 @@
  */
 
 "use strict";
 
 const REQUIRED_PARAMS = ["client_id", "content_uri", "oauth_uri", "profile_uri", "state"];
 const HAWK_TOKEN_LENGTH = 64;
 
 Components.utils.import("resource://gre/modules/NetUtil.jsm");
+Components.utils.importGlobalProperties(["URL"]);
 
 /**
  * Entry point for HTTP requests.
  */
 function handleRequest(request, response) {
-  // Look at the query string but ignore past the encoded ? when deciding on the handler.
-  dump("loop_fxa.sjs request for: " + request.queryString + "\n");
-  switch (request.queryString.replace(/%3F.*/,"")) {
+  // Convert the query string to a path with a placeholder base of example.com
+  let url = new URL(request.queryString.replace(/%3F.*/,""), "http://www.example.com");
+  dump("loop_fxa.sjs request for: " + url.pathname + "\n");
+  switch (url.pathname) {
     case "/setup_params": // Test-only
       setup_params(request, response);
       return;
     case "/fxa-oauth/params":
       params(request, response);
       return;
-    case encodeURIComponent("/oauth/authorization"):
+    case "/" + encodeURIComponent("/oauth/authorization"):
       oauth_authorization(request, response);
       return;
     case "/fxa-oauth/token":
       token(request, response);
       return;
     case "/registration":
-      registration(request, response);
+      if (request.method == "DELETE") {
+        delete_registration(request, response);
+      } else {
+        registration(request, response);
+      }
       return;
     case "/get_registration": // Test-only
       get_registration(request, response);
       return;
     case "/profile/profile":
       profile(request, response);
       return;
   }
@@ -197,16 +203,41 @@ function registration(request, response)
     response.setStatusLine(request.httpVersion, 401, "Missing Hawk");
     response.write("401 Missing Hawk Authorization header");
     return;
   }
   setSharedState("/registration", body);
 }
 
 /**
+ * DELETE /registration
+ *
+ * Hawk Authorization headers are required.
+ */
+function delete_registration(request, response) {
+  if (!request.hasHeader("Authorization") ||
+      !request.getHeader("Authorization").startsWith("Hawk")) {
+    response.setStatusLine(request.httpVersion, 401, "Missing Hawk");
+    response.write("401 Missing Hawk Authorization header");
+    return;
+  }
+
+  // Do some query string munging due to the SJS file using a base with a trailing "?"
+  // making the path become a query parameter. This is because we aren't actually
+  // registering endpoints at the root of the hostname e.g. /registration.
+  let url = new URL(request.queryString.replace(/%3F.*/,""), "http://www.example.com");
+  let registration = JSON.parse(getSharedState("/registration"));
+  if (registration.simplePushURL == url.searchParams.get("simplePushURL")) {
+    setSharedState("/registration", "");
+  } else {
+    response.setStatusLine(request.httpVersion, 400, "Bad Request");
+  }
+}
+
+/**
  * GET /get_registration
  *
  * Used for testing purposes to check if registration succeeded by returning the POST body.
  */
 function get_registration(request, response) {
   response.setHeader("Content-Type", "application/json; charset=utf-8", false);
   response.write(getSharedState("/registration"));
 }