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
This commit is contained in:
+15
-3
@@ -13,6 +13,8 @@ from starlette.middleware.base import BaseHTTPMiddleware
|
|||||||
from starlette.requests import Request
|
from starlette.requests import Request
|
||||||
from starlette.responses import JSONResponse, RedirectResponse, Response
|
from starlette.responses import JSONResponse, RedirectResponse, Response
|
||||||
|
|
||||||
|
from muxplex.settings import load_federation_key
|
||||||
|
|
||||||
_log = logging.getLogger(__name__)
|
_log = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
@@ -199,11 +201,21 @@ class AuthMiddleware(BaseHTTPMiddleware):
|
|||||||
if cookie and verify_session_cookie(self.secret, cookie, self.ttl_seconds):
|
if cookie and verify_session_cookie(self.secret, cookie, self.ttl_seconds):
|
||||||
return await call_next(request)
|
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", "")
|
auth_header = request.headers.get("authorization", "")
|
||||||
if self.federation_key and auth_header.lower().startswith("bearer "):
|
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:]
|
token = auth_header[7:]
|
||||||
if hmac.compare_digest(token, self.federation_key):
|
if hmac.compare_digest(token, federation_key):
|
||||||
return await call_next(request)
|
return await call_next(request)
|
||||||
_log.warning("federation: rejected Bearer from %s", client_host)
|
_log.warning("federation: rejected Bearer from %s", client_host)
|
||||||
|
|
||||||
|
|||||||
+6
-1
@@ -148,12 +148,17 @@ async def _sync_settings_with_remotes(
|
|||||||
},
|
},
|
||||||
"settings_updated_at": local_ts,
|
"settings_updated_at": local_ts,
|
||||||
}
|
}
|
||||||
await http_client.put(
|
put_resp = await http_client.put(
|
||||||
f"{url}/api/settings/sync",
|
f"{url}/api/settings/sync",
|
||||||
json=payload,
|
json=payload,
|
||||||
headers=headers,
|
headers=headers,
|
||||||
timeout=5.0,
|
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.
|
# If equal: no action.
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
_log.warning("Settings sync with %s failed: %s", url, exc)
|
_log.warning("Settings sync with %s failed: %s", url, exc)
|
||||||
|
|||||||
@@ -487,8 +487,16 @@ def _make_federation_app(federation_key: str = "fed-key") -> FastAPI:
|
|||||||
return test_app
|
return test_app
|
||||||
|
|
||||||
|
|
||||||
def test_middleware_valid_bearer_token_passes():
|
def test_middleware_valid_bearer_token_passes(monkeypatch):
|
||||||
"""Non-localhost with valid Bearer token and federation_key configured passes through (200)."""
|
"""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")
|
app = _make_federation_app(federation_key="my-federation-secret")
|
||||||
client = TestClient(app, base_url="http://192.168.1.1")
|
client = TestClient(app, base_url="http://192.168.1.1")
|
||||||
response = client.get(
|
response = client.get(
|
||||||
@@ -498,8 +506,11 @@ def test_middleware_valid_bearer_token_passes():
|
|||||||
assert response.text == "OK"
|
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)."""
|
"""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")
|
app = _make_federation_app(federation_key="my-federation-secret")
|
||||||
client = TestClient(app, base_url="http://192.168.1.1", follow_redirects=False)
|
client = TestClient(app, base_url="http://192.168.1.1", follow_redirects=False)
|
||||||
response = client.get(
|
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
|
# Bearer should be skipped when federation_key is empty, falls through to 401
|
||||||
assert response.status_code == 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"
|
||||||
|
)
|
||||||
|
|||||||
@@ -248,3 +248,25 @@ async def test_sync_skips_remote_with_no_url():
|
|||||||
|
|
||||||
http_client.get.assert_not_called()
|
http_client.get.assert_not_called()
|
||||||
http_client.put.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"
|
||||||
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user