diff --git a/schemas/biocommons.py b/schemas/biocommons.py index 2128c83f..405ec3c0 100644 --- a/schemas/biocommons.py +++ b/schemas/biocommons.py @@ -9,7 +9,14 @@ from datetime import datetime, timezone from typing import Annotated, List, Literal, Optional, Self -from pydantic import BaseModel, EmailStr, Field, HttpUrl, StringConstraints +from pydantic import ( + AfterValidator, + BaseModel, + EmailStr, + Field, + HttpUrl, +) +from pydantic_core import PydanticCustomError import schemas from schemas import Resource, Service @@ -20,14 +27,56 @@ VALID_PASSWORD_REGEX = re.compile( f"^(?=.*?[A-Z])(?=.*?[a-z])(?=.*?[0-9])(?=.*?[{ALLOWED_SPECIAL_CHARS}]).{{8,}}$" ) +PASSWORD_FORMAT_MESSAGE = ( + "Password must contain at least one uppercase letter, one lowercase letter, one number, " + f"and one special character. Allowed special characters: {ALLOWED_SPECIAL_CHARS}" +) + + +def ValidatedString( + *, + min_length: int | None = None, + max_length: int | None = None, + pattern: str | re.Pattern[str] | None = None, + messages: dict[Literal["min_length", "max_length", "pattern"] , str] | None = None, +): + """ + Define a string type where we can customize the error messages to make + them more user-friendly – pydantic's + StringConstraints doesn't support this easily + """ + compiled = re.compile(pattern) if pattern else None + + def _check(v: str) -> str: + if not isinstance(v, str): + raise PydanticCustomError("string_type", "Value must be a string.") + + if min_length is not None and len(v) < min_length: + raise PydanticCustomError("string_too_short", messages["min_length"] or f"Must be at least {min_length} characters.") + + if max_length is not None and len(v) > max_length: + raise PydanticCustomError("string_too_long", messages["max_length"] or f"Must be at most {max_length} characters.") + + if compiled and not compiled.fullmatch(v): + raise PydanticCustomError("string_pattern_mismatch", messages["pattern"] or "Invalid format.") + + return v + + # Use only AfterValidator so OUR messages are the ones users see + return Annotated[str, AfterValidator(_check)] + AppId = Literal["biocommons", "galaxy", "bpa"] -BiocommonsUsername = Annotated[ - str, StringConstraints(min_length=3, max_length=128, pattern="^[-_a-z0-9]+$") -] -BiocommonsPassword = Annotated[ - str, StringConstraints(min_length=8, max_length=128, pattern=VALID_PASSWORD_REGEX) -] +BiocommonsUsername = ValidatedString(min_length=3, max_length=128, pattern="^[-_a-z0-9]+$", messages={ + "min_length": "Username must be at least 3 characters.", + "max_length": "Username must be 128 characters or less.", + "pattern": "Username must only contain lowercase letters, numbers, hyphens and underscores." +}) +BiocommonsPassword = ValidatedString(min_length=8, max_length=128, pattern=VALID_PASSWORD_REGEX, messages={ + "min_length": "Password must be at least 8 characters.", + "max_length": "Password must be 128 characters or less.", + "pattern": PASSWORD_FORMAT_MESSAGE +}) class BPAMetadata(BaseModel): diff --git a/tests/datagen.py b/tests/datagen.py index 7b99ee9c..81490c51 100644 --- a/tests/datagen.py +++ b/tests/datagen.py @@ -1,10 +1,12 @@ import random from string import ascii_letters, digits +from faker import Faker from polyfactory.decorators import post_generated from polyfactory.factories.pydantic_factory import ModelFactory from schemas.biocommons import ( + ALLOWED_SPECIAL_CHARS, Auth0UserData, BiocommonsAppMetadata, BiocommonsRegisterData, @@ -15,6 +17,27 @@ from schemas.tokens import AccessTokenPayload from schemas.user import SessionUser +fake = Faker() + + +class BiocommonsProviders: + @staticmethod + def biocommons_username() -> str: + # Must pass regex ^[-_a-z0-9]+$ and length 3–128 + return fake.slug() + + @staticmethod + def biocommons_password() -> str: + pw = fake.password( + length=20, + special_chars=True, + digits=True, + upper_case=True, + lower_case=True, + ) + pw = "".join(c for c in pw if c.isalnum() or c in ALLOWED_SPECIAL_CHARS) + return pw + def random_auth0_id() -> str: return "auth0|" + "".join(random.choices("0123456789abcdef", k=24)) @@ -38,8 +61,13 @@ class BiocommonsRegisterDataFactory(ModelFactory[BiocommonsRegisterData]): def connection(cls) -> str: return "Username-Password-Authentication" + password = BiocommonsProviders.biocommons_password + username = BiocommonsProviders.biocommons_username -class BiocommonsRegistrationRequestFactory(ModelFactory[BiocommonsRegistrationRequest]): ... + +class BiocommonsRegistrationRequestFactory(ModelFactory[BiocommonsRegistrationRequest]): + password = BiocommonsProviders.biocommons_password + username = BiocommonsProviders.biocommons_username class SessionUserFactory(ModelFactory[SessionUser]): ... @@ -50,6 +78,8 @@ class Auth0UserDataFactory(ModelFactory[Auth0UserData]): def user_id(cls) -> str: return random_auth0_id() + username = BiocommonsProviders.biocommons_username + class GalaxyRegistrationDataFactory(ModelFactory[GalaxyRegistrationData]): @post_generated @@ -60,6 +90,8 @@ def confirmPassword(cls, password: str) -> str: """ return password + password = BiocommonsProviders.biocommons_password + username = BiocommonsProviders.biocommons_username class BPARegistrationDataFactory(ModelFactory[BPARegistrationRequest]): """Factory for generating BPA registration test data.""" @@ -73,6 +105,9 @@ def get_default_organizations(cls) -> dict: "ausarg": True, } + password = BiocommonsProviders.biocommons_password + username = BiocommonsProviders.biocommons_username + class AppMetadataFactory(ModelFactory[BiocommonsAppMetadata]): ... @@ -83,3 +118,6 @@ class BiocommonsRegistrationDataFactory(ModelFactory[BiocommonsRegistrationReque @classmethod def bundle(cls) -> str: return "bpa-galaxy" + + password = BiocommonsProviders.biocommons_password + username = BiocommonsProviders.biocommons_username diff --git a/tests/schemas/test_biocommons_schemas.py b/tests/schemas/test_biocommons_schemas.py index fcce64d9..d19c0b28 100644 --- a/tests/schemas/test_biocommons_schemas.py +++ b/tests/schemas/test_biocommons_schemas.py @@ -1,7 +1,12 @@ import pytest from pydantic import TypeAdapter -from schemas.biocommons import BiocommonsPassword, BiocommonsUsername +from schemas.biocommons import ( + ALLOWED_SPECIAL_CHARS, + PASSWORD_FORMAT_MESSAGE, + BiocommonsPassword, + BiocommonsUsername, +) @pytest.mark.parametrize("password", [ @@ -9,6 +14,7 @@ "k$M2FZa@", "6*@sZ", "Jd9sugcfjgWXY@Dzje^83!mcfM@A$YZ8be^bUhrBZ8s$KjbbNwAHr*bdiEhmLyMPyPowFU@rX4k8h5KCh#qm9bYS5RUmtjaLmVds", + *[f"Password1{x}" for x in ALLOWED_SPECIAL_CHARS] ]) def test_valid_password(password: str): password_adapter = TypeAdapter(BiocommonsPassword) @@ -17,21 +23,33 @@ def test_valid_password(password: str): -@pytest.mark.parametrize("password", [ - # No lowercase - "ABCD1234!", - # No capital - "abcd1234!", +@pytest.mark.parametrize("password,expected_error", [ # Too short - "aB1!", - # No special character - "abCD1234" + ("aB1!", "Password must be at least 8 characters."), + ("Abc12!", "Password must be at least 8 characters."), + ("", "Password must be at least 8 characters."), + # Too long (more than 128 characters) + ("A" * 129 + "bc123!", "Password must be 128 characters or less."), + # Missing uppercase letter + ("abcd1234!", PASSWORD_FORMAT_MESSAGE), + # Missing lowercase letter + ("ABCD1234!", PASSWORD_FORMAT_MESSAGE), + # Missing number + ("AbcdEfgh!", PASSWORD_FORMAT_MESSAGE), + # Missing special character + ("abCD1234", PASSWORD_FORMAT_MESSAGE), + # Invalid special characters + ("Password123.", PASSWORD_FORMAT_MESSAGE), ]) -def test_invalid_password(password: str): +def test_invalid_password(password: str, expected_error: str): + """Test that invalid passwords raise appropriate validation errors.""" password_adapter = TypeAdapter(BiocommonsPassword) - with pytest.raises(ValueError): + with pytest.raises(ValueError) as exc_info: password_adapter.validate_python(password) + # Check that the error message contains our custom message + assert expected_error in str(exc_info.value) + @pytest.mark.parametrize("username", [ "abc", @@ -44,12 +62,24 @@ def test_valid_username(username: str): assert result == username -@pytest.mark.parametrize("username", [ - "ab", - "a.b", - "x" * 129 # Too long +@pytest.mark.parametrize("username,expected_error", [ + # Too short (less than 3 characters) + ("ab", "Username must be at least 3 characters."), + # Too long (more than 128 characters) + ("x" * 129, "Username must be 128 characters or less."), + # Invalid characters + ("a.b", "Username must only contain lowercase letters, numbers, hyphens and underscores."), + ("user name", "Username must only contain lowercase letters, numbers, hyphens and underscores."), + ("User123", "Username must only contain lowercase letters, numbers, hyphens and underscores."), + # Unicode characters + ("usér123", "Username must only contain lowercase letters, numbers, hyphens and underscores."), + ("user™", "Username must only contain lowercase letters, numbers, hyphens and underscores."), ]) -def test_invalid_username(username: str): +def test_invalid_username(username: str, expected_error: str): + """Test that invalid usernames raise appropriate validation errors.""" username_adapter = TypeAdapter(BiocommonsUsername) - with pytest.raises(ValueError): + with pytest.raises(ValueError) as exc_info: username_adapter.validate_python(username) + + # Check that the error message contains our custom message + assert expected_error in str(exc_info.value)