AAI-289 better validation/error messages for username and password

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):

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

tests/schemas/test_biocommons_schemas.py

@@ -1,14 +1,20 @@
 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", [
    "V6Zs^B8E",
    "k$M2FZa@",
    "6*@s&#5Z",
    "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)