WIP checkpoint: Address review feedback: Scaffold FastAPI backend project structure

backend/app/config.py

@@ -18,6 +18,8 @@ class Settings(BaseSettings):
     REDIS_URL: str = "redis://localhost:6379/0"
 
     # JWT / Auth
+    # NOTE: production must override SECRET_KEY with a secure random value
+    # (e.g. `python -c "import secrets; print(secrets.token_urlsafe(32))"`)
     SECRET_KEY: str = "change-me-to-a-secure-random-string"
     ALGORITHM: str = "HS256"
     ACCESS_TOKEN_EXPIRE_MINUTES: int = 30

backend/app/dependencies.py

@@ -35,7 +35,12 @@ async def get_current_user(
     except JWTError:
         raise credentials_exception
 
-    result = await db.execute(select(User).where(User.id == int(user_id)))
+    try:
+        user_id_int = int(user_id)
+    except (ValueError, TypeError):
+        raise credentials_exception
+
+    result = await db.execute(select(User).where(User.id == user_id_int))
     user = result.scalar_one_or_none()
     if user is None:
         raise credentials_exception

backend/app/routers/assets.py

@@ -6,6 +6,7 @@ from fastapi import APIRouter, Depends, HTTPException, UploadFile, File, Query
 from sqlalchemy import select
 from sqlalchemy.ext.asyncio import AsyncSession
 
+from app.config import settings
 from app.database import get_db
 from app.dependencies import CurrentUser
 from app.models.asset import Asset, AssetType
@@ -40,8 +41,7 @@ class AssetResponse(BaseModel):
     created_at: str
     updated_at: str
 
-    class Config:
-        from_attributes = True
+    model_config = {"from_attributes": True}
 
 
 class AssetUpdate(BaseModel):
@@ -167,11 +167,24 @@ async def upload_asset_file(
     if asset is None or asset.owner_id != current_user.id:
         raise HTTPException(status_code=404, detail="Asset not found")
 
-    # TODO: Implement actual file upload logic (S3, local filesystem, etc.)
-    file_path = f"uploads/{asset_id}_{file.filename}"
+    # Sanitize filename to prevent path traversal
+    safe_filename = file.filename.replace("/", "_").replace("\\", "_") if file.filename else f"asset_{asset_id}"
+    file_path = f"uploads/{asset_id}_{safe_filename}"
+
+    # Validate file size against configured maximum
+    max_bytes = settings.MAX_UPLOAD_SIZE_MB * 1024 * 1024
+    content = await file.read()
+    file_size = len(content)
+    if file_size > max_bytes:
+        raise HTTPException(
+            status_code=413,
+            detail=f"File size {file_size} exceeds maximum allowed size {max_bytes} bytes",
+        )
+
+    # TODO: Implement actual file write logic (S3, local filesystem, etc.)
     asset.file_path = file_path
     asset.mime_type = file.content_type
-    asset.file_size = 0  # Will be updated after actual write
+    asset.file_size = file_size
 
     await db.commit()
     await db.refresh(asset)

backend/app/routers/auth.py

@@ -3,6 +3,7 @@
 from datetime import datetime, timedelta, timezone
 
 from fastapi import APIRouter, Depends, HTTPException, status
+from fastapi.security import OAuth2PasswordRequestForm
 from pydantic import BaseModel, EmailStr
 
 from jose import jwt
@@ -33,11 +34,6 @@ class RegisterRequest(BaseModel):
     password: str
 
 
-class LoginRequest(BaseModel):
-    username: str
-    password: str
-
-
 class UserResponse(BaseModel):
     id: int
     username: str
@@ -104,12 +100,16 @@ async def register(req: RegisterRequest, db: AsyncSession = Depends(get_db)):
 
 
 @router.post("/login", response_model=TokenResponse)
-async def login(req: LoginRequest, db: AsyncSession = Depends(get_db)):
-    """Authenticate user and return an access token."""
-    result = await db.execute(select(User).where(User.username == req.username))
+async def login(form: OAuth2PasswordRequestForm = Depends(), db: AsyncSession = Depends(get_db)):
+    """Authenticate user and return an access token.
+
+    Accepts form-encoded credentials (username/password) as expected by
+    OAuth2PasswordBearer, so the Swagger UI "Authorize" flow works correctly.
+    """
+    result = await db.execute(select(User).where(User.username == form.username))
     user = result.scalar_one_or_none()
 
-    if not user or not verify_password(req.password, user.hashed_password):
+    if not user or not verify_password(form.password, user.hashed_password):
         raise HTTPException(
             status_code=status.HTTP_401_UNAUTHORIZED,
             detail="Incorrect username or password",

backend/app/routers/worlds.py

@@ -3,6 +3,7 @@
 from typing import Annotated
 
 from fastapi import APIRouter, Depends, HTTPException, Query
+from pydantic import BaseModel, Field
 from sqlalchemy import select
 from sqlalchemy.ext.asyncio import AsyncSession
 from sqlalchemy.orm import selectinload
@@ -16,92 +17,111 @@ router = APIRouter()
 DbDep = Annotated[AsyncSession, Depends(get_db)]
 
 
-@router.get("/")
+# --- Pydantic schemas ---
+
+class WorldCreate(BaseModel):
+    """Schema for creating a world."""
+    name: str = Field(..., min_length=1, max_length=255)
+    description: str | None = None
+    visibility: WorldVisibility = WorldVisibility.PUBLIC
+
+
+class WorldUpdate(BaseModel):
+    """Schema for updating a world."""
+    name: str | None = Field(None, min_length=1, max_length=255)
+    description: str | None = None
+    visibility: WorldVisibility | None = None
+
+
+class WorldResponse(BaseModel):
+    """Schema for world responses."""
+    id: int
+    name: str
+    description: str | None
+    visibility: WorldVisibility
+    owner_id: int
+    created_at: str | None = None
+
+    model_config = {"from_attributes": True}
+
+
+# --- Endpoints ---
+
+@router.get("/", response_model=list[WorldResponse])
 async def list_worlds(
+    current_user: CurrentUser,
     db: DbDep,
     skip: int = Query(0, ge=0),
     limit: int = Query(50, ge=1, le=100),
     visibility: WorldVisibility | None = None,
 ):
-    """List worlds with optional visibility filter."""
+    """List worlds visible to the current user.
+
+    PUBLIC worlds are visible to everyone.
+    PRIVATE/FRIENDS_ONLY worlds are visible to their owner.
+    """
     query = select(World).options(selectinload(World.owner))
+
+    # Filter by visibility if specified
     if visibility:
         query = query.where(World.visibility == visibility)
+    else:
+        # Show PUBLIC worlds + owner's own worlds
+        query = query.where(
+            (World.visibility == WorldVisibility.PUBLIC)
+            | (World.owner_id == current_user.id)
+        )
+
     query = query.offset(skip).limit(limit).order_by(World.created_at.desc())
     result = await db.execute(query)
     worlds = result.scalars().all()
-    return [
-        {
-            "id": w.id,
-            "name": w.name,
-            "description": w.description,
-            "visibility": w.visibility.value if w.visibility else None,
-            "owner_id": w.owner_id,
-            "created_at": w.created_at.isoformat() if w.created_at else None,
-        }
-        for w in worlds
-    ]
+    return worlds
 
 
-@router.get("/{world_id}")
+@router.get("/{world_id}", response_model=WorldResponse)
 async def get_world(
     world_id: int,
+    current_user: CurrentUser,
     db: DbDep,
 ):
-    """Get a single world by ID."""
+    """Get a single world by ID, respecting visibility."""
     result = await db.execute(
         select(World).where(World.id == world_id).options(selectinload(World.owner))
     )
     world = result.scalar_one_or_none()
     if not world:
         raise HTTPException(status_code=404, detail="World not found")
-    return {
-        "id": world.id,
-        "name": world.name,
-        "description": world.description,
-        "visibility": world.visibility.value if world.visibility else None,
-        "owner_id": world.owner_id,
-        "created_at": world.created_at.isoformat() if world.created_at else None,
-    }
+
+    # Allow access if world is PUBLIC or owned by current user
+    if world.visibility != WorldVisibility.PUBLIC and world.owner_id != current_user.id:
+        raise HTTPException(status_code=404, detail="World not found")
+
+    return world
 
 
-@router.post("/")
+@router.post("/", response_model=WorldResponse, status_code=201)
 async def create_world(
-    world_data: dict,
+    world_data: WorldCreate,
     current_user: CurrentUser,
     db: DbDep,
 ):
     """Create a new world."""
-    name = world_data.get("name")
-    if not name:
-        raise HTTPException(status_code=400, detail="World name is required")
-
     world = World(
-        name=name,
-        description=world_data.get("description"),
+        name=world_data.name,
+        description=world_data.description,
         owner_id=current_user.id,
-        visibility=WorldVisibility(
-            world_data.get("visibility", "public")
-        ),
+        visibility=world_data.visibility,
     )
     db.add(world)
     await db.commit()
     await db.refresh(world)
-
-    return {
-        "id": world.id,
-        "name": world.name,
-        "description": world.description,
-        "visibility": world.visibility.value,
-        "owner_id": world.owner_id,
-        "created_at": world.created_at.isoformat() if world.created_at else None,
-    }
+    return world
 
 
-@router.put("/{world_id}")
+@router.put("/{world_id}", response_model=WorldResponse)
 async def update_world(
     world_id: int,
-    world_data: dict,
+    world_data: WorldUpdate,
     current_user: CurrentUser,
     db: DbDep,
 ):
@@ -114,27 +134,16 @@ async def update_world(
     if world.owner_id != current_user.id:
         raise HTTPException(status_code=403, detail="Not owner of this world")
 
-    if world_data.get("name"):
-        world.name = world_data["name"]
-    if world_data.get("description") is not None:
-        world.description = world_data["description"]
-    if world_data.get("visibility"):
-        world.visibility = WorldVisibility(world_data["visibility"])
+    update_data = world_data.model_dump(exclude_unset=True)
+    for field, value in update_data.items():
+        setattr(world, field, value)
 
     await db.commit()
     await db.refresh(world)
-
-    return {
-        "id": world.id,
-        "name": world.name,
-        "description": world.description,
-        "visibility": world.visibility.value if world.visibility else None,
-        "owner_id": world.owner_id,
-        "created_at": world.created_at.isoformat() if world.created_at else None,
-    }
+    return world
 
 
-@router.delete("/{world_id}")
+@router.delete("/{world_id}", status_code=204)
 async def delete_world(
     world_id: int,
     current_user: CurrentUser,
@@ -151,4 +160,3 @@ async def delete_world(
 
     await db.delete(world)
     await db.commit()
-    return {"detail": "World deleted"}