Count per-address limits on the hop a trusted proxy added; limit signup
The limits read the leftmost X-Forwarded-For entry, which the client writes, and the LAN nginx appended to it. The address now comes from uvicorn's proxy-headers middleware (rightmost hop not in FORWARDED_ALLOW_IPS), applied in the app too so tunnel-replayed calls resolve the same way; nginx sets the header to $remote_addr and the compose backend trusts the Docker networks. /users/signup gets a per-address limit of 10 an hour.
This commit is contained in:
@@ -107,16 +107,13 @@ def _too_many(bucket: str, limit: int, window: float, record: bool = True) -> bo
|
||||
|
||||
|
||||
def _client_ip(request: Request) -> str:
|
||||
"""The caller's address, or the proxy's if it did not forward one.
|
||||
"""The caller's address, as the proxy-headers middleware resolved it.
|
||||
|
||||
Behind Traefik `request.client.host` is the proxy for every request, which
|
||||
made every per-address limit here one global bucket — so one caller could
|
||||
lock out everyone. The leftmost forwarded entry is the client, and is what
|
||||
the panel pairing screen already reads.
|
||||
That is the rightmost forwarded hop not added by a trusted proxy
|
||||
(`FORWARDED_ALLOW_IPS`). The leftmost one, which this used to read, is
|
||||
whatever the client wrote, so every limit keyed on it could be dodged by
|
||||
sending a new one each time.
|
||||
"""
|
||||
forwarded = request.headers.get("x-forwarded-for", "").split(",")[0].strip()
|
||||
if forwarded:
|
||||
return forwarded
|
||||
return request.client.host if request.client else "unknown"
|
||||
|
||||
|
||||
|
||||
@@ -213,11 +213,10 @@ def _describe(request: Request) -> str:
|
||||
# it reads badly on the approval screen.
|
||||
"""
|
||||
agent = request.headers.get("user-agent", "").strip()[:120]
|
||||
forwarded = request.headers.get("x-forwarded-for", "").split(",")[0].strip()
|
||||
# Proxied requests are replayed into this process over an ASGI transport,
|
||||
# which reports every caller as localhost; the forwarded address is the
|
||||
# only true one there, and behind the local reverse proxy it is too.
|
||||
address = forwarded or (request.client.host if request.client else "")
|
||||
# Resolved from X-Forwarded-For by the proxy-headers middleware, which is
|
||||
# what makes a call replayed off the tunnel show the device's address
|
||||
# rather than localhost — and stops a device naming any address it likes.
|
||||
address = request.client.host if request.client else ""
|
||||
return " · ".join(part for part in (agent or "Unknown device", address) if part)
|
||||
|
||||
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import uuid
|
||||
from typing import Annotated, Any
|
||||
|
||||
from fastapi import APIRouter, Depends, HTTPException, Query
|
||||
from fastapi import APIRouter, Depends, HTTPException, Query, Request
|
||||
from sqlmodel import col, func, select
|
||||
|
||||
from fluksio import crud
|
||||
@@ -11,6 +11,7 @@ from fluksio.api.deps import (
|
||||
get_current_active_superuser,
|
||||
)
|
||||
from fluksio.api.routes.cloud import forget_remote_user
|
||||
from fluksio.api.routes.oauth import too_many
|
||||
from fluksio.core.config import settings
|
||||
from fluksio.core.security import get_password_hash, verify_password
|
||||
from fluksio.models import (
|
||||
@@ -28,6 +29,11 @@ from fluksio.utils import generate_new_account_email, send_email
|
||||
|
||||
router = APIRouter(prefix="/users", tags=["users"])
|
||||
|
||||
#: Sign-ups one address may attempt per window, once `SIGNUP_ENABLED` opens
|
||||
#: the route to anyone who reaches it.
|
||||
SIGNUP_ATTEMPTS = 10
|
||||
SIGNUP_WINDOW_S = 3600.0
|
||||
|
||||
|
||||
@router.get(
|
||||
"/",
|
||||
@@ -150,12 +156,18 @@ def delete_user_me(session: SessionDep, current_user: CurrentUser) -> Any:
|
||||
|
||||
|
||||
@router.post("/signup", response_model=UserPublic)
|
||||
def register_user(session: SessionDep, user_in: UserRegister) -> Any:
|
||||
def register_user(request: Request, session: SessionDep, user_in: UserRegister) -> Any:
|
||||
"""
|
||||
Create new user without the need to be logged in.
|
||||
"""
|
||||
if not settings.SIGNUP_ENABLED:
|
||||
raise HTTPException(status_code=403, detail="Signup is disabled")
|
||||
# Every attempt counts, not only the refused ones: a successful one is
|
||||
# an account, and an Argon2 hash on the shared threadpool.
|
||||
if too_many(request, "signup", SIGNUP_ATTEMPTS, SIGNUP_WINDOW_S):
|
||||
raise HTTPException(
|
||||
status_code=429, detail="Too many sign-ups from here. Try again later."
|
||||
)
|
||||
user = crud.get_user_by_email(session=session, email=user_in.email)
|
||||
if user:
|
||||
raise HTTPException(
|
||||
|
||||
@@ -55,6 +55,10 @@ class Settings(BaseSettings):
|
||||
ACCESS_TOKEN_EXPIRE_MINUTES: int = 60 * 24 * 8
|
||||
FRONTEND_HOST: str = "http://localhost:5173"
|
||||
ENVIRONMENT: Literal["local", "staging", "production"] = "local"
|
||||
#: The proxies whose X-Forwarded-For is believed: uvicorn's own setting,
|
||||
#: read here too so a call replayed off the portal tunnel resolves its
|
||||
#: caller the same way. IPs or networks, comma-separated; `*` trusts all.
|
||||
FORWARDED_ALLOW_IPS: str = "127.0.0.1"
|
||||
|
||||
#: Everything this instance keeps: the database, the flow repository,
|
||||
#: secrets, artifacts and the user venv. The paths below derive from it
|
||||
|
||||
@@ -16,6 +16,7 @@ from fastapi.responses import JSONResponse
|
||||
from fastapi.routing import APIRoute
|
||||
from fluksio_worker.worker_main import ARTIFACT_DIR_ENV, ARTIFACT_VOLATILE_DIR_ENV
|
||||
from starlette.middleware.cors import CORSMiddleware
|
||||
from uvicorn.middleware.proxy_headers import ProxyHeadersMiddleware
|
||||
|
||||
from fluksio import __version__
|
||||
from fluksio.api.main import api_router
|
||||
@@ -433,6 +434,14 @@ if settings.all_cors_origins:
|
||||
allow_headers=["*"],
|
||||
)
|
||||
|
||||
# The caller's address, for every per-address limit: the rightmost
|
||||
# X-Forwarded-For hop that is not a trusted proxy. uvicorn does this for what
|
||||
# it serves, but a call the portal tunnel replays arrives over an in-process
|
||||
# transport as 127.0.0.1 and never passes through it. Doing it again here
|
||||
# covers that, with uvicorn's own semantics, and is a no-op on a request
|
||||
# uvicorn already resolved. Added last, so it is the outermost layer.
|
||||
app.add_middleware(ProxyHeadersMiddleware, trusted_hosts=settings.FORWARDED_ALLOW_IPS)
|
||||
|
||||
app.include_router(api_router, prefix=settings.API_V1_STR)
|
||||
|
||||
|
||||
|
||||
@@ -4,6 +4,7 @@ from fastapi.testclient import TestClient
|
||||
|
||||
from fluksio.cloud import config as cloud_config
|
||||
from fluksio.core.config import settings
|
||||
from fluksio.main import app
|
||||
|
||||
PREFIX = f"{settings.API_V1_STR}/panels"
|
||||
DASHBOARDS = f"{settings.API_V1_STR}/dashboards"
|
||||
@@ -680,11 +681,13 @@ def test_the_device_asking_is_named_before_anyone_approves(
|
||||
client: TestClient, superuser_token_headers: dict[str, str]
|
||||
) -> None:
|
||||
"""Approving a code adopts whatever holds it, so it is worth a look."""
|
||||
started = client.post(
|
||||
# Behind a trusted proxy (127.0.0.1 by default), as the tunnel replays it.
|
||||
proxied = TestClient(app, client=("127.0.0.1", 50000))
|
||||
started = proxied.post(
|
||||
f"{PREFIX}/pair",
|
||||
headers={
|
||||
"user-agent": "Mozilla/5.0 (X11; CrOS aarch64)",
|
||||
"x-forwarded-for": "203.0.113.7, 10.0.0.1",
|
||||
"x-forwarded-for": "198.51.100.1, 203.0.113.7",
|
||||
},
|
||||
).json()
|
||||
|
||||
@@ -693,8 +696,9 @@ def test_the_device_asking_is_named_before_anyone_approves(
|
||||
)
|
||||
assert looked.status_code == 200
|
||||
assert "CrOS" in looked.json()["device"]
|
||||
# The first hop, not the proxy that relayed it.
|
||||
# The hop the proxy added, not the one the device wrote in front of it.
|
||||
assert "203.0.113.7" in looked.json()["device"]
|
||||
assert "198.51.100.1" not in looked.json()["device"]
|
||||
assert looked.json()["remote"] is False
|
||||
|
||||
# Nobody without an account gets to enumerate what is waiting.
|
||||
|
||||
@@ -5,8 +5,11 @@ from fastapi.testclient import TestClient
|
||||
from sqlmodel import Session, select
|
||||
|
||||
from fluksio import crud
|
||||
from fluksio.api.routes import oauth
|
||||
from fluksio.api.routes.users import SIGNUP_ATTEMPTS
|
||||
from fluksio.core.config import settings
|
||||
from fluksio.core.security import verify_password
|
||||
from fluksio.main import app
|
||||
from fluksio.models import User, UserCreate
|
||||
from tests.utils.user import create_random_user
|
||||
from tests.utils.utils import random_email, random_lower_string
|
||||
@@ -373,6 +376,32 @@ def test_register_user_already_exists_error(client: TestClient) -> None:
|
||||
assert r.json()["detail"] == "The user with this email already exists in the system"
|
||||
|
||||
|
||||
def test_signup_is_limited_per_address_behind_a_proxy() -> None:
|
||||
"""The limit counts the hop the trusted proxy added, not one the client wrote.
|
||||
|
||||
The leftmost hop is the client's own to fill in, so keying on it let one
|
||||
caller look like a new one on every attempt.
|
||||
"""
|
||||
proxied = TestClient(app, client=("127.0.0.1", 50000)) # a trusted proxy
|
||||
url = f"{settings.API_V1_STR}/users/signup"
|
||||
taken = {"email": settings.FIRST_SUPERUSER, "password": random_lower_string()}
|
||||
|
||||
def attempt(forwarded: str) -> int:
|
||||
return proxied.post(
|
||||
url, json=taken, headers={"x-forwarded-for": forwarded}
|
||||
).status_code
|
||||
|
||||
try:
|
||||
with patch("fluksio.core.config.settings.SIGNUP_ENABLED", True):
|
||||
for n in range(SIGNUP_ATTEMPTS):
|
||||
assert attempt(f"198.51.100.{n}, 203.0.113.7") == 400
|
||||
assert attempt("198.51.100.99, 203.0.113.7") == 429
|
||||
assert attempt("203.0.113.8") == 400
|
||||
finally:
|
||||
for address in ("203.0.113.7", "203.0.113.8"):
|
||||
oauth._hits.pop(f"signup:{address}", None)
|
||||
|
||||
|
||||
def test_update_user(
|
||||
client: TestClient, superuser_token_headers: dict[str, str], db: Session
|
||||
) -> None:
|
||||
|
||||
@@ -98,6 +98,11 @@ services:
|
||||
- FRONTEND_HOST=${FRONTEND_HOST?Variable not set}
|
||||
- ENVIRONMENT=${ENVIRONMENT}
|
||||
- BACKEND_CORS_ORIGINS=${BACKEND_CORS_ORIGINS}
|
||||
# Whose X-Forwarded-For is believed: Traefik's and the frontend's nginx,
|
||||
# which reach this from the compose networks (Docker's default pools sit
|
||||
# in 172.16.0.0/12). Both write the header themselves, so the hop they
|
||||
# add is the caller. No port is published here, so nothing else can.
|
||||
- FORWARDED_ALLOW_IPS=${FORWARDED_ALLOW_IPS:-127.0.0.1,172.16.0.0/12}
|
||||
- SECRET_KEY=${SECRET_KEY?Variable not set}
|
||||
- FIRST_SUPERUSER=${FIRST_SUPERUSER?Variable not set}
|
||||
- FIRST_SUPERUSER_PASSWORD=${FIRST_SUPERUSER_PASSWORD?Variable not set}
|
||||
|
||||
@@ -13,7 +13,10 @@ location /api {
|
||||
|
||||
proxy_http_version 1.1;
|
||||
proxy_set_header Host $host;
|
||||
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
|
||||
# Replaced, not appended to: whatever the client sent would otherwise sit
|
||||
# in front of the one hop the backend believes, and anything reading the
|
||||
# header naively would take it for the caller.
|
||||
proxy_set_header X-Forwarded-For $remote_addr;
|
||||
proxy_set_header X-Forwarded-Proto $scheme;
|
||||
# The flow websocket (/api/v1/flows/ws) upgrades through here; passing the
|
||||
# client's own Connection header covers both it and ordinary requests.
|
||||
|
||||
@@ -73,6 +73,7 @@ stands.
|
||||
| `DOMAIN` | `localhost` | what the API and OAuth issuer are built from |
|
||||
| `FRONTEND_HOST` | `http://localhost:5173` | used in mails, OAuth metadata and panel pairing links |
|
||||
| `BACKEND_CORS_ORIGINS` | `[]` | comma-separated; `FRONTEND_HOST` is always allowed |
|
||||
| `FORWARDED_ALLOW_IPS` | `127.0.0.1` (`127.0.0.1,172.16.0.0/12` in the compose stack) | proxies whose `X-Forwarded-For` is believed, as uvicorn reads it; the sign-in, sign-up and OAuth limits count the rightmost hop none of them added |
|
||||
|
||||
## Where the interface looks for the API
|
||||
|
||||
|
||||
Reference in New Issue
Block a user