ash: Remove GAIA password option from in-session auth dialog

It is a security concern that users potentially get trained to use
their GAIA password for 3rd party websites / apps, so after discussing
with the browser side and GAIA team, we decided to not offer the
password option in this auth dialog.

The dialog will be triggered if either PIN or fingerprint is available.

Bug: b/156258540
Change-Id: I9dffaa1fed36ca476cb5976d5229d5f49278e240
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2487826
Commit-Queue: Yicheng Li <yichengli@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819675}

ash/in_session_auth/auth_dialog_contents_view.cc

@@ -413,16 +413,15 @@ views::LabelButton* AuthDialogContentsView::AddButton(const std::string& text,
   return view;
 }
 
-void AuthDialogContentsView::OnAuthSubmit(const base::string16& password) {
-  InSessionAuthDialogController::Get()->AuthenticateUserWithPasswordOrPin(
-      base::UTF16ToUTF8(password),
-      base::BindOnce(&AuthDialogContentsView::OnPasswordOrPinAuthComplete,
+void AuthDialogContentsView::OnAuthSubmit(const base::string16& pin) {
+  InSessionAuthDialogController::Get()->AuthenticateUserWithPin(
+      base::UTF16ToUTF8(pin),
+      base::BindOnce(&AuthDialogContentsView::OnPinAuthComplete,
                      weak_factory_.GetWeakPtr()));
 }
 
-// TODO(b/156258540): Clear password/PIN if auth failed and retry is allowed.
-void AuthDialogContentsView::OnPasswordOrPinAuthComplete(
-    base::Optional<bool> success) {}
+// TODO(b/156258540): Clear PIN if auth failed and retry is allowed.
+void AuthDialogContentsView::OnPinAuthComplete(base::Optional<bool> success) {}
 
 void AuthDialogContentsView::OnFingerprintAuthComplete(
     bool success,

ash/in_session_auth/auth_dialog_contents_view.h

@@ -83,8 +83,8 @@ class AuthDialogContentsView : public views::View,
   // Called when the user submits password or PIN.
   void OnAuthSubmit(const base::string16& password);
 
-  // Called when password/PIN authentication of the user completes.
-  void OnPasswordOrPinAuthComplete(base::Optional<bool> success);
+  // Called when PIN authentication of the user completes.
+  void OnPinAuthComplete(base::Optional<bool> success);
 
   // Called when fingerprint authentication completes.
   void OnFingerprintAuthComplete(bool success,

ash/in_session_auth/in_session_auth_dialog_controller_impl.cc

@@ -35,8 +35,8 @@ void InSessionAuthDialogControllerImpl::ShowAuthenticationDialog(
 
   AccountId account_id =
       Shell::Get()->session_controller()->GetActiveAccountId();
-  // Password should always be available.
-  uint32_t auth_methods = AuthDialogContentsView::kAuthPassword;
+  // GAIA password option is not offered.
+  uint32_t auth_methods = 0;
 
   if (client_->IsFingerprintAuthAvailable(account_id)) {
     client_->StartFingerprintAuthSession(
@@ -73,6 +73,14 @@ void InSessionAuthDialogControllerImpl::OnPinCanAuthenticate(
   if (pin_auth_available)
     auth_methods |= AuthDialogContentsView::kAuthPin;
 
+  if (auth_methods == 0) {
+    // If neither fingerprint nor PIN is available, we shouldn't receive the
+    // request.
+    LOG(ERROR) << "Neither fingerprint nor PIN is available.";
+    Cancel();
+    return;
+  }
+
   dialog_ = std::make_unique<InSessionAuthDialog>(auth_methods);
 }
 
@@ -87,16 +95,20 @@ void InSessionAuthDialogControllerImpl::DestroyAuthenticationDialog() {
   dialog_.reset();
 }
 
-void InSessionAuthDialogControllerImpl::AuthenticateUserWithPasswordOrPin(
-    const std::string& password,
+void InSessionAuthDialogControllerImpl::AuthenticateUserWithPin(
+    const std::string& pin,
     OnAuthenticateCallback callback) {
   DCHECK(client_);
 
   // TODO(b/156258540): Check that PIN is enabled / set up for this user.
-  bool authenticated_by_pin = base::ContainsOnlyChars(password, "0123456789");
+
+  if (!base::ContainsOnlyChars(pin, "0123456789")) {
+    OnAuthenticateComplete(std::move(callback), false);
+    return;
+  }
 
   client_->AuthenticateUserWithPasswordOrPin(
-      password, authenticated_by_pin,
+      pin, true,
       base::BindOnce(&InSessionAuthDialogControllerImpl::OnAuthenticateComplete,
                      weak_factory_.GetWeakPtr(), std::move(callback)));
 }

ash/in_session_auth/in_session_auth_dialog_controller_impl.h

@@ -33,9 +33,8 @@ class InSessionAuthDialogControllerImpl : public InSessionAuthDialogController {
   void SetClient(InSessionAuthDialogClient* client) override;
   void ShowAuthenticationDialog(FinishCallback finish_callback) override;
   void DestroyAuthenticationDialog() override;
-  void AuthenticateUserWithPasswordOrPin(
-      const std::string& password,
-      OnAuthenticateCallback callback) override;
+  void AuthenticateUserWithPin(const std::string& pin,
+                               OnAuthenticateCallback callback) override;
   void AuthenticateUserWithFingerprint(
       base::OnceCallback<void(bool, FingerprintState)> callback) override;
   void Cancel() override;

ash/in_session_auth/in_session_auth_dialog_controller_impl_unittest.cc

@@ -17,23 +17,23 @@ namespace {
 
 using InSessionAuthDialogControllerImplTest = AshTestBase;
 
-TEST_F(InSessionAuthDialogControllerImplTest, PasswordAuthSuccess) {
+TEST_F(InSessionAuthDialogControllerImplTest, PinAuthSuccess) {
   InSessionAuthDialogController* controller =
       Shell::Get()->in_session_auth_dialog_controller();
   auto client = std::make_unique<MockInSessionAuthDialogClient>();
 
-  std::string password = "password";
+  std::string pin = "123456";
 
   EXPECT_CALL(*client, AuthenticateUserWithPasswordOrPin(
-                           password, /* authenticated_by_pin = */ false, _))
-      .WillOnce([](const std::string& password, bool authenticated_by_pin,
+                           pin, /* authenticated_by_pin = */ true, _))
+      .WillOnce([](const std::string& pin, bool authenticated_by_pin,
                    base::OnceCallback<void(bool success)> controller_callback) {
         std::move(controller_callback).Run(true);
       });
 
   base::Optional<bool> view_callback_result;
-  controller->AuthenticateUserWithPasswordOrPin(
-      password,
+  controller->AuthenticateUserWithPin(
+      pin,
       /* View callback will be executed during controller callback. */
       base::BindLambdaForTesting(
           [&view_callback_result](base::Optional<bool> did_auth) {
@@ -44,23 +44,23 @@ TEST_F(InSessionAuthDialogControllerImplTest, PasswordAuthSuccess) {
   EXPECT_TRUE(*view_callback_result);
 }
 
-TEST_F(InSessionAuthDialogControllerImplTest, PasswordAuthFail) {
+TEST_F(InSessionAuthDialogControllerImplTest, PinAuthFail) {
   InSessionAuthDialogController* controller =
       Shell::Get()->in_session_auth_dialog_controller();
   auto client = std::make_unique<MockInSessionAuthDialogClient>();
 
-  std::string password = "password";
+  std::string pin = "123456";
 
   EXPECT_CALL(*client, AuthenticateUserWithPasswordOrPin(
-                           password, /* authenticated_by_pin = */ false, _))
-      .WillOnce([](const std::string& password, bool authenticated_by_pin,
+                           pin, /* authenticated_by_pin = */ true, _))
+      .WillOnce([](const std::string& pin, bool authenticated_by_pin,
                    base::OnceCallback<void(bool success)> controller_callback) {
         std::move(controller_callback).Run(false);
       });
 
   base::Optional<bool> view_callback_result;
-  controller->AuthenticateUserWithPasswordOrPin(
-      password,
+  controller->AuthenticateUserWithPin(
+      pin,
       /* View callback will be executed during controller callback. */
       base::BindLambdaForTesting(
           [&view_callback_result](base::Optional<bool> did_auth) {

ash/public/cpp/in_session_auth_dialog_controller.h

@@ -34,12 +34,11 @@ class ASH_PUBLIC_EXPORT InSessionAuthDialogController {
   // Destroys the authentication dialog.
   virtual void DestroyAuthenticationDialog() = 0;
 
-  // Takes a password or PIN and sends it to InSessionAuthDialogClient to
+  // Takes a PIN and sends it to InSessionAuthDialogClient to
   // authenticate. The InSessionAuthDialogClient should already know the current
   // session's active user, so the user account is not provided here.
-  virtual void AuthenticateUserWithPasswordOrPin(
-      const std::string& password,
-      OnAuthenticateCallback callback) = 0;
+  virtual void AuthenticateUserWithPin(const std::string& pin,
+                                       OnAuthenticateCallback callback) = 0;
 
   // Requests ChromeOS to report fingerprint scan result through |callback|.
   virtual void AuthenticateUserWithFingerprint(

chrome/browser/ui/ash/in_session_auth_dialog_client.cc

@@ -119,7 +119,6 @@ void InSessionAuthDialogClient::AuthenticateUserWithPasswordOrPin(
         user_context.GetAccountId(), *user_context.GetKey(),
         base::BindOnce(&InSessionAuthDialogClient::OnPinAttemptDone,
                        weak_factory_.GetWeakPtr(), user_context));
-    // OnPinAttemptDone will call AuthenticateWithPassword if attempt fails.
     return;
   }
 
@@ -143,8 +142,11 @@ void InSessionAuthDialogClient::OnPinAttemptDone(
     }
     OnAuthSuccess(user_context);
   } else {
-    // PIN authentication has failed; try submitting as a normal password.
-    AuthenticateWithPassword(user_context);
+    // Do not try submitting as password.
+    if (pending_auth_state_) {
+      std::move(pending_auth_state_->callback).Run(false);
+      pending_auth_state_.reset();
+    }
   }
 }
 

chrome/browser/ui/ash/in_session_auth_dialog_client_unittest.cc

@@ -42,9 +42,8 @@ class FakeInSessionAuthDialogController
   void SetClient(ash::InSessionAuthDialogClient* client) override {}
   void ShowAuthenticationDialog(FinishCallback callback) override {}
   void DestroyAuthenticationDialog() override {}
-  void AuthenticateUserWithPasswordOrPin(
-      const std::string& password,
-      OnAuthenticateCallback callback) override {}
+  void AuthenticateUserWithPin(const std::string& pin,
+                               OnAuthenticateCallback callback) override {}
   void AuthenticateUserWithFingerprint(
       base::OnceCallback<void(bool, ash::FingerprintState)> callback) override {
   }