From bfc2b73791520a065451f9d71bf60feeab56826d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez?= Date: Wed, 15 Jan 2014 13:07:20 +0100 Subject: [PATCH] Bug 959535 - FxAccountsManager should clear its account cache when FxAccounts performs a logout. r=markh --- services/fxaccounts/FxAccounts.jsm | 6 +-- services/fxaccounts/FxAccountsCommon.js | 4 ++ services/fxaccounts/FxAccountsManager.jsm | 37 +++++++++++++------ .../fxaccounts/tests/xpcshell/test_manager.js | 11 ++++++ 4 files changed, 44 insertions(+), 14 deletions(-) diff --git a/services/fxaccounts/FxAccounts.jsm b/services/fxaccounts/FxAccounts.jsm index 68d2a6154386..c1af92496a51 100644 --- a/services/fxaccounts/FxAccounts.jsm +++ b/services/fxaccounts/FxAccounts.jsm @@ -111,7 +111,7 @@ InternalMethods.prototype = { this.abortExistingFlow(); this.signedInUser = null; // clear in-memory cache return this.signedInUserStorage.set(null).then(() => { - this.notifyObservers("fxaccounts:onlogout"); + this.notifyObservers(ONLOGOUT_NOTIFICATION); }); }, @@ -198,7 +198,7 @@ InternalMethods.prototype = { // We are now ready for business. This should only be invoked once // per setSignedInUser(), regardless of whether we've rebooted since // setSignedInUser() was called. - internal.notifyObservers("fxaccounts:onlogin"); + internal.notifyObservers(ONLOGIN_NOTIFICATION); return data; }.bind(this)); }, @@ -347,7 +347,7 @@ InternalMethods.prototype = { }, notifyObservers: function(topic) { - log.debug("Notifying observers of user login"); + log.debug("Notifying observers of " + topic); Services.obs.notifyObservers(null, topic, null); }, diff --git a/services/fxaccounts/FxAccountsCommon.js b/services/fxaccounts/FxAccountsCommon.js index 1c65449bcaee..474418b097e2 100644 --- a/services/fxaccounts/FxAccountsCommon.js +++ b/services/fxaccounts/FxAccountsCommon.js @@ -41,6 +41,10 @@ this.KEY_LIFETIME = 1000 * 3600 * 12; // 12 hours this.POLL_SESSION = 1000 * 60 * 5; // 5 minutes this.POLL_STEP = 1000 * 3; // 3 seconds +// Observer notifications. +this.ONLOGIN_NOTIFICATION = "fxaccounts:onlogin"; +this.ONLOGOUT_NOTIFICATION = "fxaccounts:onlogout"; + // Server errno. // From https://github.com/mozilla/fxa-auth-server/blob/master/docs/api.md#response-format this.ERRNO_ACCOUNT_ALREADY_EXISTS = 101; diff --git a/services/fxaccounts/FxAccountsManager.jsm b/services/fxaccounts/FxAccountsManager.jsm index cbae6cd0f959..9d664e9fb5c5 100644 --- a/services/fxaccounts/FxAccountsManager.jsm +++ b/services/fxaccounts/FxAccountsManager.jsm @@ -26,6 +26,19 @@ XPCOMUtils.defineLazyModuleGetter(this, "FxAccountsClient", this.FxAccountsManager = { + init: function() { + Services.obs.addObserver(this, ONLOGOUT_NOTIFICATION, false); + }, + + observe: function(aSubject, aTopic, aData) { + if (aTopic !== ONLOGOUT_NOTIFICATION) { + return; + } + + // Remove the cached session if we get a logout notification. + this._activeSession = null; + }, + // We don't really need to save fxAccounts instance but this way we allow // to mock FxAccounts from tests. _fxAccounts: fxAccounts, @@ -156,22 +169,25 @@ this.FxAccountsManager = { return Promise.resolve(); } - return this._fxAccounts.signOut(this._activeSession.sessionToken).then( + // We clear the local session cache as soon as we get the onlogout + // notification triggered within FxAccounts.signOut, so we save the + // session token value to be able to remove the remote server session + // in case that we have network connection. + let sessionToken = this._activeSession.sessionToken; + + return this._fxAccounts.signOut(sessionToken).then( () => { - // If there is no connection, removing the local session should be - // enough. The client can create new sessions up to the limit (100?). + // At this point the local session should already be removed. + + // The client can create new sessions up to the limit (100?). // Orphaned tokens on the server will eventually be garbage collected. if (Services.io.offline) { - this._activeSession = null; return Promise.resolve(); } // Otherwise, we try to remove the remote session. let client = this._createFxAccountsClient(); - return client.signOut(this._activeSession.sessionToken).then( + return client.signOut(sessionToken).then( result => { - // Even if there is a remote server error, we remove the local - // session. - this._activeSession = null; let error = this._getError(result); if (error) { return Promise.reject({ @@ -183,9 +199,6 @@ this.FxAccountsManager = { return Promise.resolve(); }, reason => { - // Even if there is a remote server error, we remove the local - // session. - this._activeSession = null; return this._serverError(reason); } ); @@ -413,3 +426,5 @@ this.FxAccountsManager = { ); } }; + +FxAccountsManager.init(); diff --git a/services/fxaccounts/tests/xpcshell/test_manager.js b/services/fxaccounts/tests/xpcshell/test_manager.js index 0261f6826e99..cac8fcdda438 100644 --- a/services/fxaccounts/tests/xpcshell/test_manager.js +++ b/services/fxaccounts/tests/xpcshell/test_manager.js @@ -121,6 +121,7 @@ FxAccountsManager._fxAccounts = { signOut: function() { let deferred = Promise.defer(); this._signedInUser = null; + Services.obs.notifyObservers(null, ONLOGOUT_NOTIFICATION, null); deferred.resolve(); return deferred.promise; } @@ -581,3 +582,13 @@ add_test(function(test_queryAccount_no_accountId) { } ); }); + +add_test(function() { + do_print("= Test 23 | fxaccounts:onlogout notification ="); + do_check_true(FxAccountsManager._activeSession != null); + Services.obs.notifyObservers(null, ONLOGOUT_NOTIFICATION, null); + do_execute_soon(function() { + do_check_null(FxAccountsManager._activeSession); + run_next_test(); + }); +});