From d5416128434aea661b43d886926dda560de70641 Mon Sep 17 00:00:00 2001 From: Brian Krabach Date: Wed, 8 Apr 2026 22:39:03 -0700 Subject: [PATCH] fix: read federation key live on each request and check PUT sync response Bug 1 (auth.py): AuthMiddleware.dispatch() was using self.federation_key (set once at startup) for Bearer token validation. If the key was generated or rotated via POST /api/federation/generate-key after startup, the old (often empty) value caused all federation auth to silently return 401. Fix: import load_federation_key from muxplex.settings and call it fresh on every non-exempt, non-cookie request. Also adds a warning log when a Bearer token is received but no key is configured on this server. Bug 2 (main.py): _sync_settings_with_remotes() discarded the PUT response, silently swallowing 401/500 errors from the remote sync endpoint. Fix: capture the PUT response as put_resp. Handle 409 (Conflict = remote is newer) with a debug log; raise_for_status() for any other non-2xx so errors propagate to the outer except and are logged as warnings. Tests: - test_dispatch_calls_load_federation_key_live: pattern test confirming load_federation_key() is called inside dispatch() - test_dispatch_does_not_use_stale_self_federation_key_for_bearer: pattern test confirming self.federation_key is gone from the live bearer check - test_sync_put_response_calls_raise_for_status: pattern test confirming raise_for_status() is called on the PUT response in the sync function - Updated existing bearer tests to monkeypatch load_federation_key so they are isolated from any real key file on disk --- muxplex/auth.py | 24 +++++++++--- muxplex/main.py | 7 +++- muxplex/tests/test_auth.py | 48 ++++++++++++++++++++++-- muxplex/tests/test_settings_sync_poll.py | 22 +++++++++++ 4 files changed, 91 insertions(+), 10 deletions(-) diff --git a/muxplex/auth.py b/muxplex/auth.py index bc1b636..cef4eb9 100644 --- a/muxplex/auth.py +++ b/muxplex/auth.py @@ -13,6 +13,8 @@ from starlette.middleware.base import BaseHTTPMiddleware from starlette.requests import Request from starlette.responses import JSONResponse, RedirectResponse, Response +from muxplex.settings import load_federation_key + _log = logging.getLogger(__name__) @@ -199,13 +201,23 @@ class AuthMiddleware(BaseHTTPMiddleware): if cookie and verify_session_cookie(self.secret, cookie, self.ttl_seconds): return await call_next(request) - # 4a. Bearer token (server-to-server federation) + # 4a. Bearer token (server-to-server federation). + # Read the key fresh from disk on every request so a key generated or + # rotated after startup (via POST /api/federation/generate-key) takes + # effect immediately without a server restart. auth_header = request.headers.get("authorization", "") - if self.federation_key and auth_header.lower().startswith("bearer "): - token = auth_header[7:] - if hmac.compare_digest(token, self.federation_key): - return await call_next(request) - _log.warning("federation: rejected Bearer from %s", client_host) + if auth_header.lower().startswith("bearer "): + federation_key = load_federation_key() + if not federation_key: + _log.warning( + "federation: Bearer token received from %s but no key configured on this server", + client_host, + ) + else: + token = auth_header[7:] + if hmac.compare_digest(token, federation_key): + return await call_next(request) + _log.warning("federation: rejected Bearer from %s", client_host) # 5. Authorization: Basic header auth_header = request.headers.get("authorization", "") diff --git a/muxplex/main.py b/muxplex/main.py index bbdeb4e..1e7617b 100644 --- a/muxplex/main.py +++ b/muxplex/main.py @@ -148,12 +148,17 @@ async def _sync_settings_with_remotes( }, "settings_updated_at": local_ts, } - await http_client.put( + put_resp = await http_client.put( f"{url}/api/settings/sync", json=payload, headers=headers, timeout=5.0, ) + if put_resp.status_code == 409: + # Remote is newer — let the next sync cycle pull. + _log.debug("Settings sync push to %s: 409 (remote is newer)", url) + else: + put_resp.raise_for_status() # If equal: no action. except Exception as exc: _log.warning("Settings sync with %s failed: %s", url, exc) diff --git a/muxplex/tests/test_auth.py b/muxplex/tests/test_auth.py index d88da8d..1d8351d 100644 --- a/muxplex/tests/test_auth.py +++ b/muxplex/tests/test_auth.py @@ -487,8 +487,16 @@ def _make_federation_app(federation_key: str = "fed-key") -> FastAPI: return test_app -def test_middleware_valid_bearer_token_passes(): - """Non-localhost with valid Bearer token and federation_key configured passes through (200).""" +def test_middleware_valid_bearer_token_passes(monkeypatch): + """Non-localhost with valid Bearer token passes through (200). + + Mocks load_federation_key in muxplex.auth so the test is isolated from any + real key file on disk — the fix reads the key live on each request instead + of using the stale self.federation_key set at startup. + """ + import muxplex.auth as auth_mod + + monkeypatch.setattr(auth_mod, "load_federation_key", lambda: "my-federation-secret") app = _make_federation_app(federation_key="my-federation-secret") client = TestClient(app, base_url="http://192.168.1.1") response = client.get( @@ -498,8 +506,11 @@ def test_middleware_valid_bearer_token_passes(): assert response.text == "OK" -def test_middleware_invalid_bearer_token_falls_through(): +def test_middleware_invalid_bearer_token_falls_through(monkeypatch): """Non-localhost with wrong Bearer token falls through to 401 (not accepted).""" + import muxplex.auth as auth_mod + + monkeypatch.setattr(auth_mod, "load_federation_key", lambda: "my-federation-secret") app = _make_federation_app(federation_key="my-federation-secret") client = TestClient(app, base_url="http://192.168.1.1", follow_redirects=False) response = client.get( @@ -525,3 +536,34 @@ def test_middleware_bearer_skipped_when_no_federation_key(): ) # Bearer should be skipped when federation_key is empty, falls through to 401 assert response.status_code == 401 + + +# --------------------------------------------------------------------------- +# Pattern tests: live federation key read in dispatch() +# --------------------------------------------------------------------------- + + +def test_dispatch_calls_load_federation_key_live(): + """dispatch() must call load_federation_key() live on each request, not use cached self.federation_key.""" + import inspect + + import muxplex.auth as auth_mod + + source = inspect.getsource(auth_mod.AuthMiddleware.dispatch) + assert "load_federation_key()" in source, ( + "dispatch() must call load_federation_key() live on each request " + "so key changes after startup are picked up without a restart" + ) + + +def test_dispatch_does_not_use_stale_self_federation_key_for_bearer(): + """dispatch() must NOT use self.federation_key for the live Bearer token check.""" + import inspect + + import muxplex.auth as auth_mod + + source = inspect.getsource(auth_mod.AuthMiddleware.dispatch) + assert "self.federation_key" not in source, ( + "dispatch() must not use self.federation_key (stale cached value); " + "use load_federation_key() instead so generate-key takes effect immediately" + ) diff --git a/muxplex/tests/test_settings_sync_poll.py b/muxplex/tests/test_settings_sync_poll.py index fdc87b2..e661fc8 100644 --- a/muxplex/tests/test_settings_sync_poll.py +++ b/muxplex/tests/test_settings_sync_poll.py @@ -248,3 +248,25 @@ async def test_sync_skips_remote_with_no_url(): http_client.get.assert_not_called() http_client.put.assert_not_called() + + +# --------------------------------------------------------------------------- +# Pattern test: PUT response is checked via raise_for_status() +# --------------------------------------------------------------------------- + + +def test_sync_put_response_calls_raise_for_status(): + """_sync_settings_with_remotes must capture the PUT response and call raise_for_status() on it.""" + import inspect + + source = inspect.getsource(main_mod._sync_settings_with_remotes) + # The PUT response must be assigned to a variable (not fire-and-forget) + assert "put_resp" in source, ( + "_sync_settings_with_remotes must capture the PUT response in put_resp " + "so errors (401, 500) are not silently swallowed" + ) + # raise_for_status() must be called on the captured response + assert "raise_for_status" in source, ( + "_sync_settings_with_remotes must call raise_for_status() on the PUT response " + "so non-2xx errors propagate to the outer exception handler" + )