Commit bb73287f authored by Jialiu Lin's avatar Jialiu Lin Committed by Commit Bot

Get AccountInfo struct right before it being used

account_info_ is initialized when ChromePasswordProtectionService is
created (browser startup time).So if user changed their signin state
afterward, CPPS cannot capture it. We should in fact get AccountInfo
struct from SigninManager whenever it is being used.

Bug: 772041
Change-Id: Idedbcc71c444d84446662d2b9ed595901ac25ae8
Reviewed-on: https://chromium-review.googlesource.com/702915
Commit-Queue: Jialiu Lin <jialiul@chromium.org>
Reviewed-by: default avatarVarun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506934}
parent 62a67617
...@@ -122,7 +122,6 @@ ChromePasswordProtectionService::ChromePasswordProtectionService( ...@@ -122,7 +122,6 @@ ChromePasswordProtectionService::ChromePasswordProtectionService(
password_manager::prefs::kSyncPasswordHash, password_manager::prefs::kSyncPasswordHash,
base::Bind(&ChromePasswordProtectionService::OnGaiaPasswordChanged, base::Bind(&ChromePasswordProtectionService::OnGaiaPasswordChanged,
base::Unretained(this))); base::Unretained(this)));
InitializeAccountInfo();
} }
ChromePasswordProtectionService::~ChromePasswordProtectionService() { ChromePasswordProtectionService::~ChromePasswordProtectionService() {
...@@ -409,14 +408,14 @@ void ChromePasswordProtectionService::MaybeLogPasswordReuseDetectedEvent( ...@@ -409,14 +408,14 @@ void ChromePasswordProtectionService::MaybeLogPasswordReuseDetectedEvent(
PasswordProtectionService::SyncAccountType PasswordProtectionService::SyncAccountType
ChromePasswordProtectionService::GetSyncAccountType() { ChromePasswordProtectionService::GetSyncAccountType() {
if (!account_info_ || account_info_->account_id.empty() || const AccountInfo account_info = GetAccountInfo();
account_info_->hosted_domain.empty()) { if (account_info.account_id.empty() || account_info.hosted_domain.empty()) {
return LoginReputationClientRequest::PasswordReuseEvent::NOT_SIGNED_IN; return LoginReputationClientRequest::PasswordReuseEvent::NOT_SIGNED_IN;
} }
// For gmail or googlemail account, the hosted_domain will always be // For gmail or googlemail account, the hosted_domain will always be
// kNoHostedDomainFound. // kNoHostedDomainFound.
return account_info_->hosted_domain == return account_info.hosted_domain ==
std::string(AccountTrackerService::kNoHostedDomainFound) std::string(AccountTrackerService::kNoHostedDomainFound)
? LoginReputationClientRequest::PasswordReuseEvent::GMAIL ? LoginReputationClientRequest::PasswordReuseEvent::GMAIL
: LoginReputationClientRequest::PasswordReuseEvent::GSUITE; : LoginReputationClientRequest::PasswordReuseEvent::GSUITE;
...@@ -599,19 +598,16 @@ bool ChromePasswordProtectionService::UserClickedThroughSBInterstitial( ...@@ -599,19 +598,16 @@ bool ChromePasswordProtectionService::UserClickedThroughSBInterstitial(
current_threat_type == SB_THREAT_TYPE_URL_CLIENT_SIDE_MALWARE; current_threat_type == SB_THREAT_TYPE_URL_CLIENT_SIDE_MALWARE;
} }
void ChromePasswordProtectionService::InitializeAccountInfo() { AccountInfo ChromePasswordProtectionService::GetAccountInfo() {
SigninManagerBase* signin_manager = SigninManagerBase* signin_manager =
SigninManagerFactory::GetForProfileIfExists(profile_); SigninManagerFactory::GetForProfileIfExists(profile_);
DCHECK(signin_manager);
if (signin_manager) { return signin_manager->GetAuthenticatedAccountInfo();
account_info_ = base::MakeUnique<AccountInfo>(
signin_manager->GetAuthenticatedAccountInfo());
}
} }
GURL ChromePasswordProtectionService::GetChangePasswordURL() { GURL ChromePasswordProtectionService::GetChangePasswordURL() {
DCHECK(!account_info_->email.empty()); const AccountInfo account_info = GetAccountInfo();
std::string account_email = account_info_->email; std::string account_email = account_info.email;
// This page will prompt for re-auth and then will prompt for a new password. // This page will prompt for re-auth and then will prompt for a new password.
std::string account_url = std::string account_url =
"https://myaccount.google.com/signinoptions/" "https://myaccount.google.com/signinoptions/"
...@@ -645,7 +641,6 @@ ChromePasswordProtectionService::ChromePasswordProtectionService( ...@@ -645,7 +641,6 @@ ChromePasswordProtectionService::ChromePasswordProtectionService(
ui_manager_(ui_manager), ui_manager_(ui_manager),
trigger_manager_(nullptr), trigger_manager_(nullptr),
profile_(profile) { profile_(profile) {
InitializeAccountInfo();
} }
} // namespace safe_browsing } // namespace safe_browsing
...@@ -157,8 +157,8 @@ class ChromePasswordProtectionService : public PasswordProtectionService { ...@@ -157,8 +157,8 @@ class ChromePasswordProtectionService : public PasswordProtectionService {
void UpdateSecurityState(SBThreatType threat_type, void UpdateSecurityState(SBThreatType threat_type,
content::WebContents* web_contents) override; content::WebContents* web_contents) override;
// Sets |account_info_| based on |profile_|. // Gets |account_info_| based on |profile_|.
void InitializeAccountInfo(); AccountInfo GetAccountInfo();
// Gets change password URl based on |account_info_|. // Gets change password URl based on |account_info_|.
GURL GetChangePasswordURL(); GURL GetChangePasswordURL();
...@@ -216,8 +216,6 @@ class ChromePasswordProtectionService : public PasswordProtectionService { ...@@ -216,8 +216,6 @@ class ChromePasswordProtectionService : public PasswordProtectionService {
TriggerManager* trigger_manager_; TriggerManager* trigger_manager_;
// Profile associated with this instance. // Profile associated with this instance.
Profile* profile_; Profile* profile_;
// AccountInfo associated with this |profile_|.
std::unique_ptr<AccountInfo> account_info_;
scoped_refptr<SafeBrowsingNavigationObserverManager> scoped_refptr<SafeBrowsingNavigationObserverManager>
navigation_observer_manager_; navigation_observer_manager_;
base::ObserverList<Observer> observer_list_; base::ObserverList<Observer> observer_list_;
......
...@@ -80,10 +80,6 @@ class ChromePasswordProtectionServiceBrowserTest : public InProcessBrowserTest { ...@@ -80,10 +80,6 @@ class ChromePasswordProtectionServiceBrowserTest : public InProcessBrowserTest {
helper->GetSecurityInfo(out_security_info); helper->GetSecurityInfo(out_security_info);
} }
void SetDefaultProfileEmail(ChromePasswordProtectionService* service) {
service->account_info_->email = "foo@bar.com";
}
protected: protected:
base::test::ScopedFeatureList scoped_feature_list_; base::test::ScopedFeatureList scoped_feature_list_;
base::HistogramTester histograms_; base::HistogramTester histograms_;
...@@ -92,7 +88,6 @@ class ChromePasswordProtectionServiceBrowserTest : public InProcessBrowserTest { ...@@ -92,7 +88,6 @@ class ChromePasswordProtectionServiceBrowserTest : public InProcessBrowserTest {
IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest, IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
SuccessfullyChangePassword) { SuccessfullyChangePassword) {
ChromePasswordProtectionService* service = GetService(/*is_incognito=*/false); ChromePasswordProtectionService* service = GetService(/*is_incognito=*/false);
SetDefaultProfileEmail(service);
Profile* profile = browser()->profile(); Profile* profile = browser()->profile();
content::WebContents* web_contents = content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents(); browser()->tab_strip_model()->GetActiveWebContents();
......
...@@ -241,13 +241,11 @@ TEST_F(ChromePasswordProtectionServiceTest, VerifyGetSyncAccountType) { ...@@ -241,13 +241,11 @@ TEST_F(ChromePasswordProtectionServiceTest, VerifyGetSyncAccountType) {
signin_manager->SetAuthenticatedAccountInfo(kTestAccountID, kTestEmail); signin_manager->SetAuthenticatedAccountInfo(kTestAccountID, kTestEmail);
SetUpSyncAccount(std::string(AccountTrackerService::kNoHostedDomainFound), SetUpSyncAccount(std::string(AccountTrackerService::kNoHostedDomainFound),
std::string(kTestAccountID), std::string(kTestEmail)); std::string(kTestAccountID), std::string(kTestEmail));
service_->InitializeAccountInfo();
EXPECT_EQ(LoginReputationClientRequest::PasswordReuseEvent::GMAIL, EXPECT_EQ(LoginReputationClientRequest::PasswordReuseEvent::GMAIL,
service_->GetSyncAccountType()); service_->GetSyncAccountType());
SetUpSyncAccount("example.edu", std::string(kTestAccountID), SetUpSyncAccount("example.edu", std::string(kTestAccountID),
std::string(kTestEmail)); std::string(kTestEmail));
service_->InitializeAccountInfo();
EXPECT_EQ(LoginReputationClientRequest::PasswordReuseEvent::GSUITE, EXPECT_EQ(LoginReputationClientRequest::PasswordReuseEvent::GSUITE,
service_->GetSyncAccountType()); service_->GetSyncAccountType());
} }
...@@ -427,7 +425,6 @@ TEST_F(ChromePasswordProtectionServiceTest, VerifyGetChangePasswordURL) { ...@@ -427,7 +425,6 @@ TEST_F(ChromePasswordProtectionServiceTest, VerifyGetChangePasswordURL) {
signin_manager->SetAuthenticatedAccountInfo(kTestAccountID, kTestEmail); signin_manager->SetAuthenticatedAccountInfo(kTestAccountID, kTestEmail);
SetUpSyncAccount("example.com", std::string(kTestAccountID), SetUpSyncAccount("example.com", std::string(kTestAccountID),
std::string(kTestEmail)); std::string(kTestEmail));
service_->InitializeAccountInfo();
EXPECT_EQ(GURL("https://accounts.google.com/" EXPECT_EQ(GURL("https://accounts.google.com/"
"AccountChooser?Email=foo%40example.com&continue=https%3A%2F%" "AccountChooser?Email=foo%40example.com&continue=https%3A%2F%"
"2Fmyaccount.google.com%2Fsigninoptions%2Fpassword%3Futm_" "2Fmyaccount.google.com%2Fsigninoptions%2Fpassword%3Futm_"
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment