feat: persist private profile photos with account updates

papyrus/api/routes/users.py

@@ -1,21 +1,49 @@
 """User routes."""
 
 from typing import Annotated
+from uuid import UUID
 
-from fastapi import APIRouter, Depends
+from fastapi import APIRouter, Depends, File, Form, Request, Response, UploadFile
 from sqlalchemy.ext.asyncio import AsyncSession
 
 from papyrus.api.deps import CurrentUserId
+from papyrus.config import get_settings
 from papyrus.core.database import get_db
+from papyrus.core.rate_limit import get_authenticated_user_key, limiter
 from papyrus.schemas.auth import ChangePasswordRequest
 from papyrus.schemas.common import MessageResponse
 from papyrus.schemas.user import UpdateUserRequest, User, UserPreferences
+from papyrus.services import avatars
 from papyrus.services import users as user_service
 
 router = APIRouter()
 DBSession = Annotated[AsyncSession, Depends(get_db)]
 
 
+@router.post("/me/avatar", response_model=User, summary="Save profile with private photo")
+@limiter.limit(lambda: f"{get_settings().rate_limit_upload}/minute", key_func=get_authenticated_user_key)
+async def upload_avatar(
+    request: Request,
+    user_id: CurrentUserId,
+    db: DBSession,
+    file: Annotated[UploadFile, File()],
+    display_name: Annotated[str, Form(min_length=1, max_length=100)],
+) -> User:
+    user = await avatars.upload_avatar(db, user_id, file=file, display_name=display_name)
+    return await user_service.user_response(db, user)
+
+
+@router.get("/me/avatar/{avatar_id}", summary="Download current private profile photo")
+async def download_avatar(user_id: CurrentUserId, db: DBSession, avatar_id: UUID) -> Response:
+    contents, content_type = await avatars.download_avatar(db, user_id, avatar_id)
+
+    return Response(
+        content=contents,
+        media_type=content_type,
+        headers={"Cache-Control": "private, no-store", "X-Content-Type-Options": "nosniff"},
+    )
+
+
 @router.get(
     "/me",
     response_model=User,

papyrus/services/avatars.py

@@ -0,0 +1,116 @@
+"""Private account photos stored under the existing owner media directory."""
+
+from pathlib import Path
+from uuid import UUID, uuid4
+
+import structlog
+from fastapi import UploadFile
+from sqlalchemy import select
+from sqlalchemy.ext.asyncio import AsyncSession
+
+from papyrus.core.exceptions import ForbiddenError, NotFoundError, ValidationError
+from papyrus.models import User
+from papyrus.services.media import media_root
+
+MAX_AVATAR_BYTES = 5 * 1024 * 1024
+AVATAR_PREFIX = "/users/me/avatar/"
+logger = structlog.get_logger(__name__)
+
+
+async def lock_user(session: AsyncSession, user_id: UUID) -> User:
+    user = await session.scalar(
+        select(User).where(User.user_id == user_id).with_for_update().execution_options(populate_existing=True)
+    )
+
+    if user is None or user.disabled_at is not None:
+        raise ForbiddenError("User account is unavailable")
+
+    return user
+
+
+def avatar_path(user_id: UUID, avatar_url: str | None) -> Path | None:
+    if avatar_url is None or not avatar_url.startswith(AVATAR_PREFIX):
+        return None
+
+    try:
+        avatar_id = UUID(avatar_url.removeprefix(AVATAR_PREFIX))
+    except ValueError:
+        return None
+
+    return media_root() / str(user_id) / "avatars" / str(avatar_id)
+
+
+def _content_type(contents: bytes) -> str:
+    if contents.startswith(b"\xff\xd8\xff") and contents.endswith(b"\xff\xd9"):
+        return "image/jpeg"
+
+    if contents.startswith(b"\x89PNG\r\n\x1a\n") and contents.endswith(b"IEND\xaeB`\x82"):
+        return "image/png"
+
+    raise ValidationError("Choose a JPEG or PNG photo")
+
+
+def delete_previous_avatar(user_id: UUID, avatar_url: str | None) -> None:
+    path = avatar_path(user_id, avatar_url)
+
+    if path is not None:
+        try:
+            path.unlink(missing_ok=True)
+        except OSError:
+            logger.exception("Previous account photo cleanup failed", user_id=str(user_id))
+
+
+async def upload_avatar(
+    session: AsyncSession,
+    user_id: UUID,
+    *,
+    file: UploadFile,
+    display_name: str,
+) -> User:
+    """Commit the name and photo together, retaining the old photo on failure.
+
+    Photos have a separate fixed 5 MiB allowance and are not library media assets.
+    Owner-directory cleanup includes them when account deletion becomes permanent.
+    """
+    contents = await file.read(MAX_AVATAR_BYTES + 1)
+
+    if len(contents) > MAX_AVATAR_BYTES:
+        raise ValidationError("Photo must be 5 MB or smaller")
+
+    _content_type(contents)
+    user = await lock_user(session, user_id)
+    previous_url = user.avatar_url
+    avatar_id = uuid4()
+    avatar_url = f"{AVATAR_PREFIX}{avatar_id}"
+    path = media_root() / str(user_id) / "avatars" / str(avatar_id)
+    path.parent.mkdir(parents=True, exist_ok=True)
+
+    try:
+        path.write_bytes(contents)
+        user.avatar_url = avatar_url
+        user.display_name = display_name
+        await session.commit()
+    except Exception:
+        await session.rollback()
+        path.unlink(missing_ok=True)
+        raise
+
+    delete_previous_avatar(user_id, previous_url)
+    await session.refresh(user)
+    return user
+
+
+async def download_avatar(session: AsyncSession, user_id: UUID, avatar_id: UUID) -> tuple[bytes, str]:
+    user = await session.get(User, user_id)
+    avatar_url = f"{AVATAR_PREFIX}{avatar_id}"
+
+    if user is None or user.avatar_url != avatar_url:
+        raise NotFoundError("Photo was not found")
+
+    path = avatar_path(user_id, avatar_url)
+
+    if path is None or not path.is_file():
+        raise NotFoundError("Photo was not found")
+
+    contents = path.read_bytes()
+    return contents, _content_type(contents)

papyrus/services/users.py

@@ -13,6 +13,7 @@
 from papyrus.models import AuthSession, PasswordCredential, User, UserIdentity
 from papyrus.schemas.user import UpdateUserRequest
 from papyrus.schemas.user import User as UserResponse
+from papyrus.services import avatars
 
 
 def _now() -> datetime:
@@ -45,15 +46,25 @@ async def get_user_profile(session: AsyncSession, user_id: UUID) -> User:
 
 
 async def update_user_profile(session: AsyncSession, user_id: UUID, request: UpdateUserRequest) -> User:
-    user = await _get_active_user(session, user_id)
+    user = await avatars.lock_user(session, user_id)
+    previous_avatar = user.avatar_url
+
+    if request.avatar_url is not None:
+        avatar_url = str(request.avatar_url)
+
+        if avatar_url.startswith(avatars.AVATAR_PREFIX) and avatar_url != previous_avatar:
+            raise ValidationError("Upload a photo before using it in your profile")
+
+        user.avatar_url = avatar_url or None
 
     if request.display_name is not None:
         user.display_name = request.display_name
 
-    if request.avatar_url is not None:
-        user.avatar_url = str(request.avatar_url)
-
     await session.commit()
+
+    if user.avatar_url != previous_avatar:
+        avatars.delete_previous_avatar(user_id, previous_avatar)
+
     await session.refresh(user)
     return user
 

tests/api/routes/test_avatars.py

@@ -0,0 +1,150 @@
+import base64
+from datetime import UTC, datetime, timedelta
+from io import BytesIO
+from pathlib import Path
+from uuid import UUID, uuid4
+
+import pytest
+from fastapi import UploadFile
+from httpx import AsyncClient
+from sqlalchemy.ext.asyncio import AsyncSession
+
+from papyrus.models import User
+from papyrus.services import account_deletion, avatars
+
+PHOTO = base64.b64decode("iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+jP1kAAAAASUVORK5CYII=")
+
+
+@pytest.fixture(autouse=True)
+def photo_storage(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
+    monkeypatch.setattr("papyrus.main.settings.media_storage_root", str(tmp_path))
+
+
+async def upload(client: AsyncClient, headers: dict[str, str], contents: bytes = PHOTO):
+    return await client.post(
+        "/v1/users/me/avatar",
+        headers=headers,
+        data={"display_name": "Photo Reader"},
+        files={"file": ("photo.png", contents, "image/png")},
+    )
+
+
+async def test_photo_survives_login_and_replacement_removes_previous_file(
+    client: AsyncClient, auth_headers: dict[str, str], auth_user: dict[str, str], tmp_path: Path
+) -> None:
+    saved = await upload(client, auth_headers)
+    assert saved.status_code == 200
+    first_url = saved.json()["avatar_url"]
+    first_path = avatars.avatar_path(UUID(auth_user["user_id"]), first_url)
+    assert first_path is not None and first_path.read_bytes() == PHOTO
+    downloaded = await client.get(f"/v1{first_url}", headers=auth_headers)
+    assert downloaded.status_code == 200
+    assert downloaded.content == PHOTO
+    assert downloaded.headers["content-type"] == "image/png"
+    assert downloaded.headers["cache-control"] == "private, no-store"
+    login = await client.post(
+        "/v1/auth/login", json={"email": "user@example.com", "password": auth_user["password"], "client_type": "mobile"}
+    )
+
+    assert login.status_code == 200
+    assert login.json()["user"]["avatar_url"] == first_url
+    assert login.json()["user"]["display_name"] == "Photo Reader"
+    replaced = await upload(client, auth_headers)
+    assert replaced.status_code == 200
+    assert replaced.json()["avatar_url"] != first_url
+    assert not first_path.exists()
+    assert len(list(tmp_path.rglob("avatars/*"))) == 1
+    assert (await client.get(f"/v1{first_url}", headers=auth_headers)).status_code == 404
+
+
+async def test_photo_requires_authentication_and_owner(
+    client: AsyncClient, auth_headers: dict[str, str], auth_user: dict[str, str], db_session: AsyncSession
+) -> None:
+    saved = await upload(client, auth_headers)
+    url = saved.json()["avatar_url"]
+    assert (await client.get(f"/v1{url}")).status_code == 401
+    assert (await upload(client, {})).status_code == 401
+    other = await client.post(
+        "/v1/auth/register",
+        json={"email": "other@example.com", "password": "SecureP@ss123", "display_name": "Other"},
+    )
+
+    assert other.status_code == 201
+    other_headers = {"Authorization": f"Bearer {other.json()['access_token']}"}
+    assert (await client.get(f"/v1{url}", headers=other_headers)).status_code == 404
+    forged = await client.patch("/v1/users/me", headers=other_headers, json={"avatar_url": url})
+    assert forged.status_code == 400
+    user = await db_session.get(User, UUID(auth_user["user_id"]))
+    assert user is not None
+    user.disabled_at = datetime.now(UTC)
+    await db_session.commit()
+    assert (await client.get(f"/v1{url}", headers=auth_headers)).status_code == 403
+    assert (await upload(client, auth_headers)).status_code == 403
+
+
+@pytest.mark.parametrize(
+    "contents",
+    [b"", b"<svg></svg>", PHOTO[:-8], b"x" * (avatars.MAX_AVATAR_BYTES + 1)],
+    ids=["empty", "svg", "truncated", "oversize"],
+)
+async def test_invalid_upload_keeps_previous_profile(
+    client: AsyncClient, auth_headers: dict[str, str], tmp_path: Path, contents: bytes
+) -> None:
+    saved = await upload(client, auth_headers)
+    rejected = await upload(client, auth_headers, contents)
+    assert rejected.status_code == 400
+    current = await client.get("/v1/users/me", headers=auth_headers)
+    assert current.json() == saved.json()
+    assert len(list(tmp_path.rglob("avatars/*"))) == 1
+
+
+async def test_commit_failure_retains_photo_and_name(
+    client: AsyncClient,
+    auth_headers: dict[str, str],
+    auth_user: dict[str, str],
+    db_session: AsyncSession,
+    tmp_path: Path,
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    saved = await upload(client, auth_headers)
+
+    async def fail_commit() -> None:
+        raise RuntimeError("Database unavailable")
+
+    with monkeypatch.context() as patch:
+        patch.setattr(db_session, "commit", fail_commit)
+
+        with pytest.raises(RuntimeError, match="Database unavailable"):
+            await avatars.upload_avatar(
+                db_session,
+                UUID(auth_user["user_id"]),
+                file=UploadFile(filename="new.png", file=BytesIO(PHOTO)),
+                display_name="Unsaved name",
+            )
+
+    current = await client.get("/v1/users/me", headers=auth_headers)
+    assert current.json() == saved.json()
+    assert len(list(tmp_path.rglob("avatars/*"))) == 1
+
+
+async def test_permanent_account_cleanup_removes_photo_but_preserves_other_owners(
+    client: AsyncClient,
+    auth_headers: dict[str, str],
+    auth_user: dict[str, str],
+    db_session: AsyncSession,
+    tmp_path: Path,
+) -> None:
+    saved = await upload(client, auth_headers)
+    path = avatars.avatar_path(UUID(auth_user["user_id"]), saved.json()["avatar_url"])
+    other_path = tmp_path / str(uuid4()) / "avatars" / str(uuid4())
+    other_path.parent.mkdir(parents=True)
+    other_path.write_bytes(PHOTO)
+    user = await db_session.get(User, UUID(auth_user["user_id"]))
+    assert user is not None
+    user.disabled_at = datetime.now(UTC) - timedelta(days=31)
+    user.deletion_due_at = datetime.now(UTC) - timedelta(days=1)
+    await db_session.commit()
+    assert await account_deletion.purge_due_accounts(db_session) == 1
+    assert await account_deletion.cleanup_deleted_media(db_session) == 1
+    assert path is not None and not path.exists()
+    assert other_path.read_bytes() == PHOTO