Revert "Only show recovery button on login screen if configured"

This reverts commit 6da3054. Reason for revert: Missing profile pictures bug b/303844582 Original change's description: > Only show recovery button on login screen if configured > > Bug: b:289355862 > Change-Id: Ifddab9b79e79fc7d70f0f87a983aedaf23fa1faf > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4687055 > Reviewed-by: Martin Bidlingmaier <mbid@google.com> > Commit-Queue: Elie Maamari <emaamari@google.com> > Cr-Commit-Position: refs/heads/main@{#1193539} Bug: b:289355862, b:303844582 Change-Id: I78373b29758b140de2fb9b344a353dc1858631da Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4925199 Reviewed-by: Elie Maamari <emaamari@google.com> Reviewed-by: Denis Kuznetsov <antrim@chromium.org> Commit-Queue: Anastasiia N <anastasiian@chromium.org> Cr-Commit-Position: refs/heads/main@{#1210091}
chromium · Oct 16, 2023 · beb7e41 · beb7e41
1 parent b6e60ce
commit beb7e41
Show file tree

Hide file tree

Showing 9 changed files with 8 additions and 80 deletions.
diff --git a/ash/ash_strings.grd b/ash/ash_strings.grd
@@ -4603,9 +4603,6 @@ Some features are limited to increase battery life.
       <message name="IDS_ASH_LOGIN_RECOVER_USER_BUTTON" desc="Label of the button to recover your password">
         Recover user
       </message>
-      <message name="IDS_ASH_LOGIN_FORGOT_PASSWORD_BUTTON" desc="Label of the button to recover your user data when recovery is not configured">
-        Forgot password
-      </message>
       <message name="IDS_ASH_LOGIN_ERROR_CAPS_LOCK_HINT" desc="A hint text for the login and lock screens that Caps Lock is on">
         Caps Lock is on.
       </message>

diff --git a/ash/ash_strings_grd/IDS_ASH_LOGIN_FORGOT_PASSWORD_BUTTON.png.sha1 b/ash/ash_strings_grd/IDS_ASH_LOGIN_FORGOT_PASSWORD_BUTTON.png.sha1
diff --git a/ash/login/ui/lock_contents_view.cc b/ash/login/ui/lock_contents_view.cc
@@ -715,11 +715,9 @@ void LockContentsView::ApplyUserChanges(
   for (const LoginUserInfo& user : users) {
     UserState* old_state = FindStateForUser(user.basic_user_info.account_id);
     if (old_state) {
-      // Update the state of the recovery factor since it might have change.
-      old_state->is_recovery_configured = user.is_recovery_configured;
       new_users.push_back(std::move(*old_state));
     } else {
-      new_users.emplace_back(user);
+      new_users.push_back(UserState(user));
     }
   }
 
@@ -2251,10 +2249,7 @@ void LockContentsView::ShowAuthErrorMessage() {
       auto recover_user_button = std::make_unique<PillButton>(
           base::BindRepeating(&LockContentsView::RecoverUserButtonPressed,
                               base::Unretained(this)),
-          l10n_util::GetStringUTF16(
-              user_state->is_recovery_configured
-                  ? IDS_ASH_LOGIN_RECOVER_USER_BUTTON
-                  : IDS_ASH_LOGIN_FORGOT_PASSWORD_BUTTON));
+          l10n_util::GetStringUTF16(IDS_ASH_LOGIN_RECOVER_USER_BUTTON));
 
       container->AddChildView(std::move(recover_user_button));
     }

diff --git a/ash/login/ui/user_state.cc b/ash/login/ui/user_state.cc
@@ -20,7 +20,6 @@ UserState::UserState(const LoginUserInfo& user_info)
   disable_auth = !user_info.is_multi_user_sign_in_allowed &&
                  Shell::Get()->session_controller()->GetSessionState() ==
                      session_manager::SessionState::LOGIN_SECONDARY;
-  is_recovery_configured = user_info.is_recovery_configured;
 }
 
 UserState::UserState(UserState&&) = default;

diff --git a/ash/login/ui/user_state.h b/ash/login/ui/user_state.h
@@ -36,7 +36,6 @@ class UserState {
   bool auth_factor_is_hiding_password = false;
   // When present, indicates that the TPM is locked.
   absl::optional<base::TimeDelta> time_until_tpm_unlock = absl::nullopt;
-  bool is_recovery_configured = false;
 };
 
 }  // namespace ash

diff --git a/ash/public/cpp/login_types.h b/ash/public/cpp/login_types.h
@@ -291,9 +291,6 @@ struct ASH_PUBLIC_EXPORT LoginUserInfo {
 
   // True if this user chooses to use 24 hour clock in preference.
   bool use_24hour_clock = false;
-
-  // True if recovery is enabled as a login factor.
-  bool is_recovery_configured = false;
 };
 
 enum class AuthDisabledReason {

diff --git a/chrome/browser/ash/login/screens/user_selection_screen.cc b/chrome/browser/ash/login/screens/user_selection_screen.cc
@@ -13,16 +13,13 @@
 #include "ash/constants/ash_pref_names.h"
 #include "ash/constants/ash_switches.h"
 #include "ash/public/cpp/login/login_utils.h"
-#include "ash/public/cpp/login_screen.h"
-#include "ash/public/cpp/login_screen_model.h"
 #include "base/command_line.h"
 #include "base/functional/bind.h"
 #include "base/functional/callback.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/raw_ptr.h"
 #include "base/metrics/histogram_macros.h"
-#include "base/ranges/algorithm.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "base/trace_event/trace_event.h"
@@ -54,8 +51,6 @@
 #include "chromeos/ash/components/cryptohome/cryptohome_parameters.h"
 #include "chromeos/ash/components/dbus/dbus_thread_manager.h"
 #include "chromeos/ash/components/dbus/userdataauth/userdataauth_client.h"
-#include "chromeos/ash/components/login/auth/auth_performer.h"
-#include "chromeos/ash/components/login/auth/public/auth_session_intent.h"
 #include "chromeos/ash/components/proximity_auth/screenlock_bridge.h"
 #include "chromeos/ash/components/settings/cros_settings_names.h"
 #include "chromeos/dbus/tpm_manager/tpm_manager.pb.h"
@@ -496,7 +491,6 @@ UserSelectionScreen::UserSelectionScreen(DisplayedScreen display_type)
           base::BindRepeating(
               &UserSelectionScreen::OnAllowedInputMethodsChanged,
               base::Unretained(this)));
-
   OnAllowedInputMethodsChanged();
 }
 
@@ -785,7 +779,7 @@ void UserSelectionScreen::AttemptEasyUnlock(const AccountId& account_id) {
 
 std::vector<LoginUserInfo>
 UserSelectionScreen::UpdateAndReturnUserListForAsh() {
-  user_info_list_.clear();
+  std::vector<LoginUserInfo> user_info_list;
 
   const AccountId owner = GetOwnerAccountId();
   const bool is_signin_to_add = IsSigninToAdd();
@@ -898,39 +892,10 @@ UserSelectionScreen::UpdateAndReturnUserListForAsh() {
           account_id, user_info.public_account_info->default_locale);
     }
 
-    auto user_context = std::make_unique<UserContext>(*user);
-
-    auth_performer_.StartAuthSession(
-        std::move(user_context), false, AuthSessionIntent::kVerifyOnly,
-        base::BindOnce(&UserSelectionScreen::OnStartAuthSession,
-                       weak_factory_.GetWeakPtr()));
-
-    user_info_list_.push_back(std::move(user_info));
+    user_info_list.push_back(std::move(user_info));
   }
 
-  return user_info_list_;
-}
-
-void UserSelectionScreen::OnStartAuthSession(
-    bool user_exists,
-    std::unique_ptr<UserContext> user_context,
-    absl::optional<AuthenticationError> error) {
-  auto user_info = base::ranges::find_if(
-      std::begin(user_info_list_), std::end(user_info_list_),
-      [&user_context](const LoginUserInfo& info) {
-        return info.basic_user_info.account_id == user_context->GetAccountId();
-      });
-  if (user_info == std::end(user_info_list_)) {
-    LOG(ERROR) << "Failed to find user info in user info list, could not "
-                  "retrieve status of recovery factor";
-    return;
-  }
-  bool is_recovery_configured =
-      user_context->GetAuthFactorsData().FindRecoveryFactor() != nullptr;
-  if (is_recovery_configured != user_info->is_recovery_configured) {
-    user_info->is_recovery_configured = is_recovery_configured;
-    LoginScreen::Get()->GetModel()->SetUserList(user_info_list_);
-  }
+  return user_info_list;
 }
 
 void UserSelectionScreen::SetUsersLoaded(bool loaded) {

diff --git a/chrome/browser/ash/login/screens/user_selection_screen.h b/chrome/browser/ash/login/screens/user_selection_screen.h
@@ -18,7 +18,6 @@
 #include "chrome/browser/ash/login/user_online_signin_notifier.h"
 #include "chrome/browser/ash/system/system_clock.h"
 #include "chromeos/ash/components/dbus/cryptohome/rpc.pb.h"
-#include "chromeos/ash/components/login/auth/auth_performer.h"
 #include "chromeos/ash/components/proximity_auth/screenlock_bridge.h"
 #include "components/account_id/account_id.h"
 #include "components/session_manager/core/session_manager_observer.h"
@@ -33,8 +32,6 @@ namespace ash {
 class SmartLockService;
 class UserBoardView;
 struct LoginUserInfo;
-class UserContext;
-class AuthenticationError;
 
 enum class DisplayedScreen { SIGN_IN_SCREEN, USER_ADDING_SCREEN, LOCK_SCREEN };
 
@@ -123,10 +120,6 @@ class UserSelectionScreen
                            bool reauth_required);
   void OnAllowedInputMethodsChanged();
 
-  void OnStartAuthSession(bool user_exists,
-                          std::unique_ptr<UserContext> user_context,
-                          absl::optional<AuthenticationError> error);
-
   // Purpose of the screen.
   const DisplayedScreen display_type_;
 
@@ -173,10 +166,6 @@ class UserSelectionScreen
                           UserOnlineSigninNotifier::Observer>
       scoped_observation_{this};
 
-  AuthPerformer auth_performer_{UserDataAuthClient::Get()};
-
-  std::vector<LoginUserInfo> user_info_list_;
-
   base::WeakPtrFactory<UserSelectionScreen> weak_factory_{this};
 };
 

diff --git a/chromeos/ash/components/dbus/userdataauth/fake_userdataauth_client.cc b/chromeos/ash/components/dbus/userdataauth/fake_userdataauth_client.cc
@@ -534,22 +534,10 @@ void FakeUserDataAuthClient::TestApi::AddKey(
     const cryptohome::Key& key) {
   UserCryptohomeState& user_state = GetUserState(account_id);
 
-  auto [label, fake_auth_factor] = KeyToFakeAuthFactor(
-      key, FakeUserDataAuthClient::Get()->enable_auth_check_);
-
   const auto [factor_it, was_inserted] =
-      user_state.auth_factors.emplace(label, fake_auth_factor);
-
-  // In some tests, we might add a gaia password to a user which already
-  // has one, because we automatically give users a password during
-  // `FakeUserDataAuthClient::StartAuthSession`, which might have been
-  // called prior. In that case, we override the value of the password factor.
-  if (factor_it->first != kCryptohomeGaiaKeyLabel) {
-    CHECK(was_inserted) << "Factor already exists";
-  } else {
-    // Override value of password factor.
-    factor_it->second = fake_auth_factor;
-  }
+      user_state.auth_factors.insert(KeyToFakeAuthFactor(
+          key, FakeUserDataAuthClient::Get()->enable_auth_check_));
+  CHECK(was_inserted) << "Factor already exists";
 }
 
 void FakeUserDataAuthClient::TestApi::AddRecoveryFactor(