pure refactor: move auth checks into decorator

securedrop_client/logic.py

@@ -18,6 +18,7 @@
 """
 import arrow
 from datetime import datetime
+import functools
 import inspect
 import logging
 import os
@@ -48,6 +49,18 @@
 logger = logging.getLogger(__name__)
 
 
+def login_required(f):
+    @functools.wraps(f)
+    def decorated_function(self, *args, **kwargs):
+        if not self.api:
+            self.on_action_requiring_login()
+            return
+        else:
+            return f(self, *args, **kwargs)
+
+    return decorated_function
+
+
 class APICallRunner(QObject):
     """
     Used to call the SecureDrop API in a non-blocking manner.
@@ -471,15 +484,12 @@ def on_update_star_failure(self, result: UpdateStarJobException) -> None:
         error = _('Failed to update star.')
         self.gui.update_error_status(error)
 
+    @login_required
     def update_star(self, source_db_object, callback):
         """
         Star or unstar. The callback here is the API sync as we first make sure
         that we apply the change to the server, and then update locally.
         """
-        if not self.api:  # Then we should tell the user they need to login.
-            self.on_action_requiring_login()
-            return
-
         job = UpdateStarJob(source_db_object.uuid, source_db_object.is_starred)
         job.success_signal.connect(self.on_update_star_success, type=Qt.QueuedConnection)
         job.success_signal.connect(callback, type=Qt.QueuedConnection)
@@ -517,6 +527,7 @@ def set_status(self, message, duration=5000):
         """
         self.gui.update_activity_status(message, duration)
 
+    @login_required
     def _submit_download_job(self,
                              object_type: Union[Type[db.Reply], Type[db.Message], Type[db.File]],
                              uuid: str) -> None:
@@ -670,6 +681,7 @@ def print_file(self, file_uuid: str) -> None:
 
         self.export.begin_print.emit([file_location])
 
+    @login_required
     def on_submission_download(
         self,
         submission_type: Union[Type[db.File], Type[db.Message]],
@@ -678,11 +690,8 @@ def on_submission_download(
         """
         Download the file associated with the Submission (which may be a File or Message).
         """
-        if self.api:
-            self._submit_download_job(submission_type, submission_uuid)
-            self.set_status(_('Downloading file'))
-        else:
-            self.on_action_requiring_login()
+        self._submit_download_job(submission_type, submission_uuid)
+        self.set_status(_('Downloading file'))
 
     def on_file_download_success(self, uuid: Any) -> None:
         """
@@ -722,6 +731,7 @@ def on_delete_source_failure(self, result: Exception) -> None:
         error = _('Failed to delete source at server')
         self.gui.update_error_status(error)
 
+    @login_required
     def delete_source(self, source: db.Source):
         """
         Performs a delete operation on source record.
@@ -731,16 +741,13 @@ def delete_source(self, source: db.Source):
         synchronize the server records with the local state. If not,
         the failure handler will display an error.
         """
-        if not self.api:  # Then we should tell the user they need to login.
-            self.on_action_requiring_login()
-            return
-
         job = DeleteSourceJob(source.uuid)
         job.success_signal.connect(self.on_delete_source_success, type=Qt.QueuedConnection)
         job.failure_signal.connect(self.on_delete_source_failure, type=Qt.QueuedConnection)
 
         self.api_job_queue.enqueue(job)
 
+    @login_required
     def send_reply(self, source_uuid: str, reply_uuid: str, message: str) -> None:
         """
         Send a reply to a source.

tests/test_logic.py

@@ -763,6 +763,7 @@ def test_Controller_on_file_download_Submission_no_auth(homedir, config, session
     """If the controller is not authenticated, do not enqueue a download job"""
     mock_gui = mocker.MagicMock()
     co = Controller('http://localhost', mock_gui, session_maker, homedir)
+    co.on_action_requiring_login = mocker.MagicMock()
     co.api = None
 
     mock_success_signal = mocker.MagicMock()
@@ -785,6 +786,7 @@ def test_Controller_on_file_download_Submission_no_auth(homedir, config, session
     assert not mock_queue.enqueue.called
     assert not mock_success_signal.connect.called
     assert not mock_failure_signal.connect.called
+    assert co.on_action_requiring_login.called
 
 
 def test_Controller_on_file_downloaded_success(homedir, config, mocker, session_maker):
@@ -1000,6 +1002,7 @@ def test_Controller_download_new_replies_with_new_reply(mocker, session, session
     user-facing status message when a new reply is found.
     """
     co = Controller('http://localhost', mocker.MagicMock(), session_maker, homedir)
+    co.api = 'Api token has a value'
     reply = factory.Reply(source=factory.Source())
     mocker.patch('securedrop_client.storage.find_new_replies', return_value=[reply])
     success_signal = mocker.MagicMock()
@@ -1102,6 +1105,7 @@ def test_Controller_download_new_messages_with_new_message(mocker, session, sess
     usre-facing status message when a new message is found.
     """
     co = Controller('http://localhost', mocker.MagicMock(), session_maker, homedir)
+    co.api = 'Api token has a value'
     message = factory.Message(source=factory.Source())
     mocker.patch('securedrop_client.storage.find_new_messages', return_value=[message])
     success_signal = mocker.MagicMock()