Validate sync credentials before task enqueue

kolibri/core/tasks/api.py

@@ -1012,12 +1012,11 @@ def startpeerfacilityimport(self, request):
         """
 
         baseurl, facility_id, username, password = validate_peer_sync_job(request)
+        validate_and_create_sync_credentials(baseurl, facility_id, username, password)
         sync_args = validate_sync_task(request)
         job_data = prepare_peer_sync_job(
             baseurl,
             facility_id,
-            username,
-            password,
             no_push=True,
             no_provision=True,
             extra_metadata=prepare_sync_task(*sync_args, type="SYNCPEER/PULL"),
@@ -1036,12 +1035,11 @@ def startpeerfacilitysync(self, request):
         Initiate a SYNC (PULL + PUSH) of a specific facility from another device.
         """
         baseurl, facility_id, username, password = validate_peer_sync_job(request)
+        validate_and_create_sync_credentials(baseurl, facility_id, username, password)
         sync_args = validate_sync_task(request)
         job_data = prepare_peer_sync_job(
             baseurl,
             facility_id,
-            username,
-            password,
             extra_metadata=prepare_sync_task(*sync_args, type="SYNCPEER/FULL"),
         )
         job_id = facility_queue.enqueue(call_command, "sync", **job_data)
@@ -1218,15 +1216,15 @@ def validate_peer_sync_job(request):
     return (baseurl, facility_id, username, password)
 
 
-def prepare_peer_sync_job(baseurl, facility_id, username, password, **kwargs):
-    """
-    Initializes and validates connection to peer with username and password for the sync command. If
-    already initialized, the username and password do not need to be supplied
+def validate_and_create_sync_credentials(
+    baseurl, facility_id, username, password, user_id=None
+):
     """
-    # get the `user` for the sync command if present for provisioning
-    user_id = kwargs.get("user", None)
-    job_data = prepare_sync_job(facility_id, baseurl=baseurl, **kwargs)
+    Validates user credentials for syncing by performing certificate verification, which will also
+    save any certificates after successful authentication
 
+    :param user_id: Optional user ID for SoUD use case
+    """
     # call this in case user directly syncs without migrating database
     if not ScopeDefinition.objects.filter():
         call_command("loaddata", "scopedefinitions")
@@ -1257,7 +1255,13 @@ def prepare_peer_sync_job(baseurl, facility_id, username, password, **kwargs):
         else:
             raise AuthenticationFailed(e)
 
-    return job_data
+
+def prepare_peer_sync_job(baseurl, facility_id, **kwargs):
+    """
+    Initializes and validates connection to peer with username and password for the sync command. If
+    already initialized, the username and password do not need to be supplied
+    """
+    return prepare_sync_job(facility_id, baseurl=baseurl, **kwargs)
 
 
 def prepare_soud_sync_job(baseurl, facility_id, user_id, **kwargs):

kolibri/core/tasks/test/test_api.py

@@ -26,6 +26,7 @@
 from kolibri.core.tasks.api import prepare_sync_job
 from kolibri.core.tasks.api import prepare_sync_task
 from kolibri.core.tasks.api import ResourceGoneError
+from kolibri.core.tasks.api import validate_and_create_sync_credentials
 from kolibri.core.tasks.api import validate_facility
 from kolibri.core.tasks.api import validate_peer_sync_job
 from kolibri.core.tasks.api import validate_sync_task
@@ -684,10 +685,10 @@ def test_startdataportalbulksync(self, facility_queue):
 
     @patch("kolibri.core.tasks.api.validate_peer_sync_job")
     @patch("kolibri.core.tasks.api.prepare_peer_sync_job")
-    @patch("kolibri.core.tasks.api.get_client_and_server_certs")
+    @patch("kolibri.core.tasks.api.validate_and_create_sync_credentials")
     def test_startpeerfacilityimport(
         self,
-        get_client_and_server_certs,
+        validate_and_create_sync_credentials,
         prepare_peer_sync_job,
         validate_peer_sync_job,
         facility_queue,
@@ -749,19 +750,25 @@ def test_startpeerfacilityimport(
         prepare_peer_sync_job.assert_has_calls(
             [
                 call(
-                    *request_data.keys(),
+                    "baseurl",
+                    "facility",
                     no_push=True,
                     no_provision=True,
-                    extra_metadata=extra_metadata
+                    extra_metadata=extra_metadata,
                 )
             ]
         )
         facility_queue.enqueue.assert_called_with(call_command, "sync", **prepared_data)
 
     @patch("kolibri.core.tasks.api.prepare_peer_sync_job")
     @patch("kolibri.core.tasks.api.validate_peer_sync_job")
+    @patch("kolibri.core.tasks.api.validate_and_create_sync_credentials")
     def test_startpeerfacilitysync(
-        self, validate_peer_sync_job, prepare_peer_sync_job, facility_queue
+        self,
+        validate_and_create_sync_credentials,
+        validate_peer_sync_job,
+        prepare_peer_sync_job,
+        facility_queue,
     ):
         user = self.superuser
 
@@ -817,7 +824,7 @@ def test_startpeerfacilitysync(
         self.assertEqual(response.status_code, 200)
         self.assertJobResponse(fake_job_data, response)
         prepare_peer_sync_job.assert_has_calls(
-            [call(*request_data.keys(), extra_metadata=extra_metadata)]
+            [call("baseurl", "facility", extra_metadata=extra_metadata)]
         )
         facility_queue.enqueue.assert_called_with(call_command, "sync", **prepared_data)
 
@@ -986,8 +993,10 @@ def test_validate_peer_sync_job(
             cancellable=False,
             extra_metadata=dict(type="test"),
         )
+        req_params = validate_peer_sync_job(req)
+        validate_and_create_sync_credentials(*req_params)
         actual = prepare_peer_sync_job(
-            *validate_peer_sync_job(req), extra_metadata=dict(type="test")
+            *req_params[:2], extra_metadata=dict(type="test")
         )
         self.assertEqual(expected, actual)
 
@@ -1038,7 +1047,7 @@ def test_validate_peer_sync_job__cannot_connect(self, NetworkClient):
     @patch("kolibri.core.tasks.api.MorangoProfileController")
     @patch("kolibri.core.tasks.api.NetworkClient")
     @patch("kolibri.core.tasks.api.get_dataset_id")
-    def test_validate_peer_sync_job__unknown_facility(
+    def test_validate_and_create_sync_credentials__unknown_facility(
         self, get_dataset_id, NetworkClient, MorangoProfileController
     ):
         req = Mock(
@@ -1061,15 +1070,13 @@ def test_validate_peer_sync_job__unknown_facility(
         get_dataset_id.side_effect = CommandError()
 
         with self.assertRaises(AuthenticationFailed):
-            prepare_peer_sync_job(
-                *validate_peer_sync_job(req), extra_metadata=dict(type="test")
-            )
+            validate_and_create_sync_credentials(*validate_peer_sync_job(req))
 
     @patch("kolibri.core.tasks.api.MorangoProfileController")
     @patch("kolibri.core.tasks.api.NetworkClient")
     @patch("kolibri.core.tasks.api.get_client_and_server_certs")
     @patch("kolibri.core.tasks.api.get_dataset_id")
-    def test_validate_peer_sync_job__not_authenticated(
+    def test_validate_and_create_sync_credentials__not_authenticated(
         self,
         get_dataset_id,
         get_client_and_server_certs,
@@ -1092,15 +1099,13 @@ def test_validate_peer_sync_job__not_authenticated(
         get_client_and_server_certs.side_effect = CommandError()
 
         with self.assertRaises(PermissionDenied):
-            prepare_peer_sync_job(
-                *validate_peer_sync_job(req), extra_metadata=dict(type="test")
-            )
+            validate_and_create_sync_credentials(*validate_peer_sync_job(req))
 
     @patch("kolibri.core.tasks.api.MorangoProfileController")
     @patch("kolibri.core.tasks.api.NetworkClient")
     @patch("kolibri.core.tasks.api.get_client_and_server_certs")
     @patch("kolibri.core.tasks.api.get_dataset_id")
-    def test_validate_peer_sync_job__authentication_failed(
+    def test_validate_and_create_sync_credentials__authentication_failed(
         self,
         get_dataset_id,
         get_client_and_server_certs,
@@ -1128,6 +1133,4 @@ def test_validate_peer_sync_job__authentication_failed(
         get_client_and_server_certs.side_effect = CommandError()
 
         with self.assertRaises(AuthenticationFailed):
-            prepare_peer_sync_job(
-                *validate_peer_sync_job(req), extra_metadata=dict(type="test")
-            )
+            validate_and_create_sync_credentials(*validate_peer_sync_job(req))

kolibri/plugins/setup_wizard/tasks.py

@@ -19,9 +19,9 @@
 from kolibri.core.device.permissions import LODUserHasSyncPermissions
 from kolibri.core.device.permissions import NotProvisionedCanPost
 from kolibri.core.error_constants import DEVICE_LIMITATIONS
-from kolibri.core.tasks.api import prepare_peer_sync_job
 from kolibri.core.tasks.api import prepare_soud_sync_job
 from kolibri.core.tasks.api import prepare_sync_task
+from kolibri.core.tasks.api import validate_and_create_sync_credentials
 from kolibri.core.tasks.decorators import register_task
 
 
@@ -102,6 +102,9 @@ def validate_soud_credentials(request, task_description):
 
     instance_model = InstanceIDModel.get_or_create_current_instance()[0]
 
+    validate_and_create_sync_credentials(
+        baseurl, facility_id, username, password, user_id=user_id
+    )
     extra_metadata = prepare_sync_task(
         facility_id,
         user_id,
@@ -117,8 +120,6 @@ def validate_soud_credentials(request, task_description):
     extra_metadata["full_name"] = full_name
 
     return {
-        "username": username,
-        "password": password,
         "user_id": user_id,
         "extra_metadata": extra_metadata,
         "baseurl": baseurl,
@@ -135,14 +136,11 @@ def validate_soud_credentials(request, task_description):
     ],
 )
 def startprovisionsoud(
-    username=None,
-    password=None,
     baseurl=None,
     facility_id=None,
     user_id=None,
     extra_metadata={},
 ):
-    prepare_peer_sync_job(baseurl, facility_id, username, password, user=user_id)
     job_data = prepare_soud_sync_job(
         baseurl, facility_id, user_id, extra_metadata=extra_metadata
     )