Commit d8605f9d authored by Jan Wilken Dörrie's avatar Jan Wilken Dörrie Committed by Commit Bot

[Passwords] Move TTF insecure-origins-only check to renderer

This change moves the check to conditionally disable Touch To Fill
on secure origins to the renderer. Checking this in the browser is
not sufficient, as it can lead to UI bugs where the renderer believes
Touch To Fill is shown while it reality it is not.

Bug: 1014042
Change-Id: Ie6e1ef57bad0dc3991e38f3c535836935561b569
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1871512Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707827}
parent ba11109c
...@@ -7,10 +7,8 @@ ...@@ -7,10 +7,8 @@
#include <utility> #include <utility>
#include "base/logging.h" #include "base/logging.h"
#include "base/metrics/field_trial_params.h"
#include "chrome/browser/password_manager/chrome_password_manager_client.h" #include "chrome/browser/password_manager/chrome_password_manager_client.h"
#include "chrome/browser/touch_to_fill/touch_to_fill_view.h" #include "chrome/browser/touch_to_fill/touch_to_fill_view.h"
#include "components/autofill/core/common/autofill_features.h"
#include "components/favicon/core/favicon_service.h" #include "components/favicon/core/favicon_service.h"
#include "components/password_manager/core/browser/android_affiliation/affiliation_utils.h" #include "components/password_manager/core/browser/android_affiliation/affiliation_utils.h"
#include "components/password_manager/core/browser/origin_credential_store.h" #include "components/password_manager/core/browser/origin_credential_store.h"
...@@ -43,27 +41,18 @@ TouchToFillController::~TouchToFillController() = default; ...@@ -43,27 +41,18 @@ TouchToFillController::~TouchToFillController() = default;
void TouchToFillController::Show(base::span<const CredentialPair> credentials, void TouchToFillController::Show(base::span<const CredentialPair> credentials,
base::WeakPtr<PasswordManagerDriver> driver) { base::WeakPtr<PasswordManagerDriver> driver) {
// Disable Touch To Fill for secure origins if specified by the server.
const GURL& url = driver->GetLastCommittedURL();
const TouchToFillView::IsOriginSecure is_origin_secure(
network::IsUrlPotentiallyTrustworthy(url));
if (base::GetFieldTrialParamByFeatureAsBool(
autofill::features::kAutofillTouchToFill, "insecure-origins-only",
/*default_value=*/false) &&
is_origin_secure) {
driver->TouchToFillDismissed();
return;
}
DCHECK(!driver_ || driver_.get() == driver.get()); DCHECK(!driver_ || driver_.get() == driver.get());
driver_ = std::move(driver); driver_ = std::move(driver);
if (!view_) if (!view_)
view_ = TouchToFillViewFactory::Create(this); view_ = TouchToFillViewFactory::Create(this);
const GURL& url = driver_->GetLastCommittedURL();
view_->Show(url_formatter::FormatUrlForSecurityDisplay( view_->Show(url_formatter::FormatUrlForSecurityDisplay(
url, url_formatter::SchemeDisplay::OMIT_HTTP_AND_HTTPS), url, url_formatter::SchemeDisplay::OMIT_HTTP_AND_HTTPS),
is_origin_secure, credentials); TouchToFillView::IsOriginSecure(
network::IsUrlPotentiallyTrustworthy(url)),
credentials);
} }
void TouchToFillController::OnCredentialSelected( void TouchToFillController::OnCredentialSelected(
......
...@@ -9,8 +9,6 @@ ...@@ -9,8 +9,6 @@
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "components/autofill/core/common/autofill_features.h"
#include "components/password_manager/core/browser/origin_credential_store.h" #include "components/password_manager/core/browser/origin_credential_store.h"
#include "components/password_manager/core/browser/stub_password_manager_driver.h" #include "components/password_manager/core/browser/stub_password_manager_driver.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
...@@ -143,16 +141,3 @@ TEST_F(TouchToFillControllerTest, Dismiss) { ...@@ -143,16 +141,3 @@ TEST_F(TouchToFillControllerTest, Dismiss) {
EXPECT_CALL(driver(), TouchToFillDismissed); EXPECT_CALL(driver(), TouchToFillDismissed);
touch_to_fill_controller().OnDismiss(); touch_to_fill_controller().OnDismiss();
} }
TEST_F(TouchToFillControllerTest, CanDisableOnHttps) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeatureWithParameters(
autofill::features::kAutofillTouchToFill,
{
{"insecure-origins-only", "true"},
});
EXPECT_CALL(driver(), TouchToFillDismissed);
EXPECT_CALL(view(), Show).Times(0);
touch_to_fill_controller().Show({}, driver().AsWeakPtr());
}
...@@ -24,6 +24,7 @@ ...@@ -24,6 +24,7 @@
#include "components/autofill/content/renderer/password_generation_agent.h" #include "components/autofill/content/renderer/password_generation_agent.h"
#include "components/autofill/content/renderer/test_password_autofill_agent.h" #include "components/autofill/content/renderer/test_password_autofill_agent.h"
#include "components/autofill/core/common/autofill_constants.h" #include "components/autofill/core/common/autofill_constants.h"
#include "components/autofill/core/common/autofill_features.h"
#include "components/autofill/core/common/autofill_switches.h" #include "components/autofill/core/common/autofill_switches.h"
#include "components/autofill/core/common/form_data.h" #include "components/autofill/core/common/form_data.h"
#include "components/autofill/core/common/form_field_data.h" #include "components/autofill/core/common/form_field_data.h"
...@@ -1761,6 +1762,19 @@ TEST_F(PasswordAutofillAgentTest, DontTryToShowTouchToFillReadonlyPassword) { ...@@ -1761,6 +1762,19 @@ TEST_F(PasswordAutofillAgentTest, DontTryToShowTouchToFillReadonlyPassword) {
password_autofill_agent_->TryToShowTouchToFill(password_element_)); password_autofill_agent_->TryToShowTouchToFill(password_element_));
} }
TEST_F(PasswordAutofillAgentTest, DontShowTouchToFillOnSecurePageIfParamIsSet) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeatureWithParameters(
features::kAutofillTouchToFill, {{"insecure-origins-only", "true"}});
// Reload the page with a secure origin.
LoadHTMLWithUrlOverride(kFormHTML, "https://example.com");
SimulateOnFillPasswordForm(fill_data_);
EXPECT_FALSE(
password_autofill_agent_->TryToShowTouchToFill(password_element_));
}
TEST_F(PasswordAutofillAgentTest, TouchToFillDismissed) { TEST_F(PasswordAutofillAgentTest, TouchToFillDismissed) {
SimulateOnFillPasswordForm(fill_data_); SimulateOnFillPasswordForm(fill_data_);
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/i18n/case_conversion.h" #include "base/i18n/case_conversion.h"
#include "base/metrics/field_trial_params.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/no_destructor.h" #include "base/no_destructor.h"
#include "base/numerics/safe_conversions.h" #include "base/numerics/safe_conversions.h"
...@@ -29,6 +30,7 @@ ...@@ -29,6 +30,7 @@
#include "components/autofill/content/renderer/prefilled_values_detector.h" #include "components/autofill/content/renderer/prefilled_values_detector.h"
#include "components/autofill/content/renderer/renderer_save_password_progress_logger.h" #include "components/autofill/content/renderer/renderer_save_password_progress_logger.h"
#include "components/autofill/core/common/autofill_constants.h" #include "components/autofill/core/common/autofill_constants.h"
#include "components/autofill/core/common/autofill_features.h"
#include "components/autofill/core/common/autofill_util.h" #include "components/autofill/core/common/autofill_util.h"
#include "components/autofill/core/common/form_field_data.h" #include "components/autofill/core/common/form_field_data.h"
#include "components/autofill/core/common/mojom/autofill_types.mojom.h" #include "components/autofill/core/common/mojom/autofill_types.mojom.h"
...@@ -781,6 +783,19 @@ bool PasswordAutofillAgent::ShouldSuppressKeyboard() { ...@@ -781,6 +783,19 @@ bool PasswordAutofillAgent::ShouldSuppressKeyboard() {
bool PasswordAutofillAgent::TryToShowTouchToFill( bool PasswordAutofillAgent::TryToShowTouchToFill(
const WebFormControlElement& control_element) { const WebFormControlElement& control_element) {
// Don't show Touch To Fill if it should only be enabled for insecure origins
// and we are currently on a potentially trustworthy origin.
if (base::GetFieldTrialParamByFeatureAsBool(features::kAutofillTouchToFill,
"insecure-origins-only",
/*default_value=*/false) &&
render_frame()
->GetWebFrame()
->GetDocument()
.GetSecurityOrigin()
.IsPotentiallyTrustworthy()) {
return false;
}
if (touch_to_fill_state_ != TouchToFillState::kShouldShow) if (touch_to_fill_state_ != TouchToFillState::kShouldShow)
return false; return false;
......
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