From 3d260133feb022041965feb6df42a232cd8323e1 Mon Sep 17 00:00:00 2001
From: Ernest Nguyen Hung <[email protected]>
Date: Tue, 15 Sep 2026 04:15:16 -0700
Subject: [PATCH] [signin] Defer persisting account until `OnMintTokenSuccess`

This CL defers calling `SetGaiaIdForExtension` until
`OnMintTokenSuccess` for `getAuthToken` extension api, guaranteeing that
the extension's account binding is only written to `ExtensionPrefs`
after the server successfully validates the consent result.

Fixed: 497087197
Change-Id: I1eafb042ad1a2958254f6bbeae6ee03159937489
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8381041
Reviewed-by: Alex Ilin <[email protected]>
Commit-Queue: Ernest Nguyen Hung <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1697979}
---
diff --git a/chrome/browser/extensions/api/identity/identity_get_auth_token_function.cc b/chrome/browser/extensions/api/identity/identity_get_auth_token_function.cc
index c4af70f..b37a00e5 100644
--- a/chrome/browser/extensions/api/identity/identity_get_auth_token_function.cc
+++ b/chrome/browser/extensions/api/identity/identity_get_auth_token_function.cc
@@ -698,10 +698,15 @@
 
   IdentityTokenCacheValue token = IdentityTokenCacheValue::CreateToken(
       result.access_token, result.granted_scopes, result.time_to_live);
-  IdentityAPI::GetFactoryInstance()
-      ->Get(GetProfile())
-      ->token_cache()
-      ->SetToken(token_key_, token);
+  IdentityAPI* id_api = IdentityAPI::GetFactoryInstance()->Get(GetProfile());
+
+  id_api->token_cache()->SetToken(token_key_, token);
+  // Persist the account once the remote consent flow has been verified by a
+  // successful mint token response.
+  if (remote_consent_approved_) {
+    id_api->SetGaiaIdForExtension(token_key_.extension_id,
+                                  token_key_.account_info.gaia);
+  }
 
   CompleteMintTokenFlow();
   CompleteFunctionWithResult(result.access_token, result.granted_scopes);
@@ -892,16 +897,16 @@
     }
   }
 
-  IdentityAPI* id_api = IdentityAPI::GetFactoryInstance()->Get(GetProfile());
-  id_api->SetGaiaIdForExtension(token_key_.extension_id, gaia_id);
-
   // 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_info = account.GetCoreAccountInfo();
-  id_api->token_cache()->SetToken(
-      new_token_key,
-      IdentityTokenCacheValue::CreateRemoteConsentApproved(consent_result));
+  IdentityAPI::GetFactoryInstance()
+      ->Get(GetProfile())
+      ->token_cache()
+      ->SetToken(
+          new_token_key,
+          IdentityTokenCacheValue::CreateRemoteConsentApproved(consent_result));
   CompleteMintTokenFlow();
   token_key_ = new_token_key;
   consent_result_ = consent_result;
diff --git a/chrome/browser/extensions/api/identity/identity_get_auth_token_function.h b/chrome/browser/extensions/api/identity/identity_get_auth_token_function.h
index 53bdc5b2..dec25637 100644
--- a/chrome/browser/extensions/api/identity/identity_get_auth_token_function.h
+++ b/chrome/browser/extensions/api/identity/identity_get_auth_token_function.h
@@ -278,7 +278,10 @@
 #endif
   std::unique_ptr<GaiaRemoteConsentFlow> gaia_remote_consent_flow_;
   std::string consent_result_;
-  // Added for debugging https://crbug.com/40134189.
+  // Indicates whether remote consent was approved for this request.
+  // Used for UMA logging and to gate persisting the account binding in
+  // `OnMintTokenSuccess`. See https://crbug.com/40134189 and
+  // https://crbug.com/497087197.
   bool remote_consent_approved_ = false;
 
   // Invoked when IdentityAPI is shut down.
diff --git a/chrome/browser/extensions/api/identity/identity_apitest.cc b/chrome/browser/extensions/api/identity/identity_apitest.cc
index ed66eaf..c35f503 100644
--- a/chrome/browser/extensions/api/identity/identity_apitest.cc
+++ b/chrome/browser/extensions/api/identity/identity_apitest.cc
@@ -148,6 +148,7 @@
 using ::testing::_;
 using ::testing::HasSubstr;
 using ::testing::Return;
+using ::testing::StartsWith;
 
 namespace extensions {
 
@@ -2969,6 +2970,78 @@
   }
 }
 
+// Regression test for https://crbug.com/497087197. Tests that when remote
+// consent flow is approved but the subsequent mint token flow fails (e.g.,
+// server rejects an invalid or forged consent result), Chrome does NOT persist
+// the account to prefs. Chrome should continue using the primary account in
+// subsequent `getAuthToken` calls for that extension.
+IN_PROC_BROWSER_TEST_F(
+    GetAuthTokenFunctionTest,
+    MultiSecondaryInteractiveRemoteConsentMintTokenFailureDoesNotPersistAccount) {
+  if (id_api()->AreExtensionsRestrictedToPrimaryAccount()) {
+    GTEST_SKIP() << "Extensions are restricted to the primary account.";
+  }
+
+  const CoreAccountId primary_account_id = SignIn("[email protected]");
+  const AccountInfo secondary_account =
+      identity_test_env()->MakeAccountAvailable("[email protected]");
+  const extensions::Extension* extension = CreateExtension(CLIENT_ID | SCOPES);
+
+  {
+    auto func = base::MakeRefCounted<FakeGetAuthTokenFunction>();
+    func->set_extension(extension);
+    func->push_mint_token_result(
+        TestOAuth2MintTokenFlow::REMOTE_CONSENT_SUCCESS);
+    func->push_mint_token_result(TestOAuth2MintTokenFlow::MINT_TOKEN_FAILURE);
+    func->set_remote_consent_gaia_id(secondary_account.GetGaiaId());
+    func->set_auto_login_access_token(false);
+
+    base::RunLoop run_loop;
+    on_access_token_requested_ = run_loop.QuitClosure();
+    RunFunctionAsync(func.get(), /*args=*/"[{\"interactive\": true}]");
+    run_loop.Run();
+
+    IssueLoginAccessTokenForAccount(primary_account_id);
+    IssueLoginAccessTokenForAccount(secondary_account.GetAccountId());
+
+    ASSERT_THAT(WaitForError(func.get()), StartsWith(errors::kAuthFailure));
+    // The unverified account must NOT be persisted in prefs.
+    ASSERT_EQ(id_api()->GetGaiaIdForExtension(extension->id()), std::nullopt);
+    histogram_tester()->ExpectUniqueSample(
+        kGetAuthTokenResultHistogramName,
+        IdentityGetAuthTokenError::State::kMintTokenAuthFailure, 1);
+    histogram_tester()->ExpectUniqueSample(
+        kGetAuthTokenResultAfterConsentApprovedHistogramName,
+        IdentityGetAuthTokenError::State::kMintTokenAuthFailure, 1);
+  }
+
+  {
+    // Clear in-memory token cache to simulate a browser restart.
+    id_api()->token_cache()->EraseAllTokens();
+
+    // A subsequent `getAuthToken` call without an account parameter should
+    // fall back to the primary account rather than the secondary account.
+    auto func = base::MakeRefCounted<FakeGetAuthTokenFunction>();
+    func->set_extension(extension);
+    func->push_mint_token_result(TestOAuth2MintTokenFlow::MINT_TOKEN_SUCCESS);
+
+    std::string access_token;
+    std::set<std::string> granted_scopes;
+    RunGetAuthTokenFunction(func.get(), /*args=*/"[{}]", profile(),
+                            &access_token, &granted_scopes);
+    EXPECT_EQ(access_token, kAccessToken);
+    EXPECT_EQ(func->GetExtensionTokenKeyForTest()->account_info.account_id,
+              primary_account_id);
+    EXPECT_EQ(id_api()->GetGaiaIdForExtension(extension->id()), std::nullopt);
+    histogram_tester()->ExpectBucketCount(
+        kGetAuthTokenResultHistogramName,
+        IdentityGetAuthTokenError::State::kNone, 1);
+    histogram_tester()->ExpectBucketCount(
+        kGetAuthTokenResultAfterConsentApprovedHistogramName,
+        IdentityGetAuthTokenError::State::kNone, 0);
+  }
+}
+
 // Tests two concurrent remote consent flows. Both of them should succeed.
 // The second flow starts while the first one is blocked on an interactive mint
 // token flow. This is a regression test for https://crbug.com/40134189.

diff --git a/chrome/browser/extensions/api/identity/identity_apitest.cc b/chrome/browser/extensions/api/identity/identity_apitest.cc
index ed66eaf..c35f503 100644
--- a/chrome/browser/extensions/api/identity/identity_apitest.cc
+++ b/chrome/browser/extensions/api/identity/identity_apitest.cc
@@ -148,6 +148,7 @@
 using ::testing::_;
 using ::testing::HasSubstr;
 using ::testing::Return;
+using ::testing::StartsWith;
 
 namespace extensions {
 
@@ -2969,6 +2970,78 @@
   }
 }
 
+// Regression test for https://crbug.com/497087197. Tests that when remote
+// consent flow is approved but the subsequent mint token flow fails (e.g.,
+// server rejects an invalid or forged consent result), Chrome does NOT persist
+// the account to prefs. Chrome should continue using the primary account in
+// subsequent `getAuthToken` calls for that extension.
+IN_PROC_BROWSER_TEST_F(
+    GetAuthTokenFunctionTest,
+    MultiSecondaryInteractiveRemoteConsentMintTokenFailureDoesNotPersistAccount) {
+  if (id_api()->AreExtensionsRestrictedToPrimaryAccount()) {
+    GTEST_SKIP() << "Extensions are restricted to the primary account.";
+  }
+
+  const CoreAccountId primary_account_id = SignIn("[email protected]");
+  const AccountInfo secondary_account =
+      identity_test_env()->MakeAccountAvailable("[email protected]");
+  const extensions::Extension* extension = CreateExtension(CLIENT_ID | SCOPES);
+
+  {
+    auto func = base::MakeRefCounted<FakeGetAuthTokenFunction>();
+    func->set_extension(extension);
+    func->push_mint_token_result(
+        TestOAuth2MintTokenFlow::REMOTE_CONSENT_SUCCESS);
+    func->push_mint_token_result(TestOAuth2MintTokenFlow::MINT_TOKEN_FAILURE);
+    func->set_remote_consent_gaia_id(secondary_account.GetGaiaId());
+    func->set_auto_login_access_token(false);
+
+    base::RunLoop run_loop;
+    on_access_token_requested_ = run_loop.QuitClosure();
+    RunFunctionAsync(func.get(), /*args=*/"[{\"interactive\": true}]");
+    run_loop.Run();
+
+    IssueLoginAccessTokenForAccount(primary_account_id);
+    IssueLoginAccessTokenForAccount(secondary_account.GetAccountId());
+
+    ASSERT_THAT(WaitForError(func.get()), StartsWith(errors::kAuthFailure));
+    // The unverified account must NOT be persisted in prefs.
+    ASSERT_EQ(id_api()->GetGaiaIdForExtension(extension->id()), std::nullopt);
+    histogram_tester()->ExpectUniqueSample(
+        kGetAuthTokenResultHistogramName,
+        IdentityGetAuthTokenError::State::kMintTokenAuthFailure, 1);
+    histogram_tester()->ExpectUniqueSample(
+        kGetAuthTokenResultAfterConsentApprovedHistogramName,
+        IdentityGetAuthTokenError::State::kMintTokenAuthFailure, 1);
+  }
+
+  {
+    // Clear in-memory token cache to simulate a browser restart.
+    id_api()->token_cache()->EraseAllTokens();
+
+    // A subsequent `getAuthToken` call without an account parameter should
+    // fall back to the primary account rather than the secondary account.
+    auto func = base::MakeRefCounted<FakeGetAuthTokenFunction>();
+    func->set_extension(extension);
+    func->push_mint_token_result(TestOAuth2MintTokenFlow::MINT_TOKEN_SUCCESS);
+
+    std::string access_token;
+    std::set<std::string> granted_scopes;
+    RunGetAuthTokenFunction(func.get(), /*args=*/"[{}]", profile(),
+                            &access_token, &granted_scopes);
+    EXPECT_EQ(access_token, kAccessToken);
+    EXPECT_EQ(func->GetExtensionTokenKeyForTest()->account_info.account_id,
+              primary_account_id);
+    EXPECT_EQ(id_api()->GetGaiaIdForExtension(extension->id()), std::nullopt);
+    histogram_tester()->ExpectBucketCount(
+        kGetAuthTokenResultHistogramName,
+        IdentityGetAuthTokenError::State::kNone, 1);
+    histogram_tester()->ExpectBucketCount(
+        kGetAuthTokenResultAfterConsentApprovedHistogramName,
+        IdentityGetAuthTokenError::State::kNone, 0);
+  }
+}
+
 // Tests two concurrent remote consent flows. Both of them should succeed.
 // The second flow starts while the first one is blocked on an interactive mint
 // token flow. This is a regression test for https://crbug.com/40134189.