Skip to content

Commit

Permalink
Merge pull request learningequality#11425 from jredrejo/allow_email_i…
Browse files Browse the repository at this point in the history
…n_usernames

Allow email in usernames
  • Loading branch information
jredrejo authored Oct 19, 2023
2 parents 3c012b2 + b24d587 commit beb3c8c
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 10 deletions.
28 changes: 28 additions & 0 deletions kolibri/core/auth/migrations/0024_extend_username_length.py
Original file line number Diff line number Diff line change
@@ -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",
),
),
]
34 changes: 25 additions & 9 deletions kolibri/core/auth/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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)
Expand Down
19 changes: 18 additions & 1 deletion kolibri/core/auth/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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 (
Expand Down
26 changes: 26 additions & 0 deletions kolibri/core/auth/test/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": "[email protected]",
"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
Expand Down
20 changes: 20 additions & 0 deletions kolibri/core/auth/test/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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="[email protected]",
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):
Expand Down

0 comments on commit beb3c8c

Please sign in to comment.