Bug 1000395 - Do not sign user out at start of refreshAuth. r=jedp
authorSam Penrose <spenrose@mozilla.com>
Mon, 28 Apr 2014 11:45:58 -0700
changeset 181187 908cc41657a3eca5370d2844ddb72491d21f5ebe
parent 181186 0d45d99bba09560d11c0519c93e12e99df867679
child 181188 34a7215c5d85d699cfb84748d0e31caa5248db9d
child 181254 4eac659fab4d63a8f194620317b1f5afce5df8ba
push id272
push userpvanderbeken@mozilla.com
push dateMon, 05 May 2014 16:31:18 +0000
reviewersjedp
bugs1000395
milestone32.0a1
Bug 1000395 - Do not sign user out at start of refreshAuth. r=jedp
services/fxaccounts/FxAccountsCommon.js
services/fxaccounts/FxAccountsManager.jsm
--- a/services/fxaccounts/FxAccountsCommon.js
+++ b/services/fxaccounts/FxAccountsCommon.js
@@ -100,16 +100,17 @@ this.ERROR_INVALID_BODY_PARAMETERS      
 this.ERROR_INVALID_PASSWORD               = "INVALID_PASSWORD";
 this.ERROR_INVALID_VERIFICATION_CODE      = "INVALID_VERIFICATION_CODE";
 this.ERROR_INVALID_REFRESH_AUTH_VALUE     = "INVALID_REFRESH_AUTH_VALUE";
 this.ERROR_INVALID_REQUEST_SIGNATURE      = "INVALID_REQUEST_SIGNATURE";
 this.ERROR_INTERNAL_INVALID_USER          = "INTERNAL_ERROR_INVALID_USER";
 this.ERROR_MISSING_BODY_PARAMETERS        = "MISSING_BODY_PARAMETERS";
 this.ERROR_MISSING_CONTENT_LENGTH         = "MISSING_CONTENT_LENGTH";
 this.ERROR_NO_TOKEN_SESSION               = "NO_TOKEN_SESSION";
+this.ERROR_NO_SILENT_REFRESH_AUTH         = "NO_SILENT_REFRESH_AUTH";
 this.ERROR_NOT_VALID_JSON_BODY            = "NOT_VALID_JSON_BODY";
 this.ERROR_OFFLINE                        = "OFFLINE";
 this.ERROR_REQUEST_BODY_TOO_LARGE         = "REQUEST_BODY_TOO_LARGE";
 this.ERROR_SERVER_ERROR                   = "SERVER_ERROR";
 this.ERROR_TOO_MANY_CLIENT_REQUESTS       = "TOO_MANY_CLIENT_REQUESTS";
 this.ERROR_SERVICE_TEMP_UNAVAILABLE       = "SERVICE_TEMPORARY_UNAVAILABLE";
 this.ERROR_UI_ERROR                       = "UI_ERROR";
 this.ERROR_UI_REQUEST                     = "UI_REQUEST";
--- a/services/fxaccounts/FxAccountsManager.jsm
+++ b/services/fxaccounts/FxAccountsManager.jsm
@@ -39,16 +39,20 @@ this.FxAccountsManager = {
   // We don't really need to save fxAccounts instance but this way we allow
   // to mock FxAccounts from tests.
   _fxAccounts: fxAccounts,
 
   // We keep the session details here so consumers don't need to deal with
   // session tokens and are only required to handle the email.
   _activeSession: null,
 
+  // Are we refreshing our authentication? If so, allow attempts to sign in
+  // while we are already signed in.
+  _refreshing: false,
+
   // We only expose the email and the verified status so far.
   get _user() {
     if (!this._activeSession || !this._activeSession.email) {
       return null;
     }
 
     return {
       accountId: this._activeSession.email,
@@ -98,26 +102,26 @@ this.FxAccountsManager = {
       return this._error(ERROR_INVALID_ACCOUNTID);
     }
 
     if (!aPassword) {
       return this._error(ERROR_INVALID_PASSWORD);
     }
 
     // Check that there is no signed in account first.
-    if (this._activeSession) {
+    if ((!this._refreshing) && this._activeSession) {
       return this._error(ERROR_ALREADY_SIGNED_IN_USER, {
         user: this._user
       });
     }
 
     let client = this._getFxAccountsClient();
     return this._fxAccounts.getSignedInUser().then(
       user => {
-        if (user) {
+        if ((!this._refreshing) && user) {
           return this._error(ERROR_ALREADY_SIGNED_IN_USER, {
             user: this._user
           });
         }
         return client[aMethod](aAccountId, aPassword);
       }
     ).then(
       user => {
@@ -375,31 +379,42 @@ this.FxAccountsManager = {
               user: user
             });
           }
 
           // RPs might require an authentication refresh.
           if (aOptions &&
               (typeof(aOptions.refreshAuthentication) != "undefined")) {
             let gracePeriod = aOptions.refreshAuthentication;
-            if (typeof gracePeriod != 'number' || isNaN(gracePeriod)) {
+            if (typeof(gracePeriod) !== "number" || isNaN(gracePeriod)) {
               return this._error(ERROR_INVALID_REFRESH_AUTH_VALUE);
             }
-
+            // Forcing refreshAuth to silent is a contradiction in terms,
+            // though it will sometimes succeed silently.
+            if (aOptions.silent) {
+              return this._error(ERROR_NO_SILENT_REFRESH_AUTH);
+            }
             if ((Date.now() / 1000) - this._activeSession.authAt > gracePeriod) {
               // Grace period expired, so we sign out and request the user to
               // authenticate herself again. If the authentication succeeds, we
               // will return the assertion. Otherwise, we will return an error.
-              return this._signOut().then(
-                () => {
-                  if (aOptions.silent) {
-                    return Promise.resolve(null);
-                  }
-                  return this._uiRequest(UI_REQUEST_REFRESH_AUTH,
-                                         aAudience, user.accountId);
+              this._refreshing = true;
+              return this._uiRequest(UI_REQUEST_REFRESH_AUTH,
+                                     aAudience, user.accountId).then(
+                (assertion) => {
+                  this._refreshing = false;
+                  return assertion;
+                },
+                (reason) => {
+                  this._refreshing = false;
+                  return this._signOut().then(
+                    () => {
+                      return this._error(reason);
+                    }
+                  );
                 }
               );
             }
           }
 
           return this._getAssertion(aAudience);
         }