From db52be0212320e29db375ee45a613a3fa069c4e7 Mon Sep 17 00:00:00 2001
From: Friedrich Horschig <friedrichh@chromium.org>
Date: Tue, 26 May 2026 09:59:40 -0700
Subject: [PATCH] [Android] Secure ThirdPartyCredentialManager against background spam

This CL hardens the ThirdPartyCredentialManagerImpl against potential
spam from background tabs:

1. Adds visibility and focus checks to Get and Store paths in
   ThirdPartyCredentialManagerImpl to ensure only the active, visible
   tab can trigger the Credential Manager UI.
2. Implements pending_request_ throttling to prevent rapid-fire or
   concurrent requests from spamming the system UI.
3. Integrates JNI CancellationSignal to programmatically dismiss
   theAndroid system bottom sheet when the Mojo pipe disconnects or
   the tab is closed.
4. Updates Java bridge to prefer Activity context (falling back to
   ApplicationContext if null) to ensure system UI is properly anchored.
5. Adds IsOffTheRecord() gate to Store path to prevent leaking Incognito
   origins to third-party password manager databases.

Fixed: 514067416
Change-Id: I210a41637a43ec8475243ca2eb738822d4ee1810
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7863624
Reviewed-by: Ivana Žužić <izuzic@google.com>
Commit-Queue: Friedrich Hauser <friedrichh@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1636264}
---

diff --git a/components/credential_management/android/BUILD.gn b/components/credential_management/android/BUILD.gn
index 4238d9e..2450fb65 100644
--- a/components/credential_management/android/BUILD.gn
+++ b/components/credential_management/android/BUILD.gn
@@ -27,9 +27,11 @@
   deps = [
     "//base:base_java",
     "//base:tasks_java",
+    "//content/public/android:content_java",
     "//third_party/androidx:androidx_annotation_annotation_java",
     "//third_party/androidx:androidx_credentials_credentials_java",
     "//third_party/jni_zero:jni_zero_java",
+    "//ui/android:ui_no_recycler_view_java",
     "//url:url_java",
   ]
 
@@ -101,9 +103,11 @@
     "//base:base_java",
     "//base:base_java_test_support",
     "//base:base_junit_test_support",
+    "//content/public/android:content_java",
     "//third_party/androidx:androidx_credentials_credentials_java",
     "//third_party/junit",
     "//third_party/mockito:mockito_java",
+    "//ui/android:ui_no_recycler_view_java",
     "//url:gurl_java",
   ]
 }
diff --git a/components/credential_management/android/DEPS b/components/credential_management/android/DEPS
index dba4789e..180d538a 100644
--- a/components/credential_management/android/DEPS
+++ b/components/credential_management/android/DEPS
@@ -1,6 +1,9 @@
 include_rules = [
+  "-content/public/android",
+  "+content/public/android/java/src/org/chromium/content_public",
   "+content/public/browser",
   "+net",
+  "+ui/android/java/src/org/chromium/ui",
 ]
 
 specific_include_rules = {
diff --git a/components/credential_management/android/java/src/org/chromium/components/credential_management/ThirdPartyCredentialManagerBridge.java b/components/credential_management/android/java/src/org/chromium/components/credential_management/ThirdPartyCredentialManagerBridge.java
index 744ddd5..f92254d94 100644
--- a/components/credential_management/android/java/src/org/chromium/components/credential_management/ThirdPartyCredentialManagerBridge.java
+++ b/components/credential_management/android/java/src/org/chromium/components/credential_management/ThirdPartyCredentialManagerBridge.java
@@ -4,7 +4,8 @@
 
 package org.chromium.components.credential_management;
 
-import android.content.Context;
+import android.app.Activity;
+import android.os.CancellationSignal;
 
 import androidx.credentials.CreateCredentialResponse;
 import androidx.credentials.CreatePasswordRequest;
@@ -23,10 +24,12 @@
 import org.jni_zero.JniType;
 
 import org.chromium.base.Callback;
-import org.chromium.base.ContextUtils;
 import org.chromium.base.ResettersForTesting;
+import org.chromium.base.ThreadUtils;
 import org.chromium.build.annotations.NullMarked;
 import org.chromium.build.annotations.Nullable;
+import org.chromium.content_public.browser.WebContents;
+import org.chromium.ui.base.WindowAndroid;
 import org.chromium.url.GURL;
 
 import java.util.Collections;
@@ -37,6 +40,7 @@
 @NullMarked
 class ThirdPartyCredentialManagerBridge {
     private static @Nullable CredentialManager sCredentialManagerForTesting;
+    private @Nullable CancellationSignal mCancellationSignal;
 
     @CalledByNative
     ThirdPartyCredentialManagerBridge() {}
@@ -48,16 +52,22 @@
 
     @CalledByNative
     void get(
+            @Nullable WebContents webContents,
             boolean isAutoSelectAllowed,
             boolean includePasswords,
             @JniType("std::vector") List<GURL> federations,
             String origin,
             Callback<PasswordCredentialResponse> callback) {
         // TODO(crbug.com/419810756): Add support for federated credentials.
-        Context context = ContextUtils.getApplicationContext();
+        Activity activity = getActivity(webContents);
+        if (activity == null) {
+            callback.onResult(new PasswordCredentialResponse(false, "", ""));
+            return;
+        }
+
         CredentialManager credentialManager =
                 sCredentialManagerForTesting == null
-                        ? CredentialManager.create(context)
+                        ? CredentialManager.create(activity)
                         : sCredentialManagerForTesting;
         // We're currently preventing silent access for every get request by
         // default in 3rd party mode so isAutoSelectAllowed is always set to
@@ -79,6 +89,7 @@
                         new CredentialManagerCallback<>() {
                             @Override
                             public void onError(GetCredentialException error) {
+                                mCancellationSignal = null;
                                 callback.onResult(new PasswordCredentialResponse(false, "", ""));
                                 ThirdPartyCredentialManagerMetricsRecorder
                                         .recordCredentialManagerGetResult(
@@ -87,22 +98,38 @@
 
                             @Override
                             public void onResult(GetCredentialResponse result) {
+                                mCancellationSignal = null;
                                 onGetCredentialResponse(result, callback);
                                 ThirdPartyCredentialManagerMetricsRecorder
                                         .recordCredentialManagerGetResult(
                                                 /* success= */ true, /* error= */ null);
                             }
                         };
+        mCancellationSignal = new CancellationSignal();
         credentialManager.getCredentialAsync(
-                context, getCredentialRequestBuilder.build(), null, Runnable::run, credentialCallback);
+                activity,
+                getCredentialRequestBuilder.build(),
+                mCancellationSignal,
+                ThreadUtils::postOnUiThread,
+                credentialCallback);
     }
 
     @CalledByNative
-    void store(String username, String password, String origin, Callback<Boolean> callback) {
-        Context context = ContextUtils.getApplicationContext();
+    void store(
+            @Nullable WebContents webContents,
+            String username,
+            String password,
+            String origin,
+            Callback<Boolean> callback) {
+        Activity activity = getActivity(webContents);
+        if (activity == null) {
+            callback.onResult(false);
+            return;
+        }
+
         CredentialManager credentialManager =
                 sCredentialManagerForTesting == null
-                        ? CredentialManager.create(context)
+                        ? CredentialManager.create(activity)
                         : sCredentialManagerForTesting;
         CreatePasswordRequest createPasswordRequest =
                 new CreatePasswordRequest(username, password, origin, false, false);
@@ -112,6 +139,7 @@
                         new CredentialManagerCallback<>() {
                             @Override
                             public void onError(CreateCredentialException error) {
+                                mCancellationSignal = null;
                                 callback.onResult(false);
                                 ThirdPartyCredentialManagerMetricsRecorder
                                         .recordCredentialManagerStoreResult(
@@ -120,14 +148,20 @@
 
                             @Override
                             public void onResult(CreateCredentialResponse response) {
+                                mCancellationSignal = null;
                                 callback.onResult(true);
                                 ThirdPartyCredentialManagerMetricsRecorder
                                         .recordCredentialManagerStoreResult(

diff --git a/components/credential_management/android/third_party_credential_manager_bridge_unittest.cc b/components/credential_management/android/third_party_credential_manager_bridge_unittest.cc
index 6614bc2d..cefb202f 100644
--- a/components/credential_management/android/third_party_credential_manager_bridge_unittest.cc
+++ b/components/credential_management/android/third_party_credential_manager_bridge_unittest.cc
@@ -18,6 +18,8 @@
 #include "components/credential_management/android/password_credential_response.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/browser_task_environment.h"
+#include "content/public/test/test_browser_context.h"
+#include "content/public/test/web_contents_tester.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -27,6 +29,7 @@
 const std::string kTestOrigin = "https://origin.com";
 }  // namespace
 namespace credential_management {
+using testing::_;
 using StoreCallback = base::OnceCallback<void()>;
 using GetCallback = base::OnceCallback<void(
     password_manager::CredentialManagerError,
@@ -41,7 +44,8 @@
   FakeJniDelegate& operator=(const FakeJniDelegate&) = delete;
   ~FakeJniDelegate() override = default;
 
-  void Get(bool is_auto_select_allowed,
+  void Get(content::WebContents& web_contents,
+           bool is_auto_select_allowed,
            bool include_passwords,
            const std::vector<GURL>& federations,
            const std::string& origin,
@@ -60,7 +64,8 @@
                                       true, kTestUsername, kTestPassword)));
   }
 
-  void Store(const std::u16string& username,
+  void Store(content::WebContents& web_contents,
+             const std::u16string& username,
              const std::u16string& password,
              const std::string& origin,
              base::OnceCallback<void(bool)> completion_callback) override {
@@ -69,6 +74,11 @@
         base::BindOnce(std::move(completion_callback), !simulate_errors_));
   }
 
+  void Cancel() override { cancel_called_ = true; }
+
+  bool cancel_called() const { return cancel_called_; }
+  void reset_cancel_called() { cancel_called_ = false; }
+
   void set_bridge(ThirdPartyCredentialManagerBridge* bridge) {
     bridge_ = bridge;
   }
@@ -80,12 +90,17 @@
  private:
   // The owning native ThirdPartyCredentialManagerBridge.
   raw_ptr<ThirdPartyCredentialManagerBridge> bridge_;
-  bool simulate_errors_;
+  bool simulate_errors_ = false;
+  bool cancel_called_ = false;
 };
 
 class ThirdPartyCredentialManagerBridgeTest : public testing::Test {
  public:
   void SetUp() override {
+    browser_context_ = std::make_unique<content::TestBrowserContext>();
+    web_contents_ = content::WebContentsTester::CreateTestWebContents(
+        browser_context_.get(), nullptr);
+
     auto jni_delegate = std::make_unique<FakeJniDelegate>();
     fake_jni_delegate_ = jni_delegate.get();
     bridge_ = std::make_unique<ThirdPartyCredentialManagerBridge>(
@@ -97,9 +112,12 @@
   FakeJniDelegate& fake_jni_delegate() { return *fake_jni_delegate_; }
 
   ThirdPartyCredentialManagerBridge* bridge() { return bridge_.get(); }
+  content::WebContents* web_contents() { return web_contents_.get(); }
 
  private:
   content::BrowserTaskEnvironment task_environment_;
+  std::unique_ptr<content::TestBrowserContext> browser_context_;
+  std::unique_ptr<content::WebContents> web_contents_;
   raw_ptr<FakeJniDelegate> fake_jni_delegate_;
   std::unique_ptr<ThirdPartyCredentialManagerBridge> bridge_;
 };
@@ -109,11 +127,11 @@
   base::MockCallback<GetCallback> mock_callback;
   fake_jni_delegate().set_error_simulation(false);
 
-  EXPECT_CALL(
-      mock_callback,
-      Run(password_manager::CredentialManagerError::SUCCESS, testing::_))
+  EXPECT_CALL(mock_callback,
+              Run(password_manager::CredentialManagerError::SUCCESS, _))
       .WillOnce([&]() { run_loop.Quit(); });
-  bridge()->Get(/*is_auto_select_allowed=*/false, /*include_passwords=*/true,
+  bridge()->Get(*web_contents(), /*is_auto_select_allowed=*/false,
+                /*include_passwords=*/true,
                 /*federations=*/{}, kTestOrigin, mock_callback.Get());
   run_loop.Run();
 }
@@ -123,11 +141,11 @@
   base::MockCallback<GetCallback> mock_callback;
   fake_jni_delegate().set_error_simulation(true);
 
-  EXPECT_CALL(
-      mock_callback,
-      Run(password_manager::CredentialManagerError::UNKNOWN, testing::_))
+  EXPECT_CALL(mock_callback,
+              Run(password_manager::CredentialManagerError::UNKNOWN, _))
       .WillOnce([&]() { run_loop.Quit(); });
-  bridge()->Get(/*is_auto_select_allowed=*/true, /*include_passwords=*/true,
+  bridge()->Get(*web_contents(), /*is_auto_select_allowed=*/true,
+                /*include_passwords=*/true,
                 /*federations=*/{}, kTestOrigin, mock_callback.Get());
   run_loop.Run();
 }
@@ -138,7 +156,8 @@
                          const std::optional<password_manager::CredentialInfo>&>
       future;
 
-  bridge()->Get(/*is_auto_select_allowed=*/true, /*include_passwords=*/false,
+  bridge()->Get(*web_contents(), /*is_auto_select_allowed=*/true,
+                /*include_passwords=*/false,
                 /*federations=*/{}, kTestOrigin, future.GetCallback());
   ASSERT_TRUE(future.Wait());
   EXPECT_EQ(future.Get<0>(), password_manager::CredentialManagerError::UNKNOWN);
@@ -151,7 +170,7 @@
   fake_jni_delegate().set_error_simulation(false);
 
   EXPECT_CALL(mock_callback, Run()).WillOnce([&]() { run_loop.Quit(); });
-  bridge()->Store(kTestUsername, kTestPassword, kTestOrigin,
+  bridge()->Store(*web_contents(), kTestUsername, kTestPassword, kTestOrigin,
                   mock_callback.Get());
   run_loop.Run();
 }
@@ -162,7 +181,7 @@
   fake_jni_delegate().set_error_simulation(true);
 
   EXPECT_CALL(mock_callback, Run()).WillOnce([&]() { run_loop.Quit(); });
-  bridge()->Store(kTestUsername, kTestPassword, kTestOrigin,
+  bridge()->Store(*web_contents(), kTestUsername, kTestPassword, kTestOrigin,
                   mock_callback.Get());
   run_loop.Run();
 }
@@ -177,18 +196,25 @@
   EXPECT_CALL(mock_store_callback, Run()).WillOnce([&]() {
     run_loop_store.Quit();
   });
-  bridge()->Store(kTestUsername, kTestPassword, kTestOrigin,
+  bridge()->Store(*web_contents(), kTestUsername, kTestPassword, kTestOrigin,
                   mock_store_callback.Get());
   run_loop_store.Run();
 
-  EXPECT_CALL(
-      mock_get_callback,
-      Run(password_manager::CredentialManagerError::SUCCESS, testing::_))
+  EXPECT_CALL(mock_get_callback,
+              Run(password_manager::CredentialManagerError::SUCCESS, _))
       .WillOnce([&]() { run_loop_get.Quit(); });
 
-  bridge()->Get(/*is_auto_select_allowed=*/true, /*include_passwords=*/true,
+  bridge()->Get(*web_contents(), /*is_auto_select_allowed=*/true,
+                /*include_passwords=*/true,
                 /*federations=*/{}, kTestOrigin, mock_get_callback.Get());
   run_loop_get.Run();
 }
 
+TEST_F(ThirdPartyCredentialManagerBridgeTest, TestCancel) {
+  fake_jni_delegate().reset_cancel_called();
+  ASSERT_FALSE(fake_jni_delegate().cancel_called());
+  bridge()->Cancel();
+  EXPECT_TRUE(fake_jni_delegate().cancel_called());
+}
+
 }  // namespace credential_management
diff --git a/components/credential_management/android/third_party_credential_manager_impl_unittest.cc b/components/credential_management/android/third_party_credential_manager_impl_unittest.cc
index 3ef15d42..04ef8c3 100644
--- a/components/credential_management/android/third_party_credential_manager_impl_unittest.cc
+++ b/components/credential_management/android/third_party_credential_manager_impl_unittest.cc
@@ -7,6 +7,7 @@
 #include "base/test/mock_callback.h"
 #include "components/credential_management/android/third_party_credential_manager_bridge.h"
 #include "components/password_manager/core/common/credential_manager_types.h"
+#include "content/public/browser/visibility.h"
 #include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_renderer_host.h"
 #include "content/public/test/web_contents_tester.h"
@@ -14,6 +15,8 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 using testing::_;
+using testing::Eq;
+using testing::Ref;
 
 namespace {
 const std::u16string kTestUsername = u"username";
@@ -35,7 +38,8 @@
 
   MOCK_METHOD(void,
               Get,
-              (bool is_auto_select_allowed,
+              (content::WebContents&,
+               bool is_auto_select_allowed,
                bool include_passwords,
                const std::vector<GURL>& federations,
                const std::string&,
@@ -43,11 +47,13 @@
               (override));
   MOCK_METHOD(void,
               Store,
-              (const std::u16string&,
+              (content::WebContents&,
+               const std::u16string&,
                const std::u16string&,
                const std::string&,
                StoreCallback),
               (override));
+  MOCK_METHOD(void, Cancel, (), (override));
 };
 
 class ThirdPartyCredentialManagerImplTest
@@ -83,8 +89,8 @@
 TEST_F(ThirdPartyCredentialManagerImplTest, TestStore) {
   NavigateToTestOrigin();
 
-  EXPECT_CALL(*mock_bridge(),
-              Store(kTestUsername, kTestPassword, kTestOrigin, _));
+  EXPECT_CALL(*mock_bridge(), Store(Ref(*web_contents()), kTestUsername,
+                                    kTestPassword, kTestOrigin, _));
   password_manager::CredentialInfo info = password_manager::CredentialInfo(
       ::password_manager::CredentialType::CREDENTIAL_TYPE_PASSWORD,
       /*id=*/kTestUsername,
@@ -100,7 +106,7 @@
   content::WebContentsTester::For(web_contents())
       ->NavigateAndCommit(GURL(kTestOrigin));
 
-  EXPECT_CALL(*mock_bridge(), Store(_, _, kTestOrigin, _)).Times(0);
+  EXPECT_CALL(*mock_bridge(), Store(_, _, _, kTestOrigin, _)).Times(0);
   password_manager::CredentialInfo info = password_manager::CredentialInfo(
       password_manager::CredentialType::CREDENTIAL_TYPE_EMPTY,
       /*id=*/u"",
@@ -120,7 +126,7 @@
   content::WebContentsTester::For(web_contents())
       ->NavigateAndCommit(GURL(kTestOrigin));
 
-  EXPECT_CALL(*mock_bridge(), Store(_, _, kTestOrigin, _)).Times(0);
+  EXPECT_CALL(*mock_bridge(), Store(_, _, _, kTestOrigin, _)).Times(0);
   password_manager::CredentialInfo info = password_manager::CredentialInfo(
       password_manager::CredentialType::CREDENTIAL_TYPE_PASSWORD,
       /*id=*/kTestUsername,
@@ -140,7 +146,7 @@
   content::WebContentsTester::For(web_contents())
       ->NavigateAndCommit(GURL(kTestOrigin));
 
-  EXPECT_CALL(*mock_bridge(), Store(_, _, kTestOrigin, _));
+  EXPECT_CALL(*mock_bridge(), Store(_, _, _, kTestOrigin, _));
   password_manager::CredentialInfo info = password_manager::CredentialInfo(
       password_manager::CredentialType::CREDENTIAL_TYPE_PASSWORD,
       /*id=*/u"",
@@ -156,7 +162,8 @@
   NavigateToTestOrigin();
 
   EXPECT_CALL(*mock_bridge(),
-              Get(/*is_auto_select_allowed=*/true, /*include_passwords=*/true,
+              Get(Ref(*web_contents()), /*is_auto_select_allowed=*/true,
+                  /*include_passwords=*/true,
                   /*federations=*/std::vector<GURL>(), kTestOrigin, _));
 
   credential_manager()->Get(
@@ -169,7 +176,8 @@
   NavigateToTestOrigin();
 
   EXPECT_CALL(*mock_bridge(),
-              Get(/*is_auto_select_allowed=*/false, /*include_passwords=*/true,
+              Get(Ref(*web_contents()), /*is_auto_select_allowed=*/false,
+                  /*include_passwords=*/true,
                   /*federations=*/std::vector<GURL>(), kTestOrigin, _));
 
   credential_manager()->Get(
@@ -184,7 +192,7 @@
 
   EXPECT_CALL(mock_get_callback,
               Run(password_manager::CredentialManagerError::SUCCESS,
-                  testing::Eq(password_manager::CredentialInfo())));
+                  Eq(password_manager::CredentialInfo())));
 
   credential_manager()->Get(
       /*mediation=*/password_manager::CredentialMediationRequirement::kSilent,
@@ -198,7 +206,7 @@
 
   EXPECT_CALL(mock_get_callback,
               Run(password_manager::CredentialManagerError::SUCCESS,
-                  testing::Eq(password_manager::CredentialInfo())));
+                  Eq(password_manager::CredentialInfo())));
 
   credential_manager()->Get(
       /*mediation=*/password_manager::CredentialMediationRequirement::
@@ -225,11 +233,156 @@
   // In incognito mode, Get should return immediately with an empty credential.
... (truncated)