commit 5cf8e7214a2e6dea79920680135bad6621fb4c62 parent 93a889885e1fec771d73fdff5b20cd6a6d3f11a2 Author: Mark Hammond <mhammond@skippinet.com.au> Date: Wed, 12 Nov 2025 16:59:35 +0000 Bug 1997102 - Remove support for non-oauth based login flows. r=skhamis,credential-management-reviewers,fxview-reviewers,sync-reviewers,omc-reviewers,emcminn,dimi Remove all polling for verifcation state - all verification happens in this browser via the entering of codes into the fxa content code. There's no concept of verifying this session externally. Differential Revision: https://phabricator.services.mozilla.com/D270520 Diffstat:
19 files changed, 114 insertions(+), 1659 deletions(-)
diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js @@ -2284,9 +2284,6 @@ pref("identity.fxaccounts.remote.root", "https://accounts.firefox.com/"); // The value of the context query parameter passed in fxa requests. pref("identity.fxaccounts.contextParam", "oauth_webchannel_v1"); -// Whether to use the oauth flow for desktop or not -pref("identity.fxaccounts.oauth.enabled", true); - // The remote URL of the FxA Profile Server pref("identity.fxaccounts.remote.profile.uri", "https://profile.accounts.firefox.com/v1"); diff --git a/browser/components/firefoxview/firefox-view-tabs-setup-manager.sys.mjs b/browser/components/firefoxview/firefox-view-tabs-setup-manager.sys.mjs @@ -44,9 +44,9 @@ const SYNC_SERVICE_FINISHED = "weave:service:sync:finish"; const PRIMARY_PASSWORD_UNLOCKED = "passwordmgr-crypto-login"; function openTabInWindow(window, url) { - const { switchToTabHavingURI } = - window.docShell.chromeEventHandler.ownerGlobal; - switchToTabHavingURI(url, true, {}); + // Null checks as the passed window might be closing, particularly in tests. + const ownerGlobal = window.docShell?.chromeEventHandler?.ownerGlobal; + ownerGlobal?.switchToTabHavingURI(url, true, {}); } export const TabsSetupFlowManager = new (class { diff --git a/browser/components/firefoxview/tests/browser/browser_syncedtabs_firefoxview.js b/browser/components/firefoxview/tests/browser/browser_syncedtabs_firefoxview.js @@ -2,11 +2,9 @@ * http://creativecommons.org/publicdomain/zero/1.0/ */ add_setup(async function () { - Services.prefs.setBoolPref("identity.fxaccounts.oauth.enabled", false); registerCleanupFunction(() => { // reset internal state so it doesn't affect the next tests TabsSetupFlowManager.resetInternalState(); - Services.prefs.clearUserPref("identity.fxaccounts.oauth.enabled"); }); // gSync.init() is called in a requestIdleCallback. Force its initialization. diff --git a/browser/components/uitour/test/browser_UITour_sync.js b/browser/components/uitour/test/browser_UITour_sync.js @@ -8,17 +8,11 @@ const MOCK_FLOW_ID = const MOCK_FLOW_BEGIN_TIME = 1590780440325; const MOCK_DEVICE_ID = "7e450f3337d3479b8582ea1c9bb5ba6c"; -Services.prefs.setBoolPref("identity.fxaccounts.oauth.enabled", false); -Services.prefs.setStringPref( - "identity.fxaccounts.contextParam", - "fx_desktop_v3" -); +const gFxaParams = `context=${Services.prefs.getStringPref("identity.fxaccounts.contextParam")}`; registerCleanupFunction(function () { Services.prefs.clearUserPref("identity.fxaccounts.remote.root"); Services.prefs.clearUserPref("services.sync.username"); - Services.prefs.clearUserPref("identity.fxaccounts.oauth.enabled"); - Services.prefs.clearUserPref("identity.fxaccounts.contextParam"); }); add_task(setup_UITourTest); @@ -76,10 +70,10 @@ add_UITour_task(async function test_checkSyncCounts() { add_UITour_task(async function test_firefoxAccountsNoParams() { info("Load https://accounts.firefox.com"); await gContentAPI.showFirefoxAccounts(); - await BrowserTestUtils.browserLoaded( - gTestTab.linkedBrowser, - false, - "https://example.com/?context=fx_desktop_v3&entrypoint=uitour&action=email&service=sync" + await BrowserTestUtils.browserLoaded(gTestTab.linkedBrowser, false, url => + url.startsWith( + `https://example.com/?${gFxaParams}&entrypoint=uitour&action=email&service=sync` + ) ); }); @@ -89,17 +83,20 @@ add_UITour_task(async function test_firefoxAccountsValidParams() { await BrowserTestUtils.browserLoaded( gTestTab.linkedBrowser, false, - "https://example.com/?context=fx_desktop_v3&entrypoint=uitour&action=email&service=sync&utm_foo=foo&utm_bar=bar" + url => + url.startsWith( + `https://example.com/?${gFxaParams}&entrypoint=uitour&action=email&service=sync` + ) && url.includes("utm_foo=foo&utm_bar=bar") ); }); add_UITour_task(async function test_firefoxAccountsWithEmail() { info("Load https://accounts.firefox.com"); await gContentAPI.showFirefoxAccounts(null, null, "foo@bar.com"); - await BrowserTestUtils.browserLoaded( - gTestTab.linkedBrowser, - false, - "https://example.com/?context=fx_desktop_v3&entrypoint=uitour&email=foo%40bar.com&service=sync" + await BrowserTestUtils.browserLoaded(gTestTab.linkedBrowser, false, url => + url.startsWith( + `https://example.com/?${gFxaParams}&entrypoint=uitour&email=foo%40bar.com&service=sync` + ) ); }); @@ -114,8 +111,13 @@ add_UITour_task(async function test_firefoxAccountsWithEmailAndFlowParams() { await BrowserTestUtils.browserLoaded( gTestTab.linkedBrowser, false, - "https://example.com/?context=fx_desktop_v3&entrypoint=uitour&email=foo%40bar.com&service=sync&" + - `flow_id=${MOCK_FLOW_ID}&flow_begin_time=${MOCK_FLOW_BEGIN_TIME}&device_id=${MOCK_DEVICE_ID}` + url => + url.startsWith( + `https://example.com/?${gFxaParams}&entrypoint=uitour&email=foo%40bar.com&service=sync` + ) && + url.includes( + `flow_id=${MOCK_FLOW_ID}&flow_begin_time=${MOCK_FLOW_BEGIN_TIME}&device_id=${MOCK_DEVICE_ID}` + ) ); }); @@ -164,8 +166,13 @@ add_UITour_task( await BrowserTestUtils.browserLoaded( gTestTab.linkedBrowser, false, - "https://example.com/?context=fx_desktop_v3&entrypoint=uitour&email=foo%40bar.com&service=sync&" + - `flow_id=${MOCK_FLOW_ID}&flow_begin_time=${MOCK_FLOW_BEGIN_TIME}` + url => + url.startsWith( + `https://example.com/?${gFxaParams}&entrypoint=uitour&email=foo%40bar.com&service=sync` + ) && + url.includes( + `flow_id=${MOCK_FLOW_ID}&flow_begin_time=${MOCK_FLOW_BEGIN_TIME}` + ) ); } ); @@ -184,8 +191,10 @@ add_UITour_task(async function test_firefoxAccountsWithEmailAndEntrypoints() { await BrowserTestUtils.browserLoaded( gTestTab.linkedBrowser, false, - "https://example.com/?context=fx_desktop_v3&entrypoint=entry&email=foo%40bar.com&service=sync&" + - `entrypoint_experiment=exp&entrypoint_variation=var` + url => + url.startsWith( + `https://example.com/?${gFxaParams}&entrypoint=entry&email=foo%40bar.com&service=sync` + ) && url.includes(`entrypoint_experiment=exp&entrypoint_variation=var`) ); }); @@ -201,8 +210,10 @@ add_UITour_task(async function test_firefoxAccountsNonAlphaValue() { await BrowserTestUtils.browserLoaded( gTestTab.linkedBrowser, false, - "https://example.com/?context=fx_desktop_v3&entrypoint=uitour&action=email&service=sync&utm_foo=" + - expected + url => + url.startsWith( + `https://example.com/?${gFxaParams}&entrypoint=uitour&action=email&service=sync` + ) && url.includes(`&utm_foo=` + expected) ); }); diff --git a/services/automation/ServicesAutomation.sys.mjs b/services/automation/ServicesAutomation.sys.mjs @@ -112,28 +112,6 @@ export var Authentication = { } }, - async shortWaitForVerification(ms) { - LOG("shortWaitForVerification"); - let userData = await this.getSignedInUser(); - let timeoutID; - LOG("set a timeout"); - let timeoutPromise = new Promise(resolve => { - timeoutID = lazy.setTimeout(() => { - LOG(`Warning: no verification after ${ms}ms.`); - resolve(); - }, ms); - }); - LOG("set a fxAccounts.whenVerified"); - await Promise.race([ - lazy.fxAccounts - .whenVerified(userData) - .finally(() => lazy.clearTimeout(timeoutID)), - timeoutPromise, - ]); - LOG("done"); - return this.isReady(); - }, - async _confirmUser(uri) { LOG("Open new tab and load verification page"); let mainWindow = Services.wm.getMostRecentWindow("navigator:browser"); @@ -178,7 +156,7 @@ export var Authentication = { }); }); LOG("Page Loaded"); - let didVerify = await this.shortWaitForVerification(10000); + let didVerify = false; LOG("remove tab"); mainWindow.gBrowser.removeTab(newtab); return didVerify; @@ -195,7 +173,6 @@ export var Authentication = { )}`; let triedAlready = new Set(); const tries = 10; - const normalWait = 4000; for (let i = 0; i < tries; ++i) { let resp = await fetch(restmailURI); let messages = await resp.json(); @@ -230,12 +207,9 @@ export var Authentication = { LOG("resendVerificationEmail"); await lazy.fxAccounts.resendVerificationEmail(); } - if (await this.shortWaitForVerification(normalWait)) { - return true; - } } - // One last try. - return this.shortWaitForVerification(normalWait); + // this is all old, we need verification codes now. + return false; }, async signIn(username, password) { diff --git a/services/fxaccounts/FxAccounts.sys.mjs b/services/fxaccounts/FxAccounts.sys.mjs @@ -4,7 +4,6 @@ import { CryptoUtils } from "moz-src:///services/crypto/modules/utils.sys.mjs"; import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs"; -import { clearTimeout, setTimeout } from "resource://gre/modules/Timer.sys.mjs"; import { FxAccountsStorageManager } from "resource://gre/modules/FxAccountsStorage.sys.mjs"; @@ -66,13 +65,6 @@ XPCOMUtils.defineLazyPreferenceGetter( true ); -XPCOMUtils.defineLazyPreferenceGetter( - lazy, - "oauthEnabled", - "identity.fxaccounts.oauth.enabled", - true -); - export const ERROR_INVALID_ACCOUNT_STATE = "ERROR_INVALID_ACCOUNT_STATE"; // An AccountState object holds all state related to one specific account. @@ -110,7 +102,6 @@ export function AccountState(storageManager) { AccountState.prototype = { oauthTokens: null, - whenVerifiedDeferred: null, whenKeysReadyDeferred: null, // If the storage manager has been nuked then we are no longer current. @@ -119,12 +110,6 @@ AccountState.prototype = { }, abort() { - if (this.whenVerifiedDeferred) { - this.whenVerifiedDeferred.reject( - new Error("Verification aborted; Another user signing in") - ); - this.whenVerifiedDeferred = null; - } if (this.whenKeysReadyDeferred) { this.whenKeysReadyDeferred.reject( new Error("Key fetching aborted; Another user signing in") @@ -602,20 +587,14 @@ export class FxAccounts { await this.signOut(); return null; } - if (lazy.oauthEnabled) { - // data.verified is the sessionToken status. oauth cares only about whether it has the keys. - // (Note that this never forces `.verified` to `true` even if we *do* have the keys, which - // seems slightly odd) - // Note that is the primary-password is locked we can't get the scopedKeys even if they exist, so - // we don't want to pretend the user is unverified in that case. - if (Services.logins.isLoggedIn && !data.scopedKeys) { - data.verified = false; - } - } else if (!data.verified) { - // If the email is not verified, start polling for verification, - // but return null right away. We don't want to return a promise - // that might not be fulfilled for a long time. - this._internal.startVerifiedCheck(data); + // XXX - these comments reflect old baggage, we should clean this up. + // data.verified is the sessionToken status. oauth cares only about whether it has the keys. + // (Note that this never forces `.verified` to `true` even if we *do* have the keys, which + // seems slightly odd) + // Note that is the primary-password is locked we can't get the scopedKeys even if they exist, so + // we don't want to pretend the user is unverified in that case. + if (Services.logins.isLoggedIn && !data.scopedKeys) { + data.verified = false; } delete data.scopedKeys; @@ -734,8 +713,7 @@ export class FxAccounts { * */ resendVerificationEmail() { - return this._withSessionToken((token, currentState) => { - this._internal.startPollEmailStatus(currentState, token, "start"); + return this._withSessionToken((token, _currentState) => { return this._internal.fxAccountsClient.resendVerificationEmail(token); }, false); } @@ -756,13 +734,6 @@ export class FxAccounts { }); } - // we should try and kill this too. - whenVerified(data) { - return this._withCurrentAccountState(_ => { - return this._internal.whenVerified(data); - }); - } - /** * Generate a log file for the FxA action that just completed * and refresh the input & output streams. @@ -825,7 +796,6 @@ FxAccountsInternal.prototype = { ]; } - this.currentTimer = null; // This holds the list of attached clients from the /account/attached_clients endpoint // Most calls to that endpoint generally don't need fresh data so we try to prevent // as many network requests as possible @@ -1079,10 +1049,6 @@ FxAccountsInternal.prototype = { // really something we should commit to? Why not let the write happen in // the background? Already does for updateAccountData ;) await currentAccountState.promiseInitialized; - // Starting point for polling if new user - if (!lazy.oauthEnabled && !credentials.verified) { - this.startVerifiedCheck(credentials); - } await this.notifyObservers(ONLOGIN_NOTIFICATION); await this.updateDeviceRegistration(); return currentAccountState.resolve(); @@ -1117,11 +1083,6 @@ FxAccountsInternal.prototype = { * Reset state such that any previous flow is canceled. */ abortExistingFlow() { - if (this.currentTimer) { - log.debug("Polling aborted; Another user signing in"); - clearTimeout(this.currentTimer); - this.currentTimer = 0; - } if (this._profile) { this._profile.tearDown(); this._profile = null; @@ -1137,22 +1098,6 @@ FxAccountsInternal.prototype = { return this.currentAccountState.abort(); }, - async checkVerificationStatus() { - log.trace("checkVerificationStatus"); - let state = this.currentAccountState; - let data = await state.getUserAccountData(); - if (!data) { - log.trace("checkVerificationStatus - no user data"); - return null; - } - - // Always check the verification status, even if the local state indicates - // we're already verified. If the user changed their password, the check - // will fail, and we'll enter the reauth state. - log.trace("checkVerificationStatus - forcing verification status check"); - return this.startPollEmailStatus(state, data.sessionToken, "push"); - }, - /** * Destroyes an OAuth Token by sending a request to the FxA server * @@ -1242,59 +1187,6 @@ FxAccountsInternal.prototype = { return this.currentAccountState.getUserAccountData(fieldNames); }, - /** - * Setup for and if necessary do email verification polling. - */ - loadAndPoll() { - let currentState = this.currentAccountState; - return currentState.getUserAccountData().then(data => { - if (data) { - if (!lazy.oauthEnabled) { - if (!data.verified) { - this.startPollEmailStatus( - currentState, - data.sessionToken, - "browser-startup" - ); - } - } - } - return data; - }); - }, - - startVerifiedCheck(data) { - log.debug("startVerifiedCheck", data && data.verified); - if (logPII()) { - log.debug("startVerifiedCheck with user data", data); - } - - // Get us to the verified state. This returns a promise that will fire when - // verification is complete. - - // The callers of startVerifiedCheck never consume a returned promise (ie, - // this is simply kicking off a background fetch) so we must add a rejection - // handler to avoid runtime warnings about the rejection not being handled. - this.whenVerified(data).catch(err => - log.info("startVerifiedCheck promise was rejected: " + err) - ); - }, - - whenVerified(data) { - let currentState = this.currentAccountState; - if (data.verified) { - log.debug("already verified"); - return currentState.resolve(data); - } - if (!currentState.whenVerifiedDeferred) { - log.debug("whenVerified promise starts polling for verified email"); - this.startPollEmailStatus(currentState, data.sessionToken, "start"); - } - return currentState.whenVerifiedDeferred.promise.then(result => - currentState.resolve(result) - ); - }, - async notifyObservers(topic, data) { for (let f of this.observerPreloads) { try { @@ -1305,133 +1197,6 @@ FxAccountsInternal.prototype = { Services.obs.notifyObservers(null, topic, data); }, - startPollEmailStatus(currentState, sessionToken, why) { - log.debug("entering startPollEmailStatus: " + why); - // If we were already polling, stop and start again. This could happen - // if the user requested the verification email to be resent while we - // were already polling for receipt of an earlier email. - if (this.currentTimer) { - log.debug( - "startPollEmailStatus starting while existing timer is running" - ); - clearTimeout(this.currentTimer); - this.currentTimer = null; - } - - this.pollStartDate = Date.now(); - if (!currentState.whenVerifiedDeferred) { - currentState.whenVerifiedDeferred = Promise.withResolvers(); - // This deferred might not end up with any handlers (eg, if sync - // is yet to start up.) This might cause "A promise chain failed to - // handle a rejection" messages, so add an error handler directly - // on the promise to log the error. - currentState.whenVerifiedDeferred.promise.then( - () => { - log.info("the user became verified"); - // We are now ready for business. This should only be invoked once - // per setSignedInUser(), regardless of whether we've rebooted since - // setSignedInUser() was called. - this.notifyObservers(ONVERIFIED_NOTIFICATION); - }, - err => { - log.info("the wait for user verification was stopped: " + err); - } - ); - } - return this.pollEmailStatus(currentState, sessionToken, why); - }, - - // We return a promise for testing only. Other callers can ignore this, - // since verification polling continues in the background. - async pollEmailStatus(currentState, sessionToken, why) { - log.debug("entering pollEmailStatus: " + why); - if (lazy.oauthEnabled) { - log.debug("not polling for verification because oauth is enabled"); - return; - } - let nextPollMs; - try { - const response = await this.checkEmailStatus(sessionToken, { - reason: why, - }); - log.debug("checkEmailStatus -> " + JSON.stringify(response)); - if (response && response.verified) { - await this.onPollEmailSuccess(currentState); - return; - } - } catch (error) { - if (error && error.code && error.code == 401) { - let error = new Error("Verification status check failed"); - this._rejectWhenVerified(currentState, error); - return; - } - if (error && error.retryAfter) { - // If the server told us to back off, back off the requested amount. - nextPollMs = (error.retryAfter + 3) * 1000; - log.warn( - `the server rejected our email status check and told us to try again in ${nextPollMs}ms` - ); - } else { - log.error(`checkEmailStatus failed to poll`, error); - } - } - if (why == "push") { - return; - } - let pollDuration = Date.now() - this.pollStartDate; - // Polling session expired. - if (pollDuration >= this.POLL_SESSION) { - if (currentState.whenVerifiedDeferred) { - let error = new Error("User email verification timed out."); - this._rejectWhenVerified(currentState, error); - } - log.debug("polling session exceeded, giving up"); - return; - } - // Poll email status again after a short delay. - if (nextPollMs === undefined) { - let currentMinute = Math.ceil(pollDuration / 60000); - nextPollMs = - why == "start" && - currentMinute < this.VERIFICATION_POLL_START_SLOWDOWN_THRESHOLD - ? this.VERIFICATION_POLL_TIMEOUT_INITIAL - : this.VERIFICATION_POLL_TIMEOUT_SUBSEQUENT; - } - this._scheduleNextPollEmailStatus( - currentState, - sessionToken, - nextPollMs, - why - ); - }, - - // Easy-to-mock testable method - _scheduleNextPollEmailStatus(currentState, sessionToken, nextPollMs, why) { - log.debug("polling with timeout = " + nextPollMs); - this.currentTimer = setTimeout(() => { - this.pollEmailStatus(currentState, sessionToken, why); - }, nextPollMs); - }, - - async onPollEmailSuccess(currentState) { - try { - await currentState.updateUserAccountData({ verified: true }); - const accountData = await currentState.getUserAccountData(); - this._setLastUserPref(accountData.email); - if (currentState.whenVerifiedDeferred) { - currentState.whenVerifiedDeferred.resolve(accountData); - delete currentState.whenVerifiedDeferred; - } - } catch (e) { - log.error(e); - } - }, - - _rejectWhenVerified(currentState, error) { - currentState.whenVerifiedDeferred.reject(error); - delete currentState.whenVerifiedDeferred; - }, - /** * Does the actual fetch of an oauth token for getOAuthToken() * using the account session token. @@ -1554,32 +1319,11 @@ FxAccountsInternal.prototype = { /** * Sets the user to be verified in the account state, - * This prevents any polling for the user's verification state from the FxA server */ async setUserVerified() { - await this.withCurrentAccountState(async currentState => { - // TODO: setting `verified` is unnecessary if oauthEnabled - it looks at our key state. - const userData = await currentState.getUserAccountData(); - if (!userData.verified) { - await currentState.updateUserAccountData({ verified: true }); - } - }); await this.notifyObservers(ONVERIFIED_NOTIFICATION); }, - async _getVerifiedAccountOrReject() { - let data = await this.currentAccountState.getUserAccountData(); - if (!data) { - // No signed-in user - throw this._error(ERROR_NO_ACCOUNT); - } - if (!data.verified) { - // Signed-in user has not verified email - throw this._error(ERROR_UNVERIFIED_ACCOUNT); - } - return data; - }, - // _handle* methods used by push, used when the account/device status is // changed on a different device. async _handleAccountDestroyed(uid) { @@ -1760,12 +1504,5 @@ export function getFxAccountsSingleton() { } fxAccountsSingleton = new FxAccounts(); - - // XXX Bug 947061 - We need a strategy for resuming email verification after - // browser restart - fxAccountsSingleton._internal.loadAndPoll(); - return fxAccountsSingleton; } - -// `AccountState` is exported for tests. diff --git a/services/fxaccounts/FxAccountsConfig.sys.mjs b/services/fxaccounts/FxAccountsConfig.sys.mjs @@ -335,22 +335,13 @@ export var FxAccountsConfig = { return lazy.fxAccounts.getSignedInUser(); }, - _isOAuthFlow() { - return Services.prefs.getBoolPref( - "identity.fxaccounts.oauth.enabled", - false - ); - }, - async _getAuthParams() { let params = { service: SYNC_PARAM }; - if (this._isOAuthFlow()) { - const scopes = [SCOPE_APP_SYNC, SCOPE_PROFILE]; - Object.assign( - params, - await lazy.fxAccounts._internal.beginOAuthFlow(scopes) - ); - } + const scopes = [SCOPE_APP_SYNC, SCOPE_PROFILE]; + Object.assign( + params, + await lazy.fxAccounts._internal.beginOAuthFlow(scopes) + ); return params; }, }; diff --git a/services/fxaccounts/FxAccountsKeys.sys.mjs b/services/fxaccounts/FxAccountsKeys.sys.mjs @@ -2,7 +2,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs"; import { CommonUtils } from "resource://services-common/utils.sys.mjs"; import { CryptoUtils } from "moz-src:///services/crypto/modules/utils.sys.mjs"; @@ -27,20 +26,6 @@ const DEPRECATED_DERIVED_KEYS_NAMES = [ "ecosystemAnonId", ]; -const lazy = {}; - -XPCOMUtils.defineLazyPreferenceGetter( - lazy, - "oauthEnabled", - "identity.fxaccounts.oauth.enabled", - true -); - -// Is key fetching enabled/possible in the current configuration? -function keyFetchingEnabled() { - return !lazy.oauthEnabled; -} - // This scope and its associated key material were used by the old Kinto webextension // storage backend, but has since been decommissioned. It's here entirely so that we // remove the corresponding key from storage if present. We should be safe to remove it @@ -110,25 +95,7 @@ export class FxAccountsKeys { log.info("Can't get keys; user is not verified"); return false; } - - if (userData.scopedKeys && userData.scopedKeys.hasOwnProperty(scope)) { - return true; - } - - // If we have a `keyFetchToken` we can fetch `kB`. - if (userData.keyFetchToken) { - // this is a kind of defense-in-depth for our oauth flows in case something is confused. - if (!keyFetchingEnabled()) { - log.error( - "Key management confusion: we should never have a keyFetchToken in oauth flows; ignoring it" - ); - return false; - } - return true; - } - - log.info("Can't get keys; no key material or tokens available"); - return false; + return userData.scopedKeys && userData.scopedKeys.hasOwnProperty(scope); }); } diff --git a/services/fxaccounts/FxAccountsPush.sys.mjs b/services/fxaccounts/FxAccountsPush.sys.mjs @@ -179,8 +179,9 @@ FxAccountsPushService.prototype = { this.log.trace("FxAccountsPushService _onPushMessage"); if (!message.data) { // Use the empty signal to check the verification state of the account right away - this.log.debug("empty push message - checking account status"); - this.fxai.checkVerificationStatus(); + this.log.debug( + "empty push message, but oauth doesn't require checking account status - ignoring" + ); return; } let payload = message.data.json(); diff --git a/services/fxaccounts/FxAccountsWebChannel.sys.mjs b/services/fxaccounts/FxAccountsWebChannel.sys.mjs @@ -93,13 +93,6 @@ XPCOMUtils.defineLazyPreferenceGetter( XPCOMUtils.defineLazyPreferenceGetter( lazy, - "oauthEnabled", - "identity.fxaccounts.oauth.enabled", - false -); - -XPCOMUtils.defineLazyPreferenceGetter( - lazy, "allowSyncMerge", "browser.profiles.sync.allow-danger-merge", false @@ -646,14 +639,11 @@ FxAccountsWebChannelHelpers.prototype = { } else { log.debug("Webchannel is logging new a user in."); } - // There are (or were) extra fields here we don't want to actually store. + // Stuff we never want to keep after being logged in (and no longer get in 2026) 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; - } + delete accountData.keyFetchToken; + delete accountData.unwrapBKey; // 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 @@ -663,33 +653,19 @@ FxAccountsWebChannelHelpers.prototype = { ...(accountData.services ?? {}), ...existingServices, }; - delete accountData.services; - - // 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), "webchannel" ); - - if (lazy.oauthEnabled) { - // 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 { - // Note we don't persist anything in requestedServices for non oauth flows because we act on them now. - await this._fxAccounts._internal.setSignedInUser(accountData); - await this._enableRequestedServices(requestedServices); - } + delete accountData.services; + // 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); + + this.setPreviousAccountNameHashPref(accountData.email); + await this._fxAccounts._internal.setSignedInUser(accountData); log.debug("Webchannel finished logging a user in."); }, @@ -855,11 +831,7 @@ FxAccountsWebChannelHelpers.prototype = { }, _getCapabilities() { - // 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) - : []; + let engines = Array.from(CHOOSE_WHAT_TO_SYNC_ALWAYS_AVAILABLE); for (let optionalEngine of CHOOSE_WHAT_TO_SYNC_OPTIONALLY_AVAILABLE) { if ( Services.prefs.getBoolPref( diff --git a/services/fxaccounts/tests/xpcshell/head.js b/services/fxaccounts/tests/xpcshell/head.js @@ -27,27 +27,6 @@ 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.js b/services/fxaccounts/tests/xpcshell/test_accounts.js @@ -3,9 +3,6 @@ "use strict"; -const { CryptoUtils } = ChromeUtils.importESModule( - "moz-src:///services/crypto/modules/utils.sys.mjs" -); const { FxAccounts, ERROR_INVALID_ACCOUNT_STATE } = ChromeUtils.importESModule( "resource://gre/modules/FxAccounts.sys.mjs" ); @@ -17,11 +14,8 @@ const { ERRNO_INVALID_AUTH_TOKEN, ERROR_NO_ACCOUNT, OAUTH_CLIENT_ID, - ONLOGIN_NOTIFICATION, ONLOGOUT_NOTIFICATION, - ONVERIFIED_NOTIFICATION, DEPRECATED_SCOPE_ECOSYSTEM_TELEMETRY, - PREF_LAST_FXA_USER, } = ChromeUtils.importESModule( "resource://gre/modules/FxAccountsCommon.sys.mjs" ); @@ -387,285 +381,6 @@ add_test(function test_client_mock() { }); }); -// Sign in a user, and after a little while, verify the user's email. -// Right after signing in the user, we should get the 'onlogin' notification. -// Polling should detect that the email is verified, and eventually -// 'onverified' should be observed -add_test(function test_verification_poll() { - ensureOauthNotConfigured(); - let fxa = new MockFxAccounts(); - let test_user = getTestUser("francine"); - let login_notification_received = false; - - makeObserver(ONVERIFIED_NOTIFICATION, function () { - log.debug("test_verification_poll observed onverified"); - // Once email verification is complete, we will observe onverified - fxa._internal - .getUserAccountData() - .then(user => { - // And confirm that the user's state has changed - Assert.equal(user.verified, true); - Assert.equal(user.email, test_user.email); - Assert.equal( - Services.prefs.getStringPref(PREF_LAST_FXA_USER), - CryptoUtils.sha256Base64(test_user.email) - ); - Assert.ok(login_notification_received); - }) - .finally(run_next_test); - }); - - makeObserver(ONLOGIN_NOTIFICATION, function () { - log.debug("test_verification_poll observer onlogin"); - login_notification_received = true; - }); - - fxa.setSignedInUser(test_user).then(() => { - fxa._internal.getUserAccountData().then(user => { - // The user is signing in, but email has not been verified yet - Assert.equal(user.verified, false); - do_timeout(200, function () { - log.debug("Mocking verification of francine's email"); - fxa._internal.fxAccountsClient._email = test_user.email; - fxa._internal.fxAccountsClient._verified = true; - }); - }); - }); -}); - -// Sign in the user, but never verify the email. The check-email -// poll should time out. No verifiedlogin event should be observed, and the -// internal whenVerified promise should be rejected -add_test(function test_polling_timeout() { - ensureOauthNotConfigured(); - // This test could be better - the onverified observer might fire on - // somebody else's stack, and we're not making sure that we're not receiving - // such a message. In other words, this tests either failure, or success, but - // not both. - - let fxa = new MockFxAccounts(); - let test_user = getTestUser("carol"); - - let removeObserver = makeObserver(ONVERIFIED_NOTIFICATION, function () { - do_throw("We should not be getting a login event!"); - }); - - fxa._internal.POLL_SESSION = 1; - - let p = fxa._internal.whenVerified({}); - - fxa.setSignedInUser(test_user).then(() => { - p.then( - () => { - do_throw("this should not succeed"); - }, - () => { - removeObserver(); - fxa.signOut().then(run_next_test); - } - ); - }); -}); - -// For bug 1585299 - ensure we only get a single ONVERIFIED notification. -add_task(async function test_onverified_once() { - let fxa = new MockFxAccounts(); - let user = getTestUser("francine"); - - let numNotifications = 0; - - function observe() { - numNotifications += 1; - } - Services.obs.addObserver(observe, ONVERIFIED_NOTIFICATION); - - fxa._internal.POLL_SESSION = 1; - - await fxa.setSignedInUser(user); - - Assert.ok(!(await fxa.getSignedInUser()).verified, "starts unverified"); - - await fxa._internal.startPollEmailStatus( - fxa._internal.currentAccountState, - user.sessionToken, - "start" - ); - - Assert.ok(!(await fxa.getSignedInUser()).verified, "still unverified"); - - log.debug("Mocking verification of francine's email"); - fxa._internal.fxAccountsClient._email = user.email; - fxa._internal.fxAccountsClient._verified = true; - - await fxa._internal.startPollEmailStatus( - fxa._internal.currentAccountState, - user.sessionToken, - "again" - ); - - Assert.ok((await fxa.getSignedInUser()).verified, "now verified"); - - Assert.equal(numNotifications, 1, "expect exactly 1 ONVERIFIED"); - - Services.obs.removeObserver(observe, ONVERIFIED_NOTIFICATION); - await fxa.signOut(); -}); - -add_test(function test_pollEmailStatus_start_verified() { - let fxa = new MockFxAccounts(); - let test_user = getTestUser("carol"); - - fxa._internal.POLL_SESSION = 20 * 60000; - fxa._internal.VERIFICATION_POLL_TIMEOUT_INITIAL = 50000; - - fxa.setSignedInUser(test_user).then(() => { - fxa._internal.getUserAccountData().then(user => { - fxa._internal.fxAccountsClient._email = test_user.email; - fxa._internal.fxAccountsClient._verified = true; - const mock = sinon.mock(fxa._internal); - mock.expects("_scheduleNextPollEmailStatus").never(); - fxa._internal - .startPollEmailStatus( - fxa._internal.currentAccountState, - user.sessionToken, - "start" - ) - .then(() => { - mock.verify(); - mock.restore(); - run_next_test(); - }); - }); - }); -}); - -add_test(function test_pollEmailStatus_start() { - let fxa = new MockFxAccounts(); - let test_user = getTestUser("carol"); - - fxa._internal.POLL_SESSION = 20 * 60000; - fxa._internal.VERIFICATION_POLL_TIMEOUT_INITIAL = 123456; - - fxa.setSignedInUser(test_user).then(() => { - fxa._internal.getUserAccountData().then(user => { - const mock = sinon.mock(fxa._internal); - mock - .expects("_scheduleNextPollEmailStatus") - .once() - .withArgs( - fxa._internal.currentAccountState, - user.sessionToken, - 123456, - "start" - ); - fxa._internal - .startPollEmailStatus( - fxa._internal.currentAccountState, - user.sessionToken, - "start" - ) - .then(() => { - mock.verify(); - mock.restore(); - run_next_test(); - }); - }); - }); -}); - -add_test(function test_pollEmailStatus_start_subsequent() { - let fxa = new MockFxAccounts(); - let test_user = getTestUser("carol"); - - fxa._internal.POLL_SESSION = 20 * 60000; - fxa._internal.VERIFICATION_POLL_TIMEOUT_INITIAL = 123456; - fxa._internal.VERIFICATION_POLL_TIMEOUT_SUBSEQUENT = 654321; - fxa._internal.VERIFICATION_POLL_START_SLOWDOWN_THRESHOLD = -1; - - fxa.setSignedInUser(test_user).then(() => { - fxa._internal.getUserAccountData().then(user => { - const mock = sinon.mock(fxa._internal); - mock - .expects("_scheduleNextPollEmailStatus") - .once() - .withArgs( - fxa._internal.currentAccountState, - user.sessionToken, - 654321, - "start" - ); - fxa._internal - .startPollEmailStatus( - fxa._internal.currentAccountState, - user.sessionToken, - "start" - ) - .then(() => { - mock.verify(); - mock.restore(); - run_next_test(); - }); - }); - }); -}); - -add_test(function test_pollEmailStatus_browser_startup() { - let fxa = new MockFxAccounts(); - let test_user = getTestUser("carol"); - - fxa._internal.POLL_SESSION = 20 * 60000; - fxa._internal.VERIFICATION_POLL_TIMEOUT_SUBSEQUENT = 654321; - - fxa.setSignedInUser(test_user).then(() => { - fxa._internal.getUserAccountData().then(user => { - const mock = sinon.mock(fxa._internal); - mock - .expects("_scheduleNextPollEmailStatus") - .once() - .withArgs( - fxa._internal.currentAccountState, - user.sessionToken, - 654321, - "browser-startup" - ); - fxa._internal - .startPollEmailStatus( - fxa._internal.currentAccountState, - user.sessionToken, - "browser-startup" - ) - .then(() => { - mock.verify(); - mock.restore(); - run_next_test(); - }); - }); - }); -}); - -add_test(function test_pollEmailStatus_push() { - let fxa = new MockFxAccounts(); - let test_user = getTestUser("carol"); - - fxa.setSignedInUser(test_user).then(() => { - fxa._internal.getUserAccountData().then(user => { - const mock = sinon.mock(fxa._internal); - mock.expects("_scheduleNextPollEmailStatus").never(); - fxa._internal - .startPollEmailStatus( - fxa._internal.currentAccountState, - user.sessionToken, - "push" - ) - .then(() => { - mock.verify(); - mock.restore(); - run_next_test(); - }); - }); - }); -}); - add_test(function test_getKeyForScope() { let fxa = new MockFxAccounts(); let user = getTestUser("eusebius"); @@ -696,7 +411,6 @@ add_test(function test_getKeyForScope() { }); add_task(async function test_oauth_verification() { - ensureOauthConfigured(); let fxa = new MockFxAccounts(); let user = getTestUser("eusebius"); user.verified = true; @@ -711,8 +425,6 @@ add_task(async function test_oauth_verification() { fetched = await fxa.getSignedInUser(); Assert.ok(fetched.verified); - - resetOauthConfig(); }); add_task( @@ -776,76 +488,6 @@ add_task( } ); -add_task(async function test_getKeyForScope_nonexistent_account() { - let fxa = new MockFxAccounts(); - let bismarck = getTestUser("bismarck"); - - let client = fxa._internal.fxAccountsClient; - client.accountStatus = () => Promise.resolve(false); - client.sessionStatus = () => Promise.resolve(false); - client.accountKeys = () => { - return Promise.reject({ - code: 401, - errno: ERRNO_INVALID_AUTH_TOKEN, - }); - }; - - await fxa.setSignedInUser(bismarck); - - let promiseLogout = new Promise(resolve => { - makeObserver(ONLOGOUT_NOTIFICATION, function () { - log.debug("test_getKeyForScope_nonexistent_account observed logout"); - resolve(); - }); - }); - - await Assert.rejects(fxa.keys.getKeyForScope(SCOPE_APP_SYNC), err => { - Assert.equal(err.message, ERROR_INVALID_ACCOUNT_STATE); - return true; // expected error - }); - - await promiseLogout; - - let user = await fxa._internal.getUserAccountData(); - Assert.equal(user, null); -}); - -// getKeyForScope with invalid keyFetchToken should delete keyFetchToken from storage -add_task(async function test_getKeyForScope_invalid_token() { - let fxa = new MockFxAccounts(); - let yusuf = getTestUser("yusuf"); - - let client = fxa._internal.fxAccountsClient; - client.accountStatus = () => Promise.resolve(true); // account exists. - client.sessionStatus = () => Promise.resolve(false); // session is invalid. - client.accountKeys = () => { - return Promise.reject({ - code: 401, - errno: ERRNO_INVALID_AUTH_TOKEN, - }); - }; - - await fxa.setSignedInUser(yusuf); - let user = await fxa._internal.getUserAccountData(); - Assert.notEqual(user.encryptedSendTabKeys, null); - - try { - await fxa.keys.getKeyForScope(SCOPE_APP_SYNC); - Assert.ok(false); - } catch (err) { - Assert.equal(err.code, 401); - Assert.equal(err.errno, ERRNO_INVALID_AUTH_TOKEN); - } - - user = await fxa._internal.getUserAccountData(); - Assert.equal(user.email, yusuf.email); - Assert.equal(user.keyFetchToken, null); - // We verify that encryptedSendTabKeys are also wiped - // when a user's credentials are wiped - Assert.equal(user.encryptedSendTabKeys, null); - await fxa._internal.abortExistingFlow(); -}); - // Test vectors from // https://wiki.mozilla.org/Identity/AttachedServices/KeyServerProtocol#Test_Vectors add_task(async function test_getKeyForScope_oldsync() { @@ -1008,46 +650,6 @@ add_test(function test_fetchAndUnwrapAndDeriveKeys_no_token() { }); }); -// Alice (User A) signs up but never verifies her email. Then Bob (User B) -// signs in with a verified email. Ensure that no sign-in events are triggered -// on Alice's behalf. In the end, Bob should be the signed-in user. -add_test(function test_overlapping_signins() { - ensureOauthNotConfigured(); - let fxa = new MockFxAccounts(); - let alice = getTestUser("alice"); - let bob = getTestUser("bob"); - - makeObserver(ONVERIFIED_NOTIFICATION, function () { - log.debug("test_overlapping_signins observed onverified"); - // Once email verification is complete, we will observe onverified - fxa._internal.getUserAccountData().then(user => { - Assert.equal(user.email, bob.email); - Assert.equal(user.verified, true); - run_next_test(); - }); - }); - - // Alice is the user signing in; her email is unverified. - fxa.setSignedInUser(alice).then(() => { - log.debug("Alice signing in ..."); - fxa._internal.getUserAccountData().then(user => { - Assert.equal(user.email, alice.email); - Assert.equal(user.verified, false); - log.debug("Alice has not verified her email ..."); - - // Now Bob signs in instead and actually verifies his email - log.debug("Bob signing in ..."); - fxa.setSignedInUser(bob).then(() => { - do_timeout(200, function () { - // Mock email verification ... - log.debug("Bob verifying his email ..."); - fxa._internal.fxAccountsClient._verified = true; - }); - }); - }); - }); -}); - add_task(async function test_resend_email_not_signed_in() { let fxa = new MockFxAccounts(); @@ -1128,19 +730,10 @@ add_test(function test_resend_email() { let fxa = new MockFxAccounts(); let alice = getTestUser("alice"); - let initialState = fxa._internal.currentAccountState; - // Alice is the user signing in; her email is unverified. fxa.setSignedInUser(alice).then(() => { log.debug("Alice signing in"); - // We're polling for the first email - Assert.notStrictEqual(fxa._internal.currentAccountState, initialState); - let aliceState = fxa._internal.currentAccountState; - - // The polling timer is ticking - Assert.greater(fxa._internal.currentTimer, 0); - fxa._internal.getUserAccountData().then(user => { Assert.equal(user.email, alice.email); Assert.equal(user.verified, false); @@ -1150,14 +743,6 @@ add_test(function test_resend_email() { // Mock server response; ensures that the session token actually was // passed to the client to make the hawk call Assert.equal(result, "alice's session token"); - - // Timer was not restarted - Assert.strictEqual(fxa._internal.currentAccountState, aliceState); - - // Timer is still ticking - Assert.greater(fxa._internal.currentTimer, 0); - - // Ok abort polling before we go on to the next test fxa._internal.abortExistingFlow(); run_next_test(); }); @@ -1671,34 +1256,6 @@ add_task(async function test_getSignedInUserProfile_error_uses_account_data() { Assert.ok(teardownCalled); }); -add_task(async function test_checkVerificationStatusFailed() { - let fxa = new MockFxAccounts(); - let alice = getTestUser("alice"); - alice.verified = true; - - let client = fxa._internal.fxAccountsClient; - client.recoveryEmailStatus = () => { - return Promise.reject({ - code: 401, - errno: ERRNO_INVALID_AUTH_TOKEN, - }); - }; - client.accountStatus = () => Promise.resolve(true); - client.sessionStatus = () => Promise.resolve(false); - - await fxa.setSignedInUser(alice); - let user = await fxa._internal.getUserAccountData(); - Assert.notEqual(alice.sessionToken, null); - Assert.equal(user.email, alice.email); - Assert.equal(user.verified, true); - - await fxa._internal.checkVerificationStatus(); - - user = await fxa._internal.getUserAccountData(); - Assert.equal(user.email, alice.email); - Assert.equal(user.sessionToken, null); -}); - add_task(async function test_flushLogFile() { _("Tests flushLogFile"); let account = await MakeFxAccounts(); diff --git a/services/fxaccounts/tests/xpcshell/test_accounts_config.js b/services/fxaccounts/tests/xpcshell/test_accounts_config.js @@ -6,42 +6,6 @@ const { FxAccounts } = ChromeUtils.importESModule( "resource://gre/modules/FxAccounts.sys.mjs" ); -const { CLIENT_IS_THUNDERBIRD } = ChromeUtils.importESModule( - "resource://gre/modules/FxAccountsCommon.sys.mjs" -); - -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( - "identity.fxaccounts.remote.root", - "http://example.com/" - ); - Assert.equal( - await FxAccounts.config.promiseConnectAccountURI("test"), - "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"), - CLIENT_IS_THUNDERBIRD ? "8269bacd7bbc7f80" : "5882386c6d801776" - ); - Assert.equal(url.searchParams.get("response_type"), "code"); - - Services.prefs.clearUserPref("identity.fxaccounts.remote.root"); - Services.prefs.clearUserPref("identity.fxaccounts.allowHttp"); - resetOauthConfig(); - } -); add_task(async function test_non_https_remote_server_uri() { Services.prefs.setStringPref( @@ -75,31 +39,3 @@ 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(); - Services.prefs.setStringPref("identity.fxaccounts.autoconfig.uri", ""); - Services.prefs.setStringPref( - "identity.fxaccounts.remote.root", - "https://accounts.firefox.com/" - ); - 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"), - CLIENT_IS_THUNDERBIRD ? "8269bacd7bbc7f80" : "5882386c6d801776" - ); - Assert.equal(url.searchParams.get("service"), "custom-service"); - resetOauthConfig(); -}); diff --git a/services/fxaccounts/tests/xpcshell/test_accounts_device_registration.js b/services/fxaccounts/tests/xpcshell/test_accounts_device_registration.js @@ -13,7 +13,6 @@ const { FxAccountsDevice } = ChromeUtils.importESModule( "resource://gre/modules/FxAccountsDevice.sys.mjs" ); const { - CLIENT_IS_THUNDERBIRD, ERRNO_DEVICE_SESSION_CONFLICT, ERRNO_TOO_MANY_CLIENT_REQUESTS, ERRNO_UNKNOWN_DEVICE, @@ -626,56 +625,6 @@ add_task( } ); -add_task( - { skip_if: () => CLIENT_IS_THUNDERBIRD }, - async function test_verification_updates_registration() { - const deviceName = "foo"; - ensureOauthNotConfigured(); - - const credentials = getTestUser("baz"); - const fxa = await MockFxAccounts(credentials, { - id: "device-id", - name: deviceName, - }); - - // We should already have a device registration, but without send-tab due to - // our inability to fetch keys for an unverified users. - const state = fxa._internal.currentAccountState; - const { device } = await state.getUserAccountData(); - Assert.equal(device.registeredCommandsKeys.length, 0); - - let updatePromise = new Promise(resolve => { - const old_registerOrUpdateDevice = - fxa.device._registerOrUpdateDevice.bind(fxa.device); - fxa.device._registerOrUpdateDevice = async function ( - currentState, - signedInUser - ) { - await old_registerOrUpdateDevice(currentState, signedInUser); - fxa.device._registerOrUpdateDevice = old_registerOrUpdateDevice; - resolve(); - }; - }); - - fxa._internal.checkEmailStatus = async function () { - credentials.verified = true; - return credentials; - }; - - await updatePromise; - - const { - device: newDevice, - encryptedSendTabKeys, - encryptedCloseTabKeys, - } = await state.getUserAccountData(); - Assert.equal(newDevice.registeredCommandsKeys.length, 2); - Assert.notEqual(encryptedSendTabKeys, null); - Assert.notEqual(encryptedCloseTabKeys, null); - await fxa.signOut(true); - } -); - add_task(async function test_devicelist_pushendpointexpired() { const deviceId = "mydeviceid"; const credentials = getTestUser("baz"); diff --git a/services/fxaccounts/tests/xpcshell/test_push_service.js b/services/fxaccounts/tests/xpcshell/test_push_service.js @@ -140,31 +140,6 @@ add_test(function observeLogout() { pushService.observe(null, ONLOGOUT_NOTIFICATION); }); -add_test(function observePushTopicVerify() { - let emptyMsg = { - QueryInterface() { - return this; - }, - }; - let customAccounts = Object.assign(mockFxAccounts, { - checkVerificationStatus() { - // checking verification status on push messages without data - run_next_test(); - }, - }); - - let pushService = new FxAccountsPushService({ - pushService: mockPushService, - fxai: customAccounts, - }); - - pushService.observe( - emptyMsg, - mockPushService.pushTopic, - FXA_PUSH_SCOPE_ACCOUNT_UPDATE - ); -}); - add_test(function observePushTopicDeviceConnected() { let msg = { data: { diff --git a/services/fxaccounts/tests/xpcshell/test_web_channel.js b/services/fxaccounts/tests/xpcshell/test_web_channel.js @@ -3,14 +3,8 @@ "use strict"; -const { - CLIENT_IS_THUNDERBIRD, - ON_PROFILE_CHANGE_NOTIFICATION, - WEBCHANNEL_ID, - log, -} = ChromeUtils.importESModule( - "resource://gre/modules/FxAccountsCommon.sys.mjs" -); +const { ON_PROFILE_CHANGE_NOTIFICATION, WEBCHANNEL_ID, log } = + ChromeUtils.importESModule("resource://gre/modules/FxAccountsCommon.sys.mjs"); const { CryptoUtils } = ChromeUtils.importESModule( "moz-src:///services/crypto/modules/utils.sys.mjs" ); @@ -677,50 +671,6 @@ add_task(async function test_helpers_login_another_user_signed_in() { Assert.ok(helpers._disconnect.called); }); -add_task( - async function test_helpers_login_dont_set_previous_account_name_hash_for_unverified_emails() { - let helpers = new FxAccountsWebChannelHelpers({ - fxAccounts: { - _internal: { - setSignedInUser() { - return new Promise(resolve => { - // previously signed in user preference should not be updated. - Assert.equal( - helpers.getPreviousAccountNameHashPref(), - CryptoUtils.sha256Base64("lastuser@testuser.com") - ); - resolve(); - }); - }, - }, - getSignedInUser() { - return Promise.resolve(null); - }, - telemetry: { - recordConnection() {}, - }, - }, - weaveXPCOM: { - whenLoaded() {}, - Weave: { - Service: { - configure() {}, - }, - }, - }, - }); - - // ensure the previous account pref is overwritten. - helpers.setPreviousAccountNameHashPref("lastuser@testuser.com"); - - await helpers.login({ - email: "newuser@testuser.com", - verifiedCanLinkAccount: true, - customizeSync: false, - }); - } -); - add_task(async function test_helpers_login_with_customize_sync() { let helpers = new FxAccountsWebChannelHelpers({ fxAccounts: { @@ -764,239 +714,7 @@ add_task(async function test_helpers_login_with_customize_sync() { ); }); -add_task( - { skip_if: () => CLIENT_IS_THUNDERBIRD }, - async function test_helpers_login_with_customize_sync_and_declined_engines() { - ensureOauthNotConfigured(); - let configured = false; - let helpers = new FxAccountsWebChannelHelpers({ - fxAccounts: { - _internal: { - setSignedInUser(accountData) { - return new Promise(resolve => { - // ensure fxAccounts is informed of the new user being signed in. - Assert.equal(accountData.email, "testuser@testuser.com"); - - // customizeSync should be stripped in the data. - Assert.equal(false, "customizeSync" in accountData); - Assert.equal(false, "services" in accountData); - resolve(); - }); - }, - }, - getSignedInUser() { - return Promise.resolve(null); - }, - telemetry: { - recordConnection: sinon.spy(), - }, - }, - weaveXPCOM: { - whenLoaded() {}, - Weave: { - Service: { - configure() { - configured = true; - }, - }, - }, - }, - }); - - Assert.equal( - Services.prefs.getBoolPref("services.sync.engine.addons"), - true - ); - Assert.equal( - Services.prefs.getBoolPref("services.sync.engine.bookmarks"), - true - ); - Assert.equal( - Services.prefs.getBoolPref("services.sync.engine.history"), - true - ); - Assert.equal( - Services.prefs.getBoolPref("services.sync.engine.passwords"), - true - ); - Assert.equal( - Services.prefs.getBoolPref("services.sync.engine.prefs"), - true - ); - Assert.equal(Services.prefs.getBoolPref("services.sync.engine.tabs"), true); - await helpers.login({ - email: "testuser@testuser.com", - verifiedCanLinkAccount: true, - customizeSync: true, - services: { - sync: { - offeredEngines: [ - "addons", - "bookmarks", - "history", - "passwords", - "prefs", - ], - declinedEngines: ["addons", "prefs"], - }, - }, - }); - Assert.equal( - Services.prefs.getBoolPref("services.sync.engine.addons"), - false - ); - Assert.equal( - Services.prefs.getBoolPref("services.sync.engine.bookmarks"), - true - ); - Assert.equal( - Services.prefs.getBoolPref("services.sync.engine.history"), - true - ); - Assert.equal( - Services.prefs.getBoolPref("services.sync.engine.passwords"), - true - ); - Assert.equal( - Services.prefs.getBoolPref("services.sync.engine.prefs"), - false - ); - Assert.equal(Services.prefs.getBoolPref("services.sync.engine.tabs"), true); - Assert.ok(configured, "sync was configured"); - Assert.ok( - helpers._fxAccounts.telemetry.recordConnection.calledWith( - ["sync"], - "webchannel" - ) - ); - } -); - -add_task(async function test_helpers_login_with_offered_sync_engines() { - let helpers; - let configured = false; - const setSignedInUserCalled = new Promise(resolve => { - helpers = new FxAccountsWebChannelHelpers({ - fxAccounts: { - _internal: { - async setSignedInUser(accountData) { - resolve(accountData); - }, - }, - getSignedInUser() { - return Promise.resolve(null); - }, - telemetry: { - recordConnection() {}, - }, - }, - weaveXPCOM: { - whenLoaded() {}, - Weave: { - Service: { - configure() { - configured = true; - }, - }, - }, - }, - }); - }); - - Services.prefs.setBoolPref("services.sync.engine.creditcards", false); - Services.prefs.setBoolPref("services.sync.engine.addresses", false); - - await helpers.login({ - email: "testuser@testuser.com", - verifiedCanLinkAccount: true, - customizeSync: true, - services: { - sync: { - declinedEngines: ["addresses"], - offeredEngines: ["creditcards", "addresses"], - }, - }, - }); - - const accountData = await setSignedInUserCalled; - - // ensure fxAccounts is informed of the new user being signed in. - equal(accountData.email, "testuser@testuser.com"); - - // services should be stripped in the data. - ok(!("services" in accountData)); - // credit cards was offered but not declined. - equal(Services.prefs.getBoolPref("services.sync.engine.creditcards"), true); - // addresses was offered and explicitely declined. - equal(Services.prefs.getBoolPref("services.sync.engine.addresses"), false); - ok(configured); -}); - -add_task(async function test_helpers_login_nothing_offered() { - let helpers; - let configured = false; - const setSignedInUserCalled = new Promise(resolve => { - helpers = new FxAccountsWebChannelHelpers({ - fxAccounts: { - _internal: { - async setSignedInUser(accountData) { - resolve(accountData); - }, - }, - getSignedInUser() { - return Promise.resolve(null); - }, - telemetry: { - recordConnection() {}, - }, - }, - weaveXPCOM: { - whenLoaded() {}, - Weave: { - Service: { - configure() { - configured = true; - }, - }, - }, - }, - }); - }); - - // doesn't really matter if it's *all* engines... - const allEngines = [ - "addons", - "addresses", - "bookmarks", - "creditcards", - "history", - "passwords", - "prefs", - ]; - for (let name of allEngines) { - Services.prefs.clearUserPref("services.sync.engine." + name); - } - - await helpers.login({ - email: "testuser@testuser.com", - verifiedCanLinkAccount: true, - services: { - sync: {}, - }, - }); - - const accountData = await setSignedInUserCalled; - // ensure fxAccounts is informed of the new user being signed in. - equal(accountData.email, "testuser@testuser.com"); - - for (let name of allEngines) { - Assert.ok(!Services.prefs.prefHasUserValue("services.sync.engine." + name)); - } - Assert.ok(configured); -}); - add_task(async function test_helpers_persist_requested_services() { - ensureOauthConfigured(); let accountData = null; const helpers = new FxAccountsWebChannelHelpers({ fxAccounts: { @@ -1053,8 +771,6 @@ add_task(async function test_helpers_persist_requested_services() { sync: { important: true }, new: { name: "opted in" }, }); - - resetOauthConfig(); }); add_test(function test_helpers_open_sync_preferences() { @@ -1075,40 +791,6 @@ add_test(function test_helpers_open_sync_preferences() { helpers.openSyncPreferences(mockBrowser, "fxa:verification_complete"); }); -add_task(async function test_helpers_getFxAStatus_extra_engines() { - let helpers = new FxAccountsWebChannelHelpers({ - fxAccounts: { - _internal: { - getUserAccountData() { - return Promise.resolve({ - email: "testuser@testuser.com", - sessionToken: "sessionToken", - uid: "uid", - verified: true, - }); - }, - }, - }, - privateBrowsingUtils: { - isBrowserPrivate: () => true, - }, - }); - - ensureOauthNotConfigured(); - Services.prefs.setBoolPref( - "services.sync.engine.creditcards.available", - true - ); - // Not defining "services.sync.engine.addresses.available" on purpose. - - 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: { @@ -1130,7 +812,6 @@ add_task(async function test_helpers_getFxAStatus_engines_oauth() { // disable the "addresses" engine. Services.prefs.setBoolPref("services.sync.engine.addresses.available", false); - ensureOauthConfigured(); let fxaStatus = await helpers.getFxaStatus("sync", mockSendingContext); ok(!!fxaStatus); ok(!!fxaStatus.signedInUser); @@ -1158,8 +839,6 @@ add_task(async function test_helpers_getFxAStatus_engines_oauth() { "prefs", "tabs", ]); - - resetOauthConfig(); }); add_task(async function test_helpers_getFxaStatus_allowed_signedInUser() { diff --git a/services/sync/tests/unit/test_sync_auth_manager.js b/services/sync/tests/unit/test_sync_auth_manager.js @@ -20,12 +20,7 @@ const { FxAccounts } = ChromeUtils.importESModule( const { FxAccountsClient } = ChromeUtils.importESModule( "resource://gre/modules/FxAccountsClient.sys.mjs" ); -const { - ERRNO_INVALID_AUTH_TOKEN, - ONLOGIN_NOTIFICATION, - ONVERIFIED_NOTIFICATION, - SCOPE_APP_SYNC, -} = ChromeUtils.importESModule( +const { ERRNO_INVALID_AUTH_TOKEN, SCOPE_APP_SYNC } = ChromeUtils.importESModule( "resource://gre/modules/FxAccountsCommon.sys.mjs" ); const { Service } = ChromeUtils.importESModule( @@ -370,50 +365,6 @@ add_task(async function test_ensureLoggedIn() { Assert.equal(Status.login, LOGIN_SUCCEEDED, "final ensureLoggedIn worked"); }); -add_task(async function test_syncState() { - // Avoid polling for an unverified user. - let identityConfig = makeIdentityConfig(); - let fxaInternal = makeFxAccountsInternalMock(identityConfig); - fxaInternal.startVerifiedCheck = () => {}; - configureFxAccountIdentity( - globalSyncAuthManager, - globalIdentityConfig, - fxaInternal - ); - - // arrange for no logged in user. - let fxa = globalSyncAuthManager._fxaService; - let signedInUser = - fxa._internal.currentAccountState.storageManager.accountData; - fxa._internal.currentAccountState.storageManager.accountData = null; - await Assert.rejects( - globalSyncAuthManager._ensureValidToken(true), - /no user is logged in/, - "expecting rejection due to no user" - ); - // Restore to an unverified user. - Services.prefs.setStringPref("services.sync.username", signedInUser.email); - signedInUser.verified = false; - fxa._internal.currentAccountState.storageManager.accountData = signedInUser; - Status.login = LOGIN_FAILED_LOGIN_REJECTED; - // The sync_auth observers are async, so call them directly. - await globalSyncAuthManager.observe(null, ONLOGIN_NOTIFICATION, ""); - Assert.equal( - Status.login, - LOGIN_FAILED_LOGIN_REJECTED, - "should not have changed the login state for an unverified user" - ); - - // now pretend the user because verified. - signedInUser.verified = true; - await globalSyncAuthManager.observe(null, ONVERIFIED_NOTIFICATION, ""); - Assert.equal( - Status.login, - LOGIN_SUCCEEDED, - "should have changed the login state to success" - ); -}); - add_task(async function test_tokenExpiration() { _("SyncAuthManager notices token expiration:"); let bimExp = new SyncAuthManager(); @@ -727,185 +678,6 @@ add_task(async function test_getHAWKErrors() { ); }); -add_task(async function test_getGetKeysFailing401() { - _("SyncAuthManager correctly handles 401 responses fetching keys."); - if (Services.prefs.getBoolPref("identity.fxaccounts.oauth.enabled", false)) { - return; - } - - _("Arrange for a 401 - Sync should reflect an auth error."); - let config = makeIdentityConfig(); - // We want no scopedKeys so we attempt to fetch them. - delete config.fxaccount.user.scopedKeys; - config.fxaccount.user.keyFetchToken = "keyfetchtoken"; - await initializeIdentityWithHAWKResponseFactory( - config, - function (method, data, uri) { - Assert.equal(method, "get"); - Assert.equal(uri, "http://mockedserver:9999/account/keys"); - return { - status: 401, - headers: { "content-type": "application/json" }, - body: "{}", - }; - } - ); - Assert.equal(Status.login, LOGIN_FAILED_LOGIN_REJECTED, "login was rejected"); -}); - -add_task(async function test_getGetKeysFailing503() { - _("SyncAuthManager correctly handles 5XX responses fetching keys."); - if (Services.prefs.getBoolPref("identity.fxaccounts.oauth.enabled", false)) { - return; - } - - _("Arrange for a 503 - Sync should reflect a network error."); - let config = makeIdentityConfig(); - // We want no scopedKeys so we attempt to fetch them. - delete config.fxaccount.user.scopedKeys; - config.fxaccount.user.keyFetchToken = "keyfetchtoken"; - await initializeIdentityWithHAWKResponseFactory( - config, - function (method, data, uri) { - Assert.equal(method, "get"); - Assert.equal(uri, "http://mockedserver:9999/account/keys"); - return { - status: 503, - headers: { "content-type": "application/json" }, - body: "{}", - }; - } - ); - Assert.equal( - Status.login, - LOGIN_FAILED_NETWORK_ERROR, - "state reflects network error" - ); -}); - -add_task(async function test_getKeysMissing() { - _( - "SyncAuthManager correctly handles getKeyForScope succeeding but not returning the key." - ); - if (Services.prefs.getBoolPref("identity.fxaccounts.oauth.enabled", false)) { - return; - } - - let syncAuthManager = new SyncAuthManager(); - let identityConfig = makeIdentityConfig(); - // our mock identity config already has scopedKeys remove them or we never - // try and fetch them. - delete identityConfig.fxaccount.user.scopedKeys; - identityConfig.fxaccount.user.keyFetchToken = "keyFetchToken"; - - configureFxAccountIdentity(syncAuthManager, identityConfig); - - // Mock a fxAccounts object - let fxa = new FxAccounts({ - fxAccountsClient: new MockFxAccountsClient(), - newAccountState(credentials) { - // We only expect this to be called with null indicating the (mock) - // storage should be read. - if (credentials) { - throw new Error("Not expecting to have credentials passed"); - } - let storageManager = new MockFxaStorageManager(); - storageManager.initialize(identityConfig.fxaccount.user); - return new AccountState(storageManager); - }, - }); - fxa.getOAuthTokenAndKey = () => { - // And the keys object with a mock that returns no keys. - return Promise.resolve({ key: null, token: "fake token" }); - }; - syncAuthManager._fxaService = fxa; - - await Assert.rejects( - syncAuthManager._ensureValidToken(), - /browser does not have the sync key, cannot sync/ - ); -}); - -add_task(async function test_getKeysUnexpecedError() { - _( - "SyncAuthManager correctly handles getKeyForScope throwing an unexpected error." - ); - - if (Services.prefs.getBoolPref("identity.fxaccounts.oauth.enabled", false)) { - return; - } - - let syncAuthManager = new SyncAuthManager(); - let identityConfig = makeIdentityConfig(); - // our mock identity config already has scopedKeys - remove them or we never - // try and fetch them. - delete identityConfig.fxaccount.user.scopedKeys; - identityConfig.fxaccount.user.keyFetchToken = "keyFetchToken"; - - configureFxAccountIdentity(syncAuthManager, identityConfig); - - // Mock a fxAccounts object - let fxa = new FxAccounts({ - fxAccountsClient: new MockFxAccountsClient(), - newAccountState(credentials) { - // We only expect this to be called with null indicating the (mock) - // storage should be read. - if (credentials) { - throw new Error("Not expecting to have credentials passed"); - } - let storageManager = new MockFxaStorageManager(); - storageManager.initialize(identityConfig.fxaccount.user); - return new AccountState(storageManager); - }, - }); - - fxa.getOAuthTokenAndKey = () => { - return Promise.reject("well that was unexpected"); - }; - - syncAuthManager._fxaService = fxa; - - await Assert.rejects( - syncAuthManager._ensureValidToken(), - /well that was unexpected/ - ); -}); - -add_task(async function test_signedInUserMissing() { - _( - "SyncAuthManager detects getSignedInUser returning incomplete account data" - ); - - let syncAuthManager = new SyncAuthManager(); - // Delete stored keys and the key fetch token. - delete globalIdentityConfig.fxaccount.user.scopedKeys; - delete globalIdentityConfig.fxaccount.user.keyFetchToken; - - configureFxAccountIdentity(syncAuthManager, globalIdentityConfig); - - let fxa = new FxAccounts({ - fetchAndUnwrapKeys() { - return Promise.resolve({}); - }, - fxAccountsClient: new MockFxAccountsClient(), - newAccountState(credentials) { - // We only expect this to be called with null indicating the (mock) - // storage should be read. - if (credentials) { - throw new Error("Not expecting to have credentials passed"); - } - let storageManager = new MockFxaStorageManager(); - storageManager.initialize(globalIdentityConfig.fxaccount.user); - return new AccountState(storageManager); - }, - }); - - syncAuthManager._fxaService = fxa; - - let status = await syncAuthManager.unlockAndVerifyAuthState(); - Assert.equal(status, LOGIN_FAILED_LOGIN_REJECTED); -}); - // End of tests // Utility functions follow diff --git a/services/sync/tps/extensions/tps/resource/auth/fxaccounts.sys.mjs b/services/sync/tps/extensions/tps/resource/auth/fxaccounts.sys.mjs @@ -3,7 +3,6 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ import { Log } from "resource://gre/modules/Log.sys.mjs"; -import { clearTimeout, setTimeout } from "resource://gre/modules/Timer.sys.mjs"; import { getFxAccountsSingleton } from "resource://gre/modules/FxAccounts.sys.mjs"; @@ -36,23 +35,6 @@ export var Authentication = { return null; }, - async shortWaitForVerification(ms) { - let userData = await this.getSignedInUser(); - let timeoutID; - let timeoutPromise = new Promise(resolve => { - timeoutID = setTimeout(() => { - Logger.logInfo(`Warning: no verification after ${ms}ms.`); - resolve(); - }, ms); - }); - await Promise.race([ - fxAccounts.whenVerified(userData).finally(() => clearTimeout(timeoutID)), - timeoutPromise, - ]); - userData = await this.getSignedInUser(); - return userData && userData.verified; - }, - async _openVerificationPage(uri) { let mainWindow = Services.wm.getMostRecentWindow("navigator:browser"); let newtab = mainWindow.gBrowser.addWebTab(uri); @@ -60,9 +42,9 @@ export var Authentication = { await new Promise(resolve => { win.addEventListener("loadend", resolve, { once: true }); }); - let didVerify = await this.shortWaitForVerification(10000); + Logger.logError("You are in a sad place - not waiting for verification."); mainWindow.gBrowser.removeTab(newtab); - return didVerify; + return false; }, async _completeVerification(user) { @@ -73,47 +55,10 @@ export var Authentication = { ); return false; } - Logger.logInfo("Fetching mail (from restmail) for user " + username); - let restmailURI = `https://www.restmail.net/mail/${encodeURIComponent( - username - )}`; - let triedAlready = new Set(); - const tries = 10; - const normalWait = 2000; - for (let i = 0; i < tries; ++i) { - let resp = await fetch(restmailURI); - let messages = await resp.json(); - // Sort so that the most recent emails are first. - messages.sort((a, b) => new Date(b.receivedAt) - new Date(a.receivedAt)); - for (let m of messages) { - // We look for a link that has a x-link that we haven't yet tried. - if (!m.headers["x-link"] || triedAlready.has(m.headers["x-link"])) { - continue; - } - let confirmLink = m.headers["x-link"]; - triedAlready.add(confirmLink); - Logger.logInfo("Trying confirmation link " + confirmLink); - try { - if (await this._openVerificationPage(confirmLink)) { - return true; - } - } catch (e) { - Logger.logInfo( - "Warning: Failed to follow confirmation link: " + - Log.exceptionStr(e) - ); - } - } - if (i === 0) { - // first time through after failing we'll do this. - await fxAccounts.resendVerificationEmail(); - } - if (await this.shortWaitForVerification(normalWait)) { - return true; - } - } - // One last try. - return this.shortWaitForVerification(normalWait); + Logger.logError( + "You are in a sad place - confirmation links are no longer a thing" + ); + return false; }, async deleteEmail(user) { diff --git a/toolkit/components/passwordmgr/test/browser/browser_relay_signup_flow_showToAllBrowsers.js b/toolkit/components/passwordmgr/test/browser/browser_relay_signup_flow_showToAllBrowsers.js @@ -17,11 +17,7 @@ registerCleanupFunction(() => { add_setup(async () => { await SpecialPowers.pushPrefEnv({ - set: [ - ["signon.firefoxRelay.showToAllBrowsers", true], - ["identity.fxaccounts.oauth.enabled", false], - ["identity.fxaccounts.contextParam", "fx_desktop_v3"], - ], + set: [["signon.firefoxRelay.showToAllBrowsers", true]], }); }); @@ -377,18 +373,19 @@ add_task( async function test_unauthenticated_browser_use_email_mask_opens_fxa_signin() { // We need the configured signup url to set up a mock server to respond to // the proper path value. Note: this test is effectively hard-coded to the "control" variation + const relayParams = { + service: "relay", + entrypoint_experiment: "first_offer_version", + entrypoint_variation: "control", + utm_source: "relay-integration", + utm_medium: "firefox-desktop", + utm_campaign: "first_offer_version", + utm_content: "control", + }; const fxaSigninUrlString = await gFxAccounts.constructor.config.promiseConnectAccountURI( "relay_integration", - { - service: "relay", - entrypoint_experiment: "first_offer_version", - entrypoint_variation: "control", - utm_source: "relay-integration", - utm_medium: "firefox-desktop", - utm_campaign: "first_offer_version", - utm_content: "control", - } + relayParams ); const fxaSigninURL = new URL(fxaSigninUrlString); // Now that we have a URL object, we can use its components @@ -429,9 +426,28 @@ add_task( const primaryButton = notificationPopup.querySelector( "button.popup-notification-primary-button" ); + + // oauth makes checking the url as a string difficult. + let isSigninUrl = href => { + const url = new URL(href); + if ( + url.protocol != mockFxaServerURL.protocol || + url.hostname != mockFxaServerURL.hostname || + url.port != mockFxaServerURL.port + ) { + return false; + } + for (const [name, value] of Object.entries(relayParams)) { + if (url.searchParams.get(name) != value) { + return false; + } + } + return true; + }; + const newTabPromise = BrowserTestUtils.waitForNewTab( gBrowser, - mockFxaServerURL.href, + isSigninUrl, true ); @@ -439,9 +455,8 @@ add_task( const newTab = await newTabPromise; const loadedUrl = newTab.linkedBrowser.currentURI.spec; - Assert.equal( - loadedUrl, - mockFxaServerURL.href, + Assert.ok( + isSigninUrl(loadedUrl), "Clicking Use email mask should open a new tab to FXA sign-in." ); BrowserTestUtils.removeTab(newTab);