зеркало из https://github.com/mozilla/gecko-dev.git
Bug 1928165 - Update desktop oauth flows to get the service info from the login message. r=lina,skhamis
Differential Revision: https://phabricator.services.mozilla.com/D227388
This commit is contained in:
Mark Hammond
Родитель
523d20562d
Коммит
0cb88f76d6
|
|
@ -545,7 +545,7 @@ export class FxAccounts {
|
|||
* or null if no user is signed in. This function never fails except
|
||||
* in pathological cases (eg, file-system errors, etc)
|
||||
*/
|
||||
getSignedInUser() {
|
||||
getSignedInUser(addnFields = []) {
|
||||
// Note we don't return the session token, but use it to see if we
|
||||
// should fetch the profile. Ditto scopedKeys re verified.
|
||||
const ACCT_DATA_FIELDS = [
|
||||
|
|
@ -557,7 +557,9 @@ export class FxAccounts {
|
|||
];
|
||||
const PROFILE_FIELDS = ["displayName", "avatar", "avatarDefault"];
|
||||
return this._withCurrentAccountState(async currentState => {
|
||||
const data = await currentState.getUserAccountData(ACCT_DATA_FIELDS);
|
||||
const data = await currentState.getUserAccountData(
|
||||
ACCT_DATA_FIELDS.concat(addnFields)
|
||||
);
|
||||
if (!data) {
|
||||
return null;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -289,6 +289,7 @@ export let FXA_PWDMGR_PLAINTEXT_FIELDS = new Set([
|
|||
"profileCache",
|
||||
"encryptedSendTabKeys",
|
||||
"encryptedCloseTabKeys",
|
||||
"requestedServices", // info about services which should be configured, used during oauth flows.
|
||||
]);
|
||||
|
||||
// Fields we store in secure storage if it exists.
|
||||
|
|
|
|||
|
|
@ -92,26 +92,20 @@ ChromeUtils.defineLazyGetter(lazy, "l10n", function () {
|
|||
return new Localization(["browser/sync.ftl"], true);
|
||||
});
|
||||
|
||||
// These engines were added years after Sync had been introduced, they need
|
||||
// special handling since they are system add-ons and are un-available on
|
||||
// older versions of Firefox.
|
||||
// These are only used in the non-oauth flows.
|
||||
const EXTRA_ENGINES = ["addresses", "creditcards"];
|
||||
|
||||
// These engines will be displayed to the user to pick which they would like to
|
||||
// use.
|
||||
const CHOOSE_WHAT_TO_SYNC_ALWAYS_AVAILABLE = [
|
||||
"addons",
|
||||
"bookmarks",
|
||||
"creditcards",
|
||||
"history",
|
||||
"passwords",
|
||||
"prefs",
|
||||
"tabs",
|
||||
];
|
||||
|
||||
// Engines which we need to inspect a pref to see if they are available in the oauth flows.
|
||||
const CHOOSE_WHAT_TO_SYNC_OPTIONALLY_AVAILABLE = ["addresses"];
|
||||
// Engines which we need to inspect a pref to see if they are available, and
|
||||
// possibly have their default preference value to disabled.
|
||||
const CHOOSE_WHAT_TO_SYNC_OPTIONALLY_AVAILABLE = ["addresses", "creditcards"];
|
||||
|
||||
/**
|
||||
* A helper function that extracts the message and stack from an error object.
|
||||
|
|
@ -324,7 +318,6 @@ FxAccountsWebChannel.prototype = {
|
|||
break;
|
||||
case COMMAND_FXA_STATUS:
|
||||
log.debug("fxa_status received");
|
||||
|
||||
const service = data && data.service;
|
||||
const isPairing = data && data.isPairing;
|
||||
const context = data && data.context;
|
||||
|
|
@ -478,12 +471,14 @@ FxAccountsWebChannelHelpers.prototype = {
|
|||
|
||||
_setEnabledEngines(offeredEngines, declinedEngines) {
|
||||
if (offeredEngines && declinedEngines) {
|
||||
EXTRA_ENGINES.forEach(engine => {
|
||||
log.debug("Received offered engines", offeredEngines);
|
||||
CHOOSE_WHAT_TO_SYNC_OPTIONALLY_AVAILABLE.forEach(engine => {
|
||||
if (
|
||||
offeredEngines.includes(engine) &&
|
||||
!declinedEngines.includes(engine)
|
||||
) {
|
||||
// These extra engines are disabled by default.
|
||||
log.debug(`Enabling optional engine '${engine}'`);
|
||||
Services.prefs.setBoolPref(`services.sync.engine.${engine}`, true);
|
||||
}
|
||||
});
|
||||
|
|
@ -492,83 +487,124 @@ FxAccountsWebChannelHelpers.prototype = {
|
|||
declinedEngines.forEach(engine => {
|
||||
Services.prefs.setBoolPref(`services.sync.engine.${engine}`, false);
|
||||
});
|
||||
} else {
|
||||
log.debug("Did not receive any engine selection information");
|
||||
}
|
||||
},
|
||||
/**
|
||||
* stores sync login info it in the fxaccounts service
|
||||
|
||||
/** Internal function used to configure the requested services.
|
||||
*
|
||||
* The "services" param is an object as received from the FxA server.
|
||||
*/
|
||||
async _enableRequestedServices(requestedServices) {
|
||||
if (!requestedServices) {
|
||||
log.warn(
|
||||
"fxa login completed but we don't have a record of which services were enabled."
|
||||
);
|
||||
return;
|
||||
}
|
||||
log.debug(`services requested are ${Object.keys(requestedServices)}`);
|
||||
if (requestedServices.sync) {
|
||||
const xps = await this._initializeSync();
|
||||
const { offeredEngines, declinedEngines } = requestedServices.sync;
|
||||
this._setEnabledEngines(offeredEngines, declinedEngines);
|
||||
log.debug("Webchannel is enabling sync");
|
||||
await xps.Weave.Service.configure();
|
||||
}
|
||||
},
|
||||
|
||||
/**
|
||||
* The login message is sent when the user user has initially logged in but may not be fully connected.
|
||||
* * In the non-oauth flows, if the user is verified, then the browser itself is able to transition the
|
||||
* user to fully connected.
|
||||
* * In the oauth flows, we will need an `oauth_login` message with our scoped keys to be fully connected.
|
||||
* @param accountData the user's account data and credentials
|
||||
*/
|
||||
async login(accountData) {
|
||||
const signedInUser = await this._fxAccounts.getSignedInUser();
|
||||
// This is delicate for oauth flows and edge-cases. Consider (a) user logs in but does not verify,
|
||||
// (b) browser restarts, (c) user select "finish setup", at which point they are again prompted for their password.
|
||||
// In that scenario, we've been sent this `login` message *both* at (a) and at (c).
|
||||
// Importantly, the message from (a) is the one that actually has the service information we care about
|
||||
// (eg, the sync engine selections) - (c) *will* have `services.sync` but it will be an empty object.
|
||||
// This means we need to take care to not lose the services from (a) when processing (c).
|
||||
const signedInUser = await this._fxAccounts.getSignedInUser([
|
||||
"requestedServices",
|
||||
]);
|
||||
let existingServices;
|
||||
if (signedInUser) {
|
||||
await this._disconnect();
|
||||
if (signedInUser.uid != accountData.uid) {
|
||||
log.warn(
|
||||
"the webchannel found a different user signed in - signing them out."
|
||||
);
|
||||
await this._disconnect();
|
||||
} else {
|
||||
existingServices = JSON.parse(signedInUser.requestedServices);
|
||||
log.debug(
|
||||
"Webchannel is updating the info for an already logged in user."
|
||||
);
|
||||
}
|
||||
} else {
|
||||
log.debug("Webchannel is logging new a user in.");
|
||||
}
|
||||
// We don't act on customizeSync anymore, it used to open a dialog inside
|
||||
// the browser to selecte the engines to sync but we do it on the web now.
|
||||
log.debug("Webchannel is logging a user in.");
|
||||
// There are (or were) extra fields here we don't want to actually store.
|
||||
delete accountData.customizeSync;
|
||||
delete accountData.verifiedCanLinkAccount;
|
||||
if (lazy.oauthEnabled) {
|
||||
// We once accidentally saw these from the server and got confused about who owned the key fetching.
|
||||
delete accountData.keyFetchToken;
|
||||
delete accountData.unwrapBKey;
|
||||
}
|
||||
|
||||
// Save requested services for later.
|
||||
const requestedServices = accountData.services;
|
||||
// The "services" being connected - see above re our careful handling of existing data.
|
||||
// Note that we don't attempt to merge any data - we keep the first value we see for a service
|
||||
// and ignore that service subsequently (as it will be common for subsequent messages to
|
||||
// name a service but not supply any data for it)
|
||||
const requestedServices = {
|
||||
...(accountData.services ?? {}),
|
||||
...existingServices,
|
||||
};
|
||||
delete accountData.services;
|
||||
|
||||
// the user has already been shown the "can link account"
|
||||
// screen. No need to keep this data around.
|
||||
delete accountData.verifiedCanLinkAccount;
|
||||
|
||||
// Remember who it was so we can log out next time.
|
||||
// This `verified` check is really just for our tests and pre-oauth flows.
|
||||
// However, in all cases it's misplaced - we should set it as soon as *sync*
|
||||
// starts or is configured, as it's the merging done by sync it protects against.
|
||||
// We should clean up handling of this pref in a followup.
|
||||
if (accountData.verified) {
|
||||
this.setPreviousAccountNameHashPref(accountData.email);
|
||||
}
|
||||
|
||||
await this._fxAccounts.telemetry.recordConnection(
|
||||
Object.keys(requestedServices || {}),
|
||||
Object.keys(requestedServices),
|
||||
"webchannel"
|
||||
);
|
||||
|
||||
if (lazy.oauthEnabled) {
|
||||
// XXX - work around a server issue - we should never be handed these items in oauth flows.
|
||||
// Fixed in https://github.com/mozilla/fxa/pull/17892, so this can probably be made more
|
||||
// aggressive (eg, refuse to connect if they are present or similar)
|
||||
delete accountData.keyFetchToken;
|
||||
delete accountData.unwrapBKey;
|
||||
// We need to remember the requested services because we can't act on them until we get the `oauth_login` message.
|
||||
// And because we might not get that message in this browser session (eg, the browser might restart before the
|
||||
// user enters their verification code), they are persisted with the account state.
|
||||
log.debug(`storing info for services ${Object.keys(requestedServices)}`);
|
||||
accountData.requestedServices = JSON.stringify(requestedServices);
|
||||
await this._fxAccounts._internal.setSignedInUser(accountData);
|
||||
} else {
|
||||
const xps = await this._initializeSync();
|
||||
// Note we don't persist anything in requestedServices for non oauth flows because we act on them now.
|
||||
await this._fxAccounts._internal.setSignedInUser(accountData);
|
||||
|
||||
if (requestedServices) {
|
||||
// User has enabled Sync.
|
||||
if (requestedServices.sync) {
|
||||
const { offeredEngines, declinedEngines } = requestedServices.sync;
|
||||
this._setEnabledEngines(offeredEngines, declinedEngines);
|
||||
log.debug("Webchannel is enabling sync");
|
||||
await xps.Weave.Service.configure();
|
||||
}
|
||||
}
|
||||
await this._enableRequestedServices(requestedServices);
|
||||
}
|
||||
log.debug("Webchannel finished logging a user in.");
|
||||
},
|
||||
|
||||
/**
|
||||
* Disconnects the user from Sync and FxA
|
||||
*
|
||||
*/
|
||||
_disconnect() {
|
||||
return SyncDisconnect.disconnect(false);
|
||||
},
|
||||
|
||||
/**
|
||||
* Logins in to sync by completing an OAuth flow
|
||||
* @param { Object } oauthData: The oauth code and state as returned by the server */
|
||||
* @param { Object } oauthData: The oauth code and state as returned by the server
|
||||
*/
|
||||
async oauthLogin(oauthData) {
|
||||
log.debug("Webchannel is completing the oauth flow");
|
||||
const xps = await this._initializeSync();
|
||||
const { sessionToken, email } =
|
||||
const { uid, sessionToken, email, requestedServices } =
|
||||
await this._fxAccounts._internal.getUserAccountData([
|
||||
"uid",
|
||||
"sessionToken",
|
||||
"email",
|
||||
"requestedServices",
|
||||
]);
|
||||
// First we finish the ongoing oauth flow
|
||||
const { scopedKeys, refreshToken } =
|
||||
|
|
@ -587,28 +623,31 @@ FxAccountsWebChannelHelpers.prototype = {
|
|||
// Then, we persist the sync keys
|
||||
await this._fxAccounts._internal.setScopedKeys(scopedKeys);
|
||||
|
||||
// Now that we have the scoped keys, we set our status to verified
|
||||
await this._fxAccounts._internal.setUserVerified();
|
||||
// And work out what should be enabled.
|
||||
const services = oauthData.services;
|
||||
log.trace("Webchannel is enabling services", services);
|
||||
if (services) {
|
||||
if (services.sync) {
|
||||
// User has enabled Sync.
|
||||
log.debug("Webchannel is enabling sync");
|
||||
const { offeredEngines, declinedEngines } = services.sync;
|
||||
this._setEnabledEngines(offeredEngines, declinedEngines);
|
||||
xps.Weave.Service.configure();
|
||||
try {
|
||||
let parsedRequestedServices;
|
||||
if (requestedServices) {
|
||||
parsedRequestedServices = JSON.parse(requestedServices);
|
||||
}
|
||||
} else {
|
||||
// To support fxa before it knew to send `services`, we assume it missing means "enable sync" and that the top-level
|
||||
// payload has `offeredSyncEngines` and `declinedSyncEngines`
|
||||
// TODO: remove this branch as soon as the above branch starts working - only old FxA servers need this!
|
||||
const { declinedSyncEngines, offeredSyncEngines } = oauthData;
|
||||
log.debug("Webchannel is enabling sync (deprecated message format");
|
||||
this._setEnabledEngines(offeredSyncEngines, declinedSyncEngines);
|
||||
xps.Weave.Service.configure();
|
||||
await this._enableRequestedServices(parsedRequestedServices);
|
||||
} finally {
|
||||
// We don't want them hanging around in storage.
|
||||
await this._fxAccounts._internal.updateUserAccountData({
|
||||
uid,
|
||||
requestedServices: null,
|
||||
});
|
||||
}
|
||||
|
||||
// Now that we have the scoped keys, we set our status to verified.
|
||||
// This will kick off Sync or other services we configured.
|
||||
await this._fxAccounts._internal.setUserVerified();
|
||||
log.debug("Webchannel completed oauth flows");
|
||||
},
|
||||
|
||||
/**
|
||||
* Disconnects the user from Sync and FxA
|
||||
*/
|
||||
_disconnect() {
|
||||
return SyncDisconnect.disconnect(false);
|
||||
},
|
||||
|
||||
/**
|
||||
|
|
@ -639,7 +678,6 @@ FxAccountsWebChannelHelpers.prototype = {
|
|||
let browser = sendingContext.browsingContext.top.embedderElement;
|
||||
const isPrivateBrowsing =
|
||||
this._privateBrowsingUtils.isBrowserPrivate(browser);
|
||||
log.debug("is private browsing", isPrivateBrowsing);
|
||||
return isPrivateBrowsing;
|
||||
},
|
||||
|
||||
|
|
@ -663,13 +701,15 @@ FxAccountsWebChannelHelpers.prototype = {
|
|||
// with FxA as part of a Sync flow should work all the time. If
|
||||
// Sync is broken in PB mode, users will think Firefox is broken.
|
||||
// See https://bugzilla.mozilla.org/show_bug.cgi?id=1323853
|
||||
log.debug("service", service);
|
||||
return (
|
||||
!this.isPrivateBrowsingMode(sendingContext) ||
|
||||
service === "sync" ||
|
||||
context === "fx_desktop_v3" ||
|
||||
isPairing
|
||||
//
|
||||
// XXX - This hard-coded context seems bad?
|
||||
let pb = this.isPrivateBrowsingMode(sendingContext);
|
||||
let ok =
|
||||
!pb || service === "sync" || context === "fx_desktop_v3" || isPairing;
|
||||
log.debug(
|
||||
`fxa status ok=${ok} - private=${pb}, service=${service}, context=${context}, pairing=${isPairing}`
|
||||
);
|
||||
return ok;
|
||||
},
|
||||
|
||||
/**
|
||||
|
|
@ -707,45 +747,31 @@ FxAccountsWebChannelHelpers.prototype = {
|
|||
capabilities,
|
||||
};
|
||||
},
|
||||
|
||||
_getCapabilities() {
|
||||
if (lazy.oauthEnabled) {
|
||||
let engines = Array.from(CHOOSE_WHAT_TO_SYNC_ALWAYS_AVAILABLE);
|
||||
for (let optionalEngine of CHOOSE_WHAT_TO_SYNC_OPTIONALLY_AVAILABLE) {
|
||||
if (
|
||||
Services.prefs.getBoolPref(
|
||||
`services.sync.engine.${optionalEngine}.available`,
|
||||
false
|
||||
)
|
||||
) {
|
||||
engines.push(optionalEngine);
|
||||
}
|
||||
// pre-oauth flows there we a strange setup where we just supplied the "extra" engines,
|
||||
// whereas oauth flows want them all.
|
||||
let engines = lazy.oauthEnabled
|
||||
? Array.from(CHOOSE_WHAT_TO_SYNC_ALWAYS_AVAILABLE)
|
||||
: [];
|
||||
for (let optionalEngine of CHOOSE_WHAT_TO_SYNC_OPTIONALLY_AVAILABLE) {
|
||||
if (
|
||||
Services.prefs.getBoolPref(
|
||||
`services.sync.engine.${optionalEngine}.available`,
|
||||
false
|
||||
)
|
||||
) {
|
||||
engines.push(optionalEngine);
|
||||
}
|
||||
return {
|
||||
multiService: true,
|
||||
pairing: lazy.pairingEnabled,
|
||||
choose_what_to_sync: true,
|
||||
engines,
|
||||
};
|
||||
}
|
||||
return {
|
||||
multiService: true,
|
||||
pairing: lazy.pairingEnabled,
|
||||
engines: this._getAvailableExtraEngines(),
|
||||
choose_what_to_sync: true,
|
||||
engines,
|
||||
};
|
||||
},
|
||||
|
||||
_getAvailableExtraEngines() {
|
||||
return EXTRA_ENGINES.filter(engineName => {
|
||||
try {
|
||||
return Services.prefs.getBoolPref(
|
||||
`services.sync.engine.${engineName}.available`
|
||||
);
|
||||
} catch (e) {
|
||||
return false;
|
||||
}
|
||||
});
|
||||
},
|
||||
|
||||
async changePassword(credentials) {
|
||||
// If |credentials| has fields that aren't handled by accounts storage,
|
||||
// updateUserAccountData will throw - mainly to prevent errors in code
|
||||
|
|
|
|||
|
|
@ -919,6 +919,68 @@ add_task(async function test_helpers_login_nothing_offered() {
|
|||
Assert.ok(configured);
|
||||
});
|
||||
|
||||
add_task(async function test_helpers_persist_requested_services() {
|
||||
ensureOauthConfigured();
|
||||
let accountData = null;
|
||||
const helpers = new FxAccountsWebChannelHelpers({
|
||||
fxAccounts: {
|
||||
_internal: {
|
||||
async setSignedInUser(newAccountData) {
|
||||
accountData = newAccountData;
|
||||
return accountData;
|
||||
},
|
||||
},
|
||||
async getSignedInUser() {
|
||||
return accountData;
|
||||
},
|
||||
telemetry: {
|
||||
recordConnection() {},
|
||||
},
|
||||
},
|
||||
weaveXPCOM: {
|
||||
whenLoaded() {},
|
||||
Weave: {
|
||||
Service: {},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
await helpers.login({
|
||||
uid: "auid",
|
||||
email: "testuser@testuser.com",
|
||||
verifiedCanLinkAccount: true,
|
||||
services: {
|
||||
first_only: { x: 10 }, // this data is not in the update below.
|
||||
sync: { important: true },
|
||||
},
|
||||
});
|
||||
|
||||
Assert.deepEqual(JSON.parse(accountData.requestedServices), {
|
||||
first_only: { x: 10 },
|
||||
sync: { important: true },
|
||||
});
|
||||
// A second "login" message without the services.
|
||||
await helpers.login({
|
||||
uid: "auid",
|
||||
email: "testuser@testuser.com",
|
||||
verifiedCanLinkAccount: true,
|
||||
services: {
|
||||
// the service is mentioned, but data is empty, so it's the old version of the data we want.
|
||||
sync: {},
|
||||
// a new service we never saw before, but we still want it.
|
||||
new: { name: "opted in" }, // not in original, but we want in the final.
|
||||
},
|
||||
});
|
||||
// the version with the data should remain.
|
||||
Assert.deepEqual(JSON.parse(accountData.requestedServices), {
|
||||
first_only: { x: 10 },
|
||||
sync: { important: true },
|
||||
new: { name: "opted in" },
|
||||
});
|
||||
|
||||
resetOauthConfig();
|
||||
});
|
||||
|
||||
add_test(function test_helpers_open_sync_preferences() {
|
||||
let helpers = new FxAccountsWebChannelHelpers({
|
||||
fxAccounts: {},
|
||||
|
|
@ -997,7 +1059,7 @@ add_task(async function test_helpers_getFxAStatus_engines_oauth() {
|
|||
ok(!!fxaStatus);
|
||||
ok(!!fxaStatus.signedInUser);
|
||||
// in the oauth flows we expect all engines.
|
||||
deepEqual(fxaStatus.capabilities.engines, [
|
||||
deepEqual(fxaStatus.capabilities.engines.toSorted(), [
|
||||
"addons",
|
||||
"bookmarks",
|
||||
"creditcards",
|
||||
|
|
@ -1010,15 +1072,15 @@ add_task(async function test_helpers_getFxAStatus_engines_oauth() {
|
|||
// try again with addresses enabled.
|
||||
Services.prefs.setBoolPref("services.sync.engine.addresses.available", true);
|
||||
fxaStatus = await helpers.getFxaStatus("sync", mockSendingContext);
|
||||
deepEqual(fxaStatus.capabilities.engines, [
|
||||
deepEqual(fxaStatus.capabilities.engines.toSorted(), [
|
||||
"addons",
|
||||
"addresses",
|
||||
"bookmarks",
|
||||
"creditcards",
|
||||
"history",
|
||||
"passwords",
|
||||
"prefs",
|
||||
"tabs",
|
||||
"addresses",
|
||||
]);
|
||||
|
||||
resetOauthConfig();
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче