From f2851bcb7a756221f8f746ea5783e4a940b0110f Mon Sep 17 00:00:00 2001 From: Felipe Cardoso Date: Tue, 4 Mar 2025 17:34:15 +0100 Subject: [PATCH] Refactor phone number validation and enhance test coverage Improved phone number validation logic with stricter rules and better error messages in `UserBase`. Updated access token expiration to 1 day in config. Added extensive tests for phone number validation, including valid and invalid cases across different formats. --- backend/app/core/config.py | 2 +- backend/app/schemas/users.py | 34 ++++- .../{app/auth => tests/schemas}/__init__.py | 0 backend/tests/schemas/test_user_schemas.py | 127 ++++++++++++++++++ 4 files changed, 157 insertions(+), 6 deletions(-) rename backend/{app/auth => tests/schemas}/__init__.py (100%) create mode 100644 backend/tests/schemas/test_user_schemas.py diff --git a/backend/app/core/config.py b/backend/app/core/config.py index 994a404..63f7d01 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -41,7 +41,7 @@ class Settings(BaseSettings): # JWT configuration SECRET_KEY: str = "your_secret_key_here" ALGORITHM: str = "HS256" - ACCESS_TOKEN_EXPIRE_MINUTES: int = 30 + ACCESS_TOKEN_EXPIRE_MINUTES: int = 1440 # 1 day # CORS configuration BACKEND_CORS_ORIGINS: List[str] = ["http://localhost:3000"] diff --git a/backend/app/schemas/users.py b/backend/app/schemas/users.py index ec7c1f1..f568976 100644 --- a/backend/app/schemas/users.py +++ b/backend/app/schemas/users.py @@ -48,14 +48,38 @@ class UserUpdate(BaseModel): preferences: Optional[Dict[str, Any]] = None @field_validator('phone_number') - @classmethod def validate_phone_number(cls, v: Optional[str]) -> Optional[str]: if v is None: return v - # Simple regex for phone validation - if not re.match(r'^\+?[0-9\s\-\(\)]{8,20}$', v): - raise ValueError('Invalid phone number format') - return v + + # Return early for empty strings or whitespace-only strings + if not v or v.strip() == "": + raise ValueError('Phone number cannot be empty') + + # Remove all spaces and formatting characters + cleaned = re.sub(r'[\s\-\(\)]', '', v) + + # Basic pattern: + # Must start with + or 0 + # After + must have at least 8 digits + # After 0 must have at least 8 digits + # Maximum total length of 15 digits (international standard) + # Only allowed characters are + at start and digits + pattern = r'^(?:\+[0-9]{8,14}|0[0-9]{8,14})$' + + if not re.match(pattern, cleaned): + raise ValueError('Phone number must start with + or 0 followed by 8-14 digits') + + # Additional validation to catch specific invalid cases + if cleaned.count('+') > 1: + raise ValueError('Phone number can only contain one + symbol at the start') + + # Check for any non-digit characters (except the leading +) + if not all(c.isdigit() for c in cleaned[1:]): + raise ValueError('Phone number can only contain digits after the prefix') + + return cleaned + class UserInDB(UserBase): diff --git a/backend/app/auth/__init__.py b/backend/tests/schemas/__init__.py similarity index 100% rename from backend/app/auth/__init__.py rename to backend/tests/schemas/__init__.py diff --git a/backend/tests/schemas/test_user_schemas.py b/backend/tests/schemas/test_user_schemas.py new file mode 100644 index 0000000..67bf7b5 --- /dev/null +++ b/backend/tests/schemas/test_user_schemas.py @@ -0,0 +1,127 @@ +# tests/schemas/test_user_schemas.py +import pytest +import re +from pydantic import ValidationError + +from app.schemas.users import UserBase, UserCreate + +class TestPhoneNumberValidation: + """Tests for phone number validation in user schemas""" + + def test_valid_swiss_numbers(self): + """Test valid Swiss phone numbers are accepted""" + # International format + user = UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number="+41791234567") + assert user.phone_number == "+41791234567" + + # Local format + user = UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number="0791234567") + assert user.phone_number == "0791234567" + + # With formatting characters + user = UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number="+41 79 123 45 67") + assert re.sub(r'[\s\-\(\)]', '', user.phone_number) == "+41791234567" + + user = UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number="079 123 45 67") + assert re.sub(r'[\s\-\(\)]', '', user.phone_number) == "0791234567" + + user = UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number="+41-79-123-45-67") + assert re.sub(r'[\s\-\(\)]', '', user.phone_number) == "+41791234567" + + user = UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number="079-123-45-67") + assert re.sub(r'[\s\-\(\)]', '', user.phone_number) == "0791234567" + + user = UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number="+41 (79) 123 45 67") + assert re.sub(r'[\s\-\(\)]', '', user.phone_number) == "+41791234567" + + user = UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number="079 (123) 45 67") + assert re.sub(r'[\s\-\(\)]', '', user.phone_number) == "0791234567" + + def test_valid_italian_numbers(self): + """Test valid Italian phone numbers are accepted""" + # International format + user = UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number="+393451234567") + assert user.phone_number == "+393451234567" + + user = UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number="+39345123456") + assert user.phone_number == "+39345123456" + + # Local format + user = UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number="03451234567") + assert user.phone_number == "03451234567" + + user = UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number="0345123456789") + assert user.phone_number == "0345123456789" + + # With formatting characters + user = UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number="+39 345 123 4567") + assert re.sub(r'[\s\-\(\)]', '', user.phone_number) == "+393451234567" + + user = UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number="0345 123 4567") + assert re.sub(r'[\s\-\(\)]', '', user.phone_number) == "03451234567" + + user = UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number="+39-345-123-4567") + assert re.sub(r'[\s\-\(\)]', '', user.phone_number) == "+393451234567" + + user = UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number="0345-123-4567") + assert re.sub(r'[\s\-\(\)]', '', user.phone_number) == "03451234567" + + user = UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number="+39 (345) 123 4567") + assert re.sub(r'[\s\-\(\)]', '', user.phone_number) == "+393451234567" + + user = UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number="0345 (123) 4567") + assert re.sub(r'[\s\-\(\)]', '', user.phone_number) == "03451234567" + + def test_none_phone_number(self): + """Test that None is accepted as a valid value (optional phone number)""" + user = UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number=None) + assert user.phone_number is None + + def test_invalid_phone_numbers(self): + """Test that invalid phone numbers are rejected""" + invalid_numbers = [ + # Too short + "+12", + "012", + + # Invalid characters + "+41xyz123456", + "079abc4567", + "123-abc-7890", + "+1(800)CALL-NOW", + + # Completely invalid formats + "++4412345678", # Double plus + "()+41123456", # Misplaced parentheses + + # Empty string + "", + # Spaces only + " ", + ] + + for number in invalid_numbers: + with pytest.raises(ValidationError): + UserBase(email="test@example.com", first_name="Test", last_name="User", phone_number=number) + + def test_phone_validation_in_user_create(self): + """Test that phone validation also works in UserCreate schema""" + # Valid phone number + user = UserCreate( + email="test@example.com", + first_name="Test", + last_name="User", + password="Password123", + phone_number="+41791234567" + ) + assert user.phone_number == "+41791234567" + + # Invalid phone number should raise ValidationError + with pytest.raises(ValidationError): + UserCreate( + email="test@example.com", + first_name="Test", + last_name="User", + password="Password123", + phone_number="invalid-number" + ) \ No newline at end of file