Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-26 22:32:46 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Backed out changeset 623e19026939 (bug 1855264) for causing crashes on test_sync_auth_manager.js. CLOSED TREE

Browse Source
This commit is contained in:
Natalia Csoregi 2024-02-20 19:08:00 +02:00
parent 767d90e2a8
commit c26f7461fc
4 changed files with 32 additions and 97 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
services/fxaccounts/FxAccounts.sys.mjs
View File

@ -65,8 +65,6 @@ 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 -
@ -172,7 +170,7 @@ AccountState.prototype = {
delete updatedFields.uid;
}
if (!this.isCurrent) {
return Promise.reject(new Error(ERROR_INVALID_ACCOUNT_STATE));
return Promise.reject(new Error("Another user has signed in"));
}
return this.storageManager.updateAccountData(updatedFields);
},
@ -181,11 +179,11 @@ AccountState.prototype = {
if (!this.isCurrent) {
log.info(
"An accountState promise was resolved, but was actually rejected" +
" due to the account state changing. This can happen if a new account signed in, or" +
" the account was signed out. Originally resolved with, ",
" due to a different user being signed in. Originally resolved" +
" with",
result
);
return Promise.reject(new Error(ERROR_INVALID_ACCOUNT_STATE));
return Promise.reject(new Error("A different user signed in"));
}
return Promise.resolve(result);
},
@ -197,13 +195,12 @@ AccountState.prototype = {
// problems.
if (!this.isCurrent) {
log.info(
"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, ",
"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",
error
);
return Promise.reject(new Error(ERROR_INVALID_ACCOUNT_STATE));
return Promise.reject(new Error("A different user signed in"));
}
return Promise.reject(error);
},
@ -218,7 +215,7 @@ AccountState.prototype = {
// A preamble for the cache helpers...
_cachePreamble() {
if (!this.isCurrent) {
throw new Error(ERROR_INVALID_ACCOUNT_STATE);
throw new Error("Another user has signed in");
}
},
@ -469,37 +466,6 @@ 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

36
services/fxaccounts/tests/xpcshell/test_accounts.js
View File

@ -6,7 +6,7 @@
const { CryptoUtils } = ChromeUtils.importESModule(
"resource://services-crypto/utils.sys.mjs"
);
const { FxAccounts, ERROR_INVALID_ACCOUNT_STATE } = ChromeUtils.importESModule(
const { FxAccounts } = ChromeUtils.importESModule(
"resource://gre/modules/FxAccounts.sys.mjs"
);
const { FxAccountsClient } = ChromeUtils.importESModule(
@ -777,10 +777,11 @@ add_task(async function test_getKeyForScope_nonexistent_account() {
});
});
await Assert.rejects(fxa.keys.getKeyForScope(SCOPE_OLD_SYNC), err => {
Assert.equal(err.message, ERROR_INVALID_ACCOUNT_STATE);
return true; // expected error
});
// 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 promiseLogout;
@ -1404,31 +1405,6 @@ 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;

29
services/sync/modules/sync_auth.sys.mjs
View File

@ -387,28 +387,22 @@ SyncAuthManager.prototype = {
// Do the token dance, with a retry in case of transient auth failure.
// We need to prove that we know the sync key in order to get a token
// from the tokenserver.
let getToken = async (key, accessToken) => {
let getToken = async key => {
this._log.info("Getting a sync token from", this._tokenServerUrl);
let token = await this._fetchTokenUsingOAuth(key, accessToken);
let token = await this._fetchTokenUsingOAuth(key);
this._log.trace("Successfully got a token");
return token;
};
const ttl = fxAccountsCommon.OAUTH_TOKEN_FOR_SYNC_LIFETIME_SECONDS;
try {
let token, key;
try {
this._log.info("Getting sync key");
const tokenAndKey = await fxa.getOAuthTokenAndKey({
scope: SCOPE_OLD_SYNC,
ttl,
});
key = tokenAndKey.key;
key = await fxa.keys.getKeyForScope(SCOPE_OLD_SYNC);
if (!key) {
throw new Error("browser does not have the sync key, cannot sync");
}
token = await getToken(key, tokenAndKey.token);
token = await getToken(key);
} catch (err) {
// If we get a 401 fetching the token it may be that our auth tokens needed
// to be regenerated; retry exactly once.
@ -418,11 +412,8 @@ SyncAuthManager.prototype = {
this._log.warn(
"Token server returned 401, retrying token fetch with fresh credentials"
);
const tokenAndKey = await fxa.getOAuthTokenAndKey({
scope: SCOPE_OLD_SYNC,
ttl,
});
token = await getToken(tokenAndKey.key, tokenAndKey.token);
key = await fxa.keys.getKeyForScope(SCOPE_OLD_SYNC);
token = await getToken(key);
}
// TODO: Make it be only 80% of the duration, so refresh the token
// before it actually expires. This is to avoid sync storage errors
@ -469,13 +460,17 @@ SyncAuthManager.prototype = {
},
/**
* Exchanges an OAuth access_token for a TokenServer token.
* Generates an OAuth access_token using the OLD_SYNC scope and exchanges it
* for a TokenServer token.
*
* @returns {Promise}
* @private
*/
async _fetchTokenUsingOAuth(key, accessToken) {
async _fetchTokenUsingOAuth(key) {
this._log.debug("Getting a token using OAuth");
const fxa = this._fxaService;
const ttl = fxAccountsCommon.OAUTH_TOKEN_FOR_SYNC_LIFETIME_SECONDS;
const accessToken = await fxa.getOAuthToken({ scope: SCOPE_OLD_SYNC, ttl });
const headers = {
"X-KeyId": key.kid,
};

12
services/sync/tests/unit/test_sync_auth_manager.js
View File

@ -37,8 +37,9 @@ const { TokenServerClient, TokenServerClientServerError } =
ChromeUtils.importESModule(
"resource://services-common/tokenserverclient.sys.mjs"
);
const { AccountState, ERROR_INVALID_ACCOUNT_STATE } =
ChromeUtils.importESModule("resource://gre/modules/FxAccounts.sys.mjs");
const { AccountState } = ChromeUtils.importESModule(
"resource://gre/modules/FxAccounts.sys.mjs"
);
const SECOND_MS = 1000;
const MINUTE_MS = SECOND_MS * 60;
@ -191,11 +192,8 @@ add_task(async function test_initialializeWithAuthErrorAndDeletedAccount() {
await Assert.rejects(
syncAuthManager._ensureValidToken(),
err => {
Assert.equal(err.message, ERROR_INVALID_ACCOUNT_STATE);
return true; // expected error
},
"should reject because the account was deleted"
AuthenticationError,
"should reject due to an auth error"
);
Assert.ok(accessTokenWithSessionTokenCalled);