From 9717484083e66b78eedfa14e539d51382aba760f Mon Sep 17 00:00:00 2001 From: Mark Hammond Date: Sat, 14 Jun 2014 14:33:56 +1000 Subject: Bug 1013064 (part 4) - browserid_identity and sync changes to support FxA credentials in the login manager. r=ckarlof,rnewman --- services/sync/modules/browserid_identity.js | 61 ++++++++++++++++++++-- services/sync/modules/identity.js | 19 +++++++ services/sync/modules/service.js | 20 ++++--- .../sync/tests/unit/test_browserid_identity.js | 15 ++++++ 4 files changed, 102 insertions(+), 13 deletions(-) diff --git a/services/sync/modules/browserid_identity.js b/services/sync/modules/browserid_identity.js --- a/services/sync/modules/browserid_identity.js +++ b/services/sync/modules/browserid_identity.js @@ -394,41 +394,83 @@ this.BrowserIDManager.prototype = { this._syncKeyUpdated = true; this._shouldHaveSyncKeyBundle = false; }, /** * The current state of the auth credentials. * * This essentially validates that enough credentials are available to use - * Sync. + * Sync, although it effectively ignores the state of the master-password - + * if that's locked and that's the only problem we can see, say everything + * is OK - unlockAndVerifyAuthState will be used to perform the unlock + * and re-verification if necessary. */ get currentAuthState() { if (this._authFailureReason) { this._log.info("currentAuthState returning " + this._authFailureReason + " due to previous failure"); return this._authFailureReason; } // TODO: need to revisit this. Currently this isn't ready to go until // both the username and syncKeyBundle are both configured and having no // username seems to make things fail fast so that's good. if (!this.username) { return LOGIN_FAILED_NO_USERNAME; } // No need to check this.syncKey as our getter for that attribute // uses this.syncKeyBundle - // If bundle creation started, but failed. - if (this._shouldHaveSyncKeyBundle && !this.syncKeyBundle) { - return LOGIN_FAILED_NO_PASSPHRASE; + // If bundle creation started, but failed due to any reason other than + // the MP being locked... + if (this._shouldHaveSyncKeyBundle && !this.syncKeyBundle && !Utils.mpLocked()) { + // Return a state that says a re-auth is necessary so we can get keys. + return LOGIN_FAILED_LOGIN_REJECTED; } return STATUS_OK; }, + // Do we currently have keys, or do we have enough that we should be able + // to successfully fetch them? + _canFetchKeys: function() { + let userData = this._signedInUser; + // a keyFetchToken means we can almost certainly grab them. + // kA and kB means we already have them. + return userData && (userData.keyFetchToken || (userData.kA && userData.kB)); + }, + + /** + * Verify the current auth state, unlocking the master-password if necessary. + * + * Returns a promise that resolves with the current auth state after + * attempting to unlock. + */ + unlockAndVerifyAuthState: function() { + if (this._canFetchKeys()) { + return Promise.resolve(STATUS_OK); + } + // so no keys - ensure MP unlocked. + if (!Utils.ensureMPUnlocked()) { + // user declined to unlock, so we don't know if they are stored there. + return Promise.resolve(MASTER_PASSWORD_LOCKED); + } + // now we are unlocked we must re-fetch the user data as we may now have + // the details that were previously locked away. + return this._fxaService.getSignedInUser().then( + accountData => { + this._updateSignedInUser(accountData); + // If we still can't get keys it probably means the user authenticated + // without unlocking the MP or cleared the saved logins, so we've now + // lost them - the user will need to reauth before continuing. + return this._canFetchKeys() ? STATUS_OK : LOGIN_FAILED_LOGIN_REJECTED; + } + ); + }, + /** * Do we have a non-null, not yet expired token for the user currently * signed in? */ hasValidToken: function() { if (!this._token) { return false; } @@ -444,16 +486,24 @@ this.BrowserIDManager.prototype = { if (tokenServerURI.endsWith("/")) { // trailing slashes cause problems... tokenServerURI = tokenServerURI.slice(0, -1); } let log = this._log; let client = this._tokenServerClient; let fxa = this._fxaService; let userData = this._signedInUser; + // We need kA and kB for things to work. If we don't have them, just + // return null for the token - sync calling unlockAndVerifyAuthState() + // before actually syncing will setup the error states if necessary. + if (!this._canFetchKeys()) { + log.info("_fetchTokenForUser has no keys to use."); + return null; + } + log.info("Fetching assertion and token from: " + tokenServerURI); let maybeFetchKeys = () => { // This is called at login time and every time we need a new token - in // the latter case we already have kA and kB, so optimise that case. if (userData.kA && userData.kB) { return; } @@ -519,17 +569,18 @@ this.BrowserIDManager.prototype = { // 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); // set it to the "fatal" LOGIN_FAILED_LOGIN_REJECTED reason. this._authFailureReason = LOGIN_FAILED_LOGIN_REJECTED; } else { - this._log.error("Non-authentication error in _fetchTokenForUser: " + err.message); + this._log.error("Non-authentication error in _fetchTokenForUser: " + + (err.message || err)); // for now assume it is just a transient network related problem. this._authFailureReason = LOGIN_FAILED_NETWORK_ERROR; } // 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; Weave.Status.login = this._authFailureReason; diff --git a/services/sync/modules/identity.js b/services/sync/modules/identity.js --- a/services/sync/modules/identity.js +++ b/services/sync/modules/identity.js @@ -373,16 +373,35 @@ IdentityManager.prototype = { if (!this.syncKeyBundle) { return LOGIN_FAILED_INVALID_PASSPHRASE; } return STATUS_OK; }, /** + * Verify the current auth state, unlocking the master-password if necessary. + * + * Returns a promise that resolves with the current auth state after + * attempting to unlock. + */ + unlockAndVerifyAuthState: function() { + // Try to fetch the passphrase - this will prompt for MP unlock as a + // side-effect... + try { + this.syncKey; + } catch (ex) { + this._log.debug("Fetching passphrase threw " + ex + + "; assuming master password locked."); + return Promise.resolve(MASTER_PASSWORD_LOCKED); + } + return Promise.resolve(STATUS_OK); + }, + + /** * Persist credentials to password store. * * When credentials are updated, they are changed in memory only. This will * need to be called to save them to the underlying password store. * * If the password store is locked (e.g. if the master password hasn't been * entered), this could throw an exception. */ diff --git a/services/sync/modules/service.js b/services/sync/modules/service.js --- a/services/sync/modules/service.js +++ b/services/sync/modules/service.js @@ -679,27 +679,31 @@ Sync11Service.prototype = { } if (!this.identity.username) { this._log.warn("No username in verifyLogin."); this.status.login = LOGIN_FAILED_NO_USERNAME; return false; } - // Unlock master password, or return. // Attaching auth credentials to a request requires access to // passwords, which means that Resource.get can throw MP-related // exceptions! - // Try to fetch the passphrase first, while we still have control. - try { - this.identity.syncKey; - } catch (ex) { - this._log.debug("Fetching passphrase threw " + ex + - "; assuming master password locked."); - this.status.login = MASTER_PASSWORD_LOCKED; + // So we ask the identity to verify the login state after unlocking the + // master password (ie, this call is expected to prompt for MP unlock + // if necessary) while we still have control. + let cb = Async.makeSpinningCallback(); + this.identity.unlockAndVerifyAuthState().then( + result => cb(null, result), + cb + ); + let unlockedState = cb.wait(); + this._log.debug("Fetching unlocked auth state returned " + unlockedState); + if (unlockedState != STATUS_OK) { + this.status.login = unlockedState; return false; } try { // Make sure we have a cluster to verify against. // This is a little weird, if we don't get a node we pretend // to succeed, since that probably means we just don't have storage. if (this.clusterURL == "" && !this._clusterManager.setCluster()) { diff --git a/services/sync/tests/unit/test_browserid_identity.js b/services/sync/tests/unit/test_browserid_identity.js --- a/services/sync/tests/unit/test_browserid_identity.js +++ b/services/sync/tests/unit/test_browserid_identity.js @@ -78,19 +78,33 @@ add_task(function test_initialializeWith browseridManager.initializeWithCurrentIdentity(); yield browseridManager.whenReadyToAuthenticate.promise; do_check_true(!!browseridManager._token); do_check_true(browseridManager.hasValidToken()); do_check_eq(browseridManager.account, identityConfig.fxaccount.user.email); } ); +add_task(function test_initialializeWithNoKeys() { + _("Verify start after initializeWithCurrentIdentity without kA, kB or keyFetchToken"); + let identityConfig = makeIdentityConfig(); + delete identityConfig.fxaccount.user.kA; + delete identityConfig.fxaccount.user.kB; + // there's no keyFetchToken by default, so the initialize should fail. + configureFxAccountIdentity(browseridManager, identityConfig); + + yield browseridManager.initializeWithCurrentIdentity(); + yield browseridManager.whenReadyToAuthenticate.promise; + do_check_eq(Status.login, LOGIN_SUCCEEDED, "login succeeded even without keys"); + do_check_false(browseridManager._canFetchKeys(), "_canFetchKeys reflects lack of keys"); +}); add_test(function test_getResourceAuthenticator() { _("BrowserIDManager supplies a Resource Authenticator callback which returns a Hawk header."); + configureFxAccountIdentity(browseridManager); let authenticator = browseridManager.getResourceAuthenticator(); do_check_true(!!authenticator); let req = {uri: CommonUtils.makeURI( "https://example.net/somewhere/over/the/rainbow"), method: 'GET'}; let output = authenticator(req, 'GET'); do_check_true('headers' in output); do_check_true('authorization' in output.headers); @@ -235,16 +249,17 @@ add_test(function test_RESTResourceAuthe (getTimestampDelta(authHeader, now) - 12 * HOUR_MS) < 2 * MINUTE_MS); run_next_test(); }); add_task(function test_ensureLoggedIn() { configureFxAccountIdentity(browseridManager); yield browseridManager.initializeWithCurrentIdentity(); + yield browseridManager.whenReadyToAuthenticate.promise; Assert.equal(Status.login, LOGIN_SUCCEEDED, "original initialize worked"); yield browseridManager.ensureLoggedIn(); Assert.equal(Status.login, LOGIN_SUCCEEDED, "original ensureLoggedIn worked"); Assert.ok(browseridManager._shouldHaveSyncKeyBundle, "_shouldHaveSyncKeyBundle should always be true after ensureLogin completes."); // arrange for no logged in user. let fxa = browseridManager._fxaService