Merging upstream version 125.0.1.

Signed-off-by: Daniel Baumann <daniel.baumann@progress-linux.org>

15 files changed, 115 insertions, 62 deletions

diff --git a/services/fxaccounts/FxAccounts.sys.mjs b/services/fxaccounts/FxAccounts.sys.mjs
index 18169c6b2d..790a6195f8 100644
--- a/services/fxaccounts/FxAccounts.sys.mjs
+++ b/services/fxaccounts/FxAccounts.sys.mjs
@@ -65,6 +65,8 @@ XPCOMUtils.defineLazyPreferenceGetter(
   true
 );
 
+export const ERROR_INVALID_ACCOUNT_STATE = "ERROR_INVALID_ACCOUNT_STATE";
+
 // An AccountState object holds all state related to one specific account.
 // It is considered "private" to the FxAccounts modules.
 // Only one AccountState is ever "current" in the FxAccountsInternal object -
@@ -170,7 +172,7 @@ AccountState.prototype = {
       delete updatedFields.uid;
     }
     if (!this.isCurrent) {
-      return Promise.reject(new Error("Another user has signed in"));
+      return Promise.reject(new Error(ERROR_INVALID_ACCOUNT_STATE));
     }
     return this.storageManager.updateAccountData(updatedFields);
   },
@@ -179,11 +181,11 @@ AccountState.prototype = {
     if (!this.isCurrent) {
       log.info(
         "An accountState promise was resolved, but was actually rejected" +
-          " due to a different user being signed in. Originally resolved" +
-          " with",
+          " due to the account state changing. This can happen if a new account signed in, or" +
+          " the account was signed out. Originally resolved with, ",
         result
       );
-      return Promise.reject(new Error("A different user signed in"));
+      return Promise.reject(new Error(ERROR_INVALID_ACCOUNT_STATE));
     }
     return Promise.resolve(result);
   },
@@ -195,12 +197,13 @@ AccountState.prototype = {
     // problems.
     if (!this.isCurrent) {
       log.info(
-        "An accountState promise was rejected, but we are ignoring that " +
-          "reason and rejecting it due to a different user being signed in. " +
-          "Originally rejected with",
+        "An accountState promise was rejected, but we are ignoring that" +
+          " reason and rejecting it due to the account state changing. This can happen if" +
+          " a different account signed in or the account was signed out" +
+          " originally resolved with, ",
         error
       );
-      return Promise.reject(new Error("A different user signed in"));
+      return Promise.reject(new Error(ERROR_INVALID_ACCOUNT_STATE));
     }
     return Promise.reject(error);
   },
@@ -215,7 +218,7 @@ AccountState.prototype = {
   // A preamble for the cache helpers...
   _cachePreamble() {
     if (!this.isCurrent) {
-      throw new Error("Another user has signed in");
+      throw new Error(ERROR_INVALID_ACCOUNT_STATE);
     }
   },
 
@@ -466,6 +469,37 @@ export class FxAccounts {
     }
   }
 
+  /** Gets both the OAuth token and the users scoped keys for that token
+   * and verifies that both operations were done for the same user,
+   * preventing race conditions where a caller
+   * can get the key for one user, and the id of another if the user
+   * is rapidly switching between accounts
+   *
+   * @param options
+   *        {
+   *          scope: string the oauth scope being requested. This must
+   *          be a scope with an associated key, otherwise an error
+   *          will be thrown that the key is not available.
+   *          ttl: (number) OAuth token TTL in seconds
+   *        }
+   *
+   * @return Promise.<Object | Error>
+   * The promise resolve to both the access token being requested, and the scoped key
+   *        {
+   *         token: (string) access token
+   *         key: (object) the scoped key object
+   *        }
+   * The promise can reject, with one of the errors `getOAuthToken`, `FxAccountKeys.getKeyForScope`, or
+   * error if the user changed in-between operations
+   */
+  getOAuthTokenAndKey(options = {}) {
+    return this._withCurrentAccountState(async () => {
+      const key = await this.keys.getKeyForScope(options.scope);
+      const token = await this.getOAuthToken(options);
+      return { token, key };
+    });
+  }
+
   /**
    * Remove an OAuth token from the token cache. Callers should call this
    * after they determine a token is invalid, so a new token will be fetched
diff --git a/services/fxaccounts/FxAccountsClient.sys.mjs b/services/fxaccounts/FxAccountsClient.sys.mjs
index 9dc80ff419..a62da40fd8 100644
--- a/services/fxaccounts/FxAccountsClient.sys.mjs
+++ b/services/fxaccounts/FxAccountsClient.sys.mjs
@@ -59,7 +59,7 @@ FxAccountsClient.prototype = {
   /*
    * Return current time in milliseconds
    *
-   * Not used by this module, but made available to the FxAccounts.jsm
+   * Not used by this module, but made available to the FxAccounts.sys.mjs
    * that uses this client.
    */
   now() {
@@ -498,7 +498,7 @@ FxAccountsClient.prototype = {
    */
   accountExists(email) {
     return this.signIn(email, "").then(
-      cantHappen => {
+      () => {
         throw new Error("How did I sign in with an empty password?");
       },
       expectedError => {
diff --git a/services/fxaccounts/FxAccountsProfile.sys.mjs b/services/fxaccounts/FxAccountsProfile.sys.mjs
index de8bdb2f0e..8022a6d8a8 100644
--- a/services/fxaccounts/FxAccountsProfile.sys.mjs
+++ b/services/fxaccounts/FxAccountsProfile.sys.mjs
@@ -52,7 +52,7 @@ FxAccountsProfile.prototype = {
   // making another request to determine if it is fresh or not.
   PROFILE_FRESHNESS_THRESHOLD: 120000, // 2 minutes
 
-  observe(subject, topic, data) {
+  observe(subject, topic) {
     // If we get a profile change notification from our webchannel it means
     // the user has just changed their profile via the web, so we want to
     // ignore our "freshness threshold"
diff --git a/services/fxaccounts/FxAccountsWebChannel.sys.mjs b/services/fxaccounts/FxAccountsWebChannel.sys.mjs
index fdd0b75e93..f8d7a3362d 100644
--- a/services/fxaccounts/FxAccountsWebChannel.sys.mjs
+++ b/services/fxaccounts/FxAccountsWebChannel.sys.mjs
@@ -431,7 +431,7 @@ FxAccountsWebChannelHelpers.prototype = {
     // A sync-specific hack - we want to ensure sync has been initialized
     // before we set the signed-in user.
     // XXX - probably not true any more, especially now we have observerPreloads
-    // in FxAccounts.jsm?
+    // in FxAccounts.sys.mjs?
     let xps =
       this._weaveXPCOM ||
       Cc["@mozilla.org/weave/service;1"].getService(Ci.nsISupports)
diff --git a/services/fxaccounts/tests/mochitest/test_invalidEmailCase.html b/services/fxaccounts/tests/mochitest/test_invalidEmailCase.html
index 4b5e943591..24353afbc1 100644
--- a/services/fxaccounts/tests/mochitest/test_invalidEmailCase.html
+++ b/services/fxaccounts/tests/mochitest/test_invalidEmailCase.html
@@ -49,7 +49,7 @@ MockStorage.prototype = Object.freeze({
   getOAuthTokens() {
     return Promise.resolve(null);
   },
-  setOAuthTokens(contents) {
+  setOAuthTokens() {
     return Promise.resolve();
   },
 });
diff --git a/services/fxaccounts/tests/xpcshell/test_accounts.js b/services/fxaccounts/tests/xpcshell/test_accounts.js
index c4aec73a03..239adb206f 100644
--- a/services/fxaccounts/tests/xpcshell/test_accounts.js
+++ b/services/fxaccounts/tests/xpcshell/test_accounts.js
@@ -6,7 +6,7 @@
 const { CryptoUtils } = ChromeUtils.importESModule(
   "resource://services-crypto/utils.sys.mjs"
 );
-const { FxAccounts } = ChromeUtils.importESModule(
+const { FxAccounts, ERROR_INVALID_ACCOUNT_STATE } = ChromeUtils.importESModule(
   "resource://gre/modules/FxAccounts.sys.mjs"
 );
 const { FxAccountsClient } = ChromeUtils.importESModule(
@@ -120,7 +120,7 @@ function MockFxAccountsClient() {
 
   // mock calls up to the auth server to determine whether the
   // user account has been verified
-  this.recoveryEmailStatus = async function (sessionToken) {
+  this.recoveryEmailStatus = async function () {
     // simulate a call to /recovery_email/status
     return {
       email: this._email,
@@ -139,7 +139,7 @@ function MockFxAccountsClient() {
     return !this._deletedOnServer;
   };
 
-  this.accountKeys = function (keyFetchToken) {
+  this.accountKeys = function () {
     return new Promise(resolve => {
       do_timeout(50, () => {
         resolve({
@@ -188,7 +188,7 @@ Object.setPrototypeOf(
  * mock the now() method, so that we can simulate the passing of
  * time and verify that signatures expire correctly.
  */
-function MockFxAccounts(credentials = null) {
+function MockFxAccounts() {
   let result = new FxAccounts({
     VERIFICATION_POLL_TIMEOUT_INITIAL: 100, // 100ms
 
@@ -453,10 +453,10 @@ add_test(function test_polling_timeout() {
 
   fxa.setSignedInUser(test_user).then(() => {
     p.then(
-      success => {
+      () => {
         do_throw("this should not succeed");
       },
-      fail => {
+      () => {
         removeObserver();
         fxa.signOut().then(run_next_test);
       }
@@ -471,7 +471,7 @@ add_task(async function test_onverified_once() {
 
   let numNotifications = 0;
 
-  function observe(aSubject, aTopic, aData) {
+  function observe() {
     numNotifications += 1;
   }
   Services.obs.addObserver(observe, ONVERIFIED_NOTIFICATION);
@@ -777,11 +777,10 @@ add_task(async function test_getKeyForScope_nonexistent_account() {
     });
   });
 
-  // XXX - the exception message here isn't ideal, but doesn't really matter...
-  await Assert.rejects(
-    fxa.keys.getKeyForScope(SCOPE_OLD_SYNC),
-    /A different user signed in/
-  );
+  await Assert.rejects(fxa.keys.getKeyForScope(SCOPE_OLD_SYNC), err => {
+    Assert.equal(err.message, ERROR_INVALID_ACCOUNT_STATE);
+    return true; // expected error
+  });
 
   await promiseLogout;
 
@@ -972,17 +971,17 @@ add_test(function test_fetchAndUnwrapAndDeriveKeys_no_token() {
 
   makeObserver(ONLOGOUT_NOTIFICATION, function () {
     log.debug("test_fetchAndUnwrapKeys_no_token observed logout");
-    fxa._internal.getUserAccountData().then(user2 => {
+    fxa._internal.getUserAccountData().then(() => {
       fxa._internal.abortExistingFlow().then(run_next_test);
     });
   });
 
   fxa
     .setSignedInUser(user)
-    .then(user2 => {
+    .then(() => {
       return fxa.keys._fetchAndUnwrapAndDeriveKeys();
     })
-    .catch(error => {
+    .catch(() => {
       log.info("setSignedInUser correctly rejected");
     });
 });
@@ -1273,11 +1272,7 @@ add_task(async function test_getOAuthTokenCachedScopeNormalization() {
   let numOAuthTokenCalls = 0;
 
   let client = fxa._internal.fxAccountsClient;
-  client.accessTokenWithSessionToken = async (
-    _sessionTokenHex,
-    _clientId,
-    scopeString
-  ) => {
+  client.accessTokenWithSessionToken = async (_sessionTokenHex, _clientId) => {
     numOAuthTokenCalls++;
     return MOCK_TOKEN_RESPONSE;
   };
@@ -1405,6 +1400,31 @@ add_test(function test_getOAuthToken_error() {
   });
 });
 
+add_test(async function test_getOAuthTokenAndKey_errors_if_user_change() {
+  const fxa = new MockFxAccounts();
+  const alice = getTestUser("alice");
+  const bob = getTestUser("bob");
+  alice.verified = true;
+  bob.verified = true;
+
+  fxa.getOAuthToken = async () => {
+    // We mock what would happen if the user got changed
+    // after we got the access token
+    await fxa.setSignedInUser(bob);
+    return "access token";
+  };
+  fxa.keys.getKeyForScope = () => Promise.resolve("key!");
+  await fxa.setSignedInUser(alice);
+  await Assert.rejects(
+    fxa.getOAuthTokenAndKey({ scope: "foo", ttl: 10 }),
+    err => {
+      Assert.equal(err.message, ERROR_INVALID_ACCOUNT_STATE);
+      return true; // expected error
+    }
+  );
+  run_next_test();
+});
+
 add_task(async function test_listAttachedOAuthClients() {
   const ONE_HOUR = 60 * 60 * 1000;
   const ONE_DAY = 24 * ONE_HOUR;
diff --git a/services/fxaccounts/tests/xpcshell/test_accounts_device_registration.js b/services/fxaccounts/tests/xpcshell/test_accounts_device_registration.js
index 68337eb69e..4b6ac58879 100644
--- a/services/fxaccounts/tests/xpcshell/test_accounts_device_registration.js
+++ b/services/fxaccounts/tests/xpcshell/test_accounts_device_registration.js
@@ -80,7 +80,7 @@ function MockFxAccountsClient(device) {
 
   // mock calls up to the auth server to determine whether the
   // user account has been verified
-  this.recoveryEmailStatus = function (sessionToken) {
+  this.recoveryEmailStatus = function () {
     // simulate a call to /recovery_email/status
     return Promise.resolve({
       email: this._email,
@@ -104,8 +104,7 @@ function MockFxAccountsClient(device) {
     return Promise.resolve(!!uid && !this._deletedOnServer);
   };
 
-  this.registerDevice = (st, name, type) =>
-    Promise.resolve({ id: device.id, name });
+  this.registerDevice = (st, name) => Promise.resolve({ id: device.id, name });
   this.updateDevice = (st, id, name) => Promise.resolve({ id, name });
   this.signOut = () => Promise.resolve({});
   this.getDeviceList = st =>
@@ -655,7 +654,7 @@ add_task(async function test_verification_updates_registration() {
     };
   });
 
-  fxa._internal.checkEmailStatus = async function (sessionToken) {
+  fxa._internal.checkEmailStatus = async function () {
     credentials.verified = true;
     return credentials;
   };
@@ -792,7 +791,7 @@ add_task(async function test_refreshDeviceList() {
   };
   const deviceListUpdateObserver = {
     count: 0,
-    observe(subject, topic, data) {
+    observe() {
       this.count++;
     },
   };
diff --git a/services/fxaccounts/tests/xpcshell/test_commands.js b/services/fxaccounts/tests/xpcshell/test_commands.js
index 3fa42da439..c72f76193a 100644
--- a/services/fxaccounts/tests/xpcshell/test_commands.js
+++ b/services/fxaccounts/tests/xpcshell/test_commands.js
@@ -174,7 +174,7 @@ add_task(async function test_sendtab_receive() {
 
   const fxai = FxaInternalMock();
   const sendTab = new SendTab(commands, fxai);
-  sendTab._encrypt = (bytes, device) => {
+  sendTab._encrypt = bytes => {
     return bytes;
   };
   sendTab._decrypt = bytes => {
@@ -387,7 +387,7 @@ add_task(async function test_commands_handleCommands() {
     },
   };
   const commands = new FxAccountsCommands(fxAccounts);
-  commands.sendTab.handle = (sender, data, reason) => {
+  commands.sendTab.handle = () => {
     return {
       title: "testTitle",
       uri: "https://testURI",
@@ -436,7 +436,7 @@ add_task(async function test_commands_handleCommands_invalid_tab() {
     },
   };
   const commands = new FxAccountsCommands(fxAccounts);
-  commands.sendTab.handle = (sender, data, reason) => {
+  commands.sendTab.handle = () => {
     return {
       title: "badUriTab",
       uri: "file://path/to/pdf",
diff --git a/services/fxaccounts/tests/xpcshell/test_oauth_token_storage.js b/services/fxaccounts/tests/xpcshell/test_oauth_token_storage.js
index 798c439212..8575f7065c 100644
--- a/services/fxaccounts/tests/xpcshell/test_oauth_token_storage.js
+++ b/services/fxaccounts/tests/xpcshell/test_oauth_token_storage.js
@@ -94,7 +94,7 @@ Object.setPrototypeOf(
   FxAccountsClient.prototype
 );
 
-function MockFxAccounts(device = {}) {
+function MockFxAccounts() {
   return new FxAccounts({
     fxAccountsClient: new MockFxAccountsClient(),
     newAccountState(credentials) {
diff --git a/services/fxaccounts/tests/xpcshell/test_pairing.js b/services/fxaccounts/tests/xpcshell/test_pairing.js
index eac3112242..245db66f66 100644
--- a/services/fxaccounts/tests/xpcshell/test_pairing.js
+++ b/services/fxaccounts/tests/xpcshell/test_pairing.js
@@ -61,7 +61,7 @@ const fxAccounts = {
   },
   _internal: {
     keys: {
-      getKeyForScope(scope) {
+      getKeyForScope() {
         return {
           kid: "123456",
           k: KSYNC,
diff --git a/services/fxaccounts/tests/xpcshell/test_profile.js b/services/fxaccounts/tests/xpcshell/test_profile.js
index f8137b5691..39acf653d4 100644
--- a/services/fxaccounts/tests/xpcshell/test_profile.js
+++ b/services/fxaccounts/tests/xpcshell/test_profile.js
@@ -142,10 +142,10 @@ add_test(function fetchAndCacheProfile_always_bumps_cachedAt() {
   profile._cachedAt = 12345;
 
   return profile._fetchAndCacheProfile().then(
-    result => {
+    () => {
       do_throw("Should not succeed");
     },
-    err => {
+    () => {
       Assert.notEqual(profile._cachedAt, 12345, "cachedAt has been bumped");
       run_next_test();
     }
@@ -164,7 +164,7 @@ add_test(function fetchAndCacheProfile_sendsETag() {
   };
   let profile = CreateFxAccountsProfile(fxa, client);
 
-  return profile._fetchAndCacheProfile().then(result => {
+  return profile._fetchAndCacheProfile().then(() => {
     run_next_test();
   });
 });
@@ -282,7 +282,7 @@ add_test(function fetchAndCacheProfile_alreadyCached() {
   };
 
   let profile = CreateFxAccountsProfile(fxa, client);
-  profile._cacheProfile = function (toCache) {
+  profile._cacheProfile = function () {
     do_throw("This method should not be called.");
   };
 
@@ -614,7 +614,7 @@ add_test(function getProfile_has_cached_fetch_deleted() {
 
   // instead of checking this in a mocked "save" function, just check after the
   // observer
-  makeObserver(ON_PROFILE_CHANGE_NOTIFICATION, function (subject, topic, data) {
+  makeObserver(ON_PROFILE_CHANGE_NOTIFICATION, function () {
     profile.getProfile().then(profileData => {
       Assert.equal(null, profileData.avatar);
       run_next_test();
diff --git a/services/fxaccounts/tests/xpcshell/test_profile_client.js b/services/fxaccounts/tests/xpcshell/test_profile_client.js
index 22fcc293f8..5b6e4ffdff 100644
--- a/services/fxaccounts/tests/xpcshell/test_profile_client.js
+++ b/services/fxaccounts/tests/xpcshell/test_profile_client.js
@@ -39,7 +39,7 @@ let mockResponse = function (response) {
           Request.ifNoneMatchSet = true;
         }
       },
-      async dispatch(method, payload) {
+      async dispatch() {
         this.response = response;
         return this.response;
       },
@@ -74,7 +74,7 @@ let mockResponseError = function (error) {
   return function () {
     return {
       setHeader() {},
-      async dispatch(method, payload) {
+      async dispatch() {
         throw error;
       },
     };
@@ -221,7 +221,7 @@ add_test(function server401ResponseThenSuccess() {
   let numRequests = 0;
   let numAuthHeaders = 0;
   // Like mockResponse but we want access to headers etc.
-  client._Request = function (requestUri) {
+  client._Request = function () {
     return {
       setHeader(name, value) {
         if (name == "Authorization") {
@@ -229,7 +229,7 @@ add_test(function server401ResponseThenSuccess() {
           Assert.equal(value, "Bearer " + lastToken);
         }
       },
-      async dispatch(method, payload) {
+      async dispatch() {
         this.response = responses[numRequests];
         ++numRequests;
         return this.response;
@@ -283,7 +283,7 @@ add_test(function server401ResponsePersists() {
 
   let numRequests = 0;
   let numAuthHeaders = 0;
-  client._Request = function (requestUri) {
+  client._Request = function () {
     return {
       setHeader(name, value) {
         if (name == "Authorization") {
@@ -291,7 +291,7 @@ add_test(function server401ResponsePersists() {
           Assert.equal(value, "Bearer " + lastToken);
         }
       },
-      async dispatch(method, payload) {
+      async dispatch() {
         this.response = response;
         ++numRequests;
         return this.response;
diff --git a/services/fxaccounts/tests/xpcshell/test_push_service.js b/services/fxaccounts/tests/xpcshell/test_push_service.js
index 0441888847..216f5d8cc8 100644
--- a/services/fxaccounts/tests/xpcshell/test_push_service.js
+++ b/services/fxaccounts/tests/xpcshell/test_push_service.js
@@ -179,7 +179,7 @@ add_test(function observePushTopicDeviceConnected() {
       return this;
     },
   };
-  let obs = (subject, topic, data) => {
+  let obs = (subject, topic) => {
     Services.obs.removeObserver(obs, topic);
     run_next_test();
   };
@@ -392,7 +392,7 @@ add_test(function observePushTopicProfileUpdated() {
       return this;
     },
   };
-  let obs = (subject, topic, data) => {
+  let obs = (subject, topic) => {
     Services.obs.removeObserver(obs, topic);
     run_next_test();
   };
diff --git a/services/fxaccounts/tests/xpcshell/test_storage_manager.js b/services/fxaccounts/tests/xpcshell/test_storage_manager.js
index 05c565d2f4..df29ca881d 100644
--- a/services/fxaccounts/tests/xpcshell/test_storage_manager.js
+++ b/services/fxaccounts/tests/xpcshell/test_storage_manager.js
@@ -62,7 +62,7 @@ MockedSecureStorage.prototype = {
   // "TypeError: this.STORAGE_LOCKED is not a constructor"
   STORAGE_LOCKED: function () {},
   /* eslint-enable object-shorthand */
-  async get(uid, email) {
+  async get() {
     this.fetchCount++;
     if (this.locked) {
       throw new this.STORAGE_LOCKED();
diff --git a/services/fxaccounts/tests/xpcshell/test_web_channel.js b/services/fxaccounts/tests/xpcshell/test_web_channel.js
index 48f043d0b9..020ac4d905 100644
--- a/services/fxaccounts/tests/xpcshell/test_web_channel.js
+++ b/services/fxaccounts/tests/xpcshell/test_web_channel.js
@@ -202,7 +202,7 @@ add_test(function test_error_message_remove_profile_path() {
   const toTest = Object.keys(errors).length;
   for (const key in errors) {
     let error = errors[key];
-    channel._channel.send = (message, context) => {
+    channel._channel.send = message => {
       equal(
         message.data.error.message,
         error.expected,
@@ -403,7 +403,7 @@ add_test(function test_fxa_status_message() {
   });
 
   channel._channel = {
-    send(response, sendingContext) {
+    send(response) {
       Assert.equal(response.command, "fxaccounts:fxa_status");
       Assert.equal(response.messageId, 123);
 
@@ -513,7 +513,7 @@ add_task(async function test_helpers_login_set_previous_account_name_hash() {
   let helpers = new FxAccountsWebChannelHelpers({
     fxAccounts: {
       _internal: {
-        setSignedInUser(accountData) {
+        setSignedInUser() {
           return new Promise(resolve => {
             // previously signed in user preference is updated.
             Assert.equal(
@@ -554,7 +554,7 @@ add_task(
     let helpers = new FxAccountsWebChannelHelpers({
       fxAccounts: {
         _internal: {
-          setSignedInUser(accountData) {
+          setSignedInUser() {
             return new Promise(resolve => {
               // previously signed in user preference should not be updated.
               Assert.equal(