From 0ae7776349adbd8e884e322b78becc9caf030524 Mon Sep 17 00:00:00 2001 From: Mark Hammond Date: Sat, 19 Oct 2024 18:36:12 +0000 Subject: [PATCH] Bug 1924175 - support service= URL param in fxa oauth flows. r=lina Differential Revision: https://phabricator.services.mozilla.com/D225401 --- services/fxaccounts/FxAccountsConfig.sys.mjs | 8 +++- .../fxaccounts/FxAccountsWebChannel.sys.mjs | 28 ++++++++++--- services/fxaccounts/tests/xpcshell/head.js | 21 ++++++++++ .../tests/xpcshell/test_accounts_config.js | 32 +++++++++++++++ .../tests/xpcshell/test_web_channel.js | 40 +++++++++++++++++++ 5 files changed, 121 insertions(+), 8 deletions(-) diff --git a/services/fxaccounts/FxAccountsConfig.sys.mjs b/services/fxaccounts/FxAccountsConfig.sys.mjs index 160b2d96ed80..a2921e9a68ca 100644 --- a/services/fxaccounts/FxAccountsConfig.sys.mjs +++ b/services/fxaccounts/FxAccountsConfig.sys.mjs @@ -343,10 +343,14 @@ export var FxAccountsConfig = { }, async _getAuthParams() { + let params = { service: SYNC_PARAM }; if (this._isOAuthFlow()) { const scopes = [SCOPE_APP_SYNC, SCOPE_PROFILE]; - return lazy.fxAccounts._internal.beginOAuthFlow(scopes); + Object.assign( + params, + await lazy.fxAccounts._internal.beginOAuthFlow(scopes) + ); } - return { service: SYNC_PARAM }; + return params; }, }; diff --git a/services/fxaccounts/FxAccountsWebChannel.sys.mjs b/services/fxaccounts/FxAccountsWebChannel.sys.mjs index cf8dab8fc361..433072122c34 100644 --- a/services/fxaccounts/FxAccountsWebChannel.sys.mjs +++ b/services/fxaccounts/FxAccountsWebChannel.sys.mjs @@ -553,15 +553,14 @@ FxAccountsWebChannelHelpers.prototype = { async oauthLogin(oauthData) { log.debug("Webchannel is completing the oauth flow"); const xps = await this._initializeSync(); - const { code, state, declinedSyncEngines, offeredSyncEngines } = oauthData; const { sessionToken } = await this._fxAccounts._internal.getUserAccountData(["sessionToken"]); // First we finish the ongoing oauth flow const { scopedKeys, refreshToken } = await this._fxAccounts._internal.completeOAuthFlow( sessionToken, - code, - state + oauthData.code, + oauthData.state ); // We don't currently use the refresh token in Firefox Desktop, lets be good citizens and revoke it. @@ -572,9 +571,26 @@ FxAccountsWebChannelHelpers.prototype = { // Now that we have the scoped keys, we set our status to verified await this._fxAccounts._internal.setUserVerified(); - this._setEnabledEngines(offeredSyncEngines, declinedSyncEngines); - log.debug("Webchannel is enabling sync"); - xps.Weave.Service.configure(); + // 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(); + } + } 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(); + } }, /** diff --git a/services/fxaccounts/tests/xpcshell/head.js b/services/fxaccounts/tests/xpcshell/head.js index 6a87df8ef682..15539218fa72 100644 --- a/services/fxaccounts/tests/xpcshell/head.js +++ b/services/fxaccounts/tests/xpcshell/head.js @@ -27,6 +27,27 @@ const MOCK_ACCOUNT_KEYS = { }, }; +function ensureOauthConfigured() { + Services.prefs.setBoolPref("identity.fxaccounts.oauth.enabled", true); + Services.prefs.setStringPref( + "identity.fxaccounts.contextParam", + "oauth_webchannel_v1" + ); +} + +function ensureOauthNotConfigured() { + Services.prefs.setBoolPref("identity.fxaccounts.oauth.enabled", false); + Services.prefs.setStringPref( + "identity.fxaccounts.contextParam", + "fx_desktop_v3" + ); +} + +function resetOauthConfig() { + Services.prefs.clearUserPref("identity.fxaccounts.oauth.enabled"); + Services.prefs.clearUserPref("identity.fxaccounts.contextParam"); +} + (function initFxAccountsTestingInfrastructure() { do_get_profile(); diff --git a/services/fxaccounts/tests/xpcshell/test_accounts_config.js b/services/fxaccounts/tests/xpcshell/test_accounts_config.js index 3af0c83a3345..b0523110d72b 100644 --- a/services/fxaccounts/tests/xpcshell/test_accounts_config.js +++ b/services/fxaccounts/tests/xpcshell/test_accounts_config.js @@ -9,6 +9,7 @@ const { FxAccounts } = ChromeUtils.importESModule( add_task( async function test_non_https_remote_server_uri_with_requireHttps_false() { + ensureOauthNotConfigured(); Services.prefs.setStringPref("identity.fxaccounts.autoconfig.uri", ""); Services.prefs.setBoolPref("identity.fxaccounts.allowHttp", true); Services.prefs.setStringPref( @@ -20,8 +21,19 @@ add_task( "http://example.com/?context=fx_desktop_v3&entrypoint=test&action=email&service=sync" ); + ensureOauthConfigured(); + let url = new URL(await FxAccounts.config.promiseConnectAccountURI("test")); + Assert.equal(url.host, "example.com"); + Assert.equal(url.searchParams.get("context"), "oauth_webchannel_v1"); + Assert.equal(url.searchParams.get("service"), "sync"); + Assert.equal(url.searchParams.get("entrypoint"), "test"); + Assert.equal(url.searchParams.get("action"), "email"); + Assert.equal(url.searchParams.get("client_id"), "5882386c6d801776"); + Assert.equal(url.searchParams.get("response_type"), "code"); + Services.prefs.clearUserPref("identity.fxaccounts.remote.root"); Services.prefs.clearUserPref("identity.fxaccounts.allowHttp"); + resetOauthConfig(); } ); @@ -57,3 +69,23 @@ add_task(async function test_is_production_config() { Assert.ok(!FxAccounts.config.isProductionConfig()); Services.prefs.clearUserPref("identity.sync.tokenserver.uri"); }); + +add_task(async function test_promise_account_service_param() { + ensureOauthNotConfigured(); + Assert.equal( + await FxAccounts.config.promiseConnectAccountURI("test", { + service: "custom-service", + }), + "https://accounts.firefox.com/?context=fx_desktop_v3&entrypoint=test&action=email&service=custom-service" + ); + ensureOauthConfigured(); + let url = new URL( + await FxAccounts.config.promiseConnectAccountURI("test", { + service: "custom-service", + }) + ); + Assert.equal(url.searchParams.get("context"), "oauth_webchannel_v1"); + Assert.equal(url.searchParams.get("client_id"), "5882386c6d801776"); + Assert.equal(url.searchParams.get("service"), "custom-service"); + resetOauthConfig(); +}); diff --git a/services/fxaccounts/tests/xpcshell/test_web_channel.js b/services/fxaccounts/tests/xpcshell/test_web_channel.js index f97f155335f7..7c33c4895006 100644 --- a/services/fxaccounts/tests/xpcshell/test_web_channel.js +++ b/services/fxaccounts/tests/xpcshell/test_web_channel.js @@ -956,6 +956,7 @@ add_task(async function test_helpers_getFxAStatus_extra_engines() { }, }); + ensureOauthNotConfigured(); Services.prefs.setBoolPref( "services.sync.engine.creditcards.available", true @@ -965,7 +966,46 @@ add_task(async function test_helpers_getFxAStatus_extra_engines() { let fxaStatus = await helpers.getFxaStatus("sync", mockSendingContext); ok(!!fxaStatus); ok(!!fxaStatus.signedInUser); + // in the non-oauth flows we only expect "extra" engines. deepEqual(fxaStatus.capabilities.engines, ["creditcards"]); + resetOauthConfig(); +}); + +add_task(async function test_helpers_getFxAStatus_engines_oauth() { + let helpers = new FxAccountsWebChannelHelpers({ + fxAccounts: { + _internal: { + getUserAccountData() { + return Promise.resolve({ + email: "testuser@testuser.com", + sessionToken: "sessionToken", + uid: "uid", + verified: true, + }); + }, + }, + }, + privateBrowsingUtils: { + isBrowserPrivate: () => true, + }, + }); + + ensureOauthConfigured(); + let fxaStatus = await helpers.getFxaStatus("sync", mockSendingContext); + ok(!!fxaStatus); + ok(!!fxaStatus.signedInUser); + // in the oauth flows we expect all engines. + deepEqual(fxaStatus.capabilities.engines, [ + "addons", + "addresses", + "bookmarks", + "creditcards", + "history", + "passwords", + "preferences", + "tabs", + ]); + resetOauthConfig(); }); add_task(async function test_helpers_getFxaStatus_allowed_signedInUser() {