Skip to content

Commit db9b5da

Browse files
committed
Bug 2064395 - Rework FELT locking config and drive Felt UI persistence from lock signals
1 parent 8d5b2e7 commit db9b5da

8 files changed

Lines changed: 153 additions & 189 deletions

File tree

browser/app/profile/firefox.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,7 @@ pref("enterprise.prompt_on_signout", true);
3333
pref("app.update.checkOnlyInstance.enabled", false);
3434
pref("app.update.background.enabled", true);
3535
// Allow locking the session (persist behind OS auth) instead of signing out.
36-
pref("enterprise.session.locking.enabled", false);
37-
pref("enterprise.session.locking.on_close", false);
36+
pref("enterprise.locking.browser_close", false);
3837
#endif
3938

4039
// Set add-ons abuse report related prefs specific to Firefox Desktop.

browser/components/enterprisepolicies/Policies.sys.mjs

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4065,19 +4065,18 @@ export var Policies = {
40654065

40664066
SignOut: {
40674067
onBeforeAddons(manager, param) {
4068-
if (param.OnClose) {
4069-
// Disabling sign-out on close means locking and persisting instead.
4070-
const lock = !param.OnClose.Enabled;
4071-
const locked = param.OnClose.Locked ?? false;
4072-
lazy.PoliciesUtils.setDefaultPref(
4073-
"enterprise.session.locking.enabled",
4074-
lock,
4075-
locked
4076-
);
4068+
const TRIGGERS = {
4069+
BrowserClose: "enterprise.locking.browser_close",
4070+
};
4071+
const configuredTriggers = Object.entries(TRIGGERS).filter(
4072+
([key]) => param[key]
4073+
);
4074+
for (const [key, pref] of configuredTriggers) {
4075+
const { Action, Locked } = param[key];
40774076
lazy.PoliciesUtils.setDefaultPref(
4078-
"enterprise.session.locking.on_close",
4079-
lock,
4080-
locked
4077+
pref,
4078+
Action === "lock",
4079+
Locked ?? false
40814080
);
40824081
}
40834082
},

browser/components/enterprisepolicies/schemas/policies-schema.json

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4130,23 +4130,24 @@
41304130
"description": "Control whether the managed session is signed out or locked when the browser closes.",
41314131
"examples": [
41324132
{
4133-
"OnClose": {
4134-
"Enabled": true
4133+
"BrowserClose": {
4134+
"Action": "lock"
41354135
}
41364136
}
41374137
],
41384138
"properties": {
4139-
"OnClose": {
4139+
"BrowserClose": {
41404140
"type": "object",
41414141
"properties": {
4142-
"Enabled": {
4143-
"type": "boolean"
4142+
"Action": {
4143+
"type": "string",
4144+
"enum": ["signout", "lock"]
41444145
},
41454146
"Locked": {
41464147
"type": "boolean"
41474148
}
41484149
},
4149-
"required": ["Enabled"]
4150+
"required": ["Action"]
41504151
}
41514152
}
41524153
},

browser/components/enterprisepolicies/tests/xpcshell/test_simple_pref_policies.js

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1500,29 +1500,27 @@ const POLICIES_TESTS = [
15001500
{
15011501
policies: {
15021502
SignOut: {
1503-
OnClose: {
1504-
Enabled: false,
1503+
BrowserClose: {
1504+
Action: "lock",
15051505
},
15061506
},
15071507
},
1508-
// Not signing out on close means locking/persisting the session.
1508+
// Locking on close persists the session behind OS auth instead of signing out.
15091509
unlockedPrefs: {
1510-
"enterprise.session.locking.enabled": true,
1511-
"enterprise.session.locking.on_close": true,
1510+
"enterprise.locking.browser_close": true,
15121511
},
15131512
},
15141513
{
15151514
policies: {
15161515
SignOut: {
1517-
OnClose: {
1518-
Enabled: true,
1516+
BrowserClose: {
1517+
Action: "signout",
15191518
Locked: true,
15201519
},
15211520
},
15221521
},
15231522
lockedPrefs: {
1524-
"enterprise.session.locking.enabled": false,
1525-
"enterprise.session.locking.on_close": false,
1523+
"enterprise.locking.browser_close": false,
15261524
},
15271525
},
15281526
];

toolkit/components/enterprise/modules/EnterpriseHandler.sys.mjs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,7 @@ const PROMPT_ON_SIGNOUT_PREF = "enterprise.prompt_on_signout";
2727
const COMPANY_LOGO_URL_PREF = "enterprise.configs.company_logo_url";
2828
const LEARN_MORE_URL_PREF = "enterprise.configs.learn_more_url";
2929
const WARN_ON_CLOSE_PREF = "browser.tabs.warnOnClose";
30-
const LOCK_ENABLED_PREF = "enterprise.session.locking.enabled";
31-
const LOCK_ON_CLOSE_PREF = "enterprise.session.locking.on_close";
30+
const LOCK_ON_CLOSE_PREF = "enterprise.locking.browser_close";
3231

3332
/**
3433
* Parses a given url string
@@ -588,15 +587,12 @@ export const EnterpriseHandler = {
588587

589588
/**
590589
* Whether closing the browser will lock the session (persist it behind OS
591-
* auth to resume later) rather than sign out, per the locking prefs.
590+
* auth to resume later) rather than sign out, per the locking pref.
592591
*
593592
* @returns {boolean}
594593
*/
595594
get willLockOnClose() {
596-
return (
597-
Services.prefs.getBoolPref(LOCK_ENABLED_PREF, false) &&
598-
Services.prefs.getBoolPref(LOCK_ON_CLOSE_PREF, false)
599-
);
595+
return Services.prefs.getBoolPref(LOCK_ON_CLOSE_PREF, false);
600596
},
601597

602598
/**

toolkit/components/felt/content/FeltLocking.sys.mjs

Lines changed: 75 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
* License, v. 2.0. If a copy of the MPL was not distributed with this
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

5-
const ENTERPRISE_LOCKING_ENABLED_PREF = "enterprise.session.locking.enabled";
6-
75
const lazy = {};
86

97
ChromeUtils.defineESModuleGetters(lazy, {
@@ -18,10 +16,6 @@ ChromeUtils.defineLazyGetter(lazy, "log", () => {
1816
return lazy.createEnterpriseLogger("FeltLocking");
1917
});
2018

21-
function lockingEnabled() {
22-
return Services.prefs.getBoolPref(ENTERPRISE_LOCKING_ENABLED_PREF, false);
23-
}
24-
2519
/**
2620
* The email of the currently signed-in user, used as the key under which a
2721
* locked session's refresh token is stored. Read from the cached value rather
@@ -42,19 +36,11 @@ function currentEmail() {
4236
* @returns {Promise<void>}
4337
*/
4438
async function storeToken(email, token) {
45-
// Choke point: never persist a token while the feature is disabled.
46-
if (!lockingEnabled()) {
47-
return;
48-
}
4939
const encryptedRefreshToken = await lazy.OSKeyStore.encrypt(token);
5040
lazy.FeltStorage.setLockingToken(email, encryptedRefreshToken);
5141
}
5242

5343
export const FeltLocking = {
54-
get enabled() {
55-
return lockingEnabled();
56-
},
57-
5844
/**
5945
* Attempt to resume a previously locked session for the given user. Requires
6046
* OS-level authentication and a stored, still-valid refresh token.
@@ -64,87 +50,81 @@ export const FeltLocking = {
6450
* @returns {Promise<boolean>} Whether the session was successfully unlocked.
6551
*/
6652
tryUnlock: async (email, browser) => {
67-
if (lockingEnabled()) {
68-
const token = lazy.FeltStorage.getLockingToken(email);
69-
if (token) {
70-
const { authenticated } = await lazy.OSKeyStore.ensureLoggedIn(
71-
"Trying to unlock existing session",
72-
"Firefox Enterprise"
73-
);
74-
if (authenticated) {
75-
let refreshToken;
76-
try {
77-
refreshToken = await lazy.OSKeyStore.decrypt(token, "", false);
78-
} catch (err) {
53+
// A stored token exists only because a browser-authorized lock created it,
54+
// so its presence is the authorization to resume: the locking pref lives in
55+
// the browser process, which the Felt UI process cannot read.
56+
const token = lazy.FeltStorage.getLockingToken(email);
57+
if (token) {
58+
const { authenticated } = await lazy.OSKeyStore.ensureLoggedIn(
59+
"Trying to unlock existing session",
60+
"Firefox Enterprise"
61+
);
62+
if (authenticated) {
63+
let refreshToken;
64+
try {
65+
refreshToken = await lazy.OSKeyStore.decrypt(token, "", false);
66+
} catch (err) {
67+
lazy.log.warn(
68+
`tryUnlock: decrypt failed, falling back to sign-in: ${err}`
69+
);
70+
}
71+
if (!refreshToken) {
72+
Services.felt.setTokens("", "", 0);
73+
lazy.FeltStorage.clearLockingToken(email);
74+
return false;
75+
}
76+
// Only the refresh token is available here; force a refresh below.
77+
Services.felt.setTokens("", refreshToken, 0);
78+
try {
79+
const { access_token, refresh_token, expires_at } =
80+
await lazy.ConsoleClient.refreshTokens();
81+
Services.felt.setTokens(access_token, refresh_token, expires_at);
82+
83+
await storeToken(email, refresh_token);
84+
} catch (err) {
85+
Services.felt.setTokens("", "", 0);
86+
if (err?.name === "ReauthRequiredError") {
87+
// The refresh token is genuinely invalid/revoked: drop it so we
88+
// fall back to a full SSO sign-in.
89+
lazy.FeltStorage.clearLockingToken(email);
90+
} else {
91+
// Transient failure (offline, server error, ...): keep the stored
92+
// token so the session can still be unlocked later.
7993
lazy.log.warn(
80-
`tryUnlock: decrypt failed, falling back to sign-in: ${err}`
94+
`tryUnlock: transient failure resuming from token, keeping it: ${err}`
8195
);
8296
}
83-
if (!refreshToken) {
84-
Services.felt.setTokens("", "", 0);
85-
lazy.FeltStorage.clearLockingToken(email);
86-
return false;
87-
}
88-
// Only the refresh token is available here; force a refresh below.
89-
Services.felt.setTokens("", refreshToken, 0);
90-
try {
91-
const { access_token, refresh_token, expires_at } =
92-
await lazy.ConsoleClient.refreshTokens();
93-
Services.felt.setTokens(access_token, refresh_token, expires_at);
94-
95-
await storeToken(email, refresh_token);
96-
} catch (err) {
97-
Services.felt.setTokens("", "", 0);
98-
if (err?.name === "ReauthRequiredError") {
99-
// The refresh token is genuinely invalid/revoked: drop it so we
100-
// fall back to a full SSO sign-in.
101-
lazy.FeltStorage.clearLockingToken(email);
102-
} else {
103-
// Transient failure (offline, server error, ...): keep the stored
104-
// token so the session can still be unlocked later.
105-
lazy.log.warn(
106-
`tryUnlock: transient failure resuming from token, keeping it: ${err}`
107-
);
108-
}
109-
return false;
110-
}
111-
112-
// Tokens are committed; from here a failure is a launch failure, not
113-
// a reason to fall back to SSO, so let it propagate to the caller.
114-
const parentActor =
115-
browser.browsingContext.currentWindowGlobal.domProcess.getActor(
116-
"FeltProcess"
117-
);
118-
await parentActor.receiveMessage({
119-
name: "FeltChild:StartFirefox",
120-
data: {},
121-
});
122-
return true;
97+
return false;
12398
}
99+
100+
// Tokens are committed; from here a failure is a launch failure, not
101+
// a reason to fall back to SSO, so let it propagate to the caller.
102+
const parentActor =
103+
browser.browsingContext.currentWindowGlobal.domProcess.getActor(
104+
"FeltProcess"
105+
);
106+
await parentActor.receiveMessage({
107+
name: "FeltChild:StartFirefox",
108+
data: {},
109+
});
110+
return true;
124111
}
125112
}
126113
return false;
127114
},
128115

129116
/**
130-
* Reconcile the persisted locked-session token with the current refresh token
131-
* and the enabled pref: when locking is enabled, store the (encrypted) token
132-
* so the session can later be unlocked; when it is disabled, drop any stale
133-
* token so flipping the pref off can never leave a credential behind.
117+
* Persist the (encrypted) refresh token so the session can later be unlocked.
118+
* Only ever reached via an explicit, browser-authorized lock, so it does not
119+
* consult the locking pref (which the Felt UI process cannot read).
134120
*
135121
* @param {string} refresh_token
136-
* @throws {Error} If locking is enabled but no signed-in user is known, so the
137-
* caller can fall back to signing out instead of locking.
122+
* @throws {Error} If no signed-in user is known, so the caller can fall back
123+
* to signing out instead of locking.
138124
* @returns {Promise<void>}
139125
*/
140126
store: async refresh_token => {
141127
const email = currentEmail();
142-
if (!lockingEnabled()) {
143-
if (email) {
144-
lazy.FeltStorage.clearLockingToken(email);
145-
}
146-
return;
147-
}
148128
if (!email) {
149129
throw new Error(
150130
"store: no signed-in user known, cannot persist locked session"
@@ -153,6 +133,22 @@ export const FeltLocking = {
153133
await storeToken(email, refresh_token);
154134
},
155135

136+
/**
137+
* Keep an already-persisted token in sync with a rotated refresh token. Never
138+
* creates one: persistence is authorized only by an explicit lock, so a token
139+
* refresh must not turn a non-locking session into a lockable one.
140+
*
141+
* @param {string} refresh_token
142+
* @returns {Promise<void>}
143+
*/
144+
updateStoredToken: async refresh_token => {
145+
const email = currentEmail();
146+
if (!email || !lazy.FeltStorage.getLockingToken(email)) {
147+
return;
148+
}
149+
await storeToken(email, refresh_token);
150+
},
151+
156152
/**
157153
* Remove any stored locked-session token for the current user. Always runs
158154
* (even when locking is disabled) so signing out can never leave a credential

toolkit/components/felt/content/FeltProcessParent.sys.mjs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -331,9 +331,9 @@ export class FeltProcessParent extends JSProcessActorParent {
331331
// Keep the persisted token in sync with the rotated refresh
332332
// token; a keystore failure here must not tear down a healthy
333333
// session.
334-
lazy.FeltLocking.store(refresh_token).catch(err => {
334+
lazy.FeltLocking.updateStoredToken(refresh_token).catch(err => {
335335
lazy.log.warn(
336-
`Failed to reconcile locked-session token on refresh: ${err}`
336+
`Failed to update the stored locked-session token on refresh: ${err}`
337337
);
338338
});
339339
})
@@ -1007,13 +1007,10 @@ export class FeltProcessParent extends JSProcessActorParent {
10071007
// FirefoxNormalExit (which would sign the session out).
10081008
gFeltProcessParentInstance.logoutReported = true;
10091009

1010+
// Reaching here means the browser already decided to lock (it owns the
1011+
// locking pref and only sends the lock signal when enabled), so persist
1012+
// unconditionally; store() still throws if no user is known.
10101013
const persistLock = async () => {
1011-
// Guard here as well as browser-side: if locking is disabled we must not
1012-
// silently quit without persisting or signing out, which would leave a
1013-
// dangling server session.
1014-
if (!lazy.FeltLocking.enabled) {
1015-
throw new Error("locking disabled, cannot persist session");
1016-
}
10171014
await lazy.FeltLocking.store(Services.felt.getRefreshToken());
10181015
};
10191016

0 commit comments

Comments
 (0)