From 56f82b67ad5705722d1e5983e00f5dce401c1bd3 Mon Sep 17 00:00:00 2001 From: mlerman Date: Tue, 9 Sep 2014 15:59:25 -0700 Subject: [PATCH] Enable lock even when no password hash is present. After discussions, lock will be always available if the flag dictates, and online auth will be required for the first unlock. This is subequent to https://codereview.chromium.org/497783002/. BUG=335086 Review URL: https://codereview.chromium.org/548393002 Cr-Commit-Position: refs/heads/master@{#294026} --- chrome/browser/signin/local_auth.cc | 19 ------- chrome/browser/signin/local_auth.h | 4 -- chrome/browser/signin/local_auth_unittest.cc | 2 - .../cocoa/profiles/profile_chooser_controller.mm | 2 - .../profile_chooser_controller_unittest.mm | 66 ---------------------- .../ui/views/profiles/profile_chooser_view.cc | 2 - 6 files changed, 95 deletions(-) diff --git a/chrome/browser/signin/local_auth.cc b/chrome/browser/signin/local_auth.cc index 14bf511dbca8..7748898f8e7a 100644 --- a/chrome/browser/signin/local_auth.cc +++ b/chrome/browser/signin/local_auth.cc @@ -198,23 +198,4 @@ bool ValidateLocalAuthCredentials(const Profile* profile, password); } -bool LocalAuthCredentialsExist(size_t profile_info_index) { - if (profile_info_index == std::string::npos) { - NOTREACHED(); - return false; - } - - 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 diff --git a/chrome/browser/signin/local_auth.h b/chrome/browser/signin/local_auth.h index e660e4c51343..2342a8142a2d 100644 --- a/chrome/browser/signin/local_auth.h +++ b/chrome/browser/signin/local_auth.h @@ -33,10 +33,6 @@ 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_ diff --git a/chrome/browser/signin/local_auth_unittest.cc b/chrome/browser/signin/local_auth_unittest.cc index 2da9b74cc341..e1fd26c642d8 100644 --- a/chrome/browser/signin/local_auth_unittest.cc +++ b/chrome/browser/signin/local_auth_unittest.cc @@ -35,9 +35,7 @@ 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 diff --git a/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm b/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm index 98eae5d104a8..b90c224829c6 100644 --- a/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm +++ b/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm @@ -1722,8 +1722,6 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver, 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]); diff --git a/chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm b/chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm index 0c7e58610e07..6751783ca658 100644 --- a/chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm +++ b/chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm @@ -459,69 +459,3 @@ 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(2U, [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([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(2U, [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([buttonSubviews objectAtIndex:0]); - ASSERT_TRUE(lockButton); - EXPECT_EQ(@selector(lockProfile:), [lockButton action]); - EXPECT_EQ(controller(), [lockButton target]); - EXPECT_TRUE([lockButton isEnabled]); -} diff --git a/chrome/browser/ui/views/profiles/profile_chooser_view.cc b/chrome/browser/ui/views/profiles/profile_chooser_view.cc index 39010f08cfc8..13521e65a0cd 100644 --- a/chrome/browser/ui/views/profiles/profile_chooser_view.cc +++ b/chrome/browser/ui/views/profiles/profile_chooser_view.cc @@ -1275,8 +1275,6 @@ views::View* ProfileChooserView::CreateOptionsView(bool display_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_); } -- 2.11.4.GIT