feat(6866): the proxy hop count is an admin setting, and the server shows what each count reads
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / Python lint (push) Successful in 2s
CI & Build / TypeScript typecheck (push) Successful in 32s
CI & Build / Python tests (push) Successful in 1m47s
CI & Build / integration (push) Successful in 2m1s
CI & Build / Build & push image (push) Successful in 20s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / Python lint (push) Successful in 2s
CI & Build / TypeScript typecheck (push) Successful in 32s
CI & Build / Python tests (push) Successful in 1m47s
CI & Build / integration (push) Successful in 2m1s
CI & Build / Build & push image (push) Successful in 20s
An admin can save how many reverse proxies stand in front of Grimoire. A saved count wins over TRUSTED_PROXY_HOPS, which stays the starting value. Each worker loads it at startup (under a deadline) and re-reads it every 30s; the worker that saves applies it at once. GET/PUT /api/admin/proxy returns the count, where it came from, and a readout of the admin's own request: the socket peer, the forwarding chains, what counts 0-3 would read, and the count that reads Cloudflare's visitor when Cloudflare's headers are present. Saves are audit-logged. A "too many attempts" reply now says where to look when everyone is being blocked. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,232 @@
|
||||
"""The reverse-proxy hop count an admin sets in Settings → Config.
|
||||
|
||||
Behind Cloudflare's proxied DNS the right count is 2. At the default of 1,
|
||||
every visitor reads as a Cloudflare edge address and shares one sign-in rate
|
||||
limit, so a few bad sign-ins block everyone. The count is therefore saved where
|
||||
an admin can find it, beside a readout of what each count makes of their own
|
||||
request. Covered here:
|
||||
- the saved count wins over the environment's;
|
||||
- the readout picks out the Cloudflare road;
|
||||
- the admin route validates what it saves;
|
||||
- a "too many attempts" reply says where to look.
|
||||
"""
|
||||
import asyncio
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
from quart import Quart, g
|
||||
|
||||
from grimoire import proxy
|
||||
from grimoire.config import Config
|
||||
from grimoire.proxy import client_ip, readout, request_readout, trusted_hops
|
||||
from grimoire.rate_limit import SHARED_ADDRESS_HINT
|
||||
from grimoire.services import proxy_settings
|
||||
|
||||
VISITOR = "203.0.113.7"
|
||||
CLOUDFLARE_EDGE = "172.70.1.1"
|
||||
LAN_VISITOR = "192.168.1.20"
|
||||
TRAEFIK = "10.0.0.5"
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _no_saved_count():
|
||||
proxy.set_saved_hops(None)
|
||||
yield
|
||||
proxy.set_saved_hops(None)
|
||||
|
||||
|
||||
def _context(headers: dict):
|
||||
app = Quart(__name__)
|
||||
return app.test_request_context("/", headers={"Remote-Addr": TRAEFIK, **headers})
|
||||
|
||||
|
||||
@pytest.mark.parametrize("raw, expected", [
|
||||
(0, 0), (2, 2), (5, 5), ("3", 3), (" 1 ", 1),
|
||||
(6, None), (-1, None), ("x", None), ("", None), (True, None), (1.5, None), (None, None),
|
||||
])
|
||||
def test_a_hop_count_is_a_whole_number_from_zero_to_five(raw, expected):
|
||||
assert proxy_settings.parse_hops(raw) == expected
|
||||
|
||||
|
||||
def test_the_saved_count_wins_over_the_environments(monkeypatch):
|
||||
monkeypatch.setattr(Config, "TRUSTED_PROXY_HOPS", 1)
|
||||
assert trusted_hops() == 1
|
||||
proxy.set_saved_hops(2)
|
||||
assert trusted_hops() == 2
|
||||
proxy.set_saved_hops(0)
|
||||
assert trusted_hops() == 0, "a saved 0 is a count, not an absence"
|
||||
|
||||
|
||||
async def test_the_client_address_follows_a_saved_count(monkeypatch):
|
||||
monkeypatch.setattr(Config, "TRUSTED_PROXY_HOPS", 1)
|
||||
async with _context({"X-Forwarded-For": f"{VISITOR}, {CLOUDFLARE_EDGE}"}):
|
||||
assert client_ip() == CLOUDFLARE_EDGE
|
||||
proxy.set_saved_hops(2)
|
||||
assert client_ip() == VISITOR
|
||||
|
||||
|
||||
@pytest.mark.parametrize("saved, from_env, expected", [
|
||||
(2, False, "setting"), (2, True, "setting"), (None, True, "env"), (None, False, "default"),
|
||||
])
|
||||
def test_the_source_names_where_the_count_came_from(monkeypatch, saved, from_env, expected):
|
||||
monkeypatch.setattr(Config, "TRUSTED_PROXY_HOPS_FROM_ENV", from_env)
|
||||
proxy.set_saved_hops(saved)
|
||||
assert proxy_settings.source() == expected
|
||||
|
||||
|
||||
def test_the_readout_suggests_the_count_that_reads_cloudflares_visitor():
|
||||
out = readout(TRAEFIK, f"{VISITOR}, {CLOUDFLARE_EDGE}", "https, https", VISITOR)
|
||||
by_hops = {row["hops"]: row for row in out["rows"]}
|
||||
assert by_hops[0]["client"] == TRAEFIK
|
||||
assert by_hops[1]["client"] == CLOUDFLARE_EDGE
|
||||
assert by_hops[2]["client"] == VISITOR and by_hops[2]["https"] is True
|
||||
assert out["cloudflare"] is True and out["suggested"] == 2
|
||||
assert out["forwarded_for"] == [VISITOR, CLOUDFLARE_EDGE]
|
||||
|
||||
|
||||
def test_the_readout_suggests_nothing_off_cloudflare():
|
||||
# Only the admin knows which address is theirs on a road with no
|
||||
# Cloudflare header to compare against.
|
||||
out = readout(TRAEFIK, LAN_VISITOR, "http", None)
|
||||
assert out["cloudflare"] is False and out["suggested"] is None
|
||||
assert [row["client"] for row in out["rows"]] == [TRAEFIK, LAN_VISITOR, LAN_VISITOR, LAN_VISITOR]
|
||||
assert not any(row["https"] for row in out["rows"])
|
||||
|
||||
|
||||
def test_the_readout_rows_can_disagree():
|
||||
# The guard above passes vacuously if every row reads the same address.
|
||||
out = readout(TRAEFIK, f"{VISITOR}, {CLOUDFLARE_EDGE}", "", VISITOR)
|
||||
assert len({row["client"] for row in out["rows"]}) == 3
|
||||
|
||||
|
||||
async def test_the_request_readout_reads_the_cloudflare_headers():
|
||||
headers = {"X-Forwarded-For": f"{VISITOR}, {CLOUDFLARE_EDGE}", "CF-Connecting-IP": VISITOR}
|
||||
async with _context(headers):
|
||||
out = request_readout()
|
||||
assert out["peer"] == TRAEFIK and out["cloudflare_client"] == VISITOR and out["suggested"] == 2
|
||||
|
||||
async with _context({"CF-Ray": "8a1b2c3d4e5f-LHR"}):
|
||||
out = request_readout()
|
||||
assert out["cloudflare"] is True and out["suggested"] is None
|
||||
|
||||
|
||||
async def test_a_failed_refresh_keeps_the_count_in_force():
|
||||
proxy.set_saved_hops(2)
|
||||
with patch.object(proxy_settings, "saved_hops", AsyncMock(side_effect=OSError("db down"))):
|
||||
await proxy_settings.refresh()
|
||||
assert trusted_hops() == 2
|
||||
|
||||
|
||||
async def test_a_hanging_startup_read_does_not_hold_up_the_boot(monkeypatch):
|
||||
async def _never():
|
||||
await asyncio.Event().wait()
|
||||
|
||||
monkeypatch.setattr(proxy_settings, "STARTUP_READ_TIMEOUT", 0.05)
|
||||
with patch.object(proxy_settings, "refresh", _never), \
|
||||
patch.object(proxy_settings.logger, "warning") as warn:
|
||||
await asyncio.wait_for(proxy_settings.load_at_startup(), timeout=2)
|
||||
assert warn.called
|
||||
|
||||
|
||||
def test_the_startup_read_has_a_deadline():
|
||||
assert 0 < proxy_settings.STARTUP_READ_TIMEOUT < 60
|
||||
|
||||
|
||||
# ── the admin route ──────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
async def _call(method: str, body=None):
|
||||
from grimoire.app import create_app
|
||||
from grimoire.routes import admin
|
||||
|
||||
app = create_app()
|
||||
view = admin.get_proxy_setting if method == "GET" else admin.update_proxy_setting
|
||||
async with app.test_request_context("/api/admin/proxy", method=method, json=body,
|
||||
headers={"Remote-Addr": TRAEFIK}):
|
||||
g.user = SimpleNamespace(id=1, username="admin", role="admin")
|
||||
resp = await view.__wrapped__()
|
||||
if isinstance(resp, tuple):
|
||||
return resp[1], await resp[0].get_json()
|
||||
return resp.status_code, await resp.get_json()
|
||||
|
||||
|
||||
async def _fake_save(hops):
|
||||
proxy.set_saved_hops(hops)
|
||||
|
||||
|
||||
async def test_the_route_reports_the_count_its_source_and_the_readout(monkeypatch):
|
||||
monkeypatch.setattr(Config, "TRUSTED_PROXY_HOPS", 1)
|
||||
proxy.set_saved_hops(2)
|
||||
status, body = await _call("GET")
|
||||
assert status == 200
|
||||
assert body["hops"] == 2 and body["source"] == "setting" and body["env_hops"] == 1
|
||||
assert body["max_hops"] == proxy.MAX_HOPS
|
||||
assert body["readout"]["peer"] == TRAEFIK and len(body["readout"]["rows"]) == 4
|
||||
|
||||
|
||||
async def test_saving_a_count_applies_it_and_audits_the_change(monkeypatch):
|
||||
from grimoire.routes import admin
|
||||
|
||||
monkeypatch.setattr(Config, "TRUSTED_PROXY_HOPS", 1)
|
||||
save = AsyncMock(side_effect=_fake_save)
|
||||
audit = AsyncMock()
|
||||
with patch.object(admin.proxy_settings, "save_hops", save), patch.object(admin, "log_audit", audit):
|
||||
status, body = await _call("PUT", {"hops": 2})
|
||||
assert status == 200 and body["hops"] == 2
|
||||
save.assert_awaited_once_with(2)
|
||||
details = audit.call_args.kwargs["details"]
|
||||
assert details == {"before": 1, "after": 2, "saved": 2}
|
||||
|
||||
|
||||
async def test_saving_null_goes_back_to_the_environments_count(monkeypatch):
|
||||
from grimoire.routes import admin
|
||||
|
||||
monkeypatch.setattr(Config, "TRUSTED_PROXY_HOPS", 1)
|
||||
proxy.set_saved_hops(3)
|
||||
save = AsyncMock(side_effect=_fake_save)
|
||||
with patch.object(admin.proxy_settings, "save_hops", save), patch.object(admin, "log_audit", AsyncMock()):
|
||||
status, body = await _call("PUT", {"hops": None})
|
||||
assert status == 200 and body["hops"] == 1
|
||||
save.assert_awaited_once_with(None)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("bad", [6, -1, "two", True, 1.5])
|
||||
async def test_a_count_out_of_range_is_refused_and_nothing_is_saved(bad):
|
||||
from grimoire.routes import admin
|
||||
|
||||
save = AsyncMock()
|
||||
with patch.object(admin.proxy_settings, "save_hops", save):
|
||||
status, body = await _call("PUT", {"hops": bad})
|
||||
assert status == 400 and "0 to 5" in body["error"]
|
||||
save.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("method", ["GET", "PUT"])
|
||||
async def test_the_route_is_for_admins_only(method):
|
||||
from grimoire.app import create_app
|
||||
from grimoire.routes import admin
|
||||
|
||||
view = admin.get_proxy_setting if method == "GET" else admin.update_proxy_setting
|
||||
app = create_app()
|
||||
async with app.test_request_context("/api/admin/proxy", method=method, json={"hops": 2}):
|
||||
resp = await view()
|
||||
assert resp[1] == 401
|
||||
|
||||
|
||||
# ── where a lockout shows up ─────────────────────────────────────────────────
|
||||
|
||||
|
||||
async def test_a_rate_limited_sign_in_says_where_an_admin_should_look(monkeypatch):
|
||||
from grimoire.app import create_app
|
||||
from grimoire.routes import auth
|
||||
|
||||
monkeypatch.setattr(Config, "LOCAL_AUTH_ENABLED", True)
|
||||
app = create_app()
|
||||
with patch.object(auth, "is_rate_limited", AsyncMock(return_value=True)):
|
||||
async with app.test_request_context("/api/auth/login", method="POST", json={}):
|
||||
resp, status = await auth.login()
|
||||
body = await resp.get_json()
|
||||
assert status == 429
|
||||
assert SHARED_ADDRESS_HINT in body["error"]
|
||||
assert "Settings → Config" in SHARED_ADDRESS_HINT
|
||||
Reference in New Issue
Block a user