Address TODOs

securedrop/journalist_app/__init__.py

@@ -5,13 +5,13 @@
 import i18n
 import template_filters
 import version
+from actions.exceptions import NotFoundError
 from db import db
 from flask import Flask, abort, g, json, redirect, render_template, request, url_for
 from flask_babel import gettext
 from flask_wtf.csrf import CSRFError, CSRFProtect
 from journalist_app import account, admin, api, col, main
 from journalist_app.sessions import Session, session
-from journalist_app.utils import get_source
 from models import InstanceConfig
 from sdconfig import SecureDropConfig
 from werkzeug import Response
@@ -72,6 +72,11 @@ def handle_csrf_error(e: CSRFError) -> "Response":
         session.destroy(("error", msg), session.get("locale"))
         return redirect(url_for("main.login"))
 
+    # Convert a NotFoundError raised by an action into a 404
+    @app.errorhandler(NotFoundError)
+    def handle_action_raised_not_found(e: NotFoundError) -> None:
+        abort(404)
+
     def _handle_http_exception(
         error: HTTPException,
     ) -> Tuple[Union[Response, str], Optional[int]]:
@@ -132,7 +137,6 @@ def setup_g() -> Optional[Response]:
             filesystem_id = request.form.get("filesystem_id")
             if 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/col.py

@@ -1,7 +1,7 @@
 from pathlib import Path
 
 import werkzeug
-from actions.sources_actions import DeleteSingleSourceAction
+from actions.sources_actions import DeleteSingleSourceAction, GetSingleSourceAction
 from db import db
 from encryption import EncryptionManager, GpgKeyNotFoundError
 from flask import (
@@ -27,7 +27,6 @@
     col_download_unread,
     col_star,
     col_un_star,
-    get_source,
     make_star_false,
     make_star_true,
     mark_seen,
@@ -55,7 +54,7 @@ def remove_star(filesystem_id: str) -> werkzeug.Response:
     @view.route("/<filesystem_id>")
     def col(filesystem_id: str) -> str:
         form = ReplyForm()
-        source = get_source(filesystem_id)
+        source = GetSingleSourceAction(db_session=db.session, filesystem_id=filesystem_id).perform()
         try:
             EncryptionManager.get_default().get_source_public_key(filesystem_id)
             source.has_key = True
@@ -67,7 +66,7 @@ def col(filesystem_id: str) -> str:
     @view.route("/delete/<filesystem_id>", methods=("POST",))
     def delete_single(filesystem_id: str) -> werkzeug.Response:
         """deleting a single collection from its /col page"""
-        source = get_source(filesystem_id)
+        source = GetSingleSourceAction(db_session=db.session, filesystem_id=filesystem_id).perform()
         source_designation = source.journalist_designation
         DeleteSingleSourceAction(db_session=db.session, source=source).perform()
 

securedrop/journalist_app/main.py

@@ -23,7 +23,7 @@
 from flask_babel import gettext
 from journalist_app.forms import ReplyForm
 from journalist_app.sessions import session
-from journalist_app.utils import bulk_delete, download, get_source, validate_user
+from journalist_app.utils import bulk_delete, download, validate_user
 from models import Reply, SeenReply, Submission
 from sqlalchemy.sql import func
 from store import Storage
@@ -112,18 +112,19 @@ def reply() -> werkzeug.Response:
                 flash(error, "error")
             return redirect(url_for("col.col", filesystem_id=g.filesystem_id))
 
-        g.source.interaction_count += 1
-        filename = "{}-{}-reply.gpg".format(
-            g.source.interaction_count, g.source.journalist_filename
-        )
+        source = GetSingleSourceAction(
+            db_session=db.session, filesystem_id=g.filesystem_id
+        ).perform()
+        source.interaction_count += 1
+        filename = "{}-{}-reply.gpg".format(source.interaction_count, source.journalist_filename)
         EncryptionManager.get_default().encrypt_journalist_reply(
             for_source_with_filesystem_id=g.filesystem_id,
             reply_in=form.message.data,
             encrypted_reply_path_out=Path(Storage.get_default().path(g.filesystem_id, filename)),
         )
 
         try:
-            reply = Reply(session.get_user(), g.source, filename, Storage.get_default())
+            reply = Reply(session.get_user(), source, filename, Storage.get_default())
             db.session.add(reply)
             seen_reply = SeenReply(reply=reply, journalist=session.get_user())
             db.session.add(seen_reply)
@@ -164,7 +165,12 @@ def bulk() -> Union[str, werkzeug.Response]:
         action = request.form["action"]
         error_redirect = url_for("col.col", filesystem_id=g.filesystem_id)
         doc_names_selected = request.form.getlist("doc_names_selected")
-        selected_docs = [doc for doc in g.source.collection if doc.filename in doc_names_selected]
+
+        source = GetSingleSourceAction(
+            db_session=db.session, filesystem_id=g.filesystem_id
+        ).perform()
+        selected_docs = [doc for doc in source.collection if doc.filename in doc_names_selected]
+
         if selected_docs == []:
             if action == "download":
                 flash(
@@ -194,7 +200,9 @@ def bulk() -> Union[str, werkzeug.Response]:
             return redirect(error_redirect)
 
         if action == "download":
-            source = get_source(g.filesystem_id)
+            source = GetSingleSourceAction(
+                db_session=db.session, filesystem_id=g.filesystem_id
+            ).perform()
             return download(
                 source.journalist_filename,
                 selected_docs,
@@ -207,8 +215,8 @@ def bulk() -> Union[str, werkzeug.Response]:
 
     @view.route("/download_unread/<filesystem_id>")
     def download_unread_filesystem_id(filesystem_id: str) -> werkzeug.Response:
-        # TODO: Error handler
         source = GetSingleSourceAction(db_session=db.session, filesystem_id=filesystem_id).perform()
+
         unseen_submissions = (
             Submission.query.filter(Submission.source_id == source.id)
             .filter(~Submission.seen_files.any(), ~Submission.seen_messages.any())

securedrop/journalist_app/utils.py

@@ -54,22 +54,6 @@ def commit_account_changes(user: Journalist) -> None:
             flash(gettext("Account updated."), "success")
 
 
-# TODO: Remove
-def get_source(filesystem_id: str, include_deleted: bool = False) -> Source:
-    """
-    Return the Source object with `filesystem_id`
-
-    If `include_deleted` is False, only sources with a null `deleted_at` will
-    be returned.
-    """
-    query = Source.query.filter(Source.filesystem_id == filesystem_id)
-    if not include_deleted:
-        query = query.filter_by(deleted_at=None)
-    source = get_one_or_else(query, current_app.logger, abort)
-
-    return source
-
-
 def validate_user(
     username: str,
     password: Optional[str],
@@ -287,8 +271,10 @@ def bulk_delete(
     return redirect(url_for("col.col", filesystem_id=filesystem_id))
 
 
+# TODO(AD): Turn the next two functions into a SourceUpdateStarredAction
 def make_star_true(filesystem_id: str) -> None:
-    source = get_source(filesystem_id)
+    query = Source.query.filter(Source.filesystem_id == filesystem_id)
+    source = get_one_or_else(query, current_app.logger, abort)
     if source.star:
         source.star.starred = True
     else:
@@ -297,7 +283,8 @@ def make_star_true(filesystem_id: str) -> None:
 
 
 def make_star_false(filesystem_id: str) -> None:
-    source = get_source(filesystem_id)
+    query = Source.query.filter(Source.filesystem_id == filesystem_id)
+    source = get_one_or_else(query, current_app.logger, abort)
     if not source.star:
         source_star = SourceStar(source)
         db.session.add(source_star)