Private
Public Access
Add generic file attachments to chat messages
Messages can now carry an arbitrary file (MessageFile), parallel to the existing MessageImage feature rather than a refactor of it. Files serve with Content-Disposition: attachment to force a download and prevent an uploaded HTML/SVG from executing same-origin. No content-type allowlist, same 8MB cap as images for now (a separate size-limit redesign is tracked as its own issue). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
+39
-5
@@ -1,4 +1,4 @@
|
||||
# KeepItTalking backend (Phase 1 + 2 + 4 + 5 + 6 + 7 + 8, image uploads, emoji & reactions, user profiles, site invites & email, password reset)
|
||||
# KeepItTalking backend (Phase 1 + 2 + 4 + 5 + 6 + 7 + 8, image uploads, file attachments, emoji & reactions, user profiles, site invites & email, password reset)
|
||||
|
||||
FastAPI + SQLAlchemy 2.0 (async) + PostgreSQL + Redis. Implements auth, room
|
||||
CRUD (open and private), room roles (owner/admin/member) and direct
|
||||
@@ -6,8 +6,9 @@ membership management, 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), a bot/extension layer (scoped API tokens, live bot WebSocket
|
||||
access, incoming and outgoing webhooks, message editing), image uploads in
|
||||
chat messages, emoji reactions on messages, self-service user profiles
|
||||
access, incoming and outgoing webhooks, message editing), image uploads and
|
||||
generic file attachments in chat messages, emoji reactions on messages,
|
||||
self-service user profiles
|
||||
(display name, avatar), self-service password change and a token-based
|
||||
forgot-password flow, and admin-issued email invites for new accounts
|
||||
plus email notifications when a user is added to a room. See
|
||||
@@ -364,6 +365,39 @@ 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.
|
||||
|
||||
## File attachments
|
||||
|
||||
A message can also carry a generic file attachment (`Message.file_id`,
|
||||
nullable, alongside the pre-existing `content` and `image_id`) — a
|
||||
parallel `MessageFile` model/table, not a generalization of `MessageImage`,
|
||||
so the working image feature stayed untouched. `app/storage.py`'s
|
||||
`save_image`/`delete_image` were content-agnostic already (no Pillow
|
||||
usage) and were renamed to `save_file`/`delete_file` now that both features
|
||||
share them; `ImageTooLargeError` was likewise renamed to
|
||||
`UploadTooLargeError`.
|
||||
|
||||
- `POST /api/rooms/{room_id}/files` (room-member gated, multipart) — same
|
||||
8 MB cap as images (`MAX_FILE_BYTES`, currently an alias of
|
||||
`MAX_IMAGE_BYTES`; a real, independently-configurable size-limit redesign
|
||||
is a separate later task — see the open file/image size-limit issue), but
|
||||
**no content-type allowlist** — arbitrary file types are the point of this
|
||||
endpoint, unlike `/images`.
|
||||
- `GET /api/rooms/{room_id}/files/{file_id}` (room-member gated) — 404s if
|
||||
the file doesn't belong to that room, otherwise streams it via
|
||||
`FileResponse(..., filename=...)`. Passing `filename=` makes Starlette set
|
||||
`Content-Disposition: attachment`, which forces a download in the browser
|
||||
regardless of content-type — the deliberate mitigation against a
|
||||
user-uploaded `.html`/`.svg` executing script same-origin (session-cookie
|
||||
theft) if opened directly. This is why there's no content-type blocklist
|
||||
on top of it: forcing a download already neutralizes that whole class of
|
||||
risk.
|
||||
- The WS `"message"` handler and `message_events.py` push-body/broadcast
|
||||
logic mirror the image path exactly (an optional `file_id`, validated
|
||||
against the room; push body says "`{username} sent a file`" for a
|
||||
file-only message).
|
||||
|
||||
Same orphaned-upload disk-space caveat as images applies here too.
|
||||
|
||||
## Emoji & reactions
|
||||
|
||||
An emoji picker in the frontend composer is purely client-side (a static
|
||||
@@ -402,11 +436,11 @@ scoped out.
|
||||
everywhere it's displayed.
|
||||
- `POST /api/auth/me/avatar` / `DELETE /api/auth/me/avatar` — reuse
|
||||
`app/storage.py`'s upload primitives (`read_capped`, `process_image`,
|
||||
`save_image`) from image uploads, but call `process_image(..., square=True,
|
||||
`save_file`) from image uploads, but call `process_image(..., square=True,
|
||||
max_dimension=512)` — a new option that center-crops before downscaling,
|
||||
since avatars need a fixed square shape at a much smaller size than a
|
||||
message image. Unlike message images (which never delete), the previous
|
||||
avatar file **is deleted** on replace/remove (`storage.delete_image`) —
|
||||
avatar file **is deleted** on replace/remove (`storage.delete_file`) —
|
||||
safe to do here because it's strictly one file per user, no accumulation
|
||||
risk to accept the way an orphaned message-image upload has.
|
||||
- `GET /api/users/{user_id}/avatar` (`app/routers/users.py`, new router) —
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
"""message file attachments
|
||||
|
||||
Revision ID: 5b1a142dc271
|
||||
Revises: 456cd78ca571
|
||||
Create Date: 2026-08-14 21:53:05.055721
|
||||
|
||||
"""
|
||||
from typing import Sequence, Union
|
||||
|
||||
from alembic import op
|
||||
import sqlalchemy as sa
|
||||
|
||||
|
||||
# revision identifiers, used by Alembic.
|
||||
revision: str = '5b1a142dc271'
|
||||
down_revision: Union[str, Sequence[str], None] = '456cd78ca571'
|
||||
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_files',
|
||||
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('original_filename', sa.String(length=255), nullable=False),
|
||||
sa.Column('content_type', sa.String(length=150), 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_files_room_id'), 'message_files', ['room_id'], unique=False)
|
||||
op.add_column('messages', sa.Column('file_id', sa.Uuid(), nullable=True))
|
||||
op.create_foreign_key('messages_file_id_fkey', 'messages', 'message_files', ['file_id'], ['id'])
|
||||
op.drop_constraint(op.f('messages_content_or_image_required'), 'messages', type_='check')
|
||||
op.create_check_constraint('messages_content_or_attachment_required', 'messages', 'content IS NOT NULL OR image_id IS NOT NULL OR file_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_attachment_required', 'messages', type_='check')
|
||||
op.create_check_constraint(op.f('messages_content_or_image_required'), 'messages', 'content IS NOT NULL OR image_id IS NOT NULL')
|
||||
op.drop_constraint('messages_file_id_fkey', 'messages', type_='foreignkey')
|
||||
op.drop_column('messages', 'file_id')
|
||||
op.drop_index(op.f('ix_message_files_room_id'), table_name='message_files')
|
||||
op.drop_table('message_files')
|
||||
# ### end Alembic commands ###
|
||||
@@ -5,6 +5,7 @@ from app.models.event_subscription import EventSubscription
|
||||
from app.models.invite import InviteStatus
|
||||
from app.models.membership import RoomMembership, RoomRole
|
||||
from app.models.message import Message
|
||||
from app.models.message_file import MessageFile
|
||||
from app.models.message_image import MessageImage
|
||||
from app.models.message_reaction import MessageReaction
|
||||
from app.models.password_reset import PasswordReset
|
||||
@@ -22,6 +23,7 @@ __all__ = [
|
||||
"RoomMembership",
|
||||
"RoomRole",
|
||||
"Message",
|
||||
"MessageFile",
|
||||
"MessageImage",
|
||||
"MessageReaction",
|
||||
"InviteStatus",
|
||||
|
||||
@@ -11,8 +11,8 @@ class Message(Base):
|
||||
__tablename__ = "messages"
|
||||
__table_args__ = (
|
||||
CheckConstraint(
|
||||
"content IS NOT NULL OR image_id IS NOT NULL",
|
||||
name="messages_content_or_image_required",
|
||||
"content IS NOT NULL OR image_id IS NOT NULL OR file_id IS NOT NULL",
|
||||
name="messages_content_or_attachment_required",
|
||||
),
|
||||
)
|
||||
|
||||
@@ -21,9 +21,10 @@ class Message(Base):
|
||||
user_id: Mapped[uuid.UUID] = mapped_column(ForeignKey("users.id"), 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/image_id/file_id.
|
||||
content: Mapped[str | None] = mapped_column(Text)
|
||||
image_id: Mapped[uuid.UUID | None] = mapped_column(ForeignKey("message_images.id"))
|
||||
file_id: Mapped[uuid.UUID | None] = mapped_column(ForeignKey("message_files.id"))
|
||||
created_at: Mapped[datetime] = mapped_column(
|
||||
DateTime(timezone=True), server_default=func.now(), index=True, nullable=False
|
||||
)
|
||||
@@ -32,3 +33,4 @@ class Message(Base):
|
||||
|
||||
user = relationship("User")
|
||||
image = relationship("MessageImage")
|
||||
file = relationship("MessageFile")
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
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 MessageFile(Base):
|
||||
__tablename__ = "message_files"
|
||||
|
||||
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)
|
||||
storage_filename: Mapped[str] = mapped_column(String(64), nullable=False)
|
||||
original_filename: Mapped[str] = mapped_column(String(255), nullable=False)
|
||||
# Wider than MessageImage's content_type column -- generic MIME strings
|
||||
# (e.g. the Office Open XML types) run 60-80 chars, unlike images'
|
||||
# four known short values.
|
||||
content_type: Mapped[str] = mapped_column(String(150), 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")
|
||||
@@ -22,12 +22,12 @@ from app.services.password_service import (
|
||||
)
|
||||
from app.storage import (
|
||||
ALLOWED_IMAGE_CONTENT_TYPES,
|
||||
ImageTooLargeError,
|
||||
InvalidImageError,
|
||||
delete_image,
|
||||
UploadTooLargeError,
|
||||
delete_file,
|
||||
process_image,
|
||||
read_capped,
|
||||
save_image,
|
||||
save_file,
|
||||
)
|
||||
|
||||
AVATAR_MAX_DIMENSION = 512
|
||||
@@ -91,7 +91,7 @@ async def upload_avatar(
|
||||
|
||||
try:
|
||||
data = await read_capped(file)
|
||||
except ImageTooLargeError:
|
||||
except UploadTooLargeError:
|
||||
raise HTTPException(status_code=413, detail="Image exceeds 8 MB limit")
|
||||
|
||||
try:
|
||||
@@ -102,14 +102,14 @@ async def upload_avatar(
|
||||
raise HTTPException(status_code=400, detail="File is not a valid image")
|
||||
|
||||
previous_filename = current_user.avatar_filename
|
||||
storage_filename = save_image(data, ext)
|
||||
storage_filename = save_file(data, ext)
|
||||
current_user.avatar_filename = storage_filename
|
||||
current_user.avatar_content_type = file.content_type
|
||||
await db.commit()
|
||||
await db.refresh(current_user)
|
||||
|
||||
if previous_filename:
|
||||
delete_image(previous_filename)
|
||||
delete_file(previous_filename)
|
||||
|
||||
return current_user
|
||||
|
||||
@@ -126,7 +126,7 @@ async def remove_avatar(
|
||||
await db.refresh(current_user)
|
||||
|
||||
if previous_filename:
|
||||
delete_image(previous_filename)
|
||||
delete_file(previous_filename)
|
||||
|
||||
return current_user
|
||||
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import pathlib
|
||||
import uuid
|
||||
|
||||
from fastapi import APIRouter, Depends, File, HTTPException, Query, Request, UploadFile
|
||||
@@ -11,8 +12,9 @@ from app.dependencies import (
|
||||
require_room_role,
|
||||
require_scope,
|
||||
)
|
||||
from app.models import MessageImage, RoomRole, User
|
||||
from app.schemas.message import MessageRead
|
||||
from app.models import MessageFile, MessageImage, RoomRole, User
|
||||
from app.schemas.message import MessageFileInfo, MessageRead
|
||||
from app.schemas.message_file import MessageFileCreated
|
||||
from app.schemas.message_image import MessageImageCreated
|
||||
from app.schemas.room import (
|
||||
MyRoomItem,
|
||||
@@ -71,12 +73,13 @@ from app.services.webhook_service import (
|
||||
from app.services.ssrf import UnsafeWebhookUrlError
|
||||
from app.storage import (
|
||||
ALLOWED_IMAGE_CONTENT_TYPES,
|
||||
MAX_FILE_BYTES,
|
||||
UPLOADS_DIR,
|
||||
ImageTooLargeError,
|
||||
InvalidImageError,
|
||||
UploadTooLargeError,
|
||||
process_image,
|
||||
read_capped,
|
||||
save_image,
|
||||
save_file,
|
||||
)
|
||||
|
||||
router = APIRouter(prefix="/api/rooms", tags=["rooms"])
|
||||
@@ -284,6 +287,15 @@ async def transfer_ownership_endpoint(
|
||||
)
|
||||
|
||||
|
||||
def _to_message_file_info(file: MessageFile) -> MessageFileInfo:
|
||||
return MessageFileInfo(
|
||||
id=file.id,
|
||||
filename=file.original_filename,
|
||||
size_bytes=file.size_bytes,
|
||||
content_type=file.content_type,
|
||||
)
|
||||
|
||||
|
||||
@router.get("/{room_id}/messages", response_model=list[MessageRead])
|
||||
async def get_room_messages_endpoint(
|
||||
room_id: uuid.UUID,
|
||||
@@ -304,6 +316,7 @@ async def get_room_messages_endpoint(
|
||||
username=m.user.username,
|
||||
content=m.content,
|
||||
image_id=m.image_id,
|
||||
file=_to_message_file_info(m.file) if m.file else None,
|
||||
reactions=reactions_by_message.get(m.id, []),
|
||||
created_at=m.created_at,
|
||||
edited_at=m.edited_at,
|
||||
@@ -326,7 +339,7 @@ async def upload_room_image_endpoint(
|
||||
|
||||
try:
|
||||
data = await read_capped(file)
|
||||
except ImageTooLargeError:
|
||||
except UploadTooLargeError:
|
||||
raise HTTPException(status_code=413, detail="Image exceeds 8 MB limit")
|
||||
|
||||
try:
|
||||
@@ -334,7 +347,7 @@ async def upload_room_image_endpoint(
|
||||
except InvalidImageError:
|
||||
raise HTTPException(status_code=400, detail="File is not a valid image")
|
||||
|
||||
storage_filename = save_image(data, ext)
|
||||
storage_filename = save_file(data, ext)
|
||||
image = MessageImage(
|
||||
room_id=room_id,
|
||||
uploaded_by=current_user.id,
|
||||
@@ -366,6 +379,67 @@ async def get_room_image_endpoint(
|
||||
)
|
||||
|
||||
|
||||
@router.post("/{room_id}/files", response_model=MessageFileCreated, status_code=201)
|
||||
async def upload_room_file_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)
|
||||
|
||||
try:
|
||||
data = await read_capped(file, cap=MAX_FILE_BYTES)
|
||||
except UploadTooLargeError:
|
||||
raise HTTPException(status_code=413, detail="File exceeds 8 MB limit")
|
||||
|
||||
original_filename = file.filename or "file"
|
||||
ext = pathlib.Path(original_filename).suffix
|
||||
storage_filename = save_file(data, ext)
|
||||
message_file = MessageFile(
|
||||
room_id=room_id,
|
||||
uploaded_by=current_user.id,
|
||||
storage_filename=storage_filename,
|
||||
original_filename=original_filename,
|
||||
content_type=file.content_type or "application/octet-stream",
|
||||
size_bytes=len(data),
|
||||
)
|
||||
db.add(message_file)
|
||||
await db.commit()
|
||||
await db.refresh(message_file)
|
||||
return MessageFileCreated(
|
||||
id=message_file.id,
|
||||
filename=message_file.original_filename,
|
||||
size_bytes=message_file.size_bytes,
|
||||
content_type=message_file.content_type,
|
||||
)
|
||||
|
||||
|
||||
@router.get("/{room_id}/files/{file_id}")
|
||||
async def get_room_file_endpoint(
|
||||
room_id: uuid.UUID,
|
||||
file_id: uuid.UUID,
|
||||
current_user: User = Depends(get_current_user),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
await require_room_member(room_id, current_user, db)
|
||||
message_file = await db.get(MessageFile, file_id)
|
||||
if message_file is None or message_file.room_id != room_id:
|
||||
raise HTTPException(status_code=404, detail="File not found")
|
||||
# `filename=` makes Starlette set Content-Disposition: attachment,
|
||||
# forcing a download instead of an inline render regardless of
|
||||
# content-type -- the mitigation for a same-origin-served, user-
|
||||
# uploaded file (e.g. .html/.svg) executing script in this app's own
|
||||
# origin if opened directly. No content-type allowlist needed on top
|
||||
# of this; see backend/README.md.
|
||||
return FileResponse(
|
||||
UPLOADS_DIR / message_file.storage_filename,
|
||||
media_type=message_file.content_type,
|
||||
filename=message_file.original_filename,
|
||||
headers={"Cache-Control": "private, max-age=31536000, immutable"},
|
||||
)
|
||||
|
||||
|
||||
@router.post("/{room_id}/members", response_model=RoomMemberRead, status_code=201)
|
||||
async def add_member_endpoint(
|
||||
room_id: uuid.UUID,
|
||||
|
||||
@@ -10,6 +10,15 @@ class ReactionSummary(BaseModel):
|
||||
user_ids: list[str]
|
||||
|
||||
|
||||
class MessageFileInfo(BaseModel):
|
||||
model_config = ConfigDict(from_attributes=True)
|
||||
|
||||
id: uuid.UUID
|
||||
filename: str
|
||||
size_bytes: int
|
||||
content_type: str
|
||||
|
||||
|
||||
class MessageRead(BaseModel):
|
||||
model_config = ConfigDict(from_attributes=True)
|
||||
|
||||
@@ -19,6 +28,7 @@ class MessageRead(BaseModel):
|
||||
username: str
|
||||
content: str | None
|
||||
image_id: uuid.UUID | None
|
||||
file: MessageFileInfo | None
|
||||
reactions: list[ReactionSummary]
|
||||
created_at: datetime
|
||||
edited_at: datetime | None
|
||||
|
||||
@@ -0,0 +1,10 @@
|
||||
import uuid
|
||||
|
||||
from pydantic import BaseModel
|
||||
|
||||
|
||||
class MessageFileCreated(BaseModel):
|
||||
id: uuid.UUID
|
||||
filename: str
|
||||
size_bytes: int
|
||||
content_type: str
|
||||
@@ -3,7 +3,7 @@ import uuid
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.models import Message, Room, RoomMembership, User
|
||||
from app.models import Message, MessageFile, Room, RoomMembership, User
|
||||
from app.schemas.message import ReactionSummary
|
||||
from app.services.push_service import send_push_to_user
|
||||
from app.services.webhook_service import dispatch_event
|
||||
@@ -27,11 +27,12 @@ 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"
|
||||
)
|
||||
if message.content:
|
||||
body = f"{sender.username}: {message.content}"[:120]
|
||||
elif message.file_id:
|
||||
body = f"{sender.username} sent a file"
|
||||
else:
|
||||
body = f"{sender.username} sent an image"
|
||||
payload = {
|
||||
"title": f"#{room.name}" if room else "New message",
|
||||
"body": body,
|
||||
@@ -41,7 +42,17 @@ async def _notify_offline_members(
|
||||
await send_push_to_user(db, user_id, payload)
|
||||
|
||||
|
||||
def _message_payload(message: Message, username: str) -> dict:
|
||||
async def _message_payload(db: AsyncSession, message: Message, username: str) -> dict:
|
||||
file_payload = None
|
||||
if message.file_id:
|
||||
message_file = await db.get(MessageFile, message.file_id)
|
||||
if message_file:
|
||||
file_payload = {
|
||||
"id": str(message_file.id),
|
||||
"filename": message_file.original_filename,
|
||||
"size_bytes": message_file.size_bytes,
|
||||
"content_type": message_file.content_type,
|
||||
}
|
||||
return {
|
||||
"type": "message",
|
||||
"id": str(message.id),
|
||||
@@ -50,6 +61,7 @@ def _message_payload(message: Message, username: str) -> dict:
|
||||
"username": username,
|
||||
"content": message.content,
|
||||
"image_id": str(message.image_id) if message.image_id else None,
|
||||
"file": file_payload,
|
||||
"reactions": [],
|
||||
"created_at": message.created_at.isoformat(),
|
||||
"edited_at": message.edited_at.isoformat() if message.edited_at else None,
|
||||
@@ -67,7 +79,7 @@ async def broadcast_new_message(
|
||||
"""The full side-effect sequence for a newly created message, shared by
|
||||
the WS "message" handler and the incoming-webhook receiver so both
|
||||
trigger identical fan-out/push/event behavior."""
|
||||
payload = _message_payload(message, sender.username)
|
||||
payload = await _message_payload(db, message, sender.username)
|
||||
await broadcaster.publish(room_id, payload)
|
||||
await _notify_offline_members(db, presence, room_id, sender, message)
|
||||
await dispatch_event(db, "message.created", room_id, payload)
|
||||
|
||||
@@ -24,8 +24,11 @@ async def create_message(
|
||||
user_id: uuid.UUID,
|
||||
content: str | None = None,
|
||||
image_id: uuid.UUID | None = None,
|
||||
file_id: uuid.UUID | None = None,
|
||||
) -> Message:
|
||||
message = Message(room_id=room_id, user_id=user_id, content=content, image_id=image_id)
|
||||
message = Message(
|
||||
room_id=room_id, user_id=user_id, content=content, image_id=image_id, file_id=file_id
|
||||
)
|
||||
db.add(message)
|
||||
await db.commit()
|
||||
await db.refresh(message)
|
||||
@@ -54,7 +57,7 @@ async def list_recent_messages(
|
||||
result = await db.execute(
|
||||
select(Message)
|
||||
.where(Message.room_id == room_id)
|
||||
.options(selectinload(Message.user))
|
||||
.options(selectinload(Message.user), selectinload(Message.file))
|
||||
.order_by(Message.created_at.desc())
|
||||
.limit(limit)
|
||||
)
|
||||
|
||||
@@ -11,6 +11,10 @@ from PIL import Image, UnidentifiedImageError
|
||||
UPLOADS_DIR = pathlib.Path(__file__).resolve().parent.parent.parent / "uploads"
|
||||
|
||||
MAX_IMAGE_BYTES = 8 * 1024 * 1024
|
||||
# Separate named constant (same value for now) so a later size-limit
|
||||
# redesign for generic file attachments doesn't have to touch image
|
||||
# behavior.
|
||||
MAX_FILE_BYTES = MAX_IMAGE_BYTES
|
||||
_READ_CHUNK_BYTES = 1024 * 1024
|
||||
_MAX_DIMENSION = 2000
|
||||
|
||||
@@ -23,7 +27,7 @@ ALLOWED_IMAGE_CONTENT_TYPES: dict[str, tuple[str, str]] = {
|
||||
}
|
||||
|
||||
|
||||
class ImageTooLargeError(Exception):
|
||||
class UploadTooLargeError(Exception):
|
||||
pass
|
||||
|
||||
|
||||
@@ -43,7 +47,7 @@ async def read_capped(file, cap: int = MAX_IMAGE_BYTES) -> bytes:
|
||||
break
|
||||
total += len(chunk)
|
||||
if total > cap:
|
||||
raise ImageTooLargeError()
|
||||
raise UploadTooLargeError()
|
||||
chunks.append(chunk)
|
||||
return b"".join(chunks)
|
||||
|
||||
@@ -89,14 +93,14 @@ def process_image(
|
||||
return out.getvalue(), ext
|
||||
|
||||
|
||||
def save_image(data: bytes, ext: str) -> str:
|
||||
def save_file(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
|
||||
|
||||
|
||||
def delete_image(storage_filename: str) -> None:
|
||||
def delete_file(storage_filename: str) -> None:
|
||||
"""Best-effort delete -- a missing file (already gone, or never
|
||||
written) is not an error."""
|
||||
(UPLOADS_DIR / storage_filename).unlink(missing_ok=True)
|
||||
|
||||
+19
-4
@@ -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, Message, MessageImage, RoomMembership, User
|
||||
from app.models import ApiToken, Message, MessageFile, MessageImage, RoomMembership, User
|
||||
from app.services.bot_service import resolve_token
|
||||
from app.services.message_events import (
|
||||
broadcast_message_update,
|
||||
@@ -31,6 +31,7 @@ class ClientEnvelope(BaseModel):
|
||||
room_id: uuid.UUID | None = None
|
||||
content: str | None = None
|
||||
image_id: uuid.UUID | None = None
|
||||
file_id: uuid.UUID | None = None
|
||||
message_id: uuid.UUID | None = None
|
||||
emoji: str | None = None
|
||||
|
||||
@@ -110,9 +111,16 @@ 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 and envelope.image_id is None):
|
||||
if envelope.room_id is None or (
|
||||
not envelope.content
|
||||
and envelope.image_id is None
|
||||
and envelope.file_id is None
|
||||
):
|
||||
await websocket.send_json(
|
||||
{"type": "error", "detail": "room_id and content or image_id required"}
|
||||
{
|
||||
"type": "error",
|
||||
"detail": "room_id and content or image_id or file_id required",
|
||||
}
|
||||
)
|
||||
continue
|
||||
if _missing_scope(api_token, "write:messages"):
|
||||
@@ -134,8 +142,15 @@ async def chat_endpoint(websocket: WebSocket, db: AsyncSession = Depends(get_db)
|
||||
await websocket.send_json({"type": "error", "detail": "Invalid image"})
|
||||
continue
|
||||
image_id = image.id
|
||||
file_id = None
|
||||
if envelope.file_id is not None:
|
||||
message_file = await db.get(MessageFile, envelope.file_id)
|
||||
if message_file is None or message_file.room_id != envelope.room_id:
|
||||
await websocket.send_json({"type": "error", "detail": "Invalid file"})
|
||||
continue
|
||||
file_id = message_file.id
|
||||
message = await create_message(
|
||||
db, envelope.room_id, user.id, envelope.content, image_id
|
||||
db, envelope.room_id, user.id, envelope.content, image_id, file_id
|
||||
)
|
||||
await broadcast_new_message(db, broadcaster, presence, envelope.room_id, message, user)
|
||||
|
||||
|
||||
@@ -0,0 +1,198 @@
|
||||
import uuid
|
||||
|
||||
from tests.conftest import register_and_login
|
||||
|
||||
|
||||
def _unique(prefix: str) -> str:
|
||||
return f"{prefix}-{uuid.uuid4().hex[:8]}"
|
||||
|
||||
|
||||
async def test_upload_file_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']}/files",
|
||||
files={"file": ("report.pdf", b"%PDF-1.4 not a real pdf", "application/pdf")},
|
||||
)
|
||||
assert resp.status_code == 201, resp.text
|
||||
body = resp.json()
|
||||
assert "id" in body
|
||||
assert body["filename"] == "report.pdf"
|
||||
assert body["size_bytes"] == len(b"%PDF-1.4 not a real pdf")
|
||||
assert body["content_type"] == "application/pdf"
|
||||
|
||||
|
||||
async def test_upload_any_content_type_accepted(client, db_session):
|
||||
await register_and_login(client, db_session, username=_unique("alice"))
|
||||
room = (await client.post("/api/rooms", json={"name": _unique("general")})).json()
|
||||
|
||||
# Unlike /images, /files has no content-type allowlist -- arbitrary
|
||||
# files are the point of this endpoint.
|
||||
resp = await client.post(
|
||||
f"/api/rooms/{room['id']}/files",
|
||||
files={"file": ("archive.zip", b"PK\x03\x04 not a real zip", "application/zip")},
|
||||
)
|
||||
assert resp.status_code == 201, resp.text
|
||||
|
||||
|
||||
async def test_upload_oversized_file_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()
|
||||
|
||||
oversized = b"0" * (9 * 1024 * 1024)
|
||||
resp = await client.post(
|
||||
f"/api/rooms/{room['id']}/files",
|
||||
files={"file": ("huge.bin", oversized, "application/octet-stream")},
|
||||
)
|
||||
assert resp.status_code == 413
|
||||
|
||||
|
||||
async def test_serve_file_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']}/files",
|
||||
files={"file": ("notes.txt", b"hello", "text/plain")},
|
||||
)
|
||||
file_id = upload.json()["id"]
|
||||
|
||||
ok = await client.get(f"/api/rooms/{room['id']}/files/{file_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']}/files/{file_id}")
|
||||
assert resp.status_code == 403
|
||||
|
||||
|
||||
async def test_serve_file_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']}/files",
|
||||
files={"file": ("notes.txt", b"hello", "text/plain")},
|
||||
)
|
||||
file_id = upload.json()["id"]
|
||||
|
||||
resp = await client.get(f"/api/rooms/{room_b['id']}/files/{file_id}")
|
||||
assert resp.status_code == 404
|
||||
|
||||
|
||||
async def test_serve_file_forces_download(client, db_session):
|
||||
# The security-relevant assertion for this feature: a user-uploaded
|
||||
# file must never render inline (e.g. an uploaded .html executing
|
||||
# script same-origin) -- Content-Disposition: attachment forces a
|
||||
# download in the browser regardless of content-type.
|
||||
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']}/files",
|
||||
files={"file": ("notes.txt", b"hello", "text/plain")},
|
||||
)
|
||||
file_id = upload.json()["id"]
|
||||
|
||||
resp = await client.get(f"/api/rooms/{room['id']}/files/{file_id}")
|
||||
assert resp.status_code == 200
|
||||
disposition = resp.headers["content-disposition"]
|
||||
assert "attachment" in disposition
|
||||
assert "notes.txt" in disposition
|
||||
|
||||
|
||||
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_file_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']}/files",
|
||||
files={"file": ("notes.txt", b"hello", "text/plain")},
|
||||
)
|
||||
file_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"], "file_id": file_id})
|
||||
message = ws.receive_json()
|
||||
assert message["type"] == "message"
|
||||
assert message["content"] is None
|
||||
assert message["file"]["id"] == file_id
|
||||
assert message["file"]["filename"] == "notes.txt"
|
||||
assert message["file"]["size_bytes"] == len(b"hello")
|
||||
|
||||
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["file"]["id"] == file_id
|
||||
assert persisted["file"]["filename"] == "notes.txt"
|
||||
|
||||
|
||||
def test_ws_message_requires_content_or_image_or_file(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_a_file_for_file_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()
|
||||
|
||||
_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']}/files",
|
||||
files={"file": ("notes.txt", b"hello", "text/plain")},
|
||||
)
|
||||
file_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"], "file_id": file_id})
|
||||
assert ws.receive_json()["type"] == "message"
|
||||
ws.send_json({"type": "join", "room_id": room["id"]})
|
||||
assert ws.receive_json()["type"] == "joined"
|
||||
|
||||
assert len(calls) == 1
|
||||
assert "sent a file" in calls[0]["data"]
|
||||
Reference in New Issue
Block a user