Commit f4a5aa2a authored by Antonio Gomes's avatar Antonio Gomes Committed by Commit Bot

[s13n] Convert DiceResponseHandler to IdentityManager

This CL only migrates DiceResponseHandler production code away from
SigninManager, leaves migration away from ProfileOAuth2TokenService
and unittests for a follow up step.

Note that the unittest c/b/signin/dice_response_handler_unittest.cc
was minimally updated to keep passing.

BUG=890790,902296

Change-Id: Iecdf43b80afe6514b7f19075c2bb62aadfbdbe4c
Reviewed-on: https://chromium-review.googlesource.com/c/1318550
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: default avatarMihai Sardarescu <msarda@chromium.org>
Reviewed-by: default avatarColin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606439}
parent f897aba1
...@@ -19,9 +19,9 @@ ...@@ -19,9 +19,9 @@
#include "chrome/browser/signin/account_reconcilor_factory.h" #include "chrome/browser/signin/account_reconcilor_factory.h"
#include "chrome/browser/signin/account_tracker_service_factory.h" #include "chrome/browser/signin/account_tracker_service_factory.h"
#include "chrome/browser/signin/chrome_signin_client_factory.h" #include "chrome/browser/signin/chrome_signin_client_factory.h"
#include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.h" #include "chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.h"
#include "chrome/browser/signin/profile_oauth2_token_service_factory.h" #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
#include "chrome/browser/signin/signin_manager_factory.h"
#include "chrome/browser/ui/webui/profile_helper.h" #include "chrome/browser/ui/webui/profile_helper.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h" #include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/keyed_service/content/browser_context_keyed_service_factory.h" #include "components/keyed_service/content/browser_context_keyed_service_factory.h"
...@@ -30,12 +30,12 @@ ...@@ -30,12 +30,12 @@
#include "components/signin/core/browser/profile_oauth2_token_service.h" #include "components/signin/core/browser/profile_oauth2_token_service.h"
#include "components/signin/core/browser/signin_client.h" #include "components/signin/core/browser/signin_client.h"
#include "components/signin/core/browser/signin_header_helper.h" #include "components/signin/core/browser/signin_header_helper.h"
#include "components/signin/core/browser/signin_manager.h"
#include "components/signin/core/browser/signin_metrics.h" #include "components/signin/core/browser/signin_metrics.h"
#include "google_apis/gaia/gaia_auth_fetcher.h" #include "google_apis/gaia/gaia_auth_fetcher.h"
#include "google_apis/gaia/gaia_auth_util.h" #include "google_apis/gaia/gaia_auth_util.h"
#include "google_apis/gaia/gaia_constants.h" #include "google_apis/gaia/gaia_constants.h"
#include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/google_service_auth_error.h"
#include "services/identity/public/cpp/identity_manager.h"
const int kDiceTokenFetchTimeoutSeconds = 10; const int kDiceTokenFetchTimeoutSeconds = 10;
...@@ -114,8 +114,8 @@ class DiceResponseHandlerFactory : public BrowserContextKeyedServiceFactory { ...@@ -114,8 +114,8 @@ class DiceResponseHandlerFactory : public BrowserContextKeyedServiceFactory {
DependsOn(AccountReconcilorFactory::GetInstance()); DependsOn(AccountReconcilorFactory::GetInstance());
DependsOn(AccountTrackerServiceFactory::GetInstance()); DependsOn(AccountTrackerServiceFactory::GetInstance());
DependsOn(ChromeSigninClientFactory::GetInstance()); DependsOn(ChromeSigninClientFactory::GetInstance());
DependsOn(IdentityManagerFactory::GetInstance());
DependsOn(ProfileOAuth2TokenServiceFactory::GetInstance()); DependsOn(ProfileOAuth2TokenServiceFactory::GetInstance());
DependsOn(SigninManagerFactory::GetInstance());
} }
~DiceResponseHandlerFactory() override {} ~DiceResponseHandlerFactory() override {}
...@@ -129,8 +129,8 @@ class DiceResponseHandlerFactory : public BrowserContextKeyedServiceFactory { ...@@ -129,8 +129,8 @@ class DiceResponseHandlerFactory : public BrowserContextKeyedServiceFactory {
Profile* profile = static_cast<Profile*>(context); Profile* profile = static_cast<Profile*>(context);
return new DiceResponseHandler( return new DiceResponseHandler(
ChromeSigninClientFactory::GetForProfile(profile), ChromeSigninClientFactory::GetForProfile(profile),
SigninManagerFactory::GetForProfile(profile),
ProfileOAuth2TokenServiceFactory::GetForProfile(profile), ProfileOAuth2TokenServiceFactory::GetForProfile(profile),
IdentityManagerFactory::GetForProfile(profile),
AccountTrackerServiceFactory::GetForProfile(profile), AccountTrackerServiceFactory::GetForProfile(profile),
AccountReconcilorFactory::GetForProfile(profile), AccountReconcilorFactory::GetForProfile(profile),
AboutSigninInternalsFactory::GetForProfile(profile), AboutSigninInternalsFactory::GetForProfile(profile),
...@@ -234,24 +234,24 @@ DiceResponseHandler* DiceResponseHandler::GetForProfile(Profile* profile) { ...@@ -234,24 +234,24 @@ DiceResponseHandler* DiceResponseHandler::GetForProfile(Profile* profile) {
DiceResponseHandler::DiceResponseHandler( DiceResponseHandler::DiceResponseHandler(
SigninClient* signin_client, SigninClient* signin_client,
SigninManager* signin_manager,
ProfileOAuth2TokenService* profile_oauth2_token_service, ProfileOAuth2TokenService* profile_oauth2_token_service,
identity::IdentityManager* identity_manager,
AccountTrackerService* account_tracker_service, AccountTrackerService* account_tracker_service,
AccountReconcilor* account_reconcilor, AccountReconcilor* account_reconcilor,
AboutSigninInternals* about_signin_internals, AboutSigninInternals* about_signin_internals,
signin::AccountConsistencyMethod account_consistency, signin::AccountConsistencyMethod account_consistency,
const base::FilePath& profile_path) const base::FilePath& profile_path)
: signin_manager_(signin_manager), : signin_client_(signin_client),
signin_client_(signin_client),
token_service_(profile_oauth2_token_service), token_service_(profile_oauth2_token_service),
identity_manager_(identity_manager),
account_tracker_service_(account_tracker_service), account_tracker_service_(account_tracker_service),
account_reconcilor_(account_reconcilor), account_reconcilor_(account_reconcilor),
about_signin_internals_(about_signin_internals), about_signin_internals_(about_signin_internals),
account_consistency_(account_consistency), account_consistency_(account_consistency),
profile_path_(profile_path) { profile_path_(profile_path) {
DCHECK(signin_client_); DCHECK(signin_client_);
DCHECK(signin_manager_);
DCHECK(token_service_); DCHECK(token_service_);
DCHECK(identity_manager_);
DCHECK(account_tracker_service_); DCHECK(account_tracker_service_);
DCHECK(account_reconcilor_); DCHECK(account_reconcilor_);
DCHECK(about_signin_internals_); DCHECK(about_signin_internals_);
...@@ -310,8 +310,7 @@ bool DiceResponseHandler::CanGetTokenForAccount(const std::string& gaia_id, ...@@ -310,8 +310,7 @@ bool DiceResponseHandler::CanGetTokenForAccount(const std::string& gaia_id,
account_consistency_); account_consistency_);
std::string account = std::string account =
account_tracker_service_->PickAccountIdForAccount(gaia_id, email); account_tracker_service_->PickAccountIdForAccount(gaia_id, email);
std::string chrome_account = signin_manager_->GetAuthenticatedAccountId(); bool can_get_token = (identity_manager_->GetPrimaryAccountId() == account);
bool can_get_token = (chrome_account == account);
VLOG_IF(1, !can_get_token) VLOG_IF(1, !can_get_token)
<< "[Dice] Dropping Dice signin response for " << account; << "[Dice] Dropping Dice signin response for " << account;
return can_get_token; return can_get_token;
...@@ -376,7 +375,7 @@ void DiceResponseHandler::ProcessDiceSignoutHeader( ...@@ -376,7 +375,7 @@ void DiceResponseHandler::ProcessDiceSignoutHeader(
return; return;
} }
std::string primary_account = signin_manager_->GetAuthenticatedAccountId(); std::string primary_account = identity_manager_->GetPrimaryAccountId();
bool primary_account_signed_out = false; bool primary_account_signed_out = false;
for (const auto& account_info : account_infos) { for (const auto& account_info : account_infos) {
std::string signed_out_account = std::string signed_out_account =
......
...@@ -23,10 +23,13 @@ class AccountTrackerService; ...@@ -23,10 +23,13 @@ class AccountTrackerService;
class GaiaAuthFetcher; class GaiaAuthFetcher;
class GoogleServiceAuthError; class GoogleServiceAuthError;
class SigninClient; class SigninClient;
class SigninManager;
class ProfileOAuth2TokenService; class ProfileOAuth2TokenService;
class Profile; class Profile;
namespace identity {
class IdentityManager;
}
// Exposed for testing. // Exposed for testing.
extern const int kDiceTokenFetchTimeoutSeconds; extern const int kDiceTokenFetchTimeoutSeconds;
...@@ -54,8 +57,8 @@ class DiceResponseHandler : public KeyedService { ...@@ -54,8 +57,8 @@ class DiceResponseHandler : public KeyedService {
static DiceResponseHandler* GetForProfile(Profile* profile); static DiceResponseHandler* GetForProfile(Profile* profile);
DiceResponseHandler(SigninClient* signin_client, DiceResponseHandler(SigninClient* signin_client,
SigninManager* signin_manager,
ProfileOAuth2TokenService* profile_oauth2_token_service, ProfileOAuth2TokenService* profile_oauth2_token_service,
identity::IdentityManager* identity_manager,
AccountTrackerService* account_tracker_service, AccountTrackerService* account_tracker_service,
AccountReconcilor* account_reconcilor, AccountReconcilor* account_reconcilor,
AboutSigninInternals* about_signin_internals, AboutSigninInternals* about_signin_internals,
...@@ -152,9 +155,9 @@ class DiceResponseHandler : public KeyedService { ...@@ -152,9 +155,9 @@ class DiceResponseHandler : public KeyedService {
void OnTokenExchangeFailure(DiceTokenFetcher* token_fetcher, void OnTokenExchangeFailure(DiceTokenFetcher* token_fetcher,
const GoogleServiceAuthError& error); const GoogleServiceAuthError& error);
SigninManager* signin_manager_;
SigninClient* signin_client_; SigninClient* signin_client_;
ProfileOAuth2TokenService* token_service_; ProfileOAuth2TokenService* token_service_;
identity::IdentityManager* identity_manager_;
AccountTrackerService* account_tracker_service_; AccountTrackerService* account_tracker_service_;
AccountReconcilor* account_reconcilor_; AccountReconcilor* account_reconcilor_;
AboutSigninInternals* about_signin_internals_; AboutSigninInternals* about_signin_internals_;
......
...@@ -34,6 +34,7 @@ ...@@ -34,6 +34,7 @@
#include "content/public/test/test_browser_thread_bundle.h" #include "content/public/test/test_browser_thread_bundle.h"
#include "google_apis/gaia/fake_oauth2_token_service_delegate.h" #include "google_apis/gaia/fake_oauth2_token_service_delegate.h"
#include "google_apis/gaia/gaia_constants.h" #include "google_apis/gaia/gaia_constants.h"
#include "services/identity/public/cpp/identity_test_environment.h"
#include "services/network/public/cpp/shared_url_loader_factory.h" #include "services/network/public/cpp/shared_url_loader_factory.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -132,6 +133,10 @@ class DiceResponseHandlerTest : public testing::Test, ...@@ -132,6 +133,10 @@ class DiceResponseHandlerTest : public testing::Test,
GaiaConstants::kChromeSource, GaiaConstants::kChromeSource,
&signin_client_, &signin_client_,
/*use_fake_url_fetcher=*/false), /*use_fake_url_fetcher=*/false),
identity_test_env_(&account_tracker_service_,
&token_service_,
&signin_manager_,
&cookie_service_),
about_signin_internals_(&token_service_, about_signin_internals_(&token_service_,
&account_tracker_service_, &account_tracker_service_,
&signin_manager_, &signin_manager_,
...@@ -175,7 +180,7 @@ class DiceResponseHandlerTest : public testing::Test, ...@@ -175,7 +180,7 @@ class DiceResponseHandlerTest : public testing::Test,
signin::AccountConsistencyMethod account_consistency) { signin::AccountConsistencyMethod account_consistency) {
DCHECK(!dice_response_handler_); DCHECK(!dice_response_handler_);
dice_response_handler_ = std::make_unique<DiceResponseHandler>( dice_response_handler_ = std::make_unique<DiceResponseHandler>(
&signin_client_, &signin_manager_, &token_service_, &signin_client_, &token_service_, identity_test_env_.identity_manager(),
&account_tracker_service_, account_reconcilor_.get(), &account_tracker_service_, account_reconcilor_.get(),
&about_signin_internals_, account_consistency, temp_dir_.GetPath()); &about_signin_internals_, account_consistency, temp_dir_.GetPath());
} }
...@@ -226,11 +231,12 @@ class DiceResponseHandlerTest : public testing::Test, ...@@ -226,11 +231,12 @@ class DiceResponseHandlerTest : public testing::Test,
scoped_refptr<base::TestMockTimeTaskRunner> task_runner_; scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;
sync_preferences::TestingPrefServiceSyncable pref_service_; sync_preferences::TestingPrefServiceSyncable pref_service_;
DiceTestSigninClient signin_client_; DiceTestSigninClient signin_client_;
ProfileOAuth2TokenService token_service_; FakeProfileOAuth2TokenService token_service_;
AccountTrackerService account_tracker_service_; AccountTrackerService account_tracker_service_;
SigninErrorController signin_error_controller_; SigninErrorController signin_error_controller_;
FakeSigninManager signin_manager_; FakeSigninManager signin_manager_;
FakeGaiaCookieManagerService cookie_service_; FakeGaiaCookieManagerService cookie_service_;
identity::IdentityTestEnvironment identity_test_env_;
AboutSigninInternals about_signin_internals_; AboutSigninInternals about_signin_internals_;
std::unique_ptr<AccountReconcilor> account_reconcilor_; std::unique_ptr<AccountReconcilor> account_reconcilor_;
std::unique_ptr<DiceResponseHandler> dice_response_handler_; std::unique_ptr<DiceResponseHandler> dice_response_handler_;
...@@ -493,15 +499,18 @@ TEST_F(DiceResponseHandlerTest, Timeout) { ...@@ -493,15 +499,18 @@ TEST_F(DiceResponseHandlerTest, Timeout) {
TEST_F(DiceResponseHandlerTest, SignoutMainAccount) { TEST_F(DiceResponseHandlerTest, SignoutMainAccount) {
InitializeDiceResponseHandler(signin::AccountConsistencyMethod::kDice); InitializeDiceResponseHandler(signin::AccountConsistencyMethod::kDice);
const char kSecondaryGaiaID[] = "secondary_account"; const char kSecondaryGaiaID[] = "secondary_account";
const char kSecondaryEmail[] = "other@gmail.com";
DiceResponseParams dice_params = MakeDiceParams(DiceAction::SIGNOUT); DiceResponseParams dice_params = MakeDiceParams(DiceAction::SIGNOUT);
const auto& account_info = dice_params.signout_info->account_infos[0]; const auto& account_info = dice_params.signout_info->account_infos[0];
std::string account_id = account_tracker_service_.PickAccountIdForAccount( std::string account_id = account_tracker_service_.SeedAccountInfo(
account_info.gaia_id, account_info.email); account_info.gaia_id, account_info.email);
std::string secondary_account_id = account_tracker_service_.SeedAccountInfo(
kSecondaryGaiaID, kSecondaryEmail);
// User is signed in to Chrome, and has some refresh token for a secondary // User is signed in to Chrome, and has some refresh token for a secondary
// account. // account.
signin_manager_.SignIn(account_info.gaia_id, account_info.email, "password"); signin_manager_.SignIn(account_info.gaia_id, account_info.email, "password");
token_service_.UpdateCredentials(account_id, "token1"); token_service_.UpdateCredentials(account_id, "token1");
token_service_.UpdateCredentials(kSecondaryGaiaID, "token2"); token_service_.UpdateCredentials(secondary_account_id, "token2");
EXPECT_TRUE(token_service_.RefreshTokenIsAvailable(account_id)); EXPECT_TRUE(token_service_.RefreshTokenIsAvailable(account_id));
EXPECT_TRUE(token_service_.RefreshTokenIsAvailable(kSecondaryGaiaID)); EXPECT_TRUE(token_service_.RefreshTokenIsAvailable(kSecondaryGaiaID));
EXPECT_TRUE(signin_manager_.IsAuthenticated()); EXPECT_TRUE(signin_manager_.IsAuthenticated());
...@@ -531,11 +540,10 @@ TEST_F(DiceResponseHandlerTest, MigrationSignout) { ...@@ -531,11 +540,10 @@ TEST_F(DiceResponseHandlerTest, MigrationSignout) {
dice_params.signout_info->account_infos.emplace_back(kSecondaryGaiaID, dice_params.signout_info->account_infos.emplace_back(kSecondaryGaiaID,
kSecondaryEmail, 1); kSecondaryEmail, 1);
const auto& main_account_info = dice_params.signout_info->account_infos[0]; const auto& main_account_info = dice_params.signout_info->account_infos[0];
std::string account_id = account_tracker_service_.PickAccountIdForAccount( std::string account_id = account_tracker_service_.SeedAccountInfo(
main_account_info.gaia_id, main_account_info.email); main_account_info.gaia_id, main_account_info.email);
std::string secondary_account_id = std::string secondary_account_id = account_tracker_service_.SeedAccountInfo(
account_tracker_service_.PickAccountIdForAccount(kSecondaryGaiaID, kSecondaryGaiaID, kSecondaryEmail);
kSecondaryEmail);
// User is signed in to Chrome, and has some refresh token for a secondary // User is signed in to Chrome, and has some refresh token for a secondary
// account. // account.
...@@ -565,10 +573,10 @@ TEST_F(DiceResponseHandlerTest, SignoutSecondaryAccount) { ...@@ -565,10 +573,10 @@ TEST_F(DiceResponseHandlerTest, SignoutSecondaryAccount) {
const char kMainGaiaID[] = "main_account"; const char kMainGaiaID[] = "main_account";
const char kMainEmail[] = "main@gmail.com"; const char kMainEmail[] = "main@gmail.com";
std::string main_account_id = std::string main_account_id =
account_tracker_service_.PickAccountIdForAccount(kMainGaiaID, kMainEmail); account_tracker_service_.SeedAccountInfo(kMainGaiaID, kMainEmail);
DiceResponseParams dice_params = MakeDiceParams(DiceAction::SIGNOUT); DiceResponseParams dice_params = MakeDiceParams(DiceAction::SIGNOUT);
const auto& account_info = dice_params.signout_info->account_infos[0]; const auto& account_info = dice_params.signout_info->account_infos[0];
std::string account_id = account_tracker_service_.PickAccountIdForAccount( std::string account_id = account_tracker_service_.SeedAccountInfo(
account_info.gaia_id, account_info.email); account_info.gaia_id, account_info.email);
// User is signed in to Chrome, and has some refresh token for a secondary // User is signed in to Chrome, and has some refresh token for a secondary
// account. // account.
...@@ -592,14 +600,17 @@ TEST_F(DiceResponseHandlerTest, SignoutSecondaryAccount) { ...@@ -592,14 +600,17 @@ TEST_F(DiceResponseHandlerTest, SignoutSecondaryAccount) {
TEST_F(DiceResponseHandlerTest, SignoutWebOnly) { TEST_F(DiceResponseHandlerTest, SignoutWebOnly) {
InitializeDiceResponseHandler(signin::AccountConsistencyMethod::kDice); InitializeDiceResponseHandler(signin::AccountConsistencyMethod::kDice);
const char kSecondaryAccountID[] = "secondary_account"; const char kSecondaryAccountID[] = "secondary_account";
const char kSecondaryEmail[] = "other@gmail.com";
DiceResponseParams dice_params = MakeDiceParams(DiceAction::SIGNOUT); DiceResponseParams dice_params = MakeDiceParams(DiceAction::SIGNOUT);
const auto& account_info = dice_params.signout_info->account_infos[0]; const auto& account_info = dice_params.signout_info->account_infos[0];
std::string account_id = account_tracker_service_.PickAccountIdForAccount( std::string account_id = account_tracker_service_.SeedAccountInfo(
account_info.gaia_id, account_info.email); account_info.gaia_id, account_info.email);
std::string secondary_account_id = account_tracker_service_.SeedAccountInfo(
kSecondaryAccountID, kSecondaryEmail);
// User is NOT signed in to Chrome, and has some refresh tokens for two // User is NOT signed in to Chrome, and has some refresh tokens for two
// accounts. // accounts.
token_service_.UpdateCredentials(account_id, "refresh_token"); token_service_.UpdateCredentials(account_id, "refresh_token");
token_service_.UpdateCredentials(kSecondaryAccountID, "refresh_token"); token_service_.UpdateCredentials(secondary_account_id, "refresh_token");
EXPECT_TRUE(token_service_.RefreshTokenIsAvailable(account_id)); EXPECT_TRUE(token_service_.RefreshTokenIsAvailable(account_id));
EXPECT_TRUE(token_service_.RefreshTokenIsAvailable(kSecondaryAccountID)); EXPECT_TRUE(token_service_.RefreshTokenIsAvailable(kSecondaryAccountID));
EXPECT_FALSE(signin_manager_.IsAuthenticated()); EXPECT_FALSE(signin_manager_.IsAuthenticated());
......
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