Add dummy helper tokens in crOS Account Manager

Add a couple of dummy token values in Chrome OS Account Manager for
dealing with Active Directory accounts and Gaia accounts with unknown
tokens:

  - AccountManager::kActiveDirectoryDummyToken
  - AccountManager::kInvalidToken

Bug: 820046
Bug: 904128
Test: chromeos_unittests --gtest_filter="*AccountManager*Test*"
Test: unit_tests --gtest_filter="*CrOSOAuthDelegateTest*"
Change-Id: I72ae94ef5013a61d20bac2e17bfac88ed1deb8a7
Reviewed-on: https://chromium-review.googlesource.com/c/1349973
Commit-Queue: Kush Sinha <sinhak@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611115}

chrome/browser/chromeos/oauth2_token_service_delegate_unittest.cc

@@ -35,6 +35,10 @@ namespace {
 using account_manager::AccountType::ACCOUNT_TYPE_GAIA;
 using account_manager::AccountType::ACCOUNT_TYPE_ACTIVE_DIRECTORY;
 
+constexpr char kGaiaId[] = "gaia-id";
+constexpr char kGaiaToken[] = "gaia-token";
+constexpr char kUserEmail[] = "user@gmail.com";
+
 class AccessTokenConsumer : public OAuth2AccessTokenConsumer {
  public:
   AccessTokenConsumer() = default;
@@ -159,9 +163,9 @@ class CrOSOAuthDelegateTest : public testing::Test {
 
     account_tracker_service_.Initialize(&pref_service_, base::FilePath());
 
-    account_info_ = CreateAccountInfoTestFixture("111" /* gaia_id */,
-                                                 "user@gmail.com" /* email */);
+    account_info_ = CreateAccountInfoTestFixture(kGaiaId, kUserEmail);
     account_tracker_service_.SeedAccountInfo(account_info_);
+    gaia_account_key_ = {account_info_.gaia, ACCOUNT_TYPE_GAIA};
 
     delegate_ = std::make_unique<ChromeOSOAuth2TokenServiceDelegate>(
         &account_tracker_service_, &account_manager_,
@@ -209,6 +213,7 @@ class CrOSOAuthDelegateTest : public testing::Test {
 
   base::ScopedTempDir tmp_dir_;
   AccountInfo account_info_;
+  AccountManager::AccountKey gaia_account_key_;
   AccountTrackerService account_tracker_service_;
   AccountManager account_manager_;
   SigninErrorController signin_error_controller_;
@@ -231,9 +236,7 @@ TEST_F(CrOSOAuthDelegateTest, RefreshTokenIsAvailableForGaiaAccounts) {
 
   EXPECT_FALSE(delegate_->RefreshTokenIsAvailable(account_info_.account_id));
 
-  const AccountManager::AccountKey account_key{account_info_.gaia,
-                                               ACCOUNT_TYPE_GAIA};
-  account_manager_.UpsertToken(account_key, "token");
+  account_manager_.UpsertToken(gaia_account_key_, kGaiaToken);
 
   EXPECT_TRUE(delegate_->RefreshTokenIsAvailable(account_info_.account_id));
 }
@@ -255,7 +258,7 @@ TEST_F(CrOSOAuthDelegateTest, ObserversAreNotifiedOnAuthErrorChange) {
 TEST_F(CrOSOAuthDelegateTest, ObserversAreNotifiedOnCredentialsInsertion) {
   TokenServiceObserver observer;
   delegate_->AddObserver(&observer);
-  delegate_->UpdateCredentials(account_info_.account_id, "123");
+  delegate_->UpdateCredentials(account_info_.account_id, kGaiaToken);
 
   EXPECT_EQ(1UL, observer.account_ids_.size());
   EXPECT_EQ(account_info_.account_id, *observer.account_ids_.begin());
@@ -268,7 +271,7 @@ TEST_F(CrOSOAuthDelegateTest, ObserversAreNotifiedOnCredentialsInsertion) {
 TEST_F(CrOSOAuthDelegateTest, ObserversAreNotifiedOnCredentialsUpdate) {
   TokenServiceObserver observer;
   delegate_->AddObserver(&observer);
-  delegate_->UpdateCredentials(account_info_.account_id, "123");
+  delegate_->UpdateCredentials(account_info_.account_id, kGaiaToken);
 
   EXPECT_EQ(1UL, observer.account_ids_.size());
   EXPECT_EQ(account_info_.account_id, *observer.account_ids_.begin());
@@ -281,13 +284,12 @@ TEST_F(CrOSOAuthDelegateTest, ObserversAreNotifiedOnCredentialsUpdate) {
 TEST_F(CrOSOAuthDelegateTest,
        ObserversAreNotNotifiedIfCredentialsAreNotUpdated) {
   TokenServiceObserver observer;
-  const std::string kToken = "123";
   delegate_->AddObserver(&observer);
 
-  delegate_->UpdateCredentials(account_info_.account_id, kToken);
+  delegate_->UpdateCredentials(account_info_.account_id, kGaiaToken);
   observer.account_ids_.clear();
   observer.last_err_account_id_ = std::string();
-  delegate_->UpdateCredentials(account_info_.account_id, kToken);
+  delegate_->UpdateCredentials(account_info_.account_id, kGaiaToken);
 
   EXPECT_TRUE(observer.account_ids_.empty());
   EXPECT_EQ(std::string(), observer.last_err_account_id_);
@@ -299,7 +301,7 @@ TEST_F(CrOSOAuthDelegateTest,
        BatchChangeObserversAreNotifiedOnCredentialsUpdate) {
   TokenServiceObserver observer;
   delegate_->AddObserver(&observer);
-  delegate_->UpdateCredentials(account_info_.account_id, "123");
+  delegate_->UpdateCredentials(account_info_.account_id, kGaiaToken);
 
   EXPECT_EQ(1UL, observer.batch_change_records_.size());
   EXPECT_EQ(1UL, observer.batch_change_records_[0].size());
@@ -365,9 +367,10 @@ TEST_F(CrOSOAuthDelegateTest, GetAccountsShouldNotReturnAdAccounts) {
   EXPECT_TRUE(delegate_->GetAccounts().empty());
 
   // Insert an Active Directory account into AccountManager.
-  AccountManager::AccountKey ad_account_key{"111",
+  AccountManager::AccountKey ad_account_key{"object-guid",
                                             ACCOUNT_TYPE_ACTIVE_DIRECTORY};
-  account_manager_.UpsertToken(ad_account_key, "" /* token */);
+  account_manager_.UpsertToken(ad_account_key,
+                               AccountManager::kActiveDirectoryDummyToken);
 
   // OAuth delegate should not return Active Directory accounts.
   EXPECT_TRUE(delegate_->GetAccounts().empty());
@@ -376,8 +379,7 @@ TEST_F(CrOSOAuthDelegateTest, GetAccountsShouldNotReturnAdAccounts) {
 TEST_F(CrOSOAuthDelegateTest, GetAccountsReturnsGaiaAccounts) {
   EXPECT_TRUE(delegate_->GetAccounts().empty());
 
-  AccountManager::AccountKey gaia_account_key{"111", ACCOUNT_TYPE_GAIA};
-  account_manager_.UpsertToken(gaia_account_key, "token");
+  account_manager_.UpsertToken(gaia_account_key_, kGaiaToken);
 
   std::vector<std::string> accounts = delegate_->GetAccounts();
   EXPECT_EQ(1UL, accounts.size());
@@ -387,7 +389,7 @@ TEST_F(CrOSOAuthDelegateTest, GetAccountsReturnsGaiaAccounts) {
 TEST_F(CrOSOAuthDelegateTest, UpdateCredentialsSucceeds) {
   EXPECT_TRUE(delegate_->GetAccounts().empty());
 
-  delegate_->UpdateCredentials(account_info_.account_id, "token");
+  delegate_->UpdateCredentials(account_info_.account_id, kGaiaToken);
 
   std::vector<std::string> accounts = delegate_->GetAccounts();
   EXPECT_EQ(1UL, accounts.size());
@@ -395,13 +397,11 @@ TEST_F(CrOSOAuthDelegateTest, UpdateCredentialsSucceeds) {
 }
 
 TEST_F(CrOSOAuthDelegateTest, ObserversAreNotifiedOnAccountRemoval) {
-  delegate_->UpdateCredentials(account_info_.account_id, "token");
+  delegate_->UpdateCredentials(account_info_.account_id, kGaiaToken);
 
   TokenServiceObserver observer;
   delegate_->AddObserver(&observer);
-  const AccountManager::AccountKey account_key{account_info_.gaia,
-                                               ACCOUNT_TYPE_GAIA};
-  account_manager_.RemoveAccount(account_key);
+  account_manager_.RemoveAccount(gaia_account_key_);
 
   EXPECT_EQ(1UL, observer.batch_change_records_.size());
   EXPECT_EQ(1UL, observer.batch_change_records_[0].size());
@@ -443,7 +443,7 @@ TEST_F(CrOSOAuthDelegateTest, TransientErrorsAreNotShown) {
 }
 
 TEST_F(CrOSOAuthDelegateTest, BackOffIsTriggerredForTransientErrors) {
-  delegate_->UpdateCredentials(account_info_.account_id, "123");
+  delegate_->UpdateCredentials(account_info_.account_id, kGaiaToken);
   auto transient_error = GoogleServiceAuthError(
       GoogleServiceAuthError::State::SERVICE_UNAVAILABLE);
   delegate_->UpdateAuthError(account_info_.account_id, transient_error);
@@ -480,7 +480,7 @@ TEST_F(CrOSOAuthDelegateTest, BackOffIsTriggerredForTransientErrors) {
 }
 
 TEST_F(CrOSOAuthDelegateTest, BackOffIsResetOnNetworkChange) {
-  delegate_->UpdateCredentials(account_info_.account_id, "123");
+  delegate_->UpdateCredentials(account_info_.account_id, kGaiaToken);
   auto transient_error = GoogleServiceAuthError(
       GoogleServiceAuthError::State::SERVICE_UNAVAILABLE);
   delegate_->UpdateAuthError(account_info_.account_id, transient_error);

chromeos/account_manager/account_manager.cc

@@ -93,6 +93,12 @@ std::vector<AccountManager::AccountKey> GetAccountKeys(
 
 }  // namespace
 
+// static
+const char AccountManager::kActiveDirectoryDummyToken[] = "dummy_ad_token";
+
+// static
+const char AccountManager::kInvalidToken[] = "";
+
 class AccountManager::GaiaTokenRevocationRequest : public GaiaAuthConsumer {
  public:
   GaiaTokenRevocationRequest(
@@ -289,6 +295,11 @@ void AccountManager::UpsertToken(const AccountKey& account_key,
                                  const std::string& token) {
   DCHECK_NE(init_state_, InitializationState::kNotStarted);
 
+  if (account_key.account_type ==
+      account_manager::AccountType::ACCOUNT_TYPE_ACTIVE_DIRECTORY) {
+    DCHECK_EQ(token, kActiveDirectoryDummyToken);
+  }
+
   base::OnceClosure closure =
       base::BindOnce(&AccountManager::UpsertTokenInternal,
                      weak_factory_.GetWeakPtr(), account_key, token);
@@ -370,7 +381,9 @@ bool AccountManager::IsTokenAvailable(const AccountKey& account_key) const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   auto it = tokens_.find(account_key);
-  return it != tokens_.end() && !it->second.empty();
+  return it != tokens_.end() && !it->second.empty() &&
+         it->second != kActiveDirectoryDummyToken &&
+         it->second != kInvalidToken;
 }
 
 void AccountManager::MaybeRevokeTokenOnServer(const AccountKey& account_key) {

chromeos/account_manager/account_manager.h

@@ -39,6 +39,24 @@ namespace chromeos {
 
 class CHROMEOS_EXPORT AccountManager {
  public:
+  // A dummy token stored against Active Directory accounts in Account Manager.
+  // Accounts stored in Account Manager must have a token associated with them.
+  // Active Directory accounts use Kerberos tickets for authentication, which is
+  // handled by a different infrastructure. Hence, we need a dummy token to
+  // store Active Directory accounts in Account Manager.
+  // See |AccountManager::UpsertToken|.
+  static const char kActiveDirectoryDummyToken[];
+
+  // A special token that marks an account in Account Manager as invalid, but
+  // does not remove the account. This is useful in scenarios where account
+  // names are imported from elsewhere (Chrome content area or ARC++) and their
+  // tokens are not yet known, but at the same time, these accounts need to be
+  // surfaced on the UI.
+  // Do not use this token for Active Directory accounts,
+  // |kActiveDirectoryDummyToken| is meant for that.
+  // See |AccountManager::UpsertToken|.
+  static const char kInvalidToken[];
+
   struct AccountKey {
     // |id| is obfuscated GAIA id for |AccountType::ACCOUNT_TYPE_GAIA|.
     // |id| is object GUID (|AccountId::GetObjGuid|) for
@@ -121,8 +139,11 @@ class CHROMEOS_EXPORT AccountManager {
   void RemoveAccount(const AccountKey& account_key);
 
   // Updates or inserts a token, for the account corresponding to the given
-  // |account_key|. |account_key| must be valid (|AccountKey::IsValid|).
-  // This API is idempotent.
+  // |account_key|. Use |AccountManager::kActiveDirectoryDummyToken| as the
+  // |token| for Active Directory accounts, and |AccountManager::kInvalidToken|
+  // for accounts with unkown tokens.
+  // Note: |account_key| must be valid (|AccountKey::IsValid|).
+  // Note: This API is idempotent.
   void UpsertToken(const AccountKey& account_key, const std::string& token);
 
   // Add a non owning pointer to an |AccountManager::Observer|.
@@ -144,8 +165,7 @@ class CHROMEOS_EXPORT AccountManager {
       OAuth2AccessTokenConsumer* consumer) const;
 
   // Returns |true| if an LST is available for |account_key|.
-  // Note: An LST will not be available for |account_key| if it is an Active
-  // Directory account.
+  // Note: Always returns false for Active Directory accounts.
   // Note: This method will return |false| if |AccountManager| has not been
   // initialized yet.
   bool IsTokenAvailable(const AccountKey& account_key) const;