Skip to content

Commit

Permalink
Avoid looking up the facility user twice if not needed.
Browse files Browse the repository at this point in the history
Ensure we don't error when exact username duplicates exist in a facility.
  • Loading branch information
rtibbles committed Jan 24, 2024
1 parent 6b8acac commit e20025c
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 21 deletions.
37 changes: 19 additions & 18 deletions kolibri/core/auth/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
31 changes: 28 additions & 3 deletions kolibri/core/auth/test/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down

0 comments on commit e20025c

Please sign in to comment.