Commit dc94ecc8 authored by James Cook's avatar James Cook Committed by Commit Bot

Convert supervised user auth token fetch to unconsented primary account

SplitSettingsSync will allow the user to opt-out of browser sync.
However, IdentityAccessor::GetPrimaryAccountInfo() assumes the user has
consented to browser sync. The supervised user feature isn't coupled
to browser sync consent.

Switch to using the "unconsented" primary account. On Chrome OS this
account always exists for the logged-in user account, whether or not
the user consented to browser sync. On Android, it is always equivalent
to the primary account.

See go/cros-sync-mock and go/cros-primary-account for details.

Bug: 1042400
Test: updated existing unit tests
Test: add user and signin works with both parent and child accounts
  webui for parent controls works, webui for child account setup
  works

Change-Id: Ic45dd82bb3d5e3cbd53d32bc5b68abec677ba636
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2026176
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: default avatarAga Wronska <agawronska@chromium.org>
Reviewed-by: default avatarBoris Sazonov <bsazonov@chromium.org>
Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738328}
parent 56f7e59b
......@@ -99,9 +99,10 @@ void ChildAccountService::Init() {
// If we're already signed in, check the account immediately just to be sure.
// (We might have missed an update before registering as an observer.)
// "Unconsented" because this class doesn't care about browser sync consent.
base::Optional<AccountInfo> primary_account_info =
identity_manager_->FindExtendedAccountInfoForAccountWithRefreshToken(
identity_manager_->GetPrimaryAccountInfo());
identity_manager_->GetUnconsentedPrimaryAccountInfo());
if (primary_account_info.has_value())
OnExtendedAccountInfoUpdated(primary_account_info.value());
......@@ -258,7 +259,9 @@ void ChildAccountService::OnExtendedAccountInfoUpdated(
return;
}
CoreAccountId auth_account_id = identity_manager_->GetPrimaryAccountId();
// This class doesn't care about browser sync consent.
CoreAccountId auth_account_id =
identity_manager_->GetUnconsentedPrimaryAccountId();
if (info.account_id != auth_account_id)
return;
......@@ -267,7 +270,8 @@ void ChildAccountService::OnExtendedAccountInfoUpdated(
void ChildAccountService::OnExtendedAccountInfoRemoved(
const AccountInfo& info) {
if (info.account_id != identity_manager_->GetPrimaryAccountId())
// This class doesn't care about browser sync consent.
if (info.account_id != identity_manager_->GetUnconsentedPrimaryAccountId())
return;
SetIsChildAccount(false);
......
......@@ -14,6 +14,7 @@
#include "chrome/browser/supervised_user/child_accounts/kids_management_api.h"
#include "chrome/browser/supervised_user/supervised_user_constants.h"
#include "components/signin/public/identity_manager/access_token_info.h"
#include "components/signin/public/identity_manager/consent_level.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "components/signin/public/identity_manager/primary_account_access_token_fetcher.h"
#include "net/base/load_flags.h"
......@@ -86,7 +87,8 @@ FamilyInfoFetcher::FamilyInfoFetcher(
signin::IdentityManager* identity_manager,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
: consumer_(consumer),
primary_account_id_(identity_manager->GetPrimaryAccountId()),
// This feature doesn't care about browser sync consent.
primary_account_id_(identity_manager->GetUnconsentedPrimaryAccountId()),
identity_manager_(identity_manager),
url_loader_factory_(std::move(url_loader_factory)),
access_token_expired_(false) {}
......@@ -128,7 +130,9 @@ void FamilyInfoFetcher::StartFetchingAccessToken() {
"family_info_fetcher", identity_manager_, scopes,
base::BindOnce(&FamilyInfoFetcher::OnAccessTokenFetchComplete,
base::Unretained(this)),
signin::PrimaryAccountAccessTokenFetcher::Mode::kWaitUntilAvailable);
signin::PrimaryAccountAccessTokenFetcher::Mode::kWaitUntilAvailable,
// This feature doesn't care about browser sync consent.
signin::ConsentLevel::kNotRequired);
}
void FamilyInfoFetcher::OnAccessTokenFetchComplete(
......
......@@ -16,6 +16,8 @@
#include "base/test/task_environment.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/values.h"
#include "build/build_config.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "components/signin/public/identity_manager/identity_test_environment.h"
#include "net/base/net_errors.h"
#include "net/http/http_status_code.h"
......@@ -140,8 +142,27 @@ class FamilyInfoFetcherTest
fetcher_->StartGetFamilyMembers();
}
CoreAccountInfo SetPrimaryAccount() {
#if defined(OS_CHROMEOS)
return identity_test_env_.SetUnconsentedPrimaryAccount(kAccountId);
#elif defined(OS_ANDROID)
// TODO(https://crbug.com/1046746): Change to SetUnconsentedPrimaryAccount()
// when Android supports the concept of an unconsented primary account that
// is different than the primary account.
return identity_test_env_.SetPrimaryAccount(kAccountId);
#else
#error Unsupported platform.
#endif
}
void IssueRefreshToken() {
#if defined(OS_CHROMEOS)
identity_test_env_.MakeUnconsentedPrimaryAccountAvailable(kAccountId);
#elif defined(OS_ANDROID)
identity_test_env_.MakePrimaryAccountAvailable(kAccountId);
#else
#error Unsupported platform.
#endif
}
void IssueRefreshTokenForDifferentAccount() {
......@@ -150,7 +171,7 @@ class FamilyInfoFetcherTest
void WaitForAccessTokenRequestAndIssueToken() {
identity_test_env_.WaitForAccessTokenRequestIfNecessaryAndRespondWithToken(
identity_test_env_.identity_manager()->GetPrimaryAccountId(),
identity_test_env_.identity_manager()->GetUnconsentedPrimaryAccountId(),
"access_token", base::Time::Now() + base::TimeDelta::FromHours(1));
}
......@@ -248,7 +269,7 @@ TEST_F(FamilyInfoFetcherTest, SuccessAfterWaitingForRefreshToken) {
// account_id. We don't use IssueRefreshToken() as it also sets a refresh
// token for the primary account and that's something we don't want for this
// test.
identity_test_env_.SetPrimaryAccount(kAccountId);
CoreAccountInfo account_info = SetPrimaryAccount();
StartGetFamilyProfile();
// Since there is no refresh token yet, we should not get a request for an
......@@ -258,10 +279,10 @@ TEST_F(FamilyInfoFetcherTest, SuccessAfterWaitingForRefreshToken) {
identity_test_env_.SetCallbackForNextAccessTokenRequest(
access_token_requested.Get());
// In this case we don't directly call IssueRefreshToken() as it calls
// MakePrimaryAccountAvailable(). Since we already have a primary account set
// we cannot set another one without clearing it before.
identity_test_env_.SetRefreshTokenForPrimaryAccount();
// In this case we don't directly call IssueRefreshToken() as it sets the
// primary account. We already have a primary account set so we cannot set
// another one.
identity_test_env_.SetRefreshTokenForAccount(account_info.account_id);
// Do reset the callback for access token request before using the Wait* APIs.
identity_test_env_.SetCallbackForNextAccessTokenRequest(base::OnceClosure());
......@@ -277,7 +298,7 @@ TEST_F(FamilyInfoFetcherTest, NoRefreshToken) {
// retrieve the primary account_id from IdentityManager. We don't call
// IssueRefreshToken because we don't want it to precisely issue a refresh
// token for the primary account, just set it.
identity_test_env_.SetPrimaryAccount(kAccountId);
SetPrimaryAccount();
StartGetFamilyProfile();
IssueRefreshTokenForDifferentAccount();
......@@ -298,7 +319,7 @@ TEST_F(FamilyInfoFetcherTest, GetTokenFailure) {
// On failure to get an access token we expect a token error.
EXPECT_CALL(*this, OnFailure(FamilyInfoFetcher::TOKEN_ERROR));
identity_test_env_.WaitForAccessTokenRequestIfNecessaryAndRespondWithError(
identity_test_env_.identity_manager()->GetPrimaryAccountId(),
identity_test_env_.identity_manager()->GetUnconsentedPrimaryAccountId(),
GoogleServiceAuthError(GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS));
}
......
......@@ -19,6 +19,7 @@
#include "chrome/browser/supervised_user/supervised_user_constants.h"
#include "chrome/common/chrome_switches.h"
#include "components/signin/public/identity_manager/access_token_info.h"
#include "components/signin/public/identity_manager/consent_level.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "components/signin/public/identity_manager/primary_account_access_token_fetcher.h"
#include "content/public/browser/browser_context.h"
......@@ -33,6 +34,8 @@
#include "services/network/public/cpp/simple_url_loader.h"
#include "url/gurl.h"
namespace {
const char kPermissionRequestApiPath[] = "people/me/permissionRequests";
const char kPermissionRequestApiScope[] =
"https://www.googleapis.com/auth/kid.permission";
......@@ -52,6 +55,8 @@ const char kState[] = "PENDING";
const char kPermissionRequestKey[] = "permissionRequest";
const char kIdKey[] = "id";
} // namespace
struct PermissionRequestCreatorApiary::Request {
Request(const std::string& request_type,
const std::string& object_ref,
......@@ -153,7 +158,9 @@ void PermissionRequestCreatorApiary::StartFetching(Request* request) {
base::BindOnce(
&PermissionRequestCreatorApiary::OnAccessTokenFetchComplete,
base::Unretained(this), request),
signin::PrimaryAccountAccessTokenFetcher::Mode::kImmediate);
signin::PrimaryAccountAccessTokenFetcher::Mode::kImmediate,
// This class doesn't care about browser sync consent.
signin::ConsentLevel::kNotRequired);
}
void PermissionRequestCreatorApiary::OnAccessTokenFetchComplete(
......@@ -252,8 +259,9 @@ void PermissionRequestCreatorApiary::OnSimpleLoaderComplete(
request->access_token_expired = true;
identity::ScopeSet scopes;
scopes.insert(GetApiScope());
// "Unconsented" because this class doesn't care about browser sync consent.
identity_manager_->RemoveAccessTokenFromCache(
identity_manager_->GetPrimaryAccountId(), scopes,
identity_manager_->GetUnconsentedPrimaryAccountId(), scopes,
request->access_token);
StartFetching(request);
return;
......
......@@ -44,7 +44,7 @@ class PermissionRequestCreatorApiaryTest : public testing::Test {
base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
&test_url_loader_factory_)) {
AccountInfo account_info =
identity_test_env_.MakePrimaryAccountAvailable(kEmail);
identity_test_env_.MakeUnconsentedPrimaryAccountAvailable(kEmail);
account_id_ = account_info.account_id;
permission_creator_ = std::make_unique<PermissionRequestCreatorApiary>(
identity_test_env_.identity_manager(), test_shared_loader_factory_);
......
......@@ -16,6 +16,7 @@
#include "chrome/browser/supervised_user/supervised_user_constants.h"
#include "components/google/core/common/google_util.h"
#include "components/signin/public/identity_manager/access_token_info.h"
#include "components/signin/public/identity_manager/consent_level.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "components/signin/public/identity_manager/primary_account_access_token_fetcher.h"
#include "content/public/browser/browser_context.h"
......@@ -231,7 +232,9 @@ void KidsChromeManagementClient::StartFetching(
base::BindOnce(
&KidsChromeManagementClient::OnAccessTokenFetchComplete,
base::Unretained(this), it),
signin::PrimaryAccountAccessTokenFetcher::Mode::kImmediate);
signin::PrimaryAccountAccessTokenFetcher::Mode::kImmediate,
// This class doesn't care about browser sync consent.
signin::ConsentLevel::kNotRequired);
}
void KidsChromeManagementClient::OnAccessTokenFetchComplete(
......@@ -302,7 +305,8 @@ void KidsChromeManagementClient::OnSimpleLoaderComplete(
req->access_token_expired = true;
identity::ScopeSet scopes{req->scope};
identity_manager_->RemoveAccessTokenFromCache(
identity_manager_->GetPrimaryAccountId(), scopes, token_info.token);
identity_manager_->GetUnconsentedPrimaryAccountId(), scopes,
token_info.token);
StartFetching(it);
return;
}
......
......@@ -25,9 +25,10 @@ void CustodianProfileDownloaderService::Shutdown() {
void CustodianProfileDownloaderService::DownloadProfile(
const DownloadProfileCallback& callback) {
// The user must be logged in.
// The user must be logged in. "Unconsented" because this class doesn't care
// about browser sync consent.
if (!IdentityManagerFactory::GetForProfile(custodian_profile_)
->HasPrimaryAccount()) {
->HasUnconsentedPrimaryAccount()) {
return;
}
......
......@@ -135,7 +135,9 @@ SupervisedUserGoogleAuthNavigationThrottle::ShouldProceed() {
Profile* profile =
Profile::FromBrowserContext(web_contents->GetBrowserContext());
auto* identity_manager = IdentityManagerFactory::GetForProfile(profile);
CoreAccountInfo account_info = identity_manager->GetPrimaryAccountInfo();
// This class doesn't care about browser sync consent.
CoreAccountInfo account_info =
identity_manager->GetUnconsentedPrimaryAccountInfo();
ReauthenticateChildAccount(
web_contents, account_info.email,
base::Bind(&SupervisedUserGoogleAuthNavigationThrottle::
......
......@@ -228,7 +228,8 @@ TEST_F(SupervisedUserServiceTest, ShutDownCustodianProfileDownloader) {
// Emulate being logged in, then start to download a profile so a
// ProfileDownloader gets created.
identity_test_env()->MakePrimaryAccountAvailable("logged_in@gmail.com");
identity_test_env()->MakeUnconsentedPrimaryAccountAvailable(
"logged_in@gmail.com");
downloader_service->DownloadProfile(base::Bind(&OnProfileDownloadedFail));
}
......
......@@ -98,7 +98,8 @@ void AddSupervisionHandler::GetOAuthToken(GetOAuthTokenCallback callback) {
oauth2_access_token_fetcher_ =
identity_manager_->CreateAccessTokenFetcherForAccount(
identity_manager_->GetPrimaryAccountId(), "add_supervision", scopes,
identity_manager_->GetUnconsentedPrimaryAccountId(),
"add_supervision", scopes,
base::BindOnce(&AddSupervisionHandler::OnAccessTokenFetchComplete,
weak_ptr_factory_.GetWeakPtr(), std::move(callback)),
signin::AccessTokenFetcher::Mode::kImmediate);
......
......@@ -47,7 +47,8 @@ class AddSupervisionBrowserTest : public InProcessBrowserTest {
// TODO(danan): See if this is possible to do this instead using
// FakeGaia.IssueOAuthToken().
identity_test_env_ = std::make_unique<signin::IdentityTestEnvironment>();
identity_test_env_->MakePrimaryAccountAvailable("example@gmail.com");
identity_test_env_->MakeUnconsentedPrimaryAccountAvailable(
"example@gmail.com");
// This makes the identity manager return the string "access_token" for the
// access token.
identity_test_env_->SetAutomaticIssueOfAccessTokens(true);
......
......@@ -475,6 +475,9 @@ class IdentityManager : public KeyedService,
// These test helpers need to use some of the private methods below.
friend CoreAccountInfo SetPrimaryAccount(IdentityManager* identity_manager,
const std::string& email);
friend CoreAccountInfo SetUnconsentedPrimaryAccount(
IdentityManager* identity_manager,
const std::string& email);
friend void SetRefreshTokenForPrimaryAccount(
IdentityManager* identity_manager,
const std::string& token_value);
......
......@@ -252,6 +252,11 @@ CoreAccountInfo IdentityTestEnvironment::SetPrimaryAccount(
return signin::SetPrimaryAccount(identity_manager(), email);
}
CoreAccountInfo IdentityTestEnvironment::SetUnconsentedPrimaryAccount(
const std::string& email) {
return signin::SetUnconsentedPrimaryAccount(identity_manager(), email);
}
void IdentityTestEnvironment::SetRefreshTokenForPrimaryAccount() {
signin::SetRefreshTokenForPrimaryAccount(identity_manager());
}
......
......@@ -90,6 +90,10 @@ class IdentityTestEnvironment : public IdentityManager::DiagnosticsObserver {
// CoreAccountInfo of the newly-set account.
CoreAccountInfo SetPrimaryAccount(const std::string& email);
// As above, but adds an "unconsented" primary account. See ./README.md for
// the distinction between primary and unconsented primary accounts.
CoreAccountInfo SetUnconsentedPrimaryAccount(const std::string& email);
// Sets a refresh token for the primary account (which must already be set).
// Before updating the refresh token, blocks until refresh tokens are loaded.
// After updating the token, blocks until the update is processed by
......
......@@ -74,6 +74,21 @@ void UpdateRefreshTokenForAccount(
run_loop.Run();
}
// Ensures that an account for |email| exists in the AccountTrackerService,
// seeding it if necessary. Returns AccountInfo for the account.
AccountInfo EnsureAccountExists(AccountTrackerService* account_tracker_service,
const std::string& email) {
AccountInfo account_info =
account_tracker_service->FindAccountInfoByEmail(email);
if (account_info.account_id.empty()) {
std::string gaia_id = GetTestGaiaIdForEmail(email);
account_tracker_service->SeedAccountInfo(gaia_id, email);
account_info = account_tracker_service->FindAccountInfoByEmail(email);
DCHECK(!account_info.account_id.empty());
}
return account_info;
}
} // namespace
CoreAccountInfo SetPrimaryAccount(IdentityManager* identity_manager,
......@@ -83,18 +98,9 @@ CoreAccountInfo SetPrimaryAccount(IdentityManager* identity_manager,
identity_manager->GetPrimaryAccountManager();
DCHECK(!primary_account_manager->IsAuthenticated());
AccountTrackerService* account_tracker_service =
identity_manager->GetAccountTrackerService();
AccountInfo account_info =
account_tracker_service->FindAccountInfoByEmail(email);
if (account_info.account_id.empty()) {
std::string gaia_id = GetTestGaiaIdForEmail(email);
account_tracker_service->SeedAccountInfo(gaia_id, email);
account_info = account_tracker_service->FindAccountInfoByEmail(email);
}
std::string gaia_id = account_info.gaia;
DCHECK(!gaia_id.empty());
EnsureAccountExists(identity_manager->GetAccountTrackerService(), email);
DCHECK(!account_info.gaia.empty());
primary_account_manager->SignIn(email);
......@@ -103,6 +109,24 @@ CoreAccountInfo SetPrimaryAccount(IdentityManager* identity_manager,
return identity_manager->GetPrimaryAccountInfo();
}
CoreAccountInfo SetUnconsentedPrimaryAccount(IdentityManager* identity_manager,
const std::string& email) {
DCHECK(!identity_manager->HasUnconsentedPrimaryAccount());
AccountInfo account_info =
EnsureAccountExists(identity_manager->GetAccountTrackerService(), email);
DCHECK(!account_info.gaia.empty());
PrimaryAccountManager* primary_account_manager =
identity_manager->GetPrimaryAccountManager();
primary_account_manager->SetUnconsentedPrimaryAccountInfo(account_info);
DCHECK(identity_manager->HasUnconsentedPrimaryAccount());
DCHECK_EQ(account_info.gaia,
identity_manager->GetUnconsentedPrimaryAccountInfo().gaia);
return identity_manager->GetUnconsentedPrimaryAccountInfo();
}
void SetRefreshTokenForPrimaryAccount(IdentityManager* identity_manager,
const std::string& token_value) {
DCHECK(identity_manager->HasPrimaryAccount());
......
......@@ -53,6 +53,11 @@ class IdentityManager;
CoreAccountInfo SetPrimaryAccount(IdentityManager* identity_manager,
const std::string& email);
// As above, but adds an "unconsented" primary account. See ./README.md for
// the distinction between primary and unconsented primary accounts.
CoreAccountInfo SetUnconsentedPrimaryAccount(IdentityManager* identity_manager,
const std::string& email);
// Sets a refresh token for the primary account (which must already be set).
// Blocks until the refresh token is set. If |token_value| is empty a default
// value will be used instead.
......
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