diff --git a/kolibri/core/auth/migrations/0024_extend_username_length.py b/kolibri/core/auth/migrations/0024_extend_username_length.py new file mode 100644 index 00000000000..86efcc629aa --- /dev/null +++ b/kolibri/core/auth/migrations/0024_extend_username_length.py @@ -0,0 +1,28 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.29 on 2023-10-17 17:51 +from __future__ import unicode_literals + +from django.db import migrations +from django.db import models + +import kolibri.core.auth.models + + +class Migration(migrations.Migration): + + dependencies = [ + ("kolibriauth", "0023_change_extra_fields_validator"), + ] + + operations = [ + migrations.AlterField( + model_name="facilityuser", + name="username", + field=models.CharField( + help_text="Required. 254 characters or fewer.", + max_length=254, + validators=[kolibri.core.auth.models.validate_username], + verbose_name="username", + ), + ), + ] diff --git a/kolibri/core/auth/models.py b/kolibri/core/auth/models.py index 442564bcd13..21554ae96eb 100644 --- a/kolibri/core/auth/models.py +++ b/kolibri/core/auth/models.py @@ -66,6 +66,7 @@ from .permissions.general import IsAdminForOwnFacility from .permissions.general import IsOwn from .permissions.general import IsSelf +from kolibri.core import error_constants from kolibri.core.auth.constants.demographics import choices as GENDER_CHOICES from kolibri.core.auth.constants.demographics import DEFERRED from kolibri.core.auth.constants.demographics import NOT_SPECIFIED @@ -344,6 +345,27 @@ def infer_dataset(self, *args, **kwargs): ) +validate_username_allowed_chars = validators.RegexValidator( + r'[\s`~!@#$%^&*()\-+={}\[\]\|\\\/:;"\'<>,\.\?]', + "Enter a valid username. This value can contain only letters, numbers, and underscores.", + code=error_constants.INVALID, + inverse_match=True, +) + +validate_username_max_length = validators.MaxLengthValidator( + 30, "Required. 30 characters or fewer. Letters and digits only" +) + + +def validate_username(value): + try: + validators.validate_email(value) + except ValidationError: + # for kolibri backwards compatibility, if the username is not an email: + validate_username_allowed_chars(value) + validate_username_max_length(value) + + class KolibriAbstractBaseUser(AbstractBaseUser): """ Our custom user type, derived from ``AbstractBaseUser`` as described in the Django docs. @@ -360,15 +382,9 @@ class Meta: username = models.CharField( "username", - max_length=30, - help_text="Required. 30 characters or fewer. Letters and digits only", - validators=[ - validators.RegexValidator( - r'[\s`~!@#$%^&*()\-+={}\[\]\|\\\/:;"\'<>,\.\?]', - "Enter a valid username. This value can contain only letters, numbers, and underscores.", - inverse_match=True, - ) - ], + max_length=254, + help_text="Required. 254 characters or fewer.", + validators=[validate_username], ) full_name = models.CharField("full name", max_length=120, blank=True) date_joined = DateTimeTzField("date joined", default=local_now, editable=False) diff --git a/kolibri/core/auth/serializers.py b/kolibri/core/auth/serializers.py index 4c9f8fd72bc..6bc8c9fbc26 100644 --- a/kolibri/core/auth/serializers.py +++ b/kolibri/core/auth/serializers.py @@ -2,6 +2,7 @@ from __future__ import print_function from __future__ import unicode_literals +from django.core.exceptions import ValidationError as DjangoValidationError from django.core.validators import MinLengthValidator from rest_framework import serializers from rest_framework.validators import UniqueTogetherValidator @@ -16,6 +17,8 @@ from .models import LearnerGroup from .models import Membership from .models import Role +from .models import validate_username_allowed_chars +from .models import validate_username_max_length from kolibri.core import error_constants from kolibri.core.auth.constants.demographics import NOT_SPECIFIED @@ -62,7 +65,21 @@ def save(self, **kwargs): return instance def validate(self, attrs): - username = attrs.get("username") + username = attrs.get("username", None) + if username is not None: + # in case a patch request does not provide username attribute + try: + validate_username_allowed_chars(username) + except DjangoValidationError as e: + raise serializers.ValidationError({"username": e.message}) + + try: + validate_username_max_length(username) + except DjangoValidationError as e: + raise serializers.ValidationError( + {"username": e.message}, code=error_constants.MAX_LENGTH + ) + # first condition is for creating object, second is for updating facility = attrs.get("facility") or getattr(self.instance, "facility") if ( diff --git a/kolibri/core/auth/test/test_api.py b/kolibri/core/auth/test/test_api.py index 27edca3e31e..3932ef058e8 100644 --- a/kolibri/core/auth/test/test_api.py +++ b/kolibri/core/auth/test/test_api.py @@ -751,6 +751,32 @@ def test_creating_user_same_username_case_insensitive(self): response.data[0]["id"], error_constants.USERNAME_ALREADY_EXISTS ) + def test_do_not_allow_emails_in_usernames(self): + data = { + "username": "bob@learningequality.org", + "password": DUMMY_PASSWORD, + "facility": self.facility.id, + } + response = self.client.post( + reverse("kolibri:core:facilityuser-list"), data, format="json" + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.data[0]["id"], error_constants.INVALID) + self.assertEqual(response.data[0]["metadata"]["field"], "username") + + def test_max_length_username_in_api(self): + data = { + "username": 32 * "gh", + "password": DUMMY_PASSWORD, + "facility": self.facility.id, + } + response = self.client.post( + reverse("kolibri:core:facilityuser-list"), data, format="json" + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.data[0]["id"], error_constants.MAX_LENGTH) + self.assertEqual(response.data[0]["metadata"]["field"], "username") + class UserUpdateTestCase(APITestCase): @classmethod diff --git a/kolibri/core/auth/test/test_models.py b/kolibri/core/auth/test/test_models.py index 5991f13032f..549a691b54d 100644 --- a/kolibri/core/auth/test/test_models.py +++ b/kolibri/core/auth/test/test_models.py @@ -680,6 +680,26 @@ def test_deserialize_empty_password(self): self.assertEqual("bob", user.username) self.assertEqual(NOT_SPECIFIED, user.password) + def test_username_validation(self): + self.facility = Facility.objects.create() + self.device_settings = DeviceSettings.objects.create() + user1 = FacilityUser.objects.create( + username="bob@learningequality.org", + password="password", + facility=self.facility, + ) + user1.full_clean() + user2 = FacilityUser.objects.create( + username="@bob", password="password", facility=self.facility + ) + with self.assertRaises(ValidationError): + user2.full_clean() + user3 = FacilityUser.objects.create( + username=32 * "gh", password="password", facility=self.facility + ) + with self.assertRaises(ValidationError): + user3.full_clean() + class CollectionHierarchyTestCase(TestCase): def test_facility_with_parent(self):