diff --git a/activate_account/factories.py b/activate_account/factories.py
index 56c0e7f1bce2a354b1349946c4be0f0e712c63b6..29c43de73d844cf13e95b5324d771001d592b421 100644
--- a/activate_account/factories.py
+++ b/activate_account/factories.py
@@ -69,9 +69,24 @@ class AccountIdentifierFactory(factory.django.DjangoModelFactory):
model = AccountIdentifier
account_id = factory.SubFactory(AccountFactory)
- last_name = factory.LazyAttribute(lambda n: factory.Faker._get_faker().last_name().upper())
date_of_birth = factory.Faker("date_of_birth")
+ @factory.lazy_attribute
+ def last_name(self):
+ # Ideally we'd duplicate what is in AccountDetails but not all Accounts have one
+ faker = factory.Faker._get_faker()
+ first_letter = self.account_id.crsid[0].upper()
+ second_letter = self.account_id.crsid[1].upper()
+ last_name = faker.last_name()
+
+ attempts = 0
+ max_attempts = 20
+ while not last_name.startswith(second_letter) and attempts < max_attempts:
+ last_name = faker.last_name()
+ attempts += 1
+
+ return f"{last_name.upper()} {first_letter}"
+
@factory.lazy_attribute
def code(self):
code_type = random.choice(["staff", "postgraduate", "pgce", "undergraduate"])
diff --git a/authentication/serializers.py b/authentication/serializers.py
index b8ff4b3da527118b66dd4e3de9132d7ff447d5ab..40dde903363883da9e8e8f887c7ada022ed3c457 100644
--- a/authentication/serializers.py
+++ b/authentication/serializers.py
@@ -123,6 +123,14 @@ class TokenRequestSerializer(serializers.Serializer):
account_identifier__date_of_birth=data["date_of_birth"],
account_identifier__code=data["code"],
)
+ except Account.MultipleObjectsReturned:
+ logger.warn(
+ "Multiple accounts found with normalised last name",
+ last_name=data.get("last_name"),
+ date_of_birth=data["date_of_birth"],
+ name_match=name_match,
+ )
+ raise InvalidGrantError("Ambiguous token request")
except Account.DoesNotExist:
# then match last_name fields being just name_match
account = Account.objects.get(
diff --git a/authentication/tests/test_login_view.py b/authentication/tests/test_login_view.py
index d5bc9d9eebb808e038ec96c15d26968e13cb2d2e..d953bc68ad24923b1b5d369f9aef22f2923618bb 100644
--- a/authentication/tests/test_login_view.py
+++ b/authentication/tests/test_login_view.py
@@ -1,3 +1,5 @@
+import random
+from string import ascii_uppercase
from unittest import mock
from urllib.parse import urlencode
@@ -6,6 +8,7 @@ from django.urls import reverse
from knox.settings import knox_settings
from rest_framework import exceptions, parsers, status
+from activate_account.factories import AccountFactory, AccountIdentifierFactory
from authentication.constants import SESSION_GRANT_TYPE
from authentication.serializers import TokenRequestSerializer
@@ -86,7 +89,7 @@ def test_valid_session_grant(client, valid_session_grant_body):
@pytest.mark.parametrize("valid_session_grant_body", ["crsid"], indirect=True)
-def test_valid_crsid_case_insensitive(request, client, valid_session_grant_body):
+def test_valid_crsid_case_insensitive(client, valid_session_grant_body):
"""CRSId comparison is case insensitive"""
field = "crsid"
assert_is_valid_login(
@@ -101,7 +104,7 @@ def test_valid_crsid_case_insensitive(request, client, valid_session_grant_body)
@pytest.mark.parametrize("valid_session_grant_body", ["last_name"], indirect=True)
-def test_valid_last_name_case_insensitive(request, client, valid_session_grant_body):
+def test_valid_last_name_case_insensitive(client, valid_session_grant_body):
"""Last name comparison is case insensitive (due to normalisation)"""
field = "last_name"
assert_is_valid_login(
@@ -116,7 +119,7 @@ def test_valid_last_name_case_insensitive(request, client, valid_session_grant_b
@pytest.mark.parametrize("valid_session_grant_body", ["last_name"], indirect=True)
-def test_valid_last_name_partial(request, client, valid_session_grant_body, faker):
+def test_valid_last_name_partial(client, valid_session_grant_body, faker):
"""Last name comparison also matches partial (starting followed by space)"""
field = "last_name"
# Just the first part with random case
@@ -136,6 +139,35 @@ def test_valid_last_name_partial(request, client, valid_session_grant_body, fake
)
+@pytest.mark.parametrize("valid_session_grant_body", ["last_name"], indirect=True)
+def test_valid_last_name_partial_multiple(client, valid_session_grant_body, account_identifier):
+ """Last name partial comparison could potentially match multiple accounts"""
+ just_first_part = account_identifier.last_name.split(" ")[0]
+ # Create another account identifier with the same date of birth, code and first part of last
+ # name (but different full last name)
+ while True:
+ alt_last_name = f"{just_first_part} {random.choice(ascii_uppercase)}"
+ if alt_last_name != account_identifier.last_name:
+ break
+ alt_account = AccountFactory()
+ AccountIdentifierFactory(
+ account_id=alt_account,
+ last_name=alt_last_name,
+ date_of_birth=account_identifier.date_of_birth,
+ code=account_identifier.code,
+ )
+ assert_is_error(
+ post_login(
+ client,
+ {
+ **valid_session_grant_body,
+ "last_name": just_first_part,
+ },
+ ),
+ "invalid_grant",
+ )
+
+
@pytest.mark.parametrize("valid_session_grant_body", ["crsid", "last_name"], indirect=True)
def test_missing_field(client, request, valid_session_grant_body):
"""Missing a required field results in 'invalid_request'"""
diff --git a/authentication/tests/test_token_request_serializer.py b/authentication/tests/test_token_request_serializer.py
index e9144d26a4ba3df17a5c56fc8409af0fe1e52773..ae9fed8da1e851e7d5de77329d5821620600b83b 100644
--- a/authentication/tests/test_token_request_serializer.py
+++ b/authentication/tests/test_token_request_serializer.py
@@ -33,6 +33,7 @@ def test_missing_crsid_and_last_name():
}
+@pytest.mark.django_db
def test_empty_post_normalise_last_name():
data = {
"grant_type": SESSION_GRANT_TYPE,