UserSerializer should validate username input and provide error to user

authentication/api_gateway/serializers.py

@@ -1,14 +1,18 @@
 """Authentication serializers"""
 
 import logging
+import re
 
 from django.contrib.auth import get_user_model
 from django.db import transaction
 from rest_framework import serializers
 
 from hubspot_sync.task_helpers import sync_hubspot_user
 from openedx.api import create_user
+from openedx.constants import OPENEDX_USERNAME_MAX_LEN
 from users.serializers import (
+    USERNAME_ERROR_MSG,
+    USERNAME_RE_PARTIAL,
     LegalAddressSerializer,
     UserProfileSerializer,
 )
@@ -25,6 +29,29 @@ class RegisterDetailsSerializer(serializers.Serializer):
     legal_address = LegalAddressSerializer(write_only=True)
     user_profile = UserProfileSerializer(write_only=True)
 
+    def validate_username(self, value):
+        """Validate username format and length"""
+        trimmed_value = value.strip()
+
+        # Check length constraints
+        if len(trimmed_value) > OPENEDX_USERNAME_MAX_LEN:
+            msg = (
+                f"Username must be no more than {OPENEDX_USERNAME_MAX_LEN} characters."
+            )
+            raise serializers.ValidationError(msg)
+
+        min_username_length = 3
+        if len(trimmed_value) < min_username_length:
+            msg = "Username must be at least 3 characters."
+            raise serializers.ValidationError(msg)
+
+        # Check character constraints using the same pattern as users.serializers
+        username_pattern = re.compile(rf"^{USERNAME_RE_PARTIAL}$")
+        if not username_pattern.match(trimmed_value):
+            raise serializers.ValidationError(USERNAME_ERROR_MSG)
+
+        return trimmed_value
+
     def create(self, validated_data):
         """Save user legal address and user profile"""
         request = self.context["request"]

authentication/api_gateway/serializers_test.py

@@ -6,6 +6,7 @@
     RegisterDetailsSerializer,
     RegisterExtraDetailsSerializer,
 )
+from openedx.constants import OPENEDX_USERNAME_MAX_LEN
 from openedx.models import OpenEdxUser
 from users.factories import UserFactory
 
@@ -44,14 +45,13 @@ def test_register_details_serializer_create(
 
 @pytest.mark.django_db
 @responses.activate
-def test_register_no_edx_user(  # noqa: PLR0913
-    mocker, settings, user, valid_address_dict, user_profile_dict, rf
-):
+def test_register_no_edx_user(mocker, settings, register_details_test_data):
     """Test the create method of RegisterDetailsSerializer"""
 
-    request = rf.post("/api/profile/details/")
-
     user = UserFactory.create(no_openedx_user=True, no_openedx_api_auth=True)
+
+    # Create a new request with the different user
+    request = register_details_test_data["request"]
     request.user = user
 
     responses.add(
@@ -63,12 +63,8 @@ def test_register_no_edx_user(  # noqa: PLR0913
 
     patched_create_edx_auth_token = mocker.patch("openedx.api.create_edx_auth_token")
 
-    data = {
-        "name": "John Doe",
-        "username": "johndoe",
-        "legal_address": valid_address_dict,
-        "user_profile": user_profile_dict,
-    }
+    data = register_details_test_data["base_data"].copy()
+    data["username"] = "johndoe"
     assert user.openedx_user is None
 
     serializer = RegisterDetailsSerializer(data=data, context={"request": request})
@@ -128,3 +124,100 @@ def test_register_extra_details_serializer_valid_data(user):
     assert validated_data["gender"] == "Male"
     assert validated_data["birth_year"] == "1990"
     assert validated_data["company"] == "TechCorp"
+
+
+@pytest.fixture
+def register_details_test_data(user, valid_address_dict, user_profile_dict, rf):
+    """Fixture providing test data for RegisterDetailsSerializer tests"""
+    request = rf.post("/api/profile/details/")
+    request.user = user
+
+    return {
+        "base_data": {
+            "name": "John Doe",
+            "legal_address": valid_address_dict,
+            "user_profile": user_profile_dict,
+        },
+        "request": request,
+    }
+
+
+@pytest.mark.django_db
+@pytest.mark.parametrize(
+    ("username", "expected_valid"),
+    [
+        ("validuser", True),  # Valid username
+        ("valid_user-123", True),  # Valid with allowed special chars
+        ("ab", False),  # Too short (less than 3 chars)
+        ("a" * (OPENEDX_USERNAME_MAX_LEN + 1), False),  # Too long (more than 30 chars)
+        ("user@domain", False),  # Invalid character (@)
+        ("user#name", False),  # Invalid character (#)
+        ("user$name", False),  # Invalid character ($)
+        (
+            "  validuser  ",
+            True,
+        ),  # Valid with leading/trailing spaces (should be trimmed)
+        ("user name", True),  # Valid with space
+        ("user.name", True),  # Valid with period
+        ("user+name", True),  # Valid with plus
+        ("user-name", True),  # Valid with hyphen
+        ("user_name", True),  # Valid with underscore
+    ],
+)
+def test_register_details_serializer_username_validation(
+    register_details_test_data, username, expected_valid
+):
+    """Test RegisterDetailsSerializer username validation"""
+    data = register_details_test_data["base_data"].copy()
+    data["username"] = username
+
+    serializer = RegisterDetailsSerializer(
+        data=data, context={"request": register_details_test_data["request"]}
+    )
+
+    if expected_valid:
+        assert serializer.is_valid(), (
+            f"Username '{username}' should be valid but got errors: {serializer.errors}"
+        )
+        # Check that spaces are trimmed for valid usernames with leading/trailing spaces
+        if username.strip() != username:
+            assert serializer.validated_data["username"] == username.strip()
+    else:
+        assert not serializer.is_valid(), (
+            f"Username '{username}' should be invalid but passed validation"
+        )
+        assert "username" in serializer.errors, (
+            f"Username error not found in {serializer.errors}"
+        )
+
+
+@pytest.mark.django_db
+def test_register_details_serializer_username_length_error_message(
+    user, valid_address_dict, user_profile_dict, rf
+):
+    """Test that username length error messages are descriptive"""
+    request = rf.post("/api/profile/details/")
+    request.user = user
+
+    # Test too long username
+    long_username = "a" * (OPENEDX_USERNAME_MAX_LEN + 1)
+    data = {
+        "name": "John Doe",
+        "username": long_username,
+        "legal_address": valid_address_dict,
+        "user_profile": user_profile_dict,
+    }
+
+    serializer = RegisterDetailsSerializer(data=data, context={"request": request})
+    assert not serializer.is_valid()
+    assert "username" in serializer.errors
+    assert f"must be no more than {OPENEDX_USERNAME_MAX_LEN} characters" in str(
+        serializer.errors["username"][0]
+    )
+
+    # Test too short username
+    data["username"] = "ab"
+    serializer = RegisterDetailsSerializer(data=data, context={"request": request})
+    assert not serializer.is_valid()
+    assert "username" in serializer.errors
+    assert "must be at least 3 characters" in str(serializer.errors["username"][0])