From 46641cdc2602137314a4e932ecb13f9b72177991 Mon Sep 17 00:00:00 2001 From: Henrich Neumann <henrich.neumann@usit.uio.no> Date: Tue, 11 Jul 2023 10:18:11 +0200 Subject: [PATCH] Disallow certain characters in first and last name on creation of invitation and creation of guest When a sponsor creates a new guest invitation, and when a guest corrects their name they must now use transcribed characters. --- frontend/package-lock.json | 30 +++++++++----- frontend/public/locales/en/common.json | 2 + frontend/public/locales/nb/common.json | 2 + frontend/public/locales/nn/common.json | 4 +- .../routes/guest/register/steps/register.tsx | 8 ++-- .../sponsor/register/stepPersonForm.tsx | 23 +++++------ frontend/src/utils/index.test.ts | 22 ++++++++++ frontend/src/utils/index.ts | 28 ++++++++++++- greg/tests/test_utils.py | 20 ++++++++- greg/utils.py | 5 +++ gregui/api/serializers/guest.py | 12 +++++- gregui/api/serializers/invitation.py | 18 +++++++- gregui/tests/api/views/test_invite_guest.py | 41 +++++++++++++++++-- 13 files changed, 180 insertions(+), 35 deletions(-) diff --git a/frontend/package-lock.json b/frontend/package-lock.json index c5066b34..37ddd870 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -6583,13 +6583,23 @@ } }, "node_modules/caniuse-lite": { - "version": "1.0.30001271", - "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001271.tgz", - "integrity": "sha512-BBruZFWmt3HFdVPS8kceTBIguKxu4f99n5JNp06OlPD/luoAMIaIK5ieV5YjnBLH3Nysai9sxj9rpJj4ZisXOA==", - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/browserslist" - } + "version": "1.0.30001512", + "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001512.tgz", + "integrity": "sha512-2S9nK0G/mE+jasCUsMPlARhRCts1ebcp2Ji8Y8PWi4NDE1iRdLCnEPHkEfeBrGC45L4isBx5ur3IQ6yTE2mRZw==", + "funding": [ + { + "type": "opencollective", + "url": "https://opencollective.com/browserslist" + }, + { + "type": "tidelift", + "url": "https://tidelift.com/funding/github/npm/caniuse-lite" + }, + { + "type": "github", + "url": "https://github.com/sponsors/ai" + } + ] }, "node_modules/capture-exit": { "version": "2.0.0", @@ -28885,9 +28895,9 @@ } }, "caniuse-lite": { - "version": "1.0.30001271", - "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001271.tgz", - "integrity": "sha512-BBruZFWmt3HFdVPS8kceTBIguKxu4f99n5JNp06OlPD/luoAMIaIK5ieV5YjnBLH3Nysai9sxj9rpJj4ZisXOA==" + "version": "1.0.30001512", + "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001512.tgz", + "integrity": "sha512-2S9nK0G/mE+jasCUsMPlARhRCts1ebcp2Ji8Y8PWi4NDE1iRdLCnEPHkEfeBrGC45L4isBx5ur3IQ6yTE2mRZw==" }, "capture-exit": { "version": "2.0.0", diff --git a/frontend/public/locales/en/common.json b/frontend/public/locales/en/common.json index 8b547a71..2a2c84e8 100644 --- a/frontend/public/locales/en/common.json +++ b/frontend/public/locales/en/common.json @@ -125,7 +125,9 @@ "nationalIdNumber": "Norwegian national ID number", "validation": { "firstNameRequired": "First name is required", + "firstNameContainsInvalidChars": "First name contains invalid characters", "lastNameRequired": "Last name is required", + "lastNameContainsInvalidChars": "Last name contains invalid characters", "dateOfBirthRequired": "Date of birth is required", "invalidIdNumber": "Invalid Norwegian national ID number", "nationalIdNumberRequired": "Norwegian national ID number required", diff --git a/frontend/public/locales/nb/common.json b/frontend/public/locales/nb/common.json index 5060e4dd..bf16582c 100644 --- a/frontend/public/locales/nb/common.json +++ b/frontend/public/locales/nb/common.json @@ -125,7 +125,9 @@ "nationalIdNumber": "Fødselsnummer/D-nummer", "validation": { "firstNameRequired": "Fornavn er obligatorisk", + "firstNameContainsInvalidChars": "Fornavn inneholder ugyldige karakterer", "lastNameRequired": "Etternavn er obligatorisk", + "lastNameContainsInvalidChars": "Etternavn inneholder ugyldige karakterer", "dateOfBirthRequired": "Fødselsdato er obligatorisk", "invalidIdNumber": "Ugyldig fødselsnummer/D-nummer", "nationalIdNumberRequired": "Fødselsnummer/D-nummer er obligatorisk", diff --git a/frontend/public/locales/nn/common.json b/frontend/public/locales/nn/common.json index d7e87e62..7a28dc41 100644 --- a/frontend/public/locales/nn/common.json +++ b/frontend/public/locales/nn/common.json @@ -125,7 +125,9 @@ "nationalIdNumber": "Fødselsnummer/D-nummer", "validation": { "firstNameRequired": "Fornamn er pÃ¥krevd", + "firstNameContainsInvalidChars": "Fornamn inneheld ugyldige karakterar", "lastNameRequired": "Etternamn er pÃ¥krevd", + "lastNameContainsInvalidChars": "Etternamn inneheld ugyldige karakterar", "dateOfBirthRequired": "Fødselsdato er pÃ¥krevd", "invalidIdNumber": "Ugyldig fødselsnummer/D-nummer", "nationalIdNumberRequired": "Fødselsnummer/D-nummer er pÃ¥krevd", @@ -235,4 +237,4 @@ "update": { "email": "E-postadressa ble endra" } -} +} \ No newline at end of file diff --git a/frontend/src/routes/guest/register/steps/register.tsx b/frontend/src/routes/guest/register/steps/register.tsx index 576b9b2a..86f52bd2 100644 --- a/frontend/src/routes/guest/register/steps/register.tsx +++ b/frontend/src/routes/guest/register/steps/register.tsx @@ -28,7 +28,9 @@ import { getAlpha2Codes, getName } from 'i18n-iso-countries' import { DatePicker } from '@mui/lab' import { subYears } from 'date-fns/fp' import { + isValidFirstName, isValidFnr, + isValidLastName, isValidMobilePhoneNumber, extractGenderOrBlank, extractBirthdateFromNationalId, @@ -320,7 +322,7 @@ const GuestRegisterStep = forwardRef( (initialGuestData.authentication_method === AuthenticationMethod.Feide || initialGuestData.authentication_method === - AuthenticationMethod.IdPorten) && + AuthenticationMethod.IdPorten) && initialGuestData.fnr !== null && initialGuestData.fnr?.length !== 0, } @@ -372,7 +374,7 @@ const GuestRegisterStep = forwardRef( name="firstName" control={control} rules={{ - required: t('common:validation.firstNameRequired').toString(), + validate: isValidFirstName, }} render={({ field: { onChange, value } }) => ( <TextField @@ -394,7 +396,7 @@ const GuestRegisterStep = forwardRef( name="lastName" control={control} rules={{ - required: t('common:validation.lastNameRequired').toString(), + validate: isValidLastName, }} render={({ field: { onChange, value } }) => ( <TextField diff --git a/frontend/src/routes/sponsor/register/stepPersonForm.tsx b/frontend/src/routes/sponsor/register/stepPersonForm.tsx index e9c8f5a1..c6075fe6 100644 --- a/frontend/src/routes/sponsor/register/stepPersonForm.tsx +++ b/frontend/src/routes/sponsor/register/stepPersonForm.tsx @@ -22,7 +22,7 @@ import useOus, { enSort, nbSort, OuData } from 'hooks/useOus' import useRoleTypes, { RoleTypeData } from 'hooks/useRoleTypes' import Autocomplete from '@mui/material/Autocomplete' import Loading from 'components/loading' -import { isValidEmail } from 'utils' +import { isValidEmail, isValidFirstName, isValidLastName } from 'utils' import { FeatureContext } from 'contexts' import { RegisterFormData } from './formData' import { PersonFormMethods } from './personFormMethods' @@ -138,7 +138,7 @@ const StepPersonForm = forwardRef( const roleEnd = getValues('role_end') if (roleEnd && startDate > roleEnd) { - // The role end date is set, but is is before the start date + // The role end date is set, but is before the start date return t('validation.startDateMustBeBeforeEndDate') } return true @@ -147,20 +147,17 @@ const StepPersonForm = forwardRef( function getFullOptionLabel(ouData: OuData) { switch (i18n.language) { case 'en': - return `${ouData.en}${ - ouData.identifier_1 ? ` (${ouData.identifier_1})` : '' - }${ouData.identifier_2 ? ` (${ouData.identifier_2})` : ''}` + return `${ouData.en}${ouData.identifier_1 ? ` (${ouData.identifier_1})` : '' + }${ouData.identifier_2 ? ` (${ouData.identifier_2})` : ''}` case 'nn': - return `${ouData.nb}${ - ouData.identifier_1 ? ` (${ouData.identifier_1})` : '' - }${ouData.identifier_2 ? ` (${ouData.identifier_2})` : ''}` + return `${ouData.nb}${ouData.identifier_1 ? ` (${ouData.identifier_1})` : '' + }${ouData.identifier_2 ? ` (${ouData.identifier_2})` : ''}` default: // There should always be a Norwegian bokmaal acronym set - return `${ouData.nb}${ - ouData.identifier_1 ? ` (${ouData.identifier_1})` : '' - }${ouData.identifier_2 ? ` (${ouData.identifier_2})` : ''}` + return `${ouData.nb}${ouData.identifier_1 ? ` (${ouData.identifier_1})` : '' + }${ouData.identifier_2 ? ` (${ouData.identifier_2})` : ''}` } } @@ -198,7 +195,7 @@ const StepPersonForm = forwardRef( error={!!errors.first_name} helperText={errors.first_name && errors.first_name.message} {...register(`first_name`, { - required: t<string>('validation.firstNameRequired'), + validate: isValidFirstName, })} /> <TextField @@ -207,7 +204,7 @@ const StepPersonForm = forwardRef( error={!!errors.last_name} helperText={errors.last_name && errors.last_name.message} {...register(`last_name`, { - required: t<string>('validation.lastNameRequired'), + validate: isValidLastName, })} inputProps={{ 'data-testid': 'lastName-input-field' }} /> diff --git a/frontend/src/utils/index.test.ts b/frontend/src/utils/index.test.ts index 5067541a..3acfa0d2 100644 --- a/frontend/src/utils/index.test.ts +++ b/frontend/src/utils/index.test.ts @@ -4,7 +4,9 @@ import { deleteCookie, setCookie, isValidEmail, + isValidFirstName, isValidFnr, + isValidLastName, isValidMobilePhoneNumber, maybeCsrfToken, submitJsonOpts, @@ -54,6 +56,26 @@ test('Invalid e-mail', async () => { }) }) +test('Valid first name', async () => { + expect(isValidFirstName('Ååæž')).toEqual(true) +}) + +test('Invalid first name', async () => { + expect(isValidFirstName('')).toEqual('common:validation.firstNameRequired') + expect(isValidFirstName('aaƂåå')).toEqual('common:validation.firstNameContainsInvalidChars') + expect(isValidFirstName('汉å—')).toEqual('common:validation.firstNameContainsInvalidChars') +}) + +test('Valid last name', async () => { + expect(isValidLastName('Ååæž')).toEqual(true) +}) + +test('Invalid last name', async () => { + expect(isValidLastName('')).toEqual('common:validation.lastNameRequired') + expect(isValidLastName('aaƂåå')).toEqual('common:validation.lastNameContainsInvalidChars') + expect(isValidLastName('汉å—')).toEqual('common:validation.lastNameContainsInvalidChars') +}) + test('Body has values', async () => { const data = { foo: 'bar' } expect(submitJsonOpts('POST', data)).toEqual({ diff --git a/frontend/src/utils/index.ts b/frontend/src/utils/index.ts index 0dd69ab5..d482cd40 100644 --- a/frontend/src/utils/index.ts +++ b/frontend/src/utils/index.ts @@ -121,6 +121,32 @@ export async function isValidEmail(data: string | undefined) { return i18n.t<string>('common:validation.invalidEmail') } +function stringContainsIllegalChars(string: string): boolean { + // Only allow ISO-8859-1 and Latin Extended-A + // eslint-disable-next-line no-control-regex + return /[^\u0000-\u017F]/g.test(string) +} + +export function isValidFirstName(data: string | undefined): string | true { + if (!data) { + return i18n.t<string>('common:validation.firstNameRequired') + } + if (stringContainsIllegalChars(data)) { + return i18n.t<string>('common:validation.firstNameContainsInvalidChars') + } + return true +} + +export function isValidLastName(data: string | undefined): string | true { + if (!data) { + return i18n.t<string>('common:validation.lastNameRequired') + } + if (stringContainsIllegalChars(data)) { + return i18n.t<string>('common:validation.lastNameContainsInvalidChars') + } + return true +} + /** * Splits a phone number into a country code and the national number. * @@ -315,7 +341,7 @@ function extractBirthdateFromFnr(nationalId: string): Date { function extractBirthdateFromDnumber(nationalId: string): Date { return suggestBirthDate( (parseInt(nationalId.charAt(0), 10) - 4).toString(10) + - nationalId.substring(1, 6) + nationalId.substring(1, 6) ) } diff --git a/greg/tests/test_utils.py b/greg/tests/test_utils.py index c77def93..696a742b 100644 --- a/greg/tests/test_utils.py +++ b/greg/tests/test_utils.py @@ -1,5 +1,11 @@ +import pytest from django.conf import settings -from greg.utils import is_valid_id_number, is_valid_so_number + +from greg.utils import ( + is_valid_id_number, + is_valid_so_number, + string_contains_illegal_chars, +) def test_so_number(): @@ -21,3 +27,15 @@ def test_not_valid_so_number(): settings.ALLOW_SO_NUMBERS = True assert not is_valid_id_number(so_number) assert not is_valid_so_number(so_number) + + +@pytest.mark.parametrize( + "string, expected_output", + [ + ("Ååæž", False), + ("aaƂåå", True), + ("汉å—", True), + ], +) +def test_string_contains_illegal_chars(string, expected_output): + assert string_contains_illegal_chars(string) == expected_output diff --git a/greg/utils.py b/greg/utils.py index bc108f35..8efa0b4b 100644 --- a/greg/utils.py +++ b/greg/utils.py @@ -220,3 +220,8 @@ def role_invitation_date_validator( raise serializers.ValidationError( f"New end date too far into the future for this type. Must be before {max_date.strftime('%Y-%m-%d')}." ) + + +def string_contains_illegal_chars(string: str) -> bool: + # Only allow ISO-8859-1 and Latin Extended-A + return bool(re.search(r"[^\u0000-\u017F]", string)) diff --git a/gregui/api/serializers/guest.py b/gregui/api/serializers/guest.py index d18f56ff..444c37bf 100644 --- a/gregui/api/serializers/guest.py +++ b/gregui/api/serializers/guest.py @@ -16,7 +16,7 @@ from greg.models import ( Person, InvitationLink, ) -from greg.utils import is_identity_duplicate +from greg.utils import is_identity_duplicate, string_contains_illegal_chars from gregui.api.serializers.identity import ( PartialIdentitySerializer, IdentityDuplicateError, @@ -144,6 +144,16 @@ class GuestRegisterSerializer(serializers.ModelSerializer): consent_instance.choice = choice consent_instance.save() + def validate_first_name(self, first_name): + if string_contains_illegal_chars(first_name): + raise serializers.ValidationError("First name contains illegal characters") + return first_name + + def validate_last_name(self, last_name): + if string_contains_illegal_chars(last_name): + raise serializers.ValidationError("Last name contains illegal characters") + return last_name + def validate_date_of_birth(self, date_of_birth): today = datetime.date.today() diff --git a/gregui/api/serializers/invitation.py b/gregui/api/serializers/invitation.py index add333aa..7feddf95 100644 --- a/gregui/api/serializers/invitation.py +++ b/gregui/api/serializers/invitation.py @@ -1,13 +1,19 @@ from rest_framework import serializers from greg.models import Person, Identity -from greg.utils import create_objects_for_invitation, is_identity_duplicate +from greg.utils import ( + create_objects_for_invitation, + is_identity_duplicate, + string_contains_illegal_chars, +) from gregui.api.serializers.identity import IdentityDuplicateError from gregui.api.serializers.role import InviteRoleSerializerUi from gregui.models import GregUserProfile class InviteGuestSerializer(serializers.ModelSerializer): + first_name = serializers.CharField(required=True) + last_name = serializers.CharField(required=True) email = serializers.EmailField(required=True) role = InviteRoleSerializerUi(required=True) uuid = serializers.UUIDField(read_only=True) @@ -29,6 +35,16 @@ class InviteGuestSerializer(serializers.ModelSerializer): return person + def validate_first_name(self, first_name): + if string_contains_illegal_chars(first_name): + raise serializers.ValidationError("First name contains illegal characters") + return first_name + + def validate_last_name(self, last_name): + if string_contains_illegal_chars(last_name): + raise serializers.ValidationError("Last name contains illegal characters") + return last_name + def validate_email(self, email): # The e-mail in the invite is the private e-mail if is_identity_duplicate(Identity.IdentityType.PRIVATE_EMAIL, email): diff --git a/gregui/tests/api/views/test_invite_guest.py b/gregui/tests/api/views/test_invite_guest.py index 51e9f545..06d28720 100644 --- a/gregui/tests/api/views/test_invite_guest.py +++ b/gregui/tests/api/views/test_invite_guest.py @@ -21,8 +21,8 @@ def test_invite_guest(client, user_sponsor, unit_foo, role_type_foo, mocker): role_end_date = datetime.datetime.today() + datetime.timedelta(days=10) data = { - "first_name": "foo木ðŸ‘Ø£", - "last_name": "غbaræ°´", + "first_name": "Å¿fooþ", + "last_name": "barÅ®", "email": "test@example.com", "role": { "start_date": (role_start_date).strftime("%Y-%m-%d"), @@ -47,8 +47,8 @@ def test_invite_guest(client, user_sponsor, unit_foo, role_type_foo, mocker): assert Person.objects.count() == 1 person = Person.objects.first() - assert person.first_name == "foo木ðŸ‘Ø£" - assert person.last_name == "غbaræ°´" + assert person.first_name == "Å¿fooþ" + assert person.last_name == "barÅ®" assert Identity.objects.filter( person=person, @@ -66,6 +66,39 @@ def test_invite_guest(client, user_sponsor, unit_foo, role_type_foo, mocker): send_invite_mock_function.assert_called() +@pytest.mark.django_db +def test_invite_guest__illegal_name(client, user_sponsor, unit_foo, role_type_foo): + test_comment = "This is a test comment" + contact_person_unit = "This is a test contact person" + role_start_date = datetime.datetime.today() + datetime.timedelta(days=1) + role_end_date = datetime.datetime.today() + datetime.timedelta(days=10) + + data = { + "first_name": "foo木ðŸ‘Ø£", + "last_name": "غbaræ°´", + "email": "test@example.com", + "role": { + "start_date": (role_start_date).strftime("%Y-%m-%d"), + "end_date": (role_end_date).strftime("%Y-%m-%d"), + "orgunit": unit_foo.id, + "type": role_type_foo.id, + "comments": test_comment, + "contact_person_unit": contact_person_unit, + }, + } + + assert len(Person.objects.all()) == 0 + + request = APIRequestFactory().post( + path=reverse("gregui-v1:invitation"), data=data, format="json" + ) + force_authenticate(request, user=user_sponsor) + + response = InvitationView.as_view()(request) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + + @pytest.mark.django_db def test_invite_cancel(client, invitation_link, invitation, role, log_in, user_sponsor): # TODO: Should all sponsors be allowed to delete arbitrary invitations? -- GitLab