Commit b18d7062 authored by Sergio Villar Senin's avatar Sergio Villar Senin Committed by Commit Bot

Migrate iOS' AuthenticationService classes

This is a first step in the migration of the different iOS'
authentication_service_* classes to the IdentityManager API. The
migration cannot be completed at this point as several APIs are not
available yet. This CL is basically passing the IdentityManager to the
different classes and using its API whenever appropiate instead of
directly calling SinginManager and TokenService.

Bug: 890817, 890818, 890819, 890820, 890820
Change-Id: I6df4466cc7f8ff1524e9736cb54376c0ee166070
Reviewed-on: https://chromium-review.googlesource.com/c/1335567Reviewed-by: default avatarColin Blundell <blundell@chromium.org>
Reviewed-by: default avatarMihai Sardarescu <msarda@chromium.org>
Commit-Queue: Sergio Villar <svillar@igalia.com>
Cr-Commit-Position: refs/heads/master@{#608788}
parent c0d58678
......@@ -21,6 +21,10 @@ namespace browser_sync {
class ProfileSyncService;
}
namespace identity {
class IdentityManager;
}
class AccountTrackerService;
class AuthenticationServiceDelegate;
@class ChromeIdentity;
......@@ -39,6 +43,7 @@ class AuthenticationService : public KeyedService,
ProfileOAuth2TokenService* token_service,
SyncSetupService* sync_setup_service,
AccountTrackerService* account_tracker,
identity::IdentityManager* identity_manager,
SigninManager* signin_manager,
browser_sync::ProfileSyncService* sync_service);
~AuthenticationService() override;
......@@ -198,6 +203,7 @@ class AuthenticationService : public KeyedService,
ProfileOAuth2TokenService* token_service_ = nullptr;
SyncSetupService* sync_setup_service_ = nullptr;
AccountTrackerService* account_tracker_ = nullptr;
identity::IdentityManager* identity_manager_ = nullptr;
SigninManager* signin_manager_ = nullptr;
browser_sync::ProfileSyncService* sync_service_ = nullptr;
......
......@@ -31,6 +31,7 @@
#include "ios/public/provider/chrome/browser/chrome_browser_provider.h"
#import "ios/public/provider/chrome/browser/signin/chrome_identity.h"
#include "ios/public/provider/chrome/browser/signin/chrome_identity_service.h"
#import "services/identity/public/cpp/identity_manager.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
......@@ -69,12 +70,14 @@ AuthenticationService::AuthenticationService(
ProfileOAuth2TokenService* token_service,
SyncSetupService* sync_setup_service,
AccountTrackerService* account_tracker,
identity::IdentityManager* identity_manager,
SigninManager* signin_manager,
browser_sync::ProfileSyncService* sync_service)
: pref_service_(pref_service),
token_service_(token_service),
sync_setup_service_(sync_setup_service),
account_tracker_(account_tracker),
identity_manager_(identity_manager),
signin_manager_(signin_manager),
sync_service_(sync_service),
identity_service_observer_(this),
......@@ -82,6 +85,7 @@ AuthenticationService::AuthenticationService(
DCHECK(pref_service_);
DCHECK(sync_setup_service_);
DCHECK(account_tracker_);
DCHECK(identity_manager_);
DCHECK(signin_manager_);
DCHECK(sync_service_);
token_service_->AddObserver(this);
......@@ -222,7 +226,11 @@ void AuthenticationService::ComputeHaveAccountsChanged() {
// While the AuthenticationService is in background, changes should be shown
// to the user and |should_prompt| is true.
ReloadCredentialsFromIdentities(!is_in_foreground_ /* should_prompt */);
std::vector<std::string> new_accounts = token_service_->GetAccounts();
std::vector<AccountInfo> new_accounts_info =
identity_manager_->GetAccountsWithRefreshTokens();
std::vector<std::string> new_accounts;
for (const AccountInfo& account_info : new_accounts_info)
new_accounts.push_back(account_info.account_id);
std::vector<std::string> old_accounts = GetAccountsInPrefs();
std::sort(new_accounts.begin(), new_accounts.end());
std::sort(old_accounts.begin(), old_accounts.end());
......@@ -270,10 +278,11 @@ void AuthenticationService::MigrateAccountsStoredInPrefsIfNeeded() {
}
void AuthenticationService::StoreAccountsInPrefs() {
std::vector<std::string> accounts(token_service_->GetAccounts());
std::vector<AccountInfo> accounts(
identity_manager_->GetAccountsWithRefreshTokens());
base::ListValue accounts_pref_value;
for (const std::string& account : accounts)
accounts_pref_value.AppendString(account);
for (const AccountInfo& account_info : accounts)
accounts_pref_value.AppendString(account_info.account_id);
pref_service_->Set(prefs::kSigninLastAccounts, accounts_pref_value);
}
......@@ -299,7 +308,7 @@ ChromeIdentity* AuthenticationService::GetAuthenticatedIdentity() {
return nil;
std::string authenticated_account_id =
signin_manager_->GetAuthenticatedAccountId();
identity_manager_->GetPrimaryAccountId();
std::string authenticated_gaia_id =
account_tracker_->GetAccountInfo(authenticated_account_id).gaia;
......@@ -328,7 +337,7 @@ void AuthenticationService::SignIn(ChromeIdentity* identity,
std::string new_authenticated_account_id =
account_tracker_->SeedAccountInfo(info);
std::string old_authenticated_account_id =
signin_manager_->GetAuthenticatedAccountId();
identity_manager_->GetPrimaryAccountId();
// |SigninManager::SetAuthenticatedAccountId| simply ignores the call if
// there is already a signed in user. Check that there is no signed in account
// or that the new signed in account matches the old one to avoid a mismatch
......@@ -342,6 +351,8 @@ void AuthenticationService::SignIn(ChromeIdentity* identity,
// Update the SigninManager with the new logged in identity.
std::string new_authenticated_username =
account_tracker_->GetAccountInfo(new_authenticated_account_id).email;
// TODO(crbug.com/889902): Remove the SigninManager usage once the
// alternative API for this call is available.
signin_manager_->OnExternalSigninCompleted(new_authenticated_username);
// Reload all credentials to match the desktop model. Exclude all the
......@@ -375,8 +386,9 @@ void AuthenticationService::SignOut(
bool is_managed = IsAuthenticatedIdentityManaged();
sync_service_->RequestStop(syncer::SyncService::CLEAR_DATA);
signin_manager_->SignOut(signout_source,
signin_metrics::SignoutDelete::IGNORE_METRIC);
identity_manager_->ClearPrimaryAccount(
identity::IdentityManager::ClearAccountTokensAction::kDefault,
signout_source, signin_metrics::SignoutDelete::IGNORE_METRIC);
breakpad_helper::SetCurrentlySignedIn(false);
cached_mdm_infos_.clear();
if (is_managed) {
......@@ -576,21 +588,21 @@ bool AuthenticationService::IsAuthenticated() {
// Reload credentials to ensure that the user is still authenticated.
ReloadCredentialsFromIdentities(true /* should_prompt */);
}
return signin_manager_->IsAuthenticated();
return identity_manager_->HasPrimaryAccount();
}
NSString* AuthenticationService::GetAuthenticatedUserEmail() {
if (!IsAuthenticated())
return nil;
std::string authenticated_username =
signin_manager_->GetAuthenticatedAccountInfo().email;
identity_manager_->GetPrimaryAccountInfo().email;
DCHECK_LT(0U, authenticated_username.length());
return base::SysUTF8ToNSString(authenticated_username);
}
bool AuthenticationService::IsAuthenticatedIdentityManaged() {
std::string hosted_domain =
signin_manager_->GetAuthenticatedAccountInfo().hosted_domain;
identity_manager_->GetPrimaryAccountInfo().hosted_domain;
return !hosted_domain.empty() &&
hosted_domain != AccountTrackerService::kNoHostedDomainFound;
}
......@@ -13,6 +13,7 @@
#include "ios/chrome/browser/signin/account_tracker_service_factory.h"
#import "ios/chrome/browser/signin/authentication_service.h"
#import "ios/chrome/browser/signin/authentication_service_delegate.h"
#include "ios/chrome/browser/signin/identity_manager_factory.h"
#include "ios/chrome/browser/signin/profile_oauth2_token_service_factory.h"
#include "ios/chrome/browser/signin/signin_manager_factory.h"
#include "ios/chrome/browser/sync/profile_sync_service_factory.h"
......@@ -54,6 +55,7 @@ AuthenticationServiceFactory::AuthenticationServiceFactory()
BrowserStateDependencyManager::GetInstance()) {
DependsOn(ios::AccountTrackerServiceFactory::GetInstance());
DependsOn(ProfileOAuth2TokenServiceFactory::GetInstance());
DependsOn(IdentityManagerFactory::GetInstance());
DependsOn(ios::SigninManagerFactory::GetInstance());
DependsOn(SyncSetupServiceFactory::GetInstance());
DependsOn(ProfileSyncServiceFactory::GetInstance());
......@@ -71,6 +73,7 @@ AuthenticationServiceFactory::BuildServiceInstanceFor(
ProfileOAuth2TokenServiceFactory::GetForBrowserState(browser_state),
SyncSetupServiceFactory::GetForBrowserState(browser_state),
ios::AccountTrackerServiceFactory::GetForBrowserState(browser_state),
IdentityManagerFactory::GetForBrowserState(browser_state),
ios::SigninManagerFactory::GetForBrowserState(browser_state),
ProfileSyncServiceFactory::GetForBrowserState(browser_state));
}
......
......@@ -10,6 +10,10 @@
#import "ios/chrome/browser/signin/authentication_service.h"
#import "ios/public/provider/chrome/browser/signin/chrome_identity.h"
namespace identity {
class IdentityManager;
}
namespace web {
class BrowserState;
}
......@@ -43,6 +47,7 @@ class AuthenticationServiceFake : public AuthenticationService {
ProfileOAuth2TokenService* token_service,
SyncSetupService* sync_setup_service,
AccountTrackerService* account_tracker,
identity::IdentityManager* identity_manager,
SigninManager* signin_manager,
browser_sync::ProfileSyncService* sync_service);
......
......@@ -11,6 +11,7 @@
#include "ios/chrome/browser/signin/account_tracker_service_factory.h"
#import "ios/chrome/browser/signin/authentication_service_delegate_fake.h"
#import "ios/chrome/browser/signin/authentication_service_factory.h"
#include "ios/chrome/browser/signin/identity_manager_factory.h"
#include "ios/chrome/browser/signin/profile_oauth2_token_service_factory.h"
#include "ios/chrome/browser/signin/signin_manager_factory.h"
#include "ios/chrome/browser/sync/profile_sync_service_factory.h"
......@@ -26,12 +27,14 @@ AuthenticationServiceFake::AuthenticationServiceFake(
ProfileOAuth2TokenService* token_service,
SyncSetupService* sync_setup_service,
AccountTrackerService* account_tracker,
identity::IdentityManager* identity_manager,
SigninManager* signin_manager,
browser_sync::ProfileSyncService* sync_service)
: AuthenticationService(pref_service,
token_service,
sync_setup_service,
account_tracker,
identity_manager,
signin_manager,
sync_service),
have_accounts_changed_(false) {}
......@@ -81,6 +84,7 @@ AuthenticationServiceFake::CreateAuthenticationService(
ProfileOAuth2TokenServiceFactory::GetForBrowserState(browser_state),
SyncSetupServiceFactory::GetForBrowserState(browser_state),
ios::AccountTrackerServiceFactory::GetForBrowserState(browser_state),
IdentityManagerFactory::GetForBrowserState(browser_state),
ios::SigninManagerFactory::GetForBrowserState(browser_state),
ProfileSyncServiceFactory::GetForBrowserState(browser_state)));
service->Initialize(std::make_unique<AuthenticationServiceDelegateFake>());
......
......@@ -27,6 +27,7 @@
#import "ios/chrome/browser/signin/authentication_service.h"
#import "ios/chrome/browser/signin/authentication_service_delegate_fake.h"
#import "ios/chrome/browser/signin/authentication_service_factory.h"
#import "ios/chrome/browser/signin/identity_manager_factory.h"
#import "ios/chrome/browser/signin/identity_test_environment_chrome_browser_state_adaptor.h"
#include "ios/chrome/browser/signin/ios_chrome_signin_client.h"
#include "ios/chrome/browser/signin/profile_oauth2_token_service_factory.h"
......@@ -180,6 +181,7 @@ class AuthenticationServiceTest : public PlatformTest,
SyncSetupServiceFactory::GetForBrowserState(browser_state_.get()),
ios::AccountTrackerServiceFactory::GetForBrowserState(
browser_state_.get()),
IdentityManagerFactory::GetForBrowserState(browser_state_.get()),
ios::SigninManagerFactory::GetForBrowserState(browser_state_.get()),
ProfileSyncServiceFactory::GetForBrowserState(browser_state_.get()));
authentication_service_->Initialize(
......@@ -425,24 +427,27 @@ TEST_F(AuthenticationServiceTest,
authentication_service_->SignIn(identity_, std::string());
identity_service_->AddIdentities(@[ @"foo3" ]);
ProfileOAuth2TokenService* token_service =
ProfileOAuth2TokenServiceFactory::GetForBrowserState(
browser_state_.get());
std::vector<std::string> accounts = token_service->GetAccounts();
std::sort(accounts.begin(), accounts.end());
auto account_compare_func = [](const AccountInfo& first,
const AccountInfo& second) {
return first.account_id < second.account_id;
};
std::vector<AccountInfo> accounts =
identity_manager()->GetAccountsWithRefreshTokens();
std::sort(accounts.begin(), accounts.end(), account_compare_func);
ASSERT_EQ(2u, accounts.size());
AccountTrackerService* account_tracker =
ios::AccountTrackerServiceFactory::GetForBrowserState(
browser_state_.get());
switch (account_tracker->GetMigrationState()) {
case AccountTrackerService::MIGRATION_NOT_STARTED:
EXPECT_EQ("foo2@foo.com", accounts[0]);
EXPECT_EQ("foo@foo.com", accounts[1]);
EXPECT_EQ("foo2@foo.com", accounts[0].account_id);
EXPECT_EQ("foo@foo.com", accounts[1].account_id);
break;
case AccountTrackerService::MIGRATION_IN_PROGRESS:
case AccountTrackerService::MIGRATION_DONE:
EXPECT_EQ("foo2ID", accounts[0]);
EXPECT_EQ("fooID", accounts[1]);
EXPECT_EQ("foo2ID", accounts[0].account_id);
EXPECT_EQ("fooID", accounts[1].account_id);
break;
case AccountTrackerService::NUM_MIGRATION_STATES:
FAIL() << "NUM_MIGRATION_STATES is not a real migration state.";
......@@ -456,20 +461,20 @@ TEST_F(AuthenticationServiceTest,
// Accounts are reloaded, "foo3@foo.com" is added as it is now in
// ChromeIdentityService.
accounts = token_service->GetAccounts();
std::sort(accounts.begin(), accounts.end());
accounts = identity_manager()->GetAccountsWithRefreshTokens();
std::sort(accounts.begin(), accounts.end(), account_compare_func);
ASSERT_EQ(3u, accounts.size());
switch (account_tracker->GetMigrationState()) {
case AccountTrackerService::MIGRATION_NOT_STARTED:
EXPECT_EQ("foo2@foo.com", accounts[0]);
EXPECT_EQ("foo3@foo.com", accounts[1]);
EXPECT_EQ("foo@foo.com", accounts[2]);
EXPECT_EQ("foo2@foo.com", accounts[0].account_id);
EXPECT_EQ("foo3@foo.com", accounts[1].account_id);
EXPECT_EQ("foo@foo.com", accounts[2].account_id);
break;
case AccountTrackerService::MIGRATION_IN_PROGRESS:
case AccountTrackerService::MIGRATION_DONE:
EXPECT_EQ("foo2ID", accounts[0]);
EXPECT_EQ("foo3ID", accounts[1]);
EXPECT_EQ("fooID", accounts[2]);
EXPECT_EQ("foo2ID", accounts[0].account_id);
EXPECT_EQ("foo3ID", accounts[1].account_id);
EXPECT_EQ("fooID", accounts[2].account_id);
break;
case AccountTrackerService::NUM_MIGRATION_STATES:
FAIL() << "NUM_MIGRATION_STATES is not a real migration state.";
......
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