Merge pull request #6217 from freedomofpress/stg-upgrade-flask-2.0

Upgrade Flask dependency to 2.0.2 and refactor applications to improve static type checks

molecule/testinfra/vars/app-qubes-staging.yml

@@ -30,7 +30,7 @@ app_ip: 10.137.0.50
 
 pip_deps:
   - name: 'Flask'
-    version: '1.0.2'
+    version: '2.0.2'
 
 apparmor_complain: []
 

molecule/testinfra/vars/app-staging.yml

@@ -29,7 +29,7 @@ app_ip: 10.0.1.2
 
 pip_deps:
   - name: 'Flask'
-    version: '1.0.2'
+    version: '2.0.2'
 
 apparmor_complain: []
 

molecule/testinfra/vars/prod.yml

@@ -29,7 +29,7 @@ app_ip: 10.20.2.2
 
 pip_deps:
   - name: 'Flask'
-    version: '1.0.2'
+    version: '2.0.2'
 
 apparmor_complain: []
 

molecule/testinfra/vars/prodVM.yml

@@ -28,7 +28,7 @@ app_ip: 10.0.1.4
 
 pip_deps:
   - name: 'Flask'
-    version: '1.0.2'
+    version: '2.0.2'
 
 apparmor_complain: []
 

molecule/testinfra/vars/qubes-staging.yml

@@ -30,7 +30,7 @@ app_ip: 10.137.0.50
 
 pip_deps:
   - name: 'Flask'
-    version: '1.0.2'
+    version: '2.0.2'
 
 apparmor_complain: []
 

molecule/testinfra/vars/staging.yml

@@ -30,7 +30,7 @@ app_ip: 10.0.1.2
 
 pip_deps:
   - name: 'Flask'
-    version: '1.0.2'
+    version: '2.0.2'
 
 apparmor_complain: []
 

securedrop/alembic/versions/3da3fcab826a_delete_orphaned_submissions.py

@@ -17,7 +17,7 @@
 try:
     from journalist_app import create_app
     from sdconfig import config
-    from store import NoFileFoundException, TooManyFilesException
+    from store import NoFileFoundException, TooManyFilesException, Storage
 except ImportError:
     # This is a fresh install, and config.py has not been created yet.
     if raise_errors:
@@ -61,8 +61,8 @@ def upgrade():
                     """).bindparams(id=submission.id)
                     )
 
-                    path = app.storage.path_without_filesystem_id(submission.filename)
-                    app.storage.move_to_shredder(path)
+                    path = Storage.get_default().path_without_filesystem_id(submission.filename)
+                    Storage.get_default().move_to_shredder(path)
                 except NoFileFoundException:
                     # The file must have been deleted by the admin, remove the row
                     conn.execute(
@@ -83,8 +83,8 @@ def upgrade():
                         """).bindparams(id=reply.id)
                     )
 
-                    path = app.storage.path_without_filesystem_id(reply.filename)
-                    app.storage.move_to_shredder(path)
+                    path = Storage.get_default().path_without_filesystem_id(reply.filename)
+                    Storage.get_default().move_to_shredder(path)
                 except NoFileFoundException:
                     # The file must have been deleted by the admin, remove the row
                     conn.execute(

securedrop/alembic/versions/b58139cfdc8c_add_checksum_columns_revoke_table.py

@@ -14,7 +14,7 @@
     from journalist_app import create_app
     from models import Submission, Reply
     from sdconfig import config
-    from store import queued_add_checksum_for_file
+    from store import queued_add_checksum_for_file, Storage
     from worker import create_queue
 except:  # noqa
     if raise_errors:
@@ -58,7 +58,7 @@ def upgrade():
                             """
             )
             for (sub_id, filesystem_id, filename) in conn.execute(query):
-                full_path = app.storage.path(filesystem_id, filename)
+                full_path = Storage.get_default().path(filesystem_id, filename)
                 create_queue().enqueue(
                     queued_add_checksum_for_file,
                     Submission,
@@ -75,7 +75,7 @@ def upgrade():
                             """
             )
             for (rep_id, filesystem_id, filename) in conn.execute(query):
-                full_path = app.storage.path(filesystem_id, filename)
+                full_path = Storage.get_default().path(filesystem_id, filename)
                 create_queue().enqueue(
                     queued_add_checksum_for_file,
                     Reply,

securedrop/dockerfiles/focal/python3/Dockerfile

@@ -65,8 +65,8 @@ RUN wget https://github.com/mozilla/geckodriver/releases/download/${GECKODRIVER_
 COPY requirements requirements
 RUN python3 -m venv /opt/venvs/securedrop-app-code && \
     /opt/venvs/securedrop-app-code/bin/pip3 install --no-deps --require-hashes -r requirements/python3/docker-requirements.txt && \
-    /opt/venvs/securedrop-app-code/bin/pip3 install --no-deps --require-hashes -r requirements/python3/securedrop-app-code-requirements.txt && \
-    /opt/venvs/securedrop-app-code/bin/pip3 install --no-deps --require-hashes -r requirements/python3/test-requirements.txt
+    /opt/venvs/securedrop-app-code/bin/pip3 install --no-deps --require-hashes -r requirements/python3/test-requirements.txt && \
+    /opt/venvs/securedrop-app-code/bin/pip3 install --no-deps --require-hashes -r requirements/python3/securedrop-app-code-requirements.txt 
 
 RUN if test $USER_NAME != root ; then useradd --no-create-home --home-dir /tmp --uid $USER_ID $USER_NAME && echo "$USER_NAME ALL=(ALL) NOPASSWD:ALL" >> /etc/sudoers ; fi && \
     cp -r /root/.local /tmp/ && chmod +x /tmp/.local/tbb/tor-browser_en-US/Browser/firefox && chmod -R 777 /tmp/.local && \

securedrop/i18n.py

@@ -245,6 +245,6 @@ def set_locale(config: SDConfig) -> None:
     Update locale info in request and session.
     """
     locale = get_locale(config)
-    g.localeinfo = RequestLocaleInfo(locale)
+    g.localeinfo = RequestLocaleInfo(locale)  # pylint: disable=assigning-non-slot
     session["locale"] = locale
-    g.locales = LOCALES
+    g.locales = LOCALES  # pylint: disable=assigning-non-slot

securedrop/journalist_app/__init__.py

@@ -1,6 +1,6 @@
 # -*- coding: utf-8 -*-
 
-from datetime import datetime, timedelta
+from datetime import datetime, timedelta, timezone
 from pathlib import Path
 
 from flask import (Flask, session, redirect, url_for, flash, g, request,
@@ -21,7 +21,6 @@
                                   JournalistInterfaceSessionInterface,
                                   cleanup_expired_revoked_tokens)
 from models import InstanceConfig, Journalist
-from store import Storage
 
 import typing
 # https://www.python.org/dev/peps/pep-0484/#runtime-or-type-checking
@@ -69,11 +68,9 @@ def create_app(config: 'SDConfig') -> Flask:
     app.config['SQLALCHEMY_DATABASE_URI'] = config.DATABASE_URI
     db.init_app(app)
 
-    # TODO: Attaching a Storage dynamically like this disables all type checking (and
-    # breaks code analysis tools) for code that uses current_app.storage; it should be refactored
-    app.storage = Storage(config.STORE_DIR, config.TEMP_DIR)
-
-    @app.errorhandler(CSRFError)
+    # TODO: enable type checking once upstream Flask fix is available. See:
+    # https://github.com/pallets/flask/issues/4295
+    @app.errorhandler(CSRFError)  # type: ignore
     def handle_csrf_error(e: CSRFError) -> 'Response':
         app.logger.error("The CSRF token is invalid.")
         session.clear()
@@ -86,9 +83,12 @@ def _handle_http_exception(
     ) -> 'Tuple[Union[Response, str], Optional[int]]':
         # Workaround for no blueprint-level 404/5 error handlers, see:
         # https://github.com/pallets/flask/issues/503#issuecomment-71383286
+        # TODO: clean up API error handling such that all except 404/5s are
+        # registered in the blueprint and 404/5s are handled at the application
+        # level.
         handler = list(app.error_handler_spec['api'][error.code].values())[0]
         if request.path.startswith('/api/') and handler:
-            return handler(error)
+            return handler(error)  # type: ignore
 
         return render_template('error.html', error=error), error.code
 
@@ -111,13 +111,13 @@ def expire_blacklisted_tokens() -> None:
         cleanup_expired_revoked_tokens()
 
     @app.before_request
-    def load_instance_config() -> None:
-        app.instance_config = InstanceConfig.get_current()
+    def update_instance_config() -> None:
+        InstanceConfig.get_default(refresh=True)
 
     @app.before_request
     def setup_g() -> 'Optional[Response]':
         """Store commonly used values in Flask's special g object"""
-        if 'expires' in session and datetime.utcnow() >= session['expires']:
+        if 'expires' in session and datetime.now(timezone.utc) >= session['expires']:
             session.clear()
             flash(gettext('You have been logged out due to inactivity.'),
                   'error')
@@ -131,24 +131,25 @@ def setup_g() -> 'Optional[Response]':
                 flash(gettext('You have been logged out due to password change'),
                       'error')
 
-        session['expires'] = datetime.utcnow() + \
+        session['expires'] = datetime.now(timezone.utc) + \
             timedelta(minutes=getattr(config,
                                       'SESSION_EXPIRATION_MINUTES',
                                       120))
 
         uid = session.get('uid', None)
         if uid:
-            g.user = Journalist.query.get(uid)
+            g.user = Journalist.query.get(uid)  # pylint: disable=assigning-non-slot
 
         i18n.set_locale(config)
 
-        if app.instance_config.organization_name:
-            g.organization_name = app.instance_config.organization_name
+        if InstanceConfig.get_default().organization_name:
+            g.organization_name = \
+                InstanceConfig.get_default().organization_name  # pylint: disable=assigning-non-slot
         else:
-            g.organization_name = gettext('SecureDrop')
+            g.organization_name = gettext('SecureDrop')  # pylint: disable=assigning-non-slot
 
         try:
-            g.logo = get_logo_url(app)
+            g.logo = get_logo_url(app)  # pylint: disable=assigning-non-slot
         except FileNotFoundError:
             app.logger.error("Site logo not found.")
 
@@ -161,8 +162,8 @@ def setup_g() -> 'Optional[Response]':
         if request.method == 'POST':
             filesystem_id = request.form.get('filesystem_id')
             if filesystem_id:
-                g.filesystem_id = filesystem_id
-                g.source = get_source(filesystem_id)
+                g.filesystem_id = filesystem_id  # pylint: disable=assigning-non-slot
+                g.source = get_source(filesystem_id)  # pylint: disable=assigning-non-slot
 
         return None
 

securedrop/journalist_app/admin.py

@@ -38,9 +38,9 @@ def index() -> str:
     def manage_config() -> Union[str, werkzeug.Response]:
         # The UI prompt ("prevent") is the opposite of the setting ("allow"):
         submission_preferences_form = SubmissionPreferencesForm(
-            prevent_document_uploads=not current_app.instance_config.allow_document_uploads)
+            prevent_document_uploads=not InstanceConfig.get_default().allow_document_uploads)
         organization_name_form = OrgNameForm(
-            organization_name=current_app.instance_config.organization_name)
+            organization_name=InstanceConfig.get_default().organization_name)
         logo_form = LogoForm()
         if logo_form.validate_on_submit():
             f = logo_form.logo.data

securedrop/journalist_app/api.py

@@ -1,12 +1,12 @@
 import collections.abc
 import json
 
-from datetime import datetime, timedelta
+from datetime import datetime, timedelta, timezone
 from typing import Tuple, Callable, Any, Set, Union
 
 import flask
 import werkzeug
-from flask import abort, Blueprint, current_app, jsonify, request
+from flask import abort, Blueprint, jsonify, request
 from functools import wraps
 
 from sqlalchemy import Column
@@ -22,7 +22,7 @@
                     BadTokenException, InvalidOTPSecretException,
                     WrongPasswordException)
 from sdconfig import SDConfig
-from store import NotEncrypted
+from store import NotEncrypted, Storage
 
 
 TOKEN_EXPIRATION_MINS = 60 * 8
@@ -116,7 +116,7 @@ def get_token() -> Tuple[flask.Response, int]:
 
         try:
             journalist = Journalist.login(username, passphrase, one_time_code)
-            token_expiry = datetime.utcnow() + timedelta(
+            token_expiry = datetime.now(timezone.utc) + timedelta(
                 seconds=TOKEN_EXPIRATION_MINS * 60)
 
             response = jsonify({
@@ -128,7 +128,7 @@ def get_token() -> Tuple[flask.Response, int]:
             })
 
             # Update access metadata
-            journalist.last_access = datetime.utcnow()
+            journalist.last_access = datetime.now(timezone.utc)
             db.session.add(journalist)
             db.session.commit()
 
@@ -253,7 +253,7 @@ def all_source_replies(source_uuid: str) -> Tuple[flask.Response, int]:
 
             source.interaction_count += 1
             try:
-                filename = current_app.storage.save_pre_encrypted_reply(
+                filename = Storage.get_default().save_pre_encrypted_reply(
                     source.filesystem_id,
                     source.interaction_count,
                     source.journalist_filename,
@@ -265,7 +265,7 @@ def all_source_replies(source_uuid: str) -> Tuple[flask.Response, int]:
             # issue #3918
             filename = path.basename(filename)
 
-            reply = Reply(user, source, filename)
+            reply = Reply(user, source, filename, Storage.get_default())
 
             reply_uuid = data.get('uuid', None)
             if reply_uuid is not None:

securedrop/journalist_app/col.py

@@ -29,6 +29,7 @@
                                   col_download_all, col_star, col_un_star,
                                   col_delete, col_delete_data, mark_seen)
 from sdconfig import SDConfig
+from store import Storage
 
 
 def make_blueprint(config: SDConfig) -> Blueprint:
@@ -118,7 +119,7 @@ def download_single_file(filesystem_id: str, fn: str) -> werkzeug.Response:
         if '..' in fn or fn.startswith('/'):
             abort(404)
 
-        file = current_app.storage.path(filesystem_id, fn)
+        file = Storage.get_default().path(filesystem_id, fn)
         if not Path(file).is_file():
             flash(
                 gettext(
@@ -137,15 +138,15 @@ def download_single_file(filesystem_id: str, fn: str) -> werkzeug.Response:
                 reply = Reply.query.filter(Reply.filename == fn).one()
                 mark_seen([reply], journalist)
             elif fn.endswith("-doc.gz.gpg") or fn.endswith("doc.zip.gpg"):
-                file = Submission.query.filter(Submission.filename == fn).one()
-                mark_seen([file], journalist)
+                submitted_file = Submission.query.filter(Submission.filename == fn).one()
+                mark_seen([submitted_file], journalist)
             else:
                 message = Submission.query.filter(Submission.filename == fn).one()
                 mark_seen([message], journalist)
         except NoResultFound as e:
             current_app.logger.error("Could not mark {} as seen: {}".format(fn, e))
 
-        return send_file(current_app.storage.path(filesystem_id, fn),
+        return send_file(Storage.get_default().path(filesystem_id, fn),
                          mimetype="application/pgp-encrypted")
 
     return view

securedrop/journalist_app/main.py

@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-from datetime import datetime
+from datetime import datetime, timezone
 from pathlib import Path
 from typing import Union
 
@@ -19,6 +19,7 @@
 from journalist_app.utils import (validate_user, bulk_delete, download,
                                   confirm_bulk_delete, get_source)
 from sdconfig import SDConfig
+from store import Storage
 
 
 def make_blueprint(config: SDConfig) -> Blueprint:
@@ -36,7 +37,7 @@ def login() -> Union[str, werkzeug.Response]:
                                                 request.form['token']))
 
                 # Update access metadata
-                user.last_access = datetime.utcnow()
+                user.last_access = datetime.now(timezone.utc)
                 db.session.add(user)
                 db.session.commit()
 
@@ -135,16 +136,16 @@ def reply() -> werkzeug.Response:
         EncryptionManager.get_default().encrypt_journalist_reply(
             for_source_with_filesystem_id=g.filesystem_id,
             reply_in=form.message.data,
-            encrypted_reply_path_out=Path(current_app.storage.path(g.filesystem_id, filename)),
+            encrypted_reply_path_out=Path(Storage.get_default().path(g.filesystem_id, filename)),
         )
 
         try:
-            reply = Reply(g.user, g.source, filename)
+            reply = Reply(g.user, g.source, filename, Storage.get_default())
             db.session.add(reply)
             seen_reply = SeenReply(reply=reply, journalist=g.user)
             db.session.add(seen_reply)
             db.session.commit()
-            store.async_add_checksum_for_file(reply)
+            store.async_add_checksum_for_file(reply, Storage.get_default())
         except Exception as exc:
             flash(gettext(
                 "An unexpected error occurred! Please "