Commit 9311dc38 authored by Balazs Engedy's avatar Balazs Engedy Committed by Commit Bot

WebAuthn: use WebContents::GetVisibility for focus checks.

After this CL, the focus checks performed by navigator.credentials.create()
requests will now succeed when called from the foreground tab in a browser
window, even if the window is not currently the active window. Invocations
from minimized windows and background tabs are still rejected. On platforms
with Visibility::OCCLUDED support, occluded windows are also considered to
be unfocused.

Furthermore, the CL has two side effects:
 -- Foreground WebContents that are currently showing a permission bubble
    or a web-modal dialog are now considered to be "focused" on all
    desktop platforms. The special handling in AuthenticatorImpl is no
    longer needed.
 -- On Linux, the foreground tab in a browser window on an inactive virtual
    screen is still considered to be focused.

While this may very well not be the right approach going forwards, it will
allow consistent behavior regardless of whether the WebAuthn UI is enabled
or disabled. (With UI enabled, focus checks are replaced by checking if the
WebAuthn dialog is currently shown.)

Bug: 849323
Change-Id: I3afc542bed3765be1e43c2fa348fc5b67a8b2aef
Reviewed-on: https://chromium-review.googlesource.com/1092859Reviewed-by: default avatarJochen Eisinger <jochen@chromium.org>
Reviewed-by: default avatarMike Wittman <wittman@chromium.org>
Reviewed-by: default avatarAdam Langley <agl@chromium.org>
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567249}
parent 65c51f30
......@@ -167,11 +167,11 @@ IN_PROC_BROWSER_TEST_F(WebAuthFocusTest, Focus) {
Browser* new_window = browser_added_observer.WaitForSingleNewBrowser();
ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(new_window));
// Operations in the (now unfocused) window should fail, even though it's
// still the active tab in that window.
// Operations in the (now unfocused) window should still succeed, as the
// calling tab is still the active tab in that window.
ASSERT_TRUE(content::ExecuteScriptAndExtractString(initial_web_contents,
register_script, &result));
EXPECT_THAT(result, ::testing::HasSubstr(kFocusErrorSubstring));
EXPECT_THAT(result, "OK");
// Check that closing the window brings things back to a focused state.
chrome::CloseWindow(new_window);
......
......@@ -151,16 +151,8 @@ bool ChromeAuthenticatorRequestDelegate::IsFocused() {
#else
auto* web_contents =
content::WebContents::FromRenderFrameHost(render_frame_host());
for (const auto* browser : *BrowserList::GetInstance()) {
const int tab_index =
browser->tab_strip_model()->GetIndexOfWebContents(web_contents);
if (tab_index != TabStripModel::kNoTab &&
browser->tab_strip_model()->active_index() == tab_index) {
return browser->window()->IsActive();
}
}
return false;
DCHECK(web_contents);
return web_contents->GetVisibility() == content::Visibility::VISIBLE;
#endif
}
......
......@@ -646,16 +646,6 @@ void AuthenticatorImpl::OnRegisterResponse(
if (attestation_preference_ !=
webauth::mojom::AttestationConveyancePreference::NONE) {
// Check for focus before (potentially) showing a permissions bubble
// that might take focus.
if (!IsFocused()) {
InvokeCallbackAndCleanup(
std::move(make_credential_response_callback_),
webauth::mojom::AuthenticatorStatus::NOT_FOCUSED, nullptr,
Focus::kDontCheck);
return;
}
request_delegate_->ShouldReturnAttestation(
relying_party_id_,
base::BindOnce(
......@@ -688,15 +678,11 @@ void AuthenticatorImpl::OnRegisterResponseAttestationDecided(
DCHECK(attestation_preference_ !=
webauth::mojom::AttestationConveyancePreference::NONE);
// At this point, the final focus check has already been done because it's
// possible that a permissions bubble might have focus and thus, if we did a
// focus check, it would (incorrectly) fail.
if (!attestation_permitted) {
InvokeCallbackAndCleanup(
std::move(make_credential_response_callback_),
webauth::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR, nullptr,
Focus::kDontCheck);
Focus::kDoCheck);
return;
}
......@@ -725,7 +711,7 @@ void AuthenticatorImpl::OnRegisterResponseAttestationDecided(
webauth::mojom::AuthenticatorStatus::SUCCESS,
CreateMakeCredentialResponse(std::move(client_data_json_),
std::move(response_data)),
Focus::kDontCheck);
Focus::kDoCheck);
}
void AuthenticatorImpl::OnSignResponse(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment