FAQ | This is a LIVE service | Changelog

Skip to content
Snippets Groups Projects

Compare revisions

Changes are shown as if the source revision was being merged into the target revision. Learn more about comparing revisions.

Source

Select target project
No results found

Target

Select target project
  • uis/devops/iam/activate-account/api
1 result
Show changes
Commits on Source (7)
Showing
with 344 additions and 90 deletions
# Changelog # Changelog
## [0.19.0](https://gitlab.developers.cam.ac.uk/uis/devops/iam/activate-account/api/compare/0.18.0...0.19.0) (2025-02-13)
### Features
* normalise last_name from name and in auth check ([afdf23e](https://gitlab.developers.cam.ac.uk/uis/devops/iam/activate-account/api/commit/afdf23ea7e732ca1b50e19fc211c77ba660d11d9))
* remove db case-insensitivity on last name and uppercase in normalisation ([67f5fd8](https://gitlab.developers.cam.ac.uk/uis/devops/iam/activate-account/api/commit/67f5fd8db116f08e44b4937f7c4aebd6138c5341))
* remove last_name from account_details view and requirement in data_manager_api view ([af5059b](https://gitlab.developers.cam.ac.uk/uis/devops/iam/activate-account/api/commit/af5059bbd6ebc045b290cd13b774379a089fbc6c))
### Bug Fixes
* catch MultipleObjectsReturned now possible with LIKE query ([355f315](https://gitlab.developers.cam.ac.uk/uis/devops/iam/activate-account/api/commit/355f315e277ff5bfa46c3da3f8b0137fcaf3c613))
## [0.18.0](https://gitlab.developers.cam.ac.uk/uis/devops/iam/activate-account/api/compare/0.17.1...0.18.0) (2025-02-11) ## [0.18.0](https://gitlab.developers.cam.ac.uk/uis/devops/iam/activate-account/api/compare/0.17.1...0.18.0) (2025-02-11)
### Features ### Features
......
...@@ -14,6 +14,8 @@ class Config(AppConfig): ...@@ -14,6 +14,8 @@ class Config(AppConfig):
#: The human-readable verbose name for this application. #: The human-readable verbose name for this application.
verbose_name = "Activate Account" verbose_name = "Activate Account"
default_auto_field = "django.db.models.BigAutoField"
def ready(self): def ready(self):
""" """
Perform application initialisation once the Django platform has been initialised. Perform application initialisation once the Django platform has been initialised.
......
...@@ -48,18 +48,20 @@ class AccountDetailsFactory(factory.django.DjangoModelFactory): ...@@ -48,18 +48,20 @@ class AccountDetailsFactory(factory.django.DjangoModelFactory):
@factory.lazy_attribute @factory.lazy_attribute
def name(self): def name(self):
# Generate first name # Generate full name
faker = factory.Faker._get_faker() faker = factory.Faker._get_faker()
first_letter = self.account.crsid[0].upper() first_letter = self.account.crsid[0].upper()
first_name = faker.first_name() second_letter = self.account.crsid[1].upper()
last_name = faker.last_name()
attempts = 0 attempts = 0
max_attempts = 20 max_attempts = 20
while not first_name.startswith(first_letter) and attempts < max_attempts: while not last_name.startswith(second_letter) and attempts < max_attempts:
first_name = faker.first_name() last_name = faker.last_name()
attempts += 1 attempts += 1
title = faker.prefix().replace(".", " ") if faker.boolean() else ""
return f"{first_name}" return f"{last_name} {title} {first_letter}."
class AccountIdentifierFactory(factory.django.DjangoModelFactory): class AccountIdentifierFactory(factory.django.DjangoModelFactory):
...@@ -67,9 +69,24 @@ class AccountIdentifierFactory(factory.django.DjangoModelFactory): ...@@ -67,9 +69,24 @@ class AccountIdentifierFactory(factory.django.DjangoModelFactory):
model = AccountIdentifier model = AccountIdentifier
account_id = factory.SubFactory(AccountFactory) account_id = factory.SubFactory(AccountFactory)
last_name = factory.Faker("last_name")
date_of_birth = factory.Faker("date_of_birth") 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 @factory.lazy_attribute
def code(self): def code(self):
code_type = random.choice(["staff", "postgraduate", "pgce", "undergraduate"]) code_type = random.choice(["staff", "postgraduate", "pgce", "undergraduate"])
......
from datetime import date
from pprint import pprint from pprint import pprint
from django.core.management.base import BaseCommand from django.core.management.base import BaseCommand
from activate_account.factories import AccountDetailsFactory from activate_account.factories import AccountFactory
from api.v1alpha.serializers import AccountSerializer from activate_account.models import Account
from data_manager_api.serializers import AccountDataSerializer
from data_manager_api.tests.utils import data_account_factory
class Command(BaseCommand): class Command(BaseCommand):
...@@ -13,13 +16,28 @@ class Command(BaseCommand): ...@@ -13,13 +16,28 @@ class Command(BaseCommand):
self.stdout.write(self.style.SUCCESS("Starting to initialize mock data...")) self.stdout.write(self.style.SUCCESS("Starting to initialize mock data..."))
for _ in range(10): for _ in range(10):
details = AccountDetailsFactory() account = AccountFactory.build()
data = data_account_factory(account)
valid_at = account.valid_at
serializer = AccountDataSerializer(data=data, valid_at=valid_at)
serializer.is_valid(raise_exception=True)
instance = serializer.save()
account = Account.objects.get(crsid=instance.crsid)
print("Created user:") print("Created user:")
pprint( pprint(
{ {
**AccountSerializer(details.account).data, "crsid": account.crsid,
"registration_code": details.account.code, "name": account.account_details.name,
} "last_name": account.account_identifier.first().last_name,
"date_of_birth": date.strftime(
account.account_identifier.first().date_of_birth, "%Y-%m-%d"
),
"code": account.account_identifier.first().code,
},
sort_dicts=False,
) )
self.stdout.write(self.style.SUCCESS("Successfully initialized mock data.")) self.stdout.write(self.style.SUCCESS("Successfully initialized mock data."))
# Generated by Django 4.2.14 on 2025-02-06 19:17
from django.db import migrations, models
from authentication.normalisation import normalise_name
def normalise_names(apps, schema_editor):
AccountIdentifier = apps.get_model("activate_account", "AccountIdentifier")
for account_identifier in AccountIdentifier.objects.all():
account_identifier.last_name = normalise_name(account_identifier.last_name)
account_identifier.save()
class Migration(migrations.Migration):
dependencies = [
("activate_account", "0007_lockout_lockout_unique_lockout"),
]
operations = [
migrations.AlterField(
model_name="accountidentifier",
name="last_name",
field=models.CharField(max_length=100),
),
migrations.AlterField(
model_name="accountidentifier",
name="id",
field=models.BigAutoField(
auto_created=True, primary_key=True, serialize=False, verbose_name="ID"
),
),
# No way to reverse this operation without storing the original data, and doing so should
# not be necessary.
migrations.RunPython(normalise_names, reverse_code=migrations.RunPython.noop),
]
...@@ -53,7 +53,8 @@ class AccountIdentifier(models.Model): ...@@ -53,7 +53,8 @@ class AccountIdentifier(models.Model):
account_id = models.ForeignKey( account_id = models.ForeignKey(
Account, on_delete=models.CASCADE, related_name="account_identifier" Account, on_delete=models.CASCADE, related_name="account_identifier"
) )
last_name = models.CharField(max_length=100, db_collation="case_insensitive") # Last name cannot have a case-insensitive collation as this prevents matching with a "LIKE"
last_name = models.CharField(max_length=100)
date_of_birth = models.DateField() date_of_birth = models.DateField()
code = models.CharField(max_length=100) code = models.CharField(max_length=100)
......
...@@ -3,30 +3,12 @@ import random ...@@ -3,30 +3,12 @@ import random
import pytest import pytest
from django.db import IntegrityError from django.db import IntegrityError
from activate_account.factories import AccountFactory, AccountIdentifierFactory from activate_account.factories import AccountFactory
from activate_account.models import Account from activate_account.models import Account
pytestmark = pytest.mark.django_db pytestmark = pytest.mark.django_db
def test_account_unique_together_case_insensitive():
account = AccountFactory(account_identifier=True)
last_name = account.account_identifier.first().last_name
code = account.account_identifier.first().code
date_of_birth = account.account_identifier.first().date_of_birth
last_name_upper = last_name.upper()
assert account.account_identifier.first().last_name != last_name_upper # Sanity check
with pytest.raises(IntegrityError):
AccountIdentifierFactory(
last_name=last_name_upper,
code=code,
date_of_birth=date_of_birth,
account_id=account,
)
def test_account_separate_uniqueness(): def test_account_separate_uniqueness():
n_accounts = random.randrange(10, 21) n_accounts = random.randrange(10, 21)
......
...@@ -18,18 +18,6 @@ def test_account_uniqueness_together(account_identifier): ...@@ -18,18 +18,6 @@ def test_account_uniqueness_together(account_identifier):
) )
def test_account_identifier_unique_together_case_insensitive(account_identifier):
last_name_upper = account_identifier.last_name.upper()
assert account_identifier.last_name != last_name_upper # Sanity check
with pytest.raises(IntegrityError):
AccountIdentifierFactory(
last_name=last_name_upper,
date_of_birth=account_identifier.date_of_birth,
code=account_identifier.code,
)
def test_same_code_across_different_accounts(): def test_same_code_across_different_accounts():
identifier_code = "SHARED_CODE" identifier_code = "SHARED_CODE"
......
...@@ -14,9 +14,6 @@ class AccountSerializer(serializers.ModelSerializer): ...@@ -14,9 +14,6 @@ class AccountSerializer(serializers.ModelSerializer):
terms_accepted = serializers.BooleanField( terms_accepted = serializers.BooleanField(
source="account_details.terms_accepted", default=False source="account_details.terms_accepted", default=False
) )
last_name = serializers.CharField(
source="account_identifier.last_name", allow_null=True, required=False
)
date_of_birth = serializers.DateField( date_of_birth = serializers.DateField(
source="account_identifier.date_of_birth", allow_null=True, required=False source="account_identifier.date_of_birth", allow_null=True, required=False
) )
...@@ -58,7 +55,6 @@ class AccountSerializer(serializers.ModelSerializer): ...@@ -58,7 +55,6 @@ class AccountSerializer(serializers.ModelSerializer):
model = Account model = Account
fields = ( fields = (
"crsid", "crsid",
"last_name",
"date_of_birth", "date_of_birth",
"name", "name",
"affiliation", "affiliation",
...@@ -67,7 +63,6 @@ class AccountSerializer(serializers.ModelSerializer): ...@@ -67,7 +63,6 @@ class AccountSerializer(serializers.ModelSerializer):
) )
read_only_fields = ( read_only_fields = (
"crsid", "crsid",
"last_name",
"date_of_birth", "date_of_birth",
"name", "name",
"affiliation", "affiliation",
...@@ -77,7 +72,6 @@ class AccountSerializer(serializers.ModelSerializer): ...@@ -77,7 +72,6 @@ class AccountSerializer(serializers.ModelSerializer):
def to_representation(self, instance): def to_representation(self, instance):
representation = super().to_representation(instance) representation = super().to_representation(instance)
account_identifier = AccountIdentifier.objects.filter(account_id=instance.pk).first() account_identifier = AccountIdentifier.objects.filter(account_id=instance.pk).first()
representation["last_name"] = account_identifier.last_name
representation["date_of_birth"] = account_identifier.date_of_birth.strftime("%Y-%m-%d") representation["date_of_birth"] = account_identifier.date_of_birth.strftime("%Y-%m-%d")
return representation return representation
......
...@@ -13,14 +13,12 @@ def test_account_details(authenticated_api_client, account, account_details, url ...@@ -13,14 +13,12 @@ def test_account_details(authenticated_api_client, account, account_details, url
assert response.status_code == status.HTTP_200_OK assert response.status_code == status.HTTP_200_OK
last_name = account.account_identifier.first().last_name
date_of_birth = account.account_identifier.first().date_of_birth date_of_birth = account.account_identifier.first().date_of_birth
assert response.data == { assert response.data == {
# The `account` is returned from the factory, so the CRSId is still cased. The saved # The `account` is returned from the factory, so the CRSId is still cased. The saved
# version in the database is always lowercased. # version in the database is always lowercased.
"crsid": account.crsid.lower(), "crsid": account.crsid.lower(),
"last_name": last_name,
"date_of_birth": date_of_birth.strftime("%Y-%m-%d"), "date_of_birth": date_of_birth.strftime("%Y-%m-%d"),
"name": account_details.name, "name": account_details.name,
"affiliation": account_details.affiliation, "affiliation": account_details.affiliation,
......
import re
import pytest import pytest
from django.conf import settings from django.conf import settings
from django.test import override_settings
from django.urls import reverse from django.urls import reverse
from rest_framework import status from rest_framework import status
...@@ -60,3 +63,14 @@ def test_missing_user_reset_token( ...@@ -60,3 +63,14 @@ def test_missing_user_reset_token(
assert response.status_code == status.HTTP_400_BAD_REQUEST assert response.status_code == status.HTTP_400_BAD_REQUEST
# Call once and not retried # Call once and not retried
assert missing_user_reset_token_response.call_count == 1 assert missing_user_reset_token_response.call_count == 1
@override_settings(FAKE_RESET_TOKEN_IF_MISSING=True)
def test_missing_user_fake_reset_token(
authenticated_api_client, url, missing_user_reset_token_response
):
response = authenticated_api_client.get(url)
assert response.status_code == status.HTTP_200_OK
# Looks like a reset token but with FAKE at the end
assert re.fullmatch(r"([0-9A-Z]{4}-){3}FAKE", response.data["token"])
import re
import unicodedata
def normalise_name(name):
"""
Performs some changes to the given name to provide something that can be used when for
comparison when attempting to authenticate with a last name. Simulates (some of) the
behaviour that jackdaw signup processes use.
"""
# Replace some special characters (most importantly hyphens) with spaces
make_spaces = [
# hyphen-like characters
"-",
"\u2010",
"\u2013",
"\u2014",
"\u2015",
# other characters jackdaw replaces with spaces
"?",
"*",
".",
"%",
"_",
]
for char in make_spaces:
name = name.replace(char, " ")
# Multiple whitespaces to a single space
name = re.sub(r"\s{2,}", " ", name)
# Jackdaw only has ascii/latin characters so we convert diacritics to their base character
name = unicodedata.normalize("NFKD", name).encode("ascii", "ignore").decode("utf8")
# Jackdaw also converts some vowels couples to a single character, but we aren't sure we want
# to do that here.
# i.e. "AA" with "A", "AE" with "A", "OE" with "O", "UE" with "U"
# Remove leading and trailing whitespaces
return name.strip().upper()
...@@ -11,6 +11,7 @@ from authentication.constants import ( ...@@ -11,6 +11,7 @@ from authentication.constants import (
SESSION_GRANT_TYPE, SESSION_GRANT_TYPE,
) )
from authentication.errors import InvalidGrantError, UnsupportedGrantTypeError from authentication.errors import InvalidGrantError, UnsupportedGrantTypeError
from authentication.normalisation import normalise_name
logger = structlog.get_logger(__name__) logger = structlog.get_logger(__name__)
...@@ -102,6 +103,7 @@ class TokenRequestSerializer(serializers.Serializer): ...@@ -102,6 +103,7 @@ class TokenRequestSerializer(serializers.Serializer):
) )
raise InvalidGrantError("Account locked out") raise InvalidGrantError("Account locked out")
name_match = None
try: try:
if "crsid" in data: if "crsid" in data:
account = Account.objects.get( account = Account.objects.get(
...@@ -110,11 +112,32 @@ class TokenRequestSerializer(serializers.Serializer): ...@@ -110,11 +112,32 @@ class TokenRequestSerializer(serializers.Serializer):
account_identifier__code=data["code"], account_identifier__code=data["code"],
) )
else: else:
account = Account.objects.get( name_match = normalise_name(data["last_name"])
account_identifier__last_name=data["last_name"], if not name_match:
account_identifier__date_of_birth=data["date_of_birth"], # If we don't have anything to compare, treat as not found
account_identifier__code=data["code"], raise Account.DoesNotExist
) try:
# first match last_name fields starting with name_match followed by a space
account = Account.objects.get(
account_identifier__last_name__istartswith=f"{name_match} ",
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(
account_identifier__last_name=name_match,
account_identifier__date_of_birth=data["date_of_birth"],
account_identifier__code=data["code"],
)
except Account.DoesNotExist: except Account.DoesNotExist:
if lockout: if lockout:
lockout.attempts += 1 lockout.attempts += 1
...@@ -135,6 +158,7 @@ class TokenRequestSerializer(serializers.Serializer): ...@@ -135,6 +158,7 @@ class TokenRequestSerializer(serializers.Serializer):
crsid=data.get("crsid"), crsid=data.get("crsid"),
last_name=data.get("last_name"), last_name=data.get("last_name"),
date_of_birth=data.get("date_of_birth"), date_of_birth=data.get("date_of_birth"),
name_match=name_match,
) )
raise InvalidGrantError("No matching account") raise InvalidGrantError("No matching account")
......
import random
from string import ascii_uppercase
from unittest import mock from unittest import mock
from urllib.parse import urlencode from urllib.parse import urlencode
...@@ -6,6 +8,7 @@ from django.urls import reverse ...@@ -6,6 +8,7 @@ from django.urls import reverse
from knox.settings import knox_settings from knox.settings import knox_settings
from rest_framework import exceptions, parsers, status from rest_framework import exceptions, parsers, status
from activate_account.factories import AccountFactory, AccountIdentifierFactory
from authentication.constants import SESSION_GRANT_TYPE from authentication.constants import SESSION_GRANT_TYPE
from authentication.serializers import TokenRequestSerializer from authentication.serializers import TokenRequestSerializer
...@@ -85,21 +88,86 @@ def test_valid_session_grant(client, valid_session_grant_body): ...@@ -85,21 +88,86 @@ def test_valid_session_grant(client, valid_session_grant_body):
assert_is_valid_login(post_login(client, valid_session_grant_body)) assert_is_valid_login(post_login(client, valid_session_grant_body))
@pytest.mark.parametrize("valid_session_grant_body", ["crsid", "last_name"], indirect=True) @pytest.mark.parametrize("valid_session_grant_body", ["crsid"], indirect=True)
def test_valid_case_insensitive(request, client, valid_session_grant_body): def test_valid_crsid_case_insensitive(client, valid_session_grant_body):
"""CRSId and Last namecomparison is case insensitive""" """CRSId comparison is case insensitive"""
field = request.node.callspec.params["valid_session_grant_body"] field = "crsid"
assert_is_valid_login(
post_login(
client,
{
**valid_session_grant_body,
field: valid_session_grant_body[field].upper(), # Upper case as fixture is lower
},
)
)
@pytest.mark.parametrize("valid_session_grant_body", ["last_name"], indirect=True)
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(
post_login(
client,
{
**valid_session_grant_body,
field: valid_session_grant_body[field].lower(), # Lower case as fixture is upper
},
)
)
@pytest.mark.parametrize("valid_session_grant_body", ["last_name"], indirect=True)
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
just_first_part = valid_session_grant_body[field].split(" ")[0].lower()
just_first_part = "".join(
just_first_part[i].upper() if faker.boolean() else just_first_part[i]
for i in range(len(just_first_part))
)
assert_is_valid_login( assert_is_valid_login(
post_login( post_login(
client, client,
{ {
**valid_session_grant_body, **valid_session_grant_body,
field: valid_session_grant_body[field].upper(), field: just_first_part,
}, },
) )
) )
@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) @pytest.mark.parametrize("valid_session_grant_body", ["crsid", "last_name"], indirect=True)
def test_missing_field(client, request, valid_session_grant_body): def test_missing_field(client, request, valid_session_grant_body):
"""Missing a required field results in 'invalid_request'""" """Missing a required field results in 'invalid_request'"""
......
import pytest
from authentication.normalisation import normalise_name
EXPECTED_NORMALISATION = [
# Fairly common "anglicized" names
("Morgan A.", "MORGAN A"),
("O'Leary P.B.", "O'LEARY P B"),
("von Neumann J.", "VON NEUMANN J"),
# Diacritics and hyphens
("Müller-Platz H.", "MULLER PLATZ H"),
("Ångström A.", "ANGSTROM A"),
("König M.A.", "KONIG M A"),
("Hernández H.P.", "HERNANDEZ H P"),
("Crête-Lafrenière P.", "CRETE LAFRENIERE P"),
("Saint\u2013Martin Dr J.", "SAINT MARTIN DR J"),
# Some characters are just lost (not good)
("Hermeß M.", "HERME M"),
("Æsir B.", "SIR B"),
("Bjørnson Prof. G.", "BJRNSON PROF G"),
# Nonsense
("Bad?Data****Entered_%Here", "BAD DATA ENTERED HERE"),
]
@pytest.mark.parametrize("unnormalised,normalised", EXPECTED_NORMALISATION)
def test_normalise_name(unnormalised, normalised):
assert normalise_name(unnormalised) == normalised
import pytest
from authentication.constants import SESSION_GRANT_TYPE from authentication.constants import SESSION_GRANT_TYPE
from authentication.errors import InvalidGrantError
from authentication.serializers import TokenRequestSerializer from authentication.serializers import TokenRequestSerializer
...@@ -28,3 +31,18 @@ def test_missing_crsid_and_last_name(): ...@@ -28,3 +31,18 @@ def test_missing_crsid_and_last_name():
assert serializer.errors == { assert serializer.errors == {
"non_field_errors": ["At least one of crsid and last name must be provided"] "non_field_errors": ["At least one of crsid and last name must be provided"]
} }
@pytest.mark.django_db
def test_empty_post_normalise_last_name():
data = {
"grant_type": SESSION_GRANT_TYPE,
"date_of_birth": "1980-03-25",
"last_name": "-?_",
"code": "ABCDEF123456",
}
serializer = TokenRequestSerializer(data=data)
# Fails with "no matching account" before checking the database
with pytest.raises(InvalidGrantError) as excinfo:
serializer.is_valid()
assert str(excinfo.value) == "No matching account"
...@@ -2,6 +2,7 @@ from django.db import transaction ...@@ -2,6 +2,7 @@ from django.db import transaction
from rest_framework import serializers from rest_framework import serializers
from activate_account.models import Account, AccountDetails, AccountIdentifier from activate_account.models import Account, AccountDetails, AccountIdentifier
from authentication.normalisation import normalise_name
class ValidAtSerializer(serializers.Serializer): class ValidAtSerializer(serializers.Serializer):
...@@ -18,7 +19,7 @@ class AccountDataSerializer(serializers.ModelSerializer): ...@@ -18,7 +19,7 @@ class AccountDataSerializer(serializers.ModelSerializer):
allow_empty=False, allow_empty=False,
write_only=True, write_only=True,
) )
last_name = serializers.CharField(write_only=True) last_name = serializers.CharField(write_only=True, required=False)
date_of_birth = serializers.DateField(write_only=True) date_of_birth = serializers.DateField(write_only=True)
class Meta: class Meta:
...@@ -53,7 +54,10 @@ class AccountDataSerializer(serializers.ModelSerializer): ...@@ -53,7 +54,10 @@ class AccountDataSerializer(serializers.ModelSerializer):
def create(self, validated_data): def create(self, validated_data):
codes = validated_data.pop("codes", []) codes = validated_data.pop("codes", [])
account_details = validated_data.pop("account_details") account_details = validated_data.pop("account_details")
last_name = validated_data.pop("last_name") # Calculate last_name from normalising full name
last_name = normalise_name(account_details.get("name", ""))
# Remove last_name if present in request
validated_data.pop("last_name", None)
date_of_birth = validated_data.pop("date_of_birth") date_of_birth = validated_data.pop("date_of_birth")
account = Account.objects.create(valid_at=self.valid_at, **validated_data) account = Account.objects.create(valid_at=self.valid_at, **validated_data)
AccountDetails.objects.create( AccountDetails.objects.create(
...@@ -74,7 +78,11 @@ class AccountDataSerializer(serializers.ModelSerializer): ...@@ -74,7 +78,11 @@ class AccountDataSerializer(serializers.ModelSerializer):
def update(self, instance, validated_data): def update(self, instance, validated_data):
codes = validated_data.pop("codes", []) codes = validated_data.pop("codes", [])
account_details_data = validated_data.pop("account_details") account_details_data = validated_data.pop("account_details")
last_name = validated_data.pop("last_name") # Calculate last_name from normalising full name
last_name = normalise_name(account_details_data.get("name", ""))
# Remove last_name if present in request
validated_data.pop("last_name", None)
date_of_birth = validated_data.pop("date_of_birth") date_of_birth = validated_data.pop("date_of_birth")
instance.account_identifier.all().delete() instance.account_identifier.all().delete()
......
...@@ -3,6 +3,7 @@ from rest_framework.exceptions import ValidationError ...@@ -3,6 +3,7 @@ from rest_framework.exceptions import ValidationError
from activate_account.factories import AccountFactory from activate_account.factories import AccountFactory
from activate_account.models import Account, AccountIdentifier from activate_account.models import Account, AccountIdentifier
from authentication.normalisation import normalise_name
from data_manager_api.serializers import AccountDataSerializer from data_manager_api.serializers import AccountDataSerializer
from data_manager_api.tests.utils import data_account_factory from data_manager_api.tests.utils import data_account_factory
...@@ -17,6 +18,7 @@ def test_create(): ...@@ -17,6 +18,7 @@ def test_create():
name = data["name"] name = data["name"]
college = data["college"] college = data["college"]
affiliation = data["affiliation"] affiliation = data["affiliation"]
dob = data["date_of_birth"]
valid_at = account.valid_at valid_at = account.valid_at
assert not Account.objects.filter(crsid=data["crsid"]).exists() # Sanity check assert not Account.objects.filter(crsid=data["crsid"]).exists() # Sanity check
...@@ -30,17 +32,15 @@ def test_create(): ...@@ -30,17 +32,15 @@ def test_create():
assert instance.crsid == account.crsid assert instance.crsid == account.crsid
account_identifiers = instance.account_identifier.all() account_identifiers = instance.account_identifier.all()
assert account_identifiers.first().code == codes[0] first_account_identifier = account_identifiers.first()
assert first_account_identifier.code == codes[0]
assert first_account_identifier.date_of_birth == dob
assert first_account_identifier.last_name == normalise_name(name)
assert instance.account_details.name == name assert instance.account_details.name == name
assert instance.account_details.college == college assert instance.account_details.college == college
assert instance.account_details.affiliation == affiliation assert instance.account_details.affiliation == affiliation
for attribute in ["date_of_birth", "last_name"]:
assert getattr(instance.account_identifier.first(), attribute) == getattr(
account_identifiers.first(), attribute
)
@pytest.mark.parametrize("partial", [True, False]) @pytest.mark.parametrize("partial", [True, False])
def test_update(partial): def test_update(partial):
...@@ -49,7 +49,7 @@ def test_update(partial): ...@@ -49,7 +49,7 @@ def test_update(partial):
initial_data = data_account_factory(account) initial_data = data_account_factory(account)
new_data = { new_data = {
"name": "Lee Sang-hyeok", "name": "Lee Sang-hyeok",
"last_name": "Lee", "last_name": "Ignored",
"date_of_birth": "1996-05-07", "date_of_birth": "1996-05-07",
"codes": ["testcode123"], "codes": ["testcode123"],
} }
...@@ -68,6 +68,7 @@ def test_update(partial): ...@@ -68,6 +68,7 @@ def test_update(partial):
instance = serializer.save() instance = serializer.save()
assert instance.account_details.name == new_data["name"] assert instance.account_details.name == new_data["name"]
assert instance.account_identifier.first().last_name == normalise_name(new_data["name"])
def test_override_nullable_fields(): def test_override_nullable_fields():
...@@ -102,10 +103,10 @@ def test_update_removes_old_codes(): ...@@ -102,10 +103,10 @@ def test_update_removes_old_codes():
def test_codes_empty_validation(): def test_codes_empty_validation():
data = { data = {
"crsid": "ab1234", "crsid": "ab1234",
"last_name": "Doe", "last_name": "Ignored",
"date_of_birth": "2000-01-01", "date_of_birth": "2000-01-01",
"codes": [], "codes": [],
"name": "John", "name": "Doe J.",
"affiliation": "Student", "affiliation": "Student",
"college": "College", "college": "College",
} }
......
...@@ -12,6 +12,7 @@ from activate_account.factories import ( ...@@ -12,6 +12,7 @@ from activate_account.factories import (
AccountIdentifierFactory, AccountIdentifierFactory,
) )
from activate_account.models import Account, AccountIdentifier from activate_account.models import Account, AccountIdentifier
from authentication.normalisation import normalise_name
from data_manager_api.tests.utils import data_account_factory from data_manager_api.tests.utils import data_account_factory
pytestmark = pytest.mark.django_db pytestmark = pytest.mark.django_db
...@@ -56,7 +57,8 @@ def test_account_details_post_creation(api_client): ...@@ -56,7 +57,8 @@ def test_account_details_post_creation(api_client):
saved_account = Account.objects.get(crsid=crsid) saved_account = Account.objects.get(crsid=crsid)
assert saved_account.crsid == data["crsid"] assert saved_account.crsid == data["crsid"]
assert saved_account.account_identifier.first().last_name == data["last_name"] # last_name is a normalised version of the name, last_name in the request is ignored
assert saved_account.account_identifier.first().last_name == normalise_name(data["name"])
assert saved_account.account_identifier.first().date_of_birth == data["date_of_birth"] assert saved_account.account_identifier.first().date_of_birth == data["date_of_birth"]
saved_codes = {code.code for code in saved_account.account_identifier.all()} saved_codes = {code.code for code in saved_account.account_identifier.all()}
...@@ -88,19 +90,17 @@ def test_account_update_codes(api_client): ...@@ -88,19 +90,17 @@ def test_account_update_codes(api_client):
def test_account_update(api_client): def test_account_update(api_client):
account = AccountFactory(account_details=True, account_identifier=True) account = AccountFactory(account_details=True, account_identifier=True)
account_details = account.account_details account_details = account.account_details
account_identifier = account.account_identifier.first()
valid_at = account.valid_at + timedelta(hours=1) valid_at = account.valid_at + timedelta(hours=1)
initial_data = data_account_factory(account) initial_data = data_account_factory(account)
new_data = { new_data = {
"name": "Ryu Min-seok", "name": "Ryu Min-seok",
"codes": ["new_code"], "codes": ["new_code"],
"last_name": "Ryu", "last_name": "Ignored", # No longer required
"date_of_birth": "1996-05-07", "date_of_birth": "1996-05-07",
} }
assert account_details.name != new_data["name"] # Sanity check assert account_details.name != new_data["name"] # Sanity check
assert account_identifier.last_name != new_data["last_name"]
response = api_client.put( response = api_client.put(
build_account_detail_url(account.crsid, valid_at), build_account_detail_url(account.crsid, valid_at),
...@@ -116,7 +116,8 @@ def test_account_update(api_client): ...@@ -116,7 +116,8 @@ def test_account_update(api_client):
crsid=account.crsid, account_details__name=new_data["name"] crsid=account.crsid, account_details__name=new_data["name"]
).exists() ).exists()
assert account.account_identifier.all().first().last_name == new_data["last_name"] # last_name is a normalised version of the name
assert account.account_identifier.all().first().last_name == normalise_name(new_data["name"])
assert ( assert (
account.account_identifier.all().first().date_of_birth.strftime("%Y-%m-%d") account.account_identifier.all().first().date_of_birth.strftime("%Y-%m-%d")
== new_data["date_of_birth"] == new_data["date_of_birth"]
...@@ -136,7 +137,7 @@ def test_account_update_valid_at_earlier(api_client): ...@@ -136,7 +137,7 @@ def test_account_update_valid_at_earlier(api_client):
new_data = { new_data = {
"name": "Lee Min-hyung", "name": "Lee Min-hyung",
"codes": ["new_code"], "codes": ["new_code"],
"last_name": "Lee", "last_name": "Ignored",
"date_of_birth": "1996-05-07", "date_of_birth": "1996-05-07",
} }
...@@ -235,7 +236,7 @@ def test_read_only_mode_update(api_client, settings): ...@@ -235,7 +236,7 @@ def test_read_only_mode_update(api_client, settings):
new_data = { new_data = {
"name": "Ryu Min-seok", "name": "Ryu Min-seok",
"codes": ["new_code"], "codes": ["new_code"],
"last_name": "Ryu", "last_name": "Ignored",
"date_of_birth": "1996-05-07", "date_of_birth": "1996-05-07",
} }
...@@ -307,10 +308,10 @@ def test_account_delete_then_update(api_client): ...@@ -307,10 +308,10 @@ def test_account_delete_then_update(api_client):
# Account is undeleted # Account is undeleted
assert Account.objects.filter(crsid=account.crsid, deleted_at=None).exists() assert Account.objects.filter(crsid=account.crsid, deleted_at=None).exists()
assert ( # last_name is a normalised version of the name
Account.objects.get(crsid=account.crsid).account_identifier.first().last_name assert Account.objects.get(
== data["last_name"] crsid=account.crsid
) ).account_identifier.first().last_name == normalise_name(data["name"])
def test_account_multiple_deletes(api_client): def test_account_multiple_deletes(api_client):
...@@ -344,24 +345,29 @@ def test_account_multiple_deletes(api_client): ...@@ -344,24 +345,29 @@ def test_account_multiple_deletes(api_client):
def test_unique_identifier_information(api_client): def test_unique_identifier_information(api_client):
account = AccountFactory.build() account = AccountFactory.build()
account_identifier = AccountIdentifierFactory.build(account_id=account) account_details = AccountDetailsFactory.build(account=account)
account_identifier = AccountIdentifierFactory.build(
account_id=account, last_name=normalise_name(account_details.name)
)
response = api_client.put( response = api_client.put(
build_account_detail_url(account.crsid, timezone.now()), build_account_detail_url(account.crsid, timezone.now()),
data_account_factory(account, {}, account_identifier), data_account_factory(account, account_details, account_identifier),
) )
assert response.status_code == status.HTTP_201_CREATED assert response.status_code == status.HTTP_201_CREATED
assert Account.objects.filter(crsid=account.crsid).exists() assert Account.objects.filter(crsid=account.crsid).exists()
# Build another account with the same name in the account details (as this is the source of
# the last_name of the account identifier)
other_account = AccountFactory.build() other_account = AccountFactory.build()
AccountDetailsFactory.build(account=other_account) account_details = AccountDetailsFactory.build(account=other_account, name=account_details.name)
assert account.crsid != other_account.crsid # Sanity check assert account.crsid != other_account.crsid # Sanity check
response = api_client.put( response = api_client.put(
build_account_detail_url(other_account.crsid, timezone.now()), build_account_detail_url(other_account.crsid, timezone.now()),
data_account_factory(other_account, {}, account_identifier), data_account_factory(other_account, account_details, account_identifier),
) )
assert response.status_code != status.HTTP_201_CREATED assert response.status_code != status.HTTP_201_CREATED
......
...@@ -16,6 +16,5 @@ def data_account_factory(account=None, account_details=None, account_identifier= ...@@ -16,6 +16,5 @@ def data_account_factory(account=None, account_details=None, account_identifier=
"college": account_details.college, "college": account_details.college,
"affiliation": account_details.affiliation, "affiliation": account_details.affiliation,
"codes": [account_identifier.code], "codes": [account_identifier.code],
"last_name": account_identifier.last_name,
"date_of_birth": account_identifier.date_of_birth, "date_of_birth": account_identifier.date_of_birth,
} }