From fb633fb2c09d3cbaf79299b6acdec2dfe606b27e Mon Sep 17 00:00:00 2001 From: Rob Hudson Date: Tue, 10 Sep 2024 17:41:44 -0700 Subject: [PATCH 1/2] Fix #230: Make REPORT_PERCENTAGE a float --- csp/checks.py | 2 +- csp/contrib/rate_limiting.py | 3 ++- csp/tests/test_checks.py | 2 +- csp/tests/test_contrib.py | 15 +++++++++++++++ docs/configuration.rst | 7 +++---- docs/migration-guide.rst | 8 ++++---- docs/reports.rst | 4 ++-- 7 files changed, 28 insertions(+), 13 deletions(-) diff --git a/csp/checks.py b/csp/checks.py index 0096b0e..3eba135 100644 --- a/csp/checks.py +++ b/csp/checks.py @@ -61,7 +61,7 @@ def migrate_settings() -> tuple[dict[str, Any], bool]: _REPORT_PERCENTAGE = getattr(settings, "CSP_REPORT_PERCENTAGE", None) if _REPORT_PERCENTAGE is not None: - config["REPORT_PERCENTAGE"] = round(_REPORT_PERCENTAGE * 100) + config["REPORT_PERCENTAGE"] = _REPORT_PERCENTAGE * 100 include_nonce_in = getattr(settings, "CSP_INCLUDE_NONCE_IN", []) diff --git a/csp/contrib/rate_limiting.py b/csp/contrib/rate_limiting.py index 88a3440..dd47ce2 100644 --- a/csp/contrib/rate_limiting.py +++ b/csp/contrib/rate_limiting.py @@ -23,7 +23,8 @@ def get_policy_parts(self, request: HttpRequest, response: HttpResponseBase, rep if policy is None: return policy_parts - remove_report = random.randint(0, 99) >= policy.get("REPORT_PERCENTAGE", 100) + # `random.random` returns a value in the range [0.0, 1.0) so all values will be < 100.0. + remove_report = random.random() * 100 >= policy.get("REPORT_PERCENTAGE", 100) if remove_report: if policy_parts.replace is None: policy_parts.replace = { diff --git a/csp/tests/test_checks.py b/csp/tests/test_checks.py index 9fc52cd..c4a422b 100644 --- a/csp/tests/test_checks.py +++ b/csp/tests/test_checks.py @@ -13,7 +13,7 @@ def test_migrate_settings() -> None: config, report_only = migrate_settings() assert config == { - "REPORT_PERCENTAGE": 25, + "REPORT_PERCENTAGE": 25.0, "EXCLUDE_URL_PREFIXES": ["/admin/"], "DIRECTIVES": {"default-src": ["'self'", "example.com"]}, } diff --git a/csp/tests/test_contrib.py b/csp/tests/test_contrib.py index c8516e6..dc89048 100644 --- a/csp/tests/test_contrib.py +++ b/csp/tests/test_contrib.py @@ -25,6 +25,21 @@ def test_report_percentage() -> None: assert 400 <= times_seen <= 600 +@override_settings(CONTENT_SECURITY_POLICY={"REPORT_PERCENTAGE": 9.9, "DIRECTIVES": {"report-uri": "x"}}) +def test_report_percentage_float() -> None: + times_seen = 0 + for _ in range(5000): + 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 + # 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 diff --git a/docs/configuration.rst b/docs/configuration.rst index f37de21..e412a52 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -110,10 +110,9 @@ policy. on the same origin. ``REPORT_PERCENTAGE`` - Percentage of requests that should see the ``report-uri`` directive. - Use this to throttle the number of CSP violation reports made to your - ``report-uri``. An **integer** between 0 and 100 (0 = no reports at all). - Ignored if ``report-uri`` isn't set. + Percentage of requests that should see the ``report-uri`` directive. Use this to throttle the + number of CSP violation reports made to your ``report-uri``. A **float** between 0.0 and 100.0 + (0.0 = no reports at all, 100.0 = always report). Ignored if ``report-uri`` isn't set. ``DIRECTIVES`` A dictionary of policy directives. Each key in the dictionary is a directive and the value is a diff --git a/docs/migration-guide.rst b/docs/migration-guide.rst index f79a15c..b5b2f27 100644 --- a/docs/migration-guide.rst +++ b/docs/migration-guide.rst @@ -130,14 +130,14 @@ The new settings would be: .. note:: - If you were using the ``CSP_REPORT_PERCENTAGE`` setting, this should be updated to be an integer - percentage and not a decimal value in the new settings format. For example, if you had - ``CSP_REPORT_PERCENTAGE = 0.1``, this should be updated to: + If you were using the ``CSP_REPORT_PERCENTAGE`` setting, this should be updated to be a float + percentage between 0.0 and 100.0. For example, if you had ``CSP_REPORT_PERCENTAGE = 0.1``, this + should be updated to ``10.0`` to represent 10% of CSP errors will be reported: .. code-block:: python CONTENT_SECURITY_POLICY = { - "REPORT_PERCENTAGE": 10, + "REPORT_PERCENTAGE": 10.0, "DIRECTIVES": { "report-uri": "/csp-report/", # ... diff --git a/docs/reports.rst b/docs/reports.rst index 8337d58..a66692f 100644 --- a/docs/reports.rst +++ b/docs/reports.rst @@ -20,7 +20,7 @@ and set the ``REPORT_PERCENTAGE`` option: ``REPORT_PERCENTAGE`` Percentage of requests that should see the ``report-uri`` directive. Use this to throttle the - number of CSP violation reports made to your ``report-uri``. An **integer** between 0 and 100 (0 - = no reports at all). Ignored if ``report-uri`` isn't set. + number of CSP violation reports made to your ``report-uri``. A **float** between 0.0 and 100.0 + (0.0 = no reports at all, 100.0 = always report). Ignored if ``report-uri`` isn't set. .. _report: http://www.w3.org/TR/CSP/#sample-violation-report From e591149763d96e64e4bf19cca05a97e2310de44a Mon Sep 17 00:00:00 2001 From: Rob Hudson Date: Tue, 10 Sep 2024 17:50:35 -0700 Subject: [PATCH 2/2] Remove ignore comment after django-stubs fix django.core.checks.registry.CheckRegistry.register is now typed correctly. --- csp/apps.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/csp/apps.py b/csp/apps.py index 38e3103..2013782 100644 --- a/csp/apps.py +++ b/csp/apps.py @@ -8,6 +8,4 @@ class CspConfig(AppConfig): name = "csp" def ready(self) -> None: - # Ignore known issue typeddjango/django-stubs #2232 - # The overload of CheckRegistry.register as a function is incomplete - checks.register(check_django_csp_lt_4_0, checks.Tags.security) # type: ignore + checks.register(check_django_csp_lt_4_0, checks.Tags.security)