Commit eeb29c79 authored by Manas Verma's avatar Manas Verma Committed by Commit Bot

Revert "[Autofill Auth] Adding capability to disable WebAuthn UI from Autofill"

This reverts commit 69671ce5.

Reason for revert: Tested in Canary, and the flow stops immediately after authentication. Will try to debug this, and possibly just go ahead with the long term solution right away.

Original change's description:
> [Autofill Auth] Adding capability to disable WebAuthn UI from Autofill
> 
> Providing an option to disable WebAuthn UI from InternalAuthenticatorImpl.
> 
> By disabling WebAuthn UI, modal dialogs will stop conflicting between the two
> components and this will give Autofill full control over the user experience.
> 
> Bug: 949269
> Change-Id: Ibf88cdb0f014c022371546df54d10e2c9eb7bc24
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1948916
> Commit-Queue: Manas Verma <manasverma@google.com>
> Reviewed-by: Martin Kreichgauer <martinkr@google.com>
> Reviewed-by: Jared Saul <jsaul@google.com>
> Reviewed-by: sebsg <sebsg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#722230}

TBR=sebsg@chromium.org,jsaul@google.com,martinkr@google.com,manasverma@google.com

Change-Id: Iab4507114f3e19e59acddd49ea89244ee7390048
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 949269
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1955910Reviewed-by: default avatarManas Verma <manasverma@google.com>
Commit-Queue: Manas Verma <manasverma@google.com>
Cr-Commit-Position: refs/heads/master@{#722571}
parent 0d2ce2a6
...@@ -37,12 +37,6 @@ InternalAuthenticatorImpl::InternalAuthenticatorImpl( ...@@ -37,12 +37,6 @@ InternalAuthenticatorImpl::InternalAuthenticatorImpl(
DCHECK(render_frame_host_); DCHECK(render_frame_host_);
DCHECK(authenticator_common_); DCHECK(authenticator_common_);
DCHECK(!effective_origin.opaque()); DCHECK(!effective_origin.opaque());
// Disabling WebAuthn modal dialogs to avoid conflict with Autofill's own
// modal dialogs. Since WebAuthn is designed for websites, rather than browser
// components, the UI can be confusing for users in the case for Autofill.
// Autofill only ever uses platform authenticators and can take place
// on any webpage.
authenticator_common_->DisableUI();
} }
InternalAuthenticatorImpl::~InternalAuthenticatorImpl() { InternalAuthenticatorImpl::~InternalAuthenticatorImpl() {
......
...@@ -889,13 +889,12 @@ void AuthenticatorCommon::MakeCredential( ...@@ -889,13 +889,12 @@ void AuthenticatorCommon::MakeCredential(
if (!connector_) if (!connector_)
connector_ = GetSystemConnector(); connector_ = GetSystemConnector();
const bool origin_is_crypto_token_extension =
OriginIsCryptoTokenExtension(caller_origin_);
// Save client data to return with the authenticator response. // Save client data to return with the authenticator response.
// TODO(kpaulhamus): Fetch and add the Channel ID/Token Binding ID public key // TODO(kpaulhamus): Fetch and add the Channel ID/Token Binding ID public key
// used to communicate with the origin. // used to communicate with the origin.
if (origin_is_crypto_token_extension) { if (OriginIsCryptoTokenExtension(caller_origin_)) {
// Cryptotoken requests should be proxied without UI.
request_delegate_->DisableUI();
// As Cryptotoken validates the origin, accept the relying party id as the // As Cryptotoken validates the origin, accept the relying party id as the
// origin from requests originating from Cryptotoken. The origin is provided // origin from requests originating from Cryptotoken. The origin is provided
// in Cryptotoken requests as the relying party name, which should be used // in Cryptotoken requests as the relying party name, which should be used
...@@ -910,10 +909,6 @@ void AuthenticatorCommon::MakeCredential( ...@@ -910,10 +909,6 @@ void AuthenticatorCommon::MakeCredential(
std::move(options->challenge), is_cross_origin); std::move(options->challenge), is_cross_origin);
} }
// Cryptotoken requests should be proxied without UI.
if (origin_is_crypto_token_extension || disable_ui_)
request_delegate_->DisableUI();
UMA_HISTOGRAM_COUNTS_100( UMA_HISTOGRAM_COUNTS_100(
"WebAuthentication.MakeCredentialExcludeCredentialsCount", "WebAuthentication.MakeCredentialExcludeCredentialsCount",
options->exclude_credentials.size()); options->exclude_credentials.size());
...@@ -1025,13 +1020,12 @@ void AuthenticatorCommon::GetAssertion( ...@@ -1025,13 +1020,12 @@ void AuthenticatorCommon::GetAssertion(
return; return;
} }
const bool origin_is_crypto_token_extension =
OriginIsCryptoTokenExtension(caller_origin_);
// Save client data to return with the authenticator response. // Save client data to return with the authenticator response.
// TODO(kpaulhamus): Fetch and add the Channel ID/Token Binding ID public key // TODO(kpaulhamus): Fetch and add the Channel ID/Token Binding ID public key
// used to communicate with the origin. // used to communicate with the origin.
if (origin_is_crypto_token_extension) { if (OriginIsCryptoTokenExtension(caller_origin)) {
request_delegate_->DisableUI();
// As Cryptotoken validates the origin, accept the relying party id as the // As Cryptotoken validates the origin, accept the relying party id as the
// origin from requests originating from Cryptotoken. // origin from requests originating from Cryptotoken.
client_data_json_ = SerializeCollectedClientDataToJson( client_data_json_ = SerializeCollectedClientDataToJson(
...@@ -1044,10 +1038,6 @@ void AuthenticatorCommon::GetAssertion( ...@@ -1044,10 +1038,6 @@ void AuthenticatorCommon::GetAssertion(
std::move(options->challenge), is_cross_origin); std::move(options->challenge), is_cross_origin);
} }
// Cryptotoken requests should be proxied without UI.
if (origin_is_crypto_token_extension || disable_ui_)
request_delegate_->DisableUI();
if (options->allow_credentials.empty()) { if (options->allow_credentials.empty()) {
if (!request_delegate_->SupportsResidentKeys()) { if (!request_delegate_->SupportsResidentKeys()) {
InvokeCallbackAndCleanup( InvokeCallbackAndCleanup(
...@@ -1560,10 +1550,6 @@ void AuthenticatorCommon::Cleanup() { ...@@ -1560,10 +1550,6 @@ void AuthenticatorCommon::Cleanup() {
blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR; blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR;
} }
void AuthenticatorCommon::DisableUI() {
disable_ui_ = true;
}
BrowserContext* AuthenticatorCommon::browser_context() const { BrowserContext* AuthenticatorCommon::browser_context() const {
return content::WebContents::FromRenderFrameHost(render_frame_host_) return content::WebContents::FromRenderFrameHost(render_frame_host_)
->GetBrowserContext(); ->GetBrowserContext();
......
...@@ -91,8 +91,6 @@ class CONTENT_EXPORT AuthenticatorCommon { ...@@ -91,8 +91,6 @@ class CONTENT_EXPORT AuthenticatorCommon {
void Cleanup(); void Cleanup();
void DisableUI();
base::flat_set<device::FidoTransportProtocol> enabled_transports_for_testing() base::flat_set<device::FidoTransportProtocol> enabled_transports_for_testing()
const { const {
return transports_; return transports_;
...@@ -207,7 +205,6 @@ class CONTENT_EXPORT AuthenticatorCommon { ...@@ -207,7 +205,6 @@ class CONTENT_EXPORT AuthenticatorCommon {
// empty_allow_list_ is true iff a GetAssertion is currently pending and the // empty_allow_list_ is true iff a GetAssertion is currently pending and the
// request did not list any credential IDs in the allow list. // request did not list any credential IDs in the allow list.
bool empty_allow_list_ = false; bool empty_allow_list_ = false;
bool disable_ui_ = false;
url::Origin caller_origin_; url::Origin caller_origin_;
std::string relying_party_id_; std::string relying_party_id_;
std::unique_ptr<base::OneShotTimer> timer_; std::unique_ptr<base::OneShotTimer> timer_;
......
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