Skip to content

Commit 1bd162d

Browse files
committed
Bug 1540309 - Track browsing context for the WebAuthn UX prompts r=nhnt11
The WebAuthn popup just used the current selected browser for its context, while we actually want to stick with the window that brought us to the party. This adds tests to ensure that the doorhanger stays on the tab it was originally attached to, avoiding the 'wandering doorhanger' problem from the bug. Differential Revision: https://phabricator.services.mozilla.com/D72255
1 parent 628ac0a commit 1bd162d

File tree

4 files changed

+137
-12
lines changed

4 files changed

+137
-12
lines changed

browser/base/content/browser.js

+15-2
Original file line numberDiff line numberDiff line change
@@ -7744,14 +7744,26 @@ var WebAuthnPromptHelper = {
77447744
let mgr = aSubject.QueryInterface(Ci.nsIU2FTokenManager);
77457745
let data = JSON.parse(aData);
77467746

7747+
// If we receive a cancel, it might be a WebAuthn prompt starting in another
7748+
// window, and the other window's browsing context will send out the
7749+
// cancellations, so any cancel action we get should prompt us to cancel.
7750+
if (data.action == "cancel") {
7751+
this.cancel(data);
7752+
}
7753+
7754+
if (
7755+
data.browsingContextId !== gBrowser.selectedBrowser.browsingContext.id
7756+
) {
7757+
// Must belong to some other window.
7758+
return;
7759+
}
7760+
77477761
if (data.action == "register") {
77487762
this.register(mgr, data);
77497763
} else if (data.action == "register-direct") {
77507764
this.registerDirect(mgr, data);
77517765
} else if (data.action == "sign") {
77527766
this.sign(mgr, data);
7753-
} else if (data.action == "cancel") {
7754-
this.cancel(data);
77557767
}
77567768
},
77577769

@@ -7819,6 +7831,7 @@ var WebAuthnPromptHelper = {
78197831

78207832
options.name = origin;
78217833
options.hideClose = true;
7834+
options.persistent = true;
78227835
options.eventCallback = event => {
78237836
if (event == "removed") {
78247837
this._current = null;

dom/webauthn/U2FTokenManager.cpp

+11-6
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,15 @@ static nsIThread* gBackgroundThread;
5050

5151
// Data for WebAuthn UI prompt notifications.
5252
static const char16_t kRegisterPromptNotifcation[] =
53-
u"{\"action\":\"register\",\"tid\":%llu,\"origin\":\"%s\"}";
53+
u"{\"action\":\"register\",\"tid\":%llu,\"origin\":\"%s\","
54+
u"\"browsingContextId\":%llu}";
5455
static const char16_t kRegisterDirectPromptNotifcation[] =
55-
u"{\"action\":\"register-direct\",\"tid\":%llu,\"origin\":\"%s\"}";
56+
u"{\"action\":\"register-direct\",\"tid\":%llu,\"origin\":\"%s\","
57+
u"\"browsingContextId\":%llu}";
5658
static const char16_t kSignPromptNotifcation[] =
57-
u"{\"action\":\"sign\",\"tid\":%llu,\"origin\":\"%s\"}";
59+
u"{\"action\":\"sign\",\"tid\":%llu,\"origin\":\"%s\","
60+
u"\"browsingContextId\":%"
61+
u"llu}";
5862
static const char16_t kCancelPromptNotifcation[] =
5963
u"{\"action\":\"cancel\",\"tid\":%llu}";
6064

@@ -340,7 +344,7 @@ void U2FTokenManager::Register(
340344
// store the transaction info until the user proceeds or cancels.
341345
NS_ConvertUTF16toUTF8 origin(aTransactionInfo.Origin());
342346
SendPromptNotification(kRegisterDirectPromptNotifcation, aTransactionId,
343-
origin.get());
347+
origin.get(), aTransactionInfo.BrowsingContextId());
344348

345349
MOZ_ASSERT(mPendingRegisterInfo.isNothing());
346350
mPendingRegisterInfo = Some(aTransactionInfo);
@@ -354,7 +358,7 @@ void U2FTokenManager::DoRegister(const WebAuthnMakeCredentialInfo& aInfo,
354358
// Show a prompt that lets the user cancel the ongoing transaction.
355359
NS_ConvertUTF16toUTF8 origin(aInfo.Origin());
356360
SendPromptNotification(kRegisterPromptNotifcation, mLastTransactionId,
357-
origin.get());
361+
origin.get(), aInfo.BrowsingContextId());
358362

359363
uint64_t tid = mLastTransactionId;
360364
mozilla::TimeStamp startTime = mozilla::TimeStamp::Now();
@@ -413,7 +417,8 @@ void U2FTokenManager::Sign(PWebAuthnTransactionParent* aTransactionParent,
413417

414418
// Show a prompt that lets the user cancel the ongoing transaction.
415419
NS_ConvertUTF16toUTF8 origin(aTransactionInfo.Origin());
416-
SendPromptNotification(kSignPromptNotifcation, aTransactionId, origin.get());
420+
SendPromptNotification(kSignPromptNotifcation, aTransactionId, origin.get(),
421+
aTransactionInfo.BrowsingContextId());
417422

418423
uint64_t tid = mLastTransactionId = aTransactionId;
419424
mozilla::TimeStamp startTime = mozilla::TimeStamp::Now();

dom/webauthn/WebAuthnManager.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -416,9 +416,9 @@ already_AddRefed<Promise> WebAuthnManager::MakeCredential(
416416
return promise.forget();
417417
}
418418

419-
WebAuthnMakeCredentialInfo info(origin, NS_ConvertUTF8toUTF16(rpId),
420-
challenge, clientDataJSON, adjustedTimeout,
421-
excludeList, Some(extra), context->Id());
419+
WebAuthnMakeCredentialInfo info(
420+
origin, NS_ConvertUTF8toUTF16(rpId), challenge, clientDataJSON,
421+
adjustedTimeout, excludeList, Some(extra), context->Top()->Id());
422422

423423
#ifdef OS_WIN
424424
if (!WinWebAuthnManager::AreWebAuthNApisAvailable()) {
@@ -624,7 +624,7 @@ already_AddRefed<Promise> WebAuthnManager::GetAssertion(
624624

625625
WebAuthnGetAssertionInfo info(origin, NS_ConvertUTF8toUTF16(rpId), challenge,
626626
clientDataJSON, adjustedTimeout, allowList,
627-
Some(extra), context->Id());
627+
Some(extra), context->Top()->Id());
628628

629629
#ifdef OS_WIN
630630
if (!WinWebAuthnManager::AreWebAuthNApisAvailable()) {

dom/webauthn/tests/browser/browser_webauthn_prompts.js

+107
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,16 @@ function promiseNotification(id) {
1919
});
2020
}
2121

22+
function triggerMainPopupCommand(popup) {
23+
info("triggering main command");
24+
let notifications = popup.childNodes;
25+
ok(notifications.length > 0, "at least one notification displayed");
26+
let notification = notifications[0];
27+
info("triggering command: " + notification.getAttribute("buttonlabel"));
28+
29+
return EventUtils.synthesizeMouseAtCenter(notification.button, {});
30+
}
31+
2232
let expectAbortError = expectError("Abort");
2333

2434
function verifyAnonymizedCertificate(result) {
@@ -115,6 +125,103 @@ add_task(async function test_register_direct_cancel() {
115125
await BrowserTestUtils.removeTab(tab);
116126
});
117127

128+
// Add two tabs, open WebAuthn in the first, switch, assert the prompt is
129+
// not visible, switch back, assert the prompt is there and cancel it.
130+
add_task(async function test_tab_switching() {
131+
// Open a new tab.
132+
let tab_one = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL);
133+
134+
// Request a new credential and wait for the prompt.
135+
let active = true;
136+
let request = promiseWebAuthnMakeCredential(tab_one, "indirect", {})
137+
.then(arrivingHereIsBad)
138+
.catch(expectAbortError)
139+
.then(() => (active = false));
140+
await promiseNotification("webauthn-prompt-register");
141+
is(PopupNotifications.panel.state, "open", "Doorhanger is visible");
142+
143+
// Open and switch to a second tab.
144+
let tab_two = await BrowserTestUtils.openNewForegroundTab(
145+
gBrowser,
146+
"https://example.org/"
147+
);
148+
149+
await TestUtils.waitForCondition(
150+
() => PopupNotifications.panel.state == "closed"
151+
);
152+
is(PopupNotifications.panel.state, "closed", "Doorhanger is hidden");
153+
154+
// Go back to the first tab
155+
await BrowserTestUtils.removeTab(tab_two);
156+
157+
await promiseNotification("webauthn-prompt-register");
158+
159+
await TestUtils.waitForCondition(
160+
() => PopupNotifications.panel.state == "open"
161+
);
162+
is(PopupNotifications.panel.state, "open", "Doorhanger is visible");
163+
164+
// Cancel the request.
165+
ok(active, "request should still be active");
166+
await triggerMainPopupCommand(PopupNotifications.panel);
167+
await request;
168+
ok(!active, "request should be stopped");
169+
170+
// Close tab.
171+
await BrowserTestUtils.removeTab(tab_one);
172+
});
173+
174+
// Add two tabs, open WebAuthn in the first, switch, assert the prompt is
175+
// not visible, switch back, assert the prompt is there and cancel it.
176+
add_task(async function test_window_switching() {
177+
// Open a new tab.
178+
let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL);
179+
180+
// Request a new credential and wait for the prompt.
181+
let active = true;
182+
let request = promiseWebAuthnMakeCredential(tab, "indirect", {})
183+
.then(arrivingHereIsBad)
184+
.catch(expectAbortError)
185+
.then(() => (active = false));
186+
await promiseNotification("webauthn-prompt-register");
187+
188+
await TestUtils.waitForCondition(
189+
() => PopupNotifications.panel.state == "open"
190+
);
191+
is(PopupNotifications.panel.state, "open", "Doorhanger is visible");
192+
193+
// Open and switch to a second window
194+
let new_window = await BrowserTestUtils.openNewBrowserWindow();
195+
await SimpleTest.promiseFocus(new_window);
196+
197+
await TestUtils.waitForCondition(
198+
() => new_window.PopupNotifications.panel.state == "closed"
199+
);
200+
is(
201+
new_window.PopupNotifications.panel.state,
202+
"closed",
203+
"Doorhanger is hidden"
204+
);
205+
206+
// Go back to the first tab
207+
await BrowserTestUtils.closeWindow(new_window);
208+
await SimpleTest.promiseFocus(window);
209+
210+
await TestUtils.waitForCondition(
211+
() => PopupNotifications.panel.state == "open"
212+
);
213+
is(PopupNotifications.panel.state, "open", "Doorhanger is still visible");
214+
215+
// Cancel the request.
216+
ok(active, "request should still be active");
217+
await triggerMainPopupCommand(PopupNotifications.panel);
218+
await request;
219+
ok(!active, "request should be stopped");
220+
221+
// Close tab.
222+
await BrowserTestUtils.removeTab(tab);
223+
});
224+
118225
add_task(async function test_setup_softtoken() {
119226
await SpecialPowers.pushPrefEnv({
120227
set: [

0 commit comments

Comments
 (0)