From e20025c4d54682287bfa801a899b0695d8b25cba Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Fri, 19 Jan 2024 13:44:59 -0800 Subject: [PATCH] Avoid looking up the facility user twice if not needed. Ensure we don't error when exact username duplicates exist in a facility. --- kolibri/core/auth/api.py | 37 +++++++++++++++--------------- kolibri/core/auth/test/test_api.py | 31 ++++++++++++++++++++++--- 2 files changed, 47 insertions(+), 21 deletions(-) diff --git a/kolibri/core/auth/api.py b/kolibri/core/auth/api.py index c7ed80ea00..8e96b78479 100644 --- a/kolibri/core/auth/api.py +++ b/kolibri/core/auth/api.py @@ -878,7 +878,23 @@ def create(self, request): status=status.HTTP_401_UNAUTHORIZED, ) - # Find the FacilityUser we're looking for use later on + user = None + if interface.enabled and valid_app_key_on_request(request): + # If we are in app context, then try to get the automatically created OS User + # if it matches the username, without needing a password. + user = self._check_os_user(request, username) + if user is None: + # Otherwise attempt full authentication + user = authenticate( + username=username, password=password, facility=facility_id + ) + if user is not None and user.is_active: + # Correct password, and the user is marked "active" + login(request, user) + # Success! + return self.get_session_response(request) + # Otherwise, try to give a helpful error message + # Find the FacilityUser we're looking for try: unauthenticated_user = FacilityUser.objects.get( username__iexact=username, facility=facility_id @@ -898,24 +914,9 @@ def create(self, request): ) except FacilityUser.MultipleObjectsReturned: # Handle case of multiple matching usernames - unauthenticated_user = FacilityUser.objects.get( + unauthenticated_user = FacilityUser.objects.filter( username__exact=username, facility=facility_id - ) - user = None - if interface.enabled and valid_app_key_on_request(request): - # If we are in app context, then try to get the automatically created OS User - # if it matches the username, without needing a password. - user = self._check_os_user(request, username) - if user is None: - # Otherwise attempt full authentication - user = authenticate( - username=username, password=password, facility=facility_id - ) - if user is not None and user.is_active: - # Correct password, and the user is marked "active" - login(request, user) - # Success! - return self.get_session_response(request) + ).first() if unauthenticated_user.password == NOT_SPECIFIED: # Here - we have a Learner whose password is "NOT_SPECIFIED" because they were created # while the "Require learners to log in with password" setting was disabled - but now diff --git a/kolibri/core/auth/test/test_api.py b/kolibri/core/auth/test/test_api.py index 2b191e99a6..e0d11baad2 100644 --- a/kolibri/core/auth/test/test_api.py +++ b/kolibri/core/auth/test/test_api.py @@ -1180,9 +1180,34 @@ def test_case_insensitive_matching_usernames(self): # Assert the expected behavior for the second user self.assertEqual(response_user2.status_code, 200) - # Cleanup: Delete the created users - self.user1.delete() - self.user2.delete() + def test_case_sensitive_matching_usernames(self): + FacilityUserFactory.create(username="shared_username", facility=self.facility) + + response_user2 = self.client.post( + reverse("kolibri:core:session-list"), + data={ + "username": "shared_username", + "password": DUMMY_PASSWORD, + "facility": self.facility.id, + }, + format="json", + ) + + # Assert the expected behavior for the second user + self.assertEqual(response_user2.status_code, 200) + + # Test no error when authentication fails + response_user3 = self.client.post( + reverse("kolibri:core:session-list"), + data={ + "username": "shared_username", + "password": "wrong_password", + "facility": self.facility.id, + }, + format="json", + ) + + self.assertEqual(response_user3.status_code, 401) class SignUpBase(object):