WebAuthn operations that are in-flight with authenticators must be cancelled when switching tabs. There's an Issue [1] opened with the WebAuthn spec for this already, but the language is _not_ in spec. Still, it's necessary for security, spec or not. This also matches how Chromium handles U2F operations during a tab switch. The spec language I've proposed is in the issue, but I did that w/o real knowledge of how this would be implemented. I think we should just do what's most appropriate for Firefox here and I will work to make the spec permit Firefox's take. [1] https://github.com/w3c/webauthn/issues/316

This was bugging me, so I stole it, Tim. Here's a patch that accomplishes the job... but I don't know how to test it with mochitests as-is. It's not really testable with the soft-token as-is, so this might be one that needs manual testing. (Adding Matt to the QA field for context before we proceed with this)

Comment on attachment 8893996 [details] Bug 1383799 - Cancel WebAuthn operations on tab-switch https://reviewboard.mozilla.org/r/165066/#review171612 It shouldn't be hard to write tests but we should get this landed, to simplify QA and have test builds earlier. File a follow-up? ::: dom/webauthn/WebAuthnManager.cpp:230 (Diff revision 1) > + MOZ_ASSERT(aListener); > + > + nsCOMPtr<nsIDocument> doc = aParent->GetExtantDoc(); > + if (doc) { > + return doc->AddSystemEventListener(kVisibilityChange, > + /* listener */ aListener, That comment is rather obvious ;) ::: dom/webauthn/WebAuthnManager.cpp:247 (Diff revision 1) > + MOZ_ASSERT(aListener); > + > + nsCOMPtr<nsIDocument> doc = aParent->GetExtantDoc(); > + if (doc) { > + return doc->RemoveSystemEventListener(kVisibilityChange, > + /* listener */ aListener, (same) ::: dom/webauthn/WebAuthnManager.cpp:268 (Diff revision 1) > + if (mCurrentParent) { > + nsresult rv = StopListeningForVisibilityEvents(mCurrentParent, this); > + Unused << NS_WARN_IF(NS_FAILED(rv)); > + } We should probably also null `mCurrentParent`? ::: dom/webauthn/WebAuthnManager.cpp:269 (Diff revision 1) > + nsresult rv = StopListeningForVisibilityEvents(mCurrentParent, this); > + Unused << NS_WARN_IF(NS_FAILED(rv)); I think we should just make `ListenForVisibilityEvents()` and `StopListeningForVisibilityEvents()` void. Then we could in those functions `WARN_IF` doc=null and the call sites would look better. ::: dom/webauthn/WebAuthnManager.cpp:940 (Diff revision 1) > + > if (mTransactionPromise) { > mTransactionPromise->MaybeReject(aError); > } > + > + StartCancel(); If we have this here we will call `SendRequestCancel()` every time we abort, even from e.g. `FinishMakeCredential()`. I think the `StartCancel()` call should go into `HandleEvent()` that then also calls `Cancel(NS_ERROR_ABORT)`.

(In reply to J.C. Jones [:jcj] from comment #1) > This was bugging me, so I stole it, Tim. Here's a patch that accomplishes Thanks btw :) > the job... but I don't know how to test it with mochitests as-is. It's not > really testable with the soft-token as-is, so this might be one that needs > manual testing. Right... so it actually is hard to write tests. I spoke too fast. A follow-up is probably good to investigate anyway.

Comment on attachment 8893996 [details] Bug 1383799 - Cancel WebAuthn operations on tab-switch https://reviewboard.mozilla.org/r/165066/#review171612 I've filed the follow-up in Bug 1388854. Thanks for the review! > We should probably also null `mCurrentParent`? good call! > I think we should just make `ListenForVisibilityEvents()` and `StopListeningForVisibilityEvents()` void. Then we could in those functions `WARN_IF` doc=null and the call sites would look better. This makes sense. I restructured it a bit; see what you think. > If we have this here we will call `SendRequestCancel()` every time we abort, even from e.g. `FinishMakeCredential()`. > > I think the `StartCancel()` call should go into `HandleEvent()` that then also calls `Cancel(NS_ERROR_ABORT)`. I was thinking of this as a feature, not a bug, but I don't recall what case I was worried about. I went ahead and changed it as you said, and if we see lingering-blinks or something, we can re-assess.

Comment on attachment 8893996 [details] Bug 1383799 - Cancel WebAuthn operations on tab-switch https://reviewboard.mozilla.org/r/165066/#review172148 Good patch. 11/10

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/f7a53ff2f8cb Cancel WebAuthn operations on tab-switch r=ttaubert

As far as tests, this is hard to automate but not as hard to test manually. I believe we will need a whole suite of similar tests that involve typical user scenarios like this.

I am adding this to a list of potential tests, and these will (hopefully) become part of the certification process for future Fx builds.

I've added a test case in Test Rail (C65531) to explicitly cover this case, under the Web Authentication > Boundary Tests section.