From 3ff320950e74fecd89bdaa4fb7609762f2806929 Mon Sep 17 00:00:00 2001
From: Antoine Augusti <antoine.augusti@cds-snc.ca>
Date: Mon, 26 Apr 2021 08:40:55 -0400
Subject: [PATCH 1/2] feat: set cache headers for static assets

---
 app/__init__.py                      | 13 +++++++++----
 app/config.py                        |  1 -
 app/templates/main_template.html     |  6 +++---
 app/templates/notify_template.html   |  2 +-
 app/templates/views/signedout.html   |  4 ++--
 tests/app/main/views/test_headers.py | 17 +++++++++++++++++
 tests/app/main/views/test_index.py   |  5 ++---
 7 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/app/__init__.py b/app/__init__.py
index 4e34ff5b5b..3956b83ce6 100644
--- a/app/__init__.py
+++ b/app/__init__.py
@@ -6,6 +6,7 @@
 from functools import partial
 from numbers import Number
 from time import monotonic
+from urllib.parse import urljoin
 
 import timeago
 from flask import (
@@ -134,7 +135,7 @@ def create_app(application):
 
     application.config.from_object(configs[notify_environment])
     asset_fingerprinter._cdn_domain = application.config['ASSET_DOMAIN']
-    asset_fingerprinter._asset_root = application.config['ASSET_PATH']
+    asset_fingerprinter._asset_root = urljoin(application.config['ADMIN_BASE_URL'], application.config['ASSET_PATH'])
 
     application.config["BABEL_DEFAULT_LOCALE"] = "en"
     babel = Babel(application)
@@ -257,7 +258,6 @@ def record_start_time():
     @application.context_processor
     def inject_global_template_variables():
         return {
-            'asset_path': application.config['ASSET_PATH'],
             'header_colour': application.config['HEADER_COLOUR'],
             'asset_url': asset_fingerprinter.get_url,
             'asset_s3_url': asset_fingerprinter.get_s3_url,
@@ -648,8 +648,13 @@ def useful_headers_after_request(response):
     ))
     if 'Cache-Control' in response.headers:
         del response.headers['Cache-Control']
-    response.headers.add(
-        'Cache-Control', 'no-store, no-cache, private, must-revalidate')
+    # Cache static assets (CSS, JS, images) for a long time
+    # as they have unique hashes thanks to the asset
+    # fingerprinter
+    if request.url.startswith(asset_fingerprinter._asset_root):
+        response.headers.add('Cache-Control', 'public, max-age=31536000, immutable')
+    else:
+        response.headers.add('Cache-Control', 'no-store, no-cache, private, must-revalidate')
     for key, value in response.headers:
         response.headers[key] = SanitiseASCII.encode(value)
     return response
diff --git a/app/config.py b/app/config.py
index 6f42ffd673..52637cf06b 100644
--- a/app/config.py
+++ b/app/config.py
@@ -143,7 +143,6 @@ class Test(Development):
     ANTIVIRUS_API_HOST = 'https://test-antivirus'
     ANTIVIRUS_API_KEY = 'test-antivirus-secret'
     ASSET_DOMAIN = 'static.example.com'
-    ASSET_PATH = 'https://static.example.com/'
 
 
 class Production(Config):
diff --git a/app/templates/main_template.html b/app/templates/main_template.html
index 8baf564795..82f06f7b96 100644
--- a/app/templates/main_template.html
+++ b/app/templates/main_template.html
@@ -12,13 +12,13 @@
   <link
     rel="stylesheet"
     media="screen"
-    href="{{ asset_path }}stylesheets/index.css"
+    href="{{ asset_url('stylesheets/index.css') }}"
   />
 
 
   <link
     rel="shortcut icon"
-    href="{{ asset_path }}images/favicon.ico?0.24.2"
+    href="{{ asset_url('images/favicon.ico') }}"
     type="image/x-icon"
   />
 
@@ -34,7 +34,7 @@
   <meta name="viewport" content="width=device-width, initial-scale=1"/>
   <meta
     property="og:image"
-    content="{{ admin_base_url }}/static/images/product/notify-main.jpg"
+    content="{{ asset_url('images/product/notify-main.jpg') }}"
   />
   <script>
     window.APP_LANG = "{{ current_lang }}"
diff --git a/app/templates/notify_template.html b/app/templates/notify_template.html
index bf762b36bb..82302684ce 100644
--- a/app/templates/notify_template.html
+++ b/app/templates/notify_template.html
@@ -6,7 +6,7 @@
     <title>{% block page_title %}Notify{% endblock %}</title>
     <link
       rel="shortcut icon"
-      href="{{ asset_path }}images/favicon.ico?0.24.2"
+      href="{{ asset_url('images/favicon.ico') }}"
       type="image/x-icon"
     />
     <link
diff --git a/app/templates/views/signedout.html b/app/templates/views/signedout.html
index 17c87bc540..a33c47159d 100644
--- a/app/templates/views/signedout.html
+++ b/app/templates/views/signedout.html
@@ -11,14 +11,14 @@
   <meta property="og:description" content="{{ description }}"/>
   <meta
     property="og:image"
-    content="{{ admin_base_url }}/static/images/product/notify-main.jpg"
+    content="{{ asset_url('images/product/notify-main.jpg') }}"
   />
   <meta property="og:url" content="{{ admin_base_url }}"/>
 
   <link
     rel="stylesheet"
     media="screen"
-    href="{{ asset_path }}stylesheets/index.css"
+    href="{{ asset_url('stylesheets/index.css') }}"
   />
 
 {% endblock %}
diff --git a/tests/app/main/views/test_headers.py b/tests/app/main/views/test_headers.py
index 7334e51c95..fbafcb996d 100644
--- a/tests/app/main/views/test_headers.py
+++ b/tests/app/main/views/test_headers.py
@@ -1,3 +1,7 @@
+import pytest
+
+from app.asset_fingerprinter import asset_fingerprinter
+
 service = [{'service_id': 1, 'service_name': 'jessie the oak tree',
             'organisation_name': 'Forest', 'consent_to_research': True,
             'contact_name': 'Forest fairy', 'organisation_type': 'Ecosystem',
@@ -40,6 +44,19 @@ def test_owasp_useful_headers_set(
     )
 
 
+@pytest.mark.parametrize('url, cache_headers', [
+    (asset_fingerprinter.get_url('stylesheets/index.css'), 'public, max-age=31536000, immutable'),
+    (asset_fingerprinter.get_url('images/favicon.ico'), 'public, max-age=31536000, immutable'),
+    (asset_fingerprinter.get_url('javascripts/main.min.js'), 'public, max-age=31536000, immutable'),
+    ('/robots.txt', 'no-store, no-cache, private, must-revalidate'),
+], ids=["CSS file", "image", "JS file", "static page"])
+def test_headers_cache_static_assets(client, url, cache_headers):
+    response = client.get(url)
+
+    assert response.status_code == 200
+    assert response.headers['Cache-Control'] == cache_headers
+
+
 def test_headers_non_ascii_characters_are_replaced(
     client,
     mocker,
diff --git a/tests/app/main/views/test_index.py b/tests/app/main/views/test_index.py
index f375458746..784a871cc3 100644
--- a/tests/app/main/views/test_index.py
+++ b/tests/app/main/views/test_index.py
@@ -189,12 +189,11 @@ def test_css_is_served_from_correct_path(client_request):
     for index, link in enumerate(
         page.select('link[rel=stylesheet]')
     ):
-
         assert link['href'].startswith([
-            'https://static.example.com/stylesheets/index.css',
+            'http://localhost:6012/static/stylesheets/index.css',
             'https://fonts.googleapis.com/css?family=Lato:400,700,900&display=swap',
             'https://fonts.googleapis.com/css?',
-            'https://static.example.com/stylesheets/main.css?',
+            'http://localhost:6012/static/stylesheets/main.css?',
         ][index])
 
 

From c25ea17911931c23c3dbf1f33b92d12ecdacc3d3 Mon Sep 17 00:00:00 2001
From: Antoine Augusti <antoine.augusti@cds-snc.ca>
Date: Mon, 26 Apr 2021 09:16:54 -0400
Subject: [PATCH 2/2] Add is_static_asset to fingerprinter

---
 app/__init__.py                            |  2 +-
 app/asset_fingerprinter.py                 |  3 +++
 tests/app/main/test_asset_fingerprinter.py | 10 ++++++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/app/__init__.py b/app/__init__.py
index 3956b83ce6..ec22d2202a 100644
--- a/app/__init__.py
+++ b/app/__init__.py
@@ -651,7 +651,7 @@ def useful_headers_after_request(response):
     # Cache static assets (CSS, JS, images) for a long time
     # as they have unique hashes thanks to the asset
     # fingerprinter
-    if request.url.startswith(asset_fingerprinter._asset_root):
+    if asset_fingerprinter.is_static_asset(request.url):
         response.headers.add('Cache-Control', 'public, max-age=31536000, immutable')
     else:
         response.headers.add('Cache-Control', 'no-store, no-cache, private, must-revalidate')
diff --git a/app/asset_fingerprinter.py b/app/asset_fingerprinter.py
index 52f4606134..facf669011 100644
--- a/app/asset_fingerprinter.py
+++ b/app/asset_fingerprinter.py
@@ -52,5 +52,8 @@ def get_asset_file_contents(self, asset_file_path):
             contents = asset_file.read()
         return contents
 
+    def is_static_asset(self, url):
+        return url and url.startswith(self._asset_root)
+
 
 asset_fingerprinter = AssetFingerprinter()
diff --git a/tests/app/main/test_asset_fingerprinter.py b/tests/app/main/test_asset_fingerprinter.py
index ca4bb2faef..34f85c0c01 100644
--- a/tests/app/main/test_asset_fingerprinter.py
+++ b/tests/app/main/test_asset_fingerprinter.py
@@ -94,6 +94,16 @@ def test_s3_url(self):
 
         assert fingerprinter.get_s3_url('foo.png') == 'https://assets.example.com/static/foo.png'
 
+    def test_is_static_asset(self):
+        fingerprinter = AssetFingerprinter(
+            asset_root='https://example.com/static/',
+            cdn_domain='assets.example.com',
+        )
+
+        assert fingerprinter.is_static_asset('https://example.com/static/image.png')
+        assert not fingerprinter.is_static_asset('https://assets.example.com/image.png')
+        assert not fingerprinter.is_static_asset('https://example.com/robots.txt')
+
 
 class TestAssetFingerprintWithUnicode(object):
     def test_can_read_self(self):