commit 6ade104ae9b7fcfb8a9221a315bb7578de92fe53
parent fbc7e6b9852d07af0a7239e5c0db51f4983dc3c4
Author: Sammy Khamis <skhamis@mozilla.com>
Date: Thu, 18 Dec 2025 01:37:28 +0000
Bug 2006503 - Users turning on sync with no password should be directed to set_password r=markh,sync-reviewers
Differential Revision: https://phabricator.services.mozilla.com/D276758
Diffstat:
4 files changed, 25 insertions(+), 84 deletions(-)
diff --git a/browser/base/content/browser-sync.js b/browser/base/content/browser-sync.js
@@ -2341,7 +2341,7 @@ var gSync = {
if (!(await FxAccounts.canConnectAccount())) {
return;
}
- const url = await FxAccounts.config.promiseConnectAccountURI(
+ const url = await FxAccounts.config.promiseSetPasswordURI(
entryPoint,
extraParams
);
diff --git a/services/fxaccounts/FxAccountsConfig.sys.mjs b/services/fxaccounts/FxAccountsConfig.sys.mjs
@@ -106,6 +106,18 @@ export var FxAccountsConfig = {
});
},
+ async promiseSetPasswordURI(entrypoint, extraParams = {}) {
+ const authParams = await this._getAuthParams();
+ return this._buildURL("post_verify/third_party_auth/set_password", {
+ extraParams: {
+ entrypoint,
+ ...authParams,
+ ...extraParams,
+ },
+ addAccountIdentifiers: true,
+ });
+ },
+
async promisePairingURI(extraParams = {}) {
return this._buildURL("pair", {
extraParams,
diff --git a/services/sync/modules/service.sys.mjs b/services/sync/modules/service.sys.mjs
@@ -967,22 +967,6 @@ Sync11Service.prototype = {
}
},
- // Checks if sync can be configured for the current FxA user.
- // Returns true if there is a signed-in user with sync keys available.
- async canConfigure() {
- let user = await fxAccounts.getSignedInUser();
- if (!user) {
- return false;
- }
- try {
- let hasKeys = await fxAccounts.keys.hasKeysForScope(SCOPE_APP_SYNC);
- return hasKeys;
- } catch (err) {
- this._log.error("Failed to check for sync keys", err);
- return false;
- }
- },
-
// configures/enabled/turns-on sync. There must be an FxA user signed in.
async configure() {
// We don't, and must not, throw if sync is already configured, because we
diff --git a/services/sync/tests/unit/test_service_attributes.js b/services/sync/tests/unit/test_service_attributes.js
@@ -91,73 +91,24 @@ add_test(function test_locked() {
run_next_test();
});
-// Tests for canConfigure and configure with sync keys
-add_task(async function test_canConfigure_no_user() {
- _("canConfigure returns false when no user is signed in");
+add_task(async function test_configure_throws_no_user() {
+ _("configure() throws when no user is signed in");
const { getFxAccountsSingleton } = ChromeUtils.importESModule(
"resource://gre/modules/FxAccounts.sys.mjs"
);
const fxAccounts = getFxAccountsSingleton();
- // Mock getSignedInUser to return null
const originalGetSignedInUser = fxAccounts.getSignedInUser;
fxAccounts.getSignedInUser = () => Promise.resolve(null);
try {
- const canConfigure = await Service.canConfigure();
- Assert.equal(canConfigure, false);
- } finally {
- fxAccounts.getSignedInUser = originalGetSignedInUser;
- }
-});
-
-add_task(async function test_canConfigure_no_keys() {
- _("canConfigure returns false when user has no sync keys");
- const { getFxAccountsSingleton } = ChromeUtils.importESModule(
- "resource://gre/modules/FxAccounts.sys.mjs"
- );
- const fxAccounts = getFxAccountsSingleton();
-
- // Mock getSignedInUser to return a user
- const originalGetSignedInUser = fxAccounts.getSignedInUser;
- fxAccounts.getSignedInUser = () =>
- Promise.resolve({ email: "test@example.com", uid: "12345" });
-
- // Mock hasKeysForScope to return false
- const originalHasKeysForScope = fxAccounts.keys.hasKeysForScope;
- fxAccounts.keys.hasKeysForScope = () => Promise.resolve(false);
-
- try {
- const canConfigure = await Service.canConfigure();
- Assert.equal(canConfigure, false);
- } finally {
- fxAccounts.getSignedInUser = originalGetSignedInUser;
- fxAccounts.keys.hasKeysForScope = originalHasKeysForScope;
- }
-});
-
-add_task(async function test_canConfigure_with_keys() {
- _("canConfigure returns true when user has sync keys");
- const { getFxAccountsSingleton } = ChromeUtils.importESModule(
- "resource://gre/modules/FxAccounts.sys.mjs"
- );
- const fxAccounts = getFxAccountsSingleton();
-
- // Mock getSignedInUser to return a user
- const originalGetSignedInUser = fxAccounts.getSignedInUser;
- fxAccounts.getSignedInUser = () =>
- Promise.resolve({ email: "test@example.com", uid: "12345" });
-
- // Mock hasKeysForScope to return true
- const originalHasKeysForScope = fxAccounts.keys.hasKeysForScope;
- fxAccounts.keys.hasKeysForScope = () => Promise.resolve(true);
-
- try {
- const canConfigure = await Service.canConfigure();
- Assert.equal(canConfigure, true);
+ await Assert.rejects(
+ Service.configure(),
+ /No FxA user is signed in/,
+ "configure() should throw when no user is signed in"
+ );
} finally {
fxAccounts.getSignedInUser = originalGetSignedInUser;
- fxAccounts.keys.hasKeysForScope = originalHasKeysForScope;
}
});
@@ -223,7 +174,6 @@ add_task(async function test_third_party_to_sync_complete_flow() {
);
const fxAccounts = getFxAccountsSingleton();
- // Save originals
const originalGetSignedInUser = fxAccounts.getSignedInUser;
const originalHasKeysForScope = fxAccounts.keys.hasKeysForScope;
@@ -235,20 +185,15 @@ add_task(async function test_third_party_to_sync_complete_flow() {
fxAccounts.keys.hasKeysForScope = () => Promise.resolve(false);
try {
- // 3. Verify sync cannot be configured without keys
- Assert.ok(!(await Service.canConfigure()));
+ await Assert.rejects(
+ Service.configure(),
+ /User does not have sync keys/,
+ "configure() should throw when no sync keys"
+ );
- // 4. Simulate receiving keys via webchannel (third-party creates password)
- // Now hasKeysForScope returns true
fxAccounts.keys.hasKeysForScope = () => Promise.resolve(true);
- // 5. Verify sync can now be configured
- Assert.ok(await Service.canConfigure());
-
- // 6. Configure sync
await Service.configure();
-
- // 7. Verify username pref is set
Assert.equal(Svc.PrefBranch.getStringPref("username"), "foo@example.com");
} finally {
fxAccounts.getSignedInUser = originalGetSignedInUser;
