From 38466101c5e66daead765a9f2654ad00d59d8076 Mon Sep 17 00:00:00 2001 From: "Nicholas H.Tollervey" Date: Wed, 24 Oct 2018 17:23:33 +0100 Subject: [PATCH 1/4] WiP for file download. Behaviour for decryption needs to be decided for further progress. --- Pipfile.lock | 23 ++++++++------------ securedrop_client/app.py | 1 + securedrop_client/gui/main.py | 1 + securedrop_client/gui/widgets.py | 24 ++++++++++++++++++--- securedrop_client/logic.py | 35 +++++++++++++++++++++++++++++++ securedrop_client/utils.py | 2 +- tests/gui/test_main.py | 1 + tests/gui/test_widgets.py | 35 +++++++++++++++++++++++++++++-- tests/test_logic.py | 36 ++++++++++++++++++++++++++++++++ 9 files changed, 138 insertions(+), 20 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 6fb1c66f6..2a22287ac 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -118,7 +118,6 @@ "sha256:41c3db2fc01e5b907288010dec72f9d0a74e37d6994e6eb56849f59fea2265ae", "sha256:8819bba37a02d143296a4d032373c4dd4aca11f6d4c9973335ca75f9c8475f59" ], - "markers": "python_version != '3.2.*' and python_version != '3.3.*' and python_version != '3.1.*' and python_version < '4' and python_version != '3.0.*' and python_version >= '2.7'", "version": "==1.24" } }, @@ -128,7 +127,6 @@ "sha256:0312ad34fcad8fac3704d441f7b317e50af620823353ec657a53e981f92920c0", "sha256:ec9ae8adaae229e4f8446952d204a3e4b5fdd2d099f9be3aaf556120135fb3ee" ], - "markers": "python_version != '3.3.*' and python_version != '3.0.*' and python_version >= '2.7' and python_version != '3.2.*' and python_version != '3.1.*'", "version": "==1.2.1" }, "attrs": { @@ -143,7 +141,6 @@ "sha256:2335065e6395b9e67ca716de5f7526736bfa6ceead690adf616d925bdc622b13", "sha256:5b94b49521f6456670fdb30cd82a4eca9412788a93fa6dd6df72c94d5a8ff2d7" ], - "markers": "python_version != '3.3.*' and python_version != '3.0.*' and python_version >= '2.7' and python_version != '3.2.*' and python_version != '3.1.*'", "version": "==7.0" }, "coverage": { @@ -187,11 +184,11 @@ }, "flake8": { "hashes": [ - "sha256:7253265f7abd8b313e3892944044a365e3f4ac3fcdcfb4298f55ee9ddf188ba0", - "sha256:c7841163e2b576d435799169b78703ad6ac1bbb0f199994fc05f700b2a90ea37" + "sha256:6a35f5b8761f45c5513e3405f110a86bea57982c3b75b766ce7b65217abe1670", + "sha256:c01f8a3963b3571a8e6bd7a4063359aff90749e160778e03817cd9b71c9e07d2" ], "index": "pypi", - "version": "==3.5.0" + "version": "==3.6.0" }, "mccabe": { "hashes": [ @@ -229,7 +226,6 @@ "sha256:447ba94990e8014ee25ec853339faf7b0fc8050cdc3289d4d71f7f410fb90095", "sha256:bde19360a8ec4dfd8a20dcb811780a30998101f078fc7ded6162f0076f50508f" ], - "markers": "python_version != '3.3.*' and python_version != '3.0.*' and python_version >= '2.7' and python_version != '3.2.*' and python_version != '3.1.*'", "version": "==0.8.0" }, "py": { @@ -237,22 +233,21 @@ "sha256:bf92637198836372b520efcba9e020c330123be8ce527e535d185ed4b6f45694", "sha256:e76826342cefe3c3d5f7e8ee4316b80d1dd8a300781612ddbc765c17ba25a6c6" ], - "markers": "python_version != '3.3.*' and python_version != '3.0.*' and python_version >= '2.7' and python_version != '3.2.*' and python_version != '3.1.*'", "version": "==1.7.0" }, "pycodestyle": { "hashes": [ - "sha256:682256a5b318149ca0d2a9185d365d8864a768a28db66a84a2ea946bcc426766", - "sha256:6c4245ade1edfad79c3446fadfc96b0de2759662dc29d07d80a6f27ad1ca6ba9" + "sha256:cbc619d09254895b0d12c2c691e237b2e91e9b2ecf5e84c26b35400f93dcfb83", + "sha256:cbfca99bd594a10f674d0cd97a3d802a1fdef635d4361e1a2658de47ed261e3a" ], - "version": "==2.3.1" + "version": "==2.4.0" }, "pyflakes": { "hashes": [ - "sha256:08bd6a50edf8cffa9fa09a463063c425ecaaf10d1eb0335a7e8b1401aef89e6f", - "sha256:8d616a382f243dbf19b54743f280b80198be0bca3a5396f1d2e1fca6223e8805" + "sha256:9a7662ec724d0120012f6e29d6248ae3727d821bba522a0e6b356eff19126a49", + "sha256:f661252913bc1dbe7fcfcbf0af0db3f42ab65aabd1a6ca68fe5d466bace94dae" ], - "version": "==1.6.0" + "version": "==2.0.0" }, "pyqt5": { "hashes": [ diff --git a/securedrop_client/app.py b/securedrop_client/app.py index faf9f1890..0ddd61e23 100644 --- a/securedrop_client/app.py +++ b/securedrop_client/app.py @@ -40,6 +40,7 @@ def init(sdc_home: str) -> None: safe_mkdir(sdc_home) + safe_mkdir(sdc_home, 'data') def excepthook(*exc_args): diff --git a/securedrop_client/gui/main.py b/securedrop_client/gui/main.py index de7299af9..a54f19577 100644 --- a/securedrop_client/gui/main.py +++ b/securedrop_client/gui/main.py @@ -157,6 +157,7 @@ def show_conversation_for(self, source): TODO: Finish this... """ conversation = ConversationView(self) + conversation.setup(self.controller) conversation.add_message('Source name: {}'.format( source.journalist_designation)) conversation.add_message('Hello, hello, is this thing switched on?') diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index 89a17965b..8794a7a84 100644 --- a/securedrop_client/gui/widgets.py +++ b/securedrop_client/gui/widgets.py @@ -429,10 +429,14 @@ class FileWidget(QWidget): Represents a file attached to a message. """ - def __init__(self, text, align): + def __init__(self, text, align, message, controller): """ + Given some text, an indication of alignment ('left' or 'right') and + a reference to the controller, make something to display a file. """ super().__init__() + self.message = message + self.controller = controller layout = QHBoxLayout() icon = QLabel() icon.setPixmap(load_image('file.png')) @@ -447,6 +451,12 @@ def __init__(self, text, align): layout.addStretch(5) self.setLayout(layout) + def mouseDoubleClickEvent(self, e): + """ + Handle a double-click via the program logic. + """ + self.controller.on_file_click(self.message) + class ConversationView(QWidget): """ @@ -470,6 +480,12 @@ def __init__(self, parent): main_layout.addWidget(scroll) self.setLayout(main_layout) + def setup(self, controller): + """ + Ensure there's a reference to program logic. + """ + self.controller = controller + def add_message(self, message, files=None): """ Add a message from the source. @@ -477,7 +493,8 @@ def add_message(self, message, files=None): self.conversation_layout.addWidget(MessageWidget(message)) if files: for f in files: - self.conversation_layout.addWidget(FileWidget(f, 'left')) + self.conversation_layout.addWidget(FileWidget(f, 'left', + message, self.controller)) def add_reply(self, reply, files=None): """ @@ -486,4 +503,5 @@ def add_reply(self, reply, files=None): self.conversation_layout.addWidget(ReplyWidget(reply)) if files: for f in files: - self.conversation_layout.addWidget(FileWidget(f, 'right')) + self.conversation_layout.addWidget(FileWidget(f, 'right', + reply, self.controller)) diff --git a/securedrop_client/logic.py b/securedrop_client/logic.py index 7c316dbb7..26a719a11 100644 --- a/securedrop_client/logic.py +++ b/securedrop_client/logic.py @@ -100,6 +100,8 @@ def __init__(self, hostname, gui, session, home: str) -> None: self.session = session # Reference to the SqlAlchemy session. self.api_thread = None # Currently active API call thread. self.sync_flag = os.path.join(home, 'sync_flag') + self.home = home # The "home" directory for client files. + self.data_dir = os.path.join(self.home, 'data') # File data. def setup(self): """ @@ -313,3 +315,36 @@ def set_status(self, message, duration=5000): duration. """ self.gui.set_status(message, duration) + + def on_file_click(self, message): + """ + Download the file associated with the associated message (which may + be a Submission or Reply). + """ + if isinstance(message, sdclientapi.Submission): + # Handle sources. + func = self.api.download_submission + else: + # Handle journalist's replies. + func = self.api.download_reply + self.call_api(func, self.on_file_download, + self.on_download_timeout, message, self.data_dir) + + def on_file_download(self): + """ + Called when a file has downloaded. Cause a refresh to the conversation + view to display the contents of the new file. + """ + if result: # pragma: no cover + # Refresh the conversation with the content of the downloaded file. + pass + else: # pragma: no cover + # Update the UI in some way to indicate a failure state. + pass + + def on_download_timeout(self): + """ + Called when downloading a file has timed out. + """ + # Update the UI in some way to indicate a failure state. + pass # pragma: no cover diff --git a/securedrop_client/utils.py b/securedrop_client/utils.py index f946c814d..15fb2fb36 100644 --- a/securedrop_client/utils.py +++ b/securedrop_client/utils.py @@ -1,7 +1,7 @@ import os -def safe_mkdir(sdc_home: str, relative_path: str=None) -> None: +def safe_mkdir(sdc_home: str, relative_path: str = None) -> None: ''' Safely create directories while checking permissions along the way. ''' diff --git a/tests/gui/test_main.py b/tests/gui/test_main.py index 8cfb276b1..f30d2ba37 100644 --- a/tests/gui/test_main.py +++ b/tests/gui/test_main.py @@ -184,6 +184,7 @@ def test_conversation_for(): is called in the expected manner with dummy data. """ w = Window() + w.controller = mock.MagicMock() w.main_view = mock.MagicMock() mock_conview = mock.MagicMock() mock_source = mock.MagicMock() diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index a37510c41..6f1f37b39 100644 --- a/tests/gui/test_widgets.py +++ b/tests/gui/test_widgets.py @@ -362,22 +362,41 @@ def test_FileWidget_init_left(): """ Check the FileWidget is configured correctly for align-left. """ - fw = FileWidget('hello', 'left') + mock_message = mock.MagicMock() + mock_controller = mock.MagicMock() + fw = FileWidget('hello', 'left', mock_message, mock_controller) layout = fw.layout() assert isinstance(layout.takeAt(0), QWidgetItem) assert isinstance(layout.takeAt(0), QWidgetItem) assert isinstance(layout.takeAt(0), QSpacerItem) + assert fw.message == mock_message + assert fw.controller == mock_controller def test_FileWidget_init_right(): """ Check the FileWidget is configured correctly for align-right. """ - fw = FileWidget('hello', 'right') + mock_message = mock.MagicMock() + mock_controller = mock.MagicMock() + fw = FileWidget('hello', 'right', mock_message, mock_controller) layout = fw.layout() assert isinstance(layout.takeAt(0), QSpacerItem) assert isinstance(layout.takeAt(0), QWidgetItem) assert isinstance(layout.takeAt(0), QWidgetItem) + assert fw.message == mock_message + assert fw.controller == mock_controller + + +def test_FileWidget_mouseDoubleClickEvent(): + """ + Should fire the expected event handler in the logic layer. + """ + mock_message = mock.MagicMock() + mock_controller = mock.MagicMock() + fw = FileWidget('hello', 'right', mock_message, mock_controller) + fw.mouseDoubleClickEvent(None) + fw.controller.on_file_click.assert_called_once_with(mock_message) def test_ConversationView_init(): @@ -388,12 +407,23 @@ def test_ConversationView_init(): assert isinstance(cv.conversation_layout, QVBoxLayout) +def test_ConversationView_setup(): + """ + Ensure the controller is set + """ + cv = ConversationView(None) + mock_controller = mock.MagicMock() + cv.setup(mock_controller) + assert cv.controller == mock_controller + + def test_ConversationView_add_message(): """ Adding a message results in a new MessageWidget added to the layout. Any associated files are added as FileWidgets. """ cv = ConversationView(None) + cv.controller = mock.MagicMock() cv.conversation_layout = mock.MagicMock() cv.add_message('hello', ['file1.pdf', ]) assert cv.conversation_layout.addWidget.call_count == 2 @@ -408,6 +438,7 @@ def test_ConversationView_add_reply(): associated files are added as FileWidgets. """ cv = ConversationView(None) + cv.controller = mock.MagicMock() cv.conversation_layout = mock.MagicMock() cv.add_reply('hello', ['file1.pdf', ]) assert cv.conversation_layout.addWidget.call_count == 2 diff --git a/tests/test_logic.py b/tests/test_logic.py index 4a4ac3c13..86096c0ad 100644 --- a/tests/test_logic.py +++ b/tests/test_logic.py @@ -566,3 +566,39 @@ def func() -> None: else: with pytest.raises(RuntimeError): func() + + +def test_Client_on_file_click_Submission(safe_tmpdir): + """ + If the handler is passed a submission, check the download_submission + function is the one called against the API. + """ + mock_gui = mock.MagicMock() + mock_session = mock.MagicMock() + cl = Client('http://localhost', mock_gui, mock_session, str(safe_tmpdir)) + s = sdclientapi.Submission(download_url='foo', filename='test', + is_read=True, size=123, source_url='foo/bar', + submission_url='bar', uuid='test') + cl.call_api = mock.MagicMock() + cl.api = mock.MagicMock() + cl.on_file_click(s) + cl.call_api.assert_called_once_with(cl.api.download_submission, + cl.on_file_download, + cl.on_download_timeout, s, cl.data_dir) + + +def test_Client_on_file_click_Reply(safe_tmpdir): + """ + If the handler is passed a reply, check the download_reply + function is the one called against the API. + """ + mock_gui = mock.MagicMock() + mock_session = mock.MagicMock() + cl = Client('http://localhost', mock_gui, mock_session, str(safe_tmpdir)) + r = mock.MagicMock() # Not a sdclientapi.Submission + cl.call_api = mock.MagicMock() + cl.api = mock.MagicMock() + cl.on_file_click(r) + cl.call_api.assert_called_once_with(cl.api.download_reply, + cl.on_file_download, + cl.on_download_timeout, r, cl.data_dir) From 16df2572b9074352852b6e3f75b3c831eef9d25b Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Thu, 25 Oct 2018 15:35:43 -0700 Subject: [PATCH 2/4] Populate conversation view with files, add rest of download logic --- Pipfile.lock | 10 ++-- securedrop_client/gui/main.py | 20 +++++-- securedrop_client/gui/widgets.py | 38 ++++++++----- securedrop_client/logic.py | 34 +++++++----- securedrop_client/models.py | 10 ++++ securedrop_client/storage.py | 12 +++++ securedrop_client/utils.py | 14 +++++ tests/gui/test_main.py | 11 +++- tests/gui/test_widgets.py | 92 +++++++++++++++++++++++++------- tests/test_logic.py | 86 +++++++++++++++++++++++++---- tests/test_models.py | 18 +++++++ tests/test_storage.py | 14 ++++- tests/test_utils.py | 20 ++++++- 13 files changed, 311 insertions(+), 68 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 2a22287ac..1ffeb02cd 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -74,10 +74,10 @@ }, "python-dateutil": { "hashes": [ - "sha256:1adb80e7a782c12e52ef9a8182bebeb73f1d7e24e374397af06fb4956c8dc5c0", - "sha256:e27001de32f627c22380a688bcc43ce83504a7bc5da472209b4c70f02829f0b8" + "sha256:2f13d3ea236aeb237e7258d5729c46eafe1506fd7f8507f34730734ed8b37454", + "sha256:f7cde3aecf8a797553d6ec49b65f0fbcffe7ffb971ccac452d181c28fd279936" ], - "version": "==2.7.3" + "version": "==2.7.4" }, "python-editor": { "hashes": [ @@ -94,10 +94,10 @@ }, "securedrop-sdk": { "hashes": [ - "sha256:e4f716077ceb52fe840f5b2033c3174f65beef6e94e48671c50af326a3ed16da" + "sha256:5e3ebfde6ef63fc9a614da5b3b9820a93b827f2f7ecb4a72178ebe6d8e2f6d2a" ], "index": "pypi", - "version": "==0.0.2" + "version": "==0.0.3" }, "six": { "hashes": [ diff --git a/securedrop_client/gui/main.py b/securedrop_client/gui/main.py index a54f19577..5c76994f7 100644 --- a/securedrop_client/gui/main.py +++ b/securedrop_client/gui/main.py @@ -154,12 +154,25 @@ def on_source_changed(self): def show_conversation_for(self, source): """ - TODO: Finish this... + Show conversation of messages and replies between a source and + journalists. """ conversation = ConversationView(self) conversation.setup(self.controller) conversation.add_message('Source name: {}'.format( source.journalist_designation)) + + # Display each conversation item in the source collection. + for conversation_item in source.collection: + if conversation_item.filename.endswith('msg.gpg'): + # TODO: Decrypt and display the message + pass + elif conversation_item.filename.endswith('reply.gpg'): + # TODO: Decrypt and display the reply + pass + else: + conversation.add_file(source, conversation_item) + conversation.add_message('Hello, hello, is this thing switched on?') conversation.add_reply('Yes, I can hear you loud and clear!') conversation.add_reply('How can I help?') @@ -194,10 +207,9 @@ def show_conversation_for(self, source): "into this. Also: someone I work with heard " "him on the phone once, talking about his " "'time' at Jackson—that contradicts his " - "resume. It really seems fishy.", - ['fishy_cv.PDF (234Kb)', ]) + "resume. It really seems fishy.") conversation.add_reply("THIS IS IT THIS IS THE TAPE EVERYONE'S " - "LOOKING FOR!!!", ['filename.pdf (32Kb)', ]) + "LOOKING FOR!!!") conversation.add_reply("Hello: I read your story on Sally Dale, and " "her lawsuit against the St. Joseph's " "Orphanage. My great-aunt was one of the nuns " diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index 8794a7a84..91eb51875 100644 --- a/securedrop_client/gui/widgets.py +++ b/securedrop_client/gui/widgets.py @@ -25,6 +25,7 @@ QPushButton, QVBoxLayout, QLineEdit, QScrollArea, QPlainTextEdit, QSpacerItem, QSizePolicy, QDialog) from securedrop_client.resources import load_svg, load_image +from securedrop_client.utils import humanize_filesize logger = logging.getLogger(__name__) @@ -426,21 +427,30 @@ def __init__(self, message): class FileWidget(QWidget): """ - Represents a file attached to a message. + Represents a file. """ - def __init__(self, text, align, message, controller): + def __init__(self, source_db_object, submission_db_object, + controller, align="left"): """ Given some text, an indication of alignment ('left' or 'right') and a reference to the controller, make something to display a file. + + Align is set to left by default because currently SecureDrop can only + accept files from sources to journalists. """ super().__init__() - self.message = message self.controller = controller + self.source = source_db_object + self.submission = submission_db_object layout = QHBoxLayout() icon = QLabel() icon.setPixmap(load_image('file.png')) - description = QLabel(text) + if submission_db_object.is_downloaded: + description = QLabel("Open") + else: + human_filesize = humanize_filesize(self.submission.size) + description = QLabel("Download ({})".format(human_filesize)) if align is not "left": # Float right... layout.addStretch(5) @@ -455,7 +465,7 @@ def mouseDoubleClickEvent(self, e): """ Handle a double-click via the program logic. """ - self.controller.on_file_click(self.message) + self.controller.on_file_click(self.source, self.submission) class ConversationView(QWidget): @@ -486,22 +496,22 @@ def setup(self, controller): """ self.controller = controller - def add_message(self, message, files=None): + def add_file(self, source_db_object, submission_db_object): + """ + Add a file from the source. + """ + self.conversation_layout.addWidget( + FileWidget(source_db_object, submission_db_object, + self.controller)) + + def add_message(self, message): """ Add a message from the source. """ self.conversation_layout.addWidget(MessageWidget(message)) - if files: - for f in files: - self.conversation_layout.addWidget(FileWidget(f, 'left', - message, self.controller)) def add_reply(self, reply, files=None): """ Add a reply from a journalist. """ self.conversation_layout.addWidget(ReplyWidget(reply)) - if files: - for f in files: - self.conversation_layout.addWidget(FileWidget(f, 'right', - reply, self.controller)) diff --git a/securedrop_client/logic.py b/securedrop_client/logic.py index 26a719a11..f5cc1d030 100644 --- a/securedrop_client/logic.py +++ b/securedrop_client/logic.py @@ -21,6 +21,7 @@ import sdclientapi import arrow from securedrop_client import storage +from securedrop_client import models from securedrop_client.utils import check_dir_permissions from PyQt5.QtCore import QObject, QThread, pyqtSignal, QTimer @@ -316,35 +317,44 @@ def set_status(self, message, duration=5000): """ self.gui.set_status(message, duration) - def on_file_click(self, message): + def on_file_click(self, source_db_object, message): """ Download the file associated with the associated message (which may be a Submission or Reply). """ - if isinstance(message, sdclientapi.Submission): - # Handle sources. + if isinstance(message, models.Submission): + # Handle submissions. func = self.api.download_submission - else: + sdk_object = sdclientapi.Submission(uuid=message.uuid) + sdk_object.filename = message.filename + sdk_object.source_uuid = source_db_object.uuid + elif isinstance(message, models.Reply): # Handle journalist's replies. func = self.api.download_reply + sdk_object = sdclientapi.Reply(uuid=message.uuid) + sdk_object.filename = message.filename + sdk_object.source_uuid = source_db_object.uuid self.call_api(func, self.on_file_download, - self.on_download_timeout, message, self.data_dir) + self.on_download_timeout, sdk_object, self.data_dir) - def on_file_download(self): + def on_file_download(self, result): """ Called when a file has downloaded. Cause a refresh to the conversation view to display the contents of the new file. """ - if result: # pragma: no cover + sha256sum, filename = self.api_runner.result + self.call_reset() + if result: + storage.mark_file_as_downloaded(os.path.basename(filename), + self.session) # Refresh the conversation with the content of the downloaded file. - pass - else: # pragma: no cover + else: # Update the UI in some way to indicate a failure state. - pass + self.set_status("Failed to download file, please try again.") def on_download_timeout(self): """ Called when downloading a file has timed out. """ - # Update the UI in some way to indicate a failure state. - pass # pragma: no cover + # Update the status bar to indicate a failure state. + self.set_status("Connection to server timed out, please try again.") diff --git a/securedrop_client/models.py b/securedrop_client/models.py index 695d4be91..f5ac748bd 100644 --- a/securedrop_client/models.py +++ b/securedrop_client/models.py @@ -37,6 +37,16 @@ def __init__(self, uuid, journalist_designation, is_flagged, public_key, def __repr__(self): return ''.format(self.journalist_designation) + @property + def collection(self): + """Return the list of submissions and replies for this source, sorted + in ascending order by the filename/interaction count.""" + collection = [] + collection.extend(self.submissions) + collection.extend(self.replies) + collection.sort(key=lambda x: int(x.filename.split('-')[0])) + return collection + class Submission(Base): __tablename__ = 'submissions' diff --git a/securedrop_client/storage.py b/securedrop_client/storage.py index 41ec9b04a..fea6733c5 100644 --- a/securedrop_client/storage.py +++ b/securedrop_client/storage.py @@ -234,3 +234,15 @@ def find_or_create_user(uuid, username, session): session.add(new_user) session.commit() return new_user + + +def mark_file_as_downloaded(local_filename, session): + """Mark file as downloaded in the database. The file itself will be + stored in the data directory. + """ + submission_db_object = session.query(Submission) \ + .filter_by(filename=local_filename) \ + .one_or_none() + submission_db_object.is_downloaded = True + session.add(submission_db_object) + session.commit() diff --git a/securedrop_client/utils.py b/securedrop_client/utils.py index 15fb2fb36..b171ae224 100644 --- a/securedrop_client/utils.py +++ b/securedrop_client/utils.py @@ -1,4 +1,5 @@ import os +import math def safe_mkdir(sdc_home: str, relative_path: str = None) -> None: @@ -45,3 +46,16 @@ def split_path(path: str) -> list: out.reverse() return out + + +def humanize_filesize(filesize): + """ + Returns a human readable string of a filesize + (with an input unit of bytes) + """ + if filesize < 1024: + return '{} bytes'.format(str(filesize)) + elif filesize < 1024 * 1024: + return '{}KB'.format(math.floor(filesize / 1024)) + else: + return '{}MB'.format(math.floor(filesize / 1024 ** 2)) diff --git a/tests/gui/test_main.py b/tests/gui/test_main.py index f30d2ba37..9817c0027 100644 --- a/tests/gui/test_main.py +++ b/tests/gui/test_main.py @@ -180,8 +180,7 @@ def test_on_source_changed(): def test_conversation_for(): """ - TODO: Finish this once we have data. Currently checks only the GUI layer - is called in the expected manner with dummy data. + Test that the source collection is displayed in the conversation view. """ w = Window() w.controller = mock.MagicMock() @@ -189,12 +188,20 @@ def test_conversation_for(): mock_conview = mock.MagicMock() mock_source = mock.MagicMock() mock_source.journalistic_designation = 'Testy McTestface' + mock_file = mock.MagicMock() + mock_file.filename = '1-my-source-doc.gpg' + mock_message = mock.MagicMock() + mock_message.filename = '2-my-source-msg.gpg' + mock_reply = mock.MagicMock() + mock_reply.filename = '3-my-source-reply.gpg' + mock_source.collection = [mock_file, mock_message, mock_reply] with mock.patch('securedrop_client.gui.main.ConversationView', mock_conview): w.show_conversation_for(mock_source) conv = mock_conview() assert conv.add_message.call_count > 0 assert conv.add_reply.call_count > 0 + assert conv.add_file.call_count > 0 def test_set_status(): diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index 6f1f37b39..1a0c68618 100644 --- a/tests/gui/test_widgets.py +++ b/tests/gui/test_widgets.py @@ -1,8 +1,10 @@ """ Make sure the UI widgets are configured correctly and work as expected. """ +from datetime import datetime from PyQt5.QtWidgets import (QLineEdit, QWidget, QApplication, QWidgetItem, QSpacerItem, QVBoxLayout) +from securedrop_client import models from securedrop_client.gui.widgets import (ToolBar, MainView, SourceList, SourceWidget, LoginDialog, SpeechBubble, ConversationWidget, @@ -362,14 +364,19 @@ def test_FileWidget_init_left(): """ Check the FileWidget is configured correctly for align-left. """ - mock_message = mock.MagicMock() mock_controller = mock.MagicMock() - fw = FileWidget('hello', 'left', mock_message, mock_controller) + source = models.Source('source-uuid', 'testy-mctestface', False, + 'mah pub key', 1, False, datetime.now()) + submission = models.Submission(source, 'submission-uuid', 123, + 'mah-reply.gpg') + submission.is_downloaded = True + + fw = FileWidget(source, submission, mock_controller, align='left') + layout = fw.layout() assert isinstance(layout.takeAt(0), QWidgetItem) assert isinstance(layout.takeAt(0), QWidgetItem) assert isinstance(layout.takeAt(0), QSpacerItem) - assert fw.message == mock_message assert fw.controller == mock_controller @@ -377,14 +384,18 @@ def test_FileWidget_init_right(): """ Check the FileWidget is configured correctly for align-right. """ - mock_message = mock.MagicMock() mock_controller = mock.MagicMock() - fw = FileWidget('hello', 'right', mock_message, mock_controller) + source = models.Source('source-uuid', 'testy-mctestface', False, + 'mah pub key', 1, False, datetime.now()) + submission = models.Submission(source, 'submission-uuid', 123, + 'mah-reply.gpg') + submission.is_downloaded = True + + fw = FileWidget(source, submission, mock_controller, align='right') layout = fw.layout() assert isinstance(layout.takeAt(0), QSpacerItem) assert isinstance(layout.takeAt(0), QWidgetItem) assert isinstance(layout.takeAt(0), QWidgetItem) - assert fw.message == mock_message assert fw.controller == mock_controller @@ -394,9 +405,15 @@ def test_FileWidget_mouseDoubleClickEvent(): """ mock_message = mock.MagicMock() mock_controller = mock.MagicMock() - fw = FileWidget('hello', 'right', mock_message, mock_controller) + source = models.Source('source-uuid', 'testy-mctestface', False, + 'mah pub key', 1, False, datetime.now()) + submission = models.Submission(source, 'submission-uuid', 123, + 'mah-reply.gpg') + submission.is_downloaded = True + + fw = FileWidget(source, submission, mock_controller) fw.mouseDoubleClickEvent(None) - fw.controller.on_file_click.assert_called_once_with(mock_message) + fw.controller.on_file_click.assert_called_once_with(source, submission) def test_ConversationView_init(): @@ -419,29 +436,68 @@ def test_ConversationView_setup(): def test_ConversationView_add_message(): """ - Adding a message results in a new MessageWidget added to the layout. Any - associated files are added as FileWidgets. + Adding a message results in a new MessageWidget added to the layout. """ cv = ConversationView(None) cv.controller = mock.MagicMock() cv.conversation_layout = mock.MagicMock() - cv.add_message('hello', ['file1.pdf', ]) - assert cv.conversation_layout.addWidget.call_count == 2 + cv.add_message('hello') + assert cv.conversation_layout.addWidget.call_count == 1 cal = cv.conversation_layout.addWidget.call_args_list assert isinstance(cal[0][0][0], MessageWidget) - assert isinstance(cal[1][0][0], FileWidget) def test_ConversationView_add_reply(): """ - Adding a reply results in a new ReplyWidget added to the layout. Any - associated files are added as FileWidgets. + Adding a reply results in a new ReplyWidget added to the layout. """ cv = ConversationView(None) cv.controller = mock.MagicMock() cv.conversation_layout = mock.MagicMock() - cv.add_reply('hello', ['file1.pdf', ]) - assert cv.conversation_layout.addWidget.call_count == 2 + cv.add_reply('hello') + assert cv.conversation_layout.addWidget.call_count == 1 cal = cv.conversation_layout.addWidget.call_args_list assert isinstance(cal[0][0][0], ReplyWidget) - assert isinstance(cal[1][0][0], FileWidget) + + +def test_ConversationView_add_downloaded_file(): + """ + Adding a file results in a new FileWidget added to the layout with the + proper QLabel. + """ + cv = ConversationView(None) + cv.controller = mock.MagicMock() + cv.conversation_layout = mock.MagicMock() + mock_source = mock.MagicMock() + mock_file = mock.MagicMock() + mock_file.is_downloaded = True + with mock.patch('securedrop_client.gui.widgets.QLabel') as mock_label, \ + mock.patch('securedrop_client.gui.widgets.QHBoxLayout.addWidget'),\ + mock.patch('securedrop_client.gui.widgets.FileWidget.setLayout'): + cv.add_file(mock_source, mock_file) + mock_label.assert_called_with("Open") + assert cv.conversation_layout.addWidget.call_count == 1 + cal = cv.conversation_layout.addWidget.call_args_list + assert isinstance(cal[0][0][0], FileWidget) + + +def test_ConversationView_add_not_downloaded_file(): + """ + Adding a file results in a new FileWidget added to the layout with the + proper QLabel. + """ + cv = ConversationView(None) + cv.controller = mock.MagicMock() + cv.conversation_layout = mock.MagicMock() + mock_source = mock.MagicMock() + mock_file = mock.MagicMock() + mock_file.is_downloaded = False + mock_file.size = 123 + with mock.patch('securedrop_client.gui.widgets.QLabel') as mock_label, \ + mock.patch('securedrop_client.gui.widgets.QHBoxLayout.addWidget'),\ + mock.patch('securedrop_client.gui.widgets.FileWidget.setLayout'): + cv.add_file(mock_source, mock_file) + mock_label.assert_called_with("Download (123 bytes)") + assert cv.conversation_layout.addWidget.call_count == 1 + cal = cv.conversation_layout.addWidget.call_args_list + assert isinstance(cal[0][0][0], FileWidget) diff --git a/tests/test_logic.py b/tests/test_logic.py index 86096c0ad..707e7b392 100644 --- a/tests/test_logic.py +++ b/tests/test_logic.py @@ -3,10 +3,11 @@ expected. """ import arrow +from datetime import datetime import os import pytest import sdclientapi -from securedrop_client import storage +from securedrop_client import storage, models from securedrop_client.logic import APICallRunner, Client from unittest import mock @@ -576,15 +577,69 @@ def test_Client_on_file_click_Submission(safe_tmpdir): mock_gui = mock.MagicMock() mock_session = mock.MagicMock() cl = Client('http://localhost', mock_gui, mock_session, str(safe_tmpdir)) - s = sdclientapi.Submission(download_url='foo', filename='test', - is_read=True, size=123, source_url='foo/bar', - submission_url='bar', uuid='test') + source = models.Source('source-uuid', 'testy-mctestface', False, + 'mah pub key', 1, False, datetime.now()) + submission = models.Submission(source, 'submission-uuid', 1234, + 'myfile.doc.gpg') cl.call_api = mock.MagicMock() cl.api = mock.MagicMock() - cl.on_file_click(s) - cl.call_api.assert_called_once_with(cl.api.download_submission, - cl.on_file_download, - cl.on_download_timeout, s, cl.data_dir) + submission_sdk_object = mock.MagicMock() + with mock.patch('sdclientapi.Submission') as mock_submission: + mock_submission.return_value = submission_sdk_object + cl.on_file_click(source, submission) + cl.call_api.assert_called_once_with( + cl.api.download_submission, cl.on_file_download, + cl.on_download_timeout, submission_sdk_object, + cl.data_dir) + + +def test_Client_on_file_download_success(safe_tmpdir): + mock_gui = mock.MagicMock() + mock_session = mock.MagicMock() + cl = Client('http://localhost', mock_gui, mock_session, str(safe_tmpdir)) + cl.update_sources = mock.MagicMock() + cl.api_runner = mock.MagicMock() + test_filename = "my-file-location-msg.gpg" + cl.api_runner.result = ("", test_filename) + cl.call_reset = mock.MagicMock() + result = True + with mock.patch('securedrop_client.logic.storage') as mock_storage: + cl.on_file_download(result) + cl.call_reset.assert_called_once_with() + mock_storage.mark_file_as_downloaded.assert_called_once_with( + test_filename, mock_session) + + +def test_Client_on_file_download_failure(safe_tmpdir): + mock_gui = mock.MagicMock() + mock_session = mock.MagicMock() + cl = Client('http://localhost', mock_gui, mock_session, str(safe_tmpdir)) + cl.update_sources = mock.MagicMock() + cl.api_runner = mock.MagicMock() + test_filename = "my-file-location-msg.gpg" + cl.api_runner.result = ("", test_filename) + cl.call_reset = mock.MagicMock() + cl.set_status = mock.MagicMock() + result = False + cl.on_file_download(result) + cl.call_reset.assert_called_once_with() + cl.set_status.assert_called_once_with( + "Failed to download file, please try again.") + + +def test_Client_on_download_timeout(safe_tmpdir): + mock_gui = mock.MagicMock() + mock_session = mock.MagicMock() + cl = Client('http://localhost', mock_gui, mock_session, str(safe_tmpdir)) + cl.update_sources = mock.MagicMock() + cl.api_runner = mock.MagicMock() + test_filename = "my-file-location-msg.gpg" + cl.api_runner.result = ("", test_filename) + cl.call_reset = mock.MagicMock() + cl.set_status = mock.MagicMock() + cl.on_download_timeout() + cl.set_status.assert_called_once_with( + "Connection to server timed out, please try again.") def test_Client_on_file_click_Reply(safe_tmpdir): @@ -595,10 +650,19 @@ def test_Client_on_file_click_Reply(safe_tmpdir): mock_gui = mock.MagicMock() mock_session = mock.MagicMock() cl = Client('http://localhost', mock_gui, mock_session, str(safe_tmpdir)) - r = mock.MagicMock() # Not a sdclientapi.Submission + source = models.Source('source-uuid', 'testy-mctestface', False, + 'mah pub key', 1, False, datetime.now()) + journalist = models.User('Testy mcTestface') + reply = models.Reply('reply-uuid', journalist, source, + 'my-reply.gpg', 123) # Not a sdclientapi.Submission cl.call_api = mock.MagicMock() cl.api = mock.MagicMock() - cl.on_file_click(r) + reply_sdk_object = mock.MagicMock() + with mock.patch('sdclientapi.Reply') as mock_reply: + mock_reply.return_value = reply_sdk_object + cl.on_file_click(source, reply) cl.call_api.assert_called_once_with(cl.api.download_reply, cl.on_file_download, - cl.on_download_timeout, r, cl.data_dir) + cl.on_download_timeout, + reply_sdk_object, + cl.data_dir, current_object=reply) diff --git a/tests/test_models.py b/tests/test_models.py index f9ba6522e..e12cc3f27 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -30,3 +30,21 @@ def test_string_representation_of_reply(): reply = Reply(source=source, journalist=user, filename="reply.gpg", size=1234, uuid='test') reply.__repr__() + + +def test_source_collection(): + # Create some test submissions and replies + source = Source(journalist_designation="testy test", uuid="test", + is_flagged=False, public_key='test', interaction_count=1, + is_starred=False, last_updated='test') + submission = Submission(source=source, uuid="test", size=123, + filename="2-test.doc.gpg") + user = User('hehe') + reply = Reply(source=source, journalist=user, filename="1-reply.gpg", + size=1234, uuid='test') + source.submissions = [submission] + source.replies = [reply] + + # Now these items should be in the source collection in the proper order + assert source.collection[0] == reply + assert source.collection[1] == submission diff --git a/tests/test_storage.py b/tests/test_storage.py index f55ba6c8a..daa83cf4a 100644 --- a/tests/test_storage.py +++ b/tests/test_storage.py @@ -12,7 +12,8 @@ get_remote_data, update_local_storage, update_sources, update_submissions, update_replies, - find_or_create_user) + find_or_create_user, + mark_file_as_downloaded) from sdclientapi import Source, Submission, Reply @@ -352,3 +353,14 @@ def test_find_or_create_user_new(): assert new_user.username == 'unknown' mock_session.add.assert_called_once_with(new_user) mock_session.commit.assert_called_once_with() + + +def test_mark_file_as_downloaded(): + mock_session = mock.MagicMock() + mock_submission = mock.MagicMock() + mock_submission.is_downloaded is False + mock_session.query().filter_by().one_or_none.return_value = mock_submission + mark_file_as_downloaded('test-filename', mock_session) + assert mock_submission.is_downloaded is True + mock_session.add.assert_called_once_with(mock_submission) + mock_session.commit.assert_called_once_with() diff --git a/tests/test_utils.py b/tests/test_utils.py index 2737cf9a9..a9e5778f8 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,5 +1,5 @@ import pytest -from securedrop_client.utils import safe_mkdir +from securedrop_client.utils import safe_mkdir, humanize_filesize def test_safe_makedirs_non_absolute(safe_tmpdir): @@ -9,3 +9,21 @@ def test_safe_makedirs_non_absolute(safe_tmpdir): safe_mkdir(home_dir, '..') assert 'not absolute' in str(e_info.value) + + +def test_humanize_file_size_bytes(): + expected_humanized_filesize = "123 bytes" + actual_humanized_filesize = humanize_filesize(123) + assert expected_humanized_filesize == actual_humanized_filesize + + +def test_humanize_file_size_kilobytes(): + expected_humanized_filesize = "123KB" + actual_humanized_filesize = humanize_filesize(123 * 1024) + assert expected_humanized_filesize == actual_humanized_filesize + + +def test_humanize_file_size_megabytes(): + expected_humanized_filesize = "123MB" + actual_humanized_filesize = humanize_filesize(123 * 1024 * 1024) + assert expected_humanized_filesize == actual_humanized_filesize From b6993de64213cdf3eccea7f06a865a61abb39cd6 Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Fri, 26 Oct 2018 09:30:09 -0700 Subject: [PATCH 3/4] Platform independent storage of files in data dir --- securedrop_client/logic.py | 18 ++++++++++++++---- securedrop_client/storage.py | 4 ++-- tests/test_logic.py | 10 +++++++--- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/securedrop_client/logic.py b/securedrop_client/logic.py index f5cc1d030..2d67b7876 100644 --- a/securedrop_client/logic.py +++ b/securedrop_client/logic.py @@ -125,7 +125,8 @@ def setup(self): self.sync_timer.timeout.connect(self.update_sync) self.sync_timer.start(30000) - def call_api(self, function, callback, timeout, *args, **kwargs): + def call_api(self, function, callback, timeout, *args, current_object=None, + **kwargs): """ Calls the function in a non-blocking manner. Upon completion calls the callback with the result. Calls timeout if the API call emits a @@ -136,6 +137,7 @@ def call_api(self, function, callback, timeout, *args, **kwargs): self.api_thread = QThread(self.gui) self.api_runner = APICallRunner(function, *args, **kwargs) self.api_runner.moveToThread(self.api_thread) + self.api_runner.current_object = current_object self.api_runner.call_finished.connect(callback) self.api_runner.timeout.connect(timeout) self.finish_api_call.connect(self.api_runner.on_cancel_timeout) @@ -335,7 +337,8 @@ def on_file_click(self, source_db_object, message): sdk_object.filename = message.filename sdk_object.source_uuid = source_db_object.uuid self.call_api(func, self.on_file_download, - self.on_download_timeout, sdk_object, self.data_dir) + self.on_download_timeout, sdk_object, self.data_dir, + current_object=message) def on_file_download(self, result): """ @@ -343,10 +346,17 @@ def on_file_download(self, result): view to display the contents of the new file. """ sha256sum, filename = self.api_runner.result + file_uuid = self.api_runner.current_object.uuid + server_filename = self.api_runner.current_object.filename self.call_reset() if result: - storage.mark_file_as_downloaded(os.path.basename(filename), - self.session) + # The filename contains the location where the file has been + # stored. On non-Qubes OSes, this will be the data directory. + # On Qubes OS, this will a ~/QubesIncoming directory. In case + # we are on Qubes, we should move the file to the data directory + # and name it the same as the server (e.g. spotless-tater-msg.gpg). + os.rename(filename, os.path.join(self.data_dir, server_filename)) + storage.mark_file_as_downloaded(file_uuid, self.session) # Refresh the conversation with the content of the downloaded file. else: # Update the UI in some way to indicate a failure state. diff --git a/securedrop_client/storage.py b/securedrop_client/storage.py index fea6733c5..9b5f9339a 100644 --- a/securedrop_client/storage.py +++ b/securedrop_client/storage.py @@ -236,12 +236,12 @@ def find_or_create_user(uuid, username, session): return new_user -def mark_file_as_downloaded(local_filename, session): +def mark_file_as_downloaded(uuid, session): """Mark file as downloaded in the database. The file itself will be stored in the data directory. """ submission_db_object = session.query(Submission) \ - .filter_by(filename=local_filename) \ + .filter_by(uuid=uuid) \ .one_or_none() submission_db_object.is_downloaded = True session.add(submission_db_object) diff --git a/tests/test_logic.py b/tests/test_logic.py index 707e7b392..a007019f3 100644 --- a/tests/test_logic.py +++ b/tests/test_logic.py @@ -590,7 +590,7 @@ def test_Client_on_file_click_Submission(safe_tmpdir): cl.call_api.assert_called_once_with( cl.api.download_submission, cl.on_file_download, cl.on_download_timeout, submission_sdk_object, - cl.data_dir) + cl.data_dir, current_object=submission) def test_Client_on_file_download_success(safe_tmpdir): @@ -601,13 +601,17 @@ def test_Client_on_file_download_success(safe_tmpdir): cl.api_runner = mock.MagicMock() test_filename = "my-file-location-msg.gpg" cl.api_runner.result = ("", test_filename) + cl.api_runner.current_object = mock.MagicMock() + test_object_uuid = 'uuid-of-downloaded-object' + cl.api_runner.current_object.uuid = test_object_uuid cl.call_reset = mock.MagicMock() result = True - with mock.patch('securedrop_client.logic.storage') as mock_storage: + with mock.patch('securedrop_client.logic.storage') as mock_storage, \ + mock.patch('os.rename'): cl.on_file_download(result) cl.call_reset.assert_called_once_with() mock_storage.mark_file_as_downloaded.assert_called_once_with( - test_filename, mock_session) + test_object_uuid, mock_session) def test_Client_on_file_download_failure(safe_tmpdir): From c277e0548e917dc685cd5006a3606cbf499a8d6a Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Fri, 26 Oct 2018 10:53:44 -0700 Subject: [PATCH 4/4] When download completes, refresh current source conversation view --- securedrop_client/gui/main.py | 4 +++- securedrop_client/logic.py | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/securedrop_client/gui/main.py b/securedrop_client/gui/main.py index 5c76994f7..8c029bab5 100644 --- a/securedrop_client/gui/main.py +++ b/securedrop_client/gui/main.py @@ -61,6 +61,7 @@ def __init__(self): widget_layout.addWidget(self.tool_bar, 1) widget_layout.addWidget(self.main_view, 6) self.setCentralWidget(self.widget) + self.current_source = None # Tracks which source is shown self.show() self.autosize_window() @@ -150,7 +151,8 @@ def on_source_changed(self): source_item = self.main_view.source_list.currentItem() source_widget = self.main_view.source_list.itemWidget(source_item) if source_widget: - self.show_conversation_for(source_widget.source) + self.current_source = source_widget.source + self.show_conversation_for(self.current_source) def show_conversation_for(self, source): """ diff --git a/securedrop_client/logic.py b/securedrop_client/logic.py index 2d67b7876..cd7277aee 100644 --- a/securedrop_client/logic.py +++ b/securedrop_client/logic.py @@ -357,7 +357,10 @@ def on_file_download(self, result): # and name it the same as the server (e.g. spotless-tater-msg.gpg). os.rename(filename, os.path.join(self.data_dir, server_filename)) storage.mark_file_as_downloaded(file_uuid, self.session) - # Refresh the conversation with the content of the downloaded file. + + # Refresh the current source conversation, bearing in mind + # that the user may have navigated to another source. + self.gui.show_conversation_for(self.gui.current_source) else: # Update the UI in some way to indicate a failure state. self.set_status("Failed to download file, please try again.")