From f2a59f798b1c1b8c8d81e001194774c9e9a7aac4 Mon Sep 17 00:00:00 2001 From: Keith Smith Date: Fri, 14 Aug 2026 12:21:42 -0600 Subject: [PATCH] Add image uploads in chat messages (Gitea issue #10) Images live on the app server's local disk (uploads/), served through an authenticated, room-membership-gated endpoint since rooms can be private. Uploads are streamed with a byte-count cap, validated as genuine decodable images with Pillow (not just a spoofed Content-Type), and downscaled to 2000px on the longer side (except GIF, to preserve animation). Backend: MessageImage model + nullable Message.content/image_id with a content-or-image CheckConstraint, upload/serve endpoints in rooms.py, WS message envelope gains image_id, push notification body says "sent an image" for image-only messages. Frontend: Composer gets an attach button with upload progress and a thumbnail chip; MessageList renders images inline with a click-to-zoom ImageLightbox. --- .gitignore | 3 + DEPLOYMENT.md | 14 ++ backend/README.md | 50 ++++- .../versions/c610bdb04567_message_images.py | 57 +++++ backend/app/models/__init__.py | 2 + backend/app/models/message.py | 15 +- backend/app/models/message_image.py | 27 +++ backend/app/routers/rooms.py | 70 ++++++- backend/app/schemas/message.py | 3 +- backend/app/schemas/message_image.py | 7 + backend/app/services/message_events.py | 12 +- backend/app/services/message_service.py | 8 +- backend/app/storage.py | 82 ++++++++ backend/app/ws/chat.py | 18 +- backend/pyproject.toml | 2 + backend/tests/test_images.py | 196 ++++++++++++++++++ deploy/backup-postgres.sh | 4 + frontend/src/api/rooms.ts | 38 +++- frontend/src/components/ChatPane.tsx | 4 +- frontend/src/components/Composer.css | 74 +++++++ frontend/src/components/Composer.tsx | 90 +++++++- frontend/src/components/ImageLightbox.css | 18 ++ frontend/src/components/ImageLightbox.tsx | 23 ++ frontend/src/components/MessageList.css | 11 + frontend/src/components/MessageList.tsx | 29 ++- frontend/src/types.ts | 6 +- frontend/src/ws/useChatSocket.ts | 6 +- 27 files changed, 829 insertions(+), 40 deletions(-) create mode 100644 backend/alembic/versions/c610bdb04567_message_images.py create mode 100644 backend/app/models/message_image.py create mode 100644 backend/app/schemas/message_image.py create mode 100644 backend/app/storage.py create mode 100644 backend/tests/test_images.py create mode 100644 frontend/src/components/ImageLightbox.css create mode 100644 frontend/src/components/ImageLightbox.tsx diff --git a/.gitignore b/.gitignore index bb757cc..1030699 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,6 @@ dist-ssr/ ## Test / coverage .pytest_cache/ .coverage + +## Uploaded content (local-disk image storage, see backend/app/routers/rooms.py) +/uploads/ diff --git a/DEPLOYMENT.md b/DEPLOYMENT.md index 7f16941..fe42e4c 100644 --- a/DEPLOYMENT.md +++ b/DEPLOYMENT.md @@ -107,6 +107,7 @@ sudo apt install -y python3 python3-venv nodejs npm git ```bash sudo useradd --system --shell /usr/sbin/nologin --home-dir /srv/chatapp --create-home chatapp sudo chown chatapp:chatapp /srv/chatapp +sudo -u chatapp mkdir -p /srv/chatapp/uploads ``` ### 3b. Clone the repo (deploy key, not a personal token) @@ -278,6 +279,13 @@ producing a gzipped `pg_dump` in `/var/backups/chatapp/` with 14-day local rotation. Off-box shipping is a placeholder in that script (commented-out rsync/S3 examples) — decide where those need to go and fill it in. +That script covers Postgres only. Uploaded chat images live on the **app** +server's disk (`/srv/chatapp/uploads`, created in §3a) — a separate machine +from the data server this script runs on — and currently have no backup +mechanism at all. Whatever off-box destination you pick above, include +`/srv/chatapp/uploads` in it too (e.g. a second `rsync` line run from the +app server). + **Test a restore** (against a scratch database, never directly onto `chatapp`): @@ -320,6 +328,12 @@ scope decisions" for the full detail on each): re-validated per delivery (DNS-rebinding gap). - Backup off-box shipping is a placeholder — decide a destination and fill in `deploy/backup-postgres.sh`. +- Uploaded chat images (`/srv/chatapp/uploads` on the app server) have no + backup coverage at all yet, on-box or off — see §7. +- Uploaded-but-never-sent images (a user attaches a file, then never hits + Send) leak an orphaned file on disk — no cleanup job for this yet. Not a + security issue (still gated by room membership to view), just an eventual + disk-space housekeeping item. None of these are new to this phase — deploying doesn't change any of them, just makes them reachable from the internet instead of localhost, which is diff --git a/backend/README.md b/backend/README.md index cdec50f..cd43c03 100644 --- a/backend/README.md +++ b/backend/README.md @@ -1,13 +1,13 @@ -# KeepItTalking backend (Phase 1 + 2 + 4 + 5 + 6 + 7 + 8) +# KeepItTalking backend (Phase 1 + 2 + 4 + 5 + 6 + 7 + 8, image uploads) FastAPI + SQLAlchemy 2.0 (async) + PostgreSQL + Redis. Implements auth, room CRUD (open and private), room roles (owner/admin/member) and invites, a WebSocket chat endpoint that fans out across multiple app-server instances via Redis pub/sub, Web Push notifications for offline room members, a -site-admin portal (user/room/bot management + an audit log), and a bot/ +site-admin portal (user/room/bot management + an audit log), a bot/ extension layer (scoped API tokens, live bot WebSocket access, incoming and -outgoing webhooks, message editing). See `../ARCHITECTURE.md` for the full -system design and the phased build plan. +outgoing webhooks, message editing), and image uploads in chat messages. See +`../ARCHITECTURE.md` for the full system design and the phased build plan. This is an **invite-only site**: there is no public registration endpoint. Accounts are created by an operator on the app server — see step 4 below. @@ -115,11 +115,13 @@ app/ require_room_member, require_room_role, require_site_admin, require_scope security.py argon2 password hashing + token generate/hash (sha256) + storage.py uploaded-image validation (Pillow), downscaling, + and on-disk save/read -- see Image uploads below cli.py `python -m app.cli create-user` / `generate-vapid-keys` models/ SQLAlchemy models (users, rooms, room_memberships, - messages, room_invites, push_subscriptions, - admin_audit_log, api_tokens, webhooks_incoming, - event_subscriptions) + messages, message_images, room_invites, + push_subscriptions, admin_audit_log, api_tokens, + webhooks_incoming, event_subscriptions) schemas/ Pydantic request/response models routers/ auth, rooms, invites, push, admin, bots, webhooks, health services/ business logic called by routers @@ -313,6 +315,40 @@ calls `POST /api/invites/{id}/accept` (or `/decline`). `GET /api/rooms/mine` lists every room (open + private) the current user belongs to, alongside their role. +## Image uploads + +A message can carry an image (`Message.image_id`, nullable), a caption +(`Message.content`, nullable), or both — a `CheckConstraint` requires at +least one. Images live on the app server's local disk (`/uploads`, +resolved the same way `app/main.py` locates `frontend/dist` — see +`app/storage.py`), not S3, matching this project's plain-two-servers +deployment; see `../DEPLOYMENT.md` for the production directory and its +(currently missing) backup coverage. + +- `POST /api/rooms/{room_id}/images` (room-member gated, multipart) — + `app/storage.read_capped` rejects (413) as soon as the streamed byte count + passes 8 MB, before buffering the whole body. `app/storage.process_image` + then opens the result with Pillow to confirm it's a genuinely decodable + image (not just a spoofed `Content-Type` header — 400 if not) and + downscales it so its longer side is ≤2000px, except GIF, left untouched so + animation isn't collapsed to a single frame. Returns the new + `message_images` row's id; the frontend attaches it to a message + afterward, it isn't a message by itself. +- `GET /api/rooms/{room_id}/images/{image_id}` (room-member gated) — 404s if + the image doesn't belong to that room, otherwise streams it with + `Cache-Control: private, max-age=31536000, immutable` (content-addressed + by a generated UUID filename, so once served it never changes). +- The WS `"message"` handler (`app/ws/chat.py`) accepts an optional + `image_id`, validated against the room before attaching. Push notification + bodies (`app/services/message_events.py`) say "`{username} sent an image`" + for an image-only message instead of a body that's just `"username: "`. + +Known gap: an uploaded-but-never-sent image (a user attaches a file, then +navigates away before hitting Send) leaks an orphaned file on disk — no +cleanup job for this yet. Not a security issue, since serving still goes +through the same room-membership gate as everything else; just an eventual +disk-space housekeeping item. + ## Notes / scope decisions - Invite-only site registration: no `POST /api/auth/register`. Accounts are diff --git a/backend/alembic/versions/c610bdb04567_message_images.py b/backend/alembic/versions/c610bdb04567_message_images.py new file mode 100644 index 0000000..6ab81cd --- /dev/null +++ b/backend/alembic/versions/c610bdb04567_message_images.py @@ -0,0 +1,57 @@ +"""message images + +Revision ID: c610bdb04567 +Revises: 3350c67553ad +Create Date: 2026-08-14 12:00:58.698701 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = 'c610bdb04567' +down_revision: Union[str, Sequence[str], None] = '3350c67553ad' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + """Upgrade schema.""" + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('message_images', + sa.Column('id', sa.Uuid(), nullable=False), + sa.Column('room_id', sa.Uuid(), nullable=False), + sa.Column('uploaded_by', sa.Uuid(), nullable=False), + sa.Column('storage_filename', sa.String(length=64), nullable=False), + sa.Column('content_type', sa.String(length=50), nullable=False), + sa.Column('size_bytes', sa.BigInteger(), nullable=False), + sa.Column('created_at', sa.DateTime(timezone=True), server_default=sa.text('now()'), nullable=False), + sa.ForeignKeyConstraint(['room_id'], ['rooms.id'], ), + sa.ForeignKeyConstraint(['uploaded_by'], ['users.id'], ), + sa.PrimaryKeyConstraint('id') + ) + op.create_index(op.f('ix_message_images_room_id'), 'message_images', ['room_id'], unique=False) + op.add_column('messages', sa.Column('image_id', sa.Uuid(), nullable=True)) + op.alter_column('messages', 'content', + existing_type=sa.TEXT(), + nullable=True) + op.create_foreign_key('messages_image_id_fkey', 'messages', 'message_images', ['image_id'], ['id']) + op.create_check_constraint('messages_content_or_image_required', 'messages', 'content IS NOT NULL OR image_id IS NOT NULL') + # ### end Alembic commands ### + + +def downgrade() -> None: + """Downgrade schema.""" + # ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint('messages_content_or_image_required', 'messages', type_='check') + op.drop_constraint('messages_image_id_fkey', 'messages', type_='foreignkey') + op.alter_column('messages', 'content', + existing_type=sa.TEXT(), + nullable=False) + op.drop_column('messages', 'image_id') + op.drop_index(op.f('ix_message_images_room_id'), table_name='message_images') + op.drop_table('message_images') + # ### end Alembic commands ### diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index 49fc152..5279f36 100644 --- a/backend/app/models/__init__.py +++ b/backend/app/models/__init__.py @@ -5,6 +5,7 @@ from app.models.event_subscription import EventSubscription from app.models.invite import InviteStatus, RoomInvite from app.models.membership import RoomMembership, RoomRole from app.models.message import Message +from app.models.message_image import MessageImage from app.models.push_subscription import PushSubscription from app.models.room import Room from app.models.user import User @@ -17,6 +18,7 @@ __all__ = [ "RoomMembership", "RoomRole", "Message", + "MessageImage", "RoomInvite", "InviteStatus", "PushSubscription", diff --git a/backend/app/models/message.py b/backend/app/models/message.py index 6e58a35..fad67ba 100644 --- a/backend/app/models/message.py +++ b/backend/app/models/message.py @@ -1,7 +1,7 @@ import uuid from datetime import datetime -from sqlalchemy import DateTime, ForeignKey, Text, func +from sqlalchemy import CheckConstraint, DateTime, ForeignKey, Text, func from sqlalchemy.orm import Mapped, mapped_column, relationship from app.models.base import Base @@ -9,11 +9,21 @@ from app.models.base import Base class Message(Base): __tablename__ = "messages" + __table_args__ = ( + CheckConstraint( + "content IS NOT NULL OR image_id IS NOT NULL", + name="messages_content_or_image_required", + ), + ) id: Mapped[uuid.UUID] = mapped_column(primary_key=True, default=uuid.uuid4) room_id: Mapped[uuid.UUID] = mapped_column(ForeignKey("rooms.id"), index=True, nullable=False) user_id: Mapped[uuid.UUID] = mapped_column(ForeignKey("users.id"), nullable=False) - content: Mapped[str] = mapped_column(Text, nullable=False) + # Nullable since Phase "image uploads": a message can be an image with + # no caption. The CheckConstraint above still requires at least one of + # content/image_id. + content: Mapped[str | None] = mapped_column(Text) + image_id: Mapped[uuid.UUID | None] = mapped_column(ForeignKey("message_images.id")) created_at: Mapped[datetime] = mapped_column( DateTime(timezone=True), server_default=func.now(), index=True, nullable=False ) @@ -21,3 +31,4 @@ class Message(Base): deleted_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) user = relationship("User") + image = relationship("MessageImage") diff --git a/backend/app/models/message_image.py b/backend/app/models/message_image.py new file mode 100644 index 0000000..2a73e9c --- /dev/null +++ b/backend/app/models/message_image.py @@ -0,0 +1,27 @@ +import uuid +from datetime import datetime + +from sqlalchemy import BigInteger, DateTime, ForeignKey, String, func +from sqlalchemy.orm import Mapped, mapped_column, relationship + +from app.models.base import Base + + +class MessageImage(Base): + __tablename__ = "message_images" + + id: Mapped[uuid.UUID] = mapped_column(primary_key=True, default=uuid.uuid4) + room_id: Mapped[uuid.UUID] = mapped_column(ForeignKey("rooms.id"), index=True, nullable=False) + uploaded_by: Mapped[uuid.UUID] = mapped_column(ForeignKey("users.id"), nullable=False) + # The on-disk filename -- a generated UUID + real extension, never the + # client-supplied original filename (avoids path-traversal/collision + # concerns from trusting client input for a filesystem path). + storage_filename: Mapped[str] = mapped_column(String(64), nullable=False) + content_type: Mapped[str] = mapped_column(String(50), nullable=False) + size_bytes: Mapped[int] = mapped_column(BigInteger, nullable=False) + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), server_default=func.now(), nullable=False + ) + + room = relationship("Room") + uploader = relationship("User") diff --git a/backend/app/routers/rooms.py b/backend/app/routers/rooms.py index dd7ba01..1451df8 100644 --- a/backend/app/routers/rooms.py +++ b/backend/app/routers/rooms.py @@ -1,6 +1,7 @@ import uuid -from fastapi import APIRouter, Depends, HTTPException, Query, Request +from fastapi import APIRouter, Depends, File, HTTPException, Query, Request, UploadFile +from fastapi.responses import FileResponse from sqlalchemy.ext.asyncio import AsyncSession from app.database import get_db @@ -10,9 +11,10 @@ from app.dependencies import ( require_room_role, require_scope, ) -from app.models import RoomRole, User +from app.models import MessageImage, RoomRole, User from app.schemas.invite import InviteCreate, InviteRead from app.schemas.message import MessageRead +from app.schemas.message_image import MessageImageCreated from app.schemas.room import ( MyRoomItem, RoomCreate, @@ -74,6 +76,15 @@ from app.services.webhook_service import ( revoke_incoming_webhook, ) from app.services.ssrf import UnsafeWebhookUrlError +from app.storage import ( + ALLOWED_IMAGE_CONTENT_TYPES, + UPLOADS_DIR, + ImageTooLargeError, + InvalidImageError, + process_image, + read_capped, + save_image, +) router = APIRouter(prefix="/api/rooms", tags=["rooms"]) @@ -291,6 +302,7 @@ async def get_room_messages_endpoint( user_id=m.user_id, username=m.user.username, content=m.content, + image_id=m.image_id, created_at=m.created_at, edited_at=m.edited_at, ) @@ -298,6 +310,60 @@ async def get_room_messages_endpoint( ] +@router.post("/{room_id}/images", response_model=MessageImageCreated, status_code=201) +async def upload_room_image_endpoint( + room_id: uuid.UUID, + file: UploadFile = File(...), + current_user: User = Depends(get_current_user), + db: AsyncSession = Depends(get_db), +): + await require_room_member(room_id, current_user, db) + + if file.content_type not in ALLOWED_IMAGE_CONTENT_TYPES: + raise HTTPException(status_code=400, detail="Unsupported image type") + + try: + data = await read_capped(file) + except ImageTooLargeError: + raise HTTPException(status_code=413, detail="Image exceeds 8 MB limit") + + try: + data, ext = process_image(data, file.content_type) + except InvalidImageError: + raise HTTPException(status_code=400, detail="File is not a valid image") + + storage_filename = save_image(data, ext) + image = MessageImage( + room_id=room_id, + uploaded_by=current_user.id, + storage_filename=storage_filename, + content_type=file.content_type, + size_bytes=len(data), + ) + db.add(image) + await db.commit() + await db.refresh(image) + return MessageImageCreated(id=image.id) + + +@router.get("/{room_id}/images/{image_id}") +async def get_room_image_endpoint( + room_id: uuid.UUID, + image_id: uuid.UUID, + current_user: User = Depends(get_current_user), + db: AsyncSession = Depends(get_db), +): + await require_room_member(room_id, current_user, db) + image = await db.get(MessageImage, image_id) + if image is None or image.room_id != room_id: + raise HTTPException(status_code=404, detail="Image not found") + return FileResponse( + UPLOADS_DIR / image.storage_filename, + media_type=image.content_type, + headers={"Cache-Control": "private, max-age=31536000, immutable"}, + ) + + def _to_invite_read(invite) -> InviteRead: return InviteRead( id=invite.id, diff --git a/backend/app/schemas/message.py b/backend/app/schemas/message.py index 7358ead..e9d8b8f 100644 --- a/backend/app/schemas/message.py +++ b/backend/app/schemas/message.py @@ -11,6 +11,7 @@ class MessageRead(BaseModel): room_id: uuid.UUID user_id: uuid.UUID username: str - content: str + content: str | None + image_id: uuid.UUID | None created_at: datetime edited_at: datetime | None diff --git a/backend/app/schemas/message_image.py b/backend/app/schemas/message_image.py new file mode 100644 index 0000000..2125606 --- /dev/null +++ b/backend/app/schemas/message_image.py @@ -0,0 +1,7 @@ +import uuid + +from pydantic import BaseModel + + +class MessageImageCreated(BaseModel): + id: uuid.UUID diff --git a/backend/app/services/message_events.py b/backend/app/services/message_events.py index 425e364..890fe4a 100644 --- a/backend/app/services/message_events.py +++ b/backend/app/services/message_events.py @@ -11,7 +11,7 @@ from app.ws.presence import Presence async def _notify_offline_members( - db: AsyncSession, presence: Presence, room_id: uuid.UUID, sender: User, content: str + db: AsyncSession, presence: Presence, room_id: uuid.UUID, sender: User, message: Message ) -> None: result = await db.execute( select(RoomMembership.user_id).where(RoomMembership.room_id == room_id) @@ -26,9 +26,14 @@ async def _notify_offline_members( return room = await db.get(Room, room_id) + body = ( + f"{sender.username}: {message.content}"[:120] + if message.content + else f"{sender.username} sent an image" + ) payload = { "title": f"#{room.name}" if room else "New message", - "body": f"{sender.username}: {content}"[:120], + "body": body, "room_id": str(room_id), } for user_id in offline_ids: @@ -43,6 +48,7 @@ def _message_payload(message: Message, username: str) -> dict: "user_id": str(message.user_id), "username": username, "content": message.content, + "image_id": str(message.image_id) if message.image_id else None, "created_at": message.created_at.isoformat(), "edited_at": message.edited_at.isoformat() if message.edited_at else None, } @@ -61,7 +67,7 @@ async def broadcast_new_message( trigger identical fan-out/push/event behavior.""" payload = _message_payload(message, sender.username) await broadcaster.publish(room_id, payload) - await _notify_offline_members(db, presence, room_id, sender, message.content) + await _notify_offline_members(db, presence, room_id, sender, message) await dispatch_event(db, "message.created", room_id, payload) diff --git a/backend/app/services/message_service.py b/backend/app/services/message_service.py index f033594..e7bd635 100644 --- a/backend/app/services/message_service.py +++ b/backend/app/services/message_service.py @@ -17,9 +17,13 @@ class NotMessageAuthorError(Exception): async def create_message( - db: AsyncSession, room_id: uuid.UUID, user_id: uuid.UUID, content: str + db: AsyncSession, + room_id: uuid.UUID, + user_id: uuid.UUID, + content: str | None = None, + image_id: uuid.UUID | None = None, ) -> Message: - message = Message(room_id=room_id, user_id=user_id, content=content) + message = Message(room_id=room_id, user_id=user_id, content=content, image_id=image_id) db.add(message) await db.commit() await db.refresh(message) diff --git a/backend/app/storage.py b/backend/app/storage.py new file mode 100644 index 0000000..74dc78a --- /dev/null +++ b/backend/app/storage.py @@ -0,0 +1,82 @@ +import io +import pathlib +import uuid + +from PIL import Image, UnidentifiedImageError + +# backend/app/storage.py -> backend/ -> repo root -- same +# resolve-relative-to-file convention FRONTEND_DIST uses in app/main.py, so +# this lands in the right place in both local dev and the /srv/chatapp +# production layout with zero new config. +UPLOADS_DIR = pathlib.Path(__file__).resolve().parent.parent.parent / "uploads" + +MAX_IMAGE_BYTES = 8 * 1024 * 1024 +_READ_CHUNK_BYTES = 1024 * 1024 +_MAX_DIMENSION = 2000 + +# (storage extension, Pillow format name) +ALLOWED_IMAGE_CONTENT_TYPES: dict[str, tuple[str, str]] = { + "image/jpeg": (".jpg", "JPEG"), + "image/png": (".png", "PNG"), + "image/gif": (".gif", "GIF"), + "image/webp": (".webp", "WEBP"), +} + + +class ImageTooLargeError(Exception): + pass + + +class InvalidImageError(Exception): + pass + + +async def read_capped(file, cap: int = MAX_IMAGE_BYTES) -> bytes: + """Reads an UploadFile-like object in chunks, raising as soon as `cap` + is exceeded rather than after buffering the whole (potentially huge) + body first.""" + chunks = [] + total = 0 + while True: + chunk = await file.read(_READ_CHUNK_BYTES) + if not chunk: + break + total += len(chunk) + if total > cap: + raise ImageTooLargeError() + chunks.append(chunk) + return b"".join(chunks) + + +def process_image(data: bytes, content_type: str) -> tuple[bytes, str]: + """Confirms `data` is a genuinely decodable image (not just a spoofed + Content-Type header) and downscales it so its longer side is + <=2000px -- except GIF, left untouched so animation isn't collapsed to + a single frame. Returns (final_bytes, storage_extension).""" + ext, pillow_format = ALLOWED_IMAGE_CONTENT_TYPES[content_type] + + try: + with Image.open(io.BytesIO(data)) as probe: + probe.verify() + except (UnidentifiedImageError, OSError, ValueError) as exc: + raise InvalidImageError() from exc + + if content_type == "image/gif": + return data, ext + + # verify() leaves the image unusable for further processing, so reopen. + image = Image.open(io.BytesIO(data)) + image.load() + if pillow_format == "JPEG" and image.mode in ("RGBA", "P"): + image = image.convert("RGB") + image.thumbnail((_MAX_DIMENSION, _MAX_DIMENSION)) + out = io.BytesIO() + image.save(out, format=pillow_format) + return out.getvalue(), ext + + +def save_image(data: bytes, ext: str) -> str: + UPLOADS_DIR.mkdir(parents=True, exist_ok=True) + storage_filename = f"{uuid.uuid4()}{ext}" + (UPLOADS_DIR / storage_filename).write_bytes(data) + return storage_filename diff --git a/backend/app/ws/chat.py b/backend/app/ws/chat.py index e4414fd..95c2420 100644 --- a/backend/app/ws/chat.py +++ b/backend/app/ws/chat.py @@ -6,7 +6,7 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession from app.database import get_db -from app.models import ApiToken, RoomMembership, User +from app.models import ApiToken, MessageImage, RoomMembership, User from app.services.bot_service import resolve_token from app.services.message_events import broadcast_message_update, broadcast_new_message from app.services.message_service import ( @@ -25,6 +25,7 @@ class ClientEnvelope(BaseModel): type: str room_id: uuid.UUID | None = None content: str | None = None + image_id: uuid.UUID | None = None message_id: uuid.UUID | None = None @@ -103,9 +104,9 @@ async def chat_endpoint(websocket: WebSocket, db: AsyncSession = Depends(get_db) joined_rooms.discard(envelope.room_id) elif envelope.type == "message": - if envelope.room_id is None or not envelope.content: + if envelope.room_id is None or (not envelope.content and envelope.image_id is None): await websocket.send_json( - {"type": "error", "detail": "room_id and content required"} + {"type": "error", "detail": "room_id and content or image_id required"} ) continue if _missing_scope(api_token, "write:messages"): @@ -120,7 +121,16 @@ async def chat_endpoint(websocket: WebSocket, db: AsyncSession = Depends(get_db) {"type": "error", "detail": "Not a member of this room"} ) continue - message = await create_message(db, envelope.room_id, user.id, envelope.content) + image_id = None + if envelope.image_id is not None: + image = await db.get(MessageImage, envelope.image_id) + if image is None or image.room_id != envelope.room_id: + await websocket.send_json({"type": "error", "detail": "Invalid image"}) + continue + image_id = image.id + message = await create_message( + db, envelope.room_id, user.id, envelope.content, image_id + ) await broadcast_new_message(db, broadcaster, presence, envelope.room_id, message, user) elif envelope.type == "edit": diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 37e5a74..77ee739 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -18,6 +18,8 @@ dependencies = [ "redis>=5.0", "httpx>=0.27", "gunicorn>=23.0", + "Pillow>=10.0", + "python-multipart>=0.0.9", ] [project.scripts] diff --git a/backend/tests/test_images.py b/backend/tests/test_images.py new file mode 100644 index 0000000..c85ef8e --- /dev/null +++ b/backend/tests/test_images.py @@ -0,0 +1,196 @@ +import io +import uuid + +from PIL import Image + +from tests.conftest import register_and_login + + +def _unique(prefix: str) -> str: + return f"{prefix}-{uuid.uuid4().hex[:8]}" + + +def _png_bytes(size: tuple[int, int] = (10, 10)) -> bytes: + buf = io.BytesIO() + Image.new("RGB", size, color=(255, 0, 0)).save(buf, format="PNG") + return buf.getvalue() + + +async def test_upload_image_succeeds(client, db_session): + await register_and_login(client, db_session, username=_unique("alice")) + room = (await client.post("/api/rooms", json={"name": _unique("general")})).json() + + resp = await client.post( + f"/api/rooms/{room['id']}/images", + files={"file": ("test.png", _png_bytes(), "image/png")}, + ) + assert resp.status_code == 201, resp.text + assert "id" in resp.json() + + +async def test_upload_oversized_image_rejected(client, db_session): + await register_and_login(client, db_session, username=_unique("alice")) + room = (await client.post("/api/rooms", json={"name": _unique("general")})).json() + + # 8MB cap -- a bit over, so it's rejected promptly by the streamed byte + # count in app/storage.read_capped without ever reaching Pillow. + oversized = b"0" * (9 * 1024 * 1024) + resp = await client.post( + f"/api/rooms/{room['id']}/images", + files={"file": ("huge.png", oversized, "image/png")}, + ) + assert resp.status_code == 413 + + +async def test_upload_non_image_rejected(client, db_session): + await register_and_login(client, db_session, username=_unique("alice")) + room = (await client.post("/api/rooms", json={"name": _unique("general")})).json() + + # Spoofed Content-Type: claims image/png but isn't decodable as one -- + # must be caught by Pillow, not just the header check. + resp = await client.post( + f"/api/rooms/{room['id']}/images", + files={"file": ("fake.png", b"not an image", "image/png")}, + ) + assert resp.status_code == 400 + + +async def test_upload_unsupported_content_type_rejected(client, db_session): + await register_and_login(client, db_session, username=_unique("alice")) + room = (await client.post("/api/rooms", json={"name": _unique("general")})).json() + + resp = await client.post( + f"/api/rooms/{room['id']}/images", + files={"file": ("doc.pdf", b"%PDF-1.4", "application/pdf")}, + ) + assert resp.status_code == 400 + + +async def test_serve_image_requires_room_membership(client, db_session): + await register_and_login(client, db_session, username=_unique("alice")) + room = (await client.post("/api/rooms", json={"name": _unique("general")})).json() + upload = await client.post( + f"/api/rooms/{room['id']}/images", + files={"file": ("test.png", _png_bytes(), "image/png")}, + ) + image_id = upload.json()["id"] + + # Sanity: the uploader themselves can fetch it. + ok = await client.get(f"/api/rooms/{room['id']}/images/{image_id}") + assert ok.status_code == 200 + + await register_and_login(client, db_session, username=_unique("bob")) + resp = await client.get(f"/api/rooms/{room['id']}/images/{image_id}") + assert resp.status_code == 403 + + +async def test_serve_image_404s_for_wrong_room(client, db_session): + await register_and_login(client, db_session, username=_unique("alice")) + room_a = (await client.post("/api/rooms", json={"name": _unique("room-a")})).json() + room_b = (await client.post("/api/rooms", json={"name": _unique("room-b")})).json() + upload = await client.post( + f"/api/rooms/{room_a['id']}/images", + files={"file": ("test.png", _png_bytes(), "image/png")}, + ) + image_id = upload.json()["id"] + + resp = await client.get(f"/api/rooms/{room_b['id']}/images/{image_id}") + assert resp.status_code == 404 + + +def _register_ws(ws_client, username: str) -> dict: + from app.schemas.user import UserCreate + from app.services.auth_service import register_user + + async def _seed(): + async with ws_client.session_factory() as session: + await register_user( + session, + UserCreate(username=username, email=f"{username}@example.com", password="password123"), + ) + + ws_client.portal.call(_seed) + resp = ws_client.post( + "/api/auth/login", json={"username_or_email": username, "password": "password123"} + ) + assert resp.status_code == 200, resp.text + return resp.json() + + +def test_ws_image_only_message_roundtrips(ws_client): + _register_ws(ws_client, _unique("alice")) + room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json() + + upload = ws_client.post( + f"/api/rooms/{room['id']}/images", + files={"file": ("test.png", _png_bytes(), "image/png")}, + ) + image_id = upload.json()["id"] + + with ws_client.websocket_connect("/ws/chat") as ws: + ws.send_json({"type": "join", "room_id": room["id"]}) + assert ws.receive_json()["type"] == "joined" + + ws.send_json({"type": "message", "room_id": room["id"], "image_id": image_id}) + message = ws.receive_json() + assert message["type"] == "message" + assert message["content"] is None + assert message["image_id"] == image_id + + history = ws_client.get(f"/api/rooms/{room['id']}/messages").json() + persisted = next(m for m in history if m["id"] == message["id"]) + assert persisted["content"] is None + assert persisted["image_id"] == image_id + + +def test_ws_message_requires_content_or_image(ws_client): + _register_ws(ws_client, _unique("alice")) + room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json() + + with ws_client.websocket_connect("/ws/chat") as ws: + ws.send_json({"type": "join", "room_id": room["id"]}) + assert ws.receive_json()["type"] == "joined" + + ws.send_json({"type": "message", "room_id": room["id"]}) + resp = ws.receive_json() + assert resp["type"] == "error" + + +def test_push_body_says_sent_an_image_for_image_only_message(ws_client, monkeypatch): + calls = [] + monkeypatch.setattr("app.services.push_service.webpush", lambda **kw: calls.append(kw)) + + alice = _register_ws(ws_client, _unique("alice")) + room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json() + + bob = _register_ws(ws_client, _unique("bob")) + ws_client.post(f"/api/rooms/{room['id']}/join") + ws_client.post( + "/api/push/subscribe", + json={ + "endpoint": f"https://push.example.com/ep-{_unique('bob')}", + "keys": {"p256dh": "p256dh-bob", "auth": "auth-bob"}, + }, + ) + + ws_client.post( + "/api/auth/login", json={"username_or_email": alice["username"], "password": "password123"} + ) + upload = ws_client.post( + f"/api/rooms/{room['id']}/images", + files={"file": ("test.png", _png_bytes(), "image/png")}, + ) + image_id = upload.json()["id"] + + with ws_client.websocket_connect("/ws/chat") as ws: + ws.send_json({"type": "join", "room_id": room["id"]}) + assert ws.receive_json()["type"] == "joined" + ws.send_json({"type": "message", "room_id": room["id"], "image_id": image_id}) + assert ws.receive_json()["type"] == "message" + # Sync barrier -- see test_push.py's identical pattern for why this + # is needed before checking server-side push side effects. + ws.send_json({"type": "join", "room_id": room["id"]}) + assert ws.receive_json()["type"] == "joined" + + assert len(calls) == 1 + assert "sent an image" in calls[0]["data"] diff --git a/deploy/backup-postgres.sh b/deploy/backup-postgres.sh index d2f760a..a4d6e09 100755 --- a/deploy/backup-postgres.sh +++ b/deploy/backup-postgres.sh @@ -9,6 +9,10 @@ # 0 3 * * * /usr/local/bin/chatapp-backup-postgres.sh # # See ../DEPLOYMENT.md for the full data-server setup this fits into. +# +# Covers Postgres only. Uploaded chat images live on the app server's disk +# (/srv/chatapp/uploads, see app/storage.py), not here -- see DEPLOYMENT.md +# §7 for that gap. set -euo pipefail diff --git a/frontend/src/api/rooms.ts b/frontend/src/api/rooms.ts index d5ca5f3..0ce03e0 100644 --- a/frontend/src/api/rooms.ts +++ b/frontend/src/api/rooms.ts @@ -1,4 +1,4 @@ -import { apiFetch } from './client' +import { apiFetch, ApiError, NetworkError } from './client' import type { Message, MyRoomItem, Room, RoomListItem, RoomMember, RoomRole } from '../types' export function listRooms(): Promise { @@ -71,3 +71,39 @@ export function transferOwnership(roomId: string, newOwnerUserId: string): Promi export function getRoomMessages(roomId: string): Promise { return apiFetch(`/api/rooms/${roomId}/messages`) } + +// Not apiFetch: that wrapper always sets Content-Type: application/json, +// which would stomp the multipart boundary the browser needs to set itself +// for a file upload. +export async function uploadRoomImage(roomId: string, file: File): Promise<{ id: string }> { + const formData = new FormData() + formData.append('file', file) + + let response: Response + try { + response = await fetch(`/api/rooms/${roomId}/images`, { + method: 'POST', + credentials: 'include', + body: formData, + }) + } catch { + throw new NetworkError() + } + + if (!response.ok) { + let detail = response.statusText + try { + const body = await response.json() + detail = body.detail ?? detail + } catch { + // response had no JSON body + } + throw new ApiError(response.status, detail) + } + + return (await response.json()) as { id: string } +} + +export function getRoomImageUrl(roomId: string, imageId: string): string { + return `/api/rooms/${roomId}/images/${imageId}` +} diff --git a/frontend/src/components/ChatPane.tsx b/frontend/src/components/ChatPane.tsx index db0cb69..3fbef82 100644 --- a/frontend/src/components/ChatPane.tsx +++ b/frontend/src/components/ChatPane.tsx @@ -99,8 +99,8 @@ export function ChatPane({ room, members, isMobile, onBack, onToggleInfo, infoOp

)} - - + + ) } diff --git a/frontend/src/components/Composer.css b/frontend/src/components/Composer.css index 58435c2..acff6f4 100644 --- a/frontend/src/components/Composer.css +++ b/frontend/src/components/Composer.css @@ -49,8 +49,82 @@ cursor: not-allowed; } +.composer-file-input { + display: none; +} + +.composer-attach { + width: 36px; + height: 36px; + flex: none; + border-radius: var(--radius); + background: transparent; + color: var(--ds-muted); + border: 1px solid var(--ds-border); + display: flex; + align-items: center; + justify-content: center; + cursor: pointer; +} + +.composer-attach:hover:not(:disabled) { + color: var(--ds-text); + border-color: var(--ds-accent); +} + +.composer-attach:disabled { + opacity: 0.5; + cursor: not-allowed; +} + +.composer-spinner { + animation: composer-spin 0.8s linear infinite; +} + +@keyframes composer-spin { + to { + transform: rotate(360deg); + } +} + +.composer-attachment { + position: relative; + width: fit-content; +} + +.composer-attachment-thumb { + max-height: 72px; + max-width: 140px; + border-radius: var(--radius); + border: 1px solid var(--ds-border); + display: block; + object-fit: cover; +} + +.composer-attachment-remove { + position: absolute; + top: -6px; + right: -6px; + width: 18px; + height: 18px; + border-radius: 50%; + background: var(--ds-surface-2); + border: 1px solid var(--ds-border); + color: var(--ds-text); + font-size: 0.7rem; + line-height: 1; + display: flex; + align-items: center; + justify-content: center; + cursor: pointer; +} + .composer-status { font-size: 0.76rem; color: var(--ds-muted); padding-left: 2px; } + +.composer-error { + color: var(--ds-danger, #e5484d); +} diff --git a/frontend/src/components/Composer.tsx b/frontend/src/components/Composer.tsx index 248d97a..0457983 100644 --- a/frontend/src/components/Composer.tsx +++ b/frontend/src/components/Composer.tsx @@ -1,16 +1,22 @@ -import { useRef, useState, type KeyboardEvent } from 'react' +import { useRef, useState, type ChangeEvent, type KeyboardEvent } from 'react' import { useOnlineStatus } from '../hooks/useOnlineStatus' +import { uploadRoomImage } from '../api/rooms' import './Composer.css' interface ComposerProps { + roomId: string roomName: string disabled?: boolean - onSend: (content: string) => void + onSend: (content: string, imageId?: string) => void } -export function Composer({ roomName, disabled, onSend }: ComposerProps) { +export function Composer({ roomId, roomName, disabled, onSend }: ComposerProps) { const [value, setValue] = useState('') + const [pendingImage, setPendingImage] = useState<{ id: string; previewUrl: string } | null>(null) + const [uploading, setUploading] = useState(false) + const [uploadError, setUploadError] = useState(null) const textareaRef = useRef(null) + const fileInputRef = useRef(null) const online = useOnlineStatus() function autoGrow() { @@ -22,9 +28,10 @@ export function Composer({ roomName, disabled, onSend }: ComposerProps) { function handleSend() { const trimmed = value.trim() - if (!trimmed) return - onSend(trimmed) + if (!trimmed && !pendingImage) return + onSend(trimmed, pendingImage?.id) setValue('') + removePendingImage() requestAnimationFrame(autoGrow) } @@ -35,9 +42,80 @@ export function Composer({ roomName, disabled, onSend }: ComposerProps) { } } + async function handleFileSelected(e: ChangeEvent) { + const file = e.target.files?.[0] + e.target.value = '' + if (!file) return + + setUploadError(null) + setUploading(true) + try { + const { id } = await uploadRoomImage(roomId, file) + setPendingImage((prev) => { + if (prev) URL.revokeObjectURL(prev.previewUrl) + return { id, previewUrl: URL.createObjectURL(file) } + }) + } catch (err) { + setUploadError(err instanceof Error ? err.message : 'Upload failed') + } finally { + setUploading(false) + } + } + + function removePendingImage() { + setPendingImage((prev) => { + if (prev) URL.revokeObjectURL(prev.previewUrl) + return null + }) + } + return (
+ {pendingImage && ( +
+ + +
+ )} + {uploadError &&
{uploadError}
}
+ +