diff --git a/backend/src/infrastructure/auth/oauth.py b/backend/src/infrastructure/auth/oauth.py index f13ae981..7c09fb6b 100644 --- a/backend/src/infrastructure/auth/oauth.py +++ b/backend/src/infrastructure/auth/oauth.py @@ -41,5 +41,6 @@ def _build_provider(name: str, client_id: str, client_secret: str): oauth_account_service = OAuthAccountService( repo=auth.repo, - new_user_fields=lambda ctx: {"name": ctx.suggested_name}, + # suggested_name is the provider's full name, unbounded; User.name is String(30). + new_user_fields=lambda ctx: {"name": ctx.suggested_name[:30]}, ) diff --git a/backend/src/modules/user/models.py b/backend/src/modules/user/models.py index 8c0d7eec..4b7a36c0 100644 --- a/backend/src/modules/user/models.py +++ b/backend/src/modules/user/models.py @@ -26,7 +26,10 @@ class User(Base, TimestampMixin, SoftDeleteMixin): ) name: Mapped[str] = mapped_column(String(30)) - username: Mapped[str] = mapped_column(String(20), unique=True, index=True) + # 32 = crudauth's OAuth username generator cap (USERNAME_MAX_LENGTH); a narrower + # column rejects OAuth signups whose sanitized username exceeds it (e.g. OIDC + # preferred_username values shaped like user@org.domain). + username: Mapped[str] = mapped_column(String(32), unique=True, index=True) email: Mapped[str] = mapped_column(String(50), unique=True, index=True) hashed_password: Mapped[str] = mapped_column(String(100)) diff --git a/backend/src/modules/user/schemas.py b/backend/src/modules/user/schemas.py index 4473cee6..2486000e 100644 --- a/backend/src/modules/user/schemas.py +++ b/backend/src/modules/user/schemas.py @@ -5,12 +5,20 @@ from ..common.schemas import PersistentDeletion, TimestampSchema +# Kept in one place because every user schema has to agree with the ``username`` +# column (``String(32)``) *and* with what crudauth's OAuth username generator +# emits: it caps at 32 and sanitizes to lowercase alphanumerics plus underscores. +# A narrower rule here does not reject the signup - crudauth writes the row +# directly - it makes the resulting user unreadable through ``UserRead``. +USERNAME_MAX_LENGTH = 32 +USERNAME_PATTERN = r"^[a-z0-9_]+$" + class UserBase(BaseModel): name: Annotated[str, Field(min_length=2, max_length=30, examples=["User Userson"])] username: Annotated[ str, - Field(min_length=2, max_length=20, pattern=r"^[a-z0-9]+$", examples=["userson"]), + Field(min_length=2, max_length=USERNAME_MAX_LENGTH, pattern=USERNAME_PATTERN, examples=["userson"]), ] email: Annotated[EmailStr, Field(examples=["user.userson@example.com"])] @@ -44,7 +52,7 @@ class UserRead(BaseModel): name: Annotated[str, Field(min_length=2, max_length=30, examples=["User Userson"])] username: Annotated[ str, - Field(min_length=2, max_length=20, pattern=r"^[a-z0-9]+$", examples=["userson"]), + Field(min_length=2, max_length=USERNAME_MAX_LENGTH, pattern=USERNAME_PATTERN, examples=["userson"]), ] email: Annotated[EmailStr, Field(examples=["user.userson@example.com"])] profile_image_url: str @@ -105,8 +113,8 @@ class UserUpdate(BaseModel): str | None, Field( min_length=2, - max_length=20, - pattern=r"^[a-z0-9]+$", + max_length=USERNAME_MAX_LENGTH, + pattern=USERNAME_PATTERN, examples=["userberg"], default=None, ), diff --git a/backend/tests/integration/auth/test_endpoints.py b/backend/tests/integration/auth/test_endpoints.py index f423047c..8a229386 100644 --- a/backend/tests/integration/auth/test_endpoints.py +++ b/backend/tests/integration/auth/test_endpoints.py @@ -325,6 +325,55 @@ async def test_oauth_callback_success_creates_user(client: AsyncClient): mock_storage.delete.assert_awaited_once() +@pytest.mark.asyncio +async def test_oauth_callback_handles_long_provider_usernames(client: AsyncClient): + """OAuth signup survives providers whose usernames sanitize to >20 chars. + + crudauth's provisioning caps generated usernames at 32 chars + (USERNAME_MAX_LENGTH), and OIDC ``preferred_username`` values shaped like + ``user@org.domain`` routinely exceed 20 after sanitizing, so the column + must fit the generator's cap. The provider's display name is unbounded + and gets truncated to ``User.name``'s width by ``new_user_fields``. + """ + valid_state = OAuthState( + state="long-name-state", + provider="google", + redirect_to="/", + code_verifier="test-code-verifier", + ) + mock_storage = MagicMock() + mock_storage.get = AsyncMock(return_value=valid_state) + mock_storage.delete = AsyncMock(return_value=None) + + mock_provider = MagicMock() + mock_provider.exchange_code = AsyncMock(return_value={"access_token": "tok"}) + mock_provider.get_user_info = AsyncMock(return_value={}) + mock_provider.process_user_info = AsyncMock( + return_value=OAuthUserInfo( + provider="google", + provider_user_id="google-uid-long", + email="long_username@example.com", + email_verified=True, + name="A" * 50, # unbounded provider display name + username="verylongusername@subdomain.example.com", # sanitizes to >20 chars + ) + ) + + with ( + patch(f"{ROUTES}.oauth_state_storage", mock_storage), + patch(f"{ROUTES}.oauth_providers", {"google": mock_provider}), + ): + response = await client.get( + "/api/v1/auth/oauth/callback/google", + params={"code": "test-code", "state": "long-name-state", "response_format": "json"}, + ) + + assert response.status_code == 200 + body = response.json() + assert body["success"] is True + assert 20 < len(body["user"]["username"]) <= 32 + + @pytest.mark.asyncio async def test_check_auth_user_not_found(client: AsyncClient): """A resolved principal whose user row is missing reports authenticated=false.""" diff --git a/docs/user-guide/authentication/user-management.md b/docs/user-guide/authentication/user-management.md index 6fb34400..b69c4b12 100644 --- a/docs/user-guide/authentication/user-management.md +++ b/docs/user-guide/authentication/user-management.md @@ -160,13 +160,15 @@ If the body changes `username` or `email`, the service also re-checks uniqueness The `UserUpdate` schema makes every field optional so clients can send partial updates: ```python +# USERNAME_MAX_LENGTH (32) and USERNAME_PATTERN are defined at the top of +# modules/user/schemas.py and shared by every schema carrying a username class UserUpdate(BaseModel): model_config = ConfigDict(extra="forbid") name: Annotated[str | None, Field(min_length=2, max_length=30, default=None)] username: Annotated[ str | None, - Field(min_length=2, max_length=20, pattern=r"^[a-z0-9]+$", default=None), + Field(min_length=2, max_length=USERNAME_MAX_LENGTH, pattern=USERNAME_PATTERN, default=None), ] email: Annotated[EmailStr | None, Field(default=None)] profile_image_url: Annotated[ diff --git a/docs/user-guide/database/index.md b/docs/user-guide/database/index.md index 649528a9..5a4eb98d 100644 --- a/docs/user-guide/database/index.md +++ b/docs/user-guide/database/index.md @@ -89,7 +89,7 @@ from pydantic import BaseModel, EmailStr, Field class UserCreate(BaseModel): name: str = Field(min_length=2, max_length=30) - username: str = Field(min_length=2, max_length=20) + username: str = Field(min_length=2, max_length=32) email: EmailStr password: str = Field(min_length=8) diff --git a/docs/user-guide/database/schemas.md b/docs/user-guide/database/schemas.md index ee1b32cb..bf9342a7 100644 --- a/docs/user-guide/database/schemas.md +++ b/docs/user-guide/database/schemas.md @@ -47,12 +47,18 @@ from pydantic import BaseModel, ConfigDict, EmailStr, Field from ..common.schemas import PersistentDeletion, TimestampSchema +# Declared once, reused by every schema that carries a username: the rule has to +# match the column (String(32)) and whatever OAuth provisioning generates +USERNAME_MAX_LENGTH = 32 +USERNAME_PATTERN = r"^[a-z0-9_]+$" + + # Common fields shared by create/update/full-record class UserBase(BaseModel): name: Annotated[str, Field(min_length=2, max_length=30, examples=["User Userson"])] username: Annotated[ str, - Field(min_length=2, max_length=20, pattern=r"^[a-z0-9]+$", examples=["userson"]), + Field(min_length=2, max_length=USERNAME_MAX_LENGTH, pattern=USERNAME_PATTERN, examples=["userson"]), ] email: Annotated[EmailStr, Field(examples=["user.userson@example.com"])] @@ -75,7 +81,7 @@ class User(TimestampSchema, UserBase, PersistentDeletion): class UserRead(BaseModel): id: int name: Annotated[str, Field(min_length=2, max_length=30)] - username: Annotated[str, Field(min_length=2, max_length=20, pattern=r"^[a-z0-9]+$")] + username: Annotated[str, Field(min_length=2, max_length=USERNAME_MAX_LENGTH, pattern=USERNAME_PATTERN)] email: EmailStr profile_image_url: str is_deleted: bool = False @@ -123,7 +129,7 @@ class UserUpdate(BaseModel): name: Annotated[str | None, Field(min_length=2, max_length=30, default=None)] username: Annotated[ str | None, - Field(min_length=2, max_length=20, pattern=r"^[a-z0-9]+$", default=None), + Field(min_length=2, max_length=USERNAME_MAX_LENGTH, pattern=USERNAME_PATTERN, default=None), ] email: Annotated[EmailStr | None, Field(default=None)] profile_image_url: Annotated[