From 5f934337c0d2a3a968d990010262c8b9780cf3d4 Mon Sep 17 00:00:00 2001 From: Mark Hammond Date: Wed, 23 May 2018 17:46:38 +1000 Subject: [PATCH] Bug 1463624 - ensure sync knows there is a valid user on signin. r=eoger MozReview-Commit-ID: 8hGU8eLYZKx --HG-- extra : rebase_source : 227aa891910a0a3d30c910cafc9a0ce7db1368d7 --- services/sync/modules/browserid_identity.js | 14 ++++------ .../tests/unit/test_browserid_identity.js | 28 +++++++++++++++++++ 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/services/sync/modules/browserid_identity.js b/services/sync/modules/browserid_identity.js index 3f3b50769253..79a8c5c7a3e4 100644 --- a/services/sync/modules/browserid_identity.js +++ b/services/sync/modules/browserid_identity.js @@ -224,11 +224,6 @@ this.BrowserIDManager.prototype = { switch (topic) { case fxAccountsCommon.ONLOGIN_NOTIFICATION: { this._log.info("A user has logged in"); - // If our existing Sync state is that we needed to reauth, clear that - // state now - it will get reset back if a problem persists. - if (Weave.Status.login == LOGIN_FAILED_LOGIN_REJECTED) { - Weave.Status.login = LOGIN_SUCCEEDED; - } this.resetCredentials(); let accountData = await this._fxaService.getSignedInUser(); this._updateSignedInUser(accountData); @@ -238,6 +233,7 @@ this.BrowserIDManager.prototype = { this._log.info("The user is not verified"); break; } + // intentional fall-through - the user is verified. } // We've been configured with an already verified user, so fall-through. case fxAccountsCommon.ONVERIFIED_NOTIFICATION: { @@ -246,16 +242,18 @@ this.BrowserIDManager.prototype = { // Set the username now - that will cause Sync to know it is configured let accountData = await this._fxaService.getSignedInUser(); this.username = accountData.email; + Weave.Status.login = LOGIN_SUCCEEDED; // And actually sync. If we've never synced before, we force a full sync. // If we have, then we are probably just reauthenticating so it's a normal sync. - // We can use any pref that must be set if we've synced before. - let isFirstSync = !Svc.Prefs.get("client.syncID", null); + // We can use any pref that must be set if we've synced before, and check + // the sync lock state because we might already be doing that first sync. + let isFirstSync = !Weave.Service.locked && !Svc.Prefs.get("client.syncID", null); if (isFirstSync) { this._log.info("Doing initial sync actions"); Svc.Prefs.set("firstSync", "resetClient"); + Services.obs.notifyObservers(null, "weave:service:setup-complete"); } - Services.obs.notifyObservers(null, "weave:service:setup-complete"); // There's no need to wait for sync to complete and it would deadlock // our AsyncObserver. Weave.Service.sync({why: "login"}); diff --git a/services/sync/tests/unit/test_browserid_identity.js b/services/sync/tests/unit/test_browserid_identity.js index 27bffb040a39..5dcfd1aeeeba 100644 --- a/services/sync/tests/unit/test_browserid_identity.js +++ b/services/sync/tests/unit/test_browserid_identity.js @@ -265,6 +265,34 @@ add_task(async function test_ensureLoggedIn() { Assert.equal(Status.login, LOGIN_SUCCEEDED, "final ensureLoggedIn worked"); }); +add_task(async function test_syncState() { + // Avoid polling for an unverified user. + let identityConfig = makeIdentityConfig(); + let fxaInternal = makeFxAccountsInternalMock(identityConfig); + fxaInternal.startVerifiedCheck = () => {}; + configureFxAccountIdentity(globalBrowseridManager, globalIdentityConfig, fxaInternal); + + // arrange for no logged in user. + let fxa = globalBrowseridManager._fxaService; + let signedInUser = fxa.internal.currentAccountState.storageManager.accountData; + fxa.internal.currentAccountState.storageManager.accountData = null; + await Assert.rejects(globalBrowseridManager._ensureValidToken(true), "expecting rejection due to no user"); + // Restore to an unverified user. + signedInUser.verified = false; + fxa.internal.currentAccountState.storageManager.accountData = signedInUser; + Status.login = LOGIN_FAILED_LOGIN_REJECTED; + // The browserid_identity observers are async, so call them directly. + await globalBrowseridManager.observe(null, ONLOGIN_NOTIFICATION, ""); + Assert.equal(Status.login, LOGIN_FAILED_LOGIN_REJECTED, + "should not have changed the login state for an unverified user"); + + // now pretend the user because verified. + signedInUser.verified = true; + await globalBrowseridManager.observe(null, ONVERIFIED_NOTIFICATION, ""); + Assert.equal(Status.login, LOGIN_SUCCEEDED, + "should have changed the login state to success"); +}); + add_task(async function test_tokenExpiration() { _("BrowserIDManager notices token expiration:"); let bimExp = new BrowserIDManager();