From e17c388ae103fdb920536f140dfb097935c5a55b Mon Sep 17 00:00:00 2001 From: Mark Hammond Date: Tue, 28 Jan 2014 17:51:09 -0800 Subject: [PATCH] Bug 959222 (part 1) - Make browserid_identity a first-class identity module. r=rnewman --- services/common/tests/unit/head_helpers.js | 10 ++ services/sync/Weave.js | 105 ++--------- services/sync/modules-testing/utils.js | 17 +- services/sync/modules/browserid_identity.js | 187 +++++++++++++++----- services/sync/modules/constants.js | 1 + services/sync/modules/identity.js | 27 ++- services/sync/modules/policies.js | 4 +- services/sync/modules/service.js | 10 +- services/sync/modules/status.js | 20 ++- 9 files changed, 238 insertions(+), 143 deletions(-) diff --git a/services/common/tests/unit/head_helpers.js b/services/common/tests/unit/head_helpers.js index 039b20219ad9..96840acfd395 100644 --- a/services/common/tests/unit/head_helpers.js +++ b/services/common/tests/unit/head_helpers.js @@ -172,3 +172,13 @@ function uninstallFakePAC() { let CID = PACSystemSettings.CID; Cm.nsIComponentRegistrar.unregisterFactory(CID, PACSystemSettings); } + +// We want to ensure the legacy provider is used for most of these tests; the +// tests that know how to deal with the Firefox Accounts identity hack things +// to ensure that still works. +function setDefaultIdentityConfig() { + Cu.import("resource://gre/modules/Services.jsm"); + Services.prefs.setBoolPref("identity.fxaccounts.enabled", false); +// Services.prefs.setBoolPref("services.sync.fxaccounts.enabled", false); +} +setDefaultIdentityConfig(); diff --git a/services/sync/Weave.js b/services/sync/Weave.js index 3f894a5fc380..2a4362302511 100644 --- a/services/sync/Weave.js +++ b/services/sync/Weave.js @@ -69,7 +69,9 @@ WeaveService.prototype = { // first check if Firefox accounts is available at all. This is so we can // get this landed without forcing Fxa to be used (and require nightly // testers to manually set this pref) - // Once we decide we want Fxa to be available, we just remove this block. + // Once we decide we want Fxa to be available, we just remove this block + // (although a fly in this ointment is tests - it might be that we must + // just set this as a pref with a default of true) let fxAccountsAvailable; try { fxAccountsAvailable = Services.prefs.getBoolPref("identity.fxaccounts.enabled"); @@ -101,58 +103,12 @@ WeaveService.prototype = { return this.fxAccountsEnabled = fxAccountsEnabled; }, - maybeInitWithFxAccountsAndEnsureLoaded: function() { - Components.utils.import("resource://services-sync/main.js"); - // FxAccounts imports lots of stuff, so only do this as we need it - Cu.import("resource://gre/modules/FxAccounts.jsm"); - - // This isn't quite sufficient here to handle all the cases. Cases - // we need to handle: - // - User is signed in to FxAccounts, btu hasn't set up sync. - return fxAccounts.getSignedInUser().then( - (accountData) => { - if (accountData) { - Cu.import("resource://services-sync/browserid_identity.js"); - // The Sync Identity module needs to be set in both these places if - // it's swapped out as we are doing here. When Weave.Service initializes - // it grabs a reference to Weave.Status._authManager, and for references - // to Weave.Service.identity to resolve correctly, we also need to reset - // Weave.Service.identity as well. - Weave.Service.identity = Weave.Status._authManager = new BrowserIDManager(), - // Init the identity module with any account data from - // firefox accounts. The Identity module will fetch the signed in - // user from fxAccounts directly. - Weave.Service.identity.initWithLoggedInUser().then(function () { - // Set the cluster data that we got from the token - Weave.Service.clusterURL = Weave.Service.identity.clusterURL; - // checkSetup() will check the auth state of the identity module - // and records that status in Weave.Status - if (Weave.Status.checkSetup() != Weave.CLIENT_NOT_CONFIGURED) { - // This makes sure that Weave.Service is loaded - Svc.Obs.notify("weave:service:setup-complete"); - // TODO: this shouldn't be here. It should be at the end - // of the promise chain of the 'fxaccounts:onverified' handler. - Weave.Utils.nextTick(Weave.Service.sync, Weave.Service); - this.ensureLoaded(); - } - }.bind(this)); - } else if (Weave.Status.checkSetup() != Weave.CLIENT_NOT_CONFIGURED) { - // This makes sure that Weave.Service is loaded - this.ensureLoaded(); - } - }, - (err) => {dump("err in getting logged in account "+err.message)} - ).then(null, (err) => {dump("err in processing logged in account "+err.message)}); - }, - observe: function (subject, topic, data) { switch (topic) { case "app-startup": let os = Cc["@mozilla.org/observer-service;1"]. getService(Ci.nsIObserverService); os.addObserver(this, "final-ui-startup", true); - os.addObserver(this, "fxaccounts:onverified", true); - os.addObserver(this, "fxaccounts:onlogout", true); break; case "final-ui-startup": @@ -160,53 +116,24 @@ WeaveService.prototype = { this.timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer); this.timer.initWithCallback({ notify: function() { - if (this.fxAccountsEnabled) { - // init the fxAccounts identity manager. - this.maybeInitWithFxAccountsAndEnsureLoaded(); - } else { - // init the "old" style, sync-specific identity manager. - // We only load more if it looks like Sync is configured. - let prefs = Services.prefs.getBranch(SYNC_PREFS_BRANCH); - if (!prefs.prefHasUserValue("username")) { - return; - } + // We only load more if it looks like Sync is configured. + let prefs = Services.prefs.getBranch(SYNC_PREFS_BRANCH); + if (!prefs.prefHasUserValue("username")) { + return; + } - // We have a username. So, do a more thorough check. This will - // import a number of modules and thus increase memory - // accordingly. We could potentially copy code performed by - // this check into this file if our above code is yielding too - // many false positives. - Components.utils.import("resource://services-sync/main.js"); - if (Weave.Status.checkSetup() != Weave.CLIENT_NOT_CONFIGURED) { - this.ensureLoaded(); - } + // We have a username. So, do a more thorough check. This will + // import a number of modules and thus increase memory + // accordingly. We could potentially copy code performed by + // this check into this file if our above code is yielding too + // many false positives. + Components.utils.import("resource://services-sync/main.js"); + if (Weave.Status.checkSetup() != Weave.CLIENT_NOT_CONFIGURED) { + this.ensureLoaded(); } }.bind(this) }, 10000, Ci.nsITimer.TYPE_ONE_SHOT); break; - - case 'fxaccounts:onverified': - // Tell sync that if this is a first sync, it should try and sync the - // server data with what is on the client - despite the name implying - // otherwise, this is what "resetClient" does. - // TOOD: This implicitly assumes we're in the CLIENT_NOT_CONFIGURED state, and - // if we're not, we should handle it here. - Components.utils.import("resource://services-sync/main.js"); // ensure 'Weave' exists - Weave.Svc.Prefs.set("firstSync", "resetClient"); - this.maybeInitWithFxAccountsAndEnsureLoaded().then(() => { - // and off we go... - // TODO: I have this being done in maybeInitWithFxAccountsAndEnsureLoaded - // because I had a bug in the promise chains that was triggering this - // too early. This should be fixed. - //Weave.Utils.nextTick(Weave.Service.sync, Weave.Service); - }); - break; - case 'fxaccounts:onlogout': - Components.utils.import("resource://services-sync/main.js"); // ensure 'Weave' exists - // startOver is throwing some errors and we can't re-log in in this - // session - so for now, we don't do this! - //Weave.Service.startOver(); - break; } } }; diff --git a/services/sync/modules-testing/utils.js b/services/sync/modules-testing/utils.js index 3f0a1f7bca1a..9c045b6f5596 100644 --- a/services/sync/modules-testing/utils.js +++ b/services/sync/modules-testing/utils.js @@ -150,7 +150,10 @@ this.configureIdentity = function(identityOverrides) { if (ns.Service.identity instanceof BrowserIDManager) { // do the FxAccounts thang... configureFxAccountIdentity(ns.Service.identity, config); - return ns.Service.identity.initWithLoggedInUser(); + return ns.Service.identity.initializeWithCurrentIdentity().then(() => { + // need to wait until this identity manager is readyToAuthenticate. + return ns.Service.identity.whenReadyToAuthenticate.promise; + }); } // old style identity provider. setBasicCredentials(config.username, config.sync.password, config.sync.syncKey); @@ -172,7 +175,9 @@ this.SyncTestingInfrastructure = function (server, username, password, syncKey) config.sync.password = password; if (syncKey) config.sync.syncKey = syncKey; - configureIdentity(config); + let cb = Async.makeSpinningCallback(); + configureIdentity(config).then(cb, cb); + cb.wait(); let i = server.identity; let uri = i.primaryScheme + "://" + i.primaryHost + ":" + @@ -224,16 +229,16 @@ this.add_identity_test = function(test, testFunction) { test.add_task(function() { note("sync"); let oldIdentity = Status._authManager; - Status._authManager = ns.Service.identity = new IdentityManager(); + Status.__authManager = ns.Service.identity = new IdentityManager(); yield testFunction(); - Status._authManager = ns.Service.identity = oldIdentity; + Status.__authManager = ns.Service.identity = oldIdentity; }); // another task for the FxAccounts identity manager. test.add_task(function() { note("FxAccounts"); let oldIdentity = Status._authManager; - Status._authManager = ns.Service.identity = new BrowserIDManager(); + Status.__authManager = ns.Service.identity = new BrowserIDManager(); yield testFunction(); - Status._authManager = ns.Service.identity = oldIdentity; + Status.__authManager = ns.Service.identity = oldIdentity; }); } diff --git a/services/sync/modules/browserid_identity.js b/services/sync/modules/browserid_identity.js index 12a85d05c5b3..db15011f08f7 100644 --- a/services/sync/modules/browserid_identity.js +++ b/services/sync/modules/browserid_identity.js @@ -18,6 +18,7 @@ Cu.import("resource://services-common/tokenserverclient.js"); Cu.import("resource://gre/modules/Services.jsm"); Cu.import("resource://services-sync/constants.js"); Cu.import("resource://gre/modules/Promise.jsm"); +Cu.import("resource://services-sync/stages/cluster.js"); // Lazy imports to prevent unnecessary load on startup. XPCOMUtils.defineLazyModuleGetter(this, "BulkKeyBundle", @@ -26,6 +27,12 @@ XPCOMUtils.defineLazyModuleGetter(this, "BulkKeyBundle", XPCOMUtils.defineLazyModuleGetter(this, "fxAccounts", "resource://gre/modules/FxAccounts.jsm"); +XPCOMUtils.defineLazyGetter(this, 'fxAccountsCommon', function() { + let ob = {}; + Cu.import("resource://gre/modules/FxAccountsCommon.js", ob); + return ob; +}); + function deriveKeyBundle(kB) { let out = CryptoUtils.hkdf(kB, undefined, "identity.mozilla.com/picl/v1/oldsync", 2*32); @@ -39,6 +46,8 @@ function deriveKeyBundle(kB) { this.BrowserIDManager = function BrowserIDManager() { this._fxaService = fxAccounts; this._tokenServerClient = new TokenServerClient(); + // will be a promise that resolves when we are ready to authenticate + this.whenReadyToAuthenticate = null; this._log = Log.repository.getLogger("Sync.BrowserIDManager"); this._log.Level = Log.Level[Svc.Prefs.get("log.logger.identity")]; @@ -53,6 +62,80 @@ this.BrowserIDManager.prototype = { _token: null, _account: null, + // it takes some time to fetch a sync key bundle, so until this flag is set, + // we don't consider the lack of a keybundle as a failure state. + _shouldHaveSyncKeyBundle: false, + + get readyToAuthenticate() { + // We are finished initializing when we *should* have a sync key bundle, + // although we might not actually have one due to auth failures etc. + return this._shouldHaveSyncKeyBundle; + }, + + initialize: function() { + Services.obs.addObserver(this, fxAccountsCommon.ONVERIFIED_NOTIFICATION, false); + Services.obs.addObserver(this, fxAccountsCommon.ONLOGOUT_NOTIFICATION, false); + return this.initializeWithCurrentIdentity(); + }, + + initializeWithCurrentIdentity: function() { + this._log.trace("initializeWithCurrentIdentity"); + Components.utils.import("resource://services-sync/main.js"); + + // Reset the world before we do anything async. + this.whenReadyToAuthenticate = Promise.defer(); + this._shouldHaveSyncKeyBundle = false; + this.username = ""; // this calls resetCredentials which drops the key bundle. + + return fxAccounts.getSignedInUser().then(accountData => { + if (!accountData) { + this._log.info("initializeWithCurrentIdentity has no user logged in"); + this._account = null; + return; + } + this._account = accountData.email; + // We start a background keybundle fetch... + this._log.info("Starting background fetch for key bundle."); + this._fetchSyncKeyBundle().then(() => { + this._shouldHaveSyncKeyBundle = true; // and we should actually have one... + this.whenReadyToAuthenticate.resolve(); + this._log.info("Background fetch for key bundle done"); + }).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); + throw err; + }); + // and we are done - the fetch continues on in the background... + }).then(null, err => { + dump("err in processing logged in account "+err.message); + }); + }, + + observe: function (subject, topic, data) { + switch (topic) { + case fxAccountsCommon.ONVERIFIED_NOTIFICATION: + case fxAccountsCommon.ONLOGIN_NOTIFICATION: + // For now, we just assume it's the same user logging back in. + // Bug 958927 exists to work out what to do if that's not true. It might + // be that the :onlogout observer does a .startOver (or maybe not - TBD) + // But for now, do nothing, and sync will just start re-synching in its + // own sweet time... + this.initializeWithCurrentIdentity(); + break; + + case fxAccountsCommon.ONLOGOUT_NOTIFICATION: + Components.utils.import("resource://services-sync/main.js"); + // Setting .username calls resetCredentials which drops the key bundle + // and resets _shouldHaveSyncKeyBundle. + this.username = ""; + this._account = null; + Weave.Service.logout(); + break; + } + }, + /** * Provide override point for testing token expiration. */ @@ -60,8 +143,6 @@ this.BrowserIDManager.prototype = { return Date.now(); }, - clusterURL: null, - get account() { return this._account; }, @@ -148,6 +229,7 @@ this.BrowserIDManager.prototype = { this._syncKey = null; this._syncKeyBundle = null; this._syncKeyUpdated = true; + this._shouldHaveSyncKeyBundle = false; }, /** @@ -166,8 +248,8 @@ this.BrowserIDManager.prototype = { // No need to check this.syncKey as our getter for that attribute // uses this.syncKeyBundle - // If bundle creation failed. - if (!this.syncKeyBundle) { + // If bundle creation started, but failed. + if (this._shouldHaveSyncKeyBundle && !this.syncKeyBundle) { return LOGIN_FAILED_NO_PASSPHRASE; } @@ -221,45 +303,24 @@ this.BrowserIDManager.prototype = { return userData; }, - // initWithLoggedInUser will fetch the logged in user from firefox accounts, - // and if such a logged in user exists, will use that user to initialize - // the identity module. Returns a Promise. - initWithLoggedInUser: function() { - // Get the signed in user from FxAccounts. - return this._fxaService.getSignedInUser() - .then(userData => { - if (!userData) { - this._log.warn("initWithLoggedInUser found no logged in user"); - throw new Error("initWithLoggedInUser found no logged in user"); - } - // Make a note of the last logged in user. - this._account = userData.email; - // Fetch a sync token for the logged in user from the token server. - return this._refreshTokenForLoggedInUser(); - }) - .then(token => { - this._token = token; - // Set the username to be the uid returned by the token server. - // TODO: check here to see if the uid is different that the current - // this.username. If so, we may need to reinit sync, detect if the new - // user has sync set up, etc - this.username = this._token.uid.toString(); - - return this._fxaService.getKeys(); - }) - .then(userData => { - // both Jelly and FxAccounts give us kA/kB as hex. - let kB = Utils.hexToBytes(userData.kB); - this._syncKeyBundle = deriveKeyBundle(kB); - - // Set the clusterURI for this user based on the endpoint in the - // token. This is a bit of a hack, and we should figure out a better - // way of distributing it to components that need it. - let clusterURI = Services.io.newURI(this._token.endpoint, null, null); - clusterURI.path = "/"; - this.clusterURL = clusterURI.spec; - this._log.info("initWithLoggedUser has username " + this.username + ", endpoint is " + this.clusterURL); - }); + _fetchSyncKeyBundle: function() { + // Fetch a sync token for the logged in user from the token server. + return this._refreshTokenForLoggedInUser( + ).then(token => { + this._token = token; + 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."); + } + // Set the username to be the uid returned by the token server. + this.username = this._token.uid.toString(); + // both Jelly and FxAccounts give us kA/kB as hex. + let kB = Utils.hexToBytes(userData.kB); + this._syncKeyBundle = deriveKeyBundle(kB); + }); }, // Refresh the sync token for the currently logged in Firefox Accounts user. @@ -368,5 +429,45 @@ this.BrowserIDManager.prototype = { } request.setHeader("authorization", header.headers.authorization); return request; + }, + + createClusterManager: function(service) { + return new BrowserIDClusterManager(service); } + }; + +/* An implementation of the ClusterManager for this identity + */ + +function BrowserIDClusterManager(service) { + ClusterManager.call(this, service); +} + +BrowserIDClusterManager.prototype = { + __proto__: ClusterManager.prototype, + + _findCluster: function() { + let promiseClusterURL = function() { + return fxAccounts.getSignedInUser().then(userData => { + return this.identity._fetchTokenForUser(userData).then(token => { + // Set the clusterURI for this user based on the endpoint in the + // token. This is a bit of a hack, and we should figure out a better + // way of distributing it to components that need it. + let clusterURI = Services.io.newURI(token.endpoint, null, null); + clusterURI.path = "/"; + return clusterURI.spec; + }); + }); + }.bind(this); + + let cb = Async.makeSpinningCallback(); + promiseClusterURL().then(function (clusterURL) { + cb(null, clusterURL); + }, + function (err) { + cb(err); + }); + return cb.wait(); + }, +} diff --git a/services/sync/modules/constants.js b/services/sync/modules/constants.js index f237280f9bb0..0e19ad28b9c0 100644 --- a/services/sync/modules/constants.js +++ b/services/sync/modules/constants.js @@ -119,6 +119,7 @@ LOGIN_FAILED_NETWORK_ERROR: "error.login.reason.network", LOGIN_FAILED_SERVER_ERROR: "error.login.reason.server", LOGIN_FAILED_INVALID_PASSPHRASE: "error.login.reason.recoverykey", LOGIN_FAILED_LOGIN_REJECTED: "error.login.reason.account", +LOGIN_FAILED_NOT_READY: "error.login.reason.initializing", // sync failure status codes METARECORD_DOWNLOAD_FAIL: "error.sync.reason.metarecord_download_fail", diff --git a/services/sync/modules/identity.js b/services/sync/modules/identity.js index 94f2be6059bf..ee51d6ac432c 100644 --- a/services/sync/modules/identity.js +++ b/services/sync/modules/identity.js @@ -9,6 +9,7 @@ this.EXPORTED_SYMBOLS = ["IdentityManager"]; const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components; Cu.import("resource://gre/modules/XPCOMUtils.jsm"); +Cu.import("resource://gre/modules/Promise.jsm"); Cu.import("resource://services-sync/constants.js"); Cu.import("resource://gre/modules/Log.jsm"); Cu.import("resource://services-sync/util.js"); @@ -21,7 +22,8 @@ for (let symbol of ["BulkKeyBundle", "SyncKeyBundle"]) { } /** - * Manages identity and authentication for Sync. + * Manages "legacy" identity and authentication for Sync. + * See browserid_identity for the Firefox Accounts based identity manager. * * The following entities are managed: * @@ -81,6 +83,24 @@ IdentityManager.prototype = { _syncKeyBundle: null, + /** + * Initialize the identity provider. Returns a promise that is resolved + * when initialization is complete and the provider can be queried for + * its state + */ + initialize: function() { + // nothing to do for this identity provider + return Promise.resolve(); + }, + + /** + * Indicates if the identity manager is still initializing + */ + get readyToAuthenticate() { + // We initialize in a fully sync manner, so we are always finished. + return true; + }, + get account() { return Svc.Prefs.get("account", this.username); }, @@ -505,5 +525,10 @@ IdentityManager.prototype = { onRESTRequestBasic: function onRESTRequestBasic(request) { let up = this.username + ":" + this.basicPassword; request.setHeader("authorization", "Basic " + btoa(up)); + }, + + createClusterManager: function(service) { + Cu.import("resource://services-sync/stages/cluster.js"); + return new ClusterManager(service); } }; diff --git a/services/sync/modules/policies.js b/services/sync/modules/policies.js index cf2931ca93af..f3a8093f692d 100644 --- a/services/sync/modules/policies.js +++ b/services/sync/modules/policies.js @@ -12,9 +12,11 @@ const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components; Cu.import("resource://gre/modules/Log.jsm"); Cu.import("resource://services-sync/constants.js"); Cu.import("resource://services-sync/engines.js"); -Cu.import("resource://services-sync/status.js"); Cu.import("resource://services-sync/util.js"); +XPCOMUtils.defineLazyModuleGetter(this, "Status", + "resource://services-sync/status.js"); + this.SyncScheduler = function SyncScheduler(service) { this.service = service; this.init(); diff --git a/services/sync/modules/service.js b/services/sync/modules/service.js index db530dfe848c..c0903c9e0e8c 100644 --- a/services/sync/modules/service.js +++ b/services/sync/modules/service.js @@ -30,7 +30,6 @@ Cu.import("resource://services-sync/policies.js"); Cu.import("resource://services-sync/record.js"); Cu.import("resource://services-sync/resource.js"); Cu.import("resource://services-sync/rest.js"); -Cu.import("resource://services-sync/stages/cluster.js"); Cu.import("resource://services-sync/stages/enginesync.js"); Cu.import("resource://services-sync/status.js"); Cu.import("resource://services-sync/userapi.js"); @@ -323,7 +322,7 @@ Sync11Service.prototype = { this._log.info("Loading Weave " + WEAVE_VERSION); - this._clusterManager = new ClusterManager(this); + this._clusterManager = this.identity.createClusterManager(this); this.recordManager = new RecordManager(this); this.enabled = true; @@ -649,6 +648,13 @@ Sync11Service.prototype = { }, verifyLogin: function verifyLogin() { + // If the identity isn't ready it might not know the username... + if (!this.identity.readyToAuthenticate) { + this._log.info("Not ready to authenticate in verifyLogin."); + this.status.login = LOGIN_FAILED_NOT_READY; + return false; + } + if (!this.identity.username) { this._log.warn("No username in verifyLogin."); this.status.login = LOGIN_FAILED_NO_USERNAME; diff --git a/services/sync/modules/status.js b/services/sync/modules/status.js index 8c5e3f03ede1..2b290e6c4f7d 100644 --- a/services/sync/modules/status.js +++ b/services/sync/modules/status.js @@ -12,13 +12,31 @@ const Cu = Components.utils; Cu.import("resource://services-sync/constants.js"); Cu.import("resource://gre/modules/Log.jsm"); Cu.import("resource://services-sync/identity.js"); +Cu.import("resource://services-sync/browserid_identity.js"); Cu.import("resource://gre/modules/Services.jsm"); +Cu.import("resource://services-common/async.js"); this.Status = { _log: Log.repository.getLogger("Sync.Status"), - _authManager: new IdentityManager(), + __authManager: null, ready: false, + get _authManager() { + if (this.__authManager) { + return this.__authManager; + } + let service = Components.classes["@mozilla.org/weave/service;1"] + .getService(Components.interfaces.nsISupports) + .wrappedJSObject; + let idClass = service.fxAccountsEnabled ? BrowserIDManager : IdentityManager; + this.__authManager = new idClass(); + // .initialize returns a promise, so we need to spin until it resolves. + let cb = Async.makeSpinningCallback(); + this.__authManager.initialize().then(cb, cb); + cb.wait(); + return this.__authManager; + }, + get service() { return this._service; },