Bug 1047667 - Unregister logged in user from the Loop server upon logout. r=jaws
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Thu, 18 Sep 2014 13:53:44 -0700
changeset 205959 0519bf931920f62e037fbed4798d2173bf348bae
parent 205958 f9a9f246b0d86bcee7f88d512aa3d7ed2f2c16e7
child 206105 34894c383a2ffde71c3f5eef3dff58e5d77246a1
push id8837
push userjwein@mozilla.com
push dateThu, 18 Sep 2014 22:27:40 +0000
treeherderfx-team@0519bf931920 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs1047667
milestone35.0a1
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
@@ -118,16 +118,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"));
 }