diff --git a/README.md b/README.md new file mode 100644 index 0000000..d313ec9 --- /dev/null +++ b/README.md @@ -0,0 +1,96 @@ +# WikiTCG + +A trading card game where every card is procedurally generated from a Wikipedia +article. Players open booster packs, build decks, trade with each other, and +play real-time matches over WebSocket. + +- **Backend** — FastAPI (`backend/`), PostgreSQL, SQLAlchemy + Alembic +- **Frontend** — SvelteKit with the static adapter (`frontend/`) +- **External** — Wikipedia API (card generation), Resend (email), Stripe (payments) + +## Running locally + +```bash +# Backend — dev server on :8000 +cd backend +pip install -r requirements.txt +uvicorn main:app --reload + +# Frontend — dev server on :5173 +cd frontend +npm install +npm run dev +``` + +Tests and checks: + +```bash +cd backend && pytest test_game.py # game logic +cd frontend && npm run check # svelte-check +``` + +## Deployment + +The stack runs as three Docker Compose services (`db`, `backend`, `frontend`) +behind Nginx Proxy Manager. No service publishes a port; NPM reaches the +containers by name over an external `proxy` network. The database sits on a +second network marked `internal: true`, so it has no route off the host. + +```bash +git pull && docker compose up -d --build +``` + +## Database and migrations + +The database has **two roles, and they are not interchangeable**: + +| Role | Used by | Rights | +| --- | --- | --- | +| `wikitcg_app` | the running app, via `DATABASE_URL` | SELECT/INSERT/UPDATE/DELETE — no DDL | +| `wikitcg` | Alembic only, via `MIGRATION_DATABASE_URL` | owns the schema | + +Migrations therefore run as `wikitcg`, never as the app role: + +```bash +docker compose run --rm backend alembic upgrade head # apply +docker compose run --rm backend alembic revision --autogenerate -m "description" +``` + +`alembic/env.py` reads `MIGRATION_DATABASE_URL` and **fails loudly if it is +unset**. There is deliberately no fallback to `DATABASE_URL`: the app role has no +DDL rights, so falling back would surface as a confusing permissions error partway +through a migration instead of an obvious configuration error up front. + +`sqlalchemy.url` in `alembic.ini` is intentionally empty — the URL comes from the +environment so no credential lives in version control. + +The split needs no sequence grants: every primary key is either a +client-generated UUID or a string. + +## Environment variables + +Set in `.env` at the repo root (untracked). See `backend/core/config.py`. + +**Required** — the app refuses to start without these: + +| Variable | Notes | +| --- | --- | +| `JWT_SECRET_KEY` | signing key for access and refresh tokens | +| `DATABASE_URL` | app role (`wikitcg_app`) | +| `RESEND_API_KEY`, `EMAIL_FROM` | transactional email | +| `STRIPE_SECRET_KEY`, `STRIPE_PUBLISHABLE_KEY`, `STRIPE_WEBHOOK_SECRET` | payments | + +**Required for migrations:** + +| Variable | Notes | +| --- | --- | +| `MIGRATION_DATABASE_URL` | schema owner (`wikitcg`) — Alembic only | + +**Optional:** + +| Variable | Default | Notes | +| --- | --- | --- | +| `FRONTEND_URL` | `http://localhost:5173` | links in outbound email | +| `CORS_ORIGINS` | `http://localhost:5173` | comma-separated | +| `WIKIRANK_USER_AGENT` | `WikiTCG/1.0` | sent to the Wikipedia API | +| `ENABLE_DOCS` | unset (disabled) | set to exactly `true` to serve `/docs`, `/redoc` and `/openapi.json`. Any other value leaves them disabled, so a new environment fails closed. | diff --git a/backend/alembic.ini b/backend/alembic.ini index 1d141c2..1718e4a 100644 --- a/backend/alembic.ini +++ b/backend/alembic.ini @@ -83,10 +83,9 @@ path_separator = os # are written from script.py.mako # output_encoding = utf-8 -# database URL. This is consumed by the user-maintained env.py script only. -# other means of configuring database URLs may be customized within the env.py -# file. -sqlalchemy.url = placeholder +# Intentionally empty: env.py reads MIGRATION_DATABASE_URL from the environment. +# Do not put a URL here — it would be a credential in version control. +sqlalchemy.url = [post_write_hooks] diff --git a/backend/alembic/env.py b/backend/alembic/env.py index 1e2c638..133c12a 100644 --- a/backend/alembic/env.py +++ b/backend/alembic/env.py @@ -1,83 +1,88 @@ +import os + from dotenv import load_dotenv load_dotenv() from logging.config import fileConfig -from sqlalchemy import engine_from_config from sqlalchemy import pool, create_engine from alembic import context from core.models import Base -import os -from dotenv import load_dotenv -load_dotenv() +# Migrations run as the schema owner (wikitcg), which is a different role from the +# one the app runs as (wikitcg_app, DML only). Deliberately no fallback to +# DATABASE_URL: falling back would connect as the app role and fail somewhere deep +# inside a CREATE/ALTER with a permissions error, instead of telling you plainly +# that the environment is misconfigured. +MIGRATION_DATABASE_URL = os.environ.get("MIGRATION_DATABASE_URL") +if not MIGRATION_DATABASE_URL: + raise RuntimeError( + "MIGRATION_DATABASE_URL is not set. Migrations must run as the schema owner " + "(wikitcg), not as the application role (wikitcg_app), which has no DDL " + "rights. In Docker: docker compose run --rm backend alembic upgrade head" + ) # this is the Alembic Config object, which provides # access to the values within the .ini file in use. config = context.config +# Note: the URL is passed straight to create_engine / context.configure rather than +# through config.set_main_option. alembic.ini is a configparser file, so a password +# containing '%' would be mangled by interpolation on the way through. + # Interpret the config file for Python logging. # This line sets up loggers basically. if config.config_file_name is not None: - fileConfig(config.config_file_name) + fileConfig(config.config_file_name) # add your model's MetaData object here # for 'autogenerate' support -# from myapp import mymodel -# target_metadata = mymodel.Base.metadata target_metadata = Base.metadata -# other values from the config, defined by the needs of env.py, -# can be acquired: -# my_important_option = config.get_main_option("my_important_option") -# ... etc. - def run_migrations_offline() -> None: - """Run migrations in 'offline' mode. + """Run migrations in 'offline' mode. - This configures the context with just a URL - and not an Engine, though an Engine is acceptable - here as well. By skipping the Engine creation - we don't even need a DBAPI to be available. + This configures the context with just a URL + and not an Engine, though an Engine is acceptable + here as well. By skipping the Engine creation + we don't even need a DBAPI to be available. - Calls to context.execute() here emit the given string to the - script output. + Calls to context.execute() here emit the given string to the + script output. - """ - url = os.environ["DATABASE_URL"] - print(url) - context.configure( - url=url, - target_metadata=target_metadata, - literal_binds=True, - dialect_opts={"paramstyle": "named"}, - ) + """ + context.configure( + url=MIGRATION_DATABASE_URL, + target_metadata=target_metadata, + literal_binds=True, + dialect_opts={"paramstyle": "named"}, + ) - with context.begin_transaction(): - context.run_migrations() + with context.begin_transaction(): + context.run_migrations() def run_migrations_online() -> None: - """Run migrations in 'online' mode. + """Run migrations in 'online' mode. - In this scenario we need to create an Engine - and associate a connection with the context. + In this scenario we need to create an Engine + and associate a connection with the context. - """ - connectable = create_engine(os.environ["DATABASE_URL"], poolclass=pool.NullPool) + """ + connectable = create_engine(MIGRATION_DATABASE_URL, poolclass=pool.NullPool) - with connectable.connect() as connection: - context.configure( - connection=connection, target_metadata=target_metadata - ) + with connectable.connect() as connection: + context.configure( + connection=connection, target_metadata=target_metadata + ) - with context.begin_transaction(): - context.run_migrations() + with context.begin_transaction(): + context.run_migrations() if context.is_offline_mode(): - run_migrations_offline() + run_migrations_offline() else: - run_migrations_online() + run_migrations_online() diff --git a/backend/alembic/versions/c9a17b4e2d80_add_token_valid_after_to_users.py b/backend/alembic/versions/c9a17b4e2d80_add_token_valid_after_to_users.py new file mode 100644 index 0000000..67f176f --- /dev/null +++ b/backend/alembic/versions/c9a17b4e2d80_add_token_valid_after_to_users.py @@ -0,0 +1,34 @@ +"""add token_valid_after to users + +Revocation cutoff for JWTs. Tokens are stateless, so before this there was no way +to invalidate a session at all — a password reset left every outstanding access and +refresh token working, including one an attacker had already stolen. + +Nullable with no default: NULL means "nothing revoked", which is the correct state +for every existing user, so this is a cheap ADD COLUMN with no table rewrite. + +Revision ID: c9a17b4e2d80 +Revises: f657d45be3ae +Create Date: 2026-07-29 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = 'c9a17b4e2d80' +down_revision: Union[str, None] = 'f657d45be3ae' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # timezone=True: compared against the JWT 'iat' claim, which is always UTC. + op.add_column('users', sa.Column('token_valid_after', sa.DateTime(timezone=True), nullable=True)) + + +def downgrade() -> None: + op.drop_column('users', 'token_valid_after') diff --git a/backend/core/auth.py b/backend/core/auth.py index 24cbdf9..5a07ef2 100644 --- a/backend/core/auth.py +++ b/backend/core/auth.py @@ -1,5 +1,5 @@ import logging -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone from jose import JWTError, jwt from passlib.context import CryptContext @@ -15,34 +15,85 @@ REFRESH_TOKEN_EXPIRE_DAYS = 30 pwd_context = CryptContext(schemes=["bcrypt"]) +# Verified against when a login names a user that doesn't exist, so the response time +# is the same either way. Without it, "no such user" returns before bcrypt runs and +# the timing difference tells an attacker which usernames are real. +DUMMY_PASSWORD_HASH = pwd_context.hash("not-a-real-password") + def hash_password(password: str) -> str: return pwd_context.hash(password) def verify_password(plain: str, hashed: str) -> bool: return pwd_context.verify(plain, hashed) +# All token timestamps are timezone-aware UTC. They used to be naive datetime.now(), +# which jose serialises as if it were already UTC — with TZ=Europe/Paris that handed +# out tokens living 1-2 hours longer than intended, drifting again at each DST change. +# +# 'iat' is carried so a password reset can invalidate everything issued before it; +# see token_valid_after on the User model. It is written as a float rather than a +# datetime: jose truncates datetimes to whole seconds, and at that resolution a token +# minted just *after* a reset is indistinguishable from one minted just before, so +# logging in immediately after a password change would be rejected. RFC 7519 permits +# a non-integer NumericDate. + def create_access_token(user_id: str) -> str: - expire = datetime.now() + timedelta(minutes=ACCESS_TOKEN_EXPIRE_MINUTES) - return jwt.encode({"sub": user_id, "exp": expire, "type": "access"}, SECRET_KEY, algorithm=ALGORITHM) + now = datetime.now(timezone.utc) + return jwt.encode( + { + "sub": user_id, + "iat": now.timestamp(), + "exp": now + timedelta(minutes=ACCESS_TOKEN_EXPIRE_MINUTES), + "type": "access", + }, + SECRET_KEY, algorithm=ALGORITHM, + ) def create_refresh_token(user_id: str) -> str: - expire = datetime.now() + timedelta(days=REFRESH_TOKEN_EXPIRE_DAYS) - return jwt.encode({"sub": user_id, "exp": expire, "type": "refresh"}, SECRET_KEY, algorithm=ALGORITHM) + now = datetime.now(timezone.utc) + return jwt.encode( + { + "sub": user_id, + "iat": now.timestamp(), + "exp": now + timedelta(days=REFRESH_TOKEN_EXPIRE_DAYS), + "type": "refresh", + }, + SECRET_KEY, algorithm=ALGORITHM, + ) -def decode_refresh_token(token: str) -> str | None: +def _decode(token: str, expected_type: str) -> dict | None: + """Verified payload, or None if the signature, expiry or type is wrong.""" try: payload = jwt.decode(token, SECRET_KEY, algorithms=[ALGORITHM]) - if payload.get("type") != "refresh": - return None - return payload.get("sub") except JWTError: return None + if payload.get("type") != expected_type: + return None + return payload + +def decode_refresh_token_payload(token: str) -> dict | None: + return _decode(token, "refresh") + +def decode_access_token_payload(token: str) -> dict | None: + return _decode(token, "access") + +def decode_refresh_token(token: str) -> str | None: + payload = _decode(token, "refresh") + return payload.get("sub") if payload else None def decode_access_token(token: str) -> str | None: - try: - payload = jwt.decode(token, SECRET_KEY, algorithms=[ALGORITHM]) - if payload.get("type") != "access": - return None - return payload.get("sub") - except JWTError: - return None \ No newline at end of file + payload = _decode(token, "access") + return payload.get("sub") if payload else None + +def token_issued_before(payload: dict, cutoff: datetime | None) -> bool: + """True if this token predates a revocation cutoff (i.e. should be rejected).""" + if cutoff is None: + return False + iat = payload.get("iat") + # Tokens minted before 'iat' was introduced have no issue time, so they can't be + # proven to postdate the cutoff — treat them as revoked rather than trusted. + if iat is None: + return True + if cutoff.tzinfo is None: + cutoff = cutoff.replace(tzinfo=timezone.utc) + return datetime.fromtimestamp(iat, timezone.utc) < cutoff \ No newline at end of file diff --git a/backend/core/config.py b/backend/core/config.py index 3275b1e..52b1c12 100644 --- a/backend/core/config.py +++ b/backend/core/config.py @@ -21,4 +21,9 @@ STRIPE_WEBHOOK_SECRET = require("STRIPE_WEBHOOK_SECRET") # Optional with sensible defaults for local dev FRONTEND_URL = optional("FRONTEND_URL", "http://localhost:5173") CORS_ORIGINS = optional("CORS_ORIGINS", "http://localhost:5173").split(",") -WIKIRANK_USER_AGENT = optional("WIKIRANK_USER_AGENT", "WikiTCG/1.0") \ No newline at end of file +WIKIRANK_USER_AGENT = optional("WIKIRANK_USER_AGENT", "WikiTCG/1.0") + +# Serves /docs, /redoc and /openapi.json, i.e. the whole API surface. Opt-in on an +# explicit "true" rather than off on a truthy value, so a typo or an unset var in +# a new environment fails closed. +ENABLE_DOCS = optional("ENABLE_DOCS").lower() == "true" \ No newline at end of file diff --git a/backend/core/dependencies.py b/backend/core/dependencies.py index 488c550..bd81784 100644 --- a/backend/core/dependencies.py +++ b/backend/core/dependencies.py @@ -1,29 +1,90 @@ +import asyncio +import time import uuid +from collections import defaultdict, deque from datetime import datetime -from fastapi import Depends, HTTPException, Request, status +from fastapi import Depends, HTTPException, Request, WebSocket, WebSocketDisconnect, status from fastapi.security import OAuth2PasswordBearer from slowapi import Limiter -from slowapi.util import get_remote_address from sqlalchemy.orm import Session -from core.auth import decode_access_token +from core.auth import decode_access_token, decode_access_token_payload, token_issued_before from core.database import get_db from core.models import User as UserModel oauth2_scheme = OAuth2PasswordBearer(tokenUrl="login") -# Shared rate limiter — registered on app.state in main.py -limiter = Limiter(key_func=get_remote_address) + +def escape_like(value: str) -> str: + r"""Neutralise LIKE wildcards in user input used as a search term. + + Without this, '%' matches every row and '_' matches any character — so a one-char + search returns the whole table. Use with .ilike(..., escape='\\'). + """ + return value.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") + + +def parse_uuid(value: str, field: str = "id") -> uuid.UUID: + """Path and body params reach us as arbitrary strings; a bare uuid.UUID() on one + raises ValueError, which surfaces as a 500 and a stack trace instead of a 400.""" + try: + return uuid.UUID(value) + except (ValueError, AttributeError, TypeError): + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=f"Invalid {field}") + + +def get_client_ip(conn: Request | WebSocket) -> str: + """The real external client, as seen through Nginx Proxy Manager. + + NPM sets X-Forwarded-For to $proxy_add_x_forwarded_for, which *appends* the peer + it saw to whatever the client sent. So the last entry is always written by NPM and + anything a client forges lands to its left — take the rightmost, never the first. + + This trusts the header because NPM is the only way in: port 8000 is unpublished and + the backend only shares the `proxy` network with NPM. If a port is ever published, + or another container is put on `proxy`, this becomes forgeable and needs revisiting. + """ + forwarded = conn.headers.get("x-forwarded-for") + if forwarded: + client = forwarded.rpartition(",")[2].strip() + if client: + return client + # No header: a direct connection, so the peer address is the client. + return conn.client.host if conn.client else "unknown" + + +# Per-user key where we can, real client IP otherwise. Without this every request +# keys on NPM's container address, i.e. one global bucket for the whole internet — +# eleven logins a minute would lock every user out. +def get_user_id_from_request(request: Request) -> str: + auth = request.headers.get("Authorization", "") + if auth.startswith("Bearer "): + user_id = decode_access_token(auth[7:]) + if user_id: + return f"user:{user_id}" + return get_client_ip(request) + + +# Default key_func for every @limiter.limit that doesn't override it. +limiter = Limiter(key_func=get_user_id_from_request) def get_current_user(token: str = Depends(oauth2_scheme), db: Session = Depends(get_db)) -> UserModel: - user_id = decode_access_token(token) - if not user_id: + payload = decode_access_token_payload(token) + if not payload or not payload.get("sub"): raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid token") - user = db.query(UserModel).filter(UserModel.id == uuid.UUID(user_id)).first() + try: + user_uuid = uuid.UUID(payload["sub"]) + except ValueError: + # Signature-verified, so not reachable without the key, but a 500 would be wrong. + raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid token") + user = db.query(UserModel).filter(UserModel.id == user_uuid).first() if not user: raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="User not found") + # Single chokepoint for revocation: the user row is already loaded, so it's free. + if token_issued_before(payload, user.token_valid_after): + raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Session expired, please log in again") # Throttle to one write per 5 minutes so every authenticated request doesn't hammer the DB now = datetime.now() if not user.last_active_at or (now - user.last_active_at).total_seconds() > 300: @@ -32,12 +93,60 @@ def get_current_user(token: str = Depends(oauth2_scheme), db: Session = Depends( return user -# Per-user key for rate limiting authenticated endpoints — prevents shared IPs (NAT/VPN) -# from having their limits pooled. Falls back to remote IP for unauthenticated requests. -def get_user_id_from_request(request: Request) -> str: - auth = request.headers.get("Authorization", "") - if auth.startswith("Bearer "): - user_id = decode_access_token(auth[7:]) - if user_id: - return f"user:{user_id}" - return get_remote_address(request) +## WebSocket connection limiting + +# slowapi decorates HTTP routes only, and WS handlers can't authenticate before +# accept() anyway, so connection attempts are limited by client IP instead. +WS_MAX_CONNECTIONS = 30 +WS_WINDOW_SECONDS = 60 +WS_AUTH_TIMEOUT_SECONDS = 10 + +_ws_attempts: dict[str, deque[float]] = defaultdict(deque) + + +async def check_ws_rate_limit(websocket: WebSocket) -> bool: + """Reject and close if this client is opening sockets too fast. + + Call before accept(). Closing an un-accepted WebSocket sends an HTTP rejection, + so a flooding client never gets a live connection or a DB session. + """ + key = get_client_ip(websocket) + now = time.monotonic() + attempts = _ws_attempts[key] + while attempts and now - attempts[0] > WS_WINDOW_SECONDS: + attempts.popleft() + if len(attempts) >= WS_MAX_CONNECTIONS: + await websocket.close(code=1008) + return False + attempts.append(now) + # Keys are unbounded otherwise: drop idle ones whenever the table gets large. + if len(_ws_attempts) > 10000: + for k in [k for k, v in _ws_attempts.items() if not v or now - v[-1] > WS_WINDOW_SECONDS]: + del _ws_attempts[k] + return True + + +async def accept_and_authenticate_ws(websocket: WebSocket) -> str | None: + """Rate-limit, accept, then take the auth token from the first frame. + + Returns the user id, or None having already closed the socket. + + The token can't be read before accept() — ASGI delivers no frames until then — so + the flood limit above is what guards the pre-auth window, and the receive timeout + stops an accepted-but-never-authenticated socket from pinning a DB session. + """ + if not await check_ws_rate_limit(websocket): + return None + await websocket.accept() + try: + token = await asyncio.wait_for(websocket.receive_text(), timeout=WS_AUTH_TIMEOUT_SECONDS) + except asyncio.TimeoutError: + await websocket.close(code=1008) + return None + except WebSocketDisconnect: + return None + user_id = decode_access_token(token) + if not user_id: + await websocket.close(code=1008) + return None + return user_id diff --git a/backend/core/models.py b/backend/core/models.py index cded94f..47e9e0a 100644 --- a/backend/core/models.py +++ b/backend/core/models.py @@ -28,6 +28,11 @@ class User(Base): email_verification_token_expires_at: Mapped[datetime | None] = mapped_column(DateTime, nullable=True) trade_wishlist: Mapped[str | None] = mapped_column(Text, nullable=True, default="") last_active_at: Mapped[datetime | None] = mapped_column(DateTime, nullable=True) + # Tokens issued before this are rejected — the only way to revoke a session, since + # tokens are pure JWTs with no server-side store. Set on both password-change paths. + # timezone=True unlike its neighbours: it is compared against JWT 'iat', which is + # always UTC, so leaving it naive would make it ambiguous against local-time columns. + token_valid_after: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) cards: Mapped[list["Card"]] = relationship(back_populates="user", cascade="all, delete-orphan") decks: Mapped[list["Deck"]] = relationship(back_populates="user", cascade="all, delete-orphan") diff --git a/backend/game/manager.py b/backend/game/manager.py index dd491a3..91040b4 100644 --- a/backend/game/manager.py +++ b/backend/game/manager.py @@ -163,6 +163,20 @@ async def send_error(ws: WebSocket, message: str): await ws.send_json({"type": "error", "message": message}) +def _frame_int(message: dict, field: str) -> int | None: + """Non-negative int from a client WebSocket frame, or None if absent or malformed. + + bool is rejected explicitly because it subclasses int, so True would pass the range + checks in rules.py and index slot 1. Everything else here is arbitrary client JSON: + a missing key used to raise KeyError and a string raised TypeError on comparison, + both of which killed the socket instead of returning an error frame. + """ + value = message.get(field) + if not isinstance(value, int) or isinstance(value, bool) or value < 0: + return None + return value + + ## Matchmaking def load_deck_cards(deck_id: str, user_id: str, db: Session) -> list | None: @@ -175,7 +189,13 @@ def load_deck_cards(deck_id: str, user_id: str, db: Session) -> list | None: deck_card_ids = [ dc.card_id for dc in db.query(DeckCardModel).filter(DeckCardModel.deck_id == deck.id).all() ] - cards = db.query(CardModel).filter(CardModel.id.in_(deck_card_ids)).all() + # The deck is owned by user_id, but its rows aren't necessarily: filtering on + # owner here means a deck that somehow got a foreign card in it can't bring + # that card into play (and can't bump times_played on someone else's row). + cards = db.query(CardModel).filter( + CardModel.id.in_(deck_card_ids), + CardModel.user_id == uuid.UUID(user_id), + ).all() return cards async def try_match(db: Session): @@ -183,6 +203,12 @@ async def try_match(db: Session): if len(queue) < 2: return + # Guard: same user queued twice (two tabs). Matching them would key `players` and + # `connections` on one id, collapsing both sides into one entry — and + # record_game_result would then credit the same account a win and a loss. + if queue[0].user_id == queue[1].user_id: + return + p1_entry = queue.pop(0) p2_entry = queue.pop(0) @@ -272,10 +298,14 @@ async def handle_action(game_id: str, user_id: str, message: dict, db: Session): err = None if action == "play_card": - err = action_play_card(state, message["hand_index"], message["slot"]) + hand_index = _frame_int(message, "hand_index") + slot = _frame_int(message, "slot") + if hand_index is None or slot is None: + err = "Invalid hand_index or slot" + else: + err = action_play_card(state, hand_index, slot) if not err: - # Find the card that was just played - slot = message["slot"] + # action_play_card returned None, so slot is in range and the board holds it card_instance = state.players[user_id].board[slot] if card_instance: try: @@ -289,9 +319,12 @@ async def handle_action(game_id: str, user_id: str, message: dict, db: Session): logger.warning(f"Failed to increment times_played for card {card_instance.card_id}: {e}") db.rollback() elif action == "sacrifice": - slot = message.get("slot") - if slot is None: - err = "No slot provided" + slot = _frame_int(message, "slot") + # Bounds must be checked here, not left to action_sacrifice: the board is indexed + # below before that call, and slot=-1 would quietly read the last occupied slot + # and leak a sacrifice_animation for it to the opponent. + if slot is None or slot >= BOARD_SIZE: + err = "Invalid slot" else: # Find the card instance_id before it's removed card = state.players[user_id].board[slot] @@ -384,6 +417,12 @@ async def handle_timeout_claim(game_id: str, claimant_id: str, db: Session) -> s state = active_games.get(game_id) if not state: return "Game not found" + # Authorization before anything else: the active-player check below passes for + # any non-participant, so without this an outsider with a game_id could name + # themselves winner. opponent_id() would then pick player_order[0] as the + # loser and the game would be popped out from under both real players. + if claimant_id not in state.players: + return "You are not in this game" if state.result: return "Game already ended" if state.active_player_id == claimant_id: diff --git a/backend/main.py b/backend/main.py index dc473d3..826814c 100644 --- a/backend/main.py +++ b/backend/main.py @@ -12,7 +12,7 @@ from fastapi.middleware.cors import CORSMiddleware from slowapi.errors import RateLimitExceeded from slowapi import _rate_limit_exceeded_handler -from core.config import CORS_ORIGINS, STRIPE_SECRET_KEY +from core.config import CORS_ORIGINS, ENABLE_DOCS, STRIPE_SECRET_KEY from core.dependencies import limiter from services.database_functions import fill_card_pool, run_cleanup_loop @@ -30,7 +30,12 @@ async def lifespan(app: FastAPI): yield -app = FastAPI(lifespan=lifespan) +app = FastAPI( + lifespan=lifespan, + docs_url="/docs" if ENABLE_DOCS else None, + redoc_url="/redoc" if ENABLE_DOCS else None, + openapi_url="/openapi.json" if ENABLE_DOCS else None, +) app.state.limiter = limiter app.add_exception_handler(RateLimitExceeded, cast(Callable, _rate_limit_exceeded_handler)) diff --git a/backend/routers/auth.py b/backend/routers/auth.py index 156a948..b04d7da 100644 --- a/backend/routers/auth.py +++ b/backend/routers/auth.py @@ -2,16 +2,17 @@ import logging import re import secrets import uuid -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone from fastapi import APIRouter, Depends, HTTPException, Request from fastapi.security import OAuth2PasswordRequestForm from pydantic import BaseModel +from sqlalchemy import func from sqlalchemy.orm import Session from core.auth import ( - create_access_token, create_refresh_token, - decode_refresh_token, hash_password, verify_password, + DUMMY_PASSWORD_HASH, create_access_token, create_refresh_token, + decode_refresh_token_payload, hash_password, token_issued_before, verify_password, ) from core.database import get_db from core.dependencies import get_current_user, limiter @@ -58,7 +59,9 @@ def validate_register(username: str, email: str, password: str) -> str | None: return "Username must be at least 2 characters" if len(username) > 16: return "Username must be 16 characters or fewer" - if not re.match(r"^[^\s@]+@[^\s@]+\.[^\s@]+$", email): + # Permissive by design — unicode names are fine. Safety comes from escaping at the + # point of use: html.escape in email_utils, and exact comparison instead of ILIKE. + if len(email) > 254 or not re.match(r"^[^\s@]+@[^\s@]+\.[A-Za-z]{2,}$", email): return "Please enter a valid email" domain = email.split("@")[-1].lower() if domain in _disposable_blocklist: @@ -76,7 +79,9 @@ def register(request: Request, req: RegisterRequest, db: Session = Depends(get_d err = validate_register(req.username, req.email, req.password) if err: raise HTTPException(status_code=400, detail=err) - if db.query(UserModel).filter(UserModel.username.ilike(req.username)).first(): + # Equality, not ilike: as a LIKE *pattern* an unescaped '%' matches every row and + # 'a_c' matches 'abc', so registration could be made to reject valid names. + if db.query(UserModel).filter(func.lower(UserModel.username) == req.username.lower()).first(): raise HTTPException(status_code=400, detail="Username already taken") if db.query(UserModel).filter(UserModel.email == req.email).first(): raise HTTPException(status_code=400, detail="Email already registered") @@ -106,8 +111,12 @@ def register(request: Request, req: RegisterRequest, db: Session = Depends(get_d @router.post("/login") @limiter.limit("10/minute") def login(request: Request, form: OAuth2PasswordRequestForm = Depends(), db: Session = Depends(get_db)): - user = db.query(UserModel).filter(UserModel.username.ilike(form.username)).first() - if not user or not verify_password(form.password, user.password_hash): + user = db.query(UserModel).filter(func.lower(UserModel.username) == form.username.lower()).first() + # Verify unconditionally: short-circuiting on a missing user would return before + # bcrypt runs, and that timing gap enumerates usernames just as well as an explicit + # "no such user" would — which the other handlers here deliberately avoid. + password_ok = verify_password(form.password, user.password_hash if user else DUMMY_PASSWORD_HASH) + if not user or not password_ok: raise HTTPException(status_code=400, detail="Invalid username or password") user.last_active_at = datetime.now() db.commit() @@ -130,6 +139,7 @@ def reset_password(request: Request, req: ResetPasswordRequest, user: UserModel if req.current_password == req.new_password: raise HTTPException(status_code=400, detail="New password must be different from current password") user.password_hash = hash_password(req.new_password) + user.token_valid_after = datetime.now(timezone.utc) db.commit() return {"message": "Password updated"} @@ -166,6 +176,9 @@ def reset_password_with_token(request: Request, req: ResetPasswordWithTokenReque if len(req.new_password) > 256: raise HTTPException(status_code=400, detail="Password must be 256 characters or fewer") user.password_hash = hash_password(req.new_password) + # This is the flow a locked-out or compromised user reaches for, so it must evict + # whoever else is holding a token for this account. + user.token_valid_after = datetime.now(timezone.utc) user.reset_token = None user.reset_token_expires_at = None db.commit() @@ -209,12 +222,20 @@ def resend_verification(request: Request, req: ResendVerificationRequest, db: Se @router.post("/auth/refresh") @limiter.limit("20/minute") def refresh(request: Request, req: RefreshRequest, db: Session = Depends(get_db)): - user_id = decode_refresh_token(req.refresh_token) - if not user_id: + payload = decode_refresh_token_payload(req.refresh_token) + if not payload or not payload.get("sub"): raise HTTPException(status_code=401, detail="Invalid or expired refresh token") - user = db.query(UserModel).filter(UserModel.id == uuid.UUID(user_id)).first() + try: + user_uuid = uuid.UUID(payload["sub"]) + except ValueError: + raise HTTPException(status_code=401, detail="Invalid or expired refresh token") + user = db.query(UserModel).filter(UserModel.id == user_uuid).first() if not user: raise HTTPException(status_code=401, detail="User not found") + # Refresh tokens live 30 days, so this is what actually stops a stolen one from + # being replayed into a fresh token after the owner resets their password. + if token_issued_before(payload, user.token_valid_after): + raise HTTPException(status_code=401, detail="Invalid or expired refresh token") user.last_active_at = datetime.now() db.commit() return { diff --git a/backend/routers/cards.py b/backend/routers/cards.py index 910bb31..c060a52 100644 --- a/backend/routers/cards.py +++ b/backend/routers/cards.py @@ -9,7 +9,7 @@ from sqlalchemy.orm import Session from game.card import _get_specific_card_async from core.database import get_db from services.database_functions import check_boosters, fill_card_pool, BOOSTER_MAX -from core.dependencies import get_current_user, limiter +from core.dependencies import escape_like, get_current_user, limiter, parse_uuid from core.models import Card as CardModel from core.models import Deck as DeckModel from core.models import DeckCard as DeckCardModel @@ -43,7 +43,7 @@ def get_cards( q = db.query(CardModel).filter(CardModel.user_id == user.id) if search: - q = q.filter(CardModel.name.ilike(f"%{search}%")) + q = q.filter(CardModel.name.ilike(f"%{escape_like(search)}%", escape="\\")) if rarities: q = q.filter(CardModel.card_rarity.in_(rarities)) if types: @@ -149,7 +149,7 @@ async def open_pack(request: Request, user: UserModel = Depends(get_current_user @router.post("/cards/{card_id}/report") def report_card(card_id: str, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): card = db.query(CardModel).filter( - CardModel.id == uuid.UUID(card_id), + CardModel.id == parse_uuid(card_id, "card_id"), CardModel.user_id == user.id ).first() if not card: @@ -163,7 +163,7 @@ def report_card(card_id: str, user: UserModel = Depends(get_current_user), db: S @limiter.limit("5/hour") async def refresh_card(request: Request, card_id: str, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): card = db.query(CardModel).filter( - CardModel.id == uuid.UUID(card_id), + CardModel.id == parse_uuid(card_id, "card_id"), CardModel.user_id == user.id ).first() if not card: @@ -206,7 +206,7 @@ async def refresh_card(request: Request, card_id: str, user: UserModel = Depends @router.post("/cards/{card_id}/favorite") def toggle_favorite(card_id: str, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): card = db.query(CardModel).filter( - CardModel.id == uuid.UUID(card_id), + CardModel.id == parse_uuid(card_id, "card_id"), CardModel.user_id == user.id ).first() if not card: @@ -219,7 +219,7 @@ def toggle_favorite(card_id: str, user: UserModel = Depends(get_current_user), d @router.post("/cards/{card_id}/willing-to-trade") def toggle_willing_to_trade(card_id: str, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): card = db.query(CardModel).filter( - CardModel.id == uuid.UUID(card_id), + CardModel.id == parse_uuid(card_id, "card_id"), CardModel.user_id == user.id ).first() if not card: diff --git a/backend/routers/decks.py b/backend/routers/decks.py index aae2647..0f0017a 100644 --- a/backend/routers/decks.py +++ b/backend/routers/decks.py @@ -1,13 +1,13 @@ import uuid from typing import List, Optional -from fastapi import APIRouter, Depends, HTTPException +from fastapi import APIRouter, Depends, HTTPException, Request from pydantic import BaseModel, Field from sqlalchemy.orm import Session, selectinload from game.card import compute_deck_type from core.database import get_db -from core.dependencies import get_current_user +from core.dependencies import get_current_user, limiter, parse_uuid from core.models import Card as CardModel from core.models import Deck as DeckModel from core.models import DeckCard as DeckCardModel @@ -18,7 +18,10 @@ router = APIRouter() class DeckUpdate(BaseModel): name: Optional[str] = Field(None, max_length=64) - card_ids: Optional[List[str]] = None + # The real constraint is the 50-cost rule enforced at game start, but cost is + # per-card so an unbounded list is still a cheap way to make us do arbitrary + # work. Cheapest cards are 1 cost, so 60 can't hide a legal deck. + card_ids: Optional[List[str]] = Field(None, max_length=60) @router.get("/decks") @@ -46,7 +49,8 @@ def get_decks(user: UserModel = Depends(get_current_user), db: Session = Depends @router.post("/decks") -def create_deck(user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): +@limiter.limit("30/minute") +def create_deck(request: Request, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): count = db.query(DeckModel).filter(DeckModel.user_id == user.id).count() deck = DeckModel(id=uuid.uuid4(), user_id=user.id, name=f"Deck #{count + 1}") db.add(deck) @@ -55,16 +59,36 @@ def create_deck(user: UserModel = Depends(get_current_user), db: Session = Depen @router.patch("/decks/{deck_id}") -def update_deck(deck_id: str, body: DeckUpdate, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): - deck = db.query(DeckModel).filter(DeckModel.id == uuid.UUID(deck_id), DeckModel.user_id == user.id).first() +# Generous: the deck editor saves on every change. +@limiter.limit("120/minute") +def update_deck(request: Request, deck_id: str, body: DeckUpdate, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): + deck = db.query(DeckModel).filter(DeckModel.id == parse_uuid(deck_id, "deck_id"), DeckModel.user_id == user.id).first() if not deck: raise HTTPException(status_code=404, detail="Deck not found") if body.name is not None: deck.name = body.name if body.card_ids is not None: + try: + parsed_ids = [uuid.UUID(cid) for cid in body.card_ids] + except ValueError: + raise HTTPException(status_code=400, detail="Invalid card IDs") + + # deck_cards is keyed on (deck_id, card_id), so a repeated id would blow up + # on flush. dict.fromkeys dedupes while keeping the submitted order. + unique_ids = list(dict.fromkeys(parsed_ids)) + + # Without this the only constraint is the FK, so any card UUID that exists + # is accepted — including another user's. Public profiles hand those out. + owned = db.query(CardModel.id).filter( + CardModel.id.in_(unique_ids), + CardModel.user_id == user.id, + ).all() + if len(owned) != len(unique_ids): + raise HTTPException(status_code=400, detail="Some cards are not in your collection") + db.query(DeckCardModel).filter(DeckCardModel.deck_id == deck.id).delete() - for card_id in body.card_ids: - db.add(DeckCardModel(deck_id=deck.id, card_id=uuid.UUID(card_id))) + for card_id in unique_ids: + db.add(DeckCardModel(deck_id=deck.id, card_id=card_id)) if deck.times_played > 0: deck.wins = 0 deck.losses = 0 @@ -75,7 +99,7 @@ def update_deck(deck_id: str, body: DeckUpdate, user: UserModel = Depends(get_cu @router.delete("/decks/{deck_id}") def delete_deck(deck_id: str, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): - deck = db.query(DeckModel).filter(DeckModel.id == uuid.UUID(deck_id), DeckModel.user_id == user.id).first() + deck = db.query(DeckModel).filter(DeckModel.id == parse_uuid(deck_id, "deck_id"), DeckModel.user_id == user.id).first() if not deck: raise HTTPException(status_code=404, detail="Deck not found") if deck.times_played > 0: @@ -88,7 +112,7 @@ def delete_deck(deck_id: str, user: UserModel = Depends(get_current_user), db: S @router.get("/decks/{deck_id}/cards") def get_deck_cards(deck_id: str, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): - deck = db.query(DeckModel).filter(DeckModel.id == uuid.UUID(deck_id), DeckModel.user_id == user.id).first() + deck = db.query(DeckModel).filter(DeckModel.id == parse_uuid(deck_id, "deck_id"), DeckModel.user_id == user.id).first() if not deck: raise HTTPException(status_code=404, detail="Deck not found") deck_cards = db.query(DeckCardModel).options( diff --git a/backend/routers/friends.py b/backend/routers/friends.py index 0e5e292..4905c5d 100644 --- a/backend/routers/friends.py +++ b/backend/routers/friends.py @@ -5,7 +5,7 @@ from sqlalchemy.orm import Session, joinedload from services import notification_manager from core.database import get_db -from core.dependencies import get_current_user, get_user_id_from_request, limiter +from core.dependencies import get_current_user, limiter, parse_uuid from core.models import Friendship as FriendshipModel from core.models import Notification as NotificationModel from core.models import User as UserModel @@ -15,7 +15,7 @@ router = APIRouter() @router.post("/users/{username}/friend-request") -@limiter.limit("10/minute", key_func=get_user_id_from_request) +@limiter.limit("10/minute") async def send_friend_request(request: Request, username: str, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): addressee = db.query(UserModel).filter(UserModel.username == username).first() if not addressee: @@ -52,8 +52,9 @@ async def send_friend_request(request: Request, username: str, user: UserModel = @router.post("/friendships/{friendship_id}/accept") -def accept_friend_request(friendship_id: str, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): - friendship = db.query(FriendshipModel).filter(FriendshipModel.id == uuid.UUID(friendship_id)).first() +@limiter.limit("30/minute") +def accept_friend_request(request: Request, friendship_id: str, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): + friendship = db.query(FriendshipModel).filter(FriendshipModel.id == parse_uuid(friendship_id, "friendship_id")).first() if not friendship: raise HTTPException(status_code=404, detail="Friendship not found") if friendship.addressee_id != user.id: @@ -66,8 +67,9 @@ def accept_friend_request(friendship_id: str, user: UserModel = Depends(get_curr @router.post("/friendships/{friendship_id}/decline") -def decline_friend_request(friendship_id: str, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): - friendship = db.query(FriendshipModel).filter(FriendshipModel.id == uuid.UUID(friendship_id)).first() +@limiter.limit("30/minute") +def decline_friend_request(request: Request, friendship_id: str, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): + friendship = db.query(FriendshipModel).filter(FriendshipModel.id == parse_uuid(friendship_id, "friendship_id")).first() if not friendship: raise HTTPException(status_code=404, detail="Friendship not found") if friendship.addressee_id != user.id: @@ -124,7 +126,7 @@ def get_friendship_status(username: str, user: UserModel = Depends(get_current_u @router.delete("/friendships/{friendship_id}") def remove_friend(friendship_id: str, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): - friendship = db.query(FriendshipModel).filter(FriendshipModel.id == uuid.UUID(friendship_id)).first() + friendship = db.query(FriendshipModel).filter(FriendshipModel.id == parse_uuid(friendship_id, "friendship_id")).first() if not friendship: raise HTTPException(status_code=404, detail="Friendship not found") if friendship.requester_id != user.id and friendship.addressee_id != user.id: diff --git a/backend/routers/games.py b/backend/routers/games.py index 3324456..4039cfb 100644 --- a/backend/routers/games.py +++ b/backend/routers/games.py @@ -8,10 +8,9 @@ from sqlalchemy import func from sqlalchemy.orm import Session, joinedload from services import notification_manager -from core.auth import decode_access_token from core.database import get_db from services.database_functions import fill_card_pool -from core.dependencies import get_current_user, get_user_id_from_request, limiter +from core.dependencies import accept_and_authenticate_ws, get_current_user, limiter, parse_uuid from game.manager import ( QueueEntry, active_games, connections, create_challenge_game, create_solo_game, handle_action, handle_disconnect, handle_timeout_claim, load_deck_cards, @@ -28,6 +27,23 @@ from routers.notifications import _serialize_notification router = APIRouter() +DECK_MAX_COST = 50 + + +def deck_total_cost(deck_id: uuid.UUID, db: Session) -> int: + card_ids = [dc.card_id for dc in db.query(DeckCardModel).filter(DeckCardModel.deck_id == deck_id).all()] + return db.query(func.sum(CardModel.cost)).filter(CardModel.id.in_(card_ids)).scalar() or 0 + + +def deck_cost_error(deck_id: uuid.UUID, db: Session, label: str = "Deck") -> str | None: + """The 50-cost rule. Enforced at game start rather than at deck save, so every path + into a game has to check it — the challenge-accept path previously didn't.""" + total = deck_total_cost(deck_id, db) + if total == 0 or total > DECK_MAX_COST: + return f"{label} total cost must be between 1 and {DECK_MAX_COST}" + return None + + def _serialize_challenge(c: GameChallengeModel, current_user_id: uuid.UUID) -> dict: deck = c.challenger_deck return { @@ -47,16 +63,22 @@ def _serialize_challenge(c: GameChallengeModel, current_user_id: uuid.UUID) -> d @router.websocket("/ws/queue") async def queue_endpoint(websocket: WebSocket, deck_id: str, db: Session = Depends(get_db)): - await websocket.accept() - - token = await websocket.receive_text() - user_id = decode_access_token(token) + user_id = await accept_and_authenticate_ws(websocket) if not user_id: + return + + # deck_id is a client-supplied query param. parse_uuid raises HTTPException, which + # means nothing on an open socket, so validate here and report it the way this + # handler reports everything else. + try: + parsed_deck_id = uuid.UUID(deck_id) + except ValueError: + await websocket.send_json({"type": "error", "message": "Invalid deck_id"}) await websocket.close(code=1008) return deck = db.query(DeckModel).filter( - DeckModel.id == uuid.UUID(deck_id), + DeckModel.id == parsed_deck_id, DeckModel.user_id == uuid.UUID(user_id) ).first() @@ -65,10 +87,9 @@ async def queue_endpoint(websocket: WebSocket, deck_id: str, db: Session = Depen await websocket.close(code=1008) return - card_ids = [dc.card_id for dc in db.query(DeckCardModel).filter(DeckCardModel.deck_id == deck.id).all()] - total_cost = db.query(func.sum(CardModel.cost)).filter(CardModel.id.in_(card_ids)).scalar() or 0 - if total_cost == 0 or total_cost > 50: - await websocket.send_json({"type": "error", "message": "Deck total cost must be between 1 and 50"}) + cost_err = deck_cost_error(deck.id, db) + if cost_err: + await websocket.send_json({"type": "error", "message": cost_err}) await websocket.close(code=1008) return @@ -91,12 +112,8 @@ async def queue_endpoint(websocket: WebSocket, deck_id: str, db: Session = Depen @router.websocket("/ws/game/{game_id}") async def game_endpoint(websocket: WebSocket, game_id: str, db: Session = Depends(get_db)): - await websocket.accept() - - token = await websocket.receive_text() - user_id = decode_access_token(token) + user_id = await accept_and_authenticate_ws(websocket) if not user_id: - await websocket.close(code=1008) return if game_id not in active_games: @@ -136,7 +153,7 @@ class AcceptGameChallengeRequest(BaseModel): @router.post("/users/{username}/challenge") -@limiter.limit("10/minute", key_func=get_user_id_from_request) +@limiter.limit("10/minute") async def create_game_challenge( request: Request, username: str, @@ -197,7 +214,9 @@ async def create_game_challenge( @router.post("/challenges/{challenge_id}/accept") +@limiter.limit("20/minute") async def accept_game_challenge( + request: Request, challenge_id: str, req: AcceptGameChallengeRequest, user: UserModel = Depends(get_current_user), @@ -239,6 +258,12 @@ async def accept_game_challenge( if not challenger_deck: raise HTTPException(status_code=400, detail="The challenger's deck no longer exists") + # Both decks, because neither was cost-checked when the challenge was created. + for d, label in ((deck, "Your deck"), (challenger_deck, "The challenger's deck")): + cost_err = deck_cost_error(d.id, db, label) + if cost_err: + raise HTTPException(status_code=400, detail=cost_err) + try: game_id = create_challenge_game( str(challenge.challenger_id), str(challenge.challenger_deck_id), @@ -282,7 +307,9 @@ async def accept_game_challenge( @router.post("/challenges/{challenge_id}/decline") +@limiter.limit("20/minute") async def decline_game_challenge( + request: Request, challenge_id: str, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db), @@ -367,21 +394,23 @@ async def claim_timeout_win(game_id: str, user: UserModel = Depends(get_current_ @router.post("/game/solo") -async def start_solo_game(deck_id: str, difficulty: int = 5, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): +# Tightest limit in the app: every call reserves 500 pool cards and leaves a game in +# memory that nothing reaps, so a loop here drains the shared pool and grows the heap. +@limiter.limit("10/hour") +async def start_solo_game(request: Request, deck_id: str, difficulty: int = 5, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): if difficulty < 1 or difficulty > 10: raise HTTPException(status_code=400, detail="Difficulty must be between 1 and 10") deck = db.query(DeckModel).filter( - DeckModel.id == uuid.UUID(deck_id), + DeckModel.id == parse_uuid(deck_id, "deck_id"), DeckModel.user_id == user.id ).first() if not deck: raise HTTPException(status_code=404, detail="Deck not found") - card_ids = [dc.card_id for dc in db.query(DeckCardModel).filter(DeckCardModel.deck_id == deck.id).all()] - total_cost = db.query(func.sum(CardModel.cost)).filter(CardModel.id.in_(card_ids)).scalar() or 0 - if total_cost == 0 or total_cost > 50: - raise HTTPException(status_code=400, detail="Deck total cost must be between 1 and 50") + cost_err = deck_cost_error(deck.id, db) + if cost_err: + raise HTTPException(status_code=400, detail=cost_err) player_cards = load_deck_cards(deck_id, str(user.id), db) if player_cards is None: diff --git a/backend/routers/notifications.py b/backend/routers/notifications.py index 30788fd..c973cd6 100644 --- a/backend/routers/notifications.py +++ b/backend/routers/notifications.py @@ -1,13 +1,12 @@ import uuid from datetime import datetime -from fastapi import APIRouter, Depends, HTTPException, WebSocket, WebSocketDisconnect +from fastapi import APIRouter, Depends, HTTPException, Request, WebSocket, WebSocketDisconnect from sqlalchemy.orm import Session from services import notification_manager -from core.auth import decode_access_token from core.database import get_db -from core.dependencies import get_current_user +from core.dependencies import accept_and_authenticate_ws, get_current_user, limiter, parse_uuid from core.models import Notification as NotificationModel from core.models import User as UserModel @@ -27,12 +26,8 @@ def _serialize_notification(n: NotificationModel) -> dict: @router.websocket("/ws/notifications") async def notifications_endpoint(websocket: WebSocket, db: Session = Depends(get_db)): - await websocket.accept() - - token = await websocket.receive_text() - user_id = decode_access_token(token) + user_id = await accept_and_authenticate_ws(websocket) if not user_id: - await websocket.close(code=1008) return user = db.query(UserModel).filter(UserModel.id == uuid.UUID(user_id)).first() @@ -82,13 +77,15 @@ def get_notifications(user: UserModel = Depends(get_current_user), db: Session = @router.post("/notifications/{notification_id}/read") +@limiter.limit("120/minute") def mark_notification_read( + request: Request, notification_id: str, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db), ): n = db.query(NotificationModel).filter( - NotificationModel.id == uuid.UUID(notification_id), + NotificationModel.id == parse_uuid(notification_id, "notification_id"), NotificationModel.user_id == user.id, ).first() if not n: @@ -99,13 +96,15 @@ def mark_notification_read( @router.delete("/notifications/{notification_id}") +@limiter.limit("120/minute") def delete_notification( + request: Request, notification_id: str, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db), ): n = db.query(NotificationModel).filter( - NotificationModel.id == uuid.UUID(notification_id), + NotificationModel.id == parse_uuid(notification_id, "notification_id"), NotificationModel.user_id == user.id, ).first() if not n: diff --git a/backend/routers/profile.py b/backend/routers/profile.py index e6119f8..fe6d56e 100644 --- a/backend/routers/profile.py +++ b/backend/routers/profile.py @@ -1,11 +1,11 @@ from datetime import datetime, timedelta -from fastapi import APIRouter, Depends, HTTPException +from fastapi import APIRouter, Depends, HTTPException, Request from pydantic import BaseModel from sqlalchemy.orm import Session from core.database import get_db -from core.dependencies import get_current_user +from core.dependencies import escape_like, get_current_user, limiter from core.models import Card as CardModel from core.models import Deck as DeckModel from core.models import User as UserModel @@ -96,13 +96,14 @@ def update_profile(req: UpdateProfileRequest, user: UserModel = Depends(get_curr @router.get("/users") -def search_users(q: str, current_user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): +@limiter.limit("60/minute") +def search_users(request: Request, q: str, current_user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): # Require auth to prevent scraping if len(q) < 2: return [] results = ( db.query(UserModel) - .filter(UserModel.username.ilike(f"%{q}%")) + .filter(UserModel.username.ilike(f"%{escape_like(q)}%", escape="\\")) .limit(20) .all() ) @@ -117,8 +118,17 @@ def search_users(q: str, current_user: UserModel = Depends(get_current_user), db ] +# Cap on cards returned per section. These lists are unbounded per user, and the +# frontend only shows one row until you expand it. +PUBLIC_PROFILE_CARD_LIMIT = 60 + + @router.get("/users/{username}") -def get_public_profile(username: str, db: Session = Depends(get_db)): +# Auth alone doesn't stop enumeration — one account can still walk every profile. +@limiter.limit("60/minute") +def get_public_profile(request: Request, username: str, current_user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): + # Require auth to prevent scraping, same as /users?q= above. This returns + # strictly more than the search endpoint does, including card ids. user = db.query(UserModel).filter(UserModel.username == username).first() if not user: raise HTTPException(status_code=404, detail="User not found") @@ -127,12 +137,14 @@ def get_public_profile(username: str, db: Session = Depends(get_db)): db.query(CardModel) .filter(CardModel.user_id == user.id, CardModel.is_favorite == True) .order_by(CardModel.received_at.desc()) + .limit(PUBLIC_PROFILE_CARD_LIMIT) .all() ) wtt_cards = ( db.query(CardModel) .filter(CardModel.user_id == user.id, CardModel.willing_to_trade == True) .order_by(CardModel.received_at.desc()) + .limit(PUBLIC_PROFILE_CARD_LIMIT) .all() ) return { diff --git a/backend/routers/store.py b/backend/routers/store.py index 08b6938..3c85ab4 100644 --- a/backend/routers/store.py +++ b/backend/routers/store.py @@ -52,7 +52,8 @@ class BuySpecificCardRequest(BaseModel): @router.post("/shards/shatter") -def shatter_cards(req: ShatterRequest, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): +@limiter.limit("30/minute") +def shatter_cards(request: Request, req: ShatterRequest, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): if not req.card_ids: raise HTTPException(status_code=400, detail="No cards selected") try: @@ -79,7 +80,9 @@ def shatter_cards(req: ShatterRequest, user: UserModel = Depends(get_current_use @router.post("/store/stripe/checkout") -def create_stripe_checkout(req: StripeCheckoutRequest, user: UserModel = Depends(get_current_user)): +# Each call creates a Stripe session, so this is spend against our Stripe account. +@limiter.limit("10/minute") +def create_stripe_checkout(request: Request, req: StripeCheckoutRequest, user: UserModel = Depends(get_current_user)): package = SHARD_PACKAGES.get(req.package_id) if not package: raise HTTPException(status_code=400, detail="Invalid package") @@ -122,6 +125,13 @@ async def stripe_webhook(request: Request, db: Session = Depends(get_db)): if event["type"] == "checkout.session.completed": data = event["data"]["object"] + # checkout.session.completed also fires for asynchronous payment methods, where + # payment_status is "unpaid" until the funds actually settle. Sessions are created + # card-only today so that can't happen yet, but crediting shards for money that + # may never arrive is not something to leave resting on a Stripe dashboard toggle. + if data.get("payment_status") != "paid": + db.commit() + return {"ok": True} user_id = data.get("metadata", {}).get("user_id") shards = data.get("metadata", {}).get("shards") if user_id and shards: @@ -177,7 +187,8 @@ async def buy_specific_card(request: Request, req: BuySpecificCardRequest, user: @router.post("/store/buy") -def store_buy(req: StoreBuyRequest, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): +@limiter.limit("30/minute") +def store_buy(request: Request, req: StoreBuyRequest, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db)): cost = STORE_PACKAGES.get(req.quantity) if cost is None: raise HTTPException(status_code=400, detail="Invalid package") diff --git a/backend/routers/trades.py b/backend/routers/trades.py index 7f2a1f6..35449fa 100644 --- a/backend/routers/trades.py +++ b/backend/routers/trades.py @@ -6,9 +6,8 @@ from pydantic import BaseModel from sqlalchemy.orm import Session from services import notification_manager -from core.auth import decode_access_token from core.database import get_db -from core.dependencies import get_current_user, get_user_id_from_request, limiter +from core.dependencies import accept_and_authenticate_ws, get_current_user, limiter, parse_uuid from core.models import Card as CardModel from core.models import Notification as NotificationModel from core.models import TradeProposal as TradeProposalModel @@ -64,12 +63,8 @@ def _serialize_proposal(p: TradeProposalModel, current_user_id: uuid.UUID, card_ @router.websocket("/ws/trade/queue") async def trade_queue_endpoint(websocket: WebSocket, db: Session = Depends(get_db)): - await websocket.accept() - - token = await websocket.receive_text() - user_id = decode_access_token(token) + user_id = await accept_and_authenticate_ws(websocket) if not user_id: - await websocket.close(code=1008) return user = db.query(UserModel).filter(UserModel.id == uuid.UUID(user_id)).first() @@ -99,12 +94,8 @@ async def trade_queue_endpoint(websocket: WebSocket, db: Session = Depends(get_d @router.websocket("/ws/trade/{trade_id}") async def trade_endpoint(websocket: WebSocket, trade_id: str, db: Session = Depends(get_db)): - await websocket.accept() - - token = await websocket.receive_text() - user_id = decode_access_token(token) + user_id = await accept_and_authenticate_ws(websocket) if not user_id: - await websocket.close(code=1008) return session = active_trades.get(trade_id) @@ -138,7 +129,7 @@ class CreateTradeProposalRequest(BaseModel): @router.post("/trade-proposals") -@limiter.limit("10/minute", key_func=get_user_id_from_request) +@limiter.limit("10/minute") async def create_trade_proposal( request: Request, req: CreateTradeProposalRequest, @@ -222,10 +213,7 @@ def get_trade_proposal( user: UserModel = Depends(get_current_user), db: Session = Depends(get_db), ): - try: - pid = uuid.UUID(proposal_id) - except ValueError: - raise HTTPException(status_code=400, detail="Invalid proposal ID") + pid = parse_uuid(proposal_id, "proposal_id") proposal = db.query(TradeProposalModel).filter(TradeProposalModel.id == pid).first() if not proposal: raise HTTPException(status_code=404, detail="Proposal not found") @@ -241,12 +229,14 @@ def get_trade_proposal( @router.post("/trade-proposals/{proposal_id}/accept") +@limiter.limit("30/minute") async def accept_trade_proposal( + request: Request, proposal_id: str, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db), ): - proposal = db.query(TradeProposalModel).filter(TradeProposalModel.id == uuid.UUID(proposal_id)).with_for_update().first() + proposal = db.query(TradeProposalModel).filter(TradeProposalModel.id == parse_uuid(proposal_id, "proposal_id")).with_for_update().first() if not proposal: raise HTTPException(status_code=404, detail="Proposal not found") if proposal.recipient_id != user.id: @@ -339,12 +329,14 @@ async def accept_trade_proposal( @router.post("/trade-proposals/{proposal_id}/decline") +@limiter.limit("30/minute") async def decline_trade_proposal( + request: Request, proposal_id: str, user: UserModel = Depends(get_current_user), db: Session = Depends(get_db), ): - proposal = db.query(TradeProposalModel).filter(TradeProposalModel.id == uuid.UUID(proposal_id)).first() + proposal = db.query(TradeProposalModel).filter(TradeProposalModel.id == parse_uuid(proposal_id, "proposal_id")).first() if not proposal: raise HTTPException(status_code=404, detail="Proposal not found") if proposal.proposer_id != user.id and proposal.recipient_id != user.id: diff --git a/backend/services/email_utils.py b/backend/services/email_utils.py index 1ed7f1c..d3e1cc4 100644 --- a/backend/services/email_utils.py +++ b/backend/services/email_utils.py @@ -1,4 +1,4 @@ -import os +import html import resend @@ -6,6 +6,8 @@ from core.config import RESEND_API_KEY, EMAIL_FROM, FRONTEND_URL def send_verification_email(to_email: str, username: str, token: str): resend.api_key = RESEND_API_KEY + # Usernames are only length-checked at registration, so they can contain markup. + username = html.escape(username) verify_url = f"{FRONTEND_URL}/verify-email?token={token}" resend.Emails.send({ @@ -31,6 +33,7 @@ def send_verification_email(to_email: str, username: str, token: str): def send_password_reset_email(to_email: str, username: str, reset_token: str): resend.api_key = RESEND_API_KEY + username = html.escape(username) reset_url = f"{FRONTEND_URL}/forgot-password/reset?token={reset_token}" resend.Emails.send({ diff --git a/frontend/src/routes/profile/[username]/+page.svelte b/frontend/src/routes/profile/[username]/+page.svelte index d158abe..ed18bf2 100644 --- a/frontend/src/routes/profile/[username]/+page.svelte +++ b/frontend/src/routes/profile/[username]/+page.svelte @@ -24,6 +24,7 @@ let profile: any = $state(null); let loading = $state(true); let notFound = $state(false); + let needsAuth = $state(false); let favExpanded = $state(false); let wttExpanded = $state(false); @@ -114,32 +115,36 @@ const username = get(page).params.username; const token = localStorage.getItem('token'); - const res = await fetch(`${API_URL}/users/${username}`); + // Profiles now require auth. Show a prompt rather than bouncing to /auth, so + // a shared profile link explains itself instead of silently vanishing. + if (!token) { needsAuth = true; loading = false; return; } + + // apiFetch (not raw fetch) so an expired access token gets refreshed + const res = await apiFetch(`${API_URL}/users/${username}`); + if (res.status === 401) { needsAuth = true; loading = false; return; } if (res.status === 404) { notFound = true; loading = false; return; } const data = await res.json(); // Redirect to own profile if logged-in user visits their own public page - if (token) { - try { - const meRes = await fetch(`${API_URL}/profile`, { - headers: { Authorization: `Bearer ${token}` } - }); - if (meRes.ok) { - const me = await meRes.json(); - if (me.username === username) { goto('/profile'); return; } - isLoggedIn = true; + try { + const meRes = await fetch(`${API_URL}/profile`, { + headers: { Authorization: `Bearer ${token}` } + }); + if (meRes.ok) { + const me = await meRes.json(); + if (me.username === username) { goto('/profile'); return; } + isLoggedIn = true; - // Check existing friendship status - const statusRes = await apiFetch(`${API_URL}/friendship-status/${username}`); - if (statusRes.ok) { - const s = await statusRes.json(); - if (s.status === 'friends') { friendStatus = 'friends'; friendshipId = s.friendship_id; } - else if (s.status === 'pending_sent') { friendStatus = 'pending'; } - else if (s.status === 'pending_received') { friendStatus = 'pending_received'; friendshipId = s.friendship_id; } - } + // Check existing friendship status + const statusRes = await apiFetch(`${API_URL}/friendship-status/${username}`); + if (statusRes.ok) { + const s = await statusRes.json(); + if (s.status === 'friends') { friendStatus = 'friends'; friendshipId = s.friendship_id; } + else if (s.status === 'pending_sent') { friendStatus = 'pending'; } + else if (s.status === 'pending_received') { friendStatus = 'pending_received'; friendshipId = s.friendship_id; } } - } catch { /* non-critical */ } - } + } + } catch { /* non-critical */ } profile = data; loading = false; @@ -151,6 +156,16 @@
Loading...
+{:else if needsAuth} +