diff options
11 files changed, 133 insertions, 17 deletions
diff --git a/chrome/browser/chromeos/login/login_utils.cc b/chrome/browser/chromeos/login/login_utils.cc index 1164450..18904b0 100644 --- a/chrome/browser/chromeos/login/login_utils.cc +++ b/chrome/browser/chromeos/login/login_utils.cc @@ -323,7 +323,8 @@ class LoginUtilsImpl : public LoginUtils, // GaiaOAuthConsumer overrides. virtual void OnGetOAuthTokenSuccess(const std::string& oauth_token) OVERRIDE; - virtual void OnGetOAuthTokenFailure() OVERRIDE; + virtual void OnGetOAuthTokenFailure( + const GoogleServiceAuthError& error) OVERRIDE; virtual void OnOAuthGetAccessTokenSuccess(const std::string& token, const std::string& secret) OVERRIDE; virtual void OnOAuthGetAccessTokenFailure( @@ -877,7 +878,8 @@ void LoginUtilsImpl::OnGetOAuthTokenSuccess(const std::string& oauth_token) { VLOG(1) << "Got OAuth request token!"; } -void LoginUtilsImpl::OnGetOAuthTokenFailure() { +void LoginUtilsImpl::OnGetOAuthTokenFailure( + const GoogleServiceAuthError& error) { // TODO(zelidrag): Pop up sync setup UI here? LOG(WARNING) << "Failed fetching OAuth request token"; } diff --git a/chrome/browser/net/gaia/gaia_oauth_consumer.h b/chrome/browser/net/gaia/gaia_oauth_consumer.h index ca3a7ab..a1b67d2 100644 --- a/chrome/browser/net/gaia/gaia_oauth_consumer.h +++ b/chrome/browser/net/gaia/gaia_oauth_consumer.h @@ -17,7 +17,7 @@ class GaiaOAuthConsumer { virtual ~GaiaOAuthConsumer() {} virtual void OnGetOAuthTokenSuccess(const std::string& oauth_token) {} - virtual void OnGetOAuthTokenFailure() {} + virtual void OnGetOAuthTokenFailure(const GoogleServiceAuthError& error) {} virtual void OnOAuthGetAccessTokenSuccess(const std::string& token, const std::string& secret) {} diff --git a/chrome/browser/net/gaia/gaia_oauth_fetcher.cc b/chrome/browser/net/gaia/gaia_oauth_fetcher.cc index 18ceeec..f9b46eb 100644 --- a/chrome/browser/net/gaia/gaia_oauth_fetcher.cc +++ b/chrome/browser/net/gaia/gaia_oauth_fetcher.cc @@ -512,7 +512,8 @@ namespace { void GaiaOAuthFetcher::OnBrowserClosing(Browser* browser, bool detail) { if (browser == popup_) { - consumer_->OnGetOAuthTokenFailure(); + consumer_->OnGetOAuthTokenFailure( + GoogleServiceAuthError(GoogleServiceAuthError::REQUEST_CANCELED)); } popup_ = NULL; } @@ -557,7 +558,8 @@ void GaiaOAuthFetcher::OnGetOAuthTokenUrlFetched( } } } else { - consumer_->OnGetOAuthTokenFailure(); + consumer_->OnGetOAuthTokenFailure( + GoogleServiceAuthError(GoogleServiceAuthError::SERVICE_UNAVAILABLE)); } } diff --git a/chrome/browser/net/gaia/gaia_oauth_fetcher_unittest.cc b/chrome/browser/net/gaia/gaia_oauth_fetcher_unittest.cc index 94a40b6..22fac88 100644 --- a/chrome/browser/net/gaia/gaia_oauth_fetcher_unittest.cc +++ b/chrome/browser/net/gaia/gaia_oauth_fetcher_unittest.cc @@ -57,7 +57,8 @@ class MockGaiaOAuthConsumer : public GaiaOAuthConsumer { ~MockGaiaOAuthConsumer() {} MOCK_METHOD1(OnGetOAuthTokenSuccess, void(const std::string& oauth_token)); - MOCK_METHOD0(OnGetOAuthTokenFailure, void()); + MOCK_METHOD1(OnGetOAuthTokenFailure, + void(const GoogleServiceAuthError& error)); MOCK_METHOD2(OnOAuthGetAccessTokenSuccess, void(const std::string& token, const std::string& secret)); diff --git a/chrome/browser/sync/signin_manager.cc b/chrome/browser/sync/signin_manager.cc index 66251d9..616a7c0 100644 --- a/chrome/browser/sync/signin_manager.cc +++ b/chrome/browser/sync/signin_manager.cc @@ -78,6 +78,7 @@ void SigninManager::SetUsername(const std::string& username) { // static void SigninManager::PrepareForSignin() { + DCHECK(!browser_sync::IsUsingOAuth()); DCHECK(username_.empty()); #if !defined(OS_CHROMEOS) // The Sign out should clear the token service credentials. @@ -89,6 +90,7 @@ void SigninManager::PrepareForSignin() { // static void SigninManager::PrepareForOAuthSignin() { + DCHECK(browser_sync::IsUsingOAuth()); DCHECK(oauth_username_.empty()); #if !defined(OS_CHROMEOS) // The Sign out should clear the token service credentials. @@ -100,6 +102,7 @@ void SigninManager::PrepareForOAuthSignin() { // Users must always sign out before they sign in again. void SigninManager::StartOAuthSignIn() { + DCHECK(browser_sync::IsUsingOAuth()); PrepareForOAuthSignin(); oauth_login_.reset(new GaiaOAuthFetcher(this, profile_->GetRequestContext(), @@ -114,6 +117,7 @@ void SigninManager::StartSignIn(const std::string& username, const std::string& password, const std::string& login_token, const std::string& login_captcha) { + DCHECK(!browser_sync::IsUsingOAuth()); PrepareForSignin(); username_.assign(username); password_.assign(password); @@ -142,6 +146,7 @@ void SigninManager::StartSignIn(const std::string& username, void SigninManager::ProvideSecondFactorAccessCode( const std::string& access_code) { + DCHECK(!browser_sync::IsUsingOAuth()); DCHECK(!username_.empty() && !password_.empty() && last_result_.data.empty()); @@ -255,9 +260,15 @@ void SigninManager::OnGetOAuthTokenSuccess(const std::string& oauth_token) { VLOG(1) << "SigninManager::SigninManager::OnGetOAuthTokenSuccess"; } -void SigninManager::OnGetOAuthTokenFailure() { +void SigninManager::OnGetOAuthTokenFailure( + const GoogleServiceAuthError& error) { DCHECK(browser_sync::IsUsingOAuth()); LOG(WARNING) << "SigninManager::OnGetOAuthTokenFailure"; + NotificationService::current()->Notify( + chrome::NOTIFICATION_GOOGLE_SIGNIN_FAILED, + Source<Profile>(profile_), + Details<const GoogleServiceAuthError>(&error)); + SignOut(); } void SigninManager::OnOAuthGetAccessTokenSuccess(const std::string& token, @@ -324,6 +335,7 @@ void SigninManager::Observe(int type, // If a GAIA service token has become available, use it to pre-login the // user to other services that depend on GAIA credentials. if (tok_details->service() == GaiaConstants::kGaiaService) { + DCHECK(!browser_sync::IsUsingOAuth()); if (client_login_.get() == NULL) { client_login_.reset(new GaiaAuthFetcher(this, GaiaConstants::kChromeSource, diff --git a/chrome/browser/sync/signin_manager.h b/chrome/browser/sync/signin_manager.h index ce6ba29..ac7b778 100644 --- a/chrome/browser/sync/signin_manager.h +++ b/chrome/browser/sync/signin_manager.h @@ -95,7 +95,8 @@ class SigninManager : public GaiaAuthConsumer, // GaiaOAuthConsumer virtual void OnGetOAuthTokenSuccess(const std::string& oauth_token) OVERRIDE; - virtual void OnGetOAuthTokenFailure() OVERRIDE; + virtual void OnGetOAuthTokenFailure( + const GoogleServiceAuthError& error) OVERRIDE; virtual void OnOAuthGetAccessTokenSuccess(const std::string& token, const std::string& secret) OVERRIDE; virtual void OnOAuthGetAccessTokenFailure( diff --git a/chrome/browser/sync/signin_manager_unittest.cc b/chrome/browser/sync/signin_manager_unittest.cc index 2df587e..7538498 100644 --- a/chrome/browser/sync/signin_manager_unittest.cc +++ b/chrome/browser/sync/signin_manager_unittest.cc @@ -7,6 +7,7 @@ #include "chrome/browser/net/gaia/token_service.h" #include "chrome/browser/net/gaia/token_service_unittest.h" #include "chrome/browser/password_manager/encryptor.h" +#include "chrome/browser/sync/util/oauth.h" #include "chrome/browser/webdata/web_data_service.h" #include "chrome/common/chrome_notification_types.h" #include "chrome/common/net/gaia/gaia_urls.h" @@ -30,7 +31,8 @@ class SigninManagerTest : public TokenServiceTestHarness { Source<Profile>(profile_.get())); } - void SimulateValidResponse() { + void SimulateValidResponseClientLogin() { + DCHECK(!browser_sync::IsUsingOAuth()); // Simulate the correct ClientLogin response. TestURLFetcher* fetcher = factory_.GetFetcherByID(0); DCHECK(fetcher); @@ -51,6 +53,18 @@ class SigninManagerTest : public TokenServiceTestHarness { "email=user@gmail.com"); } + void SimulateValidSigninOAuth() { + DCHECK(browser_sync::IsUsingOAuth()); + // Simulate a valid OAuth-based signin + manager_->OnGetOAuthTokenSuccess("oauth_token-Ev1Vu1hv"); + manager_->OnOAuthGetAccessTokenSuccess("oauth1_access_token-qOAmlrSM", + "secret-NKKn1DuR"); + manager_->OnOAuthWrapBridgeSuccess(browser_sync::SyncServiceName(), + "oauth2_wrap_access_token-R0Z3nRtw", + "3600"); + manager_->OnUserInfoSuccess("user-xZIuqTKu@gmail.com"); + } + TestURLFetcherFactory factory_; scoped_ptr<SigninManager> manager_; @@ -58,14 +72,17 @@ class SigninManagerTest : public TokenServiceTestHarness { TestNotificationTracker google_login_failure_; }; -TEST_F(SigninManagerTest, SignIn) { +// NOTE: ClientLogin's "StartSignin" is called after collecting credentials +// from the user. See also SigninManagerTest::SignInOAuth. +TEST_F(SigninManagerTest, SignInClientLogin) { + browser_sync::SetIsUsingOAuthForTest(false); manager_->Initialize(profile_.get()); EXPECT_TRUE(manager_->GetUsername().empty()); manager_->StartSignIn("username", "password", "", ""); EXPECT_FALSE(manager_->GetUsername().empty()); - SimulateValidResponse(); + SimulateValidResponseClientLogin(); // Should go into token service and stop. EXPECT_EQ(1U, google_login_success_.size()); @@ -77,10 +94,31 @@ TEST_F(SigninManagerTest, SignIn) { EXPECT_EQ("user@gmail.com", manager_->GetUsername()); } -TEST_F(SigninManagerTest, SignOut) { +// NOTE: OAuth's "StartOAuthSignIn" is called before collecting credentials +// from the user. See also SigninManagerTest::SignInClientLogin. +TEST_F(SigninManagerTest, SignInOAuth) { + browser_sync::SetIsUsingOAuthForTest(true); + manager_->Initialize(profile_.get()); + EXPECT_TRUE(manager_->GetUsername().empty()); + + SimulateValidSigninOAuth(); + EXPECT_FALSE(manager_->GetUsername().empty()); + + // Should go into token service and stop. + EXPECT_EQ(1U, google_login_success_.size()); + EXPECT_EQ(0U, google_login_failure_.size()); + + // Should persist across resets. + manager_.reset(new SigninManager()); + manager_->Initialize(profile_.get()); + EXPECT_EQ("user-xZIuqTKu@gmail.com", manager_->GetUsername()); +} + +TEST_F(SigninManagerTest, SignOutClientLogin) { + browser_sync::SetIsUsingOAuthForTest(false); manager_->Initialize(profile_.get()); manager_->StartSignIn("username", "password", "", ""); - SimulateValidResponse(); + SimulateValidResponseClientLogin(); manager_->OnClientLoginSuccess(credentials_); EXPECT_EQ("user@gmail.com", manager_->GetUsername()); @@ -92,7 +130,25 @@ TEST_F(SigninManagerTest, SignOut) { EXPECT_TRUE(manager_->GetUsername().empty()); } -TEST_F(SigninManagerTest, SignInFailure) { +TEST_F(SigninManagerTest, SignOutOAuth) { + browser_sync::SetIsUsingOAuthForTest(true); + manager_->Initialize(profile_.get()); + + SimulateValidSigninOAuth(); + EXPECT_FALSE(manager_->GetUsername().empty()); + + EXPECT_EQ("user-xZIuqTKu@gmail.com", manager_->GetUsername()); + manager_->SignOut(); + EXPECT_TRUE(manager_->GetUsername().empty()); + + // Should not be persisted anymore + manager_.reset(new SigninManager()); + manager_->Initialize(profile_.get()); + EXPECT_TRUE(manager_->GetUsername().empty()); +} + +TEST_F(SigninManagerTest, SignInFailureClientLogin) { + browser_sync::SetIsUsingOAuthForTest(false); manager_->Initialize(profile_.get()); manager_->StartSignIn("username", "password", "", ""); GoogleServiceAuthError error(GoogleServiceAuthError::REQUEST_CANCELED); @@ -109,7 +165,26 @@ TEST_F(SigninManagerTest, SignInFailure) { EXPECT_TRUE(manager_->GetUsername().empty()); } +TEST_F(SigninManagerTest, SignInFailureOAuth) { + browser_sync::SetIsUsingOAuthForTest(true); + manager_->Initialize(profile_.get()); + + GoogleServiceAuthError error(GoogleServiceAuthError::REQUEST_CANCELED); + manager_->OnGetOAuthTokenFailure(error); + + EXPECT_EQ(0U, google_login_success_.size()); + EXPECT_EQ(1U, google_login_failure_.size()); + + EXPECT_TRUE(manager_->GetUsername().empty()); + + // Should not be persisted + manager_.reset(new SigninManager()); + manager_->Initialize(profile_.get()); + EXPECT_TRUE(manager_->GetUsername().empty()); +} + TEST_F(SigninManagerTest, ProvideSecondFactorSuccess) { + browser_sync::SetIsUsingOAuthForTest(false); manager_->Initialize(profile_.get()); manager_->StartSignIn("username", "password", "", ""); GoogleServiceAuthError error(GoogleServiceAuthError::TWO_FACTOR); @@ -121,13 +196,14 @@ TEST_F(SigninManagerTest, ProvideSecondFactorSuccess) { EXPECT_FALSE(manager_->GetUsername().empty()); manager_->ProvideSecondFactorAccessCode("access"); - SimulateValidResponse(); + SimulateValidResponseClientLogin(); EXPECT_EQ(1U, google_login_success_.size()); EXPECT_EQ(1U, google_login_failure_.size()); } TEST_F(SigninManagerTest, ProvideSecondFactorFailure) { + browser_sync::SetIsUsingOAuthForTest(false); manager_->Initialize(profile_.get()); manager_->StartSignIn("username", "password", "", ""); GoogleServiceAuthError error1(GoogleServiceAuthError::TWO_FACTOR); @@ -157,6 +233,7 @@ TEST_F(SigninManagerTest, ProvideSecondFactorFailure) { } TEST_F(SigninManagerTest, SignOutMidConnect) { + browser_sync::SetIsUsingOAuthForTest(false); manager_->Initialize(profile_.get()); manager_->StartSignIn("username", "password", "", ""); manager_->SignOut(); diff --git a/chrome/browser/sync/util/oauth.cc b/chrome/browser/sync/util/oauth.cc index 6920834..ddd3dec 100644 --- a/chrome/browser/sync/util/oauth.cc +++ b/chrome/browser/sync/util/oauth.cc @@ -10,15 +10,28 @@ namespace browser_sync { +static bool has_local_override = false; +static bool local_override = false; + +// TODO(rickcam): Bug(92948): Remove IsUsingOAuth post-ClientLogin bool IsUsingOAuth() { + if (has_local_override) + return local_override; return CommandLine::ForCurrentProcess()->HasSwitch( switches::kEnableSyncOAuth); } +// TODO(rickcam): Bug(92948): Remove SyncServiceName post-ClientLogin const char* SyncServiceName() { return IsUsingOAuth() ? GaiaConstants::kSyncServiceOAuth : GaiaConstants::kSyncService; } +// TODO(rickcam): Bug(92948): Remove SetIsUsingOAuthForTest post-ClientLogin +void SetIsUsingOAuthForTest(bool is_using_oauth) { + has_local_override = true; + local_override = is_using_oauth; +} + } // namespace browser_sync diff --git a/chrome/browser/sync/util/oauth.h b/chrome/browser/sync/util/oauth.h index 815b4c4..43e2613 100644 --- a/chrome/browser/sync/util/oauth.h +++ b/chrome/browser/sync/util/oauth.h @@ -8,9 +8,15 @@ namespace browser_sync { +// TODO(rickcam): Bug(92948): Remove IsUsingOAuth after ClientLogin is gone bool IsUsingOAuth(); + +// TODO(rickcam): Bug(92948): Remove SyncServiceName post-ClientLogin const char* SyncServiceName(); +// TODO(rickcam): Bug(92948): Remove SetIsUsingOAuthForTest post-ClientLogin +void SetIsUsingOAuthForTest(bool is_using_oauth); + } // namespace browser_sync #endif // CHROME_BROWSER_SYNC_UTIL_OAUTH_H_ diff --git a/chrome/browser/ui/webui/chromeos/login/enterprise_oauth_enrollment_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/enterprise_oauth_enrollment_screen_handler.cc index cda117c..c40355f 100644 --- a/chrome/browser/ui/webui/chromeos/login/enterprise_oauth_enrollment_screen_handler.cc +++ b/chrome/browser/ui/webui/chromeos/login/enterprise_oauth_enrollment_screen_handler.cc @@ -177,7 +177,8 @@ void EnterpriseOAuthEnrollmentScreenHandler::GetLocalizedStrings( l10n_util::GetStringUTF16(IDS_ENTERPRISE_ENROLLMENT_WORKING)); } -void EnterpriseOAuthEnrollmentScreenHandler::OnGetOAuthTokenFailure() { +void EnterpriseOAuthEnrollmentScreenHandler::OnGetOAuthTokenFailure( + const GoogleServiceAuthError& error) { ResetAuth(); ShowFatalAuthError(); } diff --git a/chrome/browser/ui/webui/chromeos/login/enterprise_oauth_enrollment_screen_handler.h b/chrome/browser/ui/webui/chromeos/login/enterprise_oauth_enrollment_screen_handler.h index 7e23b2a..1eba110 100644 --- a/chrome/browser/ui/webui/chromeos/login/enterprise_oauth_enrollment_screen_handler.h +++ b/chrome/browser/ui/webui/chromeos/login/enterprise_oauth_enrollment_screen_handler.h @@ -51,7 +51,8 @@ class EnterpriseOAuthEnrollmentScreenHandler base::DictionaryValue* localized_strings) OVERRIDE; // Implements GaiaOAuthConsumer: - virtual void OnGetOAuthTokenFailure() OVERRIDE; + virtual void OnGetOAuthTokenFailure( + const GoogleServiceAuthError& error) OVERRIDE; virtual void OnOAuthGetAccessTokenFailure( const GoogleServiceAuthError& error) OVERRIDE; virtual void OnOAuthWrapBridgeSuccess(const std::string& service_scope, |