Merge pull request #5874 from freedomofpress/static-logo

Replace logo selection route with per-request global variable

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
 
@@ -53,16 +51,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:
         # Gather the count of unread submissions for each source

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):