From 69b177d5195022bb1d305a9d52c8b41c877bfe4d Mon Sep 17 00:00:00 2001 From: Mike Wichers Date: Fri, 4 Sep 2026 17:45:17 -0400 Subject: [PATCH] the LAN rule moves from nginx into the app, as a setting for keys only api_keys_from is a typed setting, lan (default, the historical rule) or anywhere, checked in admin_api._auth against the first X-Forwarded-For hop that NPM sets. It governs API keys only: a session is never restricted, the console is a session from anywhere; and the break-glass token is ALWAYS LAN-only, which is not a choice and so is not a setting. LAN ranges are facts about the network and live in code; the docker range is included because NPM, the routines and sibling containers reach the app from arrstack_arr_net. An unparseable address is not LAN - the rule fails closed. With this in place the location /api/admin/ block on NPM host 51 can come out; the app enforces what it enforced, and the toggle never touches NPM. tests/smoke_admin.py 58 -> 76. Proven on a throwaway site: key LAN 200, key WAN 403 naming the setting, session WAN 200, token LAN 200, token WAN 403; set anywhere, key WAN 200 and token WAN still 403; bogus value 422. --- app/admin_api.py | 39 ++++++++++++++++++++++++++++++++++-- app/identity.py | 14 +++++++++++++ tests/smoke_admin.py | 47 +++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 97 insertions(+), 3 deletions(-) diff --git a/app/admin_api.py b/app/admin_api.py index 47e1413..892ad09 100644 --- a/app/admin_api.py +++ b/app/admin_api.py @@ -40,6 +40,7 @@ one CAPS dictionary - never by a role comparison here. import hmac import html as _html import inspect +import ipaddress import os import re @@ -60,6 +61,26 @@ router = APIRouter(prefix="/api/admin", tags=["admin"]) docs_router = APIRouter(prefix="/api", tags=["docs"]) +# What counts as "inside" for the LAN rule. Facts about the network, so +# they live in code; the CHOICE of whether keys are LAN-only is a setting. +# The docker range is here because NPM, the routines and any sibling +# container reach this app from arrstack_arr_net, and a container on that +# network is already inside. +LAN_NETS = [ipaddress.ip_network(n) for n in + ("10.0.0.0/24", "10.0.1.0/24", "127.0.0.0/8", "172.16.0.0/12")] + + +def client_is_lan(request): + """True if the caller's address is on the LAN. Uses the first + X-Forwarded-For hop, which NPM sets; an unparseable or absent address is + NOT lan - the rule fails closed.""" + try: + ip = ipaddress.ip_address(auth._client_ip(request) or "") + except ValueError: + return False + return any(ip in n for n in LAN_NETS) + + def _person(request): """Who is calling: a session first, then an API key, then nobody. @@ -89,6 +110,10 @@ def _auth(request, token, capability, unit_id=None): """ person = _person(request) if person: + if person.get("key_scopes") is not None and not client_is_lan(request) \ + and identity.get_setting("api_keys_from") == "lan": + raise HTTPException(403, "API keys may only be used from the LAN right now " + "(site setting api_keys_from)") if not identity.can(person, capability, unit_id): raise HTTPException(403, "your account does not have %s" % capability + (" for this unit" if unit_id else "") @@ -99,6 +124,10 @@ def _auth(request, token, capability, unit_id=None): raise HTTPException(503, "admin API disabled: sign in, or set ADMIN_TOKEN") if not token or not hmac.compare_digest(token, ADMIN_TOKEN): raise HTTPException(401, "sign in, or send a valid X-Admin-Token") + # The shared token is break glass for an operator at the console. It is + # never usable from outside, and that is not a setting. + if not client_is_lan(request): + raise HTTPException(403, "the admin token is LAN-only") return "admin-token" @@ -380,13 +409,19 @@ def whoami(request: Request, x_admin_token: str = Header(None)): call with a new key.""" person = _person(request) if person: + if person.get("key_scopes") is not None and not client_is_lan(request) \ + and identity.get_setting("api_keys_from") == "lan": + raise HTTPException(403, "API keys may only be used from the LAN right now " + "(site setting api_keys_from)") return {"email": person["email"], "global_role": person.get("global_role"), - "capabilities": person["capabilities"], + "capabilities": person["capabilities"], "lan": client_is_lan(request), "via": ("key " + person["key_prefix"]) if person.get("key_scopes") is not None else "session"} if not ADMIN_TOKEN: raise HTTPException(503, "admin API disabled: sign in, or set ADMIN_TOKEN") if x_admin_token and hmac.compare_digest(x_admin_token, ADMIN_TOKEN): - return {"email": None, "via": "admin-token", "capabilities": ["*"]} + if not client_is_lan(request): + raise HTTPException(403, "the admin token is LAN-only") + return {"email": None, "via": "admin-token", "capabilities": ["*"], "lan": True} raise HTTPException(401, "sign in, send a Bearer key, or a valid X-Admin-Token") diff --git a/app/identity.py b/app/identity.py index dacb61d..b2c5ec7 100644 --- a/app/identity.py +++ b/app/identity.py @@ -867,10 +867,24 @@ def _setting_https_url(v): return v +def _setting_choice(*allowed): + def check(v): + v = _setting_text(v).lower() + if v not in allowed: + raise IdentityError(422, "value must be one of %s" % ", ".join(allowed)) + return v + return check + + # key -> (code default, checker) SETTINGS_KEYS = { "nearby_source_name": ("Continental District unit list", _setting_text), "nearby_source_url": ("https://tinyurl.com/ContinentalScouts", _setting_https_url), + # Where API keys may be used from. "lan" is the historical rule, kept as + # the default. Sessions are never restricted (the console IS a session + # from anywhere) and the break-glass token is ALWAYS LAN-only - that one + # is not a choice, so it is not a setting. See admin_api.client_is_lan. + "api_keys_from": ("lan", _setting_choice("lan", "anywhere")), } diff --git a/tests/smoke_admin.py b/tests/smoke_admin.py index c8e4548..bd68039 100644 --- a/tests/smoke_admin.py +++ b/tests/smoke_admin.py @@ -159,6 +159,51 @@ check("leader cannot touch settings", not I.can(leader, "settings:write")) check("admin spans units", I.can(admin, "unit:write_own", "u1") and I.can(admin, "unit:write_own", "u2")) check("admin holds settings:write", I.can(admin, "settings:write")) +print("\nlan rule") +import admin_api as A +class _Req: + def __init__(self, ip, cookie=None, bearer=None): + self.headers = {"x-forwarded-for": ip} if ip else {} + if bearer: self.headers["authorization"] = "Bearer " + bearer + self.cookies = {"s73_session": cookie} if cookie else {} + self.client = None +for ip, want in (("10.0.0.55", True), ("10.0.1.20", True), ("127.0.0.1", True), ("172.19.0.31", True), + ("108.36.248.87", False), ("192.168.1.9", False), ("", False), ("garbage", False)): + check("client_is_lan(%r) is %s" % (ip, want), A.client_is_lan(_Req(ip)) == want) +check("api_keys_from defaults to lan", I.get_setting("api_keys_from") == "lan") +raises("api_keys_from rejects other values", 422, I.IdentityError, I.set_setting, "api_keys_from", "vpn") +# a key from outside is refused while lan, allowed once anywhere, admin token never from outside +A.ADMIN_TOKEN = "t" +import uuid as _uuid +con = I.connect() +pid = str(_uuid.uuid4()) +con.execute("INSERT INTO people (id, email, full_name, global_role, created_at) VALUES (?,?,?,?,?)", + (pid, "lanrule@example.test", "Lan Rule", "admin", I._now())); con.commit(); con.close() +person = I.get_person(pid) +full, _row = I.mint_api_key(person, "outside", ["leads:read"]) +def _hits(req, cap="leads:read"): + try: + A._auth(req, None, cap); return 200 + except Exception as e: + return getattr(e, "status_code", 500) +check("key from the LAN passes", _hits(_Req("10.0.0.55", bearer=full)) == 200) +check("key from outside is 403 while lan", _hits(_Req("108.36.248.87", bearer=full)) == 403) +I.set_setting("api_keys_from", "anywhere", actor="test") +check("key from outside passes once anywhere", _hits(_Req("108.36.248.87", bearer=full)) == 200) +check("key still cannot exceed its scopes from anywhere", _hits(_Req("108.36.248.87", bearer=full), "nearby:write") == 403) +I.set_setting("api_keys_from", None, actor="test") +check("clearing the setting restores lan", I.get_setting("api_keys_from") == "lan" + and _hits(_Req("108.36.248.87", bearer=full)) == 403) +tok = I.start_session(pid) +check("a session from outside is never restricted", _hits(_Req("108.36.248.87", cookie=tok)) == 200) +def _tok(req): + try: + A._auth(req, "t", "leads:read"); return 200 + except Exception as e: + return getattr(e, "status_code", 500) +check("admin token from the LAN passes", _tok(_Req("10.0.0.55")) == 200) +check("admin token from outside is 403, regardless of the setting", _tok(_Req("108.36.248.87")) == 403) + print("\napi docs registry") import admin_api as A reg = A.describe_routes() @@ -169,7 +214,7 @@ check("every route's capability is read from its own _auth call (except whoami): check("key routes gate on apikeys:own", all(d["capability"] == "apikeys:own" for d in reg if "/keys" in d["path"])) check("docs carry the handler docstring", all(d["doc"] for d in reg if "/keys" in d["path"])) class _R: - headers = {"authorization": ""}; cookies = {} + headers = {"authorization": "", "x-forwarded-for": "127.0.0.1"}; cookies = {}; client = None os.environ["ADMIN_TOKEN"] = "t"; A.ADMIN_TOKEN = "t" page = A.api_docs(_R(), "t").body.decode() check("docs page renders every route", page.count("") == len(reg))