From 6a829270d2aea6d56893b3b2ed24519857a6d9a5 Mon Sep 17 00:00:00 2001 From: Workman Bot Date: Thu, 23 Apr 2026 13:44:36 +0000 Subject: [PATCH 1/2] fix: resolve issue #199 - Security: Implement Rate Limiting for Payment Endpoints --- backend/app/api/v1/endpoints/payments.py | 76 ++++++++++++++++++++++-- backend/app/core/rate_limit.py | 64 ++++++++++++++++++++ backend/app/main.py | 11 ++++ backend/requirements.txt | 1 + 4 files changed, 148 insertions(+), 4 deletions(-) create mode 100644 backend/app/core/rate_limit.py diff --git a/backend/app/api/v1/endpoints/payments.py b/backend/app/api/v1/endpoints/payments.py index 16c2a43..81ac498 100644 --- a/backend/app/api/v1/endpoints/payments.py +++ b/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): @@ -47,13 +57,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), @@ -88,11 +143,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( @@ -113,6 +173,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 @@ -128,6 +189,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", @@ -138,13 +200,16 @@ 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", @@ -152,12 +217,14 @@ def submit( 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")) @@ -165,7 +232,8 @@ def release(req: ReleaseRequest, db: Session = Depends(get_db)): @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")) diff --git a/backend/app/core/rate_limit.py b/backend/app/core/rate_limit.py new file mode 100644 index 0000000..a88d004 --- /dev/null +++ b/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", +] diff --git a/backend/app/main.py b/backend/app/main.py index 40ce0c2..bc61f6d 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -2,12 +2,16 @@ from fastapi import Depends, FastAPI from fastapi.middleware.cors import CORSMiddleware +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.rate_limit import limiter from app.db.session import get_db @@ -27,6 +31,13 @@ async def lifespan(app: FastAPI): lifespan=lifespan, ) +# Rate limiting (slowapi). The limiter is attached to app state so the +# decorators on individual endpoints can resolve it, and a middleware +# dispatches requests through it. +app.state.limiter = limiter +app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler) +app.add_middleware(SlowAPIMiddleware) + # Set all CORS enabled origins if settings.BACKEND_CORS_ORIGINS: app.add_middleware( diff --git a/backend/requirements.txt b/backend/requirements.txt index 797734b..ef9b571 100644 --- a/backend/requirements.txt +++ b/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 From 07c652be6811f8a0a624b98c52ff635d7c78bd2d Mon Sep 17 00:00:00 2001 From: Kelechukwu Izuaba <144479895+Kaycee276@users.noreply.github.com> Date: Wed, 29 Apr 2026 14:19:21 +0100 Subject: [PATCH 2/2] Add missing import statements in main.py --- backend/app/main.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/backend/app/main.py b/backend/app/main.py index 653a119..678a9b1 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -1,5 +1,6 @@ import os from contextlib import asynccontextmanager + from fastapi import Depends, FastAPI from fastapi.middleware.cors import CORSMiddleware from fastapi.staticfiles import StaticFiles @@ -8,6 +9,7 @@ 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