Bug 965116: Add basic error handling to browserid_identity to respond to various authentication errors, r=markh

services/sync/modules/browserid_identity.js

@@ -46,6 +46,15 @@ function deriveKeyBundle(kB) {
   return bundle;
 }
 
+/*
+  General authentication error for abstracting authentication
+  errors from multiple sources (e.g., from FxAccounts, TokenServer)
+    'message' is a string with a description of the error
+*/
+function AuthenticationError(message) {
+  this.message = message || "";
+}
+
 this.BrowserIDManager = function BrowserIDManager() {
   this._fxaService = fxAccounts;
   this._tokenServerClient = new TokenServerClient();
@@ -86,6 +95,7 @@ this.BrowserIDManager.prototype = {
   initialize: function() {
     Services.obs.addObserver(this, fxAccountsCommon.ONLOGIN_NOTIFICATION, false);
     Services.obs.addObserver(this, fxAccountsCommon.ONLOGOUT_NOTIFICATION, false);
+    Services.obs.addObserver(this, "weave:service:logout:finish", false);
     return this.initializeWithCurrentIdentity();
   },
 
@@ -145,8 +155,7 @@ this.BrowserIDManager.prototype = {
         this._shouldHaveSyncKeyBundle = true; // but we probably don't have one...
         this.whenReadyToAuthenticate.reject(err);
         // report what failed...
-        this._log.error("Background fetch for key bundle failed: " + err);
-        throw err;
+        this._log.error("Background fetch for key bundle failed: " + err.message);
       });
       // and we are done - the fetch continues on in the background...
     }).then(null, err => {
@@ -168,6 +177,14 @@ this.BrowserIDManager.prototype = {
       this._account = null;
       Weave.Service.logout();
       break;
+
+    case "weave:service:logout:finish":
+      // This signals an auth error with the storage server,
+      // or that the user unlinked her account from the browser.
+      // Either way, we clear our auth token. In the case of an
+      // auth error, this will force the fetch of a new one.
+      this._token = null;
+      break;
     }
   },
 
@@ -364,10 +381,12 @@ this.BrowserIDManager.prototype = {
   _fetchSyncKeyBundle: function() {
     // Fetch a sync token for the logged in user from the token server.
     return this._fxaService.getKeys().then(userData => {
-      // unlikely, but if the logged in user somehow changed between these
-      // calls we better fail.
-      if (!userData || userData.email !== this.account) {
-        throw new Error("The currently logged-in user has changed.");
+      // Unlikely, but if the logged in user somehow changed between these
+      // calls we better fail. TODO: add tests for these
+      if (!userData) {
+        throw new AuthenticationError("No userData in _fetchSyncKeyBundle");
+      } else if (userData.email !== this.account) {
+        throw new AuthenticationError("Unexpected user change in _fetchSyncKeyBundle");
       }
       return this._fetchTokenForUser(userData).then(token => {
         this._token = token;
@@ -397,7 +416,7 @@ this.BrowserIDManager.prototype = {
       let cb = function (err, token) {
         if (err) {
           log.info("TokenServerClient.getTokenFromBrowserIDAssertion() failed with: " + err.message);
-          return deferred.reject(err);
+          return deferred.reject(new AuthenticationError(err.message));
         } else {
           return deferred.resolve(token);
         }
@@ -407,19 +426,44 @@ this.BrowserIDManager.prototype = {
       return deferred.promise;
     }
 
-    let audience = Services.io.newURI(tokenServerURI, null, null).prePath;
+    function getAssertion() {
+      let audience = Services.io.newURI(tokenServerURI, null, null).prePath;
+      return fxAccounts.getAssertion(audience).then(null, err => {
+        if (err.code === 401) {
+          throw new AuthenticationError("Unable to get assertion for user");
+        } else {
+          throw err;
+        }
+      });
+    };
+
     // wait until the account email is verified and we know that
     // getAssertion() will return a real assertion (not null).
     return this._fxaService.whenVerified(userData)
-      .then(() => this._fxaService.getAssertion(audience))
+      .then(() => getAssertion())
       .then(assertion => getToken(tokenServerURI, assertion))
       .then(token => {
-        token.expiration = this._now() + (token.duration * 1000);
+        // TODO: Make it be only 80% of the duration, so refresh the token
+        // before it actually expires. This is to avoid sync storage errors
+        // otherwise, we get a nasty notification bar briefly. Bug 966568.
+        token.expiration = this._now() + (token.duration * 1000) * 0.80;
         return token;
       })
       .then(null, err => {
-        Cu.reportError("Failed to fetch token: " + err);
-        // XXX - TODO - work out how to set sync to an error state.
+        // TODO: write tests to make sure that different auth error cases are handled here
+        // properly: auth error getting assertion, auth error getting token (invalid generation
+        // and client-state error)
+        if (err instanceof AuthenticationError) {
+          this._log.error("Authentication error in _fetchTokenForUser: " + err.message);
+          // Drop the sync key bundle, but still expect to have one.
+          // This will arrange for us to be in the right 'currentAuthState'
+          // such that UI will show the right error.
+          this._shouldHaveSyncKeyBundle = true;
+          this._syncKeyBundle = null;
+          Weave.Status.login = this.currentAuthState;
+          Services.obs.notifyObservers(null, "weave:service:login:error", null);
+        }
+        throw err;
       });
   },