Commit 202917b8 authored by Yusuf Sengul's avatar Yusuf Sengul Committed by Commit Bot

SSO into any Chrome profile through GCPW

This change is a follow up to a previous Chrome SSO change[1]. Currently, Chrome SSO through GCPW would only work if the profile is the initial profile dir. That wouldn't work if initial profile dir is deleted and/or another profile is being used. This change addresses signing into profile through GCPW regardless of profile being initial profile.

Note that signing into profile doesn't work:

If the same GCPW user is authenticated in any other profile.
If any other profile was used to sign-in with GCPW before.
[1] https://chromium-review.googlesource.com/c/chromium/src/+/1984474

Bug: 1038368
Change-Id: I8719d1dbea59eb31206f9c897331fcd9674aae9b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2084721
Commit-Queue: Yusuf Sengul <yusufsn@google.com>
Reviewed-by: default avatarRakesh Soma <rakeshsoma@google.com>
Reviewed-by: default avatarMihai Sardarescu <msarda@chromium.org>
Reviewed-by: default avatarRoger Tawa <rogerta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756429}
parent b5bd6b36
......@@ -20,6 +20,7 @@
#include "components/prefs/pref_service.h"
#include "components/prefs/scoped_user_pref_update.h"
#include "components/profile_metrics/state.h"
#include "components/signin/public/base/signin_pref_names.h"
#include "components/signin/public/identity_manager/account_info.h"
#include "ui/base/resource/resource_bundle.h"
......@@ -376,6 +377,10 @@ bool ProfileAttributesEntry::IsAuthError() const {
return GetBool(kIsAuthErrorKey);
}
bool ProfileAttributesEntry::IsSignedInWithCredentialProvider() const {
return GetBool(prefs::kSignedInWithCredentialProvider);
}
size_t ProfileAttributesEntry::GetAvatarIconIndex() const {
std::string icon_url = GetString(kAvatarIconKey);
size_t icon_index = 0;
......@@ -469,6 +474,12 @@ void ProfileAttributesEntry::SetIsSigninRequired(bool value) {
LockForceSigninProfile(value);
}
void ProfileAttributesEntry::SetSignedInWithCredentialProvider(bool value) {
if (value != GetBool(prefs::kSignedInWithCredentialProvider)) {
SetBool(prefs::kSignedInWithCredentialProvider, value);
}
}
void ProfileAttributesEntry::LockForceSigninProfile(bool is_lock) {
DCHECK(is_force_signin_enabled_);
if (is_force_signin_profile_locked_ == is_lock)
......
......@@ -126,6 +126,8 @@ class ProfileAttributesEntry {
// Returns true if the profile is signed in but is in an authentication error
// state.
bool IsAuthError() const;
// Indicates that profile was signed in through native OS credential provider.
bool IsSignedInWithCredentialProvider() const;
// Returns the index of the default icon used by the profile.
size_t GetAvatarIconIndex() const;
// Returns the metrics bucket this profile should be recorded in.
......@@ -152,6 +154,7 @@ class ProfileAttributesEntry {
void SetGAIAPicture(const std::string& image_url_with_size, gfx::Image image);
void SetIsUsingGAIAPicture(bool value);
void SetIsSigninRequired(bool value);
void SetSignedInWithCredentialProvider(bool value);
void SetIsEphemeral(bool value);
void SetIsUsingDefaultName(bool value);
void SetIsUsingDefaultAvatar(bool value);
......
......@@ -1637,6 +1637,10 @@ void ProfileManager::AddProfileToStorage(Profile* profile) {
// The ProfileAttributesStorage's info must match the Identity Manager.
entry->SetAuthInfo(account_info.gaia, username,
is_consented_primary_account);
entry->SetSignedInWithCredentialProvider(profile->GetPrefs()->GetBoolean(
prefs::kSignedInWithCredentialProvider));
#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
// Sign out if force-sign-in policy is enabled and profile is not signed
// in.
......@@ -1687,13 +1691,16 @@ void ProfileManager::AddProfileToStorage(Profile* profile) {
username, is_consented_primary_account, icon_index,
supervised_user_id, account_id);
if (profile->GetPrefs()->GetBoolean(prefs::kForceEphemeralProfiles)) {
ProfileAttributesEntry* entry;
bool has_entry = storage.GetProfileAttributesWithPath(profile->GetPath(),
&entry);
bool has_entry =
storage.GetProfileAttributesWithPath(profile->GetPath(), &entry);
DCHECK(has_entry);
if (profile->GetPrefs()->GetBoolean(prefs::kForceEphemeralProfiles))
entry->SetIsEphemeral(true);
}
entry->SetSignedInWithCredentialProvider(
profile->GetPrefs()->GetBoolean(prefs::kSignedInWithCredentialProvider));
}
void ProfileManager::SetNonPersonalProfilePrefs(Profile* profile) {
......
......@@ -18,6 +18,8 @@
#include "chrome/browser/browser_process.h"
#include "chrome/browser/first_run/first_run.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_attributes_entry.h"
#include "chrome/browser/profiles/profile_attributes_storage.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/profiles/profile_window.h"
#include "chrome/browser/signin/about_signin_internals_factory.h"
......@@ -25,6 +27,7 @@
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/browser_list.h"
#include "chrome/browser/ui/webui/signin/dice_turn_sync_on_helper.h"
#include "chrome/browser/ui/webui/signin/signin_utils_desktop.h"
#include "chrome/credential_provider/common/gcp_strings.h"
#include "components/prefs/pref_service.h"
#include "components/signin/core/browser/about_signin_internals.h"
......@@ -148,17 +151,20 @@ void ImportCredentialsFromProvider(Profile* profile,
// true, importing refresh_token is allowed even if the profile has primary
// account for the user authenticated through credential provider.
void ExtractCredentialImportPreferences(
base::string16* cred_provider_user_gaia_id,
base::string16* cred_provider_gaia_id,
base::string16* cred_provider_email,
bool* allow_import_only_on_first_run,
bool* allow_import_when_primary_account_exists) {
DCHECK(cred_provider_user_gaia_id);
DCHECK(cred_provider_gaia_id);
DCHECK(cred_provider_email);
DCHECK(allow_import_only_on_first_run);
DCHECK(allow_import_when_primary_account_exists);
// Initialize to more restricted configuration.
*allow_import_only_on_first_run = true;
*allow_import_when_primary_account_exists = false;
cred_provider_user_gaia_id->clear();
cred_provider_gaia_id->clear();
cred_provider_email->clear();
base::win::RegKey key;
if (key.Open(HKEY_CURRENT_USER, credential_provider::kRegHkcuAccountsPath,
......@@ -174,9 +180,16 @@ void ExtractCredentialImportPreferences(
if (!key_account.Valid())
return;
base::string16 email;
if (key_account.ReadValue(
base::UTF8ToUTF16(credential_provider::kKeyEmail).c_str(), &email) !=
ERROR_SUCCESS) {
return;
}
// No need to return immediately if reading following registries fail. They
// will set to be stricter by default. cred_provider_user_gaia_id will be
// correctly set, though.
// will set to be stricter by default. cred_provider_gaia_id and
// cred_provider_email will be correctly set, though.
DWORD reg_import_only_on_first_run = 1;
key_account.ReadValueDW(credential_provider::kAllowImportOnlyOnFirstRun,
&reg_import_only_on_first_run);
......@@ -186,7 +199,8 @@ void ExtractCredentialImportPreferences(
credential_provider::kAllowImportWhenPrimaryAccountExists,
&reg_import_when_primary_account_exists);
*cred_provider_user_gaia_id = it.Name();
*cred_provider_gaia_id = it.Name();
*cred_provider_email = email;
*allow_import_only_on_first_run = (reg_import_only_on_first_run == 1);
*allow_import_when_primary_account_exists =
(reg_import_when_primary_account_exists == 1);
......@@ -263,43 +277,75 @@ void SetDiceTurnSyncOnHelperDelegateForTesting(
GetDiceTurnSyncOnHelperDelegateForTestingStorage()->swap(delegate);
}
// Credential provider needs to stick to profile it previously used to import
// credentials. Thus, if there is another profile that was previously signed in
// with credential provider regardless of whether user signed in or out,
// credential provider shouldn't attempt to import credentials into current
// profile.
bool IsGCPWUsedInOtherProfile(Profile* profile) {
DCHECK(profile);
ProfileManager* profile_manager = g_browser_process->profile_manager();
if (profile_manager) {
std::vector<ProfileAttributesEntry*> entries =
profile_manager->GetProfileAttributesStorage()
.GetAllProfilesAttributes();
for (const ProfileAttributesEntry* entry : entries) {
if (entry->GetPath() == profile->GetPath())
continue;
if (entry->IsSignedInWithCredentialProvider())
return true;
}
}
return false;
}
// Chrome doesn't allow signing into current profile if the same user is signed
// in another profile.
bool CanSignInToCurrentProfile(const std::string& gaia_id,
const std::string& email,
Profile* profile) {
std::string error_message;
return CanOfferSignin(profile, CAN_OFFER_SIGNIN_FOR_ALL_ACCOUNTS, gaia_id,
email, &error_message);
}
void SigninWithCredentialProviderIfPossible(Profile* profile) {
bool import_only_on_first_run = true;
bool import_when_primary_account_exists = false;
base::string16 cred_provider_user_gaia_id;
base::string16 cred_provider_gaia_id;
base::string16 cred_provider_email;
ExtractCredentialImportPreferences(&cred_provider_user_gaia_id,
&import_only_on_first_run,
ExtractCredentialImportPreferences(
&cred_provider_gaia_id, &cred_provider_email, &import_only_on_first_run,
&import_when_primary_account_exists);
if (cred_provider_user_gaia_id.empty())
if (cred_provider_gaia_id.empty() || cred_provider_email.empty())
return;
if (import_only_on_first_run && !first_run::IsChromeFirstRun())
if (!CanSignInToCurrentProfile(base::UTF16ToUTF8(cred_provider_gaia_id),
base::UTF16ToUTF8(cred_provider_email),
profile) ||
IsGCPWUsedInOtherProfile(profile)) {
return;
}
if (g_browser_process->profile_manager()->GetInitialProfileDir() !=
profile->GetPath().BaseName()) {
if (import_only_on_first_run && !first_run::IsChromeFirstRun())
return;
}
auto* identity_manager = IdentityManagerFactory::GetForProfile(profile);
base::string16 gaia_id;
if (identity_manager->HasPrimaryAccount()) {
base::string16 gaia_id =
base::UTF8ToUTF16(identity_manager->GetPrimaryAccountInfo().gaia);
gaia_id = base::UTF8ToUTF16(identity_manager->GetPrimaryAccountInfo().gaia);
if (!import_when_primary_account_exists ||
(gaia_id != cred_provider_user_gaia_id)) {
if (!import_when_primary_account_exists) {
return;
}
// If there is already a primary account in the profile, this means sync was
// turned on. So it shouldn't be turned on again.
TrySigninWithCredentialProvider(profile, gaia_id, false);
return;
}
TrySigninWithCredentialProvider(profile, base::string16(), true);
TrySigninWithCredentialProvider(profile, gaia_id, gaia_id.empty());
}
bool ReauthWithCredentialProviderIfPossible(Profile* profile) {
......
......@@ -15,7 +15,10 @@
#include "base/win/wincrypt_shim.h"
#include "build/build_config.h"
#include "chrome/browser/first_run/first_run.h"
#include "chrome/browser/profiles/profile_attributes_entry.h"
#include "chrome/browser/profiles/profile_attributes_storage.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/profiles/profile_window.h"
#include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/browser/signin/signin_util_win.h"
#include "chrome/browser/ui/browser_finder.h"
......@@ -79,8 +82,16 @@ struct SigninUtilWinBrowserTestParams {
};
void AssertSigninStarted(bool expect_is_started, Profile* profile) {
ASSERT_EQ(expect_is_started, profile->GetPrefs()->GetBoolean(
prefs::kSignedInWithCredentialProvider));
ProfileManager* profile_manager = g_browser_process->profile_manager();
ProfileAttributesStorage& storage =
profile_manager->GetProfileAttributesStorage();
ProfileAttributesEntry* entry;
ASSERT_TRUE(storage.GetProfileAttributesWithPath(profile->GetPath(), &entry));
ASSERT_EQ(expect_is_started, entry->IsSignedInWithCredentialProvider());
}
} // namespace
......@@ -143,6 +154,7 @@ class BrowserTestHelper {
base::ASCIIToUTF16(credential_provider::kKeyRefreshToken).c_str()));
}
public:
void SetSigninUtilRegistry() {
base::win::RegKey key;
CreateRegKey(&key);
......@@ -177,10 +189,17 @@ class BrowserTestHelper {
bool IsPreTest() {
std::string test_name =
::testing::UnitTest::GetInstance()->current_test_info()->name();
LOG(WARNING) << "test_name " << test_name;
LOG(INFO) << "PRE_ test_name " << test_name;
return test_name.find("PRE_") != std::string::npos;
}
bool IsPrePreTest() {
std::string test_name =
::testing::UnitTest::GetInstance()->current_test_info()->name();
LOG(INFO) << "PRE_PRE_ test_name " << test_name;
return test_name.find("PRE_PRE_") != std::string::npos;
}
private:
base::string16 gaia_id_;
base::string16 email_;
......@@ -526,3 +545,220 @@ INSTANTIATE_TEST_SUITE_P(AllowProfileWithPrimaryAccount_SameUser,
/*import_on_primary_account=*/1,
/*existing_email=*/L"foo@gmail.com",
/*expect_is_started=*/true)));
void UnblockOnProfileCreation(Profile::CreateStatus expected_final_status,
const base::Closure& quit_closure,
Profile* profile,
Profile::CreateStatus status) {
// If the status is CREATE_STATUS_CREATED, then the function will be called
// again with CREATE_STATUS_INITIALIZED.
if (status == Profile::CREATE_STATUS_CREATED)
return;
EXPECT_EQ(expected_final_status, status);
quit_closure.Run();
}
void UnblockOnProfileInitialized(const base::Closure& quit_closure,
Profile* profile,
Profile::CreateStatus status) {
UnblockOnProfileCreation(Profile::CREATE_STATUS_INITIALIZED, quit_closure,
profile, status);
}
void CreateAndSwitchToProfile(const std::string& basepath) {
ProfileManager* profile_manager = g_browser_process->profile_manager();
ASSERT_TRUE(profile_manager);
base::FilePath path = profile_manager->user_data_dir().AppendASCII(basepath);
base::RunLoop run_loop;
profile_manager->CreateProfileAsync(
path, base::Bind(&UnblockOnProfileInitialized, run_loop.QuitClosure()),
base::string16(), std::string());
// Run the message loop to allow profile creation to take place; the loop is
// terminated by UnblockOnProfileCreation when the profile is created.
run_loop.Run();
profiles::SwitchToProfile(path, false, ProfileManager::CreateCallback());
}
struct ExistingWinBrowserProfilesSigninUtilTestParams {
ExistingWinBrowserProfilesSigninUtilTestParams(
const base::string16& email_in_other_profile,
bool cred_provider_used_other_profile,
const base::string16& current_profile,
const base::string16& email_in_current_profile,
bool expect_is_started)
: email_in_other_profile(email_in_other_profile),
cred_provider_used_other_profile(cred_provider_used_other_profile),
current_profile(current_profile),
email_in_current_profile(email_in_current_profile),
expect_is_started(expect_is_started) {}
base::string16 email_in_other_profile;
bool cred_provider_used_other_profile;
base::string16 current_profile;
base::string16 email_in_current_profile;
bool expect_is_started;
};
class ExistingWinBrowserProfilesSigninUtilTest
: public BrowserTestHelper,
public InProcessBrowserTest,
public testing::WithParamInterface<
ExistingWinBrowserProfilesSigninUtilTestParams> {
public:
ExistingWinBrowserProfilesSigninUtilTest()
: BrowserTestHelper(L"gaia_id_for_foo_gmail.com",
L"foo@gmail.com",
"lst-123456",
0,
1) {}
protected:
bool SetUpUserDataDirectory() override {
registry_override_.OverrideRegistry(HKEY_CURRENT_USER);
signin_util::SetDiceTurnSyncOnHelperDelegateForTesting(
std::unique_ptr<DiceTurnSyncOnHelper::Delegate>(
new TestDiceTurnSyncOnHelperDelegate()));
if (!IsPreTest()) {
SetSigninUtilRegistry();
} else if (IsPrePreTest() && GetParam().cred_provider_used_other_profile) {
BrowserTestHelper(L"gaia_id_for_bar_gmail.com", L"bar@gmail.com",
"lst-123456", 0, 1)
.SetSigninUtilRegistry();
}
return InProcessBrowserTest::SetUpUserDataDirectory();
}
private:
registry_util::RegistryOverrideManager registry_override_;
};
// In PRE_PRE_Run, browser starts for the first time with the initial profile
// dir. If needed by the test, this step can set |email_in_other_profile| as the
// primary account in the profile or it can sign in with credential provider,
// but before this step ends, |current_profile| is created and browser switches
// to that profile just to prepare the browser for the next step.
IN_PROC_BROWSER_TEST_P(ExistingWinBrowserProfilesSigninUtilTest, PRE_PRE_Run) {
ProfileManager* profile_manager = g_browser_process->profile_manager();
Profile* profile =
profile_manager->GetLastUsedProfile(profile_manager->user_data_dir());
ASSERT_EQ(profile_manager->GetInitialProfileDir(),
profile->GetPath().BaseName());
auto* identity_manager = IdentityManagerFactory::GetForProfile(profile);
ASSERT_TRUE(identity_manager);
ASSERT_TRUE(identity_manager->HasPrimaryAccount() ==
GetParam().cred_provider_used_other_profile);
if (!GetParam().cred_provider_used_other_profile &&
!GetParam().email_in_other_profile.empty()) {
signin::MakePrimaryAccountAvailable(
identity_manager, base::UTF16ToUTF8(GetParam().email_in_other_profile));
ASSERT_TRUE(identity_manager->HasPrimaryAccount());
}
CreateAndSwitchToProfile(base::UTF16ToUTF8(GetParam().current_profile));
}
// Browser starts with the |current_profile| profile created in the previous
// step. If needed by the test, this step can set |email_in_current_profile| as
// the primary account in the profile.
IN_PROC_BROWSER_TEST_P(ExistingWinBrowserProfilesSigninUtilTest, PRE_Run) {
ProfileManager* profile_manager = g_browser_process->profile_manager();
Profile* profile =
profile_manager->GetLastUsedProfile(profile_manager->user_data_dir());
ASSERT_EQ(GetParam().current_profile, profile->GetPath().BaseName().value());
auto* identity_manager = IdentityManagerFactory::GetForProfile(profile);
ASSERT_TRUE(identity_manager);
ASSERT_FALSE(identity_manager->HasPrimaryAccount());
if (!GetParam().email_in_current_profile.empty()) {
signin::MakePrimaryAccountAvailable(
identity_manager,
base::UTF16ToUTF8(GetParam().email_in_current_profile));
ASSERT_TRUE(identity_manager->HasPrimaryAccount());
}
}
// Before this step runs, refresh token is written into fake registry. Browser
// starts with the |current_profile| profile. Depending on the test case,
// profile may have a primary account. Similarly the other profile(initial
// profile in this case) may have a primary account as well.
IN_PROC_BROWSER_TEST_P(ExistingWinBrowserProfilesSigninUtilTest, Run) {
ProfileManager* profile_manager = g_browser_process->profile_manager();
Profile* profile =
profile_manager->GetLastUsedProfile(profile_manager->user_data_dir());
ASSERT_EQ(GetParam().current_profile, profile->GetPath().BaseName().value());
AssertSigninStarted(GetParam().expect_is_started, profile);
}
INSTANTIATE_TEST_SUITE_P(
AllowCurrentProfile_NoUserSignedIn,
ExistingWinBrowserProfilesSigninUtilTest,
testing::Values(ExistingWinBrowserProfilesSigninUtilTestParams(
/*email_in_other_profile*/ L"",
/*cred_provider_used_other_profile*/ false,
/*current_profile*/ L"profile1",
/*email_in_current_profile=*/L"",
/*expect_is_started=*/true)));
INSTANTIATE_TEST_SUITE_P(
AllowCurrentProfile_SameUserSignedIn,
ExistingWinBrowserProfilesSigninUtilTest,
testing::Values(ExistingWinBrowserProfilesSigninUtilTestParams(
/*email_in_other_profile*/ L"",
/*cred_provider_used_other_profile*/ false,
/*current_profile*/ L"profile1",
/*email_in_current_profile=*/L"foo@gmail.com",
/*expect_is_started=*/true)));
INSTANTIATE_TEST_SUITE_P(
DisallowCurrentProfile_DifferentUserSignedIn,
ExistingWinBrowserProfilesSigninUtilTest,
testing::Values(ExistingWinBrowserProfilesSigninUtilTestParams(
/*email_in_other_profile*/ L"",
/*cred_provider_used_other_profile*/ false,
/*current_profile*/ L"profile1",
/*email_in_current_profile=*/L"bar@gmail.com",
/*expect_is_started=*/false)));
INSTANTIATE_TEST_SUITE_P(
DisallowCurrentProfile_SameUserSignedInDefaultProfile,
ExistingWinBrowserProfilesSigninUtilTest,
testing::Values(ExistingWinBrowserProfilesSigninUtilTestParams(
/*email_in_other_profile*/ L"foo@gmail.com",
/*cred_provider_used_other_profile*/ false,
/*current_profile*/ L"profile1",
/*email_in_current_profile=*/L"",
/*expect_is_started=*/false)));
INSTANTIATE_TEST_SUITE_P(
AllowCurrentProfile_DifferentUserSignedInDefaultProfile,
ExistingWinBrowserProfilesSigninUtilTest,
testing::Values(ExistingWinBrowserProfilesSigninUtilTestParams(
/*email_in_other_profile*/ L"bar@gmail.com",
/*cred_provider_used_other_profile*/ false,
/*current_profile*/ L"profile1",
/*email_in_current_profile=*/L"",
/*expect_is_started=*/true)));
INSTANTIATE_TEST_SUITE_P(
DisallowCurrentProfile_CredProviderUsedDefaultProfile,
ExistingWinBrowserProfilesSigninUtilTest,
testing::Values(ExistingWinBrowserProfilesSigninUtilTestParams(
/*email_in_other_profile*/ L"",
/*cred_provider_used_other_profile*/ true,
/*current_profile*/ L"profile1",
/*email_in_current_profile=*/L"",
/*expect_is_started=*/false)));
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