diff options
20 files changed, 205 insertions, 54 deletions
diff --git a/chrome/browser/android/profiles/profile_downloader_android.cc b/chrome/browser/android/profiles/profile_downloader_android.cc index c6d4785..3be584c 100644 --- a/chrome/browser/android/profiles/profile_downloader_android.cc +++ b/chrome/browser/android/profiles/profile_downloader_android.cc @@ -12,6 +12,9 @@ #include "chrome/browser/profiles/profile_downloader.h" #include "chrome/browser/profiles/profile_downloader_delegate.h" #include "chrome/browser/profiles/profile_manager.h" +#include "chrome/browser/signin/account_tracker_service_factory.h" +#include "components/signin/core/browser/account_tracker_service.h" +#include "google_apis/gaia/gaia_auth_util.h" #include "jni/ProfileDownloader_jni.h" #include "third_party/skia/include/core/SkBitmap.h" #include "ui/gfx/android/java_bitmap.h" @@ -23,11 +26,13 @@ namespace { // An account fetcher callback. class AccountInfoRetriever : public ProfileDownloaderDelegate { public: - explicit AccountInfoRetriever(Profile* profile, - const std::string& account_id, - const int desired_image_side_pixels) + AccountInfoRetriever(Profile* profile, + const std::string& account_id, + const std::string& email, + const int desired_image_side_pixels) : profile_(profile), account_id_(account_id), + email_(email), desired_image_side_pixels_(desired_image_side_pixels) {} void Start() { @@ -60,7 +65,7 @@ class AccountInfoRetriever : public ProfileDownloaderDelegate { virtual void OnProfileDownloadSuccess( ProfileDownloader* downloader) OVERRIDE { ProfileDownloaderAndroid::OnProfileDownloadSuccess( - account_id_, + email_, downloader->GetProfileFullName(), downloader->GetProfilePicture()); Shutdown(); @@ -79,8 +84,9 @@ class AccountInfoRetriever : public ProfileDownloaderDelegate { // The browser profile associated with this download request. Profile* profile_; - // The account ID (email address) to be loaded. + // The account ID and email address of account to be loaded. const std::string account_id_; + const std::string email_; // Desired side length of the profile image (in pixels). const int desired_image_side_pixels_; @@ -150,13 +156,20 @@ void StartFetchingAccountInfoFor( JNIEnv* env, jclass clazz, jobject jprofile, - jstring jaccount_id, + jstring jemail, jint image_side_pixels) { Profile* profile = ProfileAndroid::FromProfileAndroid(jprofile); - const std::string account_id = - base::android::ConvertJavaStringToUTF8(env, jaccount_id); + const std::string email = + base::android::ConvertJavaStringToUTF8(env, jemail); + // TODO(rogerta): the java code will need to pass in the gaia-id + // of the account instead of the email when chrome uses gaia-id as key. + DCHECK_EQ(AccountTrackerService::MIGRATION_NOT_STARTED, + AccountTrackerServiceFactory::GetForProfile(profile)-> + GetMigrationState()); AccountInfoRetriever* retriever = - new AccountInfoRetriever(profile, account_id, image_side_pixels); + new AccountInfoRetriever( + profile, gaia::CanonicalizeEmail(gaia::SanitizeEmail(email)), email, + image_side_pixels); retriever->Start(); } diff --git a/chrome/browser/policy/cloud/user_policy_signin_service_mobile.cc b/chrome/browser/policy/cloud/user_policy_signin_service_mobile.cc index abd580e..e194f95 100644 --- a/chrome/browser/policy/cloud/user_policy_signin_service_mobile.cc +++ b/chrome/browser/policy/cloud/user_policy_signin_service_mobile.cc @@ -193,8 +193,7 @@ void UserPolicySigninService::RegisterCloudPolicyService() { // If the user signed-out while this task was waiting then Shutdown() would // have been called, which would have invalidated this task. Since we're here // then the user must still be signed-in. - const std::string& username = signin_manager()->GetAuthenticatedUsername(); - DCHECK(!username.empty()); + DCHECK(signin_manager()->IsAuthenticated()); DCHECK(!policy_manager()->IsClientRegistered()); DCHECK(policy_manager()->core()->client()); @@ -207,7 +206,7 @@ void UserPolicySigninService::RegisterCloudPolicyService() { GetRegistrationType())); registration_helper_->StartRegistration( oauth2_token_service_, - username, + signin_manager()->GetAuthenticatedUsername(), base::Bind(&UserPolicySigninService::OnRegistrationDone, base::Unretained(this))); } diff --git a/chrome/browser/services/gcm/fake_signin_manager.cc b/chrome/browser/services/gcm/fake_signin_manager.cc index dea05ce..049b415 100644 --- a/chrome/browser/services/gcm/fake_signin_manager.cc +++ b/chrome/browser/services/gcm/fake_signin_manager.cc @@ -45,7 +45,7 @@ void FakeSigninManager::SignOut( signin_metrics::ProfileSignout signout_source_metric) { const std::string account_id = GetAuthenticatedAccountId(); const std::string username = GetAuthenticatedUsername(); - clear_authenticated_username(); + ClearAuthenticatedUsername(); profile_->GetPrefs()->ClearPref(prefs::kGoogleServicesUsername); FOR_EACH_OBSERVER(Observer, observer_list_, diff --git a/chrome/browser/signin/android_profile_oauth2_token_service.cc b/chrome/browser/signin/android_profile_oauth2_token_service.cc index c97d777..9f8fac9 100644 --- a/chrome/browser/signin/android_profile_oauth2_token_service.cc +++ b/chrome/browser/signin/android_profile_oauth2_token_service.cc @@ -13,6 +13,7 @@ #include "chrome/browser/signin/profile_oauth2_token_service_factory.h" #include "chrome/browser/sync/profile_sync_service_android.h" #include "content/public/browser/browser_thread.h" +#include "google_apis/gaia/gaia_auth_util.h" #include "google_apis/gaia/oauth2_access_token_fetcher.h" #include "jni/OAuth2TokenService_jni.h" @@ -217,7 +218,7 @@ AndroidProfileOAuth2TokenService::CreateAccessTokenFetcher( const std::string& account_id, net::URLRequestContextGetter* getter, OAuth2AccessTokenConsumer* consumer) { - DCHECK(!account_id.empty()); + ValidateAccountId(account_id); return new AndroidAccessTokenFetcher(consumer, account_id); } @@ -226,6 +227,7 @@ void AndroidProfileOAuth2TokenService::InvalidateOAuth2Token( const std::string& client_id, const ScopeSet& scopes, const std::string& access_token) { + ValidateAccountId(account_id); OAuth2TokenService::InvalidateOAuth2Token(account_id, client_id, scopes, @@ -246,6 +248,8 @@ void AndroidProfileOAuth2TokenService::ValidateAccounts( jboolean j_force_notifications) { VLOG(1) << "AndroidProfileOAuth2TokenService::ValidateAccounts from java"; std::string signed_in_account = ConvertJavaStringToUTF8(env, j_current_acc); + if (!signed_in_account.empty()) + signed_in_account = gaia::CanonicalizeEmail(signed_in_account); ValidateAccounts(signed_in_account, j_force_notifications != JNI_FALSE); } @@ -257,6 +261,12 @@ void AndroidProfileOAuth2TokenService::ValidateAccounts( std::vector<std::string> refreshed_ids; std::vector<std::string> revoked_ids; + // Canonicalize system accounts. |prev_ids| is already done. + for (size_t i = 0; i < curr_ids.size(); ++i) + curr_ids[i] = gaia::CanonicalizeEmail(curr_ids[i]); + for (size_t i = 0; i < prev_ids.size(); ++i) + ValidateAccountId(prev_ids[i]); + VLOG(1) << "AndroidProfileOAuth2TokenService::ValidateAccounts:" << " sigined_in_account=" << signed_in_account << " prev_ids=" << prev_ids.size() diff --git a/chrome/browser/signin/android_profile_oauth2_token_service.h b/chrome/browser/signin/android_profile_oauth2_token_service.h index d6d4a87..a878536 100644 --- a/chrome/browser/signin/android_profile_oauth2_token_service.h +++ b/chrome/browser/signin/android_profile_oauth2_token_service.h @@ -111,6 +111,7 @@ class AndroidProfileOAuth2TokenService : public ProfileOAuth2TokenService { // Called to notify observers when refresh tokans have been loaded. virtual void FireRefreshTokensLoaded() override; + private: // Return whether |signed_in_account| is valid and we have access // to all the tokens in |curr_account_ids|. If |force_notifications| is true, // TokenAvailable notifications will be sent anyway, even if the account was @@ -122,7 +123,6 @@ class AndroidProfileOAuth2TokenService : public ProfileOAuth2TokenService { std::vector<std::string>& revoked_ids, bool force_notifications); - private: base::android::ScopedJavaGlobalRef<jobject> java_ref_; static bool is_testing_profile_; diff --git a/chrome/browser/signin/signin_ui_util.cc b/chrome/browser/signin/signin_ui_util.cc index c122f28..8316747 100644 --- a/chrome/browser/signin/signin_ui_util.cc +++ b/chrome/browser/signin/signin_ui_util.cc @@ -102,8 +102,6 @@ void GetStatusLabelsForAuthError(Profile* profile, const SigninManagerBase& signin_manager, base::string16* status_label, base::string16* link_label) { - base::string16 username = - base::UTF8ToUTF16(signin_manager.GetAuthenticatedUsername()); base::string16 product_name = l10n_util::GetStringUTF16(IDS_PRODUCT_NAME); if (link_label) link_label->assign(l10n_util::GetStringUTF16(IDS_SYNC_RELOGIN_LINK_LABEL)); @@ -118,7 +116,7 @@ void GetStatusLabelsForAuthError(Profile* profile, case GoogleServiceAuthError::ACCOUNT_DISABLED: // If the user name is empty then the first login failed, otherwise the // credentials are out-of-date. - if (username.empty()) { + if (!signin_manager.IsAuthenticated()) { if (status_label) { status_label->assign( l10n_util::GetStringUTF16(IDS_SYNC_INVALID_USER_CREDENTIALS)); diff --git a/chrome/browser/sync/supervised_user_signin_manager_wrapper.cc b/chrome/browser/sync/supervised_user_signin_manager_wrapper.cc index 21b045e..36c24ef 100644 --- a/chrome/browser/sync/supervised_user_signin_manager_wrapper.cc +++ b/chrome/browser/sync/supervised_user_signin_manager_wrapper.cc @@ -25,12 +25,11 @@ SigninManagerBase* SupervisedUserSigninManagerWrapper::GetOriginal() { } std::string SupervisedUserSigninManagerWrapper::GetEffectiveUsername() const { - const std::string& auth_username = original_->GetAuthenticatedUsername(); #if defined(ENABLE_MANAGED_USERS) - if (auth_username.empty() && profile_->IsSupervised()) + if (!original_->IsAuthenticated() && profile_->IsSupervised()) return supervised_users::kSupervisedUserPseudoEmail; #endif - return auth_username; + return original_->GetAuthenticatedUsername(); } std::string SupervisedUserSigninManagerWrapper::GetAccountIdToUse() const { diff --git a/chrome/browser/ui/app_list/search/people/people_provider.cc b/chrome/browser/ui/app_list/search/people/people_provider.cc index 97419ae..66b3c69 100644 --- a/chrome/browser/ui/app_list/search/people/people_provider.cc +++ b/chrome/browser/ui/app_list/search/people/people_provider.cc @@ -118,12 +118,14 @@ void PeopleProvider::RequestAccessToken() { if (access_token_request_ != NULL) return; - ProfileOAuth2TokenService* token_service = - ProfileOAuth2TokenServiceFactory::GetForProfile(profile_); SigninManagerBase* signin_manager = SigninManagerFactory::GetInstance()->GetForProfile(profile_); - access_token_request_ = token_service->StartRequest( - signin_manager->GetAuthenticatedAccountId(), oauth2_scope_, this); + if (signin_manager->IsAuthenticated()) { + ProfileOAuth2TokenService* token_service = + ProfileOAuth2TokenServiceFactory::GetForProfile(profile_); + access_token_request_ = token_service->StartRequest( + signin_manager->GetAuthenticatedAccountId(), oauth2_scope_, this); + } } GURL PeopleProvider::GetQueryUrl(const std::string& query) { diff --git a/chrome/browser/ui/webui/signin/inline_login_handler_impl.cc b/chrome/browser/ui/webui/signin/inline_login_handler_impl.cc index 03dd905..9da3ddb 100644 --- a/chrome/browser/ui/webui/signin/inline_login_handler_impl.cc +++ b/chrome/browser/ui/webui/signin/inline_login_handler_impl.cc @@ -13,6 +13,7 @@ #include "base/values.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/signin/about_signin_internals_factory.h" +#include "chrome/browser/signin/account_tracker_service_factory.h" #include "chrome/browser/signin/chrome_signin_client_factory.h" #include "chrome/browser/signin/local_auth.h" #include "chrome/browser/signin/profile_oauth2_token_service_factory.h" @@ -29,6 +30,7 @@ #include "chrome/browser/ui/webui/signin/login_ui_service_factory.h" #include "chrome/common/url_constants.h" #include "components/signin/core/browser/about_signin_internals.h" +#include "components/signin/core/browser/account_tracker_service.h" #include "components/signin/core/browser/profile_oauth2_token_service.h" #include "components/signin/core/browser/signin_error_controller.h" #include "components/signin/core/browser/signin_oauth_helper.h" @@ -131,8 +133,14 @@ void InlineSigninHelper::OnSigninOAuthInformationAvailable( if (source == signin::SOURCE_AVATAR_BUBBLE_ADD_ACCOUNT || source == signin::SOURCE_REAUTH) { + // TODO(rogerta): the javascript code will need to pass in the gaia-id + // of the account instead of the email when chrome uses gaia-id as key. + DCHECK_EQ(AccountTrackerService::MIGRATION_NOT_STARTED, + AccountTrackerServiceFactory::GetForProfile(profile_)-> + GetMigrationState()); + const std::string account_id = gaia::CanonicalizeEmail(email); ProfileOAuth2TokenServiceFactory::GetForProfile(profile_)-> - UpdateCredentials(email, refresh_token); + UpdateCredentials(account_id, refresh_token); if (signin::IsAutoCloseEnabledInURL(current_url_)) { // Close the gaia sign in tab via a task to make sure we aren't in the diff --git a/components/enhanced_bookmarks/bookmark_server_service.cc b/components/enhanced_bookmarks/bookmark_server_service.cc index 08217a2..1e6f4b0 100644 --- a/components/enhanced_bookmarks/bookmark_server_service.cc +++ b/components/enhanced_bookmarks/bookmark_server_service.cc @@ -67,8 +67,7 @@ void BookmarkServerService::TriggerTokenRequest(bool cancel_previous) { if (token_request_ || url_fetcher_) return; // Fetcher is already running. - const std::string username(signin_manager_->GetAuthenticatedUsername()); - if (!username.length()) { + if (!signin_manager_->IsAuthenticated()) { // User is not signed in. CleanAfterFailure(); Notify(); @@ -77,7 +76,8 @@ void BookmarkServerService::TriggerTokenRequest(bool cancel_previous) { // Find a token. OAuth2TokenService::ScopeSet scopes; scopes.insert(GaiaConstants::kChromeSyncOAuth2Scope); - token_request_ = token_service_->StartRequest(username, scopes, this); + token_request_ = token_service_->StartRequest( + signin_manager_->GetAuthenticatedAccountId(), scopes, this); } // diff --git a/components/signin/core/browser/mutable_profile_oauth2_token_service.cc b/components/signin/core/browser/mutable_profile_oauth2_token_service.cc index 768a75a..66c2693 100644 --- a/components/signin/core/browser/mutable_profile_oauth2_token_service.cc +++ b/components/signin/core/browser/mutable_profile_oauth2_token_service.cc @@ -9,6 +9,7 @@ #include "components/signin/core/browser/webdata/token_web_data.h" #include "components/webdata/common/web_data_service_base.h" #include "google_apis/gaia/gaia_auth_fetcher.h" +#include "google_apis/gaia/gaia_auth_util.h" #include "google_apis/gaia/gaia_constants.h" #include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/oauth2_access_token_fetcher_impl.h" @@ -157,6 +158,7 @@ MutableProfileOAuth2TokenService::CreateAccessTokenFetcher( const std::string& account_id, net::URLRequestContextGetter* getter, OAuth2AccessTokenConsumer* consumer) { + ValidateAccountId(account_id); std::string refresh_token = GetRefreshToken(account_id); DCHECK(!refresh_token.empty()); return new OAuth2AccessTokenFetcherImpl(consumer, getter, refresh_token); @@ -170,6 +172,7 @@ MutableProfileOAuth2TokenService::GetRequestContext() { void MutableProfileOAuth2TokenService::LoadCredentials( const std::string& primary_account_id) { DCHECK(!primary_account_id.empty()); + ValidateAccountId(primary_account_id); DCHECK(loading_primary_account_id_.empty()); DCHECK_EQ(0, web_data_service_request_); @@ -243,10 +246,16 @@ void MutableProfileOAuth2TokenService::LoadAllCredentialsIntoMemory( } else { DCHECK(!refresh_token.empty()); std::string account_id = RemoveAccountIdPrefix(prefixed_account_id); + + // If the account_id is an email address, then canonicalize it. This + // is to support legacy account_ids, and will not be needed after + // switching to gaia-ids. + if (account_id.find('@') != std::string::npos) + account_id = gaia::CanonicalizeEmail(account_id); + refresh_tokens()[account_id].reset( new AccountInfo(this, account_id, refresh_token)); FireRefreshTokenAvailable(account_id); - // TODO(fgorski): Notify diagnostic observers. } } @@ -263,6 +272,8 @@ void MutableProfileOAuth2TokenService::LoadAllCredentialsIntoMemory( void MutableProfileOAuth2TokenService::UpdateAuthError( const std::string& account_id, const GoogleServiceAuthError& error) { + ValidateAccountId(account_id); + // Do not report connection errors as these are not actually auth errors. // We also want to avoid masking a "real" auth error just because we // subsequently get a transient network error. @@ -295,6 +306,7 @@ void MutableProfileOAuth2TokenService::UpdateCredentials( DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(!account_id.empty()); DCHECK(!refresh_token.empty()); + ValidateAccountId(account_id); signin_metrics::LogSigninAddAccount(); @@ -329,6 +341,7 @@ void MutableProfileOAuth2TokenService::UpdateCredentials( void MutableProfileOAuth2TokenService::RevokeCredentials( const std::string& account_id) { + ValidateAccountId(account_id); DCHECK(thread_checker_.CalledOnValidThread()); if (refresh_tokens_.count(account_id) > 0) { diff --git a/components/signin/core/browser/mutable_profile_oauth2_token_service.h b/components/signin/core/browser/mutable_profile_oauth2_token_service.h index 0ee82b0..e7db6a2 100644 --- a/components/signin/core/browser/mutable_profile_oauth2_token_service.h +++ b/components/signin/core/browser/mutable_profile_oauth2_token_service.h @@ -99,6 +99,8 @@ class MutableProfileOAuth2TokenService : public ProfileOAuth2TokenService, PersistenceDBUpgrade); FRIEND_TEST_ALL_PREFIXES(MutableProfileOAuth2TokenServiceTest, PersistenceLoadCredentials); + FRIEND_TEST_ALL_PREFIXES(MutableProfileOAuth2TokenServiceTest, + CanonicalizeAccountId); // WebDataServiceConsumer implementation: virtual void OnWebDataServiceRequestDone( diff --git a/components/signin/core/browser/mutable_profile_oauth2_token_service_unittest.cc b/components/signin/core/browser/mutable_profile_oauth2_token_service_unittest.cc index 26b8447..b7e9a3a 100644 --- a/components/signin/core/browser/mutable_profile_oauth2_token_service_unittest.cc +++ b/components/signin/core/browser/mutable_profile_oauth2_token_service_unittest.cc @@ -368,3 +368,16 @@ TEST_F(MutableProfileOAuth2TokenServiceTest, FetchTransientError) { EXPECT_EQ(GoogleServiceAuthError::AuthErrorNone(), oauth2_service_.signin_error_controller()->auth_error()); } + +TEST_F(MutableProfileOAuth2TokenServiceTest, CanonicalizeAccountId) { + std::map<std::string, std::string> tokens; + tokens["AccountId-user@gmail.com"] = "refresh_token"; + tokens["AccountId-Foo.Bar@gmail.com"] = "refresh_token"; + tokens["AccountId-12345"] = "refresh_token"; + + oauth2_service_.LoadAllCredentialsIntoMemory(tokens); + + EXPECT_TRUE(oauth2_service_.RefreshTokenIsAvailable("user@gmail.com")); + EXPECT_TRUE(oauth2_service_.RefreshTokenIsAvailable("foobar@gmail.com")); + EXPECT_TRUE(oauth2_service_.RefreshTokenIsAvailable("12345")); +} diff --git a/components/signin/core/browser/profile_oauth2_token_service.cc b/components/signin/core/browser/profile_oauth2_token_service.cc index 757d96e..73c513e 100644 --- a/components/signin/core/browser/profile_oauth2_token_service.cc +++ b/components/signin/core/browser/profile_oauth2_token_service.cc @@ -8,7 +8,9 @@ #include "base/message_loop/message_loop.h" #include "base/stl_util.h" #include "base/time/time.h" +#include "components/signin/core/browser/signin_client.h" #include "components/signin/core/browser/signin_error_controller.h" +#include "google_apis/gaia/gaia_auth_util.h" #include "net/url_request/url_request_context_getter.h" ProfileOAuth2TokenService::ProfileOAuth2TokenService() @@ -43,6 +45,18 @@ void ProfileOAuth2TokenService::UpdateAuthError( NOTREACHED(); } +void ProfileOAuth2TokenService::ValidateAccountId( + const std::string& account_id) const { + DCHECK(!account_id.empty()); + + // If the account is given as an email, make sure its a canonical email. + // Note that some tests don't use email strings as account id, and after + // the gaia id migration it won't be an email. So only check for + // canonicalization if the account_id is suspected to be an email. + if (account_id.find('@') != std::string::npos) + DCHECK_EQ(gaia::CanonicalizeEmail(account_id), account_id); +} + std::vector<std::string> ProfileOAuth2TokenService::GetAccounts() { NOTREACHED(); return std::vector<std::string>(); @@ -62,4 +76,3 @@ void ProfileOAuth2TokenService::UpdateCredentials( void ProfileOAuth2TokenService::RevokeAllCredentials() { // Empty implementation by default. } - diff --git a/components/signin/core/browser/profile_oauth2_token_service.h b/components/signin/core/browser/profile_oauth2_token_service.h index b6f133f..fbc1fcc 100644 --- a/components/signin/core/browser/profile_oauth2_token_service.h +++ b/components/signin/core/browser/profile_oauth2_token_service.h @@ -93,6 +93,10 @@ class ProfileOAuth2TokenService : public OAuth2TokenService, const std::string& account_id, const GoogleServiceAuthError& error) override; + // Validate that the account_id argument is valid. This method DCHECKs + // when invalid. + void ValidateAccountId(const std::string& account_id) const; + private: // The client with which this instance was initialized, or NULL. SigninClient* client_; diff --git a/components/signin/core/browser/signin_manager.cc b/components/signin/core/browser/signin_manager.cc index acb32a9..8376f2e 100644 --- a/components/signin/core/browser/signin_manager.cc +++ b/components/signin/core/browser/signin_manager.cc @@ -83,9 +83,8 @@ void SigninManager::RemoveMergeSessionObserver( SigninManager::~SigninManager() {} void SigninManager::InitTokenService() { - const std::string& account_id = GetAuthenticatedUsername(); - if (token_service_ && !account_id.empty()) - token_service_->LoadCredentials(account_id); + if (token_service_ && IsAuthenticated()) + token_service_->LoadCredentials(GetAuthenticatedAccountId()); } std::string SigninManager::SigninTypeToString(SigninManager::SigninType type) { @@ -207,7 +206,7 @@ void SigninManager::SignOut( const base::Time signin_time = base::Time::FromInternalValue( client_->GetPrefs()->GetInt64(prefs::kSignedInTime)); - clear_authenticated_username(); + ClearAuthenticatedUsername(); client_->GetPrefs()->ClearPref(prefs::kGoogleServicesHostedDomain); client_->GetPrefs()->ClearPref(prefs::kGoogleServicesUsername); client_->GetPrefs()->ClearPref(prefs::kSignedInTime); @@ -358,12 +357,12 @@ void SigninManager::CompletePendingSignin() { DCHECK(!temp_refresh_token_.empty()); DCHECK(IsAuthenticated()); - token_service_->UpdateCredentials(GetAuthenticatedUsername(), - temp_refresh_token_); + std::string account_id = GetAuthenticatedAccountId(); + token_service_->UpdateCredentials(account_id, temp_refresh_token_); temp_refresh_token_.clear(); if (client_->ShouldMergeSigninCredentialsIntoCookieJar()) - merge_session_helper_->LogIn(GetAuthenticatedUsername()); + merge_session_helper_->LogIn(account_id); } void SigninManager::OnExternalSigninCompleted(const std::string& username) { diff --git a/components/signin/core/browser/signin_manager_base.cc b/components/signin/core/browser/signin_manager_base.cc index 8d6f309..8a47464 100644 --- a/components/signin/core/browser/signin_manager_base.cc +++ b/components/signin/core/browser/signin_manager_base.cc @@ -41,16 +41,8 @@ void SigninManagerBase::Initialize(PrefService* local_state) { std::string user = client_->GetPrefs()->GetString(prefs::kGoogleServicesUsername); - if (!user.empty()) { -#if defined(OS_IOS) - // Prior to M38, Chrome on iOS did not normalize the email before setting - // it in SigninManager. |AccountReconcilor| expects the authenticated email - // to be normalized as it used as an account identifier and is compared - // to the accounts available in the cookies. - user = gaia::CanonicalizeEmail(gaia::SanitizeEmail(user)); -#endif + if (!user.empty()) SetAuthenticatedUsername(user); - } } bool SigninManagerBase::IsInitialized() const { return initialized_; } @@ -64,7 +56,7 @@ const std::string& SigninManagerBase::GetAuthenticatedUsername() const { } const std::string& SigninManagerBase::GetAuthenticatedAccountId() const { - return GetAuthenticatedUsername(); + return authenticated_account_id_; } void SigninManagerBase::SetAuthenticatedUsername(const std::string& username) { @@ -91,6 +83,11 @@ void SigninManagerBase::SetAuthenticatedUsername(const std::string& username) { DCHECK(pref_username.empty() || gaia::AreEmailsSame(username, pref_username)) << "username: " << username << "; pref_username: " << pref_username; authenticated_username_ = username; + + // Some tests don't use a real email address for the username. To support + // these cases, don't try to canonicalize these strings. + authenticated_account_id_ = (username.find('@') == std::string::npos) ? + username : gaia::CanonicalizeEmail(username); client_->GetPrefs()->SetString(prefs::kGoogleServicesUsername, username); NotifyDiagnosticsObservers(USERNAME, username); @@ -100,12 +97,13 @@ void SigninManagerBase::SetAuthenticatedUsername(const std::string& username) { client_->GetPrefs()->SetString(prefs::kGoogleServicesLastUsername, username); } -void SigninManagerBase::clear_authenticated_username() { +void SigninManagerBase::ClearAuthenticatedUsername() { authenticated_username_.clear(); + authenticated_account_id_.clear(); } bool SigninManagerBase::IsAuthenticated() const { - return !GetAuthenticatedAccountId().empty(); + return !authenticated_account_id_.empty(); } bool SigninManagerBase::AuthInProgress() const { diff --git a/components/signin/core/browser/signin_manager_base.h b/components/signin/core/browser/signin_manager_base.h index b933148..11a9f98 100644 --- a/components/signin/core/browser/signin_manager_base.h +++ b/components/signin/core/browser/signin_manager_base.h @@ -125,7 +125,7 @@ class SigninManagerBase : public KeyedService { // Used by subclass to clear authenticated_username_ instead of using // SetAuthenticatedUsername, which enforces special preconditions due // to the fact that it is part of the public API and called by clients. - void clear_authenticated_username(); + void ClearAuthenticatedUsername(); // List of observers to notify on signin events. // Makes sure list is empty on destruction. @@ -146,8 +146,9 @@ class SigninManagerBase : public KeyedService { SigninClient* client_; bool initialized_; - // Actual username after successful authentication. + // Actual username and account_id after successful authentication. std::string authenticated_username_; + std::string authenticated_account_id_; // The list of SigninDiagnosticObservers. ObserverList<signin_internals_util::SigninDiagnosticsObserver, true> diff --git a/sync/android/java/src/org/chromium/sync/signin/AccountManagerHelper.java b/sync/android/java/src/org/chromium/sync/signin/AccountManagerHelper.java index 03df234..aa4c394 100644 --- a/sync/android/java/src/org/chromium/sync/signin/AccountManagerHelper.java +++ b/sync/android/java/src/org/chromium/sync/signin/AccountManagerHelper.java @@ -24,8 +24,10 @@ import org.chromium.net.NetworkChangeNotifier; import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.Locale; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.regex.Pattern; import javax.annotation.Nullable; @@ -38,6 +40,12 @@ public class AccountManagerHelper { private static final String TAG = "AccountManagerHelper"; + private static final Pattern AT_SYMBOL = Pattern.compile("@"); + + private static final String GMAIL_COM = "gmail.com"; + + private static final String GOOGLEMAIL_COM = "googlemail.com"; + public static final String GOOGLE_ACCOUNT_TYPE = "com.google"; private static final Object sLock = new Object(); @@ -124,13 +132,27 @@ public class AccountManagerHelper { return getGoogleAccounts().length > 0; } + private String canonicalizeName(String name) { + String[] parts = AT_SYMBOL.split(name); + if (parts.length != 2) return name; + + if (GOOGLEMAIL_COM.equalsIgnoreCase(parts[1])) { + parts[1] = GMAIL_COM; + } + if (GMAIL_COM.equalsIgnoreCase(parts[1])) { + parts[0] = parts[0].replace(".", ""); + } + return (parts[0] + "@" + parts[1]).toLowerCase(Locale.US); + } + /** * Returns the account if it exists, null otherwise. */ public Account getAccountFromName(String accountName) { - Account[] accounts = mAccountManager.getAccountsByType(GOOGLE_ACCOUNT_TYPE); + String canonicalName = canonicalizeName(accountName); + Account[] accounts = getGoogleAccounts(); for (Account account : accounts) { - if (account.name.equals(accountName)) { + if (canonicalizeName(account.name).equals(canonicalName)) { return account; } } diff --git a/sync/android/javatests/src/org/chromium/sync/notifier/signin/AccountManagerHelperTest.java b/sync/android/javatests/src/org/chromium/sync/notifier/signin/AccountManagerHelperTest.java new file mode 100644 index 0000000..8633fef --- /dev/null +++ b/sync/android/javatests/src/org/chromium/sync/notifier/signin/AccountManagerHelperTest.java @@ -0,0 +1,57 @@ +// Copyright 2012 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.sync.notifier.signin; + +import android.accounts.Account; +import android.content.Context; +import android.test.InstrumentationTestCase; +import android.test.suitebuilder.annotation.SmallTest; + +import org.chromium.sync.signin.AccountManagerHelper; +import org.chromium.sync.test.util.AccountHolder; +import org.chromium.sync.test.util.MockAccountManager; + +public class AccountManagerHelperTest extends InstrumentationTestCase { + + private MockAccountManager mAccountManager; + private AccountManagerHelper mHelper; + + @Override + protected void setUp() throws Exception { + super.setUp(); + + Context context = getInstrumentation().getContext(); + mAccountManager = new MockAccountManager(context, context); + AccountManagerHelper.overrideAccountManagerHelperForTests(context, mAccountManager); + mHelper = AccountManagerHelper.get(context); + } + + private Account addTestAccount(String accountName, String password) { + Account account = AccountManagerHelper.createAccountFromName(accountName); + AccountHolder.Builder accountHolder = + AccountHolder.create().account(account).password(password).alwaysAccept(true); + mAccountManager.addAccountHolderExplicitly(accountHolder.build()); + return account; + } + + @SmallTest + public void testCanonicalAccount() throws InterruptedException { + addTestAccount("test@gmail.com", "password"); + + assertTrue(mHelper.hasAccountForName("test@gmail.com")); + assertTrue(mHelper.hasAccountForName("Test@gmail.com")); + assertTrue(mHelper.hasAccountForName("te.st@gmail.com")); + } + + @SmallTest + public void testNonCanonicalAccount() throws InterruptedException { + addTestAccount("test.me@gmail.com", "password"); + + assertTrue(mHelper.hasAccountForName("test.me@gmail.com")); + assertTrue(mHelper.hasAccountForName("testme@gmail.com")); + assertTrue(mHelper.hasAccountForName("Testme@gmail.com")); + assertTrue(mHelper.hasAccountForName("te.st.me@gmail.com")); + } +} |