From 2956fa80832abe8a2c01bcc20fff46fbbc873534 Mon Sep 17 00:00:00 2001 From: Jage9 Date: Sun, 1 Mar 2026 23:57:31 -0500 Subject: [PATCH] Fix session cookie routing and proxy-aware auth throttling --- client/public/version.js | 2 +- client/src/main.ts | 16 ++++-- deploy/apache/chgrid-vhost-snippet.conf | 2 + server/app/server.py | 52 ++++++++++++++++++-- server/tests/test_server_message_handling.py | 27 ++++++++++ 5 files changed, 90 insertions(+), 9 deletions(-) diff --git a/client/public/version.js b/client/public/version.js index 2ebe1d2..86bfd8f 100644 --- a/client/public/version.js +++ b/client/public/version.js @@ -1,5 +1,5 @@ // Maintainer-controlled web client version. // Format: YYYY.MM.DD Rn (example: 2026.02.20 R2) -window.CHGRID_WEB_VERSION = "2026.03.01 R332"; +window.CHGRID_WEB_VERSION = "2026.03.01 R333"; // Optional display timezone for timestamps. Falls back to America/Detroit if unset/invalid. window.CHGRID_TIME_ZONE = "America/Detroit"; diff --git a/client/src/main.ts b/client/src/main.ts index 9fda5f4..45410e6 100644 --- a/client/src/main.ts +++ b/client/src/main.ts @@ -270,8 +270,8 @@ const SYSTEM_SOUND_URLS = { logout: withBase('sounds/logout.ogg'), notify: withBase('sounds/notify.ogg'), } as const; -const AUTH_SESSION_COOKIE_SET_URL = withBase('auth/session/set'); -const AUTH_SESSION_COOKIE_CLEAR_URL = withBase('auth/session/clear'); +const AUTH_SESSION_COOKIE_SET_URL = '/auth/session/set'; +const AUTH_SESSION_COOKIE_CLEAR_URL = '/auth/session/clear'; const AUTH_SESSION_COOKIE_CLIENT_HEADER = 'X-Chgrid-Auth-Client'; const ACTION_SOUND_URL = withBase('sounds/action.ogg'); const FOOTSTEP_SOUND_URLS = Array.from({ length: 11 }, (_, index) => withBase(`sounds/step-${index + 1}.ogg`)); @@ -1698,7 +1698,7 @@ async function persistHttpOnlySessionCookie(sessionToken: string): Promise const token = sessionToken.trim(); if (!token) return; try { - await fetch(AUTH_SESSION_COOKIE_SET_URL, { + const response = await fetch(AUTH_SESSION_COOKIE_SET_URL, { method: 'GET', credentials: 'include', headers: { @@ -1707,15 +1707,19 @@ async function persistHttpOnlySessionCookie(sessionToken: string): Promise }, cache: 'no-store', }); + if (!response.ok) { + throw new Error(`HTTP ${response.status}`); + } } catch (error) { console.warn('Unable to persist auth cookie.', error); + pushChatMessage('Session save failed. You may need to log in again after refresh.'); } } /** Clears server-managed HttpOnly auth session cookie. */ async function clearHttpOnlySessionCookie(): Promise { try { - await fetch(AUTH_SESSION_COOKIE_CLEAR_URL, { + const response = await fetch(AUTH_SESSION_COOKIE_CLEAR_URL, { method: 'GET', credentials: 'include', headers: { @@ -1723,8 +1727,12 @@ async function clearHttpOnlySessionCookie(): Promise { }, cache: 'no-store', }); + if (!response.ok) { + throw new Error(`HTTP ${response.status}`); + } } catch (error) { console.warn('Unable to clear auth cookie.', error); + pushChatMessage('Session clear failed. Your browser may retain an old login cookie.'); } } diff --git a/deploy/apache/chgrid-vhost-snippet.conf b/deploy/apache/chgrid-vhost-snippet.conf index 6b5cfa8..0b75f26 100644 --- a/deploy/apache/chgrid-vhost-snippet.conf +++ b/deploy/apache/chgrid-vhost-snippet.conf @@ -6,6 +6,8 @@ # Proxy websocket signaling endpoint to local Python service. ProxyPass /ws ws://127.0.0.1:8765 ProxyPassReverse /ws ws://127.0.0.1:8765 +ProxyPass /auth/session/ http://127.0.0.1:8765/auth/session/ +ProxyPassReverse /auth/session/ http://127.0.0.1:8765/auth/session/ # Ensure HTML entrypoint is never cached so version updates are picked up quickly. diff --git a/server/app/server.py b/server/app/server.py index c6070a9..7c297a9 100644 --- a/server/app/server.py +++ b/server/app/server.py @@ -9,6 +9,7 @@ from contextlib import suppress from datetime import datetime, timezone from getpass import getpass from importlib.metadata import PackageNotFoundError, version as package_version +import ipaddress import json import logging import os @@ -456,10 +457,53 @@ class SignalingServer: address = getattr(client.websocket, "remote_address", None) if isinstance(address, tuple) and address: - return str(address[0]) - if isinstance(address, str): - return address - return "unknown" + peer_raw = address[0] + elif isinstance(address, str): + peer_raw = address + else: + peer_raw = None + peer_ip = SignalingServer._normalized_ip(peer_raw) + if not peer_ip: + return "unknown" + + # Trust X-Forwarded-For only from a loopback proxy hop (e.g., local Apache/nginx). + try: + peer_addr = ipaddress.ip_address(peer_ip) + except ValueError: + return peer_ip + if not peer_addr.is_loopback: + return peer_ip + + request = getattr(client.websocket, "request", None) + headers = getattr(request, "headers", None) + if headers is None: + return peer_ip + forwarded = str(headers.get("X-Forwarded-For", "")).strip() + if not forwarded: + return peer_ip + for candidate in forwarded.split(","): + parsed = SignalingServer._normalized_ip(candidate) + if parsed: + return parsed + return peer_ip + + @staticmethod + def _normalized_ip(value: object) -> str | None: + """Return normalized IP text or None when input is invalid.""" + + if value is None: + return None + text = str(value).strip() + if not text: + return None + if text.startswith("[") and text.endswith("]"): + text = text[1:-1] + if "%" in text: + text = text.split("%", 1)[0] + try: + return str(ipaddress.ip_address(text)) + except ValueError: + return None @staticmethod def _prune_failure_window(bucket: deque[float], now_s: float) -> None: diff --git a/server/tests/test_server_message_handling.py b/server/tests/test_server_message_handling.py index 796a023..e8136ad 100644 --- a/server/tests/test_server_message_handling.py +++ b/server/tests/test_server_message_handling.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio import json from pathlib import Path +from types import SimpleNamespace from time import monotonic from typing import cast import uuid @@ -23,6 +24,32 @@ def _packet_types(payloads: list[object]) -> list[str]: return [getattr(packet, "type", "") for packet in payloads] +def test_client_ip_prefers_forwarded_for_from_loopback_proxy() -> None: + server = SignalingServer("127.0.0.1", 8765, None, None) + ws = cast( + ServerConnection, + SimpleNamespace( + remote_address=("127.0.0.1", 12345), + request=SimpleNamespace(headers={"X-Forwarded-For": "198.51.100.25, 127.0.0.1"}), + ), + ) + client = ClientConnection(websocket=ws, id="u1", nickname="tester") + assert server._client_ip(client) == "198.51.100.25" + + +def test_client_ip_ignores_forwarded_for_from_non_loopback_peer() -> None: + server = SignalingServer("127.0.0.1", 8765, None, None) + ws = cast( + ServerConnection, + SimpleNamespace( + remote_address=("203.0.113.20", 12345), + request=SimpleNamespace(headers={"X-Forwarded-For": "198.51.100.25"}), + ), + ) + client = ClientConnection(websocket=ws, id="u1", nickname="tester") + assert server._client_ip(client) == "203.0.113.20" + + @pytest.mark.asyncio async def test_update_position_rejects_out_of_bounds(monkeypatch: pytest.MonkeyPatch) -> None: server = SignalingServer("127.0.0.1", 8765, None, None, grid_size=41)