Disable lock if no credentials are present

BUG=335086
TEST=Ensure new-profile-management and account-consistency are not
enabled. Then sign in a profile. Then enable new-profile-management.
The "lock" button of the avatar menu should be present but disabled.

Review URL: https://codereview.chromium.org/497783002

Cr-Commit-Position: refs/heads/master@{#291896}

chrome/browser/signin/local_auth.cc

@@ -94,6 +94,14 @@ bool DecodePasswordHashRecord(const std::string& encoded,
   return OSCrypt::DecryptString(unbase64, decoded);
 }
 
+size_t GetProfileInfoIndexOfProfile(const Profile* profile) {
+  DCHECK(profile);
+
+  ProfileInfoCache& info =
+      g_browser_process->profile_manager()->GetProfileInfoCache();
+  return info.GetIndexOfProfileWithPath(profile->GetPath());
+}
+
 }  // namespace
 
 namespace chrome {
@@ -107,6 +115,10 @@ void RegisterLocalAuthPrefs(user_prefs::PrefRegistrySyncable* registry) {
 
 void SetLocalAuthCredentials(size_t info_index,
                              const std::string& password) {
+  if (info_index == std::string::npos) {
+    NOTREACHED();
+    return;
+  }
   DCHECK(password.length());
 
   // Salt should be random data, as long as the hash length, and different with
@@ -134,20 +146,16 @@ void SetLocalAuthCredentials(size_t info_index,
 
 void SetLocalAuthCredentials(const Profile* profile,
                              const std::string& password) {
-  DCHECK(profile);
+  SetLocalAuthCredentials(GetProfileInfoIndexOfProfile(profile), password);
+}
 
-  ProfileInfoCache& info =
-      g_browser_process->profile_manager()->GetProfileInfoCache();
-  size_t info_index = info.GetIndexOfProfileWithPath(profile->GetPath());
+bool ValidateLocalAuthCredentials(size_t info_index,
+                                  const std::string& password) {
   if (info_index == std::string::npos) {
     NOTREACHED();
-    return;
+    return false;
   }
-  SetLocalAuthCredentials(info_index, password);
-}
 
-bool ValidateLocalAuthCredentials(size_t info_index,
-                                  const std::string& password) {
   std::string record;
   char encoding;
 
@@ -186,16 +194,27 @@ bool ValidateLocalAuthCredentials(size_t info_index,
 
 bool ValidateLocalAuthCredentials(const Profile* profile,
                                   const std::string& password) {
-  DCHECK(profile);
+  return ValidateLocalAuthCredentials(GetProfileInfoIndexOfProfile(profile),
+                                      password);
+}
 
-  ProfileInfoCache& info =
-      g_browser_process->profile_manager()->GetProfileInfoCache();
-  size_t info_index = info.GetIndexOfProfileWithPath(profile->GetPath());
-  if (info_index == std::string::npos) {
-    NOTREACHED();  // This should never happen but fail safely if it does.
+bool LocalAuthCredentialsExist(size_t profile_info_index) {
+  if (profile_info_index == std::string::npos) {
+    NOTREACHED();
     return false;
   }
-  return ValidateLocalAuthCredentials(info_index, password);
+
+  ProfileInfoCache& info =
+      g_browser_process->profile_manager()->GetProfileInfoCache();
+
+  std::string encodedhash =
+      info.GetLocalAuthCredentialsOfProfileAtIndex(profile_info_index);
+
+  return !encodedhash.empty();
+}
+
+bool LocalAuthCredentialsExist(const Profile* profile) {
+  return LocalAuthCredentialsExist(GetProfileInfoIndexOfProfile(profile));
 }
 
 }  // namespace chrome

chrome/browser/signin/local_auth.h

@@ -33,6 +33,10 @@ bool ValidateLocalAuthCredentials(size_t profile_info_index,
 bool ValidateLocalAuthCredentials(const Profile* profile,
                                   const std::string& password);
 
+bool LocalAuthCredentialsExist(size_t profile_info_index);
+
+bool LocalAuthCredentialsExist(const Profile* profile);
+
 }  // namespace chrome
 
 #endif  // CHROME_BROWSER_SIGNIN_LOCAL_AUTH_H_

chrome/browser/signin/local_auth_unittest.cc

@@ -35,7 +35,9 @@ TEST(LocalAuthTest, SetAndCheckCredentials) {
   std::string password("Some Password");
   EXPECT_FALSE(ValidateLocalAuthCredentials(prof, password));
 
+  EXPECT_FALSE(LocalAuthCredentialsExist(prof));
   SetLocalAuthCredentials(prof, password);
+  EXPECT_TRUE(LocalAuthCredentialsExist(prof));
   std::string passhash = cache.GetLocalAuthCredentialsOfProfileAtIndex(0);
 
   // We perform basic validation on the written record to ensure bugs don't slip

chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm

@@ -22,6 +22,7 @@
 #include "chrome/browser/profiles/profile_metrics.h"
 #include "chrome/browser/profiles/profile_window.h"
 #include "chrome/browser/profiles/profiles_state.h"
+#include "chrome/browser/signin/local_auth.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_header_helper.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
@@ -826,7 +827,7 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
 
 // Creates the "Not you" and Lock option buttons.
 - (NSView*)createOptionsViewWithRect:(NSRect)rect
-                          enableLock:(BOOL)enableLock;
+                         displayLock:(BOOL)displayLock;
 
 // Creates the account management view for the active profile.
 - (NSView*)createCurrentProfileAccountsView:(NSRect)rect;
@@ -1140,7 +1141,7 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
   base::scoped_nsobject<NSMutableArray> otherProfiles(
       [[NSMutableArray alloc] init]);
   // Local and guest profiles cannot lock their profile.
-  bool enableLock = false;
+  bool displayLock = false;
 
   // Loop over the profiles in reverse, so that they are sorted by their
   // y-coordinate, and separate them into active and "other" profiles.
@@ -1162,7 +1163,7 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
         }
       }
       currentProfileView = [self createCurrentProfileView:item];
-      enableLock = switches::IsNewProfileManagement() && item.signed_in;
+      displayLock = switches::IsNewProfileManagement() && item.signed_in;
     } else {
       [otherProfiles addObject:[self createOtherProfileView:i]];
     }
@@ -1178,7 +1179,7 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
   // Option buttons.
   NSRect rect = NSMakeRect(0, yOffset, kFixedMenuWidth, 0);
   NSView* optionsView = [self createOptionsViewWithRect:rect
-                                             enableLock:enableLock];
+                                            displayLock:displayLock];
   [container addSubview:optionsView];
   rect.origin.y = NSMaxY([optionsView frame]);
 
@@ -1690,19 +1691,21 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
 }
 
 - (NSView*)createOptionsViewWithRect:(NSRect)rect
-                          enableLock:(BOOL)enableLock {
+                         displayLock:(BOOL)displayLock {
   NSRect viewRect = NSMakeRect(0, 0,
                                rect.size.width,
                                kBlueButtonHeight + kSmallVerticalSpacing);
   base::scoped_nsobject<NSView> container([[NSView alloc] initWithFrame:rect]);
 
-  if (enableLock) {
+  if (displayLock) {
     NSButton* lockButton =
         [self hoverButtonWithRect:viewRect
                              text:l10n_util::GetNSString(
                                   IDS_PROFILES_PROFILE_SIGNOUT_BUTTON)
                   imageResourceId:IDR_ICON_PROFILES_MENU_LOCK
                            action:@selector(lockProfile:)];
+    if (!chrome::LocalAuthCredentialsExist(browser_->profile()))
+      [lockButton setEnabled:NO];
     [container addSubview:lockButton];
     viewRect.origin.y = NSMaxY([lockButton frame]);
 

chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm

@@ -451,3 +451,69 @@ TEST_F(ProfileChooserControllerTest, AccountManagementLayout) {
   EXPECT_EQ(@selector(hideAccountManagement:), [link action]);
   EXPECT_EQ(controller(), [link target]);
 }
+
+TEST_F(ProfileChooserControllerTest, SignedInProfileLockDisabled) {
+  switches::EnableNewProfileManagementForTesting(
+      CommandLine::ForCurrentProcess());
+  // Sign in the first profile.
+  ProfileInfoCache* cache = testing_profile_manager()->profile_info_cache();
+  cache->SetUserNameOfProfileAtIndex(0, base::ASCIIToUTF16(kEmail));
+  cache->SetLocalAuthCredentialsOfProfileAtIndex(0, std::string());
+
+  StartProfileChooserController();
+  NSArray* subviews = [[[controller() window] contentView] subviews];
+  ASSERT_EQ(1U, [subviews count]);
+  subviews = [[subviews objectAtIndex:0] subviews];
+
+  // Three profiles means we should have one active card, one separator, one
+  // option buttons view and a lock view. We also have an update promo for the
+  // new avatar menu.
+  // TODO(noms): Enforcing 5U fails on the waterfall debug bots, but it's not
+  // reproducible anywhere else.
+  ASSERT_GE([subviews count], 4U);
+
+  // There will be three buttons and two separators in the option buttons view.
+  NSArray* buttonSubviews = [[subviews objectAtIndex:0] subviews];
+  ASSERT_EQ(5U, [buttonSubviews count]);
+
+  // There should be a lock button.
+  NSButton* lockButton =
+      base::mac::ObjCCast<NSButton>([buttonSubviews objectAtIndex:0]);
+  ASSERT_TRUE(lockButton);
+  EXPECT_EQ(@selector(lockProfile:), [lockButton action]);
+  EXPECT_EQ(controller(), [lockButton target]);
+  EXPECT_FALSE([lockButton isEnabled]);
+}
+
+TEST_F(ProfileChooserControllerTest, SignedInProfileLockEnabled) {
+  switches::EnableNewProfileManagementForTesting(
+      CommandLine::ForCurrentProcess());
+  // Sign in the first profile.
+  ProfileInfoCache* cache = testing_profile_manager()->profile_info_cache();
+  cache->SetUserNameOfProfileAtIndex(0, base::ASCIIToUTF16(kEmail));
+  cache->SetLocalAuthCredentialsOfProfileAtIndex(0, "YourHashHere");
+
+  StartProfileChooserController();
+  NSArray* subviews = [[[controller() window] contentView] subviews];
+  ASSERT_EQ(1U, [subviews count]);
+  subviews = [[subviews objectAtIndex:0] subviews];
+
+  // Three profiles means we should have one active card, one separator, one
+  // option buttons view and a lock view. We also have an update promo for the
+  // new avatar menu.
+  // TODO(noms): Enforcing 5U fails on the waterfall debug bots, but it's not
+  // reproducible anywhere else.
+  ASSERT_GE([subviews count], 4U);
+
+  // There will be three buttons and two separators in the option buttons view.
+  NSArray* buttonSubviews = [[subviews objectAtIndex:0] subviews];
+  ASSERT_EQ(5U, [buttonSubviews count]);
+
+  // There should be a lock button.
+  NSButton* lockButton =
+      base::mac::ObjCCast<NSButton>([buttonSubviews objectAtIndex:0]);
+  ASSERT_TRUE(lockButton);
+  EXPECT_EQ(@selector(lockProfile:), [lockButton action]);
+  EXPECT_EQ(controller(), [lockButton target]);
+  EXPECT_TRUE([lockButton isEnabled]);
+}

chrome/browser/ui/views/profiles/profile_chooser_view.cc

@@ -15,6 +15,7 @@
 #include "chrome/browser/profiles/profile_metrics.h"
 #include "chrome/browser/profiles/profile_window.h"
 #include "chrome/browser/profiles/profiles_state.h"
+#include "chrome/browser/signin/local_auth.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_header_helper.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
@@ -1242,7 +1243,7 @@ views::View* ProfileChooserView::CreateOtherProfilesView(
   return view;
 }
 
-views::View* ProfileChooserView::CreateOptionsView(bool enable_lock) {
+views::View* ProfileChooserView::CreateOptionsView(bool display_lock) {
   views::View* view = new views::View();
   views::GridLayout* layout = CreateSingleColumnLayout(view, kFixedMenuWidth);
 
@@ -1269,7 +1270,7 @@ views::View* ProfileChooserView::CreateOptionsView(bool enable_lock) {
     layout->AddView(go_incognito_button_);
   }
 
-  if (enable_lock) {
+  if (display_lock) {
     layout->StartRow(1, 0);
     layout->AddView(new views::Separator(views::Separator::HORIZONTAL));
 
@@ -1277,6 +1278,8 @@ views::View* ProfileChooserView::CreateOptionsView(bool enable_lock) {
         this,
         l10n_util::GetStringUTF16(IDS_PROFILES_PROFILE_SIGNOUT_BUTTON),
         *rb->GetImageSkiaNamed(IDR_ICON_PROFILES_MENU_LOCK));
+    if (!chrome::LocalAuthCredentialsExist(browser_->profile()))
+      lock_button_->SetState(views::Button::STATE_DISABLED);
     layout->StartRow(1, 0);
     layout->AddView(lock_button_);
   }

chrome/browser/ui/views/profiles/profile_chooser_view.h

@@ -131,7 +131,7 @@ class ProfileChooserView : public views::BubbleDelegateView,
       bool is_guest);
   views::View* CreateGuestProfileView();
   views::View* CreateOtherProfilesView(const Indexes& avatars_to_show);
-  views::View* CreateOptionsView(bool enable_lock);
+  views::View* CreateOptionsView(bool display_lock);
   views::View* CreateSupervisedUserDisclaimerView();
 
   // Account Management view for the profile |avatar_item|.