Require presence of debug cookie at source registrations for trigger

verbose debug reports

WICG/attribution-reporting-api#1088

The debug cookie setting needs to be checked and stored for all sources
as neither source debug key nor source debug reporting is required
for trigger verbose debug reports, and each source could be potentially
matched to a trigger registration with debug reporting.

Note that we do not need a DB migration, instead we set the
"debug_cookie_set" value to whether the debug key was set for the
source if the corresponding field is not found in the proto msg. The
debug key should only be set when the debug cookie was set at the source
registration.

We expose the field in the Attribution Internals UI as well.

Bug: 1496976
Change-Id: Ie8dfbda3f6faec78c93062376ff883c180062c2c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4984648
Reviewed-by: Andrew Paseltiner <apaseltiner@chromium.org>
Reviewed-by: John Delaney <johnidel@chromium.org>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Commit-Queue: Nan Lin <linnan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1217136}

content/browser/attribution_reporting/attribution_aggregatable_report_golden_unittest.cc

@@ -344,6 +344,7 @@ TEST_F(AttributionAggregatableReportGoldenLatestVersionTest,
                SourceBuilder(
                    base::Time::FromMillisecondsSinceUnixEpoch(1234483200000))
                    .SetDebugKey(123)
+                   .SetDebugCookieSet(true)
                    .BuildStored())
                .SetAggregatableHistogramContributions(
                    {AggregatableHistogramContribution(/*key=*/1, /*value=*/2)})
@@ -371,6 +372,7 @@ TEST_F(AttributionAggregatableReportGoldenLatestVersionTest,
                SourceBuilder(
                    base::Time::FromMillisecondsSinceUnixEpoch(1234483300000))
                    .SetDebugKey(123)
+                   .SetDebugCookieSet(true)
                    .BuildStored())
                .SetAggregatableHistogramContributions(
                    {AggregatableHistogramContribution(/*key=*/1, /*value=*/2),
@@ -399,6 +401,7 @@ TEST_F(AttributionAggregatableReportGoldenLatestVersionTest,
                      SourceBuilder(base::Time::FromMillisecondsSinceUnixEpoch(
                                        1234483400000))
                          .SetDebugKey(123)
+                         .SetDebugCookieSet(true)
                          .BuildStored())
                      .SetAggregatableHistogramContributions(
                          {AggregatableHistogramContribution(
@@ -456,6 +459,7 @@ TEST_F(AttributionAggregatableReportGoldenLatestVersionTest,
                SourceBuilder(
                    base::Time::FromMillisecondsSinceUnixEpoch(1234483200000))
                    .SetDebugKey(123)
+                   .SetDebugCookieSet(true)
                    .BuildStored())
                .SetAggregatableHistogramContributions(
                    {AggregatableHistogramContribution(/*key=*/1, /*value=*/2)})
@@ -485,6 +489,7 @@ TEST_F(AttributionAggregatableReportGoldenLatestVersionTest,
                SourceBuilder(
                    base::Time::FromMillisecondsSinceUnixEpoch(1234483300000))
                    .SetDebugKey(123)
+                   .SetDebugCookieSet(true)
                    .BuildStored())
                .SetAggregatableHistogramContributions(
                    {AggregatableHistogramContribution(/*key=*/1, /*value=*/2),
@@ -515,6 +520,7 @@ TEST_F(AttributionAggregatableReportGoldenLatestVersionTest,
                      SourceBuilder(base::Time::FromMillisecondsSinceUnixEpoch(
                                        1234483400000))
                          .SetDebugKey(123)
+                         .SetDebugCookieSet(true)
                          .BuildStored())
                      .SetAggregatableHistogramContributions(
                          {AggregatableHistogramContribution(

content/browser/attribution_reporting/attribution_debug_report.cc

@@ -490,6 +490,10 @@ absl::optional<AttributionDebugReport> AttributionDebugReport::Create(
     return absl::nullopt;
   }
 
+  if (is_debug_cookie_set && result.source()) {
+    is_debug_cookie_set = result.source()->debug_cookie_set();
+  }
+
   base::Value::List report_body;
   base::Time original_report_time;
 

content/browser/attribution_reporting/attribution_internals.mojom

@@ -107,6 +107,7 @@ struct WebUISource {
   uint64 aggregatable_budget_consumed;
   array<uint64> aggregatable_dedup_keys;
   attribution_reporting.mojom.TriggerConfig trigger_config;
+  bool debug_cookie_set;
 
   enum Attributability {
     kAttributable,

content/browser/attribution_reporting/attribution_internals_browsertest.cc

@@ -308,6 +308,7 @@ IN_PROC_BROWSER_TEST_F(AttributionInternalsWebUiBrowserTest,
               .SetSourceEventId(std::numeric_limits<uint64_t>::max())
               .SetAttributionLogic(StoredSource::AttributionLogic::kNever)
               .SetDebugKey(19)
+              .SetDebugCookieSet(true)
               .SetDestinationSites({
                   net::SchemefulSite::Deserialize("https://a.test"),
                   net::SchemefulSite::Deserialize("https://b.test"),
@@ -386,12 +387,14 @@ IN_PROC_BROWSER_TEST_F(AttributionInternalsWebUiBrowserTest,
           table.children[1].children[15]?.innerText === '1300 / 65536' &&
           table.children[0].children[16]?.innerText === '19' &&
           table.children[1].children[16]?.innerText === '' &&
-          table.children[0].children[17]?.innerText === '' &&
-          table.children[1].children[17]?.children[0]?.children[0]?.innerText === '13' &&
-          table.children[1].children[17]?.children[0]?.children[1]?.innerText === '17' &&
+          table.children[0].children[17]?.innerText === 'true' &&
+          table.children[1].children[17]?.innerText === 'false' &&
           table.children[0].children[18]?.innerText === '' &&
-          table.children[1].children[18]?.children[0]?.children[0]?.innerText === '14' &&
-          table.children[1].children[18]?.children[0]?.children[1]?.innerText === '18' &&
+          table.children[1].children[18]?.children[0]?.children[0]?.innerText === '13' &&
+          table.children[1].children[18]?.children[0]?.children[1]?.innerText === '17' &&
+          table.children[0].children[19]?.innerText === '' &&
+          table.children[1].children[19]?.children[0]?.children[0]?.innerText === '14' &&
+          table.children[1].children[19]?.children[0]?.children[1]?.innerText === '18' &&
           table.children[0].children[1]?.innerText === 'Unattributable: noised with no reports' &&
           table.children[1].children[1]?.innerText === 'Attributable' &&
           table.children[2].children[1]?.innerText === 'Attributable: reached event-level attribution limit' &&

content/browser/attribution_reporting/attribution_internals_handler_impl.cc

@@ -91,7 +91,7 @@ attribution_internals::mojom::WebUISourcePtr WebUISource(
                 attribution_reporting::HexEncodeAggregationKey(key.second));
           }),
       source.aggregatable_budget_consumed(), source.aggregatable_dedup_keys(),
-      source.trigger_config(), attributability);
+      source.trigger_config(), source.debug_cookie_set(), attributability);
 }
 
 void ForwardSourcesToWebUI(

content/browser/attribution_reporting/attribution_manager_impl.cc

@@ -771,10 +771,7 @@ void AttributionManagerImpl::ProcessEvents() {
     absl::visit(
         base::Overloaded{
             [&](const StorableSource& source) {
-              cookie_origin = source.registration().debug_key.has_value() ||
-                                      source.registration().debug_reporting
-                                  ? &source.common_info().reporting_origin()
-                                  : nullptr;
+              cookie_origin = &source.common_info().reporting_origin();
               source_origin = &*source.common_info().source_origin();
               operation = ContentBrowserClient::AttributionReportingOperation::
                   kSourceTransitionalDebugReporting;
@@ -839,7 +836,7 @@ void AttributionManagerImpl::StoreSource(StorableSource source,
   }
 
   attribution_storage_.AsyncCall(&AttributionStorage::StoreSource)
-      .WithArgs(source)
+      .WithArgs(source, is_debug_cookie_set)
       .Then(base::BindOnce(&AttributionManagerImpl::OnSourceStored,
                            weak_factory_.GetWeakPtr(), std::move(source),
                            cleared_debug_key, is_debug_cookie_set));

content/browser/attribution_reporting/attribution_manager_impl_unittest.cc

@@ -1818,7 +1818,9 @@ TEST_F(AttributionManagerImplTest,
                 ContentBrowserClient::AttributionReportingOperation::
                     kSourceVerboseDebugReport,
                 ContentBrowserClient::AttributionReportingOperation::
-                    kTriggerVerboseDebugReport),
+                    kTriggerVerboseDebugReport,
+                ContentBrowserClient::AttributionReportingOperation::
+                    kSourceTransitionalDebugReporting),
           _, _, _, _))
       .WillRepeatedly(Return(true));
   EXPECT_CALL(
@@ -1857,7 +1859,9 @@ TEST_F(AttributionManagerImplTest, EmbedderDisallowsReporting_ReportNotSent) {
                 ContentBrowserClient::AttributionReportingOperation::
                     kSourceVerboseDebugReport,
                 ContentBrowserClient::AttributionReportingOperation::
-                    kTriggerVerboseDebugReport),
+                    kTriggerVerboseDebugReport,
+                ContentBrowserClient::AttributionReportingOperation::
+                    kSourceTransitionalDebugReporting),
           _, _, _, _))
       .WillRepeatedly(Return(true));
   EXPECT_CALL(
@@ -2286,6 +2290,7 @@ const struct {
   absl::optional<uint64_t> expected_debug_key;
   absl::optional<uint64_t> expected_cleared_key;
   bool cookie_access_allowed;
+  bool expected_debug_cookie_set;
 } kDebugKeyTestCases[] = {
     {
         "no debug key, no cookie",
@@ -2294,6 +2299,7 @@ const struct {
         absl::nullopt,
         absl::nullopt,
         true,
+        false,
     },
     {
         "has debug key, no cookie",
@@ -2302,6 +2308,7 @@ const struct {
         absl::nullopt,
         123,
         true,
+        false,
     },
     {
         "no debug key, has cookie",
@@ -2310,6 +2317,7 @@ const struct {
         absl::nullopt,
         absl::nullopt,
         true,
+        true,
     },
     {
         "has debug key, has cookie",
@@ -2318,6 +2326,7 @@ const struct {
         123,
         absl::nullopt,
         true,
+        true,
     },
     {
         "has debug key, no cookie access",
@@ -2326,6 +2335,7 @@ const struct {
         absl::nullopt,
         123,
         false,
+        false,
     },
 };
 
@@ -2353,15 +2363,13 @@ TEST_F(AttributionManagerImplTest, HandleSource_DebugKey) {
                       kSourceVerboseDebugReport),
             _, _, IsNull(), Pointee(*reporting_origin)))
         .WillRepeatedly(Return(true));
-    if (test_case.input_debug_key) {
       EXPECT_CALL(browser_client,
                   IsAttributionReportingOperationAllowed(
                       _,
                       ContentBrowserClient::AttributionReportingOperation::
                           kSourceTransitionalDebugReporting,
                       _, _, IsNull(), Pointee(*reporting_origin)))
           .WillOnce(Return(test_case.cookie_access_allowed));
-    }
     ScopedContentBrowserClientSetting setting(&browser_client);
 
     EXPECT_CALL(observer, OnSourceHandled(_, base::Time::Now(),
@@ -2374,8 +2382,11 @@ TEST_F(AttributionManagerImplTest, HandleSource_DebugKey) {
             .Build(),
         kFrameId);
 
-    EXPECT_THAT(StoredSources(),
-                ElementsAre(SourceDebugKeyIs(test_case.expected_debug_key)))
+    EXPECT_THAT(
+        StoredSources(),
+        ElementsAre(
+            AllOf(SourceDebugKeyIs(test_case.expected_debug_key),
+                  SourceDebugCookieSetIs(test_case.expected_debug_cookie_set))))
         << test_case.name;
 
     attribution_manager_->ClearData(base::Time::Min(), base::Time::Max(),
@@ -2409,7 +2420,9 @@ TEST_F(AttributionManagerImplTest, HandleTrigger_DebugKey) {
                       kSourceVerboseDebugReport,
                   ContentBrowserClient::AttributionReportingOperation::kTrigger,
                   ContentBrowserClient::AttributionReportingOperation::
-                      kTriggerVerboseDebugReport),
+                      kTriggerVerboseDebugReport,
+                  ContentBrowserClient::AttributionReportingOperation::
+                      kSourceTransitionalDebugReporting),
             _, _, _, Pointee(*reporting_origin)))
         .WillRepeatedly(Return(true));
     if (test_case.input_debug_key) {

content/browser/attribution_reporting/attribution_report_unittest.cc

@@ -211,6 +211,7 @@ TEST(AttributionReportTest, ReportBody_DebugKeys) {
                       SourceBuilder(base::Time::UnixEpoch())
                           .SetSourceEventId(100)
                           .SetDebugKey(test_case.source_debug_key)
+                          .SetDebugCookieSet(true)
                           .SetRandomizedResponseRate(0.2)
                           .BuildStored())
             .SetTriggerData(5)

content/browser/attribution_reporting/attribution_reporting.proto

@@ -43,6 +43,8 @@ message AttributionReadOnlySourceData {
   };
 
   optional TriggerDataMatching trigger_data_matching = 5;
+
+  optional bool debug_cookie_set = 6;
 }
 
 message AttributionEventLevelMetadata {

content/browser/attribution_reporting/attribution_storage.h

@@ -45,7 +45,11 @@ class AttributionStorage {
   // pair. When a source is stored, all matching sources that have already
   // converted are marked as inactive, and are no longer eligible for reporting.
   // Unconverted matching sources are not modified.
-  virtual StoreSourceResult StoreSource(const StorableSource& source) = 0;
+  //
+  // TODO(linnan): Remove default argument for `debug_cookie_set`.
+  // Alternatively, consider making this a field in `StorableSource`.
+  virtual StoreSourceResult StoreSource(const StorableSource& source,
+                                        bool debug_cookie_set = false) = 0;
 
   // Finds all stored sources matching a given `trigger`, and stores the
   // new associated report. Only active sources will receive new attributions.

content/browser/attribution_reporting/attribution_storage_sql.cc

@@ -592,6 +592,12 @@ AttributionStorageSql::ReadSourceFromStatement(sql::Statement& statement) {
           : delegate_->GetRandomizedResponseRate(
                 *source_type, *event_report_windows, max_event_level_reports);
 
+  // If "debug_cookie_set" field was not set in earlier versions, set the value
+  // to whether the debug key was set for the source.
+  bool debug_cookie_set = read_only_source_data_msg->has_debug_cookie_set()
+                              ? read_only_source_data_msg->debug_cookie_set()
+                              : debug_key.has_value();
+
   static constexpr char kDestinationSitesSql[] =
       "SELECT destination_site "
       "FROM source_destinations "
@@ -634,7 +640,8 @@ AttributionStorageSql::ReadSourceFromStatement(sql::Statement& statement) {
       max_event_level_reports, priority, std::move(*filter_data), debug_key,
       std::move(*aggregation_keys), *attribution_logic, *active_state,
       source_id, aggregatable_budget_consumed, randomized_response_rate,
-      attribution_reporting::TriggerConfig(trigger_data_matching));
+      attribution_reporting::TriggerConfig(trigger_data_matching),
+      debug_cookie_set);
   if (!stored_source.has_value()) {
     return absl::nullopt;
   }
@@ -726,9 +733,12 @@ bool AttributionStorageSql::DeactivateSources(
 }
 
 StoreSourceResult AttributionStorageSql::StoreSource(
-    const StorableSource& source) {
+    const StorableSource& source,
+    bool debug_cookie_set) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
+  CHECK(!source.registration().debug_key.has_value() || debug_cookie_set);
+
   // Force the creation of the database if it doesn't exist, as we need to
   // persist the source.
   if (!LazyInit(DbCreationPolicy::kCreateIfAbsent)) {
@@ -889,10 +899,11 @@ StoreSourceResult AttributionStorageSql::StoreSource(
 
   statement.BindBlob(14, SerializeAggregationKeys(reg.aggregation_keys));
   statement.BindBlob(15, SerializeFilterData(reg.filter_data));
-  statement.BindBlob(16,
-                     SerializeReadOnlySourceData(
-                         reg.event_report_windows, reg.max_event_level_reports,
-                         randomized_response_data.rate(), &reg.trigger_config));
+  statement.BindBlob(
+      16, SerializeReadOnlySourceData(reg.event_report_windows,
+                                      reg.max_event_level_reports,
+                                      randomized_response_data.rate(),
+                                      &reg.trigger_config, &debug_cookie_set));
 
   if (!statement.Run()) {
     return StoreSourceResult(StorableSource::Result::kInternalError);
@@ -921,7 +932,7 @@ StoreSourceResult AttributionStorageSql::StoreSource(
       reg.priority, reg.filter_data, reg.debug_key, reg.aggregation_keys,
       attribution_logic, *active_state, source_id,
       /*aggregatable_budget_consumed=*/0, randomized_response_data.rate(),
-      reg.trigger_config);
+      reg.trigger_config, debug_cookie_set);
 
   if (!stored_source.has_value() ||
       !rate_limit_table_.AddRateLimitForSource(&db_, *stored_source)) {

content/browser/attribution_reporting/attribution_storage_sql.h

@@ -130,7 +130,8 @@ class CONTENT_EXPORT AttributionStorageSql : public AttributionStorage {
   };
 
   // AttributionStorage:
-  StoreSourceResult StoreSource(const StorableSource& source) override;
+  StoreSourceResult StoreSource(const StorableSource& source,
+                                bool debug_cookie_set) override;
   CreateReportResult MaybeCreateAndStoreReport(
       const AttributionTrigger& trigger) override;
   std::vector<AttributionReport> GetAttributionReports(

content/browser/attribution_reporting/attribution_storage_sql_migrations.cc

@@ -328,7 +328,8 @@ bool To56(sql::Database& db) {
         0, SerializeReadOnlySourceData(*event_report_windows,
                                        max_event_level_reports,
                                        /*randomized_response_rate=*/-1,
-                                       /*trigger_config=*/nullptr));
+                                       /*trigger_config=*/nullptr,
+                                       /*debug_cookie_set=*/nullptr));
     set_statement.BindInt64(1, id);
     if (!set_statement.Run()) {
       return false;