diff --git a/authentication/api_gateway/serializers.py b/authentication/api_gateway/serializers.py index 6d3778a9b2..3732ffec67 100644 --- a/authentication/api_gateway/serializers.py +++ b/authentication/api_gateway/serializers.py @@ -1,6 +1,7 @@ """Authentication serializers""" import logging +import re from django.contrib.auth import get_user_model from django.db import transaction @@ -8,7 +9,10 @@ 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"] diff --git a/authentication/api_gateway/serializers_test.py b/authentication/api_gateway/serializers_test.py index 9b3c4fee69..79a0884caf 100644 --- a/authentication/api_gateway/serializers_test.py +++ b/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])