Skip to content

Commit

Permalink
Remove about flag that enables ClientOAuth. This endpoint no longer w…
Browse files Browse the repository at this point in the history 
…orks

in chrome.

BUG=139137


Review URL: https://chromiumcodereview.appspot.com/10978002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@159294 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
rogerta@chromium.org committed Sep 28, 2012
1 parent f5f6af3 commit 55c566d
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 94 deletions.
7 changes: 0 additions & 7 deletions chrome/browser/about_flags.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -743,13 +743,6 @@ const Experiment kExperiments[] = {
SINGLE_VALUE_TYPE(switches::kEnableTouchpadThreeFingerClick)
},
#endif
{
"enable-client-oauth-signin",
IDS_FLAGS_ENABLE_CLIENT_OAUTH_SIGNIN_NAME,
IDS_FLAGS_ENABLE_CLIENT_OAUTH_SIGNIN_DESCRIPTION,
kOsMac | kOsWin | kOsLinux,
SINGLE_VALUE_TYPE(switches::kEnableClientOAuthSignin)
},
#if defined(USE_ASH)
{
"show-launcher-alignment-menu",
Expand Down
7 changes: 1 addition & 6 deletions chrome/browser/sync/profile_sync_service_harness.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,7 @@ bool ProfileSyncServiceHarness::SetupSync(
service_->SetSetupInProgress(true);

// Authenticate sync client using GAIA credentials.
if (CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableClientOAuthSignin)) {
service_->signin()->StartSignInWithOAuth(username_, password_);
} else {
service_->signin()->StartSignIn(username_, password_, "", "");
}
service_->signin()->StartSignIn(username_, password_, "", "");

// Wait for the OnBackendInitialized() callback.
if (!AwaitBackendInitialized()) {
Expand Down
45 changes: 10 additions & 35 deletions chrome/browser/ui/webui/sync_setup_handler.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,11 +185,6 @@ bool AreUserNamesEqual(const string16& user1, const string16& user2) {
return NormalizeUserName(user1) == NormalizeUserName(user2);
}

bool IsClientOAuthEnabled() {
return CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableClientOAuthSignin);
}

} // namespace

SyncSetupHandler::SyncSetupHandler(ProfileManager* profile_manager)
Expand Down Expand Up @@ -551,9 +546,9 @@ void SyncSetupHandler::DisplayGaiaLoginWithErrorMessage(
// then we don't want to show username and password fields if ClientOAuth is
// being used, since those fields are ignored by the endpoint on challenges.
if (error == GoogleServiceAuthError::TWO_FACTOR)
args.SetBoolean("askForOtp", IsClientOAuthEnabled());
args.SetBoolean("askForOtp", false);
else if (error == GoogleServiceAuthError::CAPTCHA_REQUIRED)
args.SetBoolean("hideEmailAndPassword", IsClientOAuthEnabled());
args.SetBoolean("hideEmailAndPassword", false);

args.SetBoolean("editableUser", editable_user);
if (!local_error_message.empty())
Expand Down Expand Up @@ -677,15 +672,6 @@ void SyncSetupHandler::HandleSubmitAuth(const ListValue* args) {
DCHECK(!otp.empty() || !captcha.empty() || !password.empty() ||
!access_code.empty());

if (IsClientOAuthEnabled()) {
// Last error is two-factor implies otp should not be empty.
DCHECK((last_signin_error_.state() != GoogleServiceAuthError::TWO_FACTOR) ||
!otp.empty());
// Last error is captcha-required implies captcha should not be empty.
DCHECK((last_signin_error_.state() !=
GoogleServiceAuthError::CAPTCHA_REQUIRED) || !captcha.empty());
}

const std::string& solution = captcha.empty() ?
(otp.empty() ? EmptyString() : otp) : captcha;
TryLogin(username, password, solution, access_code);
Expand All @@ -707,19 +693,12 @@ void SyncSetupHandler::TryLogin(const std::string& username,
last_signin_error_ = GoogleServiceAuthError::None();

SigninManager* signin = GetSignin();
if (IsClientOAuthEnabled()) {
if (!solution.empty()) {
signin->ProvideOAuthChallengeResponse(current_error.state(),
current_error.token(), solution);
return;
}
} else {
// If we're just being called to provide an ASP, then pass it to the
// SigninManager and wait for the next step.
if (!access_code.empty()) {
signin->ProvideSecondFactorAccessCode(access_code);
return;
}

// If we're just being called to provide an ASP, then pass it to the
// SigninManager and wait for the next step.
if (!access_code.empty()) {
signin->ProvideSecondFactorAccessCode(access_code);
return;
}

// The user has submitted credentials, which indicates they don't want to
Expand All @@ -728,12 +707,8 @@ void SyncSetupHandler::TryLogin(const std::string& username,
GetSyncService()->UnsuppressAndStart();

// Kick off a sign-in through the signin manager.
if (IsClientOAuthEnabled()) {
signin->StartSignInWithOAuth(username, password);
} else {
signin->StartSignIn(username, password, current_error.captcha().token,
solution);
}
signin->StartSignIn(username, password, current_error.captcha().token,
solution);
}

void SyncSetupHandler::GaiaCredentialsValid() {
Expand Down
69 changes: 28 additions & 41 deletions chrome/browser/ui/webui/sync_setup_handler_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,20 +351,10 @@ static ProfileKeyedService* BuildSigninManagerMock(Profile* profile) {

// The boolean parameter indicates whether the test is run with ClientOAuth
// or not.
class SyncSetupHandlerTest : public testing::TestWithParam<bool> {
class SyncSetupHandlerTest : public testing::Test {
public:
SyncSetupHandlerTest() : error_(GoogleServiceAuthError::NONE) {}
virtual void SetUp() OVERRIDE {
// If the parameter is true, then use ClientOAuth for the tests. Otherwise
// use ClientLogin for the tests.
if (GetParam()) {
CommandLine::ForCurrentProcess()->AppendSwitch(
switches::kEnableClientOAuthSignin);
} else {
ASSERT_FALSE(CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableClientOAuthSignin));
}

error_ = GoogleServiceAuthError::None();
profile_.reset(ProfileSyncServiceMock::MakeSignedInTestingProfile());
mock_pss_ = static_cast<ProfileSyncServiceMock*>(
Expand Down Expand Up @@ -428,10 +418,10 @@ class SyncSetupHandlerTest : public testing::TestWithParam<bool> {
scoped_ptr<TestingSyncSetupHandler> handler_;
};

TEST_P(SyncSetupHandlerTest, Basic) {
TEST_F(SyncSetupHandlerTest, Basic) {
}

TEST_P(SyncSetupHandlerTest, DisplayBasicLogin) {
TEST_F(SyncSetupHandlerTest, DisplayBasicLogin) {
EXPECT_CALL(*mock_pss_, IsSyncEnabledAndLoggedIn())
.WillRepeatedly(Return(false));
EXPECT_CALL(*mock_pss_, IsSyncTokenAvailable())
Expand Down Expand Up @@ -459,7 +449,7 @@ TEST_P(SyncSetupHandlerTest, DisplayBasicLogin) {
profile_.get())->current_login_ui());
}

TEST_P(SyncSetupHandlerTest, DisplayForceLogin) {
TEST_F(SyncSetupHandlerTest, DisplayForceLogin) {
EXPECT_CALL(*mock_pss_, IsSyncEnabledAndLoggedIn())
.WillRepeatedly(Return(false));
EXPECT_CALL(*mock_pss_, IsSyncTokenAvailable())
Expand Down Expand Up @@ -489,7 +479,7 @@ TEST_P(SyncSetupHandlerTest, DisplayForceLogin) {
profile_.get())->current_login_ui());
}

TEST_P(SyncSetupHandlerTest, DisplayConfigureWithBackendDisabledAndCancel) {
TEST_F(SyncSetupHandlerTest, DisplayConfigureWithBackendDisabledAndCancel) {
EXPECT_CALL(*mock_pss_, IsSyncEnabledAndLoggedIn())
.WillRepeatedly(Return(true));
EXPECT_CALL(*mock_pss_, IsSyncTokenAvailable())
Expand Down Expand Up @@ -517,7 +507,7 @@ TEST_P(SyncSetupHandlerTest, DisplayConfigureWithBackendDisabledAndCancel) {
profile_.get())->current_login_ui());
}

TEST_P(SyncSetupHandlerTest,
TEST_F(SyncSetupHandlerTest,
DisplayConfigureWithBackendDisabledAndSigninSuccess) {
EXPECT_CALL(*mock_pss_, IsSyncEnabledAndLoggedIn())
.WillRepeatedly(Return(true));
Expand Down Expand Up @@ -560,7 +550,7 @@ TEST_P(SyncSetupHandlerTest,
CheckBool(dictionary, "usePassphrase", false);
}

TEST_P(SyncSetupHandlerTest,
TEST_F(SyncSetupHandlerTest,
DisplayConfigureWithBackendDisabledAndSigninFalied) {
EXPECT_CALL(*mock_pss_, IsSyncEnabledAndLoggedIn())
.WillRepeatedly(Return(true));
Expand All @@ -587,7 +577,7 @@ TEST_P(SyncSetupHandlerTest,
profile_.get())->current_login_ui());
}

TEST_P(SyncSetupHandlerTest, HandleGaiaAuthFailure) {
TEST_F(SyncSetupHandlerTest, HandleGaiaAuthFailure) {
EXPECT_CALL(*mock_pss_, IsSyncEnabledAndLoggedIn())
.WillRepeatedly(Return(false));
EXPECT_CALL(*mock_pss_, IsSyncTokenAvailable())
Expand Down Expand Up @@ -619,7 +609,7 @@ TEST_P(SyncSetupHandlerTest, HandleGaiaAuthFailure) {
kTestUser, true, "");
}

TEST_P(SyncSetupHandlerTest, HandleCaptcha) {
TEST_F(SyncSetupHandlerTest, HandleCaptcha) {
EXPECT_CALL(*mock_pss_, IsSyncEnabledAndLoggedIn())
.WillRepeatedly(Return(false));
EXPECT_CALL(*mock_pss_, IsSyncTokenAvailable())
Expand Down Expand Up @@ -653,7 +643,7 @@ TEST_P(SyncSetupHandlerTest, HandleCaptcha) {
}

// TODO(kochi): We need equivalent tests for ChromeOS.
TEST_P(SyncSetupHandlerTest, UnrecoverableErrorInitializingSync) {
TEST_F(SyncSetupHandlerTest, UnrecoverableErrorInitializingSync) {
EXPECT_CALL(*mock_pss_, IsSyncEnabledAndLoggedIn())
.WillRepeatedly(Return(false));
EXPECT_CALL(*mock_pss_, IsSyncTokenAvailable())
Expand Down Expand Up @@ -692,7 +682,7 @@ TEST_P(SyncSetupHandlerTest, UnrecoverableErrorInitializingSync) {
kTestUser, true, "");
}

TEST_P(SyncSetupHandlerTest, GaiaErrorInitializingSync) {
TEST_F(SyncSetupHandlerTest, GaiaErrorInitializingSync) {
EXPECT_CALL(*mock_pss_, IsSyncEnabledAndLoggedIn())
.WillRepeatedly(Return(false));
EXPECT_CALL(*mock_pss_, IsSyncTokenAvailable())
Expand Down Expand Up @@ -732,7 +722,7 @@ TEST_P(SyncSetupHandlerTest, GaiaErrorInitializingSync) {
kTestUser, true, "");
}

TEST_P(SyncSetupHandlerTest, TestSyncEverything) {
TEST_F(SyncSetupHandlerTest, TestSyncEverything) {
std::string args = GetConfiguration(
NULL, SYNC_ALL_DATA, GetAllTypes(), "", ENCRYPT_PASSWORDS);
ListValue list_args;
Expand All @@ -750,7 +740,7 @@ TEST_P(SyncSetupHandlerTest, TestSyncEverything) {
ExpectDone();
}

TEST_P(SyncSetupHandlerTest, TurnOnEncryptAll) {
TEST_F(SyncSetupHandlerTest, TurnOnEncryptAll) {
std::string args = GetConfiguration(
NULL, SYNC_ALL_DATA, GetAllTypes(), "", ENCRYPT_ALL_DATA);
ListValue list_args;
Expand All @@ -769,7 +759,7 @@ TEST_P(SyncSetupHandlerTest, TurnOnEncryptAll) {
ExpectDone();
}

TEST_P(SyncSetupHandlerTest, TestPassphraseStillRequired) {
TEST_F(SyncSetupHandlerTest, TestPassphraseStillRequired) {
std::string args = GetConfiguration(
NULL, SYNC_ALL_DATA, GetAllTypes(), "", ENCRYPT_PASSWORDS);
ListValue list_args;
Expand All @@ -790,7 +780,7 @@ TEST_P(SyncSetupHandlerTest, TestPassphraseStillRequired) {
ExpectConfig();
}

TEST_P(SyncSetupHandlerTest, SuccessfullySetPassphrase) {
TEST_F(SyncSetupHandlerTest, SuccessfullySetPassphrase) {
DictionaryValue dict;
dict.SetBoolean("isGooglePassphrase", true);
std::string args = GetConfiguration(&dict,
Expand All @@ -817,7 +807,7 @@ TEST_P(SyncSetupHandlerTest, SuccessfullySetPassphrase) {
ExpectDone();
}

TEST_P(SyncSetupHandlerTest, SelectCustomEncryption) {
TEST_F(SyncSetupHandlerTest, SelectCustomEncryption) {
DictionaryValue dict;
dict.SetBoolean("isGooglePassphrase", false);
std::string args = GetConfiguration(&dict,
Expand All @@ -844,7 +834,7 @@ TEST_P(SyncSetupHandlerTest, SelectCustomEncryption) {
ExpectDone();
}

TEST_P(SyncSetupHandlerTest, UnsuccessfullySetPassphrase) {
TEST_F(SyncSetupHandlerTest, UnsuccessfullySetPassphrase) {
DictionaryValue dict;
dict.SetBoolean("isGooglePassphrase", true);
std::string args = GetConfiguration(&dict,
Expand Down Expand Up @@ -881,7 +871,7 @@ TEST_P(SyncSetupHandlerTest, UnsuccessfullySetPassphrase) {

// Walks through each user selectable type, and tries to sync just that single
// data type.
TEST_P(SyncSetupHandlerTest, TestSyncIndividualTypes) {
TEST_F(SyncSetupHandlerTest, TestSyncIndividualTypes) {
for (size_t i = 0; i < arraysize(kUserSelectableTypes); ++i) {
syncer::ModelTypeSet type_to_set;
type_to_set.Put(kUserSelectableTypes[i]);
Expand All @@ -904,7 +894,7 @@ TEST_P(SyncSetupHandlerTest, TestSyncIndividualTypes) {
}
}

TEST_P(SyncSetupHandlerTest, TestSyncAllManually) {
TEST_F(SyncSetupHandlerTest, TestSyncAllManually) {
std::string args = GetConfiguration(
NULL, CHOOSE_WHAT_TO_SYNC, GetAllTypes(), "", ENCRYPT_PASSWORDS);
ListValue list_args;
Expand All @@ -921,7 +911,7 @@ TEST_P(SyncSetupHandlerTest, TestSyncAllManually) {
ExpectDone();
}

TEST_P(SyncSetupHandlerTest, ShowSyncSetup) {
TEST_F(SyncSetupHandlerTest, ShowSyncSetup) {
EXPECT_CALL(*mock_pss_, IsPassphraseRequired())
.WillRepeatedly(Return(false));
EXPECT_CALL(*mock_pss_, IsUsingSecondaryPassphrase())
Expand All @@ -934,7 +924,7 @@ TEST_P(SyncSetupHandlerTest, ShowSyncSetup) {
ExpectConfig();
}

TEST_P(SyncSetupHandlerTest, ShowSyncSetupWithAuthError) {
TEST_F(SyncSetupHandlerTest, ShowSyncSetupWithAuthError) {
// Initialize the system to a signed in state, but with an auth error.
error_ = GoogleServiceAuthError(
GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS);
Expand Down Expand Up @@ -972,7 +962,7 @@ TEST_P(SyncSetupHandlerTest, ShowSyncSetupWithAuthError) {
"");
}

TEST_P(SyncSetupHandlerTest, ShowSetupSyncEverything) {
TEST_F(SyncSetupHandlerTest, ShowSetupSyncEverything) {
EXPECT_CALL(*mock_pss_, IsPassphraseRequired())
.WillRepeatedly(Return(false));
EXPECT_CALL(*mock_pss_, IsUsingSecondaryPassphrase())
Expand Down Expand Up @@ -1004,7 +994,7 @@ TEST_P(SyncSetupHandlerTest, ShowSetupSyncEverything) {
CheckConfigDataTypeArguments(dictionary, SYNC_ALL_DATA, GetAllTypes());
}

TEST_P(SyncSetupHandlerTest, ShowSetupManuallySyncAll) {
TEST_F(SyncSetupHandlerTest, ShowSetupManuallySyncAll) {
EXPECT_CALL(*mock_pss_, IsPassphraseRequired())
.WillRepeatedly(Return(false));
EXPECT_CALL(*mock_pss_, IsUsingSecondaryPassphrase())
Expand All @@ -1023,7 +1013,7 @@ TEST_P(SyncSetupHandlerTest, ShowSetupManuallySyncAll) {
CheckConfigDataTypeArguments(dictionary, CHOOSE_WHAT_TO_SYNC, GetAllTypes());
}

TEST_P(SyncSetupHandlerTest, ShowSetupSyncForAllTypesIndividually) {
TEST_F(SyncSetupHandlerTest, ShowSetupSyncForAllTypesIndividually) {
for (size_t i = 0; i < arraysize(kUserSelectableTypes); ++i) {
EXPECT_CALL(*mock_pss_, IsPassphraseRequired())
.WillRepeatedly(Return(false));
Expand Down Expand Up @@ -1055,7 +1045,7 @@ TEST_P(SyncSetupHandlerTest, ShowSetupSyncForAllTypesIndividually) {
}
}

TEST_P(SyncSetupHandlerTest, ShowSetupGaiaPassphraseRequired) {
TEST_F(SyncSetupHandlerTest, ShowSetupGaiaPassphraseRequired) {
EXPECT_CALL(*mock_pss_, IsPassphraseRequired())
.WillRepeatedly(Return(true));
EXPECT_CALL(*mock_pss_, IsUsingSecondaryPassphrase())
Expand All @@ -1075,7 +1065,7 @@ TEST_P(SyncSetupHandlerTest, ShowSetupGaiaPassphraseRequired) {
CheckBool(dictionary, "passphraseFailed", false);
}

TEST_P(SyncSetupHandlerTest, ShowSetupCustomPassphraseRequired) {
TEST_F(SyncSetupHandlerTest, ShowSetupCustomPassphraseRequired) {
EXPECT_CALL(*mock_pss_, IsPassphraseRequired())
.WillRepeatedly(Return(true));
EXPECT_CALL(*mock_pss_, IsUsingSecondaryPassphrase())
Expand All @@ -1095,7 +1085,7 @@ TEST_P(SyncSetupHandlerTest, ShowSetupCustomPassphraseRequired) {
CheckBool(dictionary, "passphraseFailed", false);
}

TEST_P(SyncSetupHandlerTest, ShowSetupEncryptAll) {
TEST_F(SyncSetupHandlerTest, ShowSetupEncryptAll) {
EXPECT_CALL(*mock_pss_, IsPassphraseRequired())
.WillRepeatedly(Return(false));
EXPECT_CALL(*mock_pss_, IsUsingSecondaryPassphrase())
Expand All @@ -1117,7 +1107,7 @@ TEST_P(SyncSetupHandlerTest, ShowSetupEncryptAll) {

// Tests that trying to log in with an invalid username results in an error
// displayed to the user.
TEST_P(SyncSetupHandlerTest, SubmitAuthWithInvalidUsername) {
TEST_F(SyncSetupHandlerTest, SubmitAuthWithInvalidUsername) {
EXPECT_CALL(*mock_signin_, IsAllowedUsername(_)).
WillRepeatedly(Return(false));

Expand Down Expand Up @@ -1156,6 +1146,3 @@ TEST_P(SyncSetupHandlerTest, SubmitAuthWithInvalidUsername) {
LoginUIServiceFactory::GetForProfile(
profile_.get())->current_login_ui());
}

INSTANTIATE_TEST_CASE_P(SyncSetupHandlerTest, SyncSetupHandlerTest,
Values(true, false));
4 changes: 0 additions & 4 deletions chrome/common/chrome_switches.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -477,10 +477,6 @@ const char kEnableBenchmarking[] = "enable-benchmarking";
// Enables the bundled PPAPI version of Flash.
const char kEnableBundledPpapiFlash[] = "enable-bundled-ppapi-flash";

// Enables the new ClientOAuth signin flow for connecting a profile a Google
// account. When disabled, Chrome will use the ClientLogin flow instead.
const char kEnableClientOAuthSignin[] = "enable-client-oauth-signin";

// Enables the new cloud policy stack.
const char kEnableCloudPolicyService[] = "enable-cloud-policy-service";

Expand Down
1 change: 0 additions & 1 deletion chrome/common/chrome_switches.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ extern const char kEnableAutofillFeedback[];
extern const char kEnableAutologin[];
extern const char kEnableBenchmarking[];
extern const char kEnableBundledPpapiFlash[];
extern const char kEnableClientOAuthSignin[];
extern const char kEnableCloudPolicyService[];
extern const char kEnableCloudPrintProxy[];
extern const char kEnableConnectBackupJobs[];
Expand Down

0 comments on commit 55c566d

Please sign in to comment.