From 1480ec3d18cf9e90ddb92241c11300d4e59db9d6 Mon Sep 17 00:00:00 2001
From: spatel033 <177532680+spatel033@users.noreply.github.com>
Date: Wed, 4 Dec 2024 17:31:20 +0000
Subject: [PATCH] Refactor csp_nonce

---
 app/__init__.py                               | 11 +++++++++-
 app/templates/admin_template.html             |  4 +++-
 .../components/webauthn-api-check.html        |  2 +-
 app/templates/unbranded_template.html         |  2 ++
 .../views/email-link-interstitial.html        |  2 +-
 tests/app/main/views/test_headers.py          | 11 ++++++++--
 tests/conftest.py                             | 21 ++++++++++++++++++-
 7 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/app/__init__.py b/app/__init__.py
index 498dada8b8..d02c724335 100644
--- a/app/__init__.py
+++ b/app/__init__.py
@@ -1,5 +1,6 @@
 import os
 import pathlib
+import secrets
 from collections.abc import Callable
 from time import monotonic
 
@@ -219,6 +220,7 @@ def init_app(application):
     application.after_request(useful_headers_after_request)
 
     # Load user first (as we want user_id to be available for all calls to API, which service+organisation might make.
+    application.before_request(make_nonce_before_request)
     application.before_request(load_user_id_before_request)
     application.before_request(load_service_before_request)
     application.before_request(load_organisation_before_request)
@@ -336,6 +338,12 @@ def load_user_id_before_request():
     g.user_id = get_user_id_from_flask_login_session()
 
 
+def make_nonce_before_request():
+    # `govuk_frontend_jinja/template.html` can be extended and inline `<script>` can be added without CSP complaining
+    if not getattr(request, "csp_nonce", None):
+        request.csp_nonce = secrets.token_urlsafe(16)
+
+
 #  https://www.owasp.org/index.php/List_of_useful_HTTP_headers
 def useful_headers_after_request(response):
     response.headers.add("X-Content-Type-Options", "nosniff")
@@ -344,7 +352,7 @@ def useful_headers_after_request(response):
         "Content-Security-Policy",
         (
             "default-src 'self' {asset_domain} 'unsafe-inline';"
-            "script-src 'self' {asset_domain} 'unsafe-inline' 'unsafe-eval' data:;"
+            "script-src 'self' {asset_domain} 'nonce-{csp_nonce}';"
             "connect-src 'self';"
             "object-src 'self';"
             "font-src 'self' {asset_domain} data:;"
@@ -355,6 +363,7 @@ def useful_headers_after_request(response):
             "frame-src 'self';".format(
                 asset_domain=current_app.config["ASSET_DOMAIN"],
                 logo_domain=current_app.config["LOGO_CDN_DOMAIN"],
+                csp_nonce=getattr(request, "csp_nonce", ""),
             )
         ),
     )
diff --git a/app/templates/admin_template.html b/app/templates/admin_template.html
index 69a03957ce..2ef362255f 100644
--- a/app/templates/admin_template.html
+++ b/app/templates/admin_template.html
@@ -1,5 +1,7 @@
 {% extends "govuk_frontend_jinja/template.html"%}
 
+{% set cspNonce = request.csp_nonce %}
+
 {% block headIcons %}
   <link rel="icon" sizes="48x48" href="{{ asset_url('images/favicon.ico') }}">
   <link rel="icon" sizes="any" href="{{ asset_url('images/favicon.svg') }}" type="image/svg+xml">
@@ -134,5 +136,5 @@
   {% endblock %}
 
   <script type="module" src="{{ asset_url('javascripts/all-esm.mjs') }}"></script>
-  <script type="text/javascript" src="{{ asset_url('javascripts/all.js') }}"></script>
+  <script nonce="{{ request.csp_nonce }}" type="text/javascript" src="{{ asset_url('javascripts/all.js') }}"></script>
 {% endblock %}
diff --git a/app/templates/components/webauthn-api-check.html b/app/templates/components/webauthn-api-check.html
index 19bc56bc5e..170d568e83 100644
--- a/app/templates/components/webauthn-api-check.html
+++ b/app/templates/components/webauthn-api-check.html
@@ -1,5 +1,5 @@
 {% macro webauthn_api_check() %}
-  <script>
+  <script nonce="{{ request.csp_nonce }}">
     if ('credentials' in window.navigator) {
       document.body.className = ((document.body.className) ? document.body.className + ' webauthn-api-enabled' : 'webauthn-api-enabled');
     }
diff --git a/app/templates/unbranded_template.html b/app/templates/unbranded_template.html
index cfac4f6efe..65d6746d78 100644
--- a/app/templates/unbranded_template.html
+++ b/app/templates/unbranded_template.html
@@ -1,5 +1,7 @@
 {% extends "govuk_frontend_jinja/template.html"%}
 
+{% set cspNonce = request.csp_nonce %}
+
 {% block headIcons %}
 {% endblock %}
 
diff --git a/app/templates/views/email-link-interstitial.html b/app/templates/views/email-link-interstitial.html
index a0b41c965e..4c2aac4e3a 100644
--- a/app/templates/views/email-link-interstitial.html
+++ b/app/templates/views/email-link-interstitial.html
@@ -18,7 +18,7 @@
 
 </div>
 
-<script type="text/javascript">
+<script nonce="{{ request.csp_nonce }}" type="text/javascript">
   document.getElementById("use-email-auth").submit();
 </script>
 
diff --git a/tests/app/main/views/test_headers.py b/tests/app/main/views/test_headers.py
index 1bebd37dce..9d55e0e9dc 100644
--- a/tests/app/main/views/test_headers.py
+++ b/tests/app/main/views/test_headers.py
@@ -1,9 +1,13 @@
 def test_owasp_useful_headers_set(
     client_request,
+    mocker,
     mock_get_service_and_organisation_counts,
     mock_get_letter_rates,
     mock_get_sms_rate,
+    fake_nonce,
 ):
+    mocker.patch("secrets.token_urlsafe", return_value=fake_nonce)
+
     client_request.logout()
     response = client_request.get_response(".index")
 
@@ -11,7 +15,7 @@ def test_owasp_useful_headers_set(
     assert response.headers["X-XSS-Protection"] == "1; mode=block"
     assert response.headers["Content-Security-Policy"] == (
         "default-src 'self' static.example.com 'unsafe-inline';"
-        "script-src 'self' static.example.com 'unsafe-inline' 'unsafe-eval' data:;"
+        "script-src 'self' static.example.com 'nonce-TESTs5Vr8v3jgRYLoQuVwA';"
         "connect-src 'self';"
         "object-src 'self';"
         "font-src 'self' static.example.com data:;"
@@ -43,7 +47,10 @@ def test_headers_non_ascii_characters_are_replaced(
     mock_get_service_and_organisation_counts,
     mock_get_letter_rates,
     mock_get_sms_rate,
+    fake_nonce,
 ):
+    mocker.patch("secrets.token_urlsafe", return_value=fake_nonce)
+
     client_request.logout()
     mocker.patch.dict(
         "app.current_app.config",
@@ -54,7 +61,7 @@ def test_headers_non_ascii_characters_are_replaced(
 
     assert response.headers["Content-Security-Policy"] == (
         "default-src 'self' static.example.com 'unsafe-inline';"
-        "script-src 'self' static.example.com 'unsafe-inline' 'unsafe-eval' data:;"
+        "script-src 'self' static.example.com 'nonce-TESTs5Vr8v3jgRYLoQuVwA';"
         "connect-src 'self';"
         "object-src 'self';"
         "font-src 'self' static.example.com data:;"
diff --git a/tests/conftest.py b/tests/conftest.py
index 14894cb725..7414738125 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -2899,7 +2899,7 @@ def os_environ():
 
 
 @pytest.fixture  # noqa (C901 too complex)
-def client_request(request, _logged_in_client, mocker, service_one):  # noqa (C901 too complex)
+def client_request(request, _logged_in_client, mocker, service_one, fake_nonce):  # noqa (C901 too complex)
     def block_method(object, method_name, preferred_method_name):
         def blocked_method(*args, **kwargs):
             raise AttributeError(
@@ -2962,6 +2962,8 @@ def get_url(
         ):
             from flask.templating import _render
 
+            mocker.patch("secrets.token_urlsafe", return_value=fake_nonce)
+
             with mock.patch("flask.templating._render", wraps=_render) as mock_render:
                 resp = _logged_in_client.get(
                     url,
@@ -3007,6 +3009,8 @@ def get_url(
             if _test_for_non_smart_quotes:
                 ClientRequest.test_for_non_smart_quotes(page)
 
+            ClientRequest.test_for_script_csp_nonce(page)
+
             return page
 
         @staticmethod
@@ -3149,6 +3153,16 @@ def test_for_non_smart_quotes(page):
                     "'" in el.text or '"' in el.text
                 ), f"Non-smart quote or apostrophe found in <{el.name}>: {normalize_spaces(el.text)}"
 
+        @staticmethod
+        def test_for_script_csp_nonce(page):
+            for script_tag in page.select("script"):
+                src = script_tag.get("src")
+                nonce = script_tag.get("nonce")
+                if src and "javascripts/all-esm.mjs" in src:
+                    assert nonce is None
+                else:
+                    assert nonce == fake_nonce
+
     return ClientRequest
 
 
@@ -4374,3 +4388,8 @@ def mock_get_notifications_count_for_service(mocker):
         "app.notification_api_client.get_notifications_count_for_service",
         return_value=100,
     )
+
+
+@pytest.fixture(scope="function")
+def fake_nonce():
+    return "TESTs5Vr8v3jgRYLoQuVwA"