From cf2ae8ce708e0d964793e16bf20d28f6e02d36bd Mon Sep 17 00:00:00 2001
From: Rob Hudson <robhudson@mozilla.com>
Date: Tue, 9 Jul 2024 15:57:41 -0700
Subject: [PATCH] Fix #231: report percentage of 100% should always report

This also updates the RateLimited CSPMiddleware to remove both
`report-uri` and `report-to` directives based on report percentage.
---
 csp/contrib/rate_limiting.py | 22 ++++++++++++++++------
 csp/tests/test_contrib.py    | 16 ++++++++++++++++
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/csp/contrib/rate_limiting.py b/csp/contrib/rate_limiting.py
index 9633a87..c6a54f6 100644
--- a/csp/contrib/rate_limiting.py
+++ b/csp/contrib/rate_limiting.py
@@ -28,9 +28,14 @@ def build_policy(self, request: HttpRequest, response: HttpResponseBase) -> str:
             return ""
 
         report_percentage = policy.get("REPORT_PERCENTAGE", 100)
-        include_report_uri = random.randint(0, 100) < report_percentage
-        if not include_report_uri:
-            replace["report-uri"] = None
+        remove_report = random.randint(0, 99) >= report_percentage
+        if remove_report:
+            replace.update(
+                {
+                    "report-uri": None,
+                    "report-to": None,
+                }
+            )
 
         return build_policy(config=config, update=update, replace=replace, nonce=nonce)
 
@@ -46,8 +51,13 @@ def build_policy_ro(self, request: HttpRequest, response: HttpResponseBase) -> s
             return ""
 
         report_percentage = policy.get("REPORT_PERCENTAGE", 100)
-        include_report_uri = random.randint(0, 100) < report_percentage
-        if not include_report_uri:
-            replace["report-uri"] = None
+        remove_report = random.randint(0, 99) >= report_percentage
+        if remove_report:
+            replace.update(
+                {
+                    "report-uri": None,
+                    "report-to": None,
+                }
+            )
 
         return build_policy(config=config, update=update, replace=replace, nonce=nonce, report_only=True)
diff --git a/csp/tests/test_contrib.py b/csp/tests/test_contrib.py
index c62e767..c8516e6 100644
--- a/csp/tests/test_contrib.py
+++ b/csp/tests/test_contrib.py
@@ -19,10 +19,26 @@ def test_report_percentage() -> None:
         mw.process_response(request, response)
         if "report-uri" in response[HEADER]:
             times_seen += 1
+        if "report-to" in response[HEADER]:
+            times_seen += 1
     # Roughly 10%
     assert 400 <= times_seen <= 600
 
 
+@override_settings(CONTENT_SECURITY_POLICY={"REPORT_PERCENTAGE": 100, "DIRECTIVES": {"report-uri": "x"}})
+def test_report_percentage_100() -> None:
+    times_seen = 0
+    for _ in range(1000):
+        request = rf.get("/")
+        response = HttpResponse()
+        mw.process_response(request, response)
+        if "report-uri" in response[HEADER]:
+            times_seen += 1
+        if "report-to" in response[HEADER]:
+            times_seen += 1
+    assert times_seen == 1000
+
+
 @override_settings(CONTENT_SECURITY_POLICY_REPORT_ONLY={"REPORT_PERCENTAGE": 10, "DIRECTIVES": {"report-uri": "x"}})
 def test_report_percentage_report_only() -> None:
     times_seen = 0