Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API changes for listing/marking source conversation items that have been seen by journalists #5513

Merged
merged 4 commits into from
Sep 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion securedrop/create-dev-data.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

from sdconfig import config
from db import db
from models import Journalist, Reply, Source, Submission
from models import Journalist, Reply, SeenReply, Source, Submission
from specialstrings import strings


Expand Down Expand Up @@ -138,6 +138,9 @@ def create_source_and_submissions(
journalist = journalist_who_replied
reply = Reply(journalist, source, fname)
db.session.add(reply)
db.session.flush()
seen_reply = SeenReply(reply_id=reply.id, journalist_id=journalist.id)
rmol marked this conversation as resolved.
Show resolved Hide resolved
db.session.add(seen_reply)

db.session.commit()

Expand Down
52 changes: 50 additions & 2 deletions securedrop/journalist_app/api.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import collections.abc
import json

from datetime import datetime, timedelta
from typing import Tuple, Callable, Any
from typing import Tuple, Callable, Any, Set, Union

import flask
import werkzeug
Expand All @@ -15,7 +16,7 @@

from db import db
from journalist_app import utils
from models import (Journalist, Reply, Source, Submission,
from models import (Journalist, Reply, SeenReply, Source, Submission,
LoginThrottledException, InvalidUsernameException,
BadTokenException, WrongPasswordException)
from sdconfig import SDConfig
Expand Down Expand Up @@ -69,6 +70,7 @@ def get_endpoints() -> Tuple[flask.Response, int]:
'all_users_url': '/api/v1/users',
'submissions_url': '/api/v1/submissions',
'replies_url': '/api/v1/replies',
'seen_url': '/api/v1/seen',
'auth_token_url': '/api/v1/token'}
return jsonify(endpoints), 200

Expand Down Expand Up @@ -268,6 +270,9 @@ def all_source_replies(source_uuid: str) -> Tuple[flask.Response, int]:

try:
db.session.add(reply)
db.session.flush()
seen_reply = SeenReply(reply_id=reply.id, journalist_id=user.id)
db.session.add(seen_reply)
db.session.add(source)
db.session.commit()
except IntegrityError as e:
Expand Down Expand Up @@ -311,6 +316,49 @@ def get_all_replies() -> Tuple[flask.Response, int]:
return jsonify(
{'replies': [reply.to_json() for reply in replies if reply.source]}), 200

@api.route("/seen", methods=["POST"])
@token_required
def seen() -> Tuple[flask.Response, int]:
"""
Lists or marks the source conversation items that the journalist has seen.
"""
user = _authenticate_user_from_auth_header(request)

if request.method == "POST":
if request.json is None or not isinstance(request.json, collections.abc.Mapping):
abort(400, "Please send requests in valid JSON.")

if not any(map(request.json.get, ["files", "messages", "replies"])):
abort(400, "Please specify the resources to mark seen.")

# gather everything to be marked seen. if any don't exist,
# reject the request.
targets = set() # type: Set[Union[Submission, Reply]]
for file_uuid in request.json.get("files", []):
f = Submission.query.filter(Submission.uuid == file_uuid).one_or_none()
if f is None or not f.is_file:
abort(404, "file not found: {}".format(file_uuid))
targets.add(f)

for message_uuid in request.json.get("messages", []):
m = Submission.query.filter(Submission.uuid == message_uuid).one_or_none()
if m is None or not m.is_message:
abort(404, "message not found: {}".format(message_uuid))
targets.add(m)

for reply_uuid in request.json.get("replies", []):
r = Reply.query.filter(Reply.uuid == reply_uuid).one_or_none()
if r is None:
abort(404, "reply not found: {}".format(reply_uuid))
targets.add(r)

# now mark everything seen.
utils.mark_seen(list(targets), user)

return jsonify({"message": "resources marked seen"}), 200

abort(405)

@api.route('/user', methods=['GET'])
@token_required
def get_current_user() -> Tuple[flask.Response, int]:
Expand Down
20 changes: 6 additions & 14 deletions securedrop/journalist_app/col.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,15 @@
url_for,
)
from flask_babel import gettext
from sqlalchemy.exc import IntegrityError
from sqlalchemy.orm.exc import NoResultFound

from db import db
from models import Reply, SeenFile, SeenMessage, SeenReply, Submission
from models import Reply, Submission
from journalist_app.forms import ReplyForm
from journalist_app.utils import (make_star_true, make_star_false, get_source,
delete_collection, col_download_unread,
col_download_all, col_star, col_un_star,
col_delete)
col_delete, mark_seen)


def make_blueprint(config):
Expand Down Expand Up @@ -94,25 +93,18 @@ def download_single_file(filesystem_id, fn):

# mark as seen by the current user
try:
journalist_id = g.get("user").id
journalist = g.get("user")
if fn.endswith("reply.gpg"):
reply = Reply.query.filter(Reply.filename == fn).one()
seen_reply = SeenReply(reply_id=reply.id, journalist_id=journalist_id)
db.session.add(seen_reply)
mark_seen([reply], journalist)
elif fn.endswith("-doc.gz.gpg") or fn.endswith("doc.zip.gpg"):
file = Submission.query.filter(Submission.filename == fn).one()
seen_file = SeenFile(file_id=file.id, journalist_id=journalist_id)
db.session.add(seen_file)
mark_seen([file], journalist)
else:
message = Submission.query.filter(Submission.filename == fn).one()
seen_message = SeenMessage(message_id=message.id, journalist_id=journalist_id)
db.session.add(seen_message)

db.session.commit()
mark_seen([message], journalist)
except NoResultFound as e:
current_app.logger.error("Could not mark {} as seen: {}".format(fn, e))
except IntegrityError:
pass # expected not to store that a file was seen by the same user multiple times

return send_file(current_app.storage.path(filesystem_id, fn),
mimetype="application/pgp-encrypted")
Expand Down
53 changes: 28 additions & 25 deletions securedrop/journalist_app/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from flask import (g, flash, current_app, abort, send_file, redirect, url_for,
render_template, Markup, sessions, request)
from flask_babel import gettext, ngettext
from sqlalchemy.exc import IntegrityError

import i18n

Expand Down Expand Up @@ -163,6 +164,32 @@ def validate_hotp_secret(user: Journalist, otp_secret: str) -> bool:
return True


def mark_seen(targets: List[Union[Submission, Reply]], user: Journalist) -> None:
"""
Marks a list of submissions or replies seen by the given journalist.
"""
for t in targets:
try:
if isinstance(t, Submission):
t.downloaded = True
if t.is_file:
sf = SeenFile(file_id=t.id, journalist_id=user.id)
db.session.add(sf)
elif t.is_message:
sm = SeenMessage(message_id=t.id, journalist_id=user.id)
db.session.add(sm)
db.session.commit()
elif isinstance(t, Reply):
sr = SeenReply(reply_id=t.id, journalist_id=user.id)
db.session.add(sr)
db.session.commit()
except IntegrityError as e:
db.session.rollback()
if 'UNIQUE constraint failed' in str(e):
continue
raise


def download(zip_basename: str, submissions: List[Union[Submission, Reply]]) -> werkzeug.Response:
"""Send client contents of ZIP-file *zip_basename*-<timestamp>.zip
containing *submissions*. The ZIP-file, being a
Expand All @@ -179,31 +206,7 @@ def download(zip_basename: str, submissions: List[Union[Submission, Reply]]) ->
zip_basename, datetime.datetime.utcnow().strftime("%Y-%m-%d--%H-%M-%S")
)

# mark as seen by the current user
journalist_id = g.get("user").id
for item in submissions:
if item.filename.endswith("reply.gpg"):
already_seen = SeenReply.query.filter_by(
reply_id=item.id, journalist_id=journalist_id
).one_or_none()
if not already_seen:
seen_reply = SeenReply(reply_id=item.id, journalist_id=journalist_id)
db.session.add(seen_reply)
elif item.filename.endswith("-doc.gz.gpg"):
already_seen = SeenFile.query.filter_by(
file_id=item.id, journalist_id=journalist_id
).one_or_none()
if not already_seen:
seen_file = SeenFile(file_id=item.id, journalist_id=journalist_id)
db.session.add(seen_file)
else:
already_seen = SeenMessage.query.filter_by(
message_id=item.id, journalist_id=journalist_id
).one_or_none()
if not already_seen:
seen_message = SeenMessage(message_id=item.id, journalist_id=journalist_id)
db.session.add(seen_message)
db.session.commit()
mark_seen(submissions, g.user)

return send_file(
zf.name,
Expand Down
67 changes: 45 additions & 22 deletions securedrop/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,10 @@ def journalist_filename(self) -> str:
def documents_messages_count(self) -> 'Dict[str, int]':
self.docs_msgs_count = {'messages': 0, 'documents': 0}
for submission in self.submissions:
if submission.filename.endswith('msg.gpg'):
self.docs_msgs_count['messages'] += 1
elif (submission.filename.endswith('doc.gz.gpg') or
submission.filename.endswith('doc.zip.gpg')):
self.docs_msgs_count['documents'] += 1
if submission.is_message:
self.docs_msgs_count["messages"] += 1
elif submission.is_file:
self.docs_msgs_count["documents"] += 1
return self.docs_msgs_count

@property
Expand Down Expand Up @@ -212,7 +211,23 @@ def __init__(self, source: Source, filename: str) -> None:
def __repr__(self) -> str:
return '<Submission %r>' % (self.filename)

def to_json(self) -> 'Dict[str, Union[str, int, bool]]':
@property
def is_file(self) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought about doing this myself in #5505 but the pr was already so large. nice change.

return self.filename.endswith("doc.gz.gpg") or self.filename.endswith("doc.zip.gpg")

@property
def is_message(self) -> bool:
return self.filename.endswith("msg.gpg")

def to_json(self) -> "Dict[str, Union[str, int, bool]]":
seen_by = {
f.journalist.uuid for f in SeenFile.query.filter(SeenFile.file_id == self.id)
if f.journalist
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see, so this won't include the fact that there is a deleted journalist who saw a file. gonna keep reviewing and will cycle back to this. could be an issue because we still send "deleted" uuids for replies when journalists are deleted. this takes a different course.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not. I didn't see any way for sensible inclusion of those records in seen_by here. We do have Submission.seen, which will reflect those, and until we start keeping deleted journalists around, we can't distinguish between the records for the submission in the seen tables anyway, so since the only purpose of those rows at this point is establishing the global seen state, I thought Submission.seen the right answer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are also replies we have to worry about which don't have a global seen state, but I definitely see why you made this choice. Agreed that implementing #5467 + global user account for data migration will make this work fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replies do have a global seen state: if one exists, its creator has seen it. SeenReply can tell us who else has seen it, but only if journalist_id is not null, so again, I didn't see any point in including records where it is.

}
seen_by.update({
m.journalist.uuid for m in SeenMessage.query.filter(SeenMessage.message_id == self.id)
if m.journalist
})
json_submission = {
'source_url': url_for('api.single_source',
source_uuid=self.source.uuid) if self.source else None,
Expand All @@ -221,11 +236,14 @@ def to_json(self) -> 'Dict[str, Union[str, int, bool]]':
submission_uuid=self.uuid) if self.source else None,
'filename': self.filename,
'size': self.size,
'is_read': self.downloaded,
"is_file": self.is_file,
"is_message": self.is_message,
"is_read": self.seen,
'uuid': self.uuid,
'download_url': url_for('api.download_submission',
source_uuid=self.source.uuid,
submission_uuid=self.uuid) if self.source else None,
'seen_by': list(seen_by)
}
return json_submission

Expand Down Expand Up @@ -284,32 +302,37 @@ def __init__(self,
def __repr__(self) -> str:
return '<Reply %r>' % (self.filename)

def to_json(self) -> 'Dict[str, Union[str, int, bool]]':
username = "deleted"
first_name = ""
last_name = ""
uuid = "deleted"
def to_json(self) -> "Dict[str, Union[str, int, bool]]":
journalist_username = "deleted"
journalist_first_name = ""
journalist_last_name = ""
journalist_uuid = "deleted"
if self.journalist:
username = self.journalist.username
first_name = self.journalist.first_name
last_name = self.journalist.last_name
uuid = self.journalist.uuid
json_submission = {
journalist_username = self.journalist.username
journalist_first_name = self.journalist.first_name
journalist_last_name = self.journalist.last_name
journalist_uuid = self.journalist.uuid
seen_by = [
r.journalist.uuid for r in SeenReply.query.filter(SeenReply.reply_id == self.id)
if r.journalist
]
json_reply = {
'source_url': url_for('api.single_source',
source_uuid=self.source.uuid) if self.source else None,
'reply_url': url_for('api.single_reply',
source_uuid=self.source.uuid,
reply_uuid=self.uuid) if self.source else None,
'filename': self.filename,
'size': self.size,
'journalist_username': username,
'journalist_first_name': first_name,
'journalist_last_name': last_name,
'journalist_uuid': uuid,
'journalist_username': journalist_username,
'journalist_first_name': journalist_first_name,
'journalist_last_name': journalist_last_name,
'journalist_uuid': journalist_uuid,
'uuid': self.uuid,
'is_deleted_by_source': self.deleted_by_source,
'seen_by': seen_by
}
return json_submission
return json_reply


class SourceStar(db.Model):
Expand Down
2 changes: 1 addition & 1 deletion securedrop/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ def test_submissions(journalist_app):
def test_files(journalist_app, test_journo):
with journalist_app.app_context():
source, codename = utils.db_helper.init_source()
utils.db_helper.submit(source, 2)
utils.db_helper.submit(source, 2, submission_type="file")
utils.db_helper.reply(test_journo['journalist'], source, 1)
return {'source': source,
'codename': codename,
Expand Down
11 changes: 5 additions & 6 deletions securedrop/tests/test_journalist.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import crypto_util
import journalist_app as journalist_app_module
from journalist_app.utils import mark_seen
import models
from db import db
from models import (
Expand Down Expand Up @@ -1847,11 +1848,11 @@ def test_delete_collection_updates_db(journalist_app, test_journo, test_source):
source = Source.query.get(test_source["id"])
journo = Journalist.query.get(test_journo["id"])
files = utils.db_helper.submit(source, 2)
utils.db_helper.mark_files_seen(journo.id, files)
mark_seen(files, journo)
messages = utils.db_helper.submit(source, 2)
utils.db_helper.mark_messages_seen(journo.id, messages)
mark_seen(messages, journo)
replies = utils.db_helper.reply(journo, source, 2)
utils.db_helper.mark_replies_seen(journo.id, replies)
mark_seen(replies, journo)

journalist_app_module.utils.delete_collection(test_source["filesystem_id"])

Expand Down Expand Up @@ -2074,9 +2075,7 @@ def test_download_selected_submissions_and_replies_previously_seen(
db.session.add(seen_file)
seen_message = SeenMessage(message_id=selected_submissions[1].id, journalist_id=journo.id)
db.session.add(seen_message)
for reply in selected_replies:
seen_reply = SeenReply(reply_id=reply.id, journalist_id=journo.id)
db.session.add(seen_reply)
mark_seen(selected_replies, journo)
db.session.commit()

with journalist_app.test_client() as app:
Expand Down
Loading