From 0c264cf2afb0db91533018d220aea38a02d22c69 Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Tue, 4 Jun 2019 08:56:50 -0700 Subject: [PATCH 1/3] app: create SendReplyJob, send replies using general queue --- Makefile | 3 +- securedrop_client/api_jobs/base.py | 2 +- securedrop_client/api_jobs/uploads.py | 60 +++++++++++++++++++++++++++ securedrop_client/logic.py | 58 ++++++++------------------ 4 files changed, 81 insertions(+), 42 deletions(-) create mode 100644 securedrop_client/api_jobs/uploads.py diff --git a/Makefile b/Makefile index e44b8bafc..6635db99d 100644 --- a/Makefile +++ b/Makefile @@ -18,7 +18,8 @@ mypy: ## Run static type checker securedrop_client/queue.py \ securedrop_client/api_jobs/__init__.py \ securedrop_client/api_jobs/base.py \ - securedrop_client/api_jobs/downloads.py + securedrop_client/api_jobs/downloads.py \ + securedrop_client/api_jobs/uploads.py .PHONY: clean clean: ## Clean the workspace of generated resources diff --git a/securedrop_client/api_jobs/base.py b/securedrop_client/api_jobs/base.py index fbde085b6..0271c0402 100644 --- a/securedrop_client/api_jobs/base.py +++ b/securedrop_client/api_jobs/base.py @@ -56,7 +56,7 @@ def call_api(self, api_client: API, session: Session) -> Any: Method for making the actual API call and handling the result. This MUST resturn a value if the API call and other tasks were successful and MUST raise - an exception if and only iff the tasks failed. Presence of a raise exception indicates a + an exception if and only if the tasks failed. Presence of a raise exception indicates a failure. ''' raise NotImplementedError diff --git a/securedrop_client/api_jobs/uploads.py b/securedrop_client/api_jobs/uploads.py new file mode 100644 index 000000000..0589ba680 --- /dev/null +++ b/securedrop_client/api_jobs/uploads.py @@ -0,0 +1,60 @@ +import logging +import sdclientapi +import traceback + +from sdclientapi import API +from sqlalchemy.orm.session import Session + +from securedrop_client.api_jobs.base import ApiJob +from securedrop_client.crypto import GpgHelper +from securedrop_client.db import Reply, Source + +logger = logging.getLogger(__name__) + + +class SendReplyJob(ApiJob): + def __init__( + self, + source_uuid: str, + reply_uuid: str, + message: str, + gpg: GpgHelper, + ) -> None: + super().__init__() + self.source_uuid = source_uuid + self.reply_uuid = reply_uuid + self.message = message + self.gpg = gpg + + def call_api(self, api_client: API, session: Session) -> str: + try: + encrypted_reply = self.gpg.encrypt_to_source(self.source_uuid, + self.message) + except Exception: + tb = traceback.format_exc() + logger.error('Failed to encrypt to source {}:\n'.format( + self.source_uuid, tb)) + # We raise the exception as it will get handled in ApiJob._do_call_api + # Exceptions must be raised for the failure signal to be emitted. + raise + else: + sdk_reply = self._make_call(encrypted_reply, api_client) + + # Now that the call was successful, add the reply to the database locally. + source = session.query(Source).filter_by(uuid=self.source_uuid).one() + + reply_db_object = Reply( + uuid=self.reply_uuid, + source_id=source.id, + journalist_id=api_client.token_journalist_uuid, + filename=sdk_reply.filename, + ) + session.add(reply_db_object) + session.commit() + + return reply_db_object.uuid + + def _make_call(self, encrypted_reply: str, api_client: API) -> sdclientapi.Reply: + sdk_source = sdclientapi.Source(uuid=self.source_uuid) + return api_client.reply_source(sdk_source, encrypted_reply, + self.reply_uuid) diff --git a/securedrop_client/logic.py b/securedrop_client/logic.py index 626e4c901..13f82737d 100644 --- a/securedrop_client/logic.py +++ b/securedrop_client/logic.py @@ -21,7 +21,6 @@ import logging import os import sdclientapi -import traceback import uuid from gettext import gettext as _ @@ -32,9 +31,11 @@ from securedrop_client import storage from securedrop_client import db +from securedrop_client.api_jobs.downloads import DownloadSubmissionJob +from securedrop_client.api_jobs.uploads import SendReplyJob from securedrop_client.crypto import GpgHelper, CryptoError from securedrop_client.message_sync import MessageSync, ReplySync -from securedrop_client.queue import ApiJobQueue, DownloadSubmissionJob +from securedrop_client.queue import ApiJobQueue from securedrop_client.utils import check_dir_permissions logger = logging.getLogger(__name__) @@ -578,48 +579,25 @@ def delete_source(self, source): source ) - def send_reply(self, source_uuid: str, msg_uuid: str, message: str) -> None: - sdk_source = sdclientapi.Source(uuid=source_uuid) - - try: - encrypted_reply = self.gpg.encrypt_to_source(source_uuid, message) - except Exception: - tb = traceback.format_exc() - logger.error('Failed to encrypt to source {}:\n'.format(source_uuid, tb)) - self.reply_failed.emit(msg_uuid) - else: - # Guard against calling the API if we're not logged in - if self.api: - self.call_api( - self.api.reply_source, - self.on_reply_success, - self.on_reply_failure, - sdk_source, - encrypted_reply, - msg_uuid, - current_object=(source_uuid, msg_uuid), - ) - else: # pragma: no cover - logger.error('not logged in - not implemented!') - self.reply_failed.emit(msg_uuid) - - def on_reply_success(self, result, current_object: Tuple[str, str]) -> None: - source_uuid, reply_uuid = current_object - source = self.session.query(db.Source).filter_by(uuid=source_uuid).one() - - reply_db_object = db.Reply( - uuid=result.uuid, - source_id=source.id, - journalist_id=self.api.token_journalist_uuid, - filename=result.filename, + def send_reply(self, source_uuid: str, reply_uuid: str, message: str) -> None: + """ + Send a reply to a source. + """ + job = SendReplyJob( + source_uuid, + reply_uuid, + message, + self.gpg, ) - self.session.add(reply_db_object) - self.session.commit() + job.success_signal.connect(self.on_reply_success, type=Qt.QueuedConnection) + job.failure_signal.connect(self.on_reply_failure, type=Qt.QueuedConnection) + + self.api_job_queue.enqueue(job) + def on_reply_success(self, reply_uuid: str) -> None: self.reply_succeeded.emit(reply_uuid) - def on_reply_failure(self, result, current_object: Tuple[str, str]) -> None: - source_uuid, reply_uuid = current_object + def on_reply_failure(self, reply_uuid: str) -> None: self.reply_failed.emit(reply_uuid) def get_file(self, file_uuid: str) -> db.File: From ea5b57bf80c3baaa754ae135fbba07b716b7d838 Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Tue, 4 Jun 2019 15:39:36 -0700 Subject: [PATCH 2/3] test: add unit tests for SendReplyJob, update existing tests --- tests/api_jobs/test_uploads.py | 97 +++++++++++++++++++++++++++ tests/test_logic.py | 115 ++++++++++----------------------- 2 files changed, 130 insertions(+), 82 deletions(-) create mode 100644 tests/api_jobs/test_uploads.py diff --git a/tests/api_jobs/test_uploads.py b/tests/api_jobs/test_uploads.py new file mode 100644 index 000000000..f57f5d3fb --- /dev/null +++ b/tests/api_jobs/test_uploads.py @@ -0,0 +1,97 @@ +import pytest +import sdclientapi + +from securedrop_client import db +from securedrop_client.api_jobs.uploads import SendReplyJob +from securedrop_client.crypto import GpgHelper, CryptoError +from tests import factory + + +def test_send_reply_success(homedir, mocker, session, session_maker): + ''' + Check that the "happy path" of encrypting a message and sending it to the + server behaves as expected. + ''' + source = factory.Source() + session.add(source) + session.commit() + + gpg = GpgHelper(homedir, session_maker, is_qubes=False) + + api_client = mocker.MagicMock() + api_client.token_journalist_uuid = 'journalist ID sending the reply' + + encrypted_reply = 's3kr1t m3ss1dg3' + mock_encrypt = mocker.patch.object(gpg, 'encrypt_to_source', return_value=encrypted_reply) + msg_uuid = 'xyz456' + msg = 'wat' + + mock_reply_response = sdclientapi.Reply(uuid=msg_uuid, filename='5-dummy-reply') + api_client.reply_source = mocker.MagicMock() + api_client.reply_source.return_value = mock_reply_response + + mock_sdk_source = mocker.Mock() + mock_source_init = mocker.patch('securedrop_client.logic.sdclientapi.Source', + return_value=mock_sdk_source) + + job = SendReplyJob( + source.uuid, + msg_uuid, + msg, + gpg, + ) + + job.call_api(api_client, session) + + # ensure message gets encrypted + mock_encrypt.assert_called_once_with(source.uuid, msg) + mock_source_init.assert_called_once_with(uuid=source.uuid) + + # assert reply got added to db + reply = session.query(db.Reply).filter_by(uuid=msg_uuid).one() + assert reply.journalist_id == api_client.token_journalist_uuid + + +def test_send_reply_failure_gpg_error(homedir, mocker, session, session_maker): + ''' + Check that if gpg fails when sending a message, we do not call the API. + ''' + source = factory.Source() + session.add(source) + session.commit() + + gpg = GpgHelper(homedir, session_maker, is_qubes=False) + + api_client = mocker.MagicMock() + api_client.token_journalist_uuid = 'journalist ID sending the reply' + + mock_encrypt = mocker.patch.object(gpg, 'encrypt_to_source', side_effect=CryptoError) + msg_uuid = 'xyz456' + msg = 'wat' + + mock_reply_response = sdclientapi.Reply(uuid=msg_uuid, filename='5-dummy-reply') + api_client.reply_source = mocker.MagicMock() + api_client.reply_source.return_value = mock_reply_response + + mock_sdk_source = mocker.Mock() + mock_source_init = mocker.patch('securedrop_client.logic.sdclientapi.Source', + return_value=mock_sdk_source) + + job = SendReplyJob( + source.uuid, + msg_uuid, + msg, + gpg, + ) + + # Ensure that the CryptoError is raised so we can handle it in ApiJob._do_call_api + with pytest.raises(CryptoError): + job.call_api(api_client, session) + + # Ensure we attempted to encrypt the message + mock_encrypt.assert_called_once_with(source.uuid, msg) + assert mock_source_init.call_count == 0 + + # Ensure reply did not get added to db + replies = session.query(db.Reply).filter_by(uuid=msg_uuid).all() + assert len(replies) == 0 diff --git a/tests/test_logic.py b/tests/test_logic.py index c800c1a17..659e62f94 100644 --- a/tests/test_logic.py +++ b/tests/test_logic.py @@ -975,89 +975,48 @@ def test_Controller_delete_source(homedir, config, mocker, session_maker): ) -def test_Controller_send_reply_success(homedir, mocker, session_maker): +def test_Controller_send_reply_success(homedir, config, mocker, session_maker, session): ''' - Check that the "happy path" of encrypting a message and sending it to the sever behaves as - expected. + Check that a SendReplyJob is submitted to the queue when send_reply is called. ''' mock_gui = mocker.MagicMock() - co = Controller('http://localhost', mock_gui, session_maker, homedir) - co.call_api = mocker.Mock() - co.api = mocker.Mock() - encrypted_reply = 's3kr1t m3ss1dg3' - mock_encrypt = mocker.patch.object(co.gpg, 'encrypt_to_source', return_value=encrypted_reply) - source_uuid = 'abc123' - msg_uuid = 'xyz456' - msg = 'wat' - mock_sdk_source = mocker.Mock() - mock_source_init = mocker.patch('securedrop_client.logic.sdclientapi.Source', - return_value=mock_sdk_source) - - co.send_reply(source_uuid, msg_uuid, msg) - - # ensure message is encrypted - mock_encrypt.assert_called_once_with(source_uuid, msg) - - # ensure api is called - co.call_api.assert_called_once_with( - co.api.reply_source, - co.on_reply_success, - co.on_reply_failure, - mock_sdk_source, - encrypted_reply, - msg_uuid, - current_object=(source_uuid, msg_uuid), - ) - - assert mock_source_init.called # to prevent stale mocks - - -def test_Controller_send_reply_gpg_error(homedir, mocker, session_maker): - ''' - Check that if gpg fails when sending a message, we alert the UI and do *not* call the API. - ''' - mock_gui = mocker.MagicMock() + mock_success_signal = mocker.MagicMock() + mock_failure_signal = mocker.MagicMock() + mock_job = mocker.MagicMock(success_signal=mock_success_signal, + failure_signal=mock_failure_signal) + mock_job_cls = mocker.patch( + "securedrop_client.logic.SendReplyJob", return_value=mock_job) + mock_queue = mocker.patch.object(co, 'api_job_queue') - co = Controller('http://localhost', mock_gui, session_maker, homedir) + source = factory.Source() + session.add(source) + session.commit() - co.call_api = mocker.Mock() - co.api = mocker.Mock() - mock_encrypt = mocker.patch.object(co.gpg, 'encrypt_to_source', side_effect=Exception) - source_uuid = 'abc123' msg_uuid = 'xyz456' msg = 'wat' - mock_sdk_source = mocker.Mock() - mock_source_init = mocker.patch('securedrop_client.logic.sdclientapi.Source', - return_value=mock_sdk_source) - mock_reply_failed = mocker.patch.object(co, 'reply_failed') - co.send_reply(source_uuid, msg_uuid, msg) + co.send_reply(source.uuid, msg_uuid, msg) - # ensure there is an attempt to encrypt the message - mock_encrypt.assert_called_once_with(source_uuid, msg) - - # ensure we emit a failure on gpg errors - mock_reply_failed.emit.assert_called_once_with(msg_uuid) - - # ensure api not is called after a gpg error - assert not co.call_api.called + mock_job_cls.assert_called_once_with( + source.uuid, + msg_uuid, + msg, + co.gpg, + ) - assert mock_source_init.called # to prevent stale mocks + mock_queue.enqueue.assert_called_once_with(mock_job) + mock_success_signal.connect.assert_called_once_with( + co.on_reply_success, type=Qt.QueuedConnection) + mock_failure_signal.connect.assert_called_once_with( + co.on_reply_failure, type=Qt.QueuedConnection) def test_Controller_on_reply_success(homedir, mocker, session_maker, session): ''' - Check that when the result is a success, the client emits the correct signal. + Check that when the method is called, the client emits the correct signal. ''' - user = factory.User() - source = factory.Source() - msg = factory.Message(source=source) - session.add(user) - session.add(source) - session.add(msg) - session.commit() mock_gui = mocker.MagicMock() @@ -1066,38 +1025,30 @@ def test_Controller_on_reply_success(homedir, mocker, session_maker, session): reply = sdclientapi.Reply(uuid='wat', filename='1-lol') - co.api.token_journalist_uuid = user.uuid mock_reply_succeeded = mocker.patch.object(co, 'reply_succeeded') mock_reply_failed = mocker.patch.object(co, 'reply_failed') - current_object = (source.uuid, msg.uuid) - co.on_reply_success(reply, current_object) - mock_reply_succeeded.emit.assert_called_once_with(msg.uuid) + co.on_reply_success(reply.uuid) + mock_reply_succeeded.emit.assert_called_once_with(reply.uuid) assert not mock_reply_failed.emit.called - # check that this was writtent to the DB - replies = session.query(db.Reply).all() - assert len(replies) == 1 - def test_Controller_on_reply_failure(homedir, mocker, session_maker): ''' - Check that when the result is a failure, the client emits the correct signal. + Check that when the method is called, the client emits the correct signal. ''' mock_gui = mocker.MagicMock() co = Controller('http://localhost', mock_gui, session_maker, homedir) co.api = mocker.Mock() - journalist_uuid = 'abc123' - co.api.token_journalist_uuid = journalist_uuid + + reply = sdclientapi.Reply(uuid='wat', filename='1-lol') + mock_reply_succeeded = mocker.patch.object(co, 'reply_succeeded') mock_reply_failed = mocker.patch.object(co, 'reply_failed') - source_uuid = 'foo111' - msg_uuid = 'bar222' - current_object = (source_uuid, msg_uuid) - co.on_reply_failure(Exception, current_object) - mock_reply_failed.emit.assert_called_once_with(msg_uuid) + co.on_reply_failure(reply.uuid) + mock_reply_failed.emit.assert_called_once_with(reply.uuid) assert not mock_reply_succeeded.emit.called From 8b9ec4c4b284151390a493295ad92ab6f4b8e124 Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Wed, 5 Jun 2019 16:22:12 -0700 Subject: [PATCH 3/3] app: ensure replies persist in conv view when switching sources two changes: 1. We should mark replies that we send as downloaded/decrypted so that we don't keep redownloading them. 2. We should refresh the source so that we get all items from the collection when we redraw it. --- securedrop_client/api_jobs/uploads.py | 4 +++- securedrop_client/gui/widgets.py | 2 +- securedrop_client/logic.py | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/securedrop_client/api_jobs/uploads.py b/securedrop_client/api_jobs/uploads.py index 0589ba680..765cfe7e6 100644 --- a/securedrop_client/api_jobs/uploads.py +++ b/securedrop_client/api_jobs/uploads.py @@ -48,10 +48,12 @@ def call_api(self, api_client: API, session: Session) -> str: source_id=source.id, journalist_id=api_client.token_journalist_uuid, filename=sdk_reply.filename, + content=self.message, + is_downloaded=True, + is_decrypted=True ) session.add(reply_db_object) session.commit() - return reply_db_object.uuid def _make_call(self, encrypted_reply: str, api_client: API) -> sdclientapi.Reply: diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index cc8fb656e..474ff32b3 100644 --- a/securedrop_client/gui/widgets.py +++ b/securedrop_client/gui/widgets.py @@ -1390,9 +1390,9 @@ def clear_conversation(self): def update_conversation(self, collection: list) -> None: # clear all old items self.clear_conversation() + self.controller.session.refresh(self.source) # add new items for conversation_item in collection: - self.controller.session.refresh(conversation_item) if conversation_item.filename.endswith('msg.gpg'): self.add_message(conversation_item) elif conversation_item.filename.endswith('reply.gpg'): diff --git a/securedrop_client/logic.py b/securedrop_client/logic.py index 13f82737d..a88b15bc8 100644 --- a/securedrop_client/logic.py +++ b/securedrop_client/logic.py @@ -595,9 +595,11 @@ def send_reply(self, source_uuid: str, reply_uuid: str, message: str) -> None: self.api_job_queue.enqueue(job) def on_reply_success(self, reply_uuid: str) -> None: + logger.debug('Reply send success: {}'.format(reply_uuid)) self.reply_succeeded.emit(reply_uuid) def on_reply_failure(self, reply_uuid: str) -> None: + logger.debug('Reply send failure: {}'.format(reply_uuid)) self.reply_failed.emit(reply_uuid) def get_file(self, file_uuid: str) -> db.File: