Replace logo selection route with per-request global variable

Getting the logo URL via the /org-logo route's redirect meant that the
logo had to be retrieved with every page load, which of course over
Tor can take several seconds. Selecting the logo at the start of each
request and storing its URL in the Flask globals allows the static URL
to be used in the templates, with the default caching of static
assets, eliminating the extra requests.

securedrop/journalist_app/__init__.py

@@ -1,6 +1,8 @@
 # -*- coding: utf-8 -*-
 
 from datetime import datetime, timedelta
+from pathlib import Path
+
 from flask import (Flask, session, redirect, url_for, flash, g, request,
                    render_template)
 from flask_assets import Environment
@@ -36,7 +38,23 @@
     from werkzeug import Response  # noqa: F401
     from werkzeug.exceptions import HTTPException  # noqa: F401
 
-_insecure_views = ['main.login', 'main.select_logo', 'static']
+_insecure_views = ['main.login', 'static']
+
+
+def get_logo_url(app: Flask) -> str:
+    if not app.static_folder:
+        raise FileNotFoundError
+
+    custom_logo_filename = "i/custom_logo.png"
+    default_logo_filename = "i/logo.png"
+    custom_logo_path = Path(app.static_folder) / custom_logo_filename
+    default_logo_path = Path(app.static_folder) / default_logo_filename
+    if custom_logo_path.is_file():
+        return url_for("static", filename=custom_logo_filename)
+    elif default_logo_path.is_file():
+        return url_for("static", filename=default_logo_filename)
+
+    raise FileNotFoundError
 
 
 def create_app(config: 'SDConfig') -> Flask:
@@ -157,6 +175,11 @@ def setup_g() -> 'Optional[Response]':
         else:
             g.organization_name = gettext('SecureDrop')
 
+        try:
+            g.logo = get_logo_url(app)
+        except FileNotFoundError:
+            app.logger.error("Site logo not found.")
+
         if app.config["OS_PAST_EOL"]:
             g.show_os_past_eol_warning = True
         elif app.config["OS_NEAR_EOL"]:

securedrop/journalist_app/main.py

@@ -1,6 +1,4 @@
 # -*- coding: utf-8 -*-
-import os
-
 from datetime import datetime
 from typing import Union
 
@@ -51,16 +49,6 @@ def logout() -> werkzeug.Response:
         session.pop('nonce', None)
         return redirect(url_for('main.index'))
 
-    @view.route('/org-logo')
-    def select_logo() -> werkzeug.Response:
-        if current_app.static_folder is None:
-            abort(500)
-        if os.path.exists(os.path.join(current_app.static_folder, 'i',
-                          'custom_logo.png')):
-            return redirect(url_for('static', filename='i/custom_logo.png'))
-        else:
-            return redirect(url_for('static', filename='i/logo.png'))
-
     @view.route('/')
     def index() -> str:
         unstarred = []

securedrop/journalist_templates/base.html

@@ -43,7 +43,7 @@
       <div class="container">
         {% block header %}
         <div id="header">
-          <a href="{{ url_for('main.index') }}" class="no-bottom-border"><img src="{{ url_for('main.select_logo') }}" class="logo small" alt="{{ g.organization_name }} | Home" width="250"></a>
+          <a href="{{ url_for('main.index') }}" class="no-bottom-border"><img src="{{ g.logo }}" class="logo small" alt="{{ g.organization_name }} | Home" width="250"></a>
           {% include 'locales.html' %}
         </div>
         {% endblock %}

securedrop/journalist_templates/config.html

@@ -30,7 +30,7 @@ <h2 id="config-logoimage">{{ gettext('Logo Image') }}</h2>
 <p>{{ gettext('Here you can update the image displayed on the SecureDrop web interfaces:') }}</p>
 
 <p>
-  <img src="{{ url_for('main.select_logo') }}" class="logo small" alt="{{ g.organization_name }}" width="250">
+  <img src="{{ g.logo }}" class="logo small" alt="{{ g.organization_name }}" width="250">
 </p>
 
 <form method="post" enctype="multipart/form-data">

securedrop/source_app/__init__.py

@@ -1,4 +1,5 @@
 from datetime import datetime, timedelta
+from pathlib import Path
 from typing import Optional
 
 import werkzeug
@@ -28,6 +29,22 @@
 from server_os import is_os_past_eol
 
 
+def get_logo_url(app: Flask) -> str:
+    if not app.static_folder:
+        raise FileNotFoundError
+
+    custom_logo_filename = "i/custom_logo.png"
+    default_logo_filename = "i/logo.png"
+    custom_logo_path = Path(app.static_folder) / custom_logo_filename
+    default_logo_path = Path(app.static_folder) / default_logo_filename
+    if custom_logo_path.is_file():
+        return url_for("static", filename=custom_logo_filename)
+    elif default_logo_path.is_file():
+        return url_for("static", filename=default_logo_filename)
+
+    raise FileNotFoundError
+
+
 def create_app(config: SDConfig) -> Flask:
     app = Flask(__name__,
                 template_folder=config.SOURCE_TEMPLATES_DIR,
@@ -183,6 +200,11 @@ def setup_g() -> Optional[werkzeug.Response]:
         else:
             g.organization_name = gettext('SecureDrop')
 
+        try:
+            g.logo = get_logo_url(app)
+        except FileNotFoundError:
+            app.logger.error("Site logo not found.")
+
         return None
 
     @app.errorhandler(404)

securedrop/source_app/decorators.py

@@ -22,7 +22,7 @@ def ignore_static(f: Callable) -> Callable:
     a static resource."""
     @wraps(f)
     def decorated_function(*args: Any, **kwargs: Any) -> Any:
-        if request.path.startswith("/static") or request.path == "/org-logo":
+        if request.path.startswith("/static"):
             return  # don't execute the decorated function
         return f(*args, **kwargs)
     return decorated_function

securedrop/source_app/main.py

@@ -54,16 +54,6 @@ def generate() -> Union[str, werkzeug.Response]:
         session['new_user'] = True
         return render_template('generate.html', codename=codename, tab_id=tab_id)
 
-    @view.route('/org-logo')
-    def select_logo() -> werkzeug.Response:
-        if current_app.static_folder is None:
-            abort(500)
-        if os.path.exists(os.path.join(current_app.static_folder, 'i',
-                          'custom_logo.png')):
-            return redirect(url_for('static', filename='i/custom_logo.png'))
-        else:
-            return redirect(url_for('static', filename='i/logo.png'))
-
     @view.route('/create', methods=['POST'])
     def create() -> werkzeug.Response:
         if session.get('logged_in', False):

securedrop/source_templates/base.html

@@ -21,7 +21,7 @@
         {% block header %}
         <div id="header">
           <a href="{% if 'logged_in' in session %}{{ url_for('main.lookup') }}{% else %}{{ url_for('main.index') }}{% endif %}" class="no-bottom-border">
-            <img src="{{ url_for('main.select_logo') }}" class="logo small" alt="{{ g.organization_name }} | {{ gettext('SecureDrop Home') }}" width="250">
+            <img src="{{ g.logo }}" class="logo small" alt="{{ g.organization_name }} | {{ gettext('SecureDrop Home') }}" width="250">
           </a>
           {% include 'locales.html' %}
         </div>

securedrop/source_templates/index.html

@@ -30,7 +30,7 @@
            See _source_index.sass for a more full understanding. #}
         <div class="index-wrap">
           <div class="header">
-            <img src="{{ url_for('main.select_logo') }}" alt="{{ g.organization_name }} | {{ gettext('Logo Image') }}" class="logo-image">
+            <img src="{{ g.logo }}" alt="{{ g.organization_name }} | {{ gettext('Logo Image') }}" class="logo-image">
             <div id="index-locales">
               {% include 'locales.html' %}
             </div>

securedrop/tests/test_journalist.py

@@ -1603,12 +1603,10 @@ def test_logo_default_available(journalist_app):
         os.remove(custom_image_location)
 
     with journalist_app.test_client() as app:
-        response = app.get(url_for('main.select_logo'), follow_redirects=False)
-
-        assert response.status_code == 302
-        observed_headers = response.headers
-        assert 'Location' in list(observed_headers.keys())
-        assert url_for('static', filename='i/logo.png') in observed_headers['Location']
+        logo_url = journalist_app_module.get_logo_url(journalist_app)
+        assert logo_url.endswith("/static/i/logo.png")
+        response = app.get(logo_url, follow_redirects=False)
+        assert response.status_code == 200
 
 
 def test_logo_upload_with_valid_image_succeeds(journalist_app, test_admin):
@@ -1619,14 +1617,17 @@ def test_logo_upload_with_valid_image_succeeds(journalist_app, test_admin):
         original_image = logo_file.read()
 
     try:
+        logo_bytes = base64.decodebytes(
+            b"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQ"
+            b"VR42mP8/x8AAwMCAO+ip1sAAAAASUVORK5CYII="
+        )
+
         with journalist_app.test_client() as app:
             _login_user(app, test_admin['username'], test_admin['password'],
                         test_admin['otp_secret'])
             # Create 1px * 1px 'white' PNG file from its base64 string
             form = journalist_app_module.forms.LogoForm(
-                logo=(BytesIO(base64.decodebytes
-                      (b"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQ"
-                       b"VR42mP8/x8AAwMCAO+ip1sAAAAASUVORK5CYII=")), 'test.png')
+                logo=(BytesIO(logo_bytes), 'test.png')
             )
             with InstrumentedApp(journalist_app) as ins:
                 app.post(url_for('admin.manage_config'),
@@ -1635,12 +1636,11 @@ def test_logo_upload_with_valid_image_succeeds(journalist_app, test_admin):
 
                 ins.assert_message_flashed("Image updated.", "logo-success")
         with journalist_app.test_client() as app:
-            response = app.get(url_for('main.select_logo'), follow_redirects=False)
-
-            assert response.status_code == 302
-            observed_headers = response.headers
-            assert 'Location' in list(observed_headers.keys())
-            assert url_for('static', filename='i/custom_logo.png') in observed_headers['Location']
+            logo_url = journalist_app_module.get_logo_url(journalist_app)
+            assert logo_url.endswith("/static/i/custom_logo.png")
+            response = app.get(logo_url, follow_redirects=False)
+            assert response.status_code == 200
+            assert response.data == logo_bytes
     finally:
         # Restore original image to logo location for subsequent tests
         with io.open(logo_image_location, 'wb') as logo_file:

securedrop/tests/test_source.py

@@ -24,6 +24,7 @@
 from models import InstanceConfig, Source, Reply
 from source_app import main as source_app_main
 from source_app import api as source_app_api
+from source_app import get_logo_url
 from .utils.db_helper import new_codename, submit
 from .utils.instrument import InstrumentedApp
 from sdconfig import config
@@ -110,12 +111,10 @@ def test_logo_default_available(source_app):
         os.remove(custom_image_location)
 
     with source_app.test_client() as app:
-        response = app.get(url_for('main.select_logo'), follow_redirects=False)
-
-        assert response.status_code == 302
-        observed_headers = response.headers
-        assert 'Location' in list(observed_headers.keys())
-        assert url_for('static', filename='i/logo.png') in observed_headers['Location']
+        logo_url = get_logo_url(source_app)
+        assert logo_url.endswith('i/logo.png')
+        response = app.get(logo_url, follow_redirects=False)
+        assert response.status_code == 200
 
 
 def test_logo_custom_available(source_app):
@@ -126,12 +125,10 @@ def test_logo_custom_available(source_app):
         shutil.copyfile(default_image, custom_image)
 
     with source_app.test_client() as app:
-        response = app.get(url_for('main.select_logo'), follow_redirects=False)
-
-        assert response.status_code == 302
-        observed_headers = response.headers
-        assert 'Location' in list(observed_headers.keys())
-        assert url_for('static', filename='i/custom_logo.png') in observed_headers['Location']
+        logo_url = get_logo_url(source_app)
+        assert logo_url.endswith('i/custom_logo.png')
+        response = app.get(logo_url, follow_redirects=False)
+        assert response.status_code == 200
 
 
 def test_page_not_found(source_app):