fix(api): все роутеры за авторизацией, отказ от слабого JWT_SECRET, лимитер и X-Forwarded-For
get_current_user навешивается на роутер целиком, публичными остаются только health и auth (список закреплён tests/api/test_route_auth.py). API не стартует с плейсхолдером или коротким JWT_SECRET без ALLOW_DEV_SECRET=1. Лимитер логинов ограничен по числу ключей. Клиентский IP берётся из последнего хопа X-Forwarded-For.
This commit is contained in:
@@ -10,6 +10,7 @@ DATABASE_URL=postgresql+asyncpg://fintracker:change-me@db:5432/fintracker
|
|||||||
# --- auth ---
|
# --- auth ---
|
||||||
# 32+ random bytes: `openssl rand -hex 32`
|
# 32+ random bytes: `openssl rand -hex 32`
|
||||||
JWT_SECRET=change-me
|
JWT_SECRET=change-me
|
||||||
|
# the API refuses to start with a placeholder/short secret unless ALLOW_DEV_SECRET=1 (dev only)
|
||||||
ACCESS_TOKEN_TTL_SECONDS=3600
|
ACCESS_TOKEN_TTL_SECONDS=3600
|
||||||
REFRESH_TOKEN_TTL_SECONDS=2592000
|
REFRESH_TOKEN_TTL_SECONDS=2592000
|
||||||
|
|
||||||
|
|||||||
@@ -7,12 +7,13 @@ from collections.abc import AsyncIterator
|
|||||||
from contextlib import asynccontextmanager
|
from contextlib import asynccontextmanager
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
from fastapi import FastAPI
|
from fastapi import Depends, FastAPI
|
||||||
from fastapi.middleware.cors import CORSMiddleware
|
from fastapi.middleware.cors import CORSMiddleware
|
||||||
from fastapi.openapi.utils import get_openapi
|
from fastapi.openapi.utils import get_openapi
|
||||||
from fastapi.routing import APIRoute
|
from fastapi.routing import APIRoute
|
||||||
|
|
||||||
from fintracker import __version__
|
from fintracker import __version__
|
||||||
|
from fintracker.api.deps import get_current_user
|
||||||
from fintracker.api.errors import install_error_handlers
|
from fintracker.api.errors import install_error_handlers
|
||||||
from fintracker.api.routers import (
|
from fintracker.api.routers import (
|
||||||
accounts,
|
accounts,
|
||||||
@@ -44,6 +45,10 @@ log = logging.getLogger(__name__)
|
|||||||
|
|
||||||
API_PREFIX = "/api/v1"
|
API_PREFIX = "/api/v1"
|
||||||
|
|
||||||
|
_AUTHENTICATED = [Depends(get_current_user)]
|
||||||
|
"""Applied per router: a handler that forgets `CurrentUser` is still closed. Only `health` and
|
||||||
|
`auth` are mounted without it; `tests/api/test_route_auth.py` pins that list."""
|
||||||
|
|
||||||
|
|
||||||
def _operation_id(route: APIRoute) -> str:
|
def _operation_id(route: APIRoute) -> str:
|
||||||
# "auth_login" instead of "login_api_v1_auth_login_post": readable Dart method names
|
# "auth_login" instead of "login_api_v1_auth_login_post": readable Dart method names
|
||||||
@@ -55,7 +60,12 @@ def _operation_id(route: APIRoute) -> str:
|
|||||||
async def _lifespan(app: FastAPI) -> AsyncIterator[None]:
|
async def _lifespan(app: FastAPI) -> AsyncIterator[None]:
|
||||||
settings = get_settings()
|
settings = get_settings()
|
||||||
if settings.is_dev_secret:
|
if settings.is_dev_secret:
|
||||||
log.warning("JWT_SECRET is the insecure development default")
|
if not settings.allow_dev_secret:
|
||||||
|
raise RuntimeError(
|
||||||
|
"JWT_SECRET is missing, a placeholder or shorter than 32 characters; "
|
||||||
|
"set a real one (`openssl rand -hex 32`) or ALLOW_DEV_SECRET=1 for local dev"
|
||||||
|
)
|
||||||
|
log.warning("JWT_SECRET is weak; allowed by ALLOW_DEV_SECRET")
|
||||||
yield
|
yield
|
||||||
await reset_engine()
|
await reset_engine()
|
||||||
|
|
||||||
@@ -83,29 +93,29 @@ def create_app() -> FastAPI:
|
|||||||
_install_openapi(app)
|
_install_openapi(app)
|
||||||
app.include_router(health.router, prefix=API_PREFIX)
|
app.include_router(health.router, prefix=API_PREFIX)
|
||||||
app.include_router(auth.router, prefix=API_PREFIX)
|
app.include_router(auth.router, prefix=API_PREFIX)
|
||||||
app.include_router(sync.router, prefix=API_PREFIX)
|
app.include_router(sync.router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
app.include_router(accounts.router, prefix=API_PREFIX)
|
app.include_router(accounts.router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
app.include_router(categories.router, prefix=API_PREFIX)
|
app.include_router(categories.router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
app.include_router(transactions.router, prefix=API_PREFIX)
|
app.include_router(transactions.router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
app.include_router(rules.router, prefix=API_PREFIX)
|
app.include_router(rules.router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
app.include_router(networth.router, prefix=API_PREFIX)
|
app.include_router(networth.router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
app.include_router(cashflow.router, prefix=API_PREFIX)
|
app.include_router(cashflow.router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
app.include_router(analytics.router, prefix=API_PREFIX)
|
app.include_router(analytics.router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
app.include_router(events.router, prefix=API_PREFIX)
|
app.include_router(events.router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
app.include_router(imports.router, prefix=API_PREFIX)
|
app.include_router(imports.router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
# Before `instruments`: FastAPI matches in registration order, and `/instruments/{id}`
|
# Before `instruments`: FastAPI matches in registration order, and `/instruments/{id}`
|
||||||
# typed `int` does not fall through on a non-numeric segment — it answers 422. So
|
# typed `int` does not fall through on a non-numeric segment — it answers 422. So
|
||||||
# `/instruments/pending` has to be declared first or it becomes unreachable.
|
# `/instruments/pending` has to be declared first or it becomes unreachable.
|
||||||
app.include_router(imports.pending_router, prefix=API_PREFIX)
|
app.include_router(imports.pending_router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
app.include_router(instruments.router, prefix=API_PREFIX)
|
app.include_router(instruments.router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
app.include_router(links.router, prefix=API_PREFIX)
|
app.include_router(links.router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
app.include_router(metrics.router, prefix=API_PREFIX)
|
app.include_router(metrics.router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
app.include_router(goals.router, prefix=API_PREFIX)
|
app.include_router(goals.router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
app.include_router(income.router, prefix=API_PREFIX)
|
app.include_router(income.router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
app.include_router(rebalance.router, prefix=API_PREFIX)
|
app.include_router(rebalance.router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
app.include_router(tax.router, prefix=API_PREFIX)
|
app.include_router(tax.router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
app.include_router(benchmarks.router, prefix=API_PREFIX)
|
app.include_router(benchmarks.router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
app.include_router(benchmarks.analytics_router, prefix=API_PREFIX)
|
app.include_router(benchmarks.analytics_router, prefix=API_PREFIX, dependencies=_AUTHENTICATED)
|
||||||
if settings.web_dir is not None:
|
if settings.web_dir is not None:
|
||||||
mount_web(app, settings.web_dir, API_PREFIX)
|
mount_web(app, settings.web_dir, API_PREFIX)
|
||||||
return app
|
return app
|
||||||
|
|||||||
@@ -45,7 +45,9 @@ CurrentUser = Annotated[AppUser, Depends(get_current_user)]
|
|||||||
|
|
||||||
|
|
||||||
def client_ip(request: Request) -> str:
|
def client_ip(request: Request) -> str:
|
||||||
|
# Caddy overwrites a client-supplied X-Forwarded-For with the real peer address; if a
|
||||||
|
# proxy ever appends instead, the last hop is the one our own proxy vouches for.
|
||||||
fwd = request.headers.get("x-forwarded-for")
|
fwd = request.headers.get("x-forwarded-for")
|
||||||
if fwd:
|
if fwd:
|
||||||
return fwd.split(",")[0].strip()
|
return fwd.rsplit(",", 1)[-1].strip()
|
||||||
return request.client.host if request.client else "unknown"
|
return request.client.host if request.client else "unknown"
|
||||||
|
|||||||
@@ -7,19 +7,24 @@ default, so this is the only brute-force protection — keep it.
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import time
|
import time
|
||||||
from collections import defaultdict, deque
|
from collections import deque
|
||||||
|
|
||||||
|
|
||||||
class SlidingWindowLimiter:
|
class SlidingWindowLimiter:
|
||||||
def __init__(self, limit: int, window_seconds: float = 60.0) -> None:
|
def __init__(self, limit: int, window_seconds: float = 60.0, max_keys: int = 10_000) -> None:
|
||||||
self.limit = limit
|
self.limit = limit
|
||||||
self.window = window_seconds
|
self.window = window_seconds
|
||||||
self._hits: dict[str, deque[float]] = defaultdict(deque)
|
self.max_keys = max_keys
|
||||||
|
self._hits: dict[str, deque[float]] = {}
|
||||||
|
|
||||||
def hit(self, key: str) -> float | None:
|
def hit(self, key: str) -> float | None:
|
||||||
"""Register an attempt. Returns seconds to wait if over the limit, else None."""
|
"""Register an attempt. Returns seconds to wait if over the limit, else None."""
|
||||||
now = time.monotonic()
|
now = time.monotonic()
|
||||||
q = self._hits[key]
|
q = self._hits.get(key)
|
||||||
|
if q is None:
|
||||||
|
if len(self._hits) >= self.max_keys:
|
||||||
|
self._evict(now)
|
||||||
|
q = self._hits[key] = deque()
|
||||||
while q and now - q[0] > self.window:
|
while q and now - q[0] > self.window:
|
||||||
q.popleft()
|
q.popleft()
|
||||||
if len(q) >= self.limit:
|
if len(q) >= self.limit:
|
||||||
@@ -29,3 +34,10 @@ class SlidingWindowLimiter:
|
|||||||
|
|
||||||
def reset(self, key: str) -> None:
|
def reset(self, key: str) -> None:
|
||||||
self._hits.pop(key, None)
|
self._hits.pop(key, None)
|
||||||
|
|
||||||
|
def _evict(self, now: float) -> None:
|
||||||
|
"""Make room for one more key: drop the expired ones, then the oldest if still full."""
|
||||||
|
for k in [k for k, q in self._hits.items() if not q or now - q[-1] > self.window]:
|
||||||
|
del self._hits[k]
|
||||||
|
if len(self._hits) >= self.max_keys:
|
||||||
|
del self._hits[next(iter(self._hits))] # dicts keep insertion order
|
||||||
|
|||||||
@@ -27,14 +27,17 @@ class Settings(BaseSettings):
|
|||||||
jwt_secret: str = "dev-insecure-secret-change-me"
|
jwt_secret: str = "dev-insecure-secret-change-me"
|
||||||
access_token_ttl_seconds: int = 3600
|
access_token_ttl_seconds: int = 3600
|
||||||
refresh_token_ttl_seconds: int = 30 * 86400
|
refresh_token_ttl_seconds: int = 30 * 86400
|
||||||
|
allow_dev_secret: bool = False
|
||||||
|
"""Let the API start with a weak `JWT_SECRET` (local development only)."""
|
||||||
login_rate_limit_per_minute: int = 5
|
login_rate_limit_per_minute: int = 5
|
||||||
|
max_upload_bytes: int = 25 * 1024 * 1024
|
||||||
|
"""Ceiling for one broker report; Caddy enforces a slightly larger one in front."""
|
||||||
|
|
||||||
cors_origins: str = ""
|
cors_origins: str = ""
|
||||||
"""Comma-separated origins for the Flutter web build when it is NOT served by Caddy
|
"""Comma-separated origins for the Flutter web build when it is NOT served by Caddy
|
||||||
from the same origin (e.g. `flutter run -d chrome` during development)."""
|
from the same origin (e.g. `flutter run -d chrome` during development)."""
|
||||||
|
|
||||||
timezone: str = "Europe/Moscow"
|
timezone: str = "Europe/Moscow"
|
||||||
uploads_dir: Path = Path("/data/uploads")
|
|
||||||
web_dir: Path | None = None
|
web_dir: Path | None = None
|
||||||
"""Optional Flutter web build to serve at `/` (dev convenience; Caddy does this in prod)."""
|
"""Optional Flutter web build to serve at `/` (dev convenience; Caddy does this in prod)."""
|
||||||
log_level: str = "INFO"
|
log_level: str = "INFO"
|
||||||
@@ -71,7 +74,9 @@ class Settings(BaseSettings):
|
|||||||
|
|
||||||
@property
|
@property
|
||||||
def is_dev_secret(self) -> bool:
|
def is_dev_secret(self) -> bool:
|
||||||
return self.jwt_secret.startswith("dev-insecure")
|
"""The built-in default, the `.env.example` placeholder, or a secret too short to sign."""
|
||||||
|
s = self.jwt_secret
|
||||||
|
return s.startswith("dev-insecure") or "change-me" in s or len(s) < 32
|
||||||
|
|
||||||
|
|
||||||
@lru_cache
|
@lru_cache
|
||||||
|
|||||||
@@ -0,0 +1,38 @@
|
|||||||
|
"""Every route answers 401 without a token, except a short, explicit list of public ones."""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
|
|
||||||
|
from fintracker.api.app import API_PREFIX
|
||||||
|
|
||||||
|
PUBLIC = {
|
||||||
|
("GET", f"{API_PREFIX}/health"),
|
||||||
|
("POST", f"{API_PREFIX}/auth/login"),
|
||||||
|
("POST", f"{API_PREFIX}/auth/refresh"),
|
||||||
|
("POST", f"{API_PREFIX}/auth/logout"),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def _routes(app) -> list[tuple[str, str]]:
|
||||||
|
"""(METHOD, path) for every documented operation. Walks the OpenAPI schema rather than
|
||||||
|
`app.routes`: FastAPI nests included routers there in a private wrapper type."""
|
||||||
|
return sorted(
|
||||||
|
(method.upper(), path) for path, item in app.openapi()["paths"].items() for method in item
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_every_route_but_the_public_ones_requires_a_token(app, client):
|
||||||
|
routes = _routes(app)
|
||||||
|
assert len(routes) > 50 # guards against the walk silently matching nothing
|
||||||
|
assert set(routes) >= PUBLIC # a renamed public route must update this list, not vanish
|
||||||
|
|
||||||
|
open_routes = []
|
||||||
|
for method, path in routes:
|
||||||
|
if (method, path) in PUBLIC:
|
||||||
|
continue
|
||||||
|
url = re.sub(r"\{[^}]+\}", "1", path)
|
||||||
|
r = await client.request(method, url)
|
||||||
|
if r.status_code != 401:
|
||||||
|
open_routes.append(f"{method} {path} -> {r.status_code}")
|
||||||
|
assert not open_routes, "reachable without a token:\n" + "\n".join(open_routes)
|
||||||
@@ -1,3 +1,6 @@
|
|||||||
|
import pytest
|
||||||
|
|
||||||
|
|
||||||
async def test_login_refresh_logout_cycle(client, user):
|
async def test_login_refresh_logout_cycle(client, user):
|
||||||
r = await client.post("/api/v1/auth/login", json=user)
|
r = await client.post("/api/v1/auth/login", json=user)
|
||||||
assert r.status_code == 200, r.text
|
assert r.status_code == 200, r.text
|
||||||
@@ -41,3 +44,41 @@ async def test_protected_routes_need_token(client):
|
|||||||
assert r.status_code == 401
|
assert r.status_code == 401
|
||||||
r = await client.get("/api/v1/auth/me", headers={"Authorization": "Bearer garbage"})
|
r = await client.get("/api/v1/auth/me", headers={"Authorization": "Bearer garbage"})
|
||||||
assert r.status_code == 401
|
assert r.status_code == 401
|
||||||
|
|
||||||
|
|
||||||
|
def test_client_ip_trusts_only_the_last_forwarded_hop():
|
||||||
|
from starlette.requests import Request
|
||||||
|
|
||||||
|
from fintracker.api.deps import client_ip
|
||||||
|
|
||||||
|
def req(xff: str) -> Request:
|
||||||
|
return Request({"type": "http", "headers": [(b"x-forwarded-for", xff.encode())]})
|
||||||
|
|
||||||
|
assert client_ip(req("1.2.3.4")) == "1.2.3.4"
|
||||||
|
assert client_ip(req("6.6.6.6, 1.2.3.4")) == "1.2.3.4"
|
||||||
|
|
||||||
|
|
||||||
|
def test_limiter_evicts_instead_of_growing_without_bound():
|
||||||
|
from fintracker.api.ratelimit import SlidingWindowLimiter
|
||||||
|
|
||||||
|
limiter = SlidingWindowLimiter(limit=1, max_keys=3)
|
||||||
|
for i in range(10):
|
||||||
|
assert limiter.hit(f"ip-{i}") is None
|
||||||
|
assert len(limiter._hits) <= 3
|
||||||
|
assert limiter.hit("ip-9") is not None # the freshest key keeps its state
|
||||||
|
|
||||||
|
|
||||||
|
async def test_weak_jwt_secret_refuses_to_start(monkeypatch):
|
||||||
|
from fintracker.api.app import _lifespan
|
||||||
|
from fintracker.config import get_settings
|
||||||
|
|
||||||
|
monkeypatch.setenv("JWT_SECRET", "change-me")
|
||||||
|
monkeypatch.delenv("ALLOW_DEV_SECRET", raising=False)
|
||||||
|
get_settings.cache_clear()
|
||||||
|
try:
|
||||||
|
with pytest.raises(RuntimeError, match="JWT_SECRET"):
|
||||||
|
async with _lifespan(None): # type: ignore[arg-type]
|
||||||
|
pass
|
||||||
|
finally:
|
||||||
|
monkeypatch.undo()
|
||||||
|
get_settings.cache_clear()
|
||||||
|
|||||||
@@ -14,11 +14,11 @@ sync:
|
|||||||
|
|
||||||
# run API with reload against the local dev DB
|
# run API with reload against the local dev DB
|
||||||
api:
|
api:
|
||||||
cd {{backend}} && DATABASE_URL={{db_url}} uv run fintracker serve --reload
|
cd {{backend}} && DATABASE_URL={{db_url}} ALLOW_DEV_SECRET=1 uv run fintracker serve --reload
|
||||||
|
|
||||||
# API + Flutter web build from one origin (run `just build-web` first)
|
# API + Flutter web build from one origin (run `just build-web` first)
|
||||||
web:
|
web:
|
||||||
cd {{backend}} && DATABASE_URL={{db_url}} WEB_DIR=../app/build/web uv run fintracker serve --port 8000
|
cd {{backend}} && DATABASE_URL={{db_url}} ALLOW_DEV_SECRET=1 WEB_DIR=../app/build/web uv run fintracker serve --port 8000
|
||||||
|
|
||||||
# --- flutter ---
|
# --- flutter ---
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user