diff --git a/horizon/authentication.py b/horizon/authentication.py index 342bb4e3..5ad853b2 100644 --- a/horizon/authentication.py +++ b/horizon/authentication.py @@ -1,8 +1,11 @@ import hmac +import threading +import time from typing import Annotated -from fastapi import Depends, HTTPException, status +from fastapi import Depends, HTTPException, Request, status from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer +from loguru import logger from horizon.config import MOCK_API_KEY, sidecar_config from horizon.startup.api_keys import get_env_api_key @@ -65,6 +68,84 @@ def enforce_pdp_token(credentials: PdpCredentials = None): raise HTTPException(status.HTTP_401_UNAUTHORIZED, detail="Invalid PDP token") +# Rate-limit for the warn-and-allow path of enforce_pdp_token_operational. While +# ENFORCE_OPERATIONAL_ROUTE_AUTH is off, every unauthenticated request to a governed route would +# otherwise emit one WARNING - which on the high-QPS /kong decision endpoint floods the synchronous +# stdout log sink (and adds a blocking write to the request path). Instead we coalesce to one line +# per route per interval that reports how many would-be rejections were allowed; that count is also +# the rollout signal - watch it fall to zero, then flip the flag on. The gate runs in a threadpool +# (it is a sync dependency), so the shared counters are guarded by a plain threading.Lock. +_OPERATIONAL_WARN_INTERVAL_SECONDS = 60.0 +_operational_warn_lock = threading.Lock() +# path -> (would-be rejections accumulated since the last emitted line, monotonic time of that emit) +_operational_warn_state: dict[str, tuple[int, float]] = {} + + +def reset_operational_warn_throttle() -> None: + """Clear the throttle state. For tests, whose asserted warnings must not be suppressed by a prior test.""" + with _operational_warn_lock: + _operational_warn_state.clear() + + +def _operational_warn_should_emit(path: str) -> tuple[bool, int]: + """Decide whether to emit the warn-and-allow line for ``path`` now, coalescing bursts. + + Returns ``(emit_now, count_since_last_emit)``. Every call counts as one would-be rejection; a + line is emitted on the first hit for a path and then at most once per + ``_OPERATIONAL_WARN_INTERVAL_SECONDS``, carrying the number of rejections coalesced into it. + """ + now = time.monotonic() + with _operational_warn_lock: + state = _operational_warn_state.get(path) + if state is None: + _operational_warn_state[path] = (0, now) + return True, 1 + pending, last_emit = state + pending += 1 + if now - last_emit >= _OPERATIONAL_WARN_INTERVAL_SECONDS: + _operational_warn_state[path] = (0, now) + return True, pending + _operational_warn_state[path] = (pending, last_emit) + return False, pending + + +def enforce_pdp_token_operational(request: Request, credentials: PdpCredentials = None): + """PDP-token gate for the operational routes hardened by PER-15244/PER-15245, with a rollout default. + + Governed by ``ENFORCE_OPERATIONAL_ROUTE_AUTH``. When it is true this is exactly ``enforce_pdp_token``. + When it is false - the default, for a safe fleet rollout - a request that would be rejected is allowed + through but logged, so callers that don't yet send the PDP token keep working while the logs surface + them before enforcement is switched on. + + The flag is read per-request (never captured at import) so a cloud control-plane override takes + effect and tests can toggle it. Kept as a distinct, named module-level function because the + fail-closed route audit recognises auth gates by callable name - a bare ``enforce_pdp_token`` here + could not carry the conditional behaviour, and an inline lambda would be invisible to the audit. + """ + # Reuse enforce_pdp_token's exact reject logic in one place. When the flag is on, a rejection is + # honoured (re-raised). When it is off - the rollout default - the rejection is downgraded to + # warn-and-allow so no caller breaks; every would-be rejection is still counted, and surfaced in a + # per-route coalesced warning (see _operational_warn_should_emit) rather than one line per request. + try: + enforce_pdp_token(credentials) + except HTTPException as exc: + if sidecar_config.ENFORCE_OPERATIONAL_ROUTE_AUTH: + raise + emit, coalesced = _operational_warn_should_emit(request.url.path) + if emit: + logger.warning( + "ENFORCE_OPERATIONAL_ROUTE_AUTH is off: allowed {count} request(s) without a valid PDP token " + "(missing or invalid) to {method} {path} in the last ~{interval}s that would otherwise be " + "rejected (e.g. {detail}). Set ENFORCE_OPERATIONAL_ROUTE_AUTH=true to enforce the PDP token " + "on this route.", + count=coalesced, + method=request.method, + path=request.url.path, + interval=int(_OPERATIONAL_WARN_INTERVAL_SECONDS), + detail=exc.detail, + ) + + def enforce_pdp_control_key(credentials: PdpCredentials = None): if sidecar_config.CONTAINER_CONTROL_KEY == MOCK_API_KEY: raise HTTPException( diff --git a/horizon/config.py b/horizon/config.py index 87fb42dc..651774e0 100644 --- a/horizon/config.py +++ b/horizon/config.py @@ -262,6 +262,18 @@ def parse_plugins(value: Any) -> dict[str, dict[str, int | bool | str]]: # enables debug ouptut for the Kong integration endpoint KONG_INTEGRATION_DEBUG = confi.bool("KONG_INTEGRATION_DEBUG", False) + ENFORCE_OPERATIONAL_ROUTE_AUTH = confi.bool( + "ENFORCE_OPERATIONAL_ROUTE_AUTH", + False, + description="When true, enforce the PDP token on the operational routes hardened by the auth-hardening " + "rollout: the update-trigger routes (/policy-updater/trigger, /data-updater/trigger, /update_policy, " + "/update_policy_data) and the /kong decision endpoint. Defaults to FALSE for a safe fleet rollout: those " + "routes accept unauthenticated requests and only LOG the ones that would be rejected, so callers that do not " + "yet send the token keep working while you watch the logs. Flip to true (per-fleet via the cloud control " + "plane, or PDP_ENFORCE_OPERATIONAL_ROUTE_AUTH) once every caller sends the token. Every other PDP route is " + "always enforced regardless of this flag.", + ) + LOCAL_FACTS_WAIT_TIMEOUT = confi.float( "LOCAL_FACTS_WAIT_TIMEOUT", 10, diff --git a/horizon/enforcer/api.py b/horizon/enforcer/api.py index 80193b74..a8759c60 100644 --- a/horizon/enforcer/api.py +++ b/horizon/enforcer/api.py @@ -277,6 +277,12 @@ async def health(): def init_enforcer_api_router(policy_store: BasePolicyStoreClient = None): # noqa: C901 policy_store = policy_store or DEFAULT_POLICY_STORE_GETTER() router = APIRouter() + # /kong lives on its own router so it can be mounted under the ENFORCE_OPERATIONAL_ROUTE_AUTH-aware + # gate (enforce_pdp_token_operational) while every other enforcer route keeps the plain + # enforce_pdp_token gate. Same closure as the rest of the enforcer routes: /kong's handler closes + # over kong_routes_table and the nested _is_allowed helper, so a router split here avoids a much + # larger refactor. Mounted separately in PermitPDP._configure_api_routes. + kong_router = APIRouter() if sidecar_config.KONG_INTEGRATION: with Path(KONG_ROUTES_TABLE_FILE).open() as f: kong_routes_table_raw = json.load(f) @@ -543,7 +549,7 @@ async def is_allowed_nginx( ) return {"allow": False, "result": False} - @router.post( + @kong_router.post( "/kong", response_model=KongAuthorizationResult, status_code=status.HTTP_200_OK, @@ -620,7 +626,7 @@ async def is_allowed_kong(request: Request, query: KongAuthorizationQuery): ) return {"allow": False, "result": False} - return router + return router, kong_router def _extract_regex_attributes(pattern: str, url: str) -> dict: diff --git a/horizon/pdp.py b/horizon/pdp.py index 24a2b7a5..c7225ab4 100644 --- a/horizon/pdp.py +++ b/horizon/pdp.py @@ -26,7 +26,7 @@ from opal_common.logging_utils.formatter import Formatter from scalar_fastapi import get_scalar_api_reference -from horizon.authentication import enforce_pdp_token +from horizon.authentication import enforce_pdp_token, enforce_pdp_token_operational from horizon.config import MOCK_API_KEY, sidecar_config from horizon.connectivity.api import init_connectivity_router from horizon.enforcer.api import init_enforcer_api_router, init_enforcer_health_router, stats_manager @@ -108,28 +108,32 @@ def apply_config(overrides_dict: dict, config_object: Confi): def _gate_opal_trigger_routes(app: FastAPI) -> None: - """Inject ``Depends(enforce_pdp_token)`` into the OPAL-mounted trigger routes. + """Inject ``Depends(enforce_pdp_token_operational)`` into the OPAL-mounted trigger routes. OpalClient mounts ``POST /policy-updater/trigger`` and ``POST /data-updater/trigger`` on the app before ``PermitPDP`` gains control (opal_client.client._configure_api_routes), so the include_router-level dependencies used for every PDP-owned router cannot reach - them. We inject the standard PDP-token dependency into the already-mounted route objects - instead, mirroring what FastAPI itself does at ``APIRoute.__init__`` (fastapi/routing.py): - insert a parameterless sub-dependant at the head of ``route.dependant.dependencies``. + them. We inject the PDP-token dependency into the already-mounted route objects instead, + mirroring what FastAPI itself does at ``APIRoute.__init__`` (fastapi/routing.py): insert a + parameterless sub-dependant at the head of ``route.dependant.dependencies``. + + The gate is the operational wrapper, so ``ENFORCE_OPERATIONAL_ROUTE_AUTH`` governs these routes + like the sibling operational routes: unauthenticated-but-logged by default, enforced when set. The Dependant is mutated IN PLACE - the route's request handler closes over that exact object, so the check is enforced on every request; do NOT reassign ``route.dependant``. - ``enforce_pdp_token`` only reads a header, so the route's body field needs no rebuild. + ``enforce_pdp_token_operational`` only reads the header and request, so the route's body field + needs no rebuild. Fails loud if a target route is missing (e.g. an OPAL upgrade renamed it): a silently - skipped injection would leave an update-trigger endpoint unauthenticated. + skipped injection would leave an update-trigger endpoint ungated even when enforcement is on. """ gated: set[str] = set() for route in app.routes: if isinstance(route, APIRoute) and route.path in OPAL_TRIGGER_ROUTE_PATHS: route.dependant.dependencies.insert( 0, - get_parameterless_sub_dependant(depends=Depends(enforce_pdp_token), path=route.path_format), + get_parameterless_sub_dependant(depends=Depends(enforce_pdp_token_operational), path=route.path_format), ) gated.add(route.path) @@ -143,6 +147,23 @@ def _gate_opal_trigger_routes(app: FastAPI) -> None: raise SystemExit(GUNICORN_EXIT_APP) +def _warn_if_operational_route_auth_disabled() -> None: + """Warn when ``ENFORCE_OPERATIONAL_ROUTE_AUTH`` is off and the operational routes accept unauthenticated calls. + + This is the safe-rollout default (see SidecarConfig.ENFORCE_OPERATIONAL_ROUTE_AUTH): the token is not + enforced on the update-trigger routes or /kong, only logged. Surfaced at startup as well as per-request + so a fleet still in the permissive rollout phase stays visible; flip the flag on once every caller sends + the token. + """ + if not sidecar_config.ENFORCE_OPERATIONAL_ROUTE_AUTH: + logger.warning( + "ENFORCE_OPERATIONAL_ROUTE_AUTH is OFF: the update-trigger routes (/policy-updater/trigger, " + "/data-updater/trigger, /update_policy, /update_policy_data) and /kong accept requests WITHOUT " + "a valid PDP token (missing or invalid) - would-be rejections are only logged. This is the safe " + "fleet-rollout default; set ENFORCE_OPERATIONAL_ROUTE_AUTH=true to enforce the PDP token on these routes." + ) + + def _warn_if_opal_verifier_disabled(opal_client: OpalClient) -> None: """Warn loudly when the OPAL-authenticated routes are effectively open. @@ -447,7 +468,7 @@ def _configure_api_routes(self, app: FastAPI): app.on_event("shutdown")(stats_manager.stop_tasks) enforcer_health_router = init_enforcer_health_router() - enforcer_router = init_enforcer_api_router(policy_store=self._opal.policy_store) + enforcer_router, kong_router = init_enforcer_api_router(policy_store=self._opal.policy_store) local_router = init_local_cache_api_router(policy_store=self._opal.policy_store) # Init system router system_router = init_system_api_router() @@ -460,6 +481,14 @@ def _configure_api_routes(self, app: FastAPI): tags=["Authorization API"], dependencies=[Depends(enforce_pdp_token)], ) + # /kong is gated with the operational wrapper so ENFORCE_OPERATIONAL_ROUTE_AUTH governs it + # during a fleet rollout (Kong's OPA plugin may not yet forward the PDP token) without + # touching the rest of the enforcer router. + app.include_router( + kong_router, + tags=["Authorization API"], + dependencies=[Depends(enforce_pdp_token_operational)], + ) app.include_router( local_router, @@ -498,11 +527,13 @@ def _configure_api_routes(self, app: FastAPI): ) # TODO: remove this when clients update sdk version (legacy routes) + # Gated with the operational wrapper: these are aliases of the OPAL trigger routes, so they + # follow the same ENFORCE_OPERATIONAL_ROUTE_AUTH rollout default. @app.post( "/update_policy", status_code=status.HTTP_200_OK, include_in_schema=False, - dependencies=[Depends(enforce_pdp_token)], + dependencies=[Depends(enforce_pdp_token_operational)], ) async def legacy_trigger_policy_update(): logger.info("triggered policy update from api (legacy route)") @@ -515,7 +546,7 @@ async def legacy_trigger_policy_update(): "/update_policy_data", status_code=status.HTTP_200_OK, include_in_schema=False, - dependencies=[Depends(enforce_pdp_token)], + dependencies=[Depends(enforce_pdp_token_operational)], ) async def legacy_trigger_data_update(): logger.info("triggered policy data update from api (legacy route)") @@ -534,6 +565,9 @@ async def legacy_trigger_data_update(): # High-signal warning if the OPAL-authenticated routes are left open by a disabled # verifier (must never happen in a managed PDP). _warn_if_opal_verifier_disabled(self._opal) + # High-signal warning while ENFORCE_OPERATIONAL_ROUTE_AUTH is off (the rollout default) and + # the update-trigger and /kong routes accept unauthenticated requests. + _warn_if_operational_route_auth_disabled() @property def app(self): diff --git a/horizon/tests/conftest.py b/horizon/tests/conftest.py index e44935e7..6dc5e3e2 100644 --- a/horizon/tests/conftest.py +++ b/horizon/tests/conftest.py @@ -22,8 +22,23 @@ from unittest.mock import Mock import aioresponses.core as _aioresponses_core +import horizon.authentication as _authentication +import pytest from aiohttp.client_reqrep import ClientResponse as _ClientResponse + +@pytest.fixture(autouse=True) +def _reset_operational_warn_throttle(): + """Give each test a clean warn-and-allow throttle window. + + enforce_pdp_token_operational coalesces its "unauthenticated but allowed" warning to one line per + route per interval, and that state is process-global. Without this reset, a test that asserts the + warning could be silently suppressed by an earlier test that already logged for the same path. + """ + _authentication.reset_operational_warn_throttle() + yield + + if "stream_writer" in inspect.signature(_ClientResponse.__init__).parameters: class _StreamWriterCompatClientResponse(_ClientResponse): diff --git a/horizon/tests/test_authentication.py b/horizon/tests/test_authentication.py index 1ba336af..77798e19 100644 --- a/horizon/tests/test_authentication.py +++ b/horizon/tests/test_authentication.py @@ -7,6 +7,9 @@ public-route allowlist that the route-audit test relies on. """ +import time +from types import SimpleNamespace + import horizon.authentication as auth import pytest from fastapi import HTTPException, status @@ -16,8 +19,10 @@ _token_matches, enforce_pdp_control_key, enforce_pdp_token, + enforce_pdp_token_operational, ) from horizon.config import MOCK_API_KEY, sidecar_config +from loguru import logger VALID_TOKEN = "s3cr3t-token" @@ -27,6 +32,11 @@ def _creds(token: str) -> HTTPAuthorizationCredentials: return HTTPAuthorizationCredentials(scheme="Bearer", credentials=token) +def _fake_request(path: str = "/policy-updater/trigger", method: str = "POST") -> SimpleNamespace: + """A stand-in for the Request; the compat wrapper only reads ``.method`` and ``.url.path``.""" + return SimpleNamespace(method=method, url=SimpleNamespace(path=path)) + + @pytest.mark.parametrize( ("credentials", "expected"), [ @@ -69,6 +79,90 @@ def test_wrong_token_is_401(self): assert exc.value.detail == "Invalid PDP token" +class TestEnforcePdpTokenOperational: + """The ENFORCE_OPERATIONAL_ROUTE_AUTH wrapper: enforce when on, warn-and-allow when off (default).""" + + WARN_SUBSTRING = "ENFORCE_OPERATIONAL_ROUTE_AUTH is off" + + @pytest.fixture(autouse=True) + def _patch_key(self, monkeypatch): + monkeypatch.setattr(auth, "get_env_api_key", lambda: VALID_TOKEN) + + @pytest.fixture + def enforce_on(self, monkeypatch): + monkeypatch.setattr(sidecar_config, "ENFORCE_OPERATIONAL_ROUTE_AUTH", True) + + @pytest.fixture + def enforce_off(self, monkeypatch): + monkeypatch.setattr(sidecar_config, "ENFORCE_OPERATIONAL_ROUTE_AUTH", False) + + @pytest.mark.usefixtures("enforce_on") + def test_enforce_on_missing_credentials_is_401(self): + # Flag on: identical to enforce_pdp_token. + with pytest.raises(HTTPException) as exc: + enforce_pdp_token_operational(_fake_request(), credentials=None) + assert exc.value.status_code == status.HTTP_401_UNAUTHORIZED + assert exc.value.detail == "Missing Authorization header" + + @pytest.mark.usefixtures("enforce_on") + def test_enforce_on_wrong_token_is_401(self): + with pytest.raises(HTTPException) as exc: + enforce_pdp_token_operational(_fake_request(), credentials=_creds("nope")) + assert exc.value.status_code == status.HTTP_401_UNAUTHORIZED + assert exc.value.detail == "Invalid PDP token" + + @pytest.mark.usefixtures("enforce_on") + def test_enforce_on_valid_token_passes(self): + assert enforce_pdp_token_operational(_fake_request(), credentials=_creds(VALID_TOKEN)) is None + + @pytest.mark.usefixtures("enforce_off") + def test_enforce_off_missing_credentials_is_allowed_and_warns(self, capture_loguru): + # The rollout default: a request that would be rejected is let through, but logged. + assert enforce_pdp_token_operational(_fake_request(), credentials=None) is None + assert any(self.WARN_SUBSTRING in record for record in capture_loguru) + + @pytest.mark.usefixtures("enforce_off") + def test_enforce_off_wrong_token_is_allowed_and_warns(self, capture_loguru): + assert enforce_pdp_token_operational(_fake_request(), credentials=_creds("nope")) is None + assert any(self.WARN_SUBSTRING in record for record in capture_loguru) + + @pytest.mark.usefixtures("enforce_off") + def test_enforce_off_valid_token_passes_without_warning(self, capture_loguru): + # A caller that already sends the token is not flagged - only would-be rejections warn. + assert enforce_pdp_token_operational(_fake_request(), credentials=_creds(VALID_TOKEN)) is None + assert not any(self.WARN_SUBSTRING in record for record in capture_loguru) + + @pytest.mark.usefixtures("enforce_off") + def test_enforce_off_coalesces_repeated_warnings(self, capture_loguru): + # The warn-and-allow path must not log once per request (that floods the hot /kong endpoint): + # a burst of would-be rejections on one route collapses to a single warning line. + for _ in range(50): + assert enforce_pdp_token_operational(_fake_request(path="/kong"), credentials=None) is None + assert sum(self.WARN_SUBSTRING in record for record in capture_loguru) == 1 + + +def test_operational_warn_throttle_coalesces_count(): + # First hit for a path emits immediately (count 1); further hits within the interval are counted + # but suppressed; once the interval elapses the next hit emits carrying the coalesced total. + auth.reset_operational_warn_throttle() + assert auth._operational_warn_should_emit("/kong") == (True, 1) + for _ in range(4): + emit, _count = auth._operational_warn_should_emit("/kong") + assert emit is False + # Simulate the interval having elapsed without waiting on the wall clock. + pending, _last = auth._operational_warn_state["/kong"] + auth._operational_warn_state["/kong"] = (pending, time.monotonic() - (auth._OPERATIONAL_WARN_INTERVAL_SECONDS + 1)) + assert auth._operational_warn_should_emit("/kong") == (True, 5) + + +@pytest.fixture +def capture_loguru(): + records: list[str] = [] + sink_id = logger.add(lambda message: records.append(str(message)), level="WARNING") + yield records + logger.remove(sink_id) + + class TestEnforcePdpControlKey: CONTROL_KEY = "control-key" diff --git a/horizon/tests/test_enforcer_api.py b/horizon/tests/test_enforcer_api.py index 84052175..96e90db5 100644 --- a/horizon/tests/test_enforcer_api.py +++ b/horizon/tests/test_enforcer_api.py @@ -81,7 +81,18 @@ async def pdp_api_client() -> TestClient: } +@pytest.fixture +def enforce_operational_auth(monkeypatch): + """Enable ENFORCE_OPERATIONAL_ROUTE_AUTH so the conditionally-gated route (/kong) also rejects. + + The 8 other endpoints in the sweep carry the plain enforce_pdp_token gate and reject regardless; + /kong defaults to permissive (rollout default), so the sweep turns enforcement on to cover it too. + """ + monkeypatch.setattr(sidecar_config, "ENFORCE_OPERATIONAL_ROUTE_AUTH", True) + + @pytest.mark.parametrize("endpoint", PROTECTED_ENFORCER_ENDPOINTS) +@pytest.mark.usefixtures("enforce_operational_auth") def test_enforcer_endpoint_missing_token_returns_401(endpoint): client = TestClient(sidecar._app) response = client.post(endpoint, json={}) @@ -90,6 +101,7 @@ def test_enforcer_endpoint_missing_token_returns_401(endpoint): @pytest.mark.parametrize("endpoint", PROTECTED_ENFORCER_ENDPOINTS) +@pytest.mark.usefixtures("enforce_operational_auth") def test_enforcer_endpoint_invalid_token_returns_401(endpoint): client = TestClient(sidecar._app) response = client.post(endpoint, headers={"authorization": "Bearer wrong_token"}, json={}) @@ -115,11 +127,22 @@ def test_kong_endpoint_valid_token_integration_disabled_returns_503(): assert response.status_code == status.HTTP_503_SERVICE_UNAVAILABLE +def test_kong_endpoint_permissive_default_bypasses_auth(): + # Rollout default (ENFORCE_OPERATIONAL_ROUTE_AUTH off): a tokenless /kong is no longer 401 - the + # gate lets it through to the handler, which then 503s because KONG_INTEGRATION is off. The 503 + # (not 401) is the proof that the auth gate allowed it through. + client = TestClient(sidecar._app) + response = client.post("/kong", json=KONG_QUERY) + assert response.status_code == status.HTTP_503_SERVICE_UNAVAILABLE + + def test_kong_endpoint_enabled_integration_allowed_flow(tmp_path, monkeypatch): routes_file = tmp_path / "kong_routes.json" routes_file.write_text('[["^/resource1/.*$", "resource1"]]') monkeypatch.setattr("horizon.enforcer.api.KONG_ROUTES_TABLE_FILE", str(routes_file)) monkeypatch.setattr(sidecar_config, "KONG_INTEGRATION", True) + # /kong defaults to permissive; enable enforcement so the tokenless call below is a clean 401. + monkeypatch.setattr(sidecar_config, "ENFORCE_OPERATIONAL_ROUTE_AUTH", True) class FakeStateHandler: async def seen_sdk(self, _sdk: str) -> None: diff --git a/horizon/tests/test_legacy_update_routes.py b/horizon/tests/test_legacy_update_routes.py index 8cbcdc68..190353f7 100644 --- a/horizon/tests/test_legacy_update_routes.py +++ b/horizon/tests/test_legacy_update_routes.py @@ -35,6 +35,9 @@ def test_update_policy_triggers_updater(pdp: MockPermitPDP, auth: dict[str, str] def test_update_policy_rejects_unauthenticated(pdp: MockPermitPDP, monkeypatch): + # Rejection on these routes is now opt-in (ENFORCE_OPERATIONAL_ROUTE_AUTH defaults off for a + # safe fleet rollout); enable enforcement to assert the reject behaviour. + monkeypatch.setattr(sidecar_config, "ENFORCE_OPERATIONAL_ROUTE_AUTH", True) trigger = AsyncMock() monkeypatch.setattr(pdp._opal.policy_updater, "trigger_update_policy", trigger) client = TestClient(pdp._app) @@ -74,6 +77,8 @@ def test_update_policy_data_returns_503_when_updater_disabled(pdp: MockPermitPDP def test_update_policy_data_rejects_unauthenticated(pdp: MockPermitPDP, monkeypatch): + # Rejection is opt-in now (see test_update_policy_rejects_unauthenticated); enable enforcement. + monkeypatch.setattr(sidecar_config, "ENFORCE_OPERATIONAL_ROUTE_AUTH", True) get_base = AsyncMock() monkeypatch.setattr(pdp._opal.data_updater, "get_base_policy_data", get_base) client = TestClient(pdp._app) diff --git a/horizon/tests/test_opal_trigger_auth.py b/horizon/tests/test_opal_trigger_auth.py index 9d0cad7c..97368cf4 100644 --- a/horizon/tests/test_opal_trigger_auth.py +++ b/horizon/tests/test_opal_trigger_auth.py @@ -16,13 +16,16 @@ from fastapi.testclient import TestClient from horizon.config import sidecar_config from horizon.enforcer.api import stats_manager -from horizon.pdp import PermitPDP, _warn_if_opal_verifier_disabled +from horizon.pdp import PermitPDP, _warn_if_opal_verifier_disabled, _warn_if_operational_route_auth_disabled from loguru import logger from opal_client.client import OpalClient from starlette import status VALID_TOKEN = "mock_api_key" TRIGGER_ROUTES = ["/policy-updater/trigger", "/data-updater/trigger"] +# The legacy SDK aliases of the two trigger routes; gated with the same operational wrapper. +LEGACY_TRIGGER_ROUTES = ["/update_policy", "/update_policy_data"] +OPERATIONAL_UPDATE_ROUTES = TRIGGER_ROUTES + LEGACY_TRIGGER_ROUTES class MockPermitPDP(PermitPDP): @@ -49,7 +52,18 @@ def _auth(token: str) -> dict[str, str]: return {"Authorization": f"Bearer {token}"} +@pytest.fixture +def enforce_on(monkeypatch): + """Turn ENFORCE_OPERATIONAL_ROUTE_AUTH on (read per-request by the gate) so these routes reject.""" + monkeypatch.setattr(sidecar_config, "ENFORCE_OPERATIONAL_ROUTE_AUTH", True) + + +# The trigger-route enforcement tests below assert the reject behaviour, which is now opt-in +# (ENFORCE_OPERATIONAL_ROUTE_AUTH defaults to off for a safe fleet rollout), so they enable it. + + @pytest.mark.parametrize("path", TRIGGER_ROUTES) +@pytest.mark.usefixtures("enforce_on") def test_trigger_route_without_token_is_401(client: TestClient, path: str): # The actual vulnerability: OPAL-mounted trigger routes were callable unauthenticated. resp = client.post(path) @@ -59,12 +73,14 @@ def test_trigger_route_without_token_is_401(client: TestClient, path: str): @pytest.mark.parametrize("path", TRIGGER_ROUTES) def test_trigger_route_with_valid_token_is_not_blocked(client: TestClient, path: str): - # The dependency passes; the handler may 500 offline, but it must not be a 401. + # The dependency passes; the handler may 500 offline, but it must not be a 401. True in both + # enforcement modes, so left flag-agnostic. resp = client.post(path, headers=_auth(VALID_TOKEN)) assert resp.status_code != status.HTTP_401_UNAUTHORIZED @pytest.mark.parametrize("path", TRIGGER_ROUTES) +@pytest.mark.usefixtures("enforce_on") def test_trigger_route_with_wrong_token_is_401(client: TestClient, path: str): resp = client.post(path, headers=_auth("wrong-token")) assert resp.status_code == status.HTTP_401_UNAUTHORIZED @@ -73,6 +89,7 @@ def test_trigger_route_with_wrong_token_is_401(client: TestClient, path: str): @pytest.mark.parametrize("path", TRIGGER_ROUTES) @pytest.mark.parametrize("value", ["garbage", "Bearer", "Bearer ", "Bearer a b c"]) +@pytest.mark.usefixtures("enforce_on") def test_trigger_route_malformed_header_is_401_not_500(client: TestClient, path: str, value: str): # Regression for the unguarded split(" ") -> ValueError -> 500 footgun. resp = client.post(path, headers={"Authorization": value}) @@ -108,6 +125,44 @@ def test_exit_without_header_is_503_when_control_key_unset(client: TestClient): assert client.post("/_exit").status_code == status.HTTP_503_SERVICE_UNAVAILABLE +@pytest.mark.parametrize("path", OPERATIONAL_UPDATE_ROUTES) +@pytest.mark.usefixtures("enforce_on") +def test_update_route_without_token_is_401_when_enforced(client: TestClient, path: str): + # With enforcement on, the update-trigger + legacy routes reject a tokenless call. + assert client.post(path).status_code == status.HTTP_401_UNAUTHORIZED + + +@pytest.mark.parametrize("path", OPERATIONAL_UPDATE_ROUTES) +def test_update_route_without_token_is_allowed_by_default(client: TestClient, path: str): + # Rollout default (enforcement off, no flag set here): a tokenless call is not blocked. It may + # 500 on real offline I/O once it reaches the handler, but it must not be the auth 401 - which + # proves the gate allowed it through. + assert client.post(path).status_code != status.HTTP_401_UNAUTHORIZED + + +@pytest.mark.parametrize("path", OPERATIONAL_UPDATE_ROUTES) +def test_update_route_wrong_token_is_allowed_by_default(client: TestClient, path: str): + assert client.post(path, headers=_auth("wrong-token")).status_code != status.HTTP_401_UNAUTHORIZED + + +def test_default_permissive_logs_would_be_rejection(client: TestClient, capture_loguru): + # The escape hatch is observable: each would-be rejection is logged so lagging callers surface. + client.post("/policy-updater/trigger") + assert any("ENFORCE_OPERATIONAL_ROUTE_AUTH is off" in record for record in capture_loguru) + + +def test_warn_if_operational_route_auth_disabled_fires_when_off(monkeypatch, capture_loguru): + monkeypatch.setattr(sidecar_config, "ENFORCE_OPERATIONAL_ROUTE_AUTH", False) + _warn_if_operational_route_auth_disabled() + assert any("ENFORCE_OPERATIONAL_ROUTE_AUTH is OFF" in record for record in capture_loguru) + + +def test_warn_if_operational_route_auth_disabled_silent_when_on(monkeypatch, capture_loguru): + monkeypatch.setattr(sidecar_config, "ENFORCE_OPERATIONAL_ROUTE_AUTH", True) + _warn_if_operational_route_auth_disabled() + assert not any("ENFORCE_OPERATIONAL_ROUTE_AUTH is OFF" in record for record in capture_loguru) + + def test_warn_if_opal_verifier_disabled_fires(capture_loguru): # The real MockPermitPDP OpalClient runs with no public key -> verifier disabled. _warn_if_opal_verifier_disabled(_sidecar._opal) diff --git a/horizon/tests/test_route_auth_audit.py b/horizon/tests/test_route_auth_audit.py index 0291f5a4..820edd18 100644 --- a/horizon/tests/test_route_auth_audit.py +++ b/horizon/tests/test_route_auth_audit.py @@ -35,6 +35,10 @@ AUTH_GATE_CALLABLES: frozenset[str] = frozenset( { "enforce_pdp_token", + # ENFORCE_OPERATIONAL_ROUTE_AUTH rollout wrapper on the update-trigger and /kong routes. This + # audit is structural: it asserts a gate is attached, not that the gate rejects at runtime + # (that gate warns-and-allows while enforcement is off, the rollout default). + "enforce_pdp_token_operational", "enforce_pdp_control_key", "JWTAuthenticator", "require_listener_token", @@ -102,10 +106,15 @@ def test_no_route_is_unprotected(): @pytest.mark.parametrize("path", sorted(OPAL_TRIGGER_ROUTE_PATHS)) def test_opal_trigger_route_is_pdp_gated(path: str): - """Regression guard for the actual fix: the OPAL-mounted trigger routes require the PDP token.""" + """Regression guard for the actual fix: the OPAL-mounted trigger routes carry the PDP-token gate. + + They are gated with the operational wrapper (enforce_pdp_token_operational) so + ENFORCE_OPERATIONAL_ROUTE_AUTH governs them during a rollout; with the flag on the wrapper enforces + the token identically to enforce_pdp_token. This asserts the gate is attached, not its runtime mode. + """ by_path = {route.path: route for route in _sidecar._app.routes if isinstance(route, APIRoute)} assert path in by_path, f"{path} is no longer mounted (OPAL rename?) - _gate_opal_trigger_routes must be updated" - assert "enforce_pdp_token" in _route_auth_gates(by_path[path]) + assert "enforce_pdp_token_operational" in _route_auth_gates(by_path[path]) def test_audit_detects_a_bare_ungated_route():