Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- **Login: stable CSRF cookie + open favicon route.** `GET /login` no longer
rotates the `tb_csrf` double-submit cookie on every render — a background
request auth-redirected to `/login` (typically the browser's `/favicon.ico`
probe) used to rotate the cookie under the form the user was looking at, so
every submit 403'd. The render now reuses a well-formed existing cookie
(shape-checked so an arbitrary value is never echoed into the form), and
`/favicon.ico` is an open route (308 → `/static/favicon.svg`) so the probe
never bounces through the auth redirect at all.

### Deprecated

### Removed
Expand Down
30 changes: 25 additions & 5 deletions trading_bot/interfaces/api/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,16 @@ async def _generator() -> Any:
#: Login attempts per minute per client (brute-force throttle).
_LOGIN_RATE_PER_MIN = 10
#: Path prefixes reachable without a session (the login flow + assets).
_OPEN_PREFIXES = ("/login", "/logout", "/static")
#: ``/favicon.ico`` is open on purpose: a browser fetches it in the background on
#: every page — including the login page — and an auth redirect to ``/login``
#: would re-render the form and (before the stable-cookie fix in ``_login_page``)
#: rotate the CSRF cookie under the form the user is looking at.
_OPEN_PREFIXES = ("/login", "/logout", "/static", "/favicon.ico")

#: Shape of a CSRF token we minted (``secrets.token_urlsafe(32)`` → 43 urlsafe
#: chars). An existing cookie is only reused when it matches, so an arbitrary
#: value can never be echoed back into the login form.
_CSRF_SHAPE = re.compile(r"^[A-Za-z0-9_-]{43}$")

#: Hard cap on live sessions (I-7): once reached, the oldest session is evicted on a
#: new login so the in-memory map cannot grow without bound over a long-lived daemon.
Expand Down Expand Up @@ -1411,10 +1420,16 @@ async def _auth_guard(request: Request, call_next: Any) -> Any:

def _login_page(request: Request, *, error: str = "", status: int = 200) -> Any:
nxt = _safe_next(request.query_params.get("next"))
# I-13: mint (or reuse) a per-render CSRF token, embed it in the form AND
# set it as a SameSite=strict cookie — the POST must echo both (double
# submit). A fresh token each GET is fine (the browser keeps the latest).
csrf = secrets.token_urlsafe(32)
# I-13: double-submit CSRF — the POST must echo the form field AND the
# cookie, and they must match. REUSE the browser's existing cookie when it
# has the shape we mint: any background request landing on /login (e.g. an
# unauthenticated asset fetch redirected here) re-renders this page, and
# minting a fresh token per render would rotate the cookie *under* the form
# the user is already looking at — every submit would then 403. A stable
# per-browser token is the standard double-submit pattern.
csrf = request.cookies.get(_CSRF_COOKIE, "")
if not _CSRF_SHAPE.match(csrf):
csrf = secrets.token_urlsafe(32)
if templates is not None:
resp: Any = templates.TemplateResponse(
request,
Expand Down Expand Up @@ -1447,6 +1462,11 @@ def _login_page(request: Request, *, error: str = "", status: int = 200) -> Any:
)
return resp

@app.get("/favicon.ico", include_in_schema=False)
async def favicon() -> Any:
"""Serve the browser's default icon probe (open route, see _OPEN_PREFIXES)."""
return RedirectResponse("/static/favicon.svg", status_code=308)

@app.get("/login", response_class=HTMLResponse)
async def login_form(request: Request) -> Any:
return _login_page(request)
Expand Down
1 change: 1 addition & 0 deletions trading_bot/interfaces/ui/templates/login.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>trading_bot — sign in</title>
<link rel="icon" type="image/svg+xml" href="/static/favicon.svg">
<link rel="stylesheet" href="/static/style.css">
</head>
<body class="login-body">
Expand Down
47 changes: 47 additions & 0 deletions trading_bot/tests/interfaces/test_dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,53 @@ def test_auth_login_flow_authenticates() -> None:
assert client.get("/api/health").status_code == 200 # session cookie works


def test_login_survives_a_background_login_rerender_stable_csrf() -> None:
"""The CSRF cookie is stable across /login renders — the favicon race.

A browser fetches ``/favicon.ico`` in the background right after loading the
login page. Before the fix that fetch was auth-redirected to ``/login``,
whose render minted a FRESH token and rotated the ``tb_csrf`` cookie under
the form the user was already looking at — so every submit 403'd, forever.
The token embedded in the FIRST render's form must still authenticate after
a second (background) render.
"""
import re as _re

client, token = _auth_client()
first = client.get("/login")
m = _re.search(r'name="csrf" value="([^"]+)"', first.text)
assert m, "login form must embed the CSRF field"
form_csrf = m.group(1) # what the user's visible form will submit
# A background request re-renders /login (pre-fix: rotated the cookie).
client.get("/login?next=/favicon.ico")
assert client.cookies.get("tb_csrf", "") == form_csrf # cookie is STABLE
ok = client.post(
"/login",
data={"token": token, "next": "/", "csrf": form_csrf},
follow_redirects=False,
)
assert ok.status_code == 303 # the form the user sees still works


def test_favicon_is_open_and_never_bounces_to_login() -> None:
"""``/favicon.ico`` needs no session and never triggers a /login re-render."""
client, _ = _auth_client()
r = client.get("/favicon.ico", follow_redirects=False)
assert r.status_code == 308
assert r.headers["location"] == "/static/favicon.svg"


def test_login_render_rejects_a_malformed_csrf_cookie() -> None:
"""A cookie not matching our minted shape is replaced, never echoed back."""
client, _ = _auth_client()
# Raw header (not the cookie jar) so the single malformed value is exactly
# what the server sees.
page = client.get("/login", headers={"cookie": "tb_csrf=<script>alert(1)</script>"})
assert "<script>alert(1)" not in page.text
set_cookie = page.headers.get("set-cookie", "")
assert "tb_csrf=" in set_cookie and "<script>" not in set_cookie # fresh mint


# --- aggregate read endpoints (Overview data) ------------------------------ #


Expand Down
Loading