diff --git a/securedrop/journalist_app/__init__.py b/securedrop/journalist_app/__init__.py index 1730684b2de..a9e9041776d 100644 --- a/securedrop/journalist_app/__init__.py +++ b/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 diff --git a/securedrop/journalist_app/col.py b/securedrop/journalist_app/col.py index d4f1a1f770d..94da2108ab1 100644 --- a/securedrop/journalist_app/col.py +++ b/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("/") 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/", 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() diff --git a/securedrop/journalist_app/main.py b/securedrop/journalist_app/main.py index 8dfe7039acd..ab7d14fc7a8 100644 --- a/securedrop/journalist_app/main.py +++ b/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,10 +112,11 @@ 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, @@ -123,7 +124,7 @@ def reply() -> werkzeug.Response: ) 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/") 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()) diff --git a/securedrop/journalist_app/utils.py b/securedrop/journalist_app/utils.py index 7ef816ebb3c..30b1d3b61fb 100644 --- a/securedrop/journalist_app/utils.py +++ b/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)