From 235fd051e2bf72283bcc6da4ecb2d1860b6ef332 Mon Sep 17 00:00:00 2001 From: Karolis Strazdas Date: Sat, 24 Oct 2026 04:49:32 +0300 Subject: [PATCH] feat: store private profile photos with account updates --- papyrus/api/routes/users.py | 30 ++++++- papyrus/services/avatars.py | 116 ++++++++++++++++++++++++ papyrus/services/users.py | 19 +++- tests/api/routes/test_avatars.py | 150 +++++++++++++++++++++++++++++++ 4 files changed, 310 insertions(+), 5 deletions(-) create mode 100644 papyrus/services/avatars.py create mode 100644 tests/api/routes/test_avatars.py diff --git a/papyrus/api/routes/users.py b/papyrus/api/routes/users.py index 7ca81a9..e2b7fa8 100644 --- a/papyrus/api/routes/users.py +++ b/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, diff --git a/papyrus/services/avatars.py b/papyrus/services/avatars.py new file mode 100644 index 0000000..6bdb047 --- /dev/null +++ b/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) diff --git a/papyrus/services/users.py b/papyrus/services/users.py index 0e4be63..bc91cda 100644 --- a/papyrus/services/users.py +++ b/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 diff --git a/tests/api/routes/test_avatars.py b/tests/api/routes/test_avatars.py new file mode 100644 index 0000000..bcafe0e --- /dev/null +++ b/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"", 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