Commit 59e63f23 authored by Jérôme Lebel's avatar Jérôme Lebel Committed by Commit Bot

[iOS] Switching hosted domain from ChromeIdentity to ChromeIdentityService

When sign-in with a new gmail account, the Chrome identity was
initialized with no hosted domain since the SSO profile was not fetched
yet.
The property was never updated later. Even when the AuthenticationFlow
was fetching hosted domain using the AuthenticationFlowPerformer.
During the sign-in process

To fix the issue, -ChromeIdentity.hostedDomain is remove. ChromeIdentity
stays just a proxy for SSOIdentity.
ChromeIdentityService::GetCachedHostedDomainForIdentity() is added to
get the hosted domain.

Bug introduced with:
  + crrev.com/c/1687254
  + crrev.com/i/1434627
  + crrev.com/c/1687254

Related to:
  + crrev.com/c/1732084 (adding new API)
  + crrev.com/i/1560989 (adding new implementation)
  => crrev.com/c/1732085 (switching implementation) <=
  + crrev.com/i/1560990 (old implementation cleanup)
  + crrev.com/c/1732101 (cleanup API)

Bug: 987380
Change-Id: I658d3e0b109c68583db58b12a034de072adfc45f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1732085
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Reviewed-by: default avatarSylvain Defresne <sdefresne@chromium.org>
Reviewed-by: default avatarMihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#684155}
parent d1cf8db0
...@@ -494,8 +494,8 @@ TEST_F(AuthenticationServiceTest, MigrateAccountsStoredInPref) { ...@@ -494,8 +494,8 @@ TEST_F(AuthenticationServiceTest, MigrateAccountsStoredInPref) {
authentication_service()->SignIn(identity(0)); authentication_service()->SignIn(identity(0));
std::vector<std::string> accounts_in_prefs = GetAccountsInPrefs(); std::vector<std::string> accounts_in_prefs = GetAccountsInPrefs();
ASSERT_EQ(2U, accounts_in_prefs.size()); ASSERT_EQ(2U, accounts_in_prefs.size());
EXPECT_EQ("foo2@foo.com", accounts_in_prefs[0]); EXPECT_EQ("foo2@gmail.com", accounts_in_prefs[0]);
EXPECT_EQ("foo@foo.com", accounts_in_prefs[1]); EXPECT_EQ("foo@gmail.com", accounts_in_prefs[1]);
// Migrate the accounts. // Migrate the accounts.
browser_state_->GetPrefs()->SetInteger( browser_state_->GetPrefs()->SetInteger(
......
...@@ -20,7 +20,9 @@ ...@@ -20,7 +20,9 @@
namespace { namespace {
// Returns the account info for |identity| (which must not be nil). // Returns the account info for |identity| (which must not be nil).
DeviceAccountsProvider::AccountInfo GetAccountInfo(ChromeIdentity* identity) { DeviceAccountsProvider::AccountInfo GetAccountInfo(
ChromeIdentity* identity,
ios::ChromeIdentityService* identity_service) {
DCHECK(identity); DCHECK(identity);
DeviceAccountsProvider::AccountInfo account_info; DeviceAccountsProvider::AccountInfo account_info;
account_info.gaia = base::SysNSStringToUTF8([identity gaiaID]); account_info.gaia = base::SysNSStringToUTF8([identity gaiaID]);
...@@ -30,7 +32,8 @@ DeviceAccountsProvider::AccountInfo GetAccountInfo(ChromeIdentity* identity) { ...@@ -30,7 +32,8 @@ DeviceAccountsProvider::AccountInfo GetAccountInfo(ChromeIdentity* identity) {
// fetched from gaia; in that case, set account_info.hosted_domain to // fetched from gaia; in that case, set account_info.hosted_domain to
// an empty string. Otherwise, set it to the value of the hostedDomain // an empty string. Otherwise, set it to the value of the hostedDomain
// or kNoHostedDomainFound if the string is empty. // or kNoHostedDomainFound if the string is empty.
NSString* hostedDomain = [identity hostedDomain]; NSString* hostedDomain =
identity_service->GetCachedHostedDomainForIdentity(identity);
if (hostedDomain) { if (hostedDomain) {
account_info.hosted_domain = [hostedDomain length] account_info.hosted_domain = [hostedDomain length]
? base::SysNSStringToUTF8(hostedDomain) ? base::SysNSStringToUTF8(hostedDomain)
...@@ -68,11 +71,11 @@ void DeviceAccountsProviderImpl::GetAccessToken( ...@@ -68,11 +71,11 @@ void DeviceAccountsProviderImpl::GetAccessToken(
std::vector<DeviceAccountsProvider::AccountInfo> std::vector<DeviceAccountsProvider::AccountInfo>
DeviceAccountsProviderImpl::GetAllAccounts() const { DeviceAccountsProviderImpl::GetAllAccounts() const {
std::vector<AccountInfo> accounts; std::vector<AccountInfo> accounts;
NSArray* identities = ios::GetChromeBrowserProvider() ios::ChromeIdentityService* identity_service =
->GetChromeIdentityService() ios::GetChromeBrowserProvider()->GetChromeIdentityService();
->GetAllIdentities(); NSArray* identities = identity_service->GetAllIdentities();
for (ChromeIdentity* identity in identities) { for (ChromeIdentity* identity in identities) {
accounts.push_back(GetAccountInfo(identity)); accounts.push_back(GetAccountInfo(identity, identity_service));
} }
return accounts; return accounts;
} }
......
...@@ -139,11 +139,12 @@ const int64_t kAuthenticationFlowTimeoutSeconds = 10; ...@@ -139,11 +139,12 @@ const int64_t kAuthenticationFlowTimeoutSeconds = 10;
- (void)fetchManagedStatus:(ios::ChromeBrowserState*)browserState - (void)fetchManagedStatus:(ios::ChromeBrowserState*)browserState
forIdentity:(ChromeIdentity*)identity { forIdentity:(ChromeIdentity*)identity {
if (gaia::ExtractDomainName(gaia::CanonicalizeEmail( ios::ChromeIdentityService* identityService =
base::SysNSStringToUTF8(identity.userEmail))) == "gmail.com") { ios::GetChromeBrowserProvider()->GetChromeIdentityService();
// Do nothing for @gmail.com addresses as they can't have a hosted domain. NSString* hostedDomain =
// This avoids waiting for this step to complete (and a network call). identityService->GetCachedHostedDomainForIdentity(identity);
[_delegate didFetchManagedStatus:nil]; if (hostedDomain) {
[_delegate didFetchManagedStatus:hostedDomain];
return; return;
} }
......
...@@ -485,7 +485,7 @@ TEST_F(PaymentRequestMediatorTest, TestFooterItem) { ...@@ -485,7 +485,7 @@ TEST_F(PaymentRequestMediatorTest, TestFooterItem) {
EXPECT_TRUE([footer_item.text EXPECT_TRUE([footer_item.text
isEqualToString:l10n_util::GetNSStringF( isEqualToString:l10n_util::GetNSStringF(
IDS_PAYMENTS_CARD_AND_ADDRESS_SETTINGS_SIGNED_IN, IDS_PAYMENTS_CARD_AND_ADDRESS_SETTINGS_SIGNED_IN,
base::ASCIIToUTF16("username@foo.com"))]); base::ASCIIToUTF16("username@gmail.com"))]);
// Record that the first transaction completed. // Record that the first transaction completed.
pref_service()->SetBoolean(payments::kPaymentsFirstTransactionCompleted, pref_service()->SetBoolean(payments::kPaymentsFirstTransactionCompleted,
......
...@@ -19,6 +19,7 @@ source_set("signin") { ...@@ -19,6 +19,7 @@ source_set("signin") {
] ]
deps = [ deps = [
"//base", "//base",
"//google_apis",
] ]
} }
......
...@@ -34,10 +34,9 @@ ...@@ -34,10 +34,9 @@
- (NSString*)description { - (NSString*)description {
return [NSString stringWithFormat:@"<%@: %p, GaiaID: \"%@\", name: \"%@\", " return [NSString stringWithFormat:@"<%@: %p, GaiaID: \"%@\", name: \"%@\", "
@"email: \"%@\", hosted domain: \"%@\">", @"email: \"%@\">",
self.class.description, self, self.gaiaID, self.class.description, self, self.gaiaID,
self.userFullName, self.userEmail, self.userFullName, self.userEmail];
self.hostedDomain];
} }
@end @end
...@@ -46,6 +46,12 @@ typedef void (^ForgetIdentityCallback)(NSError* error); ...@@ -46,6 +46,12 @@ typedef void (^ForgetIdentityCallback)(NSError* error);
typedef void (^GetAvatarCallback)(UIImage* avatar); typedef void (^GetAvatarCallback)(UIImage* avatar);
// Callback passed to method |GetHostedDomainForIdentity()|. // Callback passed to method |GetHostedDomainForIdentity()|.
// |hosted_domain|:
// + nil, if error.
// + an empty string, if this is a consumer account (e.g. foo@gmail.com).
// + non-empty string, if the hosted domain was fetched and this account
// has a hosted domain.
// |error|: Error if failed to fetch the identity profile.
typedef void (^GetHostedDomainCallback)(NSString* hosted_domain, typedef void (^GetHostedDomainCallback)(NSString* hosted_domain,
NSError* error); NSError* error);
...@@ -196,7 +202,6 @@ class ChromeIdentityService { ...@@ -196,7 +202,6 @@ class ChromeIdentityService {
virtual void GetHostedDomainForIdentity(ChromeIdentity* identity, virtual void GetHostedDomainForIdentity(ChromeIdentity* identity,
GetHostedDomainCallback callback); GetHostedDomainCallback callback);
// DO NOT USE YET.
// Returns the identity hosted domain, for the cache only. This method // Returns the identity hosted domain, for the cache only. This method
// returns: // returns:
// + nil, if the hosted domain value was yet not fetched from the server. // + nil, if the hosted domain value was yet not fetched from the server.
......
...@@ -4,6 +4,9 @@ ...@@ -4,6 +4,9 @@
#include "ios/public/provider/chrome/browser/signin/chrome_identity_service.h" #include "ios/public/provider/chrome/browser/signin/chrome_identity_service.h"
#include "base/strings/sys_string_conversions.h"
#include "google_apis/gaia/gaia_auth_util.h"
#import "ios/public/provider/chrome/browser/signin/chrome_identity.h"
#include "ios/public/provider/chrome/browser/signin/chrome_identity_interaction_manager.h" #include "ios/public/provider/chrome/browser/signin/chrome_identity_interaction_manager.h"
#if !defined(__has_feature) || !__has_feature(objc_arc) #if !defined(__has_feature) || !__has_feature(objc_arc)
...@@ -107,6 +110,15 @@ void ChromeIdentityService::GetHostedDomainForIdentity( ...@@ -107,6 +110,15 @@ void ChromeIdentityService::GetHostedDomainForIdentity(
NSString* ChromeIdentityService::GetCachedHostedDomainForIdentity( NSString* ChromeIdentityService::GetCachedHostedDomainForIdentity(
ChromeIdentity* identity) { ChromeIdentity* identity) {
// @gmail.com accounts are end consumer accounts so it is safe to return @""
// even when SSOProfileSource has a nil profile for |sso_identity|.
//
// Note: This is also needed during the sign-in flow as it avoids waiting for
// the profile of |sso_identity| to be fetched from the server.
if (gaia::ExtractDomainName(base::SysNSStringToUTF8(identity.userEmail)) ==
"gmail.com") {
return @"";
}
return nil; return nil;
} }
......
...@@ -56,6 +56,9 @@ class FakeChromeIdentityService : public ChromeIdentityService { ...@@ -56,6 +56,9 @@ class FakeChromeIdentityService : public ChromeIdentityService {
ChromeIdentity* identity, ChromeIdentity* identity,
GetHostedDomainCallback callback) override; GetHostedDomainCallback callback) override;
virtual NSString* GetCachedHostedDomainForIdentity(
ChromeIdentity* identity) override;
MOCK_METHOD1(GetMDMDeviceStatus, MOCK_METHOD1(GetMDMDeviceStatus,
ios::MDMDeviceStatus(NSDictionary* user_info)); ios::MDMDeviceStatus(NSDictionary* user_info));
......
...@@ -29,16 +29,9 @@ UIImage* FakeGetCachedAvatarForIdentity(ChromeIdentity*) { ...@@ -29,16 +29,9 @@ UIImage* FakeGetCachedAvatarForIdentity(ChromeIdentity*) {
return provider ? provider->GetDefaultAvatar() : nil; return provider ? provider->GetDefaultAvatar() : nil;
} }
void FakeGetHostedDomainForIdentity(ChromeIdentity* identity, NSString* FakeGetHostedDomainForIdentity(ChromeIdentity* identity) {
ios::GetHostedDomainCallback callback) { return base::SysUTF8ToNSString(gaia::ExtractDomainName(
NSString* domain = base::SysUTF8ToNSString(gaia::ExtractDomainName(
gaia::CanonicalizeEmail(base::SysNSStringToUTF8(identity.userEmail)))); gaia::CanonicalizeEmail(base::SysNSStringToUTF8(identity.userEmail))));
// |GetHostedDomainForIdentity| is normally an asynchronous operation , this
// is replicated here by dispatching it.
dispatch_async(dispatch_get_main_queue(), ^{
callback(domain, nil);
});
} }
} }
...@@ -98,7 +91,7 @@ void FakeGetHostedDomainForIdentity(ChromeIdentity* identity, ...@@ -98,7 +91,7 @@ void FakeGetHostedDomainForIdentity(ChromeIdentity* identity,
@end @end
namespace ios { namespace ios {
NSString* const kIdentityEmailFormat = @"%@@foo.com"; NSString* const kIdentityEmailFormat = @"%@@gmail.com";
NSString* const kIdentityGaiaIDFormat = @"%@ID"; NSString* const kIdentityGaiaIDFormat = @"%@ID";
FakeChromeIdentityService::FakeChromeIdentityService() FakeChromeIdentityService::FakeChromeIdentityService()
...@@ -239,7 +232,22 @@ void FakeChromeIdentityService::GetAvatarForIdentity( ...@@ -239,7 +232,22 @@ void FakeChromeIdentityService::GetAvatarForIdentity(
void FakeChromeIdentityService::GetHostedDomainForIdentity( void FakeChromeIdentityService::GetHostedDomainForIdentity(
ChromeIdentity* identity, ChromeIdentity* identity,
GetHostedDomainCallback callback) { GetHostedDomainCallback callback) {
FakeGetHostedDomainForIdentity(identity, callback); NSString* domain = FakeGetHostedDomainForIdentity(identity);
// |GetHostedDomainForIdentity| is normally an asynchronous operation , this
// is replicated here by dispatching it.
dispatch_async(dispatch_get_main_queue(), ^{
callback(domain, nil);
});
}
NSString* FakeChromeIdentityService::GetCachedHostedDomainForIdentity(
ChromeIdentity* identity) {
NSString* domain =
ChromeIdentityService::GetCachedHostedDomainForIdentity(identity);
if (domain) {
return domain;
}
return FakeGetHostedDomainForIdentity(identity);
} }
void FakeChromeIdentityService::SetUpForIntegrationTests() {} void FakeChromeIdentityService::SetUpForIntegrationTests() {}
......
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