Access Context Auditing: Respect history deletions

CL enables the deletion of access records which effectively contain
browsing history by extending the AccessContextAuditService as a
HistoryServiceObserver.

Access records are deleted for top frame origins when the service is
notified that no more history entries exist for that origin.

Bug: 1101675
Change-Id: If98eda4d27f35a92f6580231d7a4c67179aa3dae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2288085
Commit-Queue: Theodore Olsauskas-Warren <sauski@google.com>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791721}

chrome/browser/browsing_data/access_context_audit_browsertest.cc

@@ -10,6 +10,7 @@
 #include "chrome/browser/browsing_data/access_context_audit_service_factory.h"
 #include "chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
+#include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/prefs/session_startup_pref.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
@@ -19,6 +20,7 @@
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/content_settings/core/browser/cookie_settings.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
+#include "components/history/core/browser/history_service.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browsing_data_remover.h"
 #include "content/public/browser/storage_partition.h"
@@ -240,6 +242,55 @@ IN_PROC_BROWSER_TEST_F(AccessContextAuditBrowserTest, CheckSessionOnly) {
   EXPECT_EQ(cookies.size(), 0u);
 }
 
+IN_PROC_BROWSER_TEST_F(AccessContextAuditBrowserTest, RemoveHistory) {
+  // Check that removing all history entries for an origin also removes all
+  // records where that origin is the top frame origin.
+  std::string replacement_path = GetPathWithHostAndPortReplaced(
+      "/browsing_data/embeds_storage_accessor.html",
+      net::HostPortPair::FromURL(embedded_.GetURL(kEmbeddedHost, "/")));
+  GURL top_level_url = top_level_.GetURL(kTopLevelHost, replacement_path);
+  ui_test_utils::NavigateToURL(browser(), top_level_url);
+
+  ui_test_utils::NavigateToURL(
+      browser(),
+      embedded_.GetURL(kEmbeddedHost, "/browsing_data/storage_accessor.html"));
+  base::RunLoop().RunUntilIdle();
+
+  auto records = GetAllAccessRecords();
+  auto cookies = GetAllCookies();
+  EXPECT_EQ(records.size(), 5u);
+
+  // Remove the history entry for the navigation to the page which embeds
+  // storage_accessor.html.
+  auto* history_service = HistoryServiceFactory::GetForProfile(
+      browser()->profile(), ServiceAccessType::EXPLICIT_ACCESS);
+  history_service->DeleteURLs({top_level_url});
+  base::RunLoop run_loop;
+  history_service->FlushForTest(run_loop.QuitClosure());
+  run_loop.Run();
+
+  // Check that only records for accesses which occurred during the direct
+  // navigation to storage_accessor.html exist.
+  records = GetAllAccessRecords();
+  cookies = GetAllCookies();
+  EXPECT_EQ(records.size(), 2u);
+  CheckContainsCookieAndRecord(cookies, records, embedded_origin(),
+                               "persistent", kEmbeddedHost, "/");
+  CheckContainsCookieAndRecord(cookies, records, embedded_origin(),
+                               "session_only", kEmbeddedHost, "/");
+
+  // The actual cookie set by the top level page should remain present, as
+  // the history deletion should only affect the access record.
+  EXPECT_EQ(cookies.size(), 3u);
+  EXPECT_NE(std::find_if(cookies.begin(), cookies.end(),
+                         [=](const net::CanonicalCookie& cookie) {
+                           return cookie.Name() == "embedder" &&
+                                  cookie.Domain() == kTopLevelHost &&
+                                  cookie.Path() == "/";
+                         }),
+            cookies.end());
+}
+
 class AccessContextAuditSessionRestoreBrowserTest
     : public AccessContextAuditBrowserTest {
  public:

chrome/browser/browsing_data/access_context_audit_database.cc

@@ -286,6 +286,26 @@ void AccessContextAuditDatabase::RemoveRecord(const AccessRecord& record) {
   remove_statement.Run();
 }
 
+void AccessContextAuditDatabase::RemoveAllRecords() {
+  // Perform deletions with the WHERE clause omitted to trigger the SQLite table
+  // truncation optimisation.
+  sql::Transaction transaction(&db_);
+  if (!transaction.Begin())
+    return;
+
+  std::string delete_cookies_table = "DELETE FROM ";
+  delete_cookies_table.append(kCookieTableName);
+  if (!db_.Execute(delete_cookies_table.c_str()))
+    return;
+
+  std::string delete_storage_api_table = "DELETE FROM ";
+  delete_storage_api_table.append(kStorageAPITableName);
+  if (!db_.Execute(delete_storage_api_table.c_str()))
+    return;
+
+  transaction.Commit();
+}
+
 void AccessContextAuditDatabase::RemoveSessionOnlyRecords(
     scoped_refptr<content_settings::CookieSettings> cookie_settings,
     const ContentSettingsForOneType& content_settings) {
@@ -389,6 +409,41 @@ void AccessContextAuditDatabase::RemoveAllRecordsForOriginStorage(
   remove_statement.Run();
 }
 
+void AccessContextAuditDatabase::RemoveAllRecordsForTopFrameOrigins(
+    const std::vector<url::Origin>& origins) {
+  sql::Transaction transaction(&db_);
+  if (!transaction.Begin())
+    return;
+
+  // Remove all records with a top frame origin present in |origins| from both
+  // the cookies and storage API tables.
+  std::string remove = "DELETE FROM ";
+  remove.append(kCookieTableName);
+  remove.append(" WHERE top_frame_origin = ?");
+  sql::Statement remove_cookies(
+      db_.GetCachedStatement(SQL_FROM_HERE, remove.c_str()));
+
+  remove = "DELETE FROM ";
+  remove.append(kStorageAPITableName);
+  remove.append(" WHERE top_frame_origin = ?");
+  sql::Statement remove_storage_apis(
+      db_.GetCachedStatement(SQL_FROM_HERE, remove.c_str()));
+
+  for (const auto& origin : origins) {
+    remove_storage_apis.BindString(0, origin.Serialize());
+    if (!remove_storage_apis.Run())
+      return;
+    remove_storage_apis.Reset(true);
+
+    remove_cookies.BindString(0, origin.Serialize());
+    if (!remove_cookies.Run())
+      return;
+    remove_cookies.Reset(true);
+  }
+
+  transaction.Commit();
+}
+
 std::vector<AccessContextAuditDatabase::AccessRecord>
 AccessContextAuditDatabase::GetAllRecords() {
   std::vector<AccessContextAuditDatabase::AccessRecord> records;

chrome/browser/browsing_data/access_context_audit_database.h

@@ -85,6 +85,9 @@ class AccessContextAuditDatabase
   // Removes a record from the database and from future calls to GetAllRecords.
   void RemoveRecord(const AccessRecord& record);
 
+  // Removes all records from the the database.
+  void RemoveAllRecords();
+
   // Removes all records that match the provided cookie details.
   void RemoveAllRecordsForCookie(const std::string& name,
                                  const std::string& domain,
@@ -94,6 +97,10 @@ class AccessContextAuditDatabase
   void RemoveAllRecordsForOriginStorage(const url::Origin& origin,
                                         StorageAPIType type);
 
+  // Remove all records with a top frame origin present in |origins|.
+  void RemoveAllRecordsForTopFrameOrigins(
+      const std::vector<url::Origin>& origins);
+
   // Removes all records for cookie domains and API origins that match session
   // only entries in |settings|
   void RemoveSessionOnlyRecords(

chrome/browser/browsing_data/access_context_audit_database_unittest.cc

@@ -62,6 +62,8 @@ constexpr char kManyContextsCookieName[] = "multiple contexts cookie";
 constexpr char kManyContextsCookieDomain[] = "multi-contexts.com";
 constexpr char kManyContextsCookiePath[] = "/";
 constexpr char kManyContextsStorageAPIOrigin[] = "https://many-contexts.com";
+constexpr char kManyVisitsTopFrameOrigin1[] = "https://mulitple-visits.com";
+constexpr char kManyVisitsTopFrameOrigin2[] = "https://mulitple-other.com";
 constexpr AccessContextAuditDatabase::StorageAPIType
     kManyContextsStorageAPIType =
         AccessContextAuditDatabase::StorageAPIType::kWebDatabase;
@@ -96,13 +98,13 @@ class AccessContextAuditDatabaseTest : public testing::Test {
   std::vector<AccessContextAuditDatabase::AccessRecord> GetTestRecords() {
     return {
         AccessContextAuditDatabase::AccessRecord(
-            url::Origin::Create(GURL("https://test.com")),
+            url::Origin::Create(GURL(kManyVisitsTopFrameOrigin1)),
             AccessContextAuditDatabase::StorageAPIType::kLocalStorage,
             url::Origin::Create(GURL("https://test.com")),
             base::Time::FromDeltaSinceWindowsEpoch(
                 base::TimeDelta::FromHours(1))),
         AccessContextAuditDatabase::AccessRecord(
-            url::Origin::Create(GURL("https://test2.com:8000")),
+            url::Origin::Create(GURL(kManyVisitsTopFrameOrigin2)),
             AccessContextAuditDatabase::StorageAPIType::kLocalStorage,
             url::Origin::Create(GURL("https://test.com")),
             base::Time::FromDeltaSinceWindowsEpoch(
@@ -114,14 +116,14 @@ class AccessContextAuditDatabaseTest : public testing::Test {
                 base::TimeDelta::FromHours(3)),
             /* is_persistent */ true),
         AccessContextAuditDatabase::AccessRecord(
-            url::Origin::Create(GURL("https://test2.com")),
+            url::Origin::Create(GURL(kManyVisitsTopFrameOrigin1)),
             kManyContextsCookieName, kManyContextsCookieDomain,
             kManyContextsCookiePath,
             base::Time::FromDeltaSinceWindowsEpoch(
                 base::TimeDelta::FromHours(4)),
             /* is_persistent */ true),
         AccessContextAuditDatabase::AccessRecord(
-            url::Origin::Create(GURL("https://test3.com")),
+            url::Origin::Create(GURL(kManyVisitsTopFrameOrigin2)),
             kManyContextsCookieName, kManyContextsCookieDomain,
             kManyContextsCookiePath,
             base::Time::FromDeltaSinceWindowsEpoch(
@@ -134,13 +136,13 @@ class AccessContextAuditDatabaseTest : public testing::Test {
             base::Time::FromDeltaSinceWindowsEpoch(
                 base::TimeDelta::FromHours(5))),
         AccessContextAuditDatabase::AccessRecord(
-            url::Origin::Create(GURL("https://test5.com:8000")),
+            url::Origin::Create(GURL(kManyVisitsTopFrameOrigin1)),
             kManyContextsStorageAPIType,
             url::Origin::Create(GURL(kManyContextsStorageAPIOrigin)),
             base::Time::FromDeltaSinceWindowsEpoch(
                 base::TimeDelta::FromHours(6))),
         AccessContextAuditDatabase::AccessRecord(
-            url::Origin::Create(GURL("https://test5.com:8000")),
+            url::Origin::Create(GURL(kManyVisitsTopFrameOrigin2)),
             kSingleContextStorageAPIType,
             url::Origin::Create(GURL(kManyContextsStorageAPIOrigin)),
             base::Time::FromDeltaSinceWindowsEpoch(
@@ -294,6 +296,30 @@ TEST_F(AccessContextAuditDatabaseTest, RemoveRecord) {
   EXPECT_EQ(0u, storage_api_rows);
 }
 
+TEST_F(AccessContextAuditDatabaseTest, RemoveAllRecords) {
+  // Check that removing all records deleted all entries from the database and
+  // from the database file.
+  auto test_records = GetTestRecords();
+  OpenDatabase();
+  database()->AddRecords(test_records);
+  ValidateDatabaseRecords(database(), test_records);
+  database()->RemoveAllRecords();
+  ValidateDatabaseRecords(database(), {});
+  CloseDatabase();
+
+  // Verify that everything is deleted from the database file.
+  sql::Database raw_db;
+  EXPECT_TRUE(raw_db.Open(db_path()));
+
+  size_t cookie_rows;
+  size_t storage_api_rows;
+  sql::test::CountTableRows(&raw_db, "cookies", &cookie_rows);
+  sql::test::CountTableRows(&raw_db, "originStorageAPIs", &storage_api_rows);
+
+  EXPECT_EQ(0u, cookie_rows);
+  EXPECT_EQ(0u, storage_api_rows);
+}
+
 TEST_F(AccessContextAuditDatabaseTest, RemoveAllCookieRecords) {
   // Check that all matching cookie records are removed from the database.
   auto test_records = GetTestRecords();
@@ -320,6 +346,33 @@ TEST_F(AccessContextAuditDatabaseTest, RemoveAllCookieRecords) {
   ValidateDatabaseRecords(database(), test_records);
 }
 
+TEST_F(AccessContextAuditDatabaseTest, RemoveAllRecordsForTopFrameOrigins) {
+  // Check that all records which have one of the provided origins as the top
+  // frame origin are removed correctly.
+  auto test_records = GetTestRecords();
+  OpenDatabase();
+  database()->AddRecords(test_records);
+  ValidateDatabaseRecords(database(), test_records);
+
+  std::vector<url::Origin> many_visit_origins = {
+      url::Origin::Create(GURL(kManyVisitsTopFrameOrigin1)),
+      url::Origin::Create(GURL(kManyVisitsTopFrameOrigin2))};
+  database()->RemoveAllRecordsForTopFrameOrigins(many_visit_origins);
+
+  test_records.erase(
+      std::remove_if(
+          test_records.begin(), test_records.end(),
+          [=](const AccessContextAuditDatabase::AccessRecord& record) {
+            return std::find_if(many_visit_origins.begin(),
+                                many_visit_origins.end(),
+                                [=](const url::Origin origin) {
+                                  return record.top_frame_origin == origin;
+                                }) != many_visit_origins.end();
+          }),
+      test_records.end());
+  ValidateDatabaseRecords(database(), test_records);
+}
+
 TEST_F(AccessContextAuditDatabaseTest, RemoveAllStorageRecords) {
   // Check that all records matching the provided origin and storage type
   // are removed.

chrome/browser/browsing_data/access_context_audit_service.cc

@@ -21,7 +21,8 @@ AccessContextAuditService::~AccessContextAuditService() = default;
 
 bool AccessContextAuditService::Init(
     const base::FilePath& database_dir,
-    network::mojom::CookieManager* cookie_manager) {
+    network::mojom::CookieManager* cookie_manager,
+    history::HistoryService* history_service) {
   database_ = base::MakeRefCounted<AccessContextAuditDatabase>(database_dir);
 
   // Tests may have provided a task runner already.
@@ -41,7 +42,7 @@ bool AccessContextAuditService::Init(
 
   cookie_manager->AddGlobalChangeListener(
       cookie_listener_receiver_.BindNewPipeAndPassRemote());
-
+  history_observer_.Add(history_service);
   return true;
 }
 
@@ -111,6 +112,35 @@ void AccessContextAuditService::OnCookieChange(
   }
 }
 
+void AccessContextAuditService::OnURLsDeleted(
+    history::HistoryService* history_service,
+    const history::DeletionInfo& deletion_info) {
+  if (deletion_info.IsAllHistory()) {
+    database_task_runner_->PostTask(
+        FROM_HERE, base::BindOnce(&AccessContextAuditDatabase::RemoveAllRecords,
+                                  database_));
+    return;
+  }
+
+  std::vector<url::Origin> deleted_origins;
+  // Map is of type {Origin -> {Count, LastVisitTime}}.
+  for (const auto& origin_urls_remaining :
+       deletion_info.deleted_urls_origin_map()) {
+    if (origin_urls_remaining.second.first > 0)
+      continue;
+    deleted_origins.emplace_back(
+        url::Origin::Create(origin_urls_remaining.first));
+  }
+
+  if (deleted_origins.size() > 0) {
+    database_task_runner_->PostTask(
+        FROM_HERE,
+        base::BindOnce(
+            &AccessContextAuditDatabase::RemoveAllRecordsForTopFrameOrigins,
+            database_, std::move(deleted_origins)));
+  }
+}
+
 void AccessContextAuditService::SetTaskRunnerForTesting(
     scoped_refptr<base::SequencedTaskRunner> task_runner) {
   DCHECK(!database_task_runner_);

chrome/browser/browsing_data/access_context_audit_service.h

@@ -8,6 +8,8 @@
 #include "chrome/browser/browsing_data/access_context_audit_database.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/browsing_data/content/local_shared_objects_container.h"
+#include "components/history/core/browser/history_service.h"
+#include "components/history/core/browser/history_service_observer.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "net/cookies/cookie_change_dispatcher.h"
 #include "services/network/public/mojom/cookie_manager.mojom.h"
@@ -17,17 +19,18 @@ typedef base::OnceCallback<void(
     std::vector<AccessContextAuditDatabase::AccessRecord>)>
     AccessContextRecordsCallback;
 
-class AccessContextAuditService
-    : public KeyedService,
-      public ::network::mojom::CookieChangeListener {
+class AccessContextAuditService : public KeyedService,
+                                  public ::network::mojom::CookieChangeListener,
+                                  public history::HistoryServiceObserver {
  public:
   explicit AccessContextAuditService(Profile* profile);
   ~AccessContextAuditService() override;
 
   // Initialises the Access Context Audit database in |database_dir|, and
-  // attaches listeners to |cookie_manager|.
+  // attaches listeners to |cookie_manager| and |history_service|.
   bool Init(const base::FilePath& database_dir,
-            network::mojom::CookieManager* cookie_manager);
+            network::mojom::CookieManager* cookie_manager,
+            history::HistoryService* history_service);
 
   // Records accesses for all cookies in |details| against |top_frame_origin|.
   void RecordCookieAccess(const net::CookieList& accessed_cookies,
@@ -49,6 +52,10 @@ class AccessContextAuditService
   // ::network::mojom::CookieChangeListener:
   void OnCookieChange(const net::CookieChangeInfo& change) override;
 
+  // history::HistoryServiceObserver:
+  void OnURLsDeleted(history::HistoryService* history_service,
+                     const history::DeletionInfo& deletion_info) override;
+
   // Override internal task runner with provided task runner. Must be called
   // before Init().
   void SetTaskRunnerForTesting(
@@ -68,6 +75,8 @@ class AccessContextAuditService
 
   mojo::Receiver<network::mojom::CookieChangeListener>
       cookie_listener_receiver_{this};
+  ScopedObserver<history::HistoryService, history::HistoryServiceObserver>
+      history_observer_{this};
 
   DISALLOW_COPY_AND_ASSIGN(AccessContextAuditService);
 };

chrome/browser/browsing_data/access_context_audit_service_factory.cc

@@ -7,6 +7,7 @@
 #include "base/memory/singleton.h"
 #include "chrome/browser/browsing_data/access_context_audit_service.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
+#include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_features.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
@@ -18,6 +19,7 @@ AccessContextAuditServiceFactory::AccessContextAuditServiceFactory()
           "AccessContextAuditService",
           BrowserContextDependencyManager::GetInstance()) {
   DependsOn(HostContentSettingsMapFactory::GetInstance());
+  DependsOn(HistoryServiceFactory::GetInstance());
 }
 
 AccessContextAuditServiceFactory*
@@ -38,17 +40,21 @@ KeyedService* AccessContextAuditServiceFactory::BuildServiceInstanceFor(
           features::kClientStorageAccessContextAuditing))
     return nullptr;
 
+  auto* profile = static_cast<Profile*>(context);
+
   // The service implementation will persist session cookies until next startup.
   // It is only used with regular profiles, which always persist session
   // cookies.
-  DCHECK(static_cast<Profile*>(context)->ShouldPersistSessionCookies());
+  DCHECK(profile->ShouldPersistSessionCookies());
 
   std::unique_ptr<AccessContextAuditService> context_audit_service(
-      new AccessContextAuditService(static_cast<Profile*>(context)));
+      new AccessContextAuditService(profile));
   if (!context_audit_service->Init(
           context->GetPath(),
           content::BrowserContext::GetDefaultStoragePartition(context)
-              ->GetCookieManagerForBrowserProcess())) {
+              ->GetCookieManagerForBrowserProcess(),
+          HistoryServiceFactory::GetForProfile(
+              profile, ServiceAccessType::EXPLICIT_ACCESS))) {
     return nullptr;
   }