Merge pull request #8239 from bjester/credential-handling

Credential handling for password-less learner certificate generation

kolibri/core/auth/backends.py

@@ -6,21 +6,27 @@
 from kolibri.core.auth.models import FacilityUser
 
 
+FACILITY_CREDENTIAL_KEY = "facility"
+
+
 class FacilityUserBackend(object):
     """
     A class that implements authentication for FacilityUsers.
     """
 
-    def authenticate(self, username=None, password=None, facility=None):
+    def authenticate(self, request, username=None, password=None, **kwargs):
         """
         Authenticates the user if the credentials correspond to a FacilityUser for the specified Facility.
 
+        :param request: The request is a required positional argument in newer versions of Django
         :param username: a string
         :param password: a string
-        :param facility: a Facility
+        :param kwargs: a dict of additional credentials (see `keyword`s)
+        :keyword facility: a Facility object or facility ID
         :return: A FacilityUser instance if successful, or None if authentication failed.
         """
         users = FacilityUser.objects.filter(username__iexact=username)
+        facility = kwargs.get(FACILITY_CREDENTIAL_KEY, None)
         if facility:
             users = users.filter(facility=facility)
         for user in users:

kolibri/core/auth/management/commands/sync.py

@@ -154,6 +154,7 @@ def handle_async(self, *args, **options):  # noqa C901
                 dataset_id,
                 network_connection,
                 user_id=user_id,
+                facility_id=facility_id,
                 noninteractive=noninteractive,
             )
 
@@ -212,6 +213,7 @@ def handle_async(self, *args, **options):  # noqa C901
                 password,
                 dataset_id,
                 network_connection,
+                facility_id=facility_id,
                 noninteractive=noninteractive,
             )
 

kolibri/core/auth/management/utils.py

@@ -15,6 +15,7 @@
 from morango.models import ScopeDefinition
 from six.moves.urllib.parse import urljoin
 
+from kolibri.core.auth.backends import FACILITY_CREDENTIAL_KEY
 from kolibri.core.auth.constants.morango_sync import ScopeDefinitions
 from kolibri.core.auth.models import Facility
 from kolibri.core.auth.models import FacilityUser
@@ -161,7 +162,13 @@ def get_baseurl(baseurl):
 
 
 def get_client_and_server_certs(
-    username, password, dataset_id, nc, user_id=None, noninteractive=False
+    username,
+    password,
+    dataset_id,
+    nc,
+    user_id=None,
+    facility_id=None,
+    noninteractive=False,
 ):
 
     # get any full-facility certificates we have for the facility
@@ -227,11 +234,18 @@ def get_client_and_server_certs(
                 username = input("Please enter username: ")
                 password = getpass.getpass("Please enter password: ")
 
+        userargs = username
+        if user_id:
+            # add facility so `FacilityUserBackend` can validate
+            userargs = {
+                FacilityUser.USERNAME_FIELD: username,
+                FACILITY_CREDENTIAL_KEY: facility_id,
+            }
         client_cert = nc.certificate_signing_request(
             server_cert,
             client_scope,
             csr_scope_params,
-            userargs=username,
+            userargs=userargs,
             password=password,
         )
     else:

kolibri/core/auth/models.py

@@ -171,7 +171,7 @@ def ensure_compatibility(self, *args, **kwargs):
         if self.learner_can_login_with_no_password and self.learner_can_edit_password:
             raise IncompatibleDeviceSettingError(
                 "Device Settings [learner_can_login_with_no_password={}] & [learner_can_edit_password={}] "
-                "values incompatible togeather.".format(
+                "values incompatible together.".format(
                     self.learner_can_login_with_no_password,
                     self.learner_can_edit_password,
                 )

kolibri/core/auth/test/test_backend.py

@@ -2,6 +2,7 @@
 from __future__ import print_function
 from __future__ import unicode_literals
 
+import mock
 from django.test import TestCase
 
 from ..backends import FacilityUserBackend
@@ -16,30 +17,46 @@ def setUpTestData(cls):
         cls.user = FacilityUser(username="Mike", facility=cls.facility)
         cls.user.set_password("foo")
         cls.user.save()
+        cls.request = mock.Mock()
 
     def test_facility_user_authenticated(self):
         self.assertEqual(
             self.user,
             FacilityUserBackend().authenticate(
-                username="Mike", password="foo", facility=self.facility
+                self.request, username="Mike", password="foo", facility=self.facility
+            ),
+        )
+
+    def test_facility_user_authenticated__facility_id(self):
+        self.assertEqual(
+            self.user,
+            FacilityUserBackend().authenticate(
+                self.request, username="Mike", password="foo", facility=self.facility.pk
             ),
         )
 
     def test_facility_user_authentication_does_not_require_facility(self):
         self.assertEqual(
             self.user,
-            FacilityUserBackend().authenticate(username="Mike", password="foo"),
+            FacilityUserBackend().authenticate(
+                self.request, username="Mike", password="foo"
+            ),
         )
 
     def test_device_owner_not_authenticated(self):
         self.assertIsNone(
-            FacilityUserBackend().authenticate(username="Chuck", password="foobar")
+            FacilityUserBackend().authenticate(
+                self.request, username="Chuck", password="foobar"
+            )
         )
 
     def test_incorrect_password_does_not_authenticate(self):
         self.assertIsNone(
             FacilityUserBackend().authenticate(
-                username="Mike", password="blahblah", facility=self.facility
+                self.request,
+                username="Mike",
+                password="blahblah",
+                facility=self.facility,
             )
         )
 
@@ -52,7 +69,11 @@ def test_nonexistent_user_returns_none(self):
         )
 
     def test_authenticate_nonexistent_user_returns_none(self):
-        self.assertIsNone(FacilityUserBackend().authenticate("foo", "bar"))
+        self.assertIsNone(
+            FacilityUserBackend().authenticate(self.request, "foo", "bar")
+        )
 
     def test_authenticate_with_wrong_password_returns_none(self):
-        self.assertIsNone(FacilityUserBackend().authenticate("Mike", "goo"))
+        self.assertIsNone(
+            FacilityUserBackend().authenticate(self.request, "Mike", "goo")
+        )

kolibri/core/auth/test/test_morango_integration.py

@@ -5,9 +5,11 @@
 import unittest
 import uuid
 
+from django.core.management import call_command
 from django.test import TestCase
 from django.utils import timezone
 from morango.models import InstanceIDModel
+from morango.models import ScopeDefinition
 from morango.models import Store
 from morango.models import syncable_models
 from morango.sync.controller import MorangoProfileController
@@ -22,6 +24,7 @@
 from ..models import Role
 from .helpers import DUMMY_PASSWORD
 from .sync_utils import multiple_kolibri_servers
+from kolibri.core.auth.management.utils import get_client_and_server_certs
 from kolibri.core.exams.models import Exam
 from kolibri.core.exams.models import ExamAssignment
 from kolibri.core.lessons.models import Lesson
@@ -66,6 +69,52 @@ def test_deserializing_field(self):
             self.fail(e.message)
 
 
+@unittest.skipIf(
+    not os.environ.get("INTEGRATION_TEST"),
+    "This test will only be run during integration testing.",
+)
+class CertificateAuthenticationTestCase(TestCase):
+    @multiple_kolibri_servers(1)
+    def test_learner_passwordless_authentication(self, servers):
+        # START: setup server
+        server = servers[0]
+        server.manage("loaddata", "scopedefinitions")
+        server.manage("loaddata", "content_test")
+        server.manage("generateuserdata", "--no-onboarding", "--num-content-items", "1")
+
+        facility = Facility.objects.using(server.db_alias).first()
+        facility.dataset.learner_can_login_with_no_password = True
+        facility.dataset.learner_can_edit_password = False
+        facility.dataset.save(using=server.db_alias)
+        learner = FacilityUser(
+            username=uuid.uuid4().hex[:30], password=DUMMY_PASSWORD, facility=facility
+        )
+        learner.save(using=server.db_alias)
+        # END: setup server
+
+        # START: local setup
+        if not ScopeDefinition.objects.filter():
+            call_command("loaddata", "scopedefinitions")
+
+        controller = MorangoProfileController(PROFILE_FACILITY_DATA)
+        network_connection = controller.create_network_connection(server.baseurl)
+
+        # if it's not working, this will throw:
+        #   requests.exceptions.HTTPError: 401 Client Error: Unauthorized for url
+        client_cert, server_cert, username = get_client_and_server_certs(
+            learner.username,
+            "NOT_THE_DUMMY_PASSWORD",
+            facility.dataset_id,
+            network_connection,
+            user_id=learner.pk,
+            facility_id=facility.id,
+            noninteractive=True,
+        )
+        self.assertIsNotNone(client_cert)
+        self.assertIsNotNone(server_cert)
+        self.assertIsNotNone(username)
+
+
 @unittest.skipIf(
     not os.environ.get("INTEGRATION_TEST"),
     "This test will only be run during integration testing.",

kolibri/core/discovery/test/test_api.py

@@ -52,7 +52,7 @@ def test_creating_good_address(self):
             format="json",
         )
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
-        self.assertEqual(response.data["base_url"], "https://kolibrihappyurl.qqq/")
+        self.assertEqual(response.data["base_url"], "https://kolibrihappyurl.qqq")
 
     def test_creating_good_address_with_one_url_timing_out(self):
         self.login(self.superuser)
@@ -63,7 +63,7 @@ def test_creating_good_address_with_one_url_timing_out(self):
         )
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
         self.assertEqual(
-            response.data["base_url"], "http://timeoutonport80url.qqq:8080/"
+            response.data["base_url"], "http://timeoutonport80url.qqq:8080"
         )
 
     def test_creating_bad_address(self):

kolibri/core/discovery/test/test_network_utils.py

@@ -144,7 +144,7 @@ def test_valid_domain_name_with_invalid_nonnumeric_port(self):
 class TestNetworkClientConnections(TestCase):
     def test_successful_connection_to_kolibri_address(self):
         nc = NetworkClient(address="kolibrihappyurl.qqq")
-        self.assertEqual(nc.base_url, "https://kolibrihappyurl.qqq/")
+        self.assertEqual(nc.base_url, "https://kolibrihappyurl.qqq")
 
     def test_unsuccessful_connection_to_unavailable_address(self):
         with self.assertRaises(errors.NetworkLocationNotFound):
@@ -156,11 +156,11 @@ def test_unsuccessful_connection_to_nonkolibri_address(self):
 
     def test_successful_connection_to_address_with_port80_timeout(self):
         nc = NetworkClient(address="timeoutonport80url.qqq")
-        self.assertEqual(nc.base_url, "http://timeoutonport80url.qqq:8080/")
+        self.assertEqual(nc.base_url, "http://timeoutonport80url.qqq:8080")
 
     def test_successful_connection_to_kolibri_base_url(self):
         nc = NetworkClient(base_url="https://kolibrihappyurl.qqq/")
-        self.assertEqual(nc.base_url, "https://kolibrihappyurl.qqq/")
+        self.assertEqual(nc.base_url, "https://kolibrihappyurl.qqq")
 
     def test_unsuccessful_connection_to_unavailable_base_url(self):
         with self.assertRaises(errors.NetworkLocationNotFound):
@@ -172,4 +172,4 @@ def test_unsuccessful_connection_to_nonkolibri_base_url(self):
 
     def test_unsuccessful_connection_to_base_url_with_timeout(self):
         with self.assertRaises(errors.NetworkLocationNotFound):
-            NetworkClient(base_url="http://timeoutonport80url.qqq/")
+            NetworkClient(base_url="http://timeoutonport80url.qqq")

kolibri/core/discovery/utils/network/client.py

@@ -51,9 +51,10 @@ def _attempt_connections(self, urls):
                     allow_redirects=True,
                     params={"v": DEVICE_INFO_VERSION},
                 )
-                # check that we successfully connected, and if we were redirected that it's still the right endpoint
-                response_path = urlparse(response.url).path
-                if response.status_code == 200 and response_path.rstrip("/").endswith(
+                # check that we successfully connected, and if we were redirected that it's still
+                # the right endpoint
+                parsed_url = urlparse(response.url)
+                if response.status_code == 200 and parsed_url.path.rstrip("/").endswith(
                     "/api/public/info"
                 ):
                     info = response.json()
@@ -65,7 +66,8 @@ def _attempt_connections(self, urls):
                             "Server is not running Kolibri or Studio"
                         )
                     logger.info("Success! We connected to: {}".format(response.url))
-                    return response.url.rstrip("/").replace("api/public/info", "")
+
+                    return "{}://{}".format(parsed_url.scheme, parsed_url.netloc)
             except (requests.RequestException) as e:
                 logger.info("Unable to connect: {}".format(e))
             except ValueError:

kolibri/core/tasks/api.py

@@ -1219,7 +1219,13 @@ def validate_peer_sync_job(request):
 
 
 def prepare_peer_sync_job(baseurl, facility_id, username, password, **kwargs):
-    job_data = prepare_sync_job(facility_id, baseurl=baseurl, **kwargs)  # ToDo
+    """
+    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
+    """
+    # 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)
 
     # call this in case user directly syncs without migrating database
     if not ScopeDefinition.objects.filter():
@@ -1237,7 +1243,13 @@ def prepare_peer_sync_job(baseurl, facility_id, username, password, **kwargs):
 
         # username and password are not required for this to succeed unless there is no cert
         get_client_and_server_certs(
-            username, password, dataset_id, network_connection, noninteractive=True
+            username,
+            password,
+            dataset_id,
+            network_connection,
+            user_id=user_id,
+            facility_id=facility_id,
+            noninteractive=True,
         )
     except (CommandError, HTTPError) as e:
         if not username and not password:
@@ -1251,11 +1263,10 @@ def prepare_peer_sync_job(baseurl, facility_id, username, password, **kwargs):
 def prepare_soud_sync_job(baseurl, facility_id, user_id, **kwargs):
     """
     A SoUD sync requires that the device is already "registered" with the server, so there
-    shouldn't be a need for username/password and the verification of those
+    shouldn't be a need for username/password and the verification of those. This eliminates the
+    validation to keep overhead low for automated single-user syncing. To initialize with a peer
+    for a SoUD, use `prepare_peer_sync_job` with `user` keyword argument
     """
-    if not ScopeDefinition.objects.filter():
-        call_command("loaddata", "scopedefinitions")
-
     kwargs.update(user=user_id)
     return prepare_sync_job(facility_id, baseurl=baseurl, **kwargs)
 

kolibri/core/tasks/test/test_api.py

@@ -1008,7 +1008,13 @@ def test_validate_peer_sync_job(
         )
 
         get_client_and_server_certs.assert_called_with(
-            "tester", "mypassword", dataset_id, network_connection, noninteractive=True
+            "tester",
+            "mypassword",
+            dataset_id,
+            network_connection,
+            user_id=None,
+            facility_id=123,
+            noninteractive=True,
         )
 
     def test_validate_peer_sync_job__no_baseurl(self):