Bug 2064395 - Lock the FELT session on browser close - #1308
Conversation
jporter-dev
added
enhancement
fffb2e3 to
45978cd
Compare
jporter-dev
changed the title
db9b5da to
9cd67f2
Compare
|
@jporter-dev When does the prompt you shared in screenshots gets displayed? What is the user input flow to it? |
| * @returns {Promise<void>} | ||
| */ | ||
| async function storeToken(email, token) { | ||
| const encryptedRefreshToken = await lazy.OSKeyStore.encrypt(token); |
There was a problem hiding this comment.
I assume OSKeyStore are the calls triggering the ask to unlock key store that are presented in screenshot. Can we make sure there's an explicit message shown in felt when we know we will ask for the key store credentials ? I believe in term of UX security it's not a good practice that we accustom people to accept "random" request to unlock system key store on our behalf, without a good explanation before.
There was a problem hiding this comment.
I think many applications show a "Unlock with TouchID" button or indicator. May be worth exploring the UX on this a bit.
There was a problem hiding this comment.
From only looking at the code, it's not clear what precisely is the flow here. Will I input my email, hiyt "login" and be presented with the OS-level prompt asking me for my password?
For an uneducated folk, how can this be distinguished from trying to steal their information ?
There was a problem hiding this comment.
I've added an "Unlock" button to FELT when it's meant to unlock instead of go to SSO signin. Pressing "Unlock" will prompt with the OS auth.
When a user attempts to sign-in using an email that has a stored token. I'll add more detail and a recording of the flow to the PR. |
…d account has a locked session
…n locking is enabled
fd18f42 to
b9a0d2c
Compare
|
@lissyx thanks for all of the feedback! I think I addressed everything, and made some other changes prompted by the posture rework and the signout on close handling. I ended up reworking this quite a bit, I think it's a bit simpler now - instead of a custom "lockAndClose" event, the lock intent is set based on the pref and is passed with the exit. This allows the browser to go through the regular shutdown flow (same way we do for "signoutAndClose"), and keeps it compatible with our existing prompt and close logic and makes the "lockOnRestart" and "lockOnCrash" paths easier to add. I also updated the FELT UX when unlocking to make it more explicit that it'll be unlocking, updated the OS auth messaging to be OS-dependent, and updated unlocking to support posture collection. |
b9a0d2c to
5f93a61
Compare
| "SignOut": { | ||
| "type": "object", | ||
| "x-category": "Authentication", | ||
| "x-compatibility": { | ||
| "firefox": { | ||
| "version_added": false | ||
| }, | ||
| "firefox_esr": { | ||
| "version_added": false | ||
| }, | ||
| "firefox_enterprise": { | ||
| "version_added": "156" | ||
| } | ||
| }, | ||
| "x-restart-required": false, | ||
| "description": "Control whether the managed session is signed out or locked when the browser closes.", | ||
| "examples": [ | ||
| { | ||
| "BrowserClose": { | ||
| "Action": "lock" | ||
| } | ||
| } | ||
| ], | ||
| "properties": { | ||
| "BrowserClose": { | ||
| "type": "object", | ||
| "properties": { | ||
| "Action": { | ||
| "type": "string", | ||
| "enum": ["signout", "lock"] | ||
| } | ||
| }, | ||
| "required": ["Action"] | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
Please make either a port of this for Thunderbird if it is a policy that should be handled there, or update allowed differences of https://searchfox.org/comm-central/source/mail/components/enterprisepolicies/tests/browser/browser_policies_differences.js
Or at least ping me when you land so I can adapt it
Also update/adapt https://docs.google.com/spreadsheets/d/1h-UmrLEBVmLVEl9eIVQlNkrP-NkgGXfxpzN5tJWCWv4/edit?pli=1&gid=1904079787#gid=1904079787 ?
There was a problem hiding this comment.
Or at least file a bug blocking thunderbird-client-poc
There was a problem hiding this comment.
I think this is the TB version of this: https://bugzilla.mozilla.org/show_bug.cgi?id=2060195
| SignOut: { | ||
| onBeforeAddons(manager, param) { | ||
| if (param.BrowserClose) { | ||
| lazy.PoliciesUtils.setAndLockPref( | ||
| "enterprise.locking.browser_close", | ||
| param.BrowserClose.Action === "lock" | ||
| ); | ||
| } | ||
| }, | ||
| onRemove(manager, oldParams) { | ||
| if (oldParams.BrowserClose) { | ||
| lazy.PoliciesUtils.unsetAndUnlockPref( | ||
| "enterprise.locking.browser_close" | ||
| ); | ||
| // unsetAndUnlockPref restores the build default but never re-locks; | ||
| // re-lock to match the locked default the enterprise build ships. | ||
| Services.prefs.lockPref("enterprise.locking.browser_close"); | ||
| } | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Same comment regarding Thunderbird
| /** | ||
| * Collect device posture for the given user's profile, for a session about to | ||
| * launch. Exposed for the unlock flow (FeltLocking.tryUnlock), which runs in | ||
| * this process and submits the posture with its resuming refresh, so posture | ||
| * collection stays owned here rather than duplicated in FeltLocking. | ||
| * | ||
| * @param {string} userId | ||
| * @returns {Promise<{posture: DevicePosture, measuredAt: number}>} | ||
| */ | ||
| async collectLaunchPosture(userId) { | ||
| const { path: profileDir } = await lazy.resolveManagedProfile({ | ||
| id: userId, | ||
| }); | ||
| const measuredAt = Date.now(); | ||
| const posture = await lazy.DevicePosture.collect({ profileDir }); | ||
| return { posture, measuredAt }; | ||
| } |
There was a problem hiding this comment.
This should be in DevicePosture.sys.mjs, similar to postureForRefresh no ? https://searchfox.org/enterprise-main/rev/b43f65e4868d2595f2b8c11a8d1df99292742dc8/toolkit/components/enterprise/modules/DevicePosture.sys.mjs#427-444
There was a problem hiding this comment.
This is more like the block here, for collecting fresh posture: https://searchfox.org/enterprise-main/source/toolkit/components/felt/content/FeltProcessParent.sys.mjs#1104-1117
I moved this logic directly into FeltLocking instead of defining this function here.
…ale token on store failure
3 participants
Description
Bugzilla: Bug-2064395
Adds session locking for FELT SSO sessions to require OS auth to resume. Instead of always signing out when the browser closes, FELT can now lock the session, controlled by policy: the refresh token is encrypted via OSKeyStore and persisted, then resumed on next launch after the user passes an OS auth prompt. If locking is disabled or unavailable, we fall back to the existing sign-out-on-close behavior.
When a session is "locked" the user will be prompted with the OS-level authentication prompt (Windows Hello, TouchID, password prompt) instead of SSO when proceeding to sign in from FELT with their email address.
enterprise.locking.on_closepref, plus aSignOutenterprise policy to control it.FeltLockingmodule handles encrypt/store, update, and unlock. The token is only ever stored encrypted in the keystore, and is dropped whenever locking is disabled or the user signs out.Screenshots
FELT login with unlock button
Unlock prompt on MacOS with TouchID
Unlock prompt on MacOS without TouchID
Keychain Permission Request on MacOS
Unlock prompt on Ubuntu
Unlock prompt on Windows with pin
Unlock prompt on Windows with password
Signout prompt with locking
Signout prompt with locking and tabs
Testing
Steps to verify changes:
enterprise.session.locking.enabledandenterprise.session.locking.on_closeto true.Expected result:
Session resumes after OS auth. With locking off, the browser signs out on close as before.