Open Bug 1880918 Opened 9 months ago Updated 5 months ago

WebAuthnPromptObserver should not live in browser.js and be per-window

Categories

(Core :: DOM: Web Authentication, task, P3)

Desktop
All
task

Tracking

()

ASSIGNED

People

(Reporter: Gijs, Assigned: jschanck, Mentored)

References

(Blocks 1 open bug)

Details

By lines of code this object is the 4th biggest thing in browser.js. It's not actually doing anything most of the time, and effectively each browser window is getting its own little observer, which observes all the webauthn-prompt calls but then doesn't actually do anything unless the browser for which the request came in is the foreground browser in its own window, otherwise, the code claims "Must belong to some other window."

There are a number of issues with this:

  • we're trying to make browser.js less of a dumping ground. :-)
  • this observer code should be running once, not in every window
  • the comment is wrong (at least a priori, ie if there isn't some other guarantee from the rust code that appears to be the only thing that can fire this notification) - the observer notification can fire from background tabs in the same window, or from subframes, and AFAICT that would lead to the prompt getting dropped on the floor. (Maybe that's OK? But it would probably be nice to at least put something in the browser console, or show a dismissed permission icon in the navbar, or something.)

Moving the code out is not particularly hard. We could move it to a sys.mjs file, and mostly keep it as-is. To get from the browsing context ID to a window we can use BrowsingContext.get(id) to get a BC ref, and then use browsingContext.topChromeWindow to find the browser window, or topFrameElement to get the browser reference, or something along those lines.

The main issue I'm seeing is how to get such a sys.mjs module up and running, ie it's a bootstrapping problem.

If there is no better way of doing this, we could always add an initial observer in BrowserGlue.sys.mjs that loads the module if/when the notification first fires, and then unregisters itself in lieu of a direct observer from the module.

:jschanck, looks like you've touched this code relatively recently - does this seem like a reasonable plan, and/or do you have a better idea of when we could load such a module (rather than eagerly, in every browser window) ?

Flags: needinfo?(jschanck)

Sounds like we should rework this. As you observed, the webauthn-prompt notifications are only sent from authrs_bridge. Many of our users will never trigger these notifications, as this code is only used 1) in tests or 2) on Linux, Free/Net/OpenBSD, and old versions of Windows and macOS.

The notifications are only sent during the lifetime of a WebAuthn transaction, and only after the parent process has received the request to handle the transaction. So we could spin up the observer from WebAuthnService once we've determined that we're going to use authrs_bridge.

Flags: needinfo?(jschanck)
Assignee: nobody → jschanck
Severity: -- → S3
Status: NEW → ASSIGNED
Priority: -- → P3
Type: defect → task
You need to log in before you can comment on or make changes to this bug.