PaymentRequest: Don't auto-focus 'Continue' button
Per new security guidelines in
docs/security/security-considerations-for-browser-ui.md:
"# Don't have a default-selected accept button
If your dialog or UI has a call-to-action triggered by a button that is
default-selected, the dialog is subject to keyjacking. An evil webpage
can trick a user into mashing or repeatedly hitting the Enter key, and
then trigger your dialog to show, causing the user to unknowingly
accept. Users should have to make an explicit selection on security- or
privacy-sensitive browser UI surfaces."
Bug: 1403539
Change-Id: I67791955574dd9ba97705c4e979a353e70a4e8e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4130374
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1088729}
diff --git a/chrome/browser/ui/views/payments/payment_request_sheet_controller.cc b/chrome/browser/ui/views/payments/payment_request_sheet_controller.cc
index 16c16ad..0173504 100644
--- a/chrome/browser/ui/views/payments/payment_request_sheet_controller.cc
+++ b/chrome/browser/ui/views/payments/payment_request_sheet_controller.cc
@@ -534,12 +534,8 @@
}
views::View* PaymentRequestSheetController::GetFirstFocusedView() {
- if (primary_button_ && primary_button_->GetEnabled())
- return primary_button_;
-
- if (secondary_button_)
- return secondary_button_;
-
+ // Do not focus either of the buttons, per guidelines in
+ // docs/security/security-considerations-for-browser-ui.md
DCHECK(content_view_);
return content_view_;
}
diff --git a/chrome/browser/ui/views/payments/payment_sheet_view_controller_browsertest.cc b/chrome/browser/ui/views/payments/payment_sheet_view_controller_browsertest.cc
index a7e0ae7..a13d6ad0 100644
--- a/chrome/browser/ui/views/payments/payment_sheet_view_controller_browsertest.cc
+++ b/chrome/browser/ui/views/payments/payment_sheet_view_controller_browsertest.cc
@@ -19,19 +19,56 @@
namespace payments {
+using PaymentSheetViewControllerTest = PaymentRequestBrowserTestBase;
+
+// The 'Continue' or 'Cancel' buttons should not be auto-focused; see
+// https://crbug.com/1403539
+IN_PROC_BROWSER_TEST_F(PaymentSheetViewControllerTest,
+ ContinueIsNotAutoFocused) {
+ // Installs two apps so that the Payment Request UI will be shown.
+ std::string a_method_name;
+ InstallPaymentApp("a.com", "/payment_request_success_responder.js",
+ &a_method_name);
+ std::string b_method_name;
+ InstallPaymentApp("b.com", "/payment_request_success_responder.js",
+ &b_method_name);
+
+ NavigateTo("/payment_request_no_shipping_test.html");
+ InvokePaymentRequestUIWithJs(content::JsReplace(
+ "buyWithMethods([{supportedMethods:$1}, {supportedMethods:$2}]);",
+ a_method_name, b_method_name));
+
+ EXPECT_TRUE(IsViewVisible(DialogViewID::PAY_BUTTON));
+ EXPECT_TRUE(IsViewVisible(DialogViewID::CANCEL_BUTTON));
+ EXPECT_TRUE(IsPayButtonEnabled());
+
+ // Neither of the actionable buttons should receive default focus.
+ EXPECT_FALSE(dialog_view()
+ ->GetViewByID(static_cast<int>(DialogViewID::PAY_BUTTON))
+ ->HasFocus());
+ EXPECT_FALSE(dialog_view()
+ ->GetViewByID(static_cast<int>(DialogViewID::CANCEL_BUTTON))
+ ->HasFocus());
+}
+
using PaymentSheetViewControllerNoShippingTest = PaymentRequestBrowserTestBase;
// If shipping and contact info are not requested, their rows should not be
// present.
IN_PROC_BROWSER_TEST_F(PaymentSheetViewControllerNoShippingTest,
NoShippingNoContactRows) {
- std::string payment_method_name;
+ // Installs two apps so that the Payment Request UI will be shown.
+ std::string a_method_name;
InstallPaymentApp("a.com", "/payment_request_success_responder.js",
- &payment_method_name);
+ &a_method_name);
+ std::string b_method_name;
+ InstallPaymentApp("b.com", "/payment_request_success_responder.js",
+ &b_method_name);
NavigateTo("/payment_request_no_shipping_test.html");
- InvokePaymentRequestUIWithJs("buyWithMethods([{supportedMethods:'" +
- payment_method_name + "'}]);");
+ InvokePaymentRequestUIWithJs(content::JsReplace(
+ "buyWithMethods([{supportedMethods:$1}, {supportedMethods:$2}]);",
+ a_method_name, b_method_name));
EXPECT_NE(nullptr, dialog_view()->GetViewByID(static_cast<int>(
DialogViewID::PAYMENT_SHEET_SUMMARY_SECTION)));
