summaryrefslogtreecommitdiffstats
path: root/services/fxaccounts
diff options
context:
space:
mode:
Diffstat (limited to 'services/fxaccounts')
-rw-r--r--services/fxaccounts/FxAccounts.sys.mjs52
-rw-r--r--services/fxaccounts/FxAccountsClient.sys.mjs4
-rw-r--r--services/fxaccounts/FxAccountsProfile.sys.mjs2
-rw-r--r--services/fxaccounts/FxAccountsWebChannel.sys.mjs2
-rw-r--r--services/fxaccounts/tests/mochitest/test_invalidEmailCase.html2
-rw-r--r--services/fxaccounts/tests/xpcshell/test_accounts.js60
-rw-r--r--services/fxaccounts/tests/xpcshell/test_accounts_device_registration.js9
-rw-r--r--services/fxaccounts/tests/xpcshell/test_commands.js6
-rw-r--r--services/fxaccounts/tests/xpcshell/test_oauth_token_storage.js2
-rw-r--r--services/fxaccounts/tests/xpcshell/test_pairing.js2
-rw-r--r--services/fxaccounts/tests/xpcshell/test_profile.js10
-rw-r--r--services/fxaccounts/tests/xpcshell/test_profile_client.js12
-rw-r--r--services/fxaccounts/tests/xpcshell/test_push_service.js4
-rw-r--r--services/fxaccounts/tests/xpcshell/test_storage_manager.js2
-rw-r--r--services/fxaccounts/tests/xpcshell/test_web_channel.js8
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(