Bug 976393 - ensure authentication errors log as much detail as possible. r=jedp
authorMark Hammond <mhammond@skippinet.com.au>
Mon, 03 Mar 2014 09:55:30 +1100
changeset 171801 6a9f8ca4c9b1f4c26e526dde27fb520181d9606a
parent 171800 2ed56735cb6af92b1f7dd80dae1b084fa1e1fa24
child 171802 7378692291b756ddda6ed1f981754b1243da2e4e
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewersjedp
bugs976393
milestone30.0a1
Bug 976393 - ensure authentication errors log as much detail as possible. r=jedp
services/common/tokenserverclient.js
services/sync/modules/browserid_identity.js
--- a/services/common/tokenserverclient.js
+++ b/services/common/tokenserverclient.js
@@ -29,31 +29,40 @@ const Prefs = new Preferences("services.
  *        (string) Error message.
  */
 this.TokenServerClientError = function TokenServerClientError(message) {
   this.name = "TokenServerClientError";
   this.message = message || "Client error.";
 }
 TokenServerClientError.prototype = new Error();
 TokenServerClientError.prototype.constructor = TokenServerClientError;
+TokenServerClientError.prototype._toStringFields = function() {
+  return {message: this.message};
+}
+TokenServerClientError.prototype.toString = function() {
+  return this.name + "(" + JSON.stringify(this._toStringFields()) + ")";
+}
 
 /**
  * Represents a TokenServerClient error that occurred in the network layer.
  *
  * @param error
  *        The underlying error thrown by the network layer.
  */
 this.TokenServerClientNetworkError =
  function TokenServerClientNetworkError(error) {
   this.name = "TokenServerClientNetworkError";
   this.error = error;
 }
 TokenServerClientNetworkError.prototype = new TokenServerClientError();
 TokenServerClientNetworkError.prototype.constructor =
   TokenServerClientNetworkError;
+TokenServerClientNetworkError.prototype._toStringFields = function() {
+  return {error: this.error};
+}
 
 /**
  * Represents a TokenServerClient error that occurred on the server.
  *
  * This type will be encountered for all non-200 response codes from the
  * server. The type of error is strongly enumerated and is stored in the
  * `cause` property. This property can have the following string values:
  *
@@ -77,24 +86,39 @@ TokenServerClientNetworkError.prototype.
  *   general -- A general server error has occurred. Clients should
  *     interpret this as an opaque failure.
  *
  * @param message
  *        (string) Error message.
  */
 this.TokenServerClientServerError =
  function TokenServerClientServerError(message, cause="general") {
+  this.now = new Date().toISOString(); // may be useful to diagnose time-skew issues.
   this.name = "TokenServerClientServerError";
   this.message = message || "Server error.";
   this.cause = cause;
 }
 TokenServerClientServerError.prototype = new TokenServerClientError();
 TokenServerClientServerError.prototype.constructor =
   TokenServerClientServerError;
 
+TokenServerClientServerError.prototype._toStringFields = function() {
+  let fields = {
+    now: this.now,
+    message: this.message,
+    cause: this.cause,
+  };
+  if (this.response) {
+    fields.response_body = this.response.body;
+    fields.response_headers = this.response.headers;
+    fields.response_status = this.response.status;
+  }
+  return fields;
+};
+
 /**
  * Represents a client to the Token Server.
  *
  * http://docs.services.mozilla.com/token/index.html
  *
  * The Token Server supports obtaining tokens for arbitrary apps by
  * constructing URI paths of the form <app>/<app_version>. However, the service
  * discovery mechanism emphasizes the use of full URIs and tries to not force
--- a/services/sync/modules/browserid_identity.js
+++ b/services/sync/modules/browserid_identity.js
@@ -50,21 +50,29 @@ function deriveKeyBundle(kB) {
   let bundle = new BulkKeyBundle();
   // [encryptionKey, hmacKey]
   bundle.keyPair = [out.slice(0, 32), out.slice(32, 64)];
   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
+  errors from multiple sources (e.g., from FxAccounts, TokenServer).
+  details is additional details about the error - it might be a string, or
+  some other error object (which should do the right thing when toString() is
+  called on it)
 */
-function AuthenticationError(message) {
-  this.message = message || "";
+function AuthenticationError(details) {
+  this.details = details;
+}
+
+AuthenticationError.prototype = {
+  toString: function() {
+    return "AuthenticationError(" + this.details + ")";
+  }
 }
 
 this.BrowserIDManager = function BrowserIDManager() {
   // NOTE: _fxaService and _tokenServerClient are replaced with mocks by
   // the test suite.
   this._fxaService = fxAccounts;
   this._tokenServerClient = new TokenServerClient();
   // will be a promise that resolves when we are ready to authenticate
@@ -157,21 +165,21 @@ this.BrowserIDManager.prototype = {
           Svc.Prefs.set("firstSync", "resetClient");
           Services.obs.notifyObservers(null, "weave:service:setup-complete", null);
           Weave.Utils.nextTick(Weave.Service.sync, Weave.Service);
         }
       }).then(null, err => {
         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.message);
+        this._log.error("Background fetch for key bundle failed: " + err);
       });
       // and we are done - the fetch continues on in the background...
     }).then(null, err => {
-      this._log.error("Processing logged in account: " + err.message);
+      this._log.error("Processing logged in account: " + err);
     });
   },
 
   observe: function (subject, topic, data) {
     this._log.debug("observed " + topic);
     switch (topic) {
     case fxAccountsCommon.ONLOGIN_NOTIFICATION:
       this.initializeWithCurrentIdentity(true);
@@ -420,17 +428,17 @@ this.BrowserIDManager.prototype = {
     log.info("Fetching assertion and token from: " + tokenServerURI);
 
     function getToken(tokenServerURI, assertion) {
       log.debug("Getting a token");
       let deferred = Promise.defer();
       let cb = function (err, token) {
         if (err) {
           log.info("TokenServerClient.getTokenFromBrowserIDAssertion() failed with: " + err.message);
-          return deferred.reject(new AuthenticationError(err.message));
+          return deferred.reject(new AuthenticationError(err));
         } else {
           log.debug("Successfully got a sync token");
           return deferred.resolve(token);
         }
       };
 
       client.getTokenFromBrowserIDAssertion(tokenServerURI, assertion, cb, headers);
       return deferred.promise;
@@ -460,17 +468,17 @@ this.BrowserIDManager.prototype = {
         token.expiration = this._now() + (token.duration * 1000) * 0.80;
         return token;
       })
       .then(null, err => {
         // 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);
+          this._log.error("Authentication error in _fetchTokenForUser: " + err);
           // 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);
         }