Commit d93823c3 authored by William Lin's avatar William Lin Committed by Commit Bot

Store CoreAccountInfo in ExtensionTokenKey

Skipping the account chooser will require the GetAuthToken function to
retrieve the gaia id of the account currently in use. Storing the
CoreAccountInfo will allow the function to always be able to retrieve
the gaia id for comparison in the scenario that CoreAccountId is an
email.

This CL replaces the CoreAccountId with CoreAccountInfo in the
ExtensionTokenKey. Additionally, relevant locations of the existing code
have been modified to use the CoreAccountId stored in CoreAccountInfo to
account for this replacement.

Bug: 1100535
Change-Id: Ie72f437aecdbfe6b98a1c5cf0caae3501f42baa7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2366033Reviewed-by: default avatarJohn Lee <johntlee@chromium.org>
Reviewed-by: default avatarAlex Ilin <alexilin@chromium.org>
Commit-Queue: William Lin <williamlin@google.com>
Cr-Commit-Position: refs/heads/master@{#800754}
parent 71a0edd4
......@@ -9,17 +9,17 @@
namespace extensions {
ExtensionTokenKey::ExtensionTokenKey(const std::string& extension_id,
const CoreAccountId& account_id,
const CoreAccountInfo& account_info,
const std::set<std::string>& scopes)
: extension_id(extension_id), account_id(account_id), scopes(scopes) {}
: extension_id(extension_id), account_info(account_info), scopes(scopes) {}
ExtensionTokenKey::ExtensionTokenKey(const ExtensionTokenKey& other) = default;
ExtensionTokenKey::~ExtensionTokenKey() {}
bool ExtensionTokenKey::operator<(const ExtensionTokenKey& rhs) const {
return std::tie(extension_id, account_id, scopes) <
std::tie(rhs.extension_id, rhs.account_id, rhs.scopes);
return std::tie(extension_id, account_info.account_id, scopes) <
std::tie(rhs.extension_id, rhs.account_info.account_id, rhs.scopes);
}
} // namespace extensions
......@@ -8,19 +8,19 @@
#include <set>
#include <string>
#include "google_apis/gaia/core_account_id.h"
#include "components/signin/public/identity_manager/account_info.h"
namespace extensions {
struct ExtensionTokenKey {
ExtensionTokenKey(const std::string& extension_id,
const CoreAccountId& account_id,
const CoreAccountInfo& account_info,
const std::set<std::string>& scopes);
ExtensionTokenKey(const ExtensionTokenKey& other);
~ExtensionTokenKey();
bool operator<(const ExtensionTokenKey& rhs) const;
std::string extension_id;
CoreAccountId account_id;
CoreAccountInfo account_info;
std::set<std::string> scopes;
};
......
......@@ -26,9 +26,20 @@ TEST(IdentityExtensionTokenKeyTest, Ordering) {
extension_ids.push_back(extension_id1);
extension_ids.push_back(extension_id2);
std::vector<CoreAccountId> user_ids;
user_ids.push_back(CoreAccountId("user_id_1"));
user_ids.push_back(CoreAccountId("user_id_2"));
std::vector<CoreAccountInfo> user_infos;
CoreAccountInfo user_1;
user_1.account_id = CoreAccountId("user_id_1");
user_1.gaia = "user_id_1";
user_1.email = "user_email_1";
CoreAccountInfo user_2;
user_2.account_id = CoreAccountId("user_id_2");
user_2.gaia = "user_id_2";
user_2.email = "user_email_2";
user_infos.push_back(user_1);
user_infos.push_back(user_2);
std::vector<std::set<std::string> > scopesets;
scopesets.push_back(scopes1);
......@@ -39,18 +50,10 @@ TEST(IdentityExtensionTokenKeyTest, Ordering) {
typedef std::vector<extensions::ExtensionTokenKey>::const_iterator
ExtensionTokenKeyIterator;
std::vector<std::string>::const_iterator extension_it;
std::vector<CoreAccountId>::const_iterator user_it;
std::vector<std::set<std::string> >::const_iterator scope_it;
for (extension_it = extension_ids.begin();
extension_it != extension_ids.end();
++extension_it) {
for (user_it = user_ids.begin(); user_it != user_ids.end(); ++user_it) {
for (scope_it = scopesets.begin(); scope_it != scopesets.end();
++scope_it) {
keys.push_back(
extensions::ExtensionTokenKey(*extension_it, *user_it, *scope_it));
for (const auto& extension_id : extension_ids) {
for (const auto& user_info : user_infos) {
for (const auto& scopes : scopesets) {
keys.emplace_back(extension_id, user_info, scopes);
}
}
}
......
......@@ -43,7 +43,7 @@ GaiaRemoteConsentFlow::GaiaRemoteConsentFlow(
const RemoteConsentResolutionData& resolution_data)
: delegate_(delegate),
profile_(profile),
account_id_(token_key.account_id),
account_id_(token_key.account_info.account_id),
resolution_data_(resolution_data),
web_flow_started_(false),
scoped_observer_(this) {}
......
......@@ -93,7 +93,12 @@ class IdentityGaiaRemoteConsentFlowTest : public testing::Test {
std::unique_ptr<TestGaiaRemoteConsentFlow> CreateTestFlow(
const std::string& window_key,
GaiaRemoteConsentFlow::Delegate* delegate) {
ExtensionTokenKey token_key("extension_id", CoreAccountId("account_id"),
CoreAccountInfo user_info;
user_info.account_id = CoreAccountId("account_id");
user_info.gaia = "account_id";
user_info.email = "email";
ExtensionTokenKey token_key("extension_id", user_info,
std::set<std::string>());
RemoteConsentResolutionData resolution_data;
resolution_data.url = GURL("https://example.com/auth/");
......
......@@ -31,10 +31,11 @@ GaiaWebAuthFlow::GaiaWebAuthFlow(Delegate* delegate,
const std::string& locale)
: delegate_(delegate),
profile_(profile),
account_id_(token_key->account_id) {
account_id_(token_key->account_info.account_id) {
TRACE_EVENT_NESTABLE_ASYNC_BEGIN2(
"identity", "GaiaWebAuthFlow", this, "extension_id",
token_key->extension_id, "account_id", token_key->account_id.ToString());
token_key->extension_id, "account_id",
token_key->account_info.account_id.ToString());
const char kOAuth2RedirectPathFormat[] = "/%s";
const char kOAuth2AuthorizeFormat[] =
......
......@@ -99,7 +99,12 @@ class IdentityGaiaWebAuthFlowTest : public testing::Test {
}
std::unique_ptr<TestGaiaWebAuthFlow> CreateTestFlow() {
ExtensionTokenKey token_key("extension_id", CoreAccountId("account_id"),
CoreAccountInfo user_info;
user_info.account_id = CoreAccountId("account_id");
user_info.gaia = "account_id";
user_info.email = "email";
ExtensionTokenKey token_key("extension_id", user_info,
std::set<std::string>());
return std::unique_ptr<TestGaiaWebAuthFlow>(new TestGaiaWebAuthFlow(
&delegate_, &token_key, "fake.client.id", ubertoken_error_state_));
......
......@@ -245,8 +245,7 @@ void IdentityGetAuthTokenFunction::FetchExtensionAccountInfo(
void IdentityGetAuthTokenFunction::OnReceivedExtensionAccountInfo(
const CoreAccountInfo* account_info) {
token_key_.account_id =
account_info ? account_info->account_id : CoreAccountId();
token_key_.account_info = account_info ? *account_info : CoreAccountInfo();
#if defined(OS_CHROMEOS)
policy::BrowserPolicyConnectorChromeOS* connector =
......@@ -362,9 +361,10 @@ bool IdentityGetAuthTokenFunction::ShouldStartSigninFlow() {
auto* identity_manager = IdentityManagerFactory::GetForProfile(GetProfile());
bool account_needs_reauth =
!identity_manager->HasAccountWithRefreshToken(token_key_.account_id) ||
!identity_manager->HasAccountWithRefreshToken(
token_key_.account_info.account_id) ||
identity_manager->HasAccountWithRefreshTokenInPersistentErrorState(
token_key_.account_id);
token_key_.account_info.account_id);
return account_needs_reauth;
}
......@@ -401,11 +401,12 @@ void IdentityGetAuthTokenFunction::StartSigninFlow() {
} else {
// Fixing an authentication error. Either there is no token, or it is in
// error.
DCHECK_EQ(token_key_.account_id, identity_manager->GetPrimaryAccountId());
DCHECK_EQ(token_key_.account_info.account_id,
identity_manager->GetPrimaryAccountId());
DCHECK(!identity_manager->HasAccountWithRefreshToken(
token_key_.account_id) ||
token_key_.account_info.account_id) ||
identity_manager->HasAccountWithRefreshTokenInPersistentErrorState(
token_key_.account_id));
token_key_.account_info.account_id));
}
}
scoped_identity_manager_observer_.Add(identity_manager);
......@@ -418,9 +419,9 @@ void IdentityGetAuthTokenFunction::StartMintTokenFlow(
IdentityMintRequestQueue::MintType type) {
#if !defined(OS_CHROMEOS)
// ChromeOS in kiosk mode may start the mint token flow without account.
DCHECK(!token_key_.account_id.empty());
DCHECK(!token_key_.account_info.IsEmpty());
DCHECK(IdentityManagerFactory::GetForProfile(GetProfile())
->HasAccountWithRefreshToken(token_key_.account_id));
->HasAccountWithRefreshToken(token_key_.account_info.account_id));
#endif
TRACE_EVENT_NESTABLE_ASYNC_BEGIN1("identity", "MintTokenFlow", this, "type",
type);
......@@ -656,10 +657,10 @@ void IdentityGetAuthTokenFunction::OnRefreshTokenUpdatedForAccount(
return;
// No specific account id was requested, use the first one we find.
if (token_key_.account_id.empty())
token_key_.account_id = account_info.account_id;
if (token_key_.account_info.IsEmpty())
token_key_.account_info = account_info;
if (token_key_.account_id == account_info.account_id) {
if (token_key_.account_info == account_info) {
// Stop listening tokens.
account_listening_mode_ = AccountListeningMode::kNotListening;
scoped_identity_manager_observer_.RemoveAll();
......@@ -689,8 +690,8 @@ void IdentityGetAuthTokenFunction::OnPrimaryAccountSet(
TRACE_EVENT_NESTABLE_ASYNC_INSTANT0("identity", "OnPrimaryAccountSet", this);
DCHECK(token_key_.account_id.empty());
token_key_.account_id = primary_account_info.account_id;
DCHECK(token_key_.account_info.IsEmpty());
token_key_.account_info = primary_account_info;
// Stop listening primary account.
DCHECK(IdentityManagerFactory::GetForProfile(GetProfile())
......@@ -846,7 +847,7 @@ void IdentityGetAuthTokenFunction::OnGaiaRemoteConsentFlowApproved(
// It's important to update the cache before calling CompleteMintTokenFlow()
// as this call may start a new request synchronously and query the cache.
ExtensionTokenKey new_token_key(token_key_);
new_token_key.account_id = account->account_id;
new_token_key.account_info = account.value();
id_api->token_cache()->SetToken(
new_token_key,
IdentityTokenCacheValue::CreateRemoteConsentApproved(consent_result));
......@@ -866,9 +867,9 @@ void IdentityGetAuthTokenFunction::OnGetAccessTokenComplete(
DCHECK(!device_access_token_request_);
DCHECK(!token_key_account_access_token_fetcher_);
if (access_token) {
TRACE_EVENT_NESTABLE_ASYNC_END1("identity", "GetAccessToken", this,
"account",
token_key_.account_id.ToString());
TRACE_EVENT_NESTABLE_ASYNC_END1(
"identity", "GetAccessToken", this, "account",
token_key_.account_info.account_id.ToString());
StartGaiaRequest(access_token.value());
} else {
......@@ -971,8 +972,9 @@ void IdentityGetAuthTokenFunction::StartTokenKeyAccountAccessTokenRequest() {
&app_client_secret)) {
token_key_account_access_token_fetcher_ =
identity_manager->CreateAccessTokenFetcherForClient(
token_key_.account_id, app_client_id, app_client_secret,
kExtensionsIdentityAPIOAuthConsumerName, signin::ScopeSet(),
token_key_.account_info.account_id, app_client_id,
app_client_secret, kExtensionsIdentityAPIOAuthConsumerName,
signin::ScopeSet(),
base::BindOnce(
&IdentityGetAuthTokenFunction::OnAccessTokenFetchCompleted,
base::Unretained(this)),
......@@ -984,8 +986,8 @@ void IdentityGetAuthTokenFunction::StartTokenKeyAccountAccessTokenRequest() {
token_key_account_access_token_fetcher_ =
identity_manager->CreateAccessTokenFetcherForAccount(
token_key_.account_id, kExtensionsIdentityAPIOAuthConsumerName,
signin::ScopeSet(),
token_key_.account_info.account_id,
kExtensionsIdentityAPIOAuthConsumerName, signin::ScopeSet(),
base::BindOnce(
&IdentityGetAuthTokenFunction::OnAccessTokenFetchCompleted,
base::Unretained(this)),
......@@ -1004,7 +1006,7 @@ void IdentityGetAuthTokenFunction::ShowExtensionLoginPrompt() {
base::Optional<AccountInfo> account =
IdentityManagerFactory::GetForProfile(GetProfile())
->FindExtendedAccountInfoForAccountWithRefreshTokenByAccountId(
token_key_.account_id);
token_key_.account_info.account_id);
std::string email_hint =
account ? account->email : email_for_default_web_account_;
......@@ -1047,7 +1049,8 @@ IdentityGetAuthTokenFunction::CreateMintTokenFlow() {
bool IdentityGetAuthTokenFunction::HasRefreshTokenForTokenKeyAccount() const {
auto* identity_manager = IdentityManagerFactory::GetForProfile(GetProfile());
return identity_manager->HasAccountWithRefreshToken(token_key_.account_id);
return identity_manager->HasAccountWithRefreshToken(
token_key_.account_info.account_id);
}
std::string IdentityGetAuthTokenFunction::GetOAuth2ClientId() const {
......
......@@ -232,7 +232,7 @@ class IdentityGetAuthTokenFunction : public ExtensionFunction,
std::string email_for_default_web_account_;
ExtensionTokenKey token_key_{/*extension_id=*/"",
/*account_id=*/CoreAccountId(),
/*account_info=*/CoreAccountInfo(),
/*scopes=*/{}};
std::string oauth2_client_id_;
// When launched in interactive mode, and if there is no existing grant,
......
......@@ -22,8 +22,13 @@ class MockRequest : public extensions::IdentityMintRequestQueue::Request {
std::unique_ptr<ExtensionTokenKey> ExtensionIdToKey(
const std::string& extension_id) {
return std::unique_ptr<ExtensionTokenKey>(new ExtensionTokenKey(
extension_id, CoreAccountId("user_id"), std::set<std::string>()));
CoreAccountInfo user_info;
user_info.account_id = CoreAccountId("user_id");
user_info.gaia = "user_id";
user_info.email = "user_email";
return std::make_unique<ExtensionTokenKey>(extension_id, user_info,
std::set<std::string>());
}
} // namespace
......
......@@ -118,7 +118,7 @@ const std::set<std::string>& IdentityTokenCacheValue::granted_scopes() const {
IdentityTokenCache::AccessTokensKey::AccessTokensKey(
const ExtensionTokenKey& key)
: extension_id(key.extension_id), account_id(key.account_id) {}
: extension_id(key.extension_id), account_id(key.account_info.account_id) {}
IdentityTokenCache::AccessTokensKey::AccessTokensKey(
const std::string& extension_id,
......
......@@ -22,7 +22,7 @@ class IdentityTokenCacheTest : public testing::Test {
void SetAccessToken(const std::string& ext_id,
const std::string& token_string,
const std::set<std::string>& scopes) {
ExtensionTokenKey key(ext_id, CoreAccountId(), scopes);
ExtensionTokenKey key(ext_id, CoreAccountInfo(), scopes);
IdentityTokenCacheValue token = IdentityTokenCacheValue::CreateToken(
token_string, scopes, base::TimeDelta::FromSeconds(3600));
cache_.SetToken(key, token);
......@@ -31,7 +31,7 @@ class IdentityTokenCacheTest : public testing::Test {
void SetRemoteConsentApprovedToken(const std::string& ext_id,
const std::string& consent_result,
const std::set<std::string>& scopes) {
ExtensionTokenKey key(ext_id, CoreAccountId(), scopes);
ExtensionTokenKey key(ext_id, CoreAccountInfo(), scopes);
IdentityTokenCacheValue token =
IdentityTokenCacheValue::CreateRemoteConsentApproved(consent_result);
cache_.SetToken(key, token);
......@@ -39,7 +39,7 @@ class IdentityTokenCacheTest : public testing::Test {
const IdentityTokenCacheValue& GetToken(const std::string& ext_id,
const std::set<std::string>& scopes) {
ExtensionTokenKey key(ext_id, CoreAccountId(), scopes);
ExtensionTokenKey key(ext_id, CoreAccountInfo(), scopes);
return cache_.GetToken(key);
}
......
......@@ -55,8 +55,13 @@ void IdentityInternalsUIBrowserTest::AddTokenToCache(
extensions::IdentityTokenCacheValue token_cache_value =
extensions::IdentityTokenCacheValue::CreateToken(
token_id, scopes_set, base::TimeDelta::FromSeconds(time_to_live));
extensions::ExtensionTokenKey key(extension_id, CoreAccountId(account_id),
scopes_set);
CoreAccountInfo user_info;
user_info.account_id = CoreAccountId(account_id);
user_info.gaia = account_id;
user_info.email = "user_email_" + account_id;
extensions::ExtensionTokenKey key(extension_id, user_info, scopes_set);
extensions::IdentityAPI::GetFactoryInstance()
->Get(browser()->profile())
->token_cache()
......
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