fix: Security: Implement Rate Limiting for Payment Endpoints

backend/app/api/v1/endpoints/payments.py

@@ -2,13 +2,22 @@
 import uuid
 from decimal import Decimal
 
-from fastapi import APIRouter, Depends, HTTPException, status
+from fastapi import APIRouter, Depends, HTTPException, Request, status
+from limits import parse as parse_limit
 from pydantic import BaseModel, Field
 from sqlalchemy.orm import Session
 from stellar_sdk import TransactionEnvelope
 
 from app.core.auth import require_client
 from app.core.config import settings
+from app.core.rate_limit import (
+    PAYMENT_PREPARE_LIMIT,
+    PAYMENT_REFUND_LIMIT,
+    PAYMENT_RELEASE_LIMIT,
+    PAYMENT_SUBMIT_FAILED_LIMIT,
+    PAYMENT_SUBMIT_LIMIT,
+    limiter,
+)
 from app.db.session import get_db
 from app.models.booking import Booking
 from app.models.user import User
@@ -22,7 +31,8 @@
 
 router = APIRouter()
 
-# deprecated: used by the insecure /hold endpoint which has been removed
+# Bucket name used for the stricter failed-submit rate limit.
+_FAILED_SUBMIT_BUCKET = "payments:submit:failed"
 
 
 class PrepareRequest(BaseModel):
@@ -49,13 +59,58 @@ class RefundRequest(BaseModel):
     amount: Decimal = Field(..., gt=0)
 
 
+def _failed_submit_item():
+    return parse_limit(PAYMENT_SUBMIT_FAILED_LIMIT)
+
+
+def _record_failed_submit(request: Request) -> None:
+    """Record a failed submission against the stricter failed-submit bucket.
+
+    Applies a tighter limit on erroring submissions (e.g. invalid XDR, unknown
+    booking) to blunt brute-force attempts without punishing legitimate users
+    who occasionally fail.
+    """
+    lim = getattr(request.app.state, "limiter", None)
+    if lim is None:
+        return
+    try:
+        key = lim._key_func(request)
+        lim._limiter.hit(_failed_submit_item(), _FAILED_SUBMIT_BUCKET, key)
+    except Exception:
+        # Never let rate-limit bookkeeping break the request flow.
+        pass
+
+
+def _check_failed_submit_quota(request: Request) -> None:
+    """Reject further calls when the failed-submit bucket is exhausted."""
+    lim = getattr(request.app.state, "limiter", None)
+    if lim is None:
+        return
+    try:
+        key = lim._key_func(request)
+        allowed = lim._limiter.test(_failed_submit_item(), _FAILED_SUBMIT_BUCKET, key)
+    except Exception:
+        # If the backend is unavailable, fail open rather than block users.
+        return
+    if not allowed:
+        raise HTTPException(
+            status_code=status.HTTP_429_TOO_MANY_REQUESTS,
+            detail=(
+                "Too many failed payment submissions. Please wait "
+                "before trying again."
+            ),
+        )
+
+
 # The old /hold endpoint has been removed due to security concerns. Clients
 # should use the two-step prepare/submit flow instead.  A request to this path
 # will now return 404 (FastAPI simply won't register it).
 
 
 @router.post("/prepare", summary="Prepare unsigned payment XDR for client signing")
+@limiter.limit(PAYMENT_PREPARE_LIMIT)
 def prepare(
+    request: Request,
     req: PrepareRequest,
     db: Session = Depends(get_db),
     current_user: User = Depends(require_client),
@@ -103,11 +158,16 @@ def prepare(
 
 
 @router.post("/submit", summary="Submit signed payment XDR from wallet")
+@limiter.limit(PAYMENT_SUBMIT_LIMIT)
 def submit(
+    request: Request,
     req: SubmitRequest,
     db: Session = Depends(get_db),
     current_user: User = Depends(require_client),
 ):
+    # Stricter limit for repeatedly-failing submissions.
+    _check_failed_submit_quota(request)
+
     # Require verified email before submitting payments (configurable)
     if settings.REQUIRE_EMAIL_VERIFICATION and not current_user.is_verified:
         raise HTTPException(
@@ -128,6 +188,7 @@ def submit(
             memo_text = memo_text.decode()
         booking_token = memo_text.replace("hold-", "")
     except Exception:
+        _record_failed_submit(request)
         raise HTTPException(
             status_code=400, detail="Invalid signed transaction XDR"
         ) from None
@@ -143,6 +204,7 @@ def submit(
             if str(row[0]).startswith(booking_token)
         ]
         if len(candidates) != 1:
+            _record_failed_submit(request)
             raise HTTPException(
                 status_code=400,
                 detail="Unable to resolve booking from transaction memo",
@@ -153,34 +215,40 @@ def submit(
     try:
         booking_uuid = uuid.UUID(str(booking_id))
     except ValueError:
+        _record_failed_submit(request)
         raise HTTPException(status_code=404, detail="Booking not found") from None
 
     booking = db.query(Booking).filter(Booking.id == booking_uuid).first()
     if not booking:
+        _record_failed_submit(request)
         raise HTTPException(status_code=404, detail="Booking not found")
 
     if booking.client.user_id != current_user.id:
+        _record_failed_submit(request)
         raise HTTPException(
             status_code=status.HTTP_403_FORBIDDEN,
             detail="You are not authorized to submit payment for this booking",
         )
 
     res = submit_signed_payment(db, req.signed_xdr)
     if res.get("status") == "error":
+        _record_failed_submit(request)
         raise HTTPException(status_code=400, detail=res.get("message"))
     return res
 
 
 @router.post("/release", summary="Release escrow to artisan")
-def release(req: ReleaseRequest, db: Session = Depends(get_db)):
+@limiter.limit(PAYMENT_RELEASE_LIMIT)
+def release(request: Request, req: ReleaseRequest, db: Session = Depends(get_db)):
     res = release_payment(db, req.booking_id, req.artisan_public, req.amount)
     if res.get("status") == "error":
         raise HTTPException(status_code=400, detail=res.get("message"))
     return res
 
 
 @router.post("/refund", summary="Refund escrow to client")
-def refund(req: RefundRequest, db: Session = Depends(get_db)):
+@limiter.limit(PAYMENT_REFUND_LIMIT)
+def refund(request: Request, req: RefundRequest, db: Session = Depends(get_db)):
     res = refund_payment(db, req.booking_id, req.client_public, req.amount)
     if res.get("status") == "error":
         raise HTTPException(status_code=400, detail=res.get("message"))

backend/app/core/rate_limit.py

@@ -0,0 +1,64 @@
+"""Rate limiting configuration using slowapi.
+
+This module centralises the ``Limiter`` instance used to protect sensitive
+endpoints (currently the payment flow) from brute-force ``booking_id``
+enumeration and Stellar-network spam.
+
+Limits can be overridden via environment variables so operators can tune
+the values without a code change.
+"""
+
+from __future__ import annotations
+
+import os
+
+from slowapi import Limiter
+from slowapi.errors import RateLimitExceeded
+from slowapi.util import get_remote_address
+from starlette.requests import Request
+
+
+def _user_or_ip(request: Request) -> str:
+    """Key rate limits by authenticated user id when available, else client IP.
+
+    This prevents a single malicious user from evading per-IP limits by
+    rotating IPs when authenticated, and prevents shared NATs from all being
+    counted together when users are logged in.
+    """
+    user = getattr(request.state, "user", None)
+    if user is not None:
+        user_id = getattr(user, "id", None)
+        if user_id is not None:
+            return f"user:{user_id}"
+    # Authorization header fallback so requests with a JWT that hasn't been
+    # resolved yet still bucket per-token rather than per-IP.
+    auth = request.headers.get("authorization")
+    if auth:
+        return f"auth:{auth}"
+    return get_remote_address(request)
+
+
+# Default limits (overridable via env vars).
+PAYMENT_PREPARE_LIMIT: str = os.getenv("RATE_LIMIT_PAYMENT_PREPARE", "10/minute")
+PAYMENT_SUBMIT_LIMIT: str = os.getenv("RATE_LIMIT_PAYMENT_SUBMIT", "5/minute")
+PAYMENT_SUBMIT_FAILED_LIMIT: str = os.getenv(
+    "RATE_LIMIT_PAYMENT_SUBMIT_FAILED", "3/minute"
+)
+PAYMENT_RELEASE_LIMIT: str = os.getenv("RATE_LIMIT_PAYMENT_RELEASE", "10/minute")
+PAYMENT_REFUND_LIMIT: str = os.getenv("RATE_LIMIT_PAYMENT_REFUND", "10/minute")
+
+
+# headers_enabled=False because several endpoints return plain dicts rather
+# than Response objects; slowapi's header-injection helper only accepts a
+# starlette Response. The SlowAPIMiddleware still enforces the limits.
+limiter = Limiter(key_func=_user_or_ip, headers_enabled=False)
+
+__all__ = [
+    "limiter",
+    "RateLimitExceeded",
+    "PAYMENT_PREPARE_LIMIT",
+    "PAYMENT_SUBMIT_LIMIT",
+    "PAYMENT_SUBMIT_FAILED_LIMIT",
+    "PAYMENT_RELEASE_LIMIT",
+    "PAYMENT_REFUND_LIMIT",
+]

backend/app/main.py

@@ -4,24 +4,24 @@
 from fastapi import Depends, FastAPI
 from fastapi.middleware.cors import CORSMiddleware
 from fastapi.staticfiles import StaticFiles
+from slowapi import _rate_limit_exceeded_handler
+from slowapi.errors import RateLimitExceeded
+from slowapi.middleware import SlowAPIMiddleware
 from sqlalchemy import text
 from sqlalchemy.orm import Session
 
 from app.api.v1.api import api_router
 from app.core.cache import cache
 from app.core.config import settings
 from app.core.exceptions import register_exception_handlers
+from app.core.rate_limit import limiter
 from app.db.session import get_db
 
-
 @asynccontextmanager
 async def lifespan(app: FastAPI):
-    # Startup
-    await cache.initialize()  # Cambio: connect() -> initialize()
+    await cache.initialize()
     yield
-    # Shutdown
-    await cache.close()  # Cambio: disconnect() -> close()
-
+    await cache.close()
 
 app = FastAPI(
     title=settings.PROJECT_NAME,
@@ -30,19 +30,22 @@ async def lifespan(app: FastAPI):
     lifespan=lifespan,
 )
 
-# Ensure static/avatars directory exists
+# Rate limiting
+app.state.limiter = limiter
+app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler)
+app.add_middleware(SlowAPIMiddleware)
+
+# Static files
 static_path = os.path.join(os.getcwd(), settings.STATIC_DIR)
 avatars_path = os.path.join(static_path, settings.AVATARS_DIR)
 if not os.path.exists(avatars_path):
     os.makedirs(avatars_path)
-
 app.mount(f"/{settings.STATIC_DIR}", StaticFiles(directory=static_path), name="static")
 
-# Register global exception handlers to ensure every error response follows
-# the standardized { error_code, message, details } schema.
+# Global exception handlers
 register_exception_handlers(app)
 
-# Set all CORS enabled origins
+# CORS
 if settings.BACKEND_CORS_ORIGINS:
     app.add_middleware(
         CORSMiddleware,
@@ -52,7 +55,6 @@ async def lifespan(app: FastAPI):
         allow_headers=["*"],
     )
 
-# Include API router
 app.include_router(api_router, prefix=settings.API_V1_STR)
 
 

backend/requirements.txt

@@ -21,3 +21,4 @@ aiohttp==3.9.1
 stellar-sdk==13.1.0
 argon2-cffi==23.1.0
 fastapi-mail==1.4.1
+slowapi==0.1.9