CrOS - Prevent multiple login attempts

LoginPinInput and LoginPasswordView view should not accept digits
from the pin pad when they are read only. This fixes an issue
that would trigger multiple authentication attempts when the digits
are inserted too fast on the pin pad when using a PIN.

Additionally, these elements will make themselves read only prior
to invoking an authentication attempt and prevent any further
attempts until they are no longer ReadOnly.

Fixed: 1133902
Bug: 1136379
Change-Id: I12dcb158b856ac1407f55220af583e9df2a39b52
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2489883Reviewed-by: Thomas Tellier <tellier@google.com>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Reviewed-by: Roman Aleksandrov <raleksandrov@google.com>
Commit-Queue: Renato Silva <rrsilva@google.com>
Cr-Commit-Position: refs/heads/master@{#823973}

ash/login/ui/login_auth_user_view_unittest.cc

@@ -358,6 +358,76 @@ TEST_P(LoginAuthUserViewUnittest, PinAndPasswordFieldMode) {
   }
 }
 
+// Tests that clicking on the digits on the pin pad inserts digits on the
+// password field, but it does not when the field is read only.
+TEST_P(LoginAuthUserViewUnittest, PinPadWorksForPasswordField) {
+  LoginAuthUserView::TestApi auth_test(view_);
+  LoginPasswordView::TestApi password_test(auth_test.password_view());
+
+  // Set up PIN without auto submit.
+  SetAuthPin(/*autosubmit_length*/ 0);
+
+  LoginPinView::TestApi pin_pad_api{auth_test.pin_view()};
+  const auto password = base::ASCIIToUTF16("123");
+
+  // Click on "1" "2" and "3"
+  pin_pad_api.ClickOnDigit(1);
+  pin_pad_api.ClickOnDigit(2);
+  pin_pad_api.ClickOnDigit(3);
+
+  EXPECT_EQ(password_test.textfield()->GetText(), password);
+
+  // Set the field to read only. Clicking on the digits has no effect.
+  auth_test.password_view()->SetReadOnly(true);
+  pin_pad_api.ClickOnDigit(4);
+  pin_pad_api.ClickOnDigit(5);
+  pin_pad_api.ClickOnDigit(6);
+
+  EXPECT_EQ(password_test.textfield()->GetText(), password);
+}
+
+// Tests that clicking on the digits on the pin pad inserts digits on the
+// PIN input field, but it does not when the field is read only.
+TEST_P(LoginAuthUserViewUnittest, PinPadWorksForPinInputField) {
+  if (!autosubmit_feature_enabled_)
+    return;
+
+  auto client = std::make_unique<MockLoginScreenClient>();
+  LoginAuthUserView::TestApi auth_test(view_);
+  LoginUserView* user_view(auth_test.user_view());
+  LoginPinInputView::TestApi pin_input_test{auth_test.pin_input_view()};
+  SetUserCount(1);
+  EXPECT_CALL(*client, AuthenticateUserWithPasswordOrPin_(
+                           user_view->current_user().basic_user_info.account_id,
+                           "123456", /*password*/
+                           true,     /*authenticated_by_pin*/
+                           _ /*callback*/));
+
+  // Set up PIN with auto submit.
+  SetAuthPin(/*autosubmit_length*/ 6);
+
+  LoginPinView::TestApi pin_pad_api{auth_test.pin_view()};
+  const auto pin = std::string("123456");
+
+  // Click on "1" "2" and "3"
+  pin_pad_api.ClickOnDigit(1);
+  pin_pad_api.ClickOnDigit(2);
+  pin_pad_api.ClickOnDigit(3);
+
+  // Ignore input when read only.
+  auth_test.pin_input_view()->SetReadOnly(true);
+  pin_pad_api.ClickOnDigit(0);
+  auth_test.pin_input_view()->SetReadOnly(false);
+
+  pin_pad_api.ClickOnDigit(4);
+  pin_pad_api.ClickOnDigit(5);
+  pin_pad_api.ClickOnDigit(6);
+
+  ASSERT_TRUE(pin_input_test.GetCode().has_value());
+  EXPECT_EQ(pin_input_test.GetCode().value(), pin);
+  base::RunLoop().RunUntilIdle();
+}
+
 // Tests the correctness of InputFieldModes used for PIN autosubmit.
 TEST_P(LoginAuthUserViewUnittest, PinAutosubmitFieldModes) {
   LoginAuthUserView::TestApi auth_test(view_);

ash/login/ui/login_password_view.cc

@@ -655,6 +655,9 @@ void LoginPasswordView::Clear() {
 }
 
 void LoginPasswordView::InsertNumber(int value) {
+  if (textfield_->GetReadOnly())
+    return;
+
   if (!textfield_->HasFocus()) {
     // RequestFocus on textfield to activate cursor.
     textfield_->RequestFocus();
@@ -801,6 +804,7 @@ void LoginPasswordView::SubmitPassword() {
   DCHECK(IsPasswordSubmittable());
   if (textfield_->GetReadOnly())
     return;
+  SetReadOnly(true);
   on_submit_.Run(textfield_->GetText());
 }
 

ash/login/ui/login_password_view.h

@@ -30,7 +30,9 @@ enum class EasyUnlockIconId;
 
 // Contains a textfield and a submit button. When the display password button
 // feature is enabled, the textfield contains a button in the form of an eye
-// icon that the user can click on to reveal the password.
+// icon that the user can click on to reveal the password. Submitting a password
+// will make it read only and prevent further submissions until the controller
+// sets ReadOnly to false again.
 //
 // This view is always rendered via layers.
 //

ash/login/ui/login_password_view_test.cc

@@ -172,6 +172,9 @@ TEST_F(LoginPasswordViewTest, PasswordSubmitIncludesPasswordText) {
 
   ASSERT_TRUE(password_.has_value());
   EXPECT_EQ(base::ASCIIToUTF16("abc1"), *password_);
+
+  // Expect the password field to be read only after submitting.
+  EXPECT_EQ(test_api.textfield()->GetReadOnly(), true);
 }
 
 // Verifies that password submit works when clicking the submit button.
@@ -189,6 +192,9 @@ TEST_F(LoginPasswordViewTest, PasswordSubmitViaButton) {
 
   ASSERT_TRUE(password_.has_value());
   EXPECT_EQ(base::ASCIIToUTF16("abc1"), *password_);
+
+  // Expect the password field to be read only after submitting.
+  EXPECT_EQ(test_api.textfield()->GetReadOnly(), true);
 }
 
 // Verifies that pressing 'Return' on the password field triggers an
@@ -227,6 +233,7 @@ TEST_F(LoginPasswordViewTest, PasswordSubmitClearsPassword) {
   // Clear password.
   password_.reset();
   view_->Clear();
+  view_->SetReadOnly(false);
   EXPECT_TRUE(is_password_field_empty_);
 
   // Submit 'b' password.

ash/login/ui/login_pin_input_view.cc

@@ -91,6 +91,7 @@ void LoginPinInput::OnModified(bool last_field_active, bool complete) {
   if (last_field_active && complete) {
     base::Optional<std::string> user_input = GetCode();
     DCHECK(on_submit_);
+    SetReadOnly(true);
     on_submit_.Run(base::UTF8ToUTF16(user_input.value_or(std::string())));
   }
 }
@@ -150,6 +151,10 @@ views::View* LoginPinInputView::TestApi::code_input() {
   return view_->code_input_;
 }
 
+base::Optional<std::string> LoginPinInputView::TestApi::GetCode() {
+  return view_->code_input_->GetCode();
+}
+
 LoginPinInputView::LoginPinInputView(const LoginPalette& palette)
     : length_(kDefaultLength), palette_(palette) {
   SetLayoutManager(std::make_unique<views::FillLayout>());
@@ -222,7 +227,8 @@ void LoginPinInputView::Backspace() {
 
 void LoginPinInputView::InsertDigit(int digit) {
   DCHECK(code_input_);
-  code_input_->InsertDigit(digit);
+  if (!is_read_only_)
+    code_input_->InsertDigit(digit);
 }
 
 void LoginPinInputView::SetReadOnly(bool read_only) {

ash/login/ui/login_pin_input_view.h

@@ -43,6 +43,7 @@ class ASH_EXPORT LoginPinInputView : public views::View {
     ~TestApi();
 
     views::View* code_input();
+    base::Optional<std::string> GetCode();
 
    private:
     LoginPinInputView* const view_;

ash/login/ui/login_pin_view.cc

@@ -21,6 +21,7 @@
 #include "ui/accessibility/ax_enums.mojom.h"
 #include "ui/accessibility/ax_node_data.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/events/base_event_utils.h"
 #include "ui/events/keycodes/dom/dom_code.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gfx/paint_vector_icon.h"
@@ -434,6 +435,12 @@ void LoginPinView::TestApi::SetBackspaceTimers(
                                          std::move(repeat_timer));
 }
 
+void LoginPinView::TestApi::ClickOnDigit(int number) const {
+  ui::MouseEvent event(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
+                       ui::EventTimeForNow(), 0, 0);
+  GetButton(number)->OnEvent(&event);
+}
+
 LoginPinView::LoginPinView(Style keyboard_style,
                            const LoginPalette& palette,
                            const OnPinKey& on_key,

ash/login/ui/login_pin_view.h

@@ -80,6 +80,8 @@ class ASH_EXPORT LoginPinView : public NonAccessibleView {
     void SetBackspaceTimers(std::unique_ptr<base::OneShotTimer> delay_timer,
                             std::unique_ptr<base::RepeatingTimer> repeat_timer);
 
+    void ClickOnDigit(int number) const;
+
    private:
     LoginPinView* const view_;
   };