diff --git a/CLAUDE.md b/CLAUDE.md index f384844..d1e8485 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -114,6 +114,10 @@ For HTTP transport, the wire protocol maps to separate endpoints: `POST /vgi/{me **Authentication**: `AuthContext` (frozen dataclass) carries `domain`, `authenticated`, `principal`, and `claims`. For HTTP transport, `make_wsgi_app(authenticate=...)` installs `_AuthMiddleware` that calls the callback on each request and populates `CallContext.auth`. Pipe transport gets anonymous auth by default. Methods can call `ctx.auth.require_authenticated()` to gate access. +**Unauthorized responses (HTTP)**: Every 401 follows `docs/unauthorized-spec.md`. `vgi_rpc/http/_unauthorized.py` defines `AuthReason` (closed set: `missing_credential`, `invalid_credential`, `expired_credential`, `insufficient_scope`, `proxy_required`, `unauthorized`), `AuthFailure` (a `ValueError` carrying a reason, so `chain_authenticate` still treats it as "try the next credential"), and `AuthenticationError` (an `RpcError` subclass the client raises, with `reason` / `detail` / `proxy_hint`). `_AuthMiddleware` classifies the callback's exception onto `req.context.vgi_auth_reason`; `_make_error_serializer` in `http/server/_errors.py` renders **JSON** (`{error, reason, detail, proxy_hint?}`) unless `Accept` contains `text/html`, in which case it renders the styled page. Both carry `VGI-Auth-Reason` and `Cache-Control: no-store`. + +The reason code names the *stage*, never a verifier's diagnosis — every proxy-proof outcome collapses onto `proxy_required`, which is what keeps `docs/proxy-proof-spec.md` §6's uniform-rejection rule intact. **The proxy note** (`VGI-Auth-Proxy-Required: true` + `proxy_hint`) is derived from *server configuration*, not from what failed on a given request, so it is identical on every 401 and discloses nothing: it exists for the deployment where the reverse proxy isn't forwarding the header and *everything* 401s. Header dependencies are auto-discovered — `mtls_authenticate*` and `proxy_proof_gate` (require mode only) call `declare_proxy_headers` on themselves, and `chain_authenticate` / `require_all` propagate; `make_wsgi_app(proxy_auth_headers=[...])` states them directly for a custom authenticator. Cross-language conformance group: `TestUnauthorized`, capability-gated on `VGI-Auth-Reason`. + **Server identity**: Each `RpcServer` gets a `server_id` (auto-generated 12-char hex or caller-supplied). This ID is attached to all log and error batches as `vgi_rpc.server_id` metadata for distributed tracing. `RpcServer` also accepts `enable_describe=True` to register the synthetic `__describe__` introspection method. **Protocol identity**: `RpcServer` computes `protocol_hash` (SHA-256) over the canonical `__describe__` payload at construction. Three answer-different questions: diff --git a/README.md b/README.md index a09a796..008c907 100644 --- a/README.md +++ b/README.md @@ -505,6 +505,7 @@ with http_connect(MyService, "http://localhost:8080") as proxy: | `enable_health_endpoint` | `True` | JSON health check at `GET {prefix}/health` (bypasses auth) | | `repo_url` | `None` | Source-repository link surfaced on the landing page | | `oauth_resource_metadata` | `None` | `OAuthResourceMetadata` for RFC 9728 discovery | +| `proxy_auth_headers` | `None` | Headers a trusted reverse proxy must inject for auth to succeed. Adds a "check the proxy configuration" note to every [401](docs/unauthorized-spec.md). The built-in mTLS and proxy-proof authenticators are discovered automatically; this is for a custom `authenticate` | | `enable_sticky` | `False` | Enable HTTP [sticky sessions](docs/sticky-sessions-spec.md) — `ctx.open_session(state)` binds a Python object to the worker for the session token's lifetime | | `sticky_default_ttl` | `300.0` | Default session TTL in seconds for sticky sessions; overridable per-call via `ctx.open_session(state, ttl=...)` | | `sticky_echo_headers` | `None` | Headers the client replays on every subsequent request in the session (e.g. Fly.io routing) | diff --git a/docs/WIRE_PROTOCOL.md b/docs/WIRE_PROTOCOL.md index 49b1891..2863403 100644 --- a/docs/WIRE_PROTOCOL.md +++ b/docs/WIRE_PROTOCOL.md @@ -551,7 +551,7 @@ Response body: IPC stream (result_schema, 0..N log batches, 1 result/error batch HTTP 200: Success (even when the response contains an error batch) HTTP 400: Protocol error (bad IPC, missing metadata, param validation failure) -HTTP 401: Authentication failure (plain-text body, NOT Arrow IPC) +HTTP 401: Authentication failure (JSON envelope or HTML page, NOT Arrow IPC) HTTP 404: Unknown method HTTP 415: Wrong Content-Type HTTP 500: Server implementation error @@ -698,7 +698,7 @@ is inside the ciphertext, so it cannot be tampered with independently. When the server has an `authenticate` callback configured: - The callback receives the HTTP request and returns an `AuthContext`. -- On failure (`ValueError` or `PermissionError`), the server returns **HTTP 401** with a **plain-text** body (NOT Arrow IPC), because no method has been resolved yet and no output schema is available. +- On failure (`ValueError` or `PermissionError`), the server returns **HTTP 401**. The body is NOT Arrow IPC — no method has been resolved yet, so no output schema is available. Its shape is the standardized envelope of `docs/unauthorized-spec.md`: a JSON object carrying a `reason` code from a closed set, mirrored on a `VGI-Auth-Reason` header, or the styled HTML page when the request's `Accept` asks for `text/html`. - Other exceptions from the callback propagate as HTTP 500. - Clients MUST detect 401 responses before attempting to parse Arrow IPC. @@ -710,8 +710,9 @@ per-worker secret. - It is a **precondition ANDed with** the `authenticate` callback above, never an alternative credential — the caller's `Authorization` header is untouched and still carries the end user. -- Failure maps to the same **HTTP 401 + plain-text body** as any other authenticate failure. The - body MUST NOT echo the reason or the claimed key id. +- Failure maps to the same **HTTP 401** as any other authenticate failure, carrying the + `proxy_required` reason code. The body MUST NOT echo the verifier's reason or the claimed key + id — every proof outcome collapses onto that one code. - `OPTIONS`, `/.well-known/`, and `{prefix}/health` are exempt in all modes, so load-balancer probes and capability discovery keep working. - A worker requiring proofs advertises `VGI-Proxy-Proof-Required: true` on every response. @@ -965,8 +966,8 @@ IPC Stream (error): | Type error in implementation | 400 Bad Request | > **Note**: Even for HTTP 400/500 responses, the response body is a valid -> Arrow IPC stream containing an error batch, except for 401 (plain text) -> and 415 (Falcon default response). +> Arrow IPC stream containing an error batch, except for 401 (JSON or HTML, +> per `docs/unauthorized-spec.md`) and 415 (Falcon default response). --- diff --git a/docs/index.md b/docs/index.md index fde66cf..ad06f3d 100644 --- a/docs/index.md +++ b/docs/index.md @@ -37,6 +37,7 @@ Define RPC interfaces as Python [`Protocol`](https://docs.python.org/3/library/t - **IPC validation** — configurable batch validation levels for untrusted data - **Large batch support** — transparent [externalization to S3/GCS](api/external.md) for oversized data - **HTTP response caps** — bound on-wire body size and external upload volume per call via [`max_response_bytes` and `max_externalized_response_bytes`](hosting.md) +- **Standardized 401s** — every HTTP rejection carries a reason code from a closed set, plus a note when the cause is likely a missing reverse-proxy configuration — see [Unauthorized Responses](unauthorized-spec.md) - **Sticky sessions** — opt-in HTTP affinity binding stateful objects (cursors, model handles) to a client across calls — see [Sticky Sessions](sticky-sessions-spec.md) - **Per-call I/O statistics** — [`CallStatistics`](api/core.md#callstatistics) tracks batches, rows, and bytes for usage accounting (access log + [OTel](api/otel.md) spans) - **Wire protocol debug logging** — enable `vgi_rpc.wire` at DEBUG for full wire-level visibility — see [Logging](api/logging.md#wire-protocol-debugging) diff --git a/docs/porting-guide.md b/docs/porting-guide.md index 644b26d..373c697 100644 --- a/docs/porting-guide.md +++ b/docs/porting-guide.md @@ -196,6 +196,18 @@ ignore the budget. - **HTTP state-token format.** Tokens are AEAD-sealed (XChaCha20-Poly1305 or ChaCha20-Poly1305, depending on what's available natively in the target language). Each port is free to choose its own plaintext encoding — Python uses length-prefixed Arrow IPC, Go uses gob, TypeScript uses JSON+BigInt, Java uses CBOR, Rust uses length-prefixed bytes — because tokens are not expected to round-trip across language ports. The behavioral contract is per-port: round-trip integrity, cross-principal replay protection (via AEAD AAD or per-principal key derivation), and TTL enforcement after authenticity. See `vgi_rpc/http/server/_state_token.py` for the Python reference. - **Per-process server identity.** `server_id` is generated once per process lifetime, NOT per call. The same string must appear in every log record from the same instance. +## HTTP unauthorized responses + +Every 401 a conformant HTTP server emits carries a coarse reason code from a closed set, on a `VGI-Auth-Reason` header and in a JSON body, plus a static note on services whose authentication depends on a reverse proxy. The full spec lives at [`docs/unauthorized-spec.md`](unauthorized-spec.md). The canonical `TestUnauthorized` group is capability-gated on `VGI-Auth-Reason`, so a port that has not adopted this skips cleanly. + +The pieces worth calling out: + +1. **Negotiate on `Accept`, and default to JSON.** `*/*` — what every RPC client sends — MUST resolve to the JSON envelope. A port may serve JSON to browsers too; it may never serve HTML to a client that did not ask for `text/html`. Getting this backwards is the pre-existing bug this spec was written to fix: the Python client used to paste an entire HTML page into an exception message. +2. **Keep the reason set closed.** Six codes, listed in §3 of the spec. A failure that maps to none of them is `unauthorized`. A client switching on the code needs the set not to grow under it in a language it does not control. +3. **The code names the stage, not the diagnosis.** Every proxy-proof outcome collapses onto `proxy_required` — this is what keeps the uniform-rejection rule of the proxy-proof spec intact while still telling an operator which layer refused. +4. **Derive the proxy note from configuration, never from the request.** Emit it on every 401 from a proxy-dependent service, identically. That is what makes it safe to show, and it is still correct in the case it exists for: a proxy that is not forwarding the header 401s *everything*. +5. **Carry declarations through composition.** If your chain / require-all helpers do not propagate an authenticator's proxy-header dependency, wrapping one silently drops the note — which is exactly the deployment where you needed it. + ## HTTP proxy proof Proxy proof is an **opt-in additive feature**: a worker can refuse any request that did not arrive through a trusted proxy, which is verified by recomputing an HMAC over a timestamp, a nonce and the worker's own identifier. The full spec lives at [`docs/proxy-proof-spec.md`](proxy-proof-spec.md). A port may: diff --git a/docs/proxy-proof-spec.md b/docs/proxy-proof-spec.md index 74c6ed6..6d18b93 100644 --- a/docs/proxy-proof-spec.md +++ b/docs/proxy-proof-spec.md @@ -170,6 +170,8 @@ Steps 1–4 involve no MAC computation and MUST be performed first. **Rejection is uniform.** The response body MUST NOT contain the verifier's message, the reason code, or any echo of `kid` — an attacker controls that field. Detail goes to logs and metrics only. In `require` mode a failure maps to **HTTP 401**, matching every existing authenticate-callback failure in the framework. +That 401 follows [`docs/unauthorized-spec.md`](unauthorized-spec.md), which is compatible with the paragraph above rather than an exception to it. Every outcome in the table above — `no_proof`, `malformed`, `unknown_kid`, `expired`, `not_yet_valid`, `bad_mac`, `replayed` — collapses onto the single reason code `proxy_required`, with an identical detail string. The code names the stage that refused the request, which the response already said by rejecting it at all; it never names which check was tripped. A `require`-mode worker also carries that spec's proxy-configuration note, again identical on every 401 and derived from configuration rather than from the request, so it cannot be used to probe anything. + ## 7. Modes | Mode | Behavior | diff --git a/docs/unauthorized-spec.md b/docs/unauthorized-spec.md new file mode 100644 index 0000000..eeeed28 --- /dev/null +++ b/docs/unauthorized-spec.md @@ -0,0 +1,137 @@ +# vgi-rpc Unauthorized Response Specification + +This document is the cross-language contract for the shape of an **HTTP 401** from a vgi-rpc service. The Python implementation in this repository is the reference; other-language implementations (Go, Rust, TypeScript, Java, …) MUST implement the contract below so the canonical `TestUnauthorized` conformance group in `vgi_rpc/conformance/_pytest_suite.py` passes against them. + +## 1. Scope and motivation + +Before this contract, a 401 was whatever the server's web framework happened to produce. Two things went wrong with that. + +**A client could not branch on it.** The only machine-readable signal was the status code, which does not distinguish "send a credential" from "your credential expired, refresh it" from "you are who you say and still may not do this". A client wanting to retry after a token refresh had to substring-match an English sentence. + +**The most common production cause was invisible.** Authentication schemes that read a header injected by a reverse proxy — mTLS forwarded as `X-SSL-Client-Cert` or `x-forwarded-client-cert`, or a proxy proof — fail identically whether the caller sent a bad certificate or the proxy was never configured to forward one. The second is far more common during a deployment, and the 401 said nothing about it. Operators rotated credentials that were never the problem. + +This spec fixes both: a coarse reason code from a closed set, and a **static** note on services whose authentication depends on a proxy. + +**HTTP-only.** Pipe / subprocess / shared-memory / unix / tcp transports do not invoke an authenticate callback at all and never produce a 401. + +## 2. What the reason code is, and is not + +The reason code names the **stage that refused the request**. It never carries a verifier's internal diagnosis. + +The distinction matters. Telling a caller "your JWT signature did not verify" versus "your JWT expired" is fine — both are facts about a token they hold. Telling them "the `kid` you supplied is not in my secret map" is not: the caller chose that `kid`, and echoing per-attempt verifier state back to whoever is probing turns a rejection into an oracle. `docs/proxy-proof-spec.md` §6 states this as a hard requirement for proxy proof, and this spec keeps it: every proxy-proof outcome — absent, malformed, unknown key, expired, bad MAC, replayed — collapses onto the single code `proxy_required`. + +Implementations MUST NOT add codes outside §3. A failure that maps onto none of them uses `unauthorized`. Keeping the set closed is what makes it usable: a client that switches on the code needs to know the set will not grow under it in a language it does not control. + +## 3. Reason codes + +| Code | Meaning | What the caller should do | +|---|---|---| +| `missing_credential` | No credential was presented at all. | Send one. | +| `invalid_credential` | A credential was presented and rejected. | Fix or re-obtain it; do not retry unchanged. | +| `expired_credential` | A well-formed credential outside its validity window. | Refresh and retry. | +| `insufficient_scope` | The caller was identified but is not permitted. | Do not retry; escalate. | +| `proxy_required` | The request did not carry evidence that it arrived through the trusted proxy. | Almost always an operator's problem, not the caller's — see §5. | +| `unauthorized` | Refused, unclassified. | Fallback. | + +`insufficient_scope` is deliberately a **401**, not a 403, because the framework's authenticate callback runs before any method is resolved: there is no route yet whose permissions could be evaluated. A service that wants a true 403 raises it from the method body. + +### 3.1 Composition + +When several alternative credentials are tried (`chain_authenticate` and its equivalents), the code reported is: + +- `missing_credential` only when **every** alternative agreed nothing was presented — the one case where "send a credential" is actionable advice. +- otherwise the first code that is not `missing_credential`. + +When a precondition gate and a credential are ANDed (`require_all`), the gate runs first, so a gate failure reports the gate's code and the credential is never consulted. + +## 4. Response + +### 4.1 Headers + +| Header | When | Value | +|---|---|---| +| `VGI-Auth-Reason` | every 401 | One code from §3. | +| `VGI-Auth-Proxy-Required` | 401s from a service whose auth depends on a proxy (§5) | `"true"`. Omitted otherwise — never `"false"`. | +| `Cache-Control` | every 401 | MUST prevent shared caching (`no-store` in the reference). A 401 is per-request and flips to 200 on the next attempt with a credential. | +| `WWW-Authenticate` | when the service declares a challenge | Unchanged from RFC 7235 / RFC 9728 behaviour. | + +Both `VGI-` headers describe a rejection, so they MUST NOT be emitted on successful responses — they are not capability advertisements. When the service uses CORS, both MUST appear in `Access-Control-Expose-Headers`, otherwise a browser client cannot read them cross-origin and is back to guessing from the body. + +### 4.2 Content negotiation + +| Request `Accept` contains `text/html` | Response | +|---|---| +| yes | The service's styled HTML 401 page. | +| no (including `*/*` and an absent header) | The JSON envelope of §4.3. | + +Substring-matching `Accept` rather than full media-type negotiation is intentional and matches the OAuth browser-redirect rule: the only clients that ask for `text/html` are browsers, and an RPC client sends `*/*`, which MUST resolve to JSON. A service MAY skip the HTML page entirely and always answer with JSON; a service MUST NOT answer a non-HTML request with HTML. + +The HTML page is presentation, not contract — nothing may be parsed out of it. When a service renders one it SHOULD show the reason code and, when applicable, the §5 note, since a human reading the page is exactly the audience for both. + +### 4.3 JSON envelope + +```json +{ + "error": "unauthorized", + "reason": "proxy_required", + "detail": "Missing x-forwarded-client-cert header", + "proxy_hint": "This service only accepts requests that arrive through its configured reverse proxy, which must set the x-forwarded-client-cert header. …" +} +``` + +| Field | Required | Notes | +|---|---|---| +| `error` | yes | Always the literal `"unauthorized"`. Marks the envelope kind so a client can tell it from a framework's default error JSON. | +| `reason` | yes | A code from §3. | +| `detail` | yes | Human-readable, may be `""`. Free text, subject to §2 — never a verifier's per-attempt state. | +| `proxy_hint` | no | Present only under §5. **Absent, not empty**, when it does not apply, so its presence alone is a usable signal. | + +`Content-Type` MUST be `application/json`. Readers MUST ignore unknown fields, and MUST treat an unrecognised `reason` as `unauthorized` — that means the server is newer, not broken. + +## 5. The proxy note + +A service whose authentication can only succeed on requests a reverse proxy has stamped MUST set `VGI-Auth-Proxy-Required: true` and include `proxy_hint` on **every** 401 it produces. + +**The note is derived from server configuration, not from what failed on this request.** A worker that requires proxy-injected evidence emits the identical note whether the proof was absent, the certificate was expired, or the bearer token behind the proxy was simply wrong. Two consequences follow, and both are the point: + +1. It discloses nothing. The note restates a static property the service already advertises through its capability headers, so it cannot be used to probe which stage rejected a given attempt. This is what lets it coexist with the uniform-rejection rule of `docs/proxy-proof-spec.md` §6. +2. It is still right in the case that matters. When a deployment's proxy is not forwarding the header, *every* request 401s, and every one of those 401s says so. + +A service MUST NOT emit the note when its authentication does not depend on a proxy — a note that appears everywhere teaches operators to ignore it. + +### 5.1 Discovering the dependency + +An implementation SHOULD discover the dependency from the authenticators it has installed rather than requiring the operator to restate it: the built-in mTLS and proxy-proof authenticators know which header they read. Composition helpers (chain, require-all) MUST carry declarations through, otherwise wrapping an authenticator silently drops the note. An implementation MUST also offer a direct way for an operator to state header names for a custom authenticator the framework cannot introspect — `make_wsgi_app(proxy_auth_headers=[...])` in the reference. + +A proxy-proof gate contributes its header only in `require` mode. In `allow` mode an absent proof never denies, so the note would misdirect. + +### 5.2 Wording + +The note text is not normative — it is prose for a human. It MUST convey: that the service is only reachable through its proxy; which header names the proxy must set; and that a rejection here is at least as likely to be a proxy misconfiguration as a bad credential. + +## 6. Client behaviour + +A client receiving a 401 MUST: + +- read `reason` from the JSON body when present, falling back to `VGI-Auth-Reason`, falling back to `unauthorized`; +- surface `proxy_hint` to whoever sees the error. The reference appends it to the exception message rather than leaving it on an attribute alone, because the place it actually gets read is a traceback in a deployment log; +- degrade without raising when the body is not the §4.3 envelope. A 401 can come from an intermediary the service never sees — a gateway, a WAF, an SSO portal — with its own idea of an error body. The reference keeps a bounded prefix of such a body, and replaces an HTML page with a one-line note rather than pasting markup into an exception message. + +A client MUST NOT attempt to parse an Arrow IPC stream from a 401 body. The rejection happens before any method is resolved, so no output schema exists. + +## 7. Conformance + +`TestUnauthorized` in `vgi_rpc/conformance/_pytest_suite.py` is the canonical suite. It is capability-gated on the server emitting `VGI-Auth-Reason`, so a port that has not adopted this contract skips rather than fails. + +| Test | Asserts | +|---|---| +| `test_reason_header_present` | Every 401 carries `VGI-Auth-Reason`. | +| `test_reason_in_closed_set` | The code is one of §3. | +| `test_json_envelope_for_machine_clients` | `Accept: */*` yields `application/json` matching §4.3, with `reason` agreeing with the header. | +| `test_html_page_for_browsers` | `Accept: text/html` yields `text/html`, and the reason header is still present. | +| `test_not_cached` | `Cache-Control` forbids shared caching. | +| `test_no_proxy_header_without_proxy_auth` | A service with no proxy dependency omits `VGI-Auth-Proxy-Required` and `proxy_hint`. | +| `test_proxy_hint_when_proxy_required` | A require-mode proxy-proof worker sets the header and includes a non-empty `proxy_hint`. Gated on the `proof_worker_factory` fixture. | +| `test_proxy_rejection_is_uniform` | Absent, malformed, and bad-MAC proofs produce the *same* reason code and the same detail — the uniform-rejection rule of `docs/proxy-proof-spec.md` §6, asserted from the outside. | + +Runners supply the existing `conformance_http_auth_port` fixture (a server whose RPC endpoints all 401) and, for the last two tests, `proof_worker_factory`. diff --git a/mkdocs.yml b/mkdocs.yml index d81c5df..bed1df1 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -136,6 +136,7 @@ nav: - Access Log Specification: access-log-spec.md - Sticky Sessions Specification: sticky-sessions-spec.md - Proxy Proof Specification: proxy-proof-spec.md + - Unauthorized Response Specification: unauthorized-spec.md - Log Shipping: log-shipping/README.md - Wire Protocol: WIRE_PROTOCOL.md - Conformance Testing: cross-language-conformance.md diff --git a/tests/test_bearer.py b/tests/test_bearer.py index f65ca31..93b0ccf 100644 --- a/tests/test_bearer.py +++ b/tests/test_bearer.py @@ -16,6 +16,8 @@ from vgi_rpc import AuthContext, CallContext, RpcServer from vgi_rpc.http import ( + AuthFailure, + AuthReason, bearer_authenticate, bearer_authenticate_static, chain_authenticate, @@ -89,18 +91,24 @@ def reject(_token: str) -> AuthContext: auth_fn(req) def test_missing_header_raises(self) -> None: - """Missing Authorization header raises ValueError.""" + """No Authorization header at all is reported as a missing credential.""" auth_fn = bearer_authenticate(validate=lambda t: _ALICE) req = _make_req() - with pytest.raises(ValueError, match="Missing"): + with pytest.raises(AuthFailure, match="Missing") as exc_info: auth_fn(req) + assert exc_info.value.reason is AuthReason.MISSING_CREDENTIAL def test_non_bearer_scheme_raises(self) -> None: - """Non-Bearer scheme raises ValueError.""" + """A non-Bearer scheme is an *invalid* credential, not a missing one. + + The caller sent something; telling them to send a credential would be + the wrong advice, and the reason code is what a client branches on. + """ auth_fn = bearer_authenticate(validate=lambda t: _ALICE) req = _make_req(authorization="Basic dXNlcjpwYXNz") - with pytest.raises(ValueError, match="Missing"): + with pytest.raises(AuthFailure, match="not a Bearer credential") as exc_info: auth_fn(req) + assert exc_info.value.reason is AuthReason.INVALID_CREDENTIAL def test_end_to_end_rpc(self) -> None: """Full round-trip: bearer auth -> RPC call -> identity returned.""" diff --git a/tests/test_unauthorized.py b/tests/test_unauthorized.py new file mode 100644 index 0000000..f77f899 --- /dev/null +++ b/tests/test_unauthorized.py @@ -0,0 +1,428 @@ +# © Copyright 2025-2026, Query.Farm LLC - https://query.farm +# SPDX-License-Identifier: Apache-2.0 + +"""Tests for the standardized 401 response. + +The cross-language part of this contract lives in ``docs/unauthorized-spec.md`` +and is asserted by the ``TestUnauthorized`` conformance group. What is here is +the Python-specific half: which reason code each built-in authenticator picks, +how declarations survive composition, and how the client parses the envelope. +""" + +from __future__ import annotations + +import json +from typing import Any + +import falcon +import falcon.testing.helpers +import pytest + +from vgi_rpc.conformance import ConformanceService, ConformanceServiceImpl +from vgi_rpc.http import ( + AuthenticationError, + AuthFailure, + AuthReason, + ProxyProofConfig, + bearer_authenticate_static, + chain_authenticate, + declare_proxy_headers, + make_sync_client, + mtls_authenticate_xfcc, + proxy_proof_gate, + require_all, +) +from vgi_rpc.http._client import _parse_unauthorized +from vgi_rpc.http._testing import _SyncTestClient, _SyncTestResponse +from vgi_rpc.http._unauthorized import classify_auth_failure, proxy_headers_of +from vgi_rpc.rpc import AuthContext, RpcServer + +_ALICE = AuthContext(domain="test", authenticated=True, principal="alice", claims={}) +_REASON_HEADER = "vgi-auth-reason" +_PROXY_HEADER = "vgi-auth-proxy-required" + + +def _client(**kwargs: Any) -> _SyncTestClient: + """Build a sync test client over the conformance service.""" + return make_sync_client(RpcServer(ConformanceService, ConformanceServiceImpl()), **kwargs) + + +def _reject(_req: falcon.Request) -> AuthContext: + raise ValueError("nope") + + +def _body(resp: _SyncTestResponse) -> dict[str, str]: + """Decode a JSON 401 envelope from the sync test client's response.""" + parsed: object = json.loads(resp.content) + assert isinstance(parsed, dict) + return parsed + + +def _text(resp: _SyncTestResponse) -> str: + """Decode an HTML 401 page from the sync test client's response.""" + return resp.content.decode() + + +# --------------------------------------------------------------------------- +# Classification +# --------------------------------------------------------------------------- + + +class TestClassification: + """Mapping an authenticate-callback exception onto a reason code.""" + + def test_auth_failure_declares_its_own(self) -> None: + """An AuthFailure's reason is used verbatim.""" + assert classify_auth_failure(AuthFailure(AuthReason.EXPIRED_CREDENTIAL)) is AuthReason.EXPIRED_CREDENTIAL + + def test_bare_value_error_is_unclassified(self) -> None: + """A custom authenticator's plain ValueError is not guessed at. + + Guessing would mean matching message text, which misclassifies the + moment someone rewords a string. + """ + assert classify_auth_failure(ValueError("Missing token")) is AuthReason.UNAUTHORIZED + + def test_permission_error_is_insufficient_scope(self) -> None: + """PermissionError means the caller got as far as being identified.""" + assert classify_auth_failure(PermissionError("no")) is AuthReason.INSUFFICIENT_SCOPE + + def test_auth_failure_default_message_is_the_code(self) -> None: + """Omitting the detail leaves the code as the message rather than blank.""" + assert str(AuthFailure(AuthReason.MISSING_CREDENTIAL)) == "missing_credential" + + +# --------------------------------------------------------------------------- +# Reason codes chosen by the built-in authenticators +# --------------------------------------------------------------------------- + + +class TestBuiltinReasons: + """Each built-in authenticator's failures land on the right code.""" + + def test_bearer_missing_header(self) -> None: + """No Authorization header at all is a missing credential.""" + client = _client(authenticate=bearer_authenticate_static(tokens={"good": _ALICE})) + resp = client.post("/echo_int", content=b"", headers={}) + assert resp.status_code == 401 + assert resp.headers[_REASON_HEADER] == "missing_credential" + + def test_bearer_wrong_token(self) -> None: + """A presented-but-unknown token is invalid, not missing.""" + client = _client(authenticate=bearer_authenticate_static(tokens={"good": _ALICE})) + resp = client.post("/echo_int", content=b"", headers={"Authorization": "Bearer bad"}) + assert resp.headers[_REASON_HEADER] == "invalid_credential" + + def test_xfcc_missing_header_is_proxy_required(self) -> None: + """An absent proxy-injected header points at the deployment, not the caller. + + Reporting this as ``missing_credential`` would send an operator + hunting for a certificate the caller may well have presented — to a + proxy that then did not forward it. + """ + client = _client(authenticate=mtls_authenticate_xfcc()) + resp = client.post("/echo_int", content=b"", headers={}) + assert resp.headers[_REASON_HEADER] == "proxy_required" + + def test_xfcc_empty_header_is_invalid(self) -> None: + """A header the proxy *did* set, but empty, is a bad credential.""" + client = _client(authenticate=mtls_authenticate_xfcc()) + resp = client.post("/echo_int", content=b"", headers={"x-forwarded-client-cert": ","}) + assert resp.headers[_REASON_HEADER] == "invalid_credential" + + def test_unclassified_callback(self) -> None: + """A custom callback raising a bare ValueError falls back cleanly.""" + client = _client(authenticate=_reject) + resp = client.post("/echo_int", content=b"", headers={}) + assert resp.headers[_REASON_HEADER] == "unauthorized" + + +class TestChainComposition: + """Reason codes across alternative credentials.""" + + def test_all_missing_reports_missing(self) -> None: + """When nothing was presented anywhere, telling the caller to send something is right.""" + chained = chain_authenticate( + bearer_authenticate_static(tokens={"good": _ALICE}), + bearer_authenticate_static(tokens={"other": _ALICE}), + ) + client = _client(authenticate=chained) + resp = client.post("/echo_int", content=b"", headers={}) + assert resp.headers[_REASON_HEADER] == "missing_credential" + + def test_one_substantive_failure_wins(self) -> None: + """A credential that was seen and rejected outranks 'you sent nothing'. + + Both alternatives read the same header here, so a bad token makes both + fail — but reporting ``missing_credential`` would be advice the caller + has already followed. + """ + + def missing(_req: falcon.Request) -> AuthContext: + raise AuthFailure(AuthReason.MISSING_CREDENTIAL, "no cookie") + + chained = chain_authenticate(missing, bearer_authenticate_static(tokens={"good": _ALICE})) + client = _client(authenticate=chained) + resp = client.post("/echo_int", content=b"", headers={"Authorization": "Bearer bad"}) + assert resp.headers[_REASON_HEADER] == "invalid_credential" + + def test_chain_still_reports_every_detail(self) -> None: + """Aggregating the codes must not cost the per-authenticator diagnostics.""" + chained = chain_authenticate(_reject, bearer_authenticate_static(tokens={"good": _ALICE})) + client = _client(authenticate=chained) + resp = client.post("/echo_int", content=b"", headers={}) + detail = _body(resp)["detail"] + assert "nope" in detail and "Missing Authorization header" in detail + + +# --------------------------------------------------------------------------- +# The proxy note +# --------------------------------------------------------------------------- + + +class TestProxyNote: + """Which services carry the proxy-configuration note, and how it is discovered.""" + + def test_absent_by_default(self) -> None: + """A service with no proxy dependency stays quiet about proxies.""" + client = _client(authenticate=bearer_authenticate_static(tokens={"good": _ALICE})) + resp = client.post("/echo_int", content=b"", headers={}) + assert _PROXY_HEADER not in resp.headers + assert "proxy_hint" not in _body(resp) + + def test_discovered_from_mtls_authenticator(self) -> None: + """The operator does not have to restate what the authenticator already knows.""" + client = _client(authenticate=mtls_authenticate_xfcc()) + resp = client.post("/echo_int", content=b"", headers={}) + assert resp.headers[_PROXY_HEADER] == "true" + assert "x-forwarded-client-cert" in _body(resp)["proxy_hint"] + + def test_declared_by_a_custom_authenticator(self) -> None: + """A third-party authenticator can opt in without a framework change. + + Declared on a local closure, not the shared ``_reject``: the helper + annotates the callable in place, so marking a function every other + test also uses would leak the note into their assertions. + """ + + def gateway(_req: falcon.Request) -> AuthContext: + raise ValueError("nope") + + client = _client(authenticate=declare_proxy_headers(gateway, "X-Gateway-Assertion")) + resp = client.post("/echo_int", content=b"", headers={}) + assert "X-Gateway-Assertion" in _body(resp)["proxy_hint"] + + def test_stated_directly_by_the_operator(self) -> None: + """An authenticator the framework cannot introspect is still coverable.""" + client = _client(authenticate=_reject, proxy_auth_headers=["X-Edge-Verified"]) + resp = client.post("/echo_int", content=b"", headers={}) + assert "X-Edge-Verified" in _body(resp)["proxy_hint"] + + def test_survives_chain_composition(self) -> None: + """Wrapping an authenticator must not silently drop the note.""" + chained = chain_authenticate(bearer_authenticate_static(tokens={"good": _ALICE}), mtls_authenticate_xfcc()) + assert proxy_headers_of(chained) == ("x-forwarded-client-cert",) + + def test_survives_require_all(self) -> None: + """A gate's header dependency reaches the app through require_all.""" + config = ProxyProofConfig(mode="require", origin_id="w1", secrets={"k1": (b"\x01" * 32, "k1")}) + assert proxy_headers_of(require_all(proxy_proof_gate(config))) == ("VGI-Proxy-Proof",) + + def test_allow_mode_declares_nothing(self) -> None: + """In allow mode an absent proof never denies, so the note would misdirect.""" + config = ProxyProofConfig(mode="allow", origin_id="w1", secrets={"k1": (b"\x01" * 32, "k1")}) + assert proxy_headers_of(require_all(proxy_proof_gate(config))) == () + + def test_proof_required_flag_alone_is_enough(self) -> None: + """An operator who only sets the advertisement flag still gets the note.""" + client = _client(authenticate=_reject, proxy_proof_required=True) + resp = client.post("/echo_int", content=b"", headers={}) + assert resp.headers[_PROXY_HEADER] == "true" + assert "VGI-Proxy-Proof" in _body(resp)["proxy_hint"] + + +# --------------------------------------------------------------------------- +# Response shape +# --------------------------------------------------------------------------- + + +class TestResponseShape: + """Negotiation, headers, and the envelope itself.""" + + def test_json_for_machine_clients(self) -> None: + """No Accept header means a machine client, which gets JSON.""" + client = _client(authenticate=_reject) + resp = client.post("/echo_int", content=b"", headers={}) + assert resp.headers["content-type"].startswith("application/json") + assert _body(resp) == {"error": "unauthorized", "reason": "unauthorized", "detail": "nope"} + + def test_wildcard_accept_is_not_html(self) -> None: + """``*/*`` is what httpx sends by default and must not select the page.""" + client = _client(authenticate=_reject) + resp = client.post("/echo_int", content=b"", headers={"Accept": "*/*"}) + assert resp.headers["content-type"].startswith("application/json") + + def test_html_for_browsers(self) -> None: + """A browser gets the styled page, with the code shown on it.""" + client = _client(authenticate=_reject) + resp = client.post("/echo_int", content=b"", headers={"Accept": "text/html,*/*;q=0.8"}) + assert resp.headers["content-type"].startswith("text/html") + assert "401" in _text(resp) and 'class="reason">unauthorized<' in _text(resp) + + def test_html_page_shows_the_proxy_note(self) -> None: + """The page is where a human reads the note, so it must be on the page.""" + client = _client(authenticate=mtls_authenticate_xfcc()) + resp = client.post("/echo_int", content=b"", headers={"Accept": "text/html"}) + assert "Is the reverse proxy configured?" in _text(resp) + assert "x-forwarded-client-cert" in _text(resp) + + def test_detail_is_escaped_on_the_page(self) -> None: + """The detail comes from a callback and reaches a browser — escape it.""" + + def reject(_req: falcon.Request) -> AuthContext: + raise ValueError("") + + client = _client(authenticate=reject) + resp = client.post("/echo_int", content=b"", headers={"Accept": "text/html"}) + assert "" not in _text(resp) + assert "<script>" in _text(resp) + + def test_not_cached(self) -> None: + """The next attempt with a credential is a 200 — no shared cache may hold this.""" + client = _client(authenticate=_reject) + resp = client.post("/echo_int", content=b"", headers={}) + assert resp.headers["cache-control"] == "no-store" + + def test_www_authenticate_survives(self) -> None: + """The serializer must not clobber the challenge Falcon already set.""" + from vgi_rpc.http import OAuthResourceMetadata + + client = _client( + authenticate=_reject, + oauth_resource_metadata=OAuthResourceMetadata( + resource="https://x.test", authorization_servers=("https://a.test",) + ), + ) + resp = client.post("/echo_int", content=b"", headers={}) + assert "Bearer" in resp.headers["www-authenticate"] + + def test_non_401_errors_are_untouched(self) -> None: + """Only 401 gets the standardized treatment; the rest keep Falcon's JSON.""" + client = _client() + resp = client.get("/vgi/nope", headers={}) + assert resp.status_code == 404 + assert _REASON_HEADER not in resp.headers + + def test_reason_headers_absent_on_success(self) -> None: + """These explain a rejection; on a 200 they would be noise.""" + client = _client(authenticate=lambda _req: _ALICE) + resp = client.get("/health", headers={}) + assert resp.status_code == 200 + assert _REASON_HEADER not in resp.headers + assert _PROXY_HEADER not in resp.headers + + def test_body_cache_is_bounded(self) -> None: + """A callback embedding request data in the detail must not grow the cache forever.""" + counter = {"n": 0} + + def reject(_req: falcon.Request) -> AuthContext: + counter["n"] += 1 + raise ValueError(f"attempt {counter['n']}") + + client = _client(authenticate=reject) + for _ in range(200): + client.post("/echo_int", content=b"", headers={}) + # Distinct details keep rendering correctly even past the cache bound. + resp = client.post("/echo_int", content=b"", headers={}) + assert _body(resp)["detail"] == f"attempt {counter['n']}" + + +# --------------------------------------------------------------------------- +# Client-side parsing +# --------------------------------------------------------------------------- + + +class TestClientParsing: + """Turning a 401 body back into a typed error.""" + + def test_full_envelope(self) -> None: + """Every field is unpacked onto the error.""" + body = json.dumps( + { + "error": "unauthorized", + "reason": "expired_credential", + "detail": "token expired", + "proxy_hint": "check the proxy", + } + ).encode() + err = _parse_unauthorized(body) + assert err.reason is AuthReason.EXPIRED_CREDENTIAL + assert err.detail == "token expired" + assert err.proxy_hint == "check the proxy" + assert err.error_type == "AuthenticationError" + + def test_proxy_hint_reaches_the_message(self) -> None: + """The place this gets read is a traceback, not an attribute inspector.""" + body = json.dumps({"reason": "proxy_required", "detail": "d", "proxy_hint": "the note"}).encode() + assert "the note" in str(_parse_unauthorized(body)) + + def test_unknown_reason_degrades(self) -> None: + """A code this client has never heard of means a newer server, not a broken one.""" + err = _parse_unauthorized(json.dumps({"reason": "from_the_future", "detail": "d"}).encode()) + assert err.reason is AuthReason.UNAUTHORIZED + assert err.detail == "d" + + def test_html_body_is_summarized(self) -> None: + """A page of markup in an exception message buries the rest of the traceback.""" + err = _parse_unauthorized(b"\n" + b"x" * 5000 + b"") + assert "HTML 401 page" in err.detail + assert "" not in err.detail + + def test_foreign_body_is_truncated(self) -> None: + """A gateway or WAF has its own idea of an error body; keep a usable prefix.""" + err = _parse_unauthorized(b"nginx says no. " + b"y" * 5000) + assert err.detail.startswith("nginx says no.") + assert len(err.detail) <= 500 + + def test_empty_body(self) -> None: + """An empty 401 still produces a message worth reading.""" + assert _parse_unauthorized(b"").detail == "unauthorized" + + def test_json_that_is_not_an_object(self) -> None: + """Valid JSON of the wrong shape falls through to the text path.""" + assert _parse_unauthorized(b'["nope"]').reason is AuthReason.UNAUTHORIZED + + def test_is_an_rpc_error(self) -> None: + """Existing ``except RpcError`` call sites keep working.""" + from vgi_rpc.rpc import RpcError + + assert issubclass(AuthenticationError, RpcError) + + +class TestClientEndToEnd: + """The proxy note survives the whole round trip into a raised exception.""" + + def test_proxy_hint_reaches_the_caller(self) -> None: + """An operator staring at a traceback is the audience for the note.""" + from vgi_rpc.http import http_connect + + client = _client(authenticate=mtls_authenticate_xfcc()) + with ( + http_connect(ConformanceService, "http://testserver", client=client) as proxy, + pytest.raises(AuthenticationError) as exc_info, + ): + proxy.echo_int(value=1) + assert exc_info.value.reason is AuthReason.PROXY_REQUIRED + assert "reverse proxy" in exc_info.value.proxy_hint + + def test_plain_401_is_still_an_rpc_error(self) -> None: + """Callers catching RpcError see no behaviour change.""" + from vgi_rpc.http import http_connect + from vgi_rpc.rpc import RpcError + + client = _client(authenticate=_reject) + with ( + http_connect(ConformanceService, "http://testserver", client=client) as proxy, + pytest.raises(RpcError, match="AuthenticationError"), + ): + proxy.echo_int(value=1) diff --git a/vgi_rpc/cli.py b/vgi_rpc/cli.py index d7d2240..598f52d 100644 --- a/vgi_rpc/cli.py +++ b/vgi_rpc/cli.py @@ -1087,7 +1087,7 @@ def _call_stream_http( header_batch: pa.RecordBatch | None = None resp_stream = BytesIO(resp.content) if method.has_header: - # Check for auth errors first (plain text, not Arrow IPC) + # Check for auth errors first (JSON envelope, not Arrow IPC) if resp.status_code == 401: _open_response_stream(resp.content, resp.status_code, IpcValidation.FULL) header_batch = _read_raw_stream_header(resp_stream, IpcValidation.FULL, on_log) diff --git a/vgi_rpc/conformance/_pytest_suite.py b/vgi_rpc/conformance/_pytest_suite.py index 21910ae..25fffe1 100644 --- a/vgi_rpc/conformance/_pytest_suite.py +++ b/vgi_rpc/conformance/_pytest_suite.py @@ -53,6 +53,21 @@ #: importable for runners that install vgi-rpc without the ``http`` extra. _PROOF_HEADER = "VGI-Proxy-Proof" _PROOF_REQUIRED_HEADER = "VGI-Proxy-Proof-Required" +_AUTH_REASON_HEADER = "VGI-Auth-Reason" +_AUTH_PROXY_REQUIRED_HEADER = "VGI-Auth-Proxy-Required" +#: The closed set of docs/unauthorized-spec.md §3. Spelled out rather than +#: derived from the enum so that a port silently widening the set in Python +#: would still fail here. +_AUTH_REASONS = frozenset( + { + "missing_credential", + "invalid_credential", + "expired_credential", + "insufficient_scope", + "proxy_required", + "unauthorized", + } +) pytestmark = pytest.mark.timeout(5) @@ -1626,6 +1641,167 @@ def test_health_does_not_require_auth(self, conformance_http_auth_port: int) -> assert 401 in candidates.values(), f"expected an RPC endpoint to require auth, got {candidates}" +class TestUnauthorized: + """Standardized 401 contract — see ``docs/unauthorized-spec.md``. + + Runs against ``conformance_http_auth_port``, the same reject-all worker + ``TestHealth`` uses. The two proxy-note tests additionally need a worker + whose auth depends on a proxy, and borrow ``proof_worker_factory``. + + The whole group is capability-gated on the server emitting + ``VGI-Auth-Reason``, so a port that has not adopted the contract skips + rather than fails. Every test that asserts on a rejection first confirms + the endpoint really is gated — a fixture pointed at a path that does not + exist would otherwise satisfy these assertions for the wrong reason. + """ + + @staticmethod + def _post(port: int, accept: str | None = None) -> Any: + """POST a well-formed unary body to whichever RPC path the runner serves. + + Runners mount RPC at different prefixes and differ in whether auth + runs before or after routing, so both candidate layouts are probed and + the one that answers 401 is used. + + Args: + port: Port of the auth-enforcing conformance worker. + accept: Value for the ``Accept`` header, or ``None`` to send none. + + Returns: + The ``httpx.Response`` for the gated endpoint. + + """ + import httpx + + headers = {"content-type": "application/vnd.apache.arrow.stream"} + if accept is not None: + headers["accept"] = accept + body = _unary_request_body("echo_int", value=1) + last: Any = None + for path in ("/echo_int", "/vgi/echo_int"): + last = httpx.post(f"http://127.0.0.1:{port}{path}", content=body, headers=headers, timeout=5.0) + if last.status_code == 401: + return last + pytest.fail(f"no RPC endpoint returned 401 on port {port}; last status {last.status_code}") + + def _gated(self, port: int, accept: str | None = None) -> Any: + """Return a 401 response, skipping the test if the port predates this contract.""" + resp = self._post(port, accept) + if _AUTH_REASON_HEADER.lower() not in resp.headers: + pytest.skip(f"server does not emit {_AUTH_REASON_HEADER}") + return resp + + def test_reason_header_present(self, conformance_http_auth_port: int) -> None: + """Every 401 carries a machine-readable reason code.""" + resp = self._gated(conformance_http_auth_port) + assert resp.headers[_AUTH_REASON_HEADER.lower()] + + def test_reason_in_closed_set(self, conformance_http_auth_port: int) -> None: + """The code comes from the closed set, so a client can switch on it.""" + resp = self._gated(conformance_http_auth_port) + reason = resp.headers[_AUTH_REASON_HEADER.lower()] + assert reason in _AUTH_REASONS, f"{reason!r} is not one of {sorted(_AUTH_REASONS)}" + + def test_json_envelope_for_machine_clients(self, conformance_http_auth_port: int) -> None: + """A client that does not ask for HTML gets the JSON envelope.""" + resp = self._gated(conformance_http_auth_port, accept="*/*") + assert resp.headers["content-type"].startswith("application/json"), resp.headers["content-type"] + body = resp.json() + assert body["error"] == "unauthorized" + assert body["reason"] in _AUTH_REASONS + assert isinstance(body["detail"], str) + # Header and body must agree, or a client reading one and logging the + # other reports two different stories about the same rejection. + assert body["reason"] == resp.headers[_AUTH_REASON_HEADER.lower()] + + def test_html_page_for_browsers(self, conformance_http_auth_port: int) -> None: + """A browser gets a page, and the reason header survives negotiation. + + Rendering a page is optional; answering an HTML request with JSON is + allowed, answering a JSON request with HTML is not. So this asserts + the content type is one of the two, and that the header — the part a + client actually parses — is there either way. + """ + resp = self._gated(conformance_http_auth_port, accept="text/html,application/xhtml+xml") + content_type = resp.headers["content-type"] + assert content_type.startswith(("text/html", "application/json")), content_type + assert resp.headers[_AUTH_REASON_HEADER.lower()] in _AUTH_REASONS + + def test_not_cached(self, conformance_http_auth_port: int) -> None: + """A 401 must not be held by a shared cache — the next attempt may be a 200.""" + resp = self._gated(conformance_http_auth_port) + cache_control = resp.headers.get("cache-control", "") + assert "no-store" in cache_control or "no-cache" in cache_control, f"got {cache_control!r}" + + def test_no_proxy_note_without_proxy_auth(self, conformance_http_auth_port: int) -> None: + """A service with no proxy dependency must stay quiet about proxies. + + A note that shows up on every service is a note operators learn to + skip, which costs exactly the deployments it exists to help. + """ + resp = self._gated(conformance_http_auth_port, accept="*/*") + assert _AUTH_PROXY_REQUIRED_HEADER.lower() not in resp.headers + assert "proxy_hint" not in resp.json() + + def test_proxy_note_when_proxy_required(self, request: pytest.FixtureRequest) -> None: + """A require-mode proxy-proof worker explains that a proxy is involved.""" + from vgi_rpc.conformance.proof_harness import ProofWorkerConfig + + with TestProxyProof._factory(request)(ProofWorkerConfig()) as worker: + resp = self._proof_post(worker, token=None) + if _AUTH_REASON_HEADER.lower() not in resp.headers: + pytest.skip(f"server does not emit {_AUTH_REASON_HEADER}") + assert resp.status_code == 401 + assert resp.headers[_AUTH_PROXY_REQUIRED_HEADER.lower()] == "true" + assert resp.headers[_AUTH_REASON_HEADER.lower()] == "proxy_required" + assert resp.json()["proxy_hint"], "proxy_hint must be present and non-empty" + + def test_proxy_rejection_is_uniform(self, request: pytest.FixtureRequest) -> None: + """Absent, malformed, and bad-MAC proofs are indistinguishable to the caller. + + This is ``docs/proxy-proof-spec.md`` §6 asserted from the outside: the + reason code names the stage, never the verifier's diagnosis, so an + attacker cannot use the response to tell which check they tripped. + """ + from vgi_rpc.conformance.proof_harness import CONFORMANCE_KID, ProofWorkerConfig, secret_bytes + from vgi_rpc.http import mint_proof + + with TestProxyProof._factory(request)(ProofWorkerConfig()) as worker: + good = mint_proof(secret_bytes(), CONFORMANCE_KID, worker.config.origin_id) + # Positive control: without it, a worker that refuses everything + # would pass this test by refusing everything identically. + assert self._proof_post(worker, token=good).status_code == 200 + + tampered = good[:-1] + ("A" if good[-1] != "A" else "B") + responses = [ + self._proof_post(worker, token=None), + self._proof_post(worker, token="not-a-proof"), + self._proof_post(worker, token=tampered), + ] + if _AUTH_REASON_HEADER.lower() not in responses[0].headers: + pytest.skip(f"server does not emit {_AUTH_REASON_HEADER}") + + reasons = {r.headers[_AUTH_REASON_HEADER.lower()] for r in responses} + assert reasons == {"proxy_required"}, f"reason leaked the verifier's diagnosis: {reasons}" + details = {r.json()["detail"] for r in responses} + assert len(details) == 1, f"detail distinguishes proof failures: {details}" + + @staticmethod + def _proof_post(worker: ProofWorker, *, token: str | None) -> Any: + """POST a well-formed unary body to a proof worker, optionally with a proof.""" + import httpx + + headers = {"content-type": "application/vnd.apache.arrow.stream", "accept": "*/*"} + if token is not None: + headers[_PROOF_HEADER] = token + return httpx.post( + worker.rpc_url("echo_int"), + content=_unary_request_body("echo_int", value=1), + headers=headers, + timeout=5.0, + ) + + class TestProxyProof: """Proxy-proof contract — see ``docs/proxy-proof-spec.md``. diff --git a/vgi_rpc/http/__init__.py b/vgi_rpc/http/__init__.py index d587208..ec9d8cc 100644 --- a/vgi_rpc/http/__init__.py +++ b/vgi_rpc/http/__init__.py @@ -52,6 +52,8 @@ ) from vgi_rpc.http._common import ( _ARROW_CONTENT_TYPE, + AUTH_PROXY_REQUIRED_HEADER, + AUTH_REASON_HEADER, MAX_REQUEST_BYTES_HEADER, MAX_UPLOAD_BYTES_HEADER, RPC_ERROR_HEADER, @@ -90,6 +92,14 @@ _SyncTestResponse, make_sync_client, ) +from vgi_rpc.http._unauthorized import ( + AuthenticationError, + AuthFailure, + AuthReason, + build_proxy_hint, + declare_proxy_headers, + proxy_headers_of, +) from vgi_rpc.http.server import make_wsgi_app, serve_http from vgi_rpc.http.server._sticky import DrainHandle, drain_handle @@ -105,6 +115,14 @@ ) __all__ = [ + "AUTH_PROXY_REQUIRED_HEADER", + "AUTH_REASON_HEADER", + "AuthFailure", + "AuthReason", + "AuthenticationError", + "build_proxy_hint", + "declare_proxy_headers", + "proxy_headers_of", "bearer_authenticate", "bearer_authenticate_static", "chain_authenticate", diff --git a/vgi_rpc/http/_bearer.py b/vgi_rpc/http/_bearer.py index b8307a4..cb6cc2c 100644 --- a/vgi_rpc/http/_bearer.py +++ b/vgi_rpc/http/_bearer.py @@ -15,10 +15,11 @@ import dataclasses import hmac -from collections.abc import Callable, Mapping +from collections.abc import Callable, Mapping, Sequence import falcon +from vgi_rpc.http._unauthorized import AuthFailure, AuthReason, declare_proxy_headers, merge_proxy_headers from vgi_rpc.rpc import AuthContext @@ -35,7 +36,9 @@ def bearer_authenticate( Args: validate: Callable that receives the raw bearer token string and returns an ``AuthContext`` on success. Must raise ``ValueError`` - when the token is invalid or unknown. + when the token is invalid or unknown — raise + :class:`vgi_rpc.http.AuthFailure` to also choose the reason code + the 401 carries. Returns: A callback ``(falcon.Request) -> AuthContext`` suitable for @@ -45,8 +48,10 @@ def bearer_authenticate( def authenticate(req: falcon.Request) -> AuthContext: auth_header = req.get_header("Authorization") or "" + if not auth_header: + raise AuthFailure(AuthReason.MISSING_CREDENTIAL, "Missing Authorization header") if not auth_header.startswith("Bearer "): - raise ValueError("Missing or invalid Authorization header") + raise AuthFailure(AuthReason.INVALID_CREDENTIAL, "Authorization header is not a Bearer credential") token = auth_header[7:] return validate(token) @@ -90,7 +95,7 @@ def validate(token: str) -> AuthContext: if hmac.compare_digest(token_b, known) and match is None: match = ctx if match is None: - raise ValueError("Unknown bearer token") + raise AuthFailure(AuthReason.INVALID_CREDENTIAL, "Unknown bearer token") return match return bearer_authenticate(validate=validate) @@ -133,21 +138,51 @@ def chain_authenticate( def authenticate(req: falcon.Request) -> AuthContext: last_error: ValueError | None = None reasons: list[str] = [] + codes: list[AuthReason] = [] for auth_fn in authenticators: try: return auth_fn(req) except ValueError as exc: last_error = exc reasons.append(str(exc) or type(exc).__name__) + codes.append(exc.reason if isinstance(exc, AuthFailure) else AuthReason.UNAUTHORIZED) # Surface every authenticator's reason. The middleware logs and returns # ``str(exc)``, so collapsing to a bare "nothing accepted it" would throw # away the only diagnostic an operator gets for a 401. detail = "; ".join(reasons) - raise ValueError(f"No authenticator accepted the request ({detail})") from last_error + raise AuthFailure( + _combine_reasons(codes), + f"No authenticator accepted the request ({detail})", + ) from last_error + declare_proxy_headers(authenticate, *merge_proxy_headers(*authenticators)) return authenticate +def _combine_reasons(codes: Sequence[AuthReason]) -> AuthReason: + """Reduce a chain's per-authenticator reason codes to the one the 401 carries. + + Args: + codes: One code per authenticator, in the order they were tried. + + Returns: + :attr:`AuthReason.MISSING_CREDENTIAL` only when *every* alternative + agreed the caller presented nothing — that is the case where telling + them to send a credential is actionable. As soon as one alternative + saw something and rejected it, that first substantive code wins, since + "you sent nothing" would be actively misleading advice. + + """ + if not codes: + return AuthReason.UNAUTHORIZED + if all(code is AuthReason.MISSING_CREDENTIAL for code in codes): + return AuthReason.MISSING_CREDENTIAL + for code in codes: + if code is not AuthReason.MISSING_CREDENTIAL: + return code + return AuthReason.UNAUTHORIZED + + class PreconditionGate: """An authenticate callback that is a *requirement*, not an alternative. @@ -165,7 +200,10 @@ class PreconditionGate: is where :func:`require_all` merges the gate's claims. """ - __slots__ = ("_fn", "claims_key", "name") + # ``vgi_proxy_headers`` is the attribute _unauthorized.proxy_headers_of + # reads. It is a declared slot rather than something declare_proxy_headers + # sets, because setattr on a slotted instance would raise. + __slots__ = ("_fn", "claims_key", "name", "vgi_proxy_headers") def __init__( self, @@ -173,6 +211,7 @@ def __init__( *, name: str, claims_key: str, + proxy_headers: Sequence[str] = (), ) -> None: """Wrap a gate callable. @@ -182,11 +221,15 @@ def __init__( failure — ``ValueError`` is swallowed by chain composition. name: Short identifier for error messages. claims_key: Key under which the gate's claims are merged. + proxy_headers: Names of headers a trusted reverse proxy must + inject for this gate to pass. Surfaced on the service's 401 + responses as the proxy-configuration note. """ self._fn = fn self.name = name self.claims_key = claims_key + self.vgi_proxy_headers = tuple(proxy_headers) def __call__(self, req: falcon.Request) -> Mapping[str, str]: """Verify the precondition, returning claims to merge. @@ -250,4 +293,8 @@ def authenticate(req: falcon.Request) -> AuthContext: merged[gate.claims_key] = claims return dataclasses.replace(ctx, claims=merged) + # The gate is the usual source of a proxy-header dependency, but inner may + # have one too (mTLS behind a proof-carrying proxy), so both are carried + # forward — otherwise wrapping would silently drop the 401's proxy note. + declare_proxy_headers(authenticate, *merge_proxy_headers(gate, inner)) return authenticate diff --git a/vgi_rpc/http/_client.py b/vgi_rpc/http/_client.py index 2c6c057..1c30756 100644 --- a/vgi_rpc/http/_client.py +++ b/vgi_rpc/http/_client.py @@ -84,6 +84,15 @@ compress as _compress_with_encoding, ) from ._retry import HttpRetryConfig, _options_with_retry, _post_with_retry +from ._unauthorized import AuthenticationError, AuthReason + +# An unrecognised reason code means the server is newer than this client, not +# that it is broken; fall back rather than raising out of the error path. +_AUTH_REASONS = frozenset(reason.value for reason in AuthReason) + +# A non-JSON 401 body is someone else's error page. Keep enough to identify +# the intermediary, not so much that it drowns the traceback. +_MAX_UNAUTHORIZED_DETAIL = 500 if TYPE_CHECKING: from vgi_rpc.introspect import ServiceDescription @@ -207,6 +216,44 @@ def _build_pointer_request_body(original_body: bytes, location_url: str) -> byte return buf.getvalue() +def _parse_unauthorized(content: bytes) -> AuthenticationError: + """Turn a 401 body into a typed error. + + Accepts the standardized JSON envelope and degrades gracefully for + anything else, because a 401 can also come from an intermediary the + service never sees — a gateway, a WAF, an SSO portal — that has its own + idea of what an error body looks like. + + Args: + content: The raw 401 response body. + + Returns: + The error to raise. + + """ + payload: object = None + with contextlib.suppress(ValueError): + payload = json.loads(content) + if isinstance(payload, dict): + raw_reason = str(payload.get("reason", "")) + reason = AuthReason(raw_reason) if raw_reason in _AUTH_REASONS else AuthReason.UNAUTHORIZED + return AuthenticationError( + reason, + str(payload.get("detail", "")), + str(payload.get("proxy_hint", "")), + ) + + text = content.decode(errors="replace").strip() + if text[:9].lower() == " HttpStreamSession: header = None resp_stream = BytesIO(resp.content) if info.header_type is not None: - # Check for auth errors first (plain text, not Arrow IPC) + # Check for auth errors first (JSON envelope, not Arrow IPC) if resp.status_code == 401: _open_response_stream(resp.content, resp.status_code, ipc_validation) header = _read_stream_header(resp_stream, info.header_type, ipc_validation, on_log, ext_cfg) diff --git a/vgi_rpc/http/_common.py b/vgi_rpc/http/_common.py index d10ac08..44f7040 100644 --- a/vgi_rpc/http/_common.py +++ b/vgi_rpc/http/_common.py @@ -39,6 +39,15 @@ MAX_UPLOAD_BYTES_HEADER = "VGI-Max-Upload-Bytes" SUPPORTED_ENCODINGS_HEADER = "VGI-Supported-Encodings" +# Standardized 401 responses (see docs/unauthorized-spec.md). Both appear on +# 401 responses only — they explain a rejection rather than advertise a +# capability, so putting them on every response would be noise. Both are +# CORS-exposed so a browser client can read them cross-origin. +AUTH_REASON_HEADER = "VGI-Auth-Reason" +"""Machine-readable reason code from the closed :class:`AuthReason` set.""" +AUTH_PROXY_REQUIRED_HEADER = "VGI-Auth-Proxy-Required" +"""``true`` when this service's auth depends on headers a reverse proxy must inject.""" + # Sticky session header conventions (HTTP-only). The cookie equivalent # (Set-Cookie / vgi-session) is intentionally out of scope for v1 — headers # multiplex cleanly across concurrent sessions, cookies do not. @@ -151,6 +160,14 @@ def decode_content_encoding( p {{ line-height: 1.7; color: #6b6b5a; }} .detail {{ margin-top: 12px; padding: 12px 16px; background: #f0ece0; border-radius: 6px; font-size: 0.9em; color: #6b6b5a; }} + .reason {{ display: inline-block; margin-bottom: 16px; padding: 3px 10px; + border-radius: 999px; background: #f0ece0; color: #6b6b5a; + font-family: 'JetBrains Mono', monospace; font-size: 0.8em; }} + .note {{ margin-top: 20px; padding: 14px 18px; background: #fdf6e3; + border: 1px solid #e0d7b8; border-left: 4px solid #b8860b; + border-radius: 6px; font-size: 0.9em; color: #6b6b5a; + text-align: left; line-height: 1.6; }} + .note strong {{ display: block; margin-bottom: 6px; color: #2c2c1e; }} footer {{ margin-top: 48px; padding: 20px 0; border-top: 1px solid #f0ece0; color: #6b6b5a; font-size: 0.85em; line-height: 1.8; }} footer a {{ color: #2d5016; font-weight: 600; }} diff --git a/vgi_rpc/http/_mtls.py b/vgi_rpc/http/_mtls.py index e5f1d98..3731936 100644 --- a/vgi_rpc/http/_mtls.py +++ b/vgi_rpc/http/_mtls.py @@ -35,8 +35,11 @@ import falcon +from vgi_rpc.http._unauthorized import AuthFailure, AuthReason, declare_proxy_headers from vgi_rpc.rpc import AuthContext +_XFCC_HEADER = "x-forwarded-client-cert" + # --------------------------------------------------------------------------- # XFCC types and parser (no cryptography needed) # --------------------------------------------------------------------------- @@ -191,12 +194,15 @@ def mtls_authenticate_xfcc( """ def authenticate(req: falcon.Request) -> AuthContext: - header_value = req.get_header("x-forwarded-client-cert") + header_value = req.get_header(_XFCC_HEADER) if not header_value: - raise ValueError("Missing x-forwarded-client-cert header") + # The client cannot fix this: the header is the proxy's to set. + # Reporting it as a missing *credential* would send an operator + # hunting for a certificate the caller already presented. + raise AuthFailure(AuthReason.PROXY_REQUIRED, f"Missing {_XFCC_HEADER} header") elements = _parse_xfcc(header_value) if not elements: - raise ValueError("Empty x-forwarded-client-cert header") + raise AuthFailure(AuthReason.INVALID_CREDENTIAL, f"Empty {_XFCC_HEADER} header") element = elements[0] if select_element == "first" else elements[-1] if validate is not None: return validate(element) @@ -220,7 +226,7 @@ def authenticate(req: falcon.Request) -> AuthContext: claims=claims, ) - return authenticate + return declare_proxy_headers(authenticate, _XFCC_HEADER) def _extract_cn(subject: str) -> str: @@ -258,19 +264,23 @@ def _parse_cert_from_header(req: falcon.Request, header: str) -> x509.Certificat Parsed X.509 certificate. Raises: - ValueError: If the header is missing, not valid PEM, or unparseable. + AuthFailure: If the header is missing, not valid PEM, or + unparseable. An absent header is reported as + ``proxy_required`` rather than a missing credential — the + header is the proxy's to inject, so its absence points at the + deployment, not at the caller. """ raw = req.get_header(header) if not raw: - raise ValueError(f"Missing {header} header") + raise AuthFailure(AuthReason.PROXY_REQUIRED, f"Missing {header} header") pem_str = unquote(raw) if not pem_str.startswith("-----BEGIN CERTIFICATE-----"): - raise ValueError("Header value is not a PEM certificate") + raise AuthFailure(AuthReason.INVALID_CREDENTIAL, "Header value is not a PEM certificate") try: return x509.load_pem_x509_certificate(pem_str.encode()) except Exception as exc: - raise ValueError(f"Failed to parse PEM certificate: {exc}") from exc + raise AuthFailure(AuthReason.INVALID_CREDENTIAL, f"Failed to parse PEM certificate: {exc}") from exc def _check_cert_expiry(cert: x509.Certificate) -> None: """Validate certificate is within its validity period. @@ -279,16 +289,16 @@ def _check_cert_expiry(cert: x509.Certificate) -> None: cert: Certificate to check. Raises: - ValueError: If the certificate is expired or not yet valid. + AuthFailure: If the certificate is expired or not yet valid. """ import datetime now = datetime.datetime.now(datetime.UTC) if now < cert.not_valid_before_utc: - raise ValueError("Certificate is not yet valid") + raise AuthFailure(AuthReason.EXPIRED_CREDENTIAL, "Certificate is not yet valid") if now > cert.not_valid_after_utc: - raise ValueError("Certificate has expired") + raise AuthFailure(AuthReason.EXPIRED_CREDENTIAL, "Certificate has expired") def mtls_authenticate( *, @@ -333,7 +343,7 @@ def authenticate(req: falcon.Request) -> AuthContext: _check_cert_expiry(cert) return validate(cert) - return authenticate + return declare_proxy_headers(authenticate, header) def mtls_authenticate_fingerprint( *, @@ -383,7 +393,7 @@ def validate(cert: x509.Certificate) -> AuthContext: fp = cert.fingerprint(hash_algo).hex() ctx = fingerprints.get(fp) if ctx is None: - raise ValueError(f"Unknown certificate fingerprint: {fp}") + raise AuthFailure(AuthReason.INVALID_CREDENTIAL, f"Unknown certificate fingerprint: {fp}") return ctx return mtls_authenticate(validate=validate, header=header, check_expiry=check_expiry) @@ -430,7 +440,7 @@ def validate(cert: x509.Certificate) -> AuthContext: cn_attrs = subject.get_attributes_for_oid(x509.oid.NameOID.COMMON_NAME) cn = str(cn_attrs[0].value) if cn_attrs else "" if allowed_subjects is not None and cn not in allowed_subjects: - raise ValueError(f"Subject CN {cn!r} not in allowed subjects") + raise AuthFailure(AuthReason.INSUFFICIENT_SCOPE, f"Subject CN {cn!r} not in allowed subjects") rfc4514 = subject.rfc4514_string() serial_hex = format(cert.serial_number, "x") not_valid_after = cert.not_valid_after_utc.isoformat() diff --git a/vgi_rpc/http/_oauth_jwt.py b/vgi_rpc/http/_oauth_jwt.py index e38b300..abc34ed 100644 --- a/vgi_rpc/http/_oauth_jwt.py +++ b/vgi_rpc/http/_oauth_jwt.py @@ -22,11 +22,12 @@ try: from joserfc import jwt - from joserfc.errors import InvalidKeyIdError, JoseError + from joserfc.errors import ExpiredTokenError, InvalidKeyIdError, JoseError from joserfc.jwk import KeySet except ImportError as _exc: raise ImportError("jwt_authenticate requires joserfc: pip install vgi-rpc[oauth]") from _exc +from vgi_rpc.http._unauthorized import AuthFailure, AuthReason from vgi_rpc.rpc import AuthContext logger = logging.getLogger(__name__) @@ -147,10 +148,22 @@ def _decode_and_validate(raw_token: str, keys: KeySet) -> dict[str, Any]: registry.validate(decoded.claims) return dict(decoded.claims) + def _reject(token: str, exc: JoseError) -> AuthFailure: + """Log the claim mismatch and build the failure to raise. + + An expired token is separated from a bad one because the two call for + different actions: refresh and retry, versus stop and re-authenticate. + """ + _log_claim_mismatch(_peek_claims(token), exc) + reason = AuthReason.EXPIRED_CREDENTIAL if isinstance(exc, ExpiredTokenError) else AuthReason.INVALID_CREDENTIAL + return AuthFailure(reason, f"Invalid JWT: {exc}") + def authenticate(req: falcon.Request) -> AuthContext: auth_header = req.get_header("Authorization") or "" + if not auth_header: + raise AuthFailure(AuthReason.MISSING_CREDENTIAL, "Missing Authorization header") if not auth_header.startswith("Bearer "): - raise ValueError("Missing or invalid Authorization header") + raise AuthFailure(AuthReason.INVALID_CREDENTIAL, "Authorization header is not a Bearer credential") token = auth_header[7:] @@ -163,14 +176,10 @@ def authenticate(req: falcon.Request) -> AuthContext: try: claims = _decode_and_validate(token, keys) except JoseError as exc: - diag = _peek_claims(token) - _log_claim_mismatch(diag, exc) - raise ValueError(f"Invalid JWT: {exc}") from exc + raise _reject(token, exc) from exc except JoseError as exc: # Expired tokens, bad claims, etc. — no point refreshing keys - diag = _peek_claims(token) - _log_claim_mismatch(diag, exc) - raise ValueError(f"Invalid JWT: {exc}") from exc + raise _reject(token, exc) from exc principal = str(claims.get(principal_claim, "")) return AuthContext( diff --git a/vgi_rpc/http/_oauth_pkce.py b/vgi_rpc/http/_oauth_pkce.py index 39606ba..107f427 100644 --- a/vgi_rpc/http/_oauth_pkce.py +++ b/vgi_rpc/http/_oauth_pkce.py @@ -37,6 +37,7 @@ from vgi_rpc.rpc import AuthContext from ._common import _ERROR_PAGE_STYLE, _FONT_IMPORTS, _VGI_LOGO_HTML +from ._unauthorized import AuthFailure, AuthReason logger = logging.getLogger(__name__) @@ -1143,7 +1144,7 @@ def make_cookie_authenticate( def authenticate(req: falcon.Request) -> AuthContext: token = req.cookies.get(cookie_name) if not token: - raise ValueError("No auth cookie") + raise AuthFailure(AuthReason.MISSING_CREDENTIAL, "No auth cookie") # Temporarily inject the cookie token as an Authorization header # so the inner authenticator (JWT, bearer, etc.) can validate it. # Safe in synchronous WSGI — the environ is per-request and mutable. diff --git a/vgi_rpc/http/_proof.py b/vgi_rpc/http/_proof.py index ecd116f..23bb528 100644 --- a/vgi_rpc/http/_proof.py +++ b/vgi_rpc/http/_proof.py @@ -39,6 +39,7 @@ from vgi_rpc.http._bearer import PreconditionGate from vgi_rpc.http._common import PROOF_HEADER, PROOF_REQUIRED_HEADER from vgi_rpc.http._replay import DEFAULT_CAPACITY, NonceCache +from vgi_rpc.http._unauthorized import REASON_ATTR, AuthReason __all__ = [ "PROOF_HEADER", @@ -98,6 +99,13 @@ def __init__(self, reason: str, detail: str = "") -> None: """Create a proof failure carrying its reason code.""" super().__init__(detail or reason) self.reason = reason + # What the *caller* is told: the coarse stage code, never `reason`. + # A 401 already said "proxy proof required" before this attribute + # existed, so this discloses nothing new — and it deliberately + # collapses every verifier outcome (no_proof, bad_mac, expired, + # unknown_kid) onto one value, keeping rejection uniform as + # docs/proxy-proof-spec.md §6 requires. + setattr(self, REASON_ATTR, AuthReason.PROXY_REQUIRED) def _b64(raw: bytes) -> str: @@ -431,4 +439,12 @@ def gate(req: falcon.Request) -> Mapping[str, str]: "reason": exc.reason, } - return PreconditionGate(gate, name=GATE_NAME, claims_key=CLAIMS_KEY) + return PreconditionGate( + gate, + name=GATE_NAME, + claims_key=CLAIMS_KEY, + # Only `require` can refuse, so only `require` should tell an operator + # a 401 might be the proxy's fault. In `allow` mode an absent proof + # never denies, and the note would misdirect them. + proxy_headers=(PROOF_HEADER,) if required else (), + ) diff --git a/vgi_rpc/http/_testing.py b/vgi_rpc/http/_testing.py index e0598d9..6014bea 100644 --- a/vgi_rpc/http/_testing.py +++ b/vgi_rpc/http/_testing.py @@ -9,7 +9,7 @@ from __future__ import annotations -from collections.abc import Callable, Mapping +from collections.abc import Callable, Mapping, Sequence from typing import TYPE_CHECKING from urllib.parse import urlparse @@ -130,6 +130,8 @@ def make_sync_client( max_request_bytes: int | None = None, max_stream_response_bytes: int | None = None, authenticate: Callable[[falcon.Request], AuthContext] | None = None, + proxy_proof_required: bool = False, + proxy_auth_headers: Sequence[str] | None = None, default_headers: dict[str, str] | None = None, upload_url_provider: UploadUrlProvider | None = None, max_upload_bytes: int | None = None, @@ -162,6 +164,8 @@ def make_sync_client( max_stream_response_bytes: **Deprecated** alias for ``max_response_bytes``. authenticate: See ``make_wsgi_app``. + proxy_proof_required: See ``make_wsgi_app``. + proxy_auth_headers: See ``make_wsgi_app``. default_headers: Headers merged into every request (e.g. auth tokens). upload_url_provider: See ``make_wsgi_app``. max_upload_bytes: See ``make_wsgi_app``. @@ -192,6 +196,8 @@ def make_sync_client( max_stream_response_bytes=max_stream_response_bytes, max_request_bytes=max_request_bytes, authenticate=authenticate, + proxy_proof_required=proxy_proof_required, + proxy_auth_headers=proxy_auth_headers, upload_url_provider=upload_url_provider, max_upload_bytes=max_upload_bytes, otel_config=otel_config, diff --git a/vgi_rpc/http/_unauthorized.py b/vgi_rpc/http/_unauthorized.py new file mode 100644 index 0000000..f4cde9f --- /dev/null +++ b/vgi_rpc/http/_unauthorized.py @@ -0,0 +1,251 @@ +# © Copyright 2025-2026, Query.Farm LLC - https://query.farm +# SPDX-License-Identifier: Apache-2.0 + +"""The standardized shape of an HTTP 401 from a vgi-rpc service. + +Every 401 a vgi-rpc server emits carries the same three things: a reason code +from a closed set, a human-readable detail, and — when the service's own +configuration says requests must arrive through a reverse proxy — a note +saying so. The normative cross-language contract is +``docs/unauthorized-spec.md``; this module is its reference implementation. + +The reason code exists because the detail string is not something a client can +branch on. It is deliberately coarse: it names the *stage* that refused the +request, never the verifier's internal diagnosis. Whether a signature was +malformed or simply wrong is an operator's business, not a caller's, and +echoing that back is how a rejection turns into an oracle. + +The proxy note is derived from **server configuration**, not from what failed +on this particular request. A worker that requires proxy-injected evidence +emits the same note on every 401 it produces, so the note reveals nothing a +caller could not already read off the service's advertised capabilities — and +it stays useful in the case operators actually hit, where the proxy is simply +not forwarding the header and the credential was never the problem. +""" + +from __future__ import annotations + +from collections.abc import Callable, Iterable +from enum import StrEnum +from typing import Any + +from vgi_rpc.rpc import RpcError + +__all__ = [ + "AuthFailure", + "AuthReason", + "AuthenticationError", + "build_proxy_hint", + "classify_auth_failure", + "declare_proxy_headers", + "merge_proxy_headers", + "proxy_headers_of", +] + + +class AuthReason(StrEnum): + """The closed set of reason codes a 401 may carry. + + Coarse by design — see the module docstring. A port that cannot map one + of its failures onto a specific member uses :attr:`UNAUTHORIZED` rather + than inventing a code, so the set stays closed across languages. + """ + + MISSING_CREDENTIAL = "missing_credential" + """No credential was presented at all.""" + + INVALID_CREDENTIAL = "invalid_credential" + """A credential was presented and rejected.""" + + EXPIRED_CREDENTIAL = "expired_credential" + """A well-formed credential that is outside its validity window.""" + + INSUFFICIENT_SCOPE = "insufficient_scope" + """The caller was identified but is not permitted.""" + + PROXY_REQUIRED = "proxy_required" + """The request did not carry evidence that it came through the trusted proxy.""" + + UNAUTHORIZED = "unauthorized" + """Refused, unclassified. The fallback for a custom authenticator.""" + + +# Every exception that wants to steer the reason code sets this attribute. +# Duck-typed rather than isinstance-checked so that an authenticator defined +# outside this package — or in a module this one must not import, such as +# ``_proof`` — can participate without a dependency edge. The name is +# deliberately not ``reason``: ``ProofError.reason`` already means the +# verifier's internal code, which must never reach a caller. +REASON_ATTR = "vgi_auth_reason" + + +class AuthFailure(ValueError): + """A rejected credential, carrying its reason code. + + Subclasses :class:`ValueError` so that + :func:`vgi_rpc.http.chain_authenticate` still treats it as "try the next + credential". A precondition that must *not* be swallowed that way raises + :class:`PermissionError` instead — see :class:`vgi_rpc.http.ProofError`. + """ + + def __init__(self, reason: AuthReason, detail: str = "") -> None: + """Create a failure with a reason code and an optional detail message. + + Args: + reason: The code to surface to the caller. + detail: Human-readable text. Defaults to the reason code itself. + + """ + super().__init__(detail or reason.value) + self.reason = reason + setattr(self, REASON_ATTR, reason) + + +class AuthenticationError(RpcError): + """Client-side view of a 401, with the server's reason code unpacked. + + Subclasses :class:`~vgi_rpc.rpc.RpcError` with ``error_type`` fixed at + ``"AuthenticationError"``, so code that catches ``RpcError`` or matches on + that type keeps working; the added attributes are for callers that want to + branch — retry after refreshing a token on + :attr:`AuthReason.EXPIRED_CREDENTIAL`, give up on + :attr:`AuthReason.INSUFFICIENT_SCOPE`. + """ + + def __init__( + self, + reason: AuthReason, + detail: str, + proxy_hint: str = "", + *, + request_id: str = "", + ) -> None: + """Build the error from a parsed 401 envelope. + + Args: + reason: The reason code the server reported. + detail: The server's human-readable text. + proxy_hint: The server's proxy-configuration note, if any. It is + appended to the exception message rather than left on an + attribute alone, because the place this actually gets read is + a traceback in a deployment log. + request_id: Correlation id, when one is known. + + """ + message = detail or reason.value + if proxy_hint: + message = f"{message}\n\n{proxy_hint}" + super().__init__("AuthenticationError", message, "", request_id=request_id) + self.reason = reason + self.detail = detail + self.proxy_hint = proxy_hint + + +def classify_auth_failure(exc: BaseException) -> AuthReason: + """Map an authenticate-callback exception onto a reason code. + + Args: + exc: The exception raised by the ``authenticate`` callback. + + Returns: + The declared reason when the exception carries one, otherwise a + conservative guess: :attr:`AuthReason.INSUFFICIENT_SCOPE` for a + :class:`PermissionError` (the caller got as far as being identified) + and :attr:`AuthReason.UNAUTHORIZED` for anything else. Guessing more + finely would mean matching on message text, which silently + misclassifies the moment someone rewords a string. + + """ + declared = getattr(exc, REASON_ATTR, None) + if isinstance(declared, AuthReason): + return declared + if isinstance(exc, PermissionError): + return AuthReason.INSUFFICIENT_SCOPE + return AuthReason.UNAUTHORIZED + + +# --------------------------------------------------------------------------- +# Proxy-dependence declaration +# --------------------------------------------------------------------------- + +_PROXY_HEADERS_ATTR = "vgi_proxy_headers" + + +def declare_proxy_headers[F: Callable[..., Any]](fn: F, *headers: str) -> F: + """Record that *fn* can only succeed on requests a reverse proxy has stamped. + + The built-in mTLS and proxy-proof authenticators call this on themselves, + so ``make_wsgi_app`` discovers the dependency without the operator + restating it. Third-party authenticators can call it too; anything not + declared can still be stated directly via ``make_wsgi_app``'s + ``proxy_auth_headers``. + + Args: + fn: The authenticate callback (or gate) to annotate, mutated in place. + *headers: Header names the proxy must inject. Duplicates and + already-declared names are collapsed. + + Returns: + *fn*, so this can wrap a callable at its return site. + + """ + merged = tuple(dict.fromkeys([*proxy_headers_of(fn), *headers])) + setattr(fn, _PROXY_HEADERS_ATTR, merged) + return fn + + +def proxy_headers_of(fn: object) -> tuple[str, ...]: + """Return the proxy-injected headers *fn* was declared to depend on. + + Args: + fn: Any object, typically an authenticate callback. ``None`` and + objects with no declaration are accepted. + + Returns: + The declared header names, or an empty tuple. + + """ + declared = getattr(fn, _PROXY_HEADERS_ATTR, ()) + if isinstance(declared, tuple | list): + return tuple(str(name) for name in declared) + return () + + +def merge_proxy_headers(*sources: object) -> tuple[str, ...]: + """Collect the proxy-header declarations of several callables in order. + + Args: + *sources: Callables (or anything) to read declarations from. + + Returns: + The union, first-seen order preserved. + + """ + names: list[str] = [] + for source in sources: + names.extend(proxy_headers_of(source)) + return tuple(dict.fromkeys(names)) + + +def build_proxy_hint(headers: Iterable[str]) -> str: + """Compose the operator-facing note about proxy-dependent authentication. + + Args: + headers: The header names a trusted proxy must inject. + + Returns: + The note text, or ``""`` when *headers* is empty. + + """ + names = tuple(dict.fromkeys(headers)) + if not names: + return "" + listed = ", ".join(names) + noun = "header" if len(names) == 1 else "headers" + return ( + f"This service only accepts requests that arrive through its configured reverse proxy, " + f"which must set the {listed} {noun}. A rejection here is as likely to be a proxy that is " + f"not forwarding {'that header' if len(names) == 1 else 'those headers'} — or a request " + f"that reached the service without passing through the proxy at all — as it is a bad " + f"credential. Check the proxy configuration before rotating credentials." + ) diff --git a/vgi_rpc/http/server/_errors.py b/vgi_rpc/http/server/_errors.py index 2a63d2c..32bb494 100644 --- a/vgi_rpc/http/server/_errors.py +++ b/vgi_rpc/http/server/_errors.py @@ -1,17 +1,30 @@ # © Copyright 2025-2026, Query.Farm LLC - https://query.farm # SPDX-License-Identifier: Apache-2.0 -"""Falcon error serializer and 404 sink for unmatched routes.""" +"""Falcon error serializer and 404 sink for unmatched routes. + +The 401 serializer is the server half of ``docs/unauthorized-spec.md``: one +reason code, one detail, and — on a service whose auth depends on a reverse +proxy — one note saying so, rendered as a page for a browser and as JSON for +everything else. +""" from __future__ import annotations import html as _html +import json from collections.abc import Callable from typing import Any import falcon -from .._common import _ERROR_PAGE_STYLE, _FONT_IMPORTS +from .._common import ( + _ERROR_PAGE_STYLE, + _FONT_IMPORTS, + AUTH_PROXY_REQUIRED_HEADER, + AUTH_REASON_HEADER, +) +from .._unauthorized import AuthReason _NOT_FOUND_HTML_TEMPLATE = ( """\ @@ -58,8 +71,10 @@ vgi-rpc logo

401 — Unauthorized

+
{reason}

Authentication is required to access this vgi-rpc service.

{detail} +{note} @@ -67,48 +82,133 @@ """ ) -_UNAUTHORIZED_BODY_CACHE: dict[str, bytes] = {} +# Keyed by (reason, detail); the proxy note is fixed per app, so it is baked +# into the closure rather than the key. Bounded because `detail` is chosen by +# the authenticate callback and a callback is free to embed request data in it. +_UNAUTHORIZED_CACHE_LIMIT = 64 -def _get_unauthorized_body(detail: str) -> bytes: - """Return cached UTF-8 bytes for a 401 HTML page with the given detail message. +def _render_unauthorized_html(reason: AuthReason, detail: str, proxy_hint: str) -> bytes: + """Render the 401 page for a browser. Args: - detail: The error description to display on the page. + reason: The reason code, shown as a chip under the heading. + detail: Human-readable rejection text, or ``""`` to omit the block. + proxy_hint: The proxy-configuration note, or ``""`` to omit it. Returns: UTF-8 encoded HTML bytes. """ - body = _UNAUTHORIZED_BODY_CACHE.get(detail) - if body is not None: - return body detail_html = f'
{_html.escape(detail)}
' if detail else "" - body = _UNAUTHORIZED_HTML_TEMPLATE.format(detail=detail_html).encode("utf-8") - # Bound cache size to prevent memory issues with unique error messages. - if len(_UNAUTHORIZED_BODY_CACHE) < 64: - _UNAUTHORIZED_BODY_CACHE[detail] = body - return body + note_html = ( + f'
Is the reverse proxy configured?{_html.escape(proxy_hint)}
' + if proxy_hint + else "" + ) + return _UNAUTHORIZED_HTML_TEMPLATE.format( + reason=_html.escape(reason.value), + detail=detail_html, + note=note_html, + ).encode("utf-8") + + +def _render_unauthorized_json(reason: AuthReason, detail: str, proxy_hint: str) -> bytes: + """Render the 401 envelope for a non-browser client. + + Args: + reason: The reason code. + detail: Human-readable rejection text. + proxy_hint: The proxy-configuration note, omitted from the payload + when empty so that its presence alone is a usable signal. + + Returns: + UTF-8 encoded JSON bytes. + + """ + payload: dict[str, str] = {"error": "unauthorized", "reason": reason.value, "detail": detail} + if proxy_hint: + payload["proxy_hint"] = proxy_hint + return json.dumps(payload, separators=(",", ":")).encode("utf-8") -def _error_serializer(req: falcon.Request, resp: falcon.Response, exc: falcon.HTTPError) -> None: - """Serialize Falcon HTTP errors as styled HTML pages. +def _wants_html(req: falcon.Request) -> bool: + """Report whether the caller is a browser that should get the styled page. - Only ``HTTPUnauthorized`` (401) gets a styled HTML page; all other errors - fall back to Falcon's default JSON serialization. + Substring-matching ``Accept`` rather than doing full media-type + negotiation is deliberate and matches the OAuth PKCE redirect middleware: + the only clients that ask for ``text/html`` are browsers, and the Arrow + client sends ``*/*``, which must resolve to JSON. Args: req: The Falcon request. - resp: The Falcon response. - exc: The Falcon HTTP error being serialized. + + Returns: + ``True`` when the request explicitly accepts HTML. """ - if isinstance(exc, falcon.HTTPUnauthorized): - resp.content_type = "text/html; charset=utf-8" - resp.data = _get_unauthorized_body(exc.description or "") - else: - resp.content_type = falcon.MEDIA_JSON - resp.data = exc.to_json() + return "text/html" in (req.get_header("Accept") or "") + + +def _make_error_serializer(proxy_hint: str = "") -> Callable[[falcon.Request, falcon.Response, falcon.HTTPError], None]: + """Build the Falcon error serializer for one app. + + Only ``HTTPUnauthorized`` (401) is given the standardized treatment; every + other error falls back to Falcon's default JSON serialization. + + Args: + proxy_hint: The app's static proxy-configuration note, or ``""`` when + its authentication does not depend on a reverse proxy. Fixed for + the life of the app — see :mod:`vgi_rpc.http._unauthorized` for + why it is not derived per request. + + Returns: + A serializer suitable for ``falcon.App.set_error_serializer``. + + """ + html_cache: dict[tuple[AuthReason, str], bytes] = {} + json_cache: dict[tuple[AuthReason, str], bytes] = {} + + def _serialize(req: falcon.Request, resp: falcon.Response, exc: falcon.HTTPError) -> None: + """Serialize one Falcon error onto the response.""" + if not isinstance(exc, falcon.HTTPUnauthorized): + resp.content_type = falcon.MEDIA_JSON + resp.data = exc.to_json() + return + + # _AuthMiddleware stashes the classified reason here. Anything else + # that raises HTTPUnauthorized — a resource, a third-party middleware + # — lands on the unclassified fallback rather than being guessed at. + reason = getattr(req.context, "vgi_auth_reason", AuthReason.UNAUTHORIZED) + if not isinstance(reason, AuthReason): + reason = AuthReason.UNAUTHORIZED + detail = exc.description or "" + + resp.set_header(AUTH_REASON_HEADER, reason.value) + if proxy_hint: + resp.set_header(AUTH_PROXY_REQUIRED_HEADER, "true") + # A 401 is request-specific and may flip to 200 on the next attempt + # with a credential; a shared cache holding onto it would be a bug. + resp.set_header("Cache-Control", "no-store") + + key = (reason, detail) + if _wants_html(req): + body = html_cache.get(key) + if body is None: + body = _render_unauthorized_html(reason, detail, proxy_hint) + if len(html_cache) < _UNAUTHORIZED_CACHE_LIMIT: + html_cache[key] = body + resp.content_type = "text/html; charset=utf-8" + else: + body = json_cache.get(key) + if body is None: + body = _render_unauthorized_json(reason, detail, proxy_hint) + if len(json_cache) < _UNAUTHORIZED_CACHE_LIMIT: + json_cache[key] = body + resp.content_type = falcon.MEDIA_JSON + resp.data = body + + return _serialize def _make_not_found_sink( diff --git a/vgi_rpc/http/server/_factory.py b/vgi_rpc/http/server/_factory.py index e804a83..ad50df8 100644 --- a/vgi_rpc/http/server/_factory.py +++ b/vgi_rpc/http/server/_factory.py @@ -8,7 +8,7 @@ import logging import os import warnings -from collections.abc import Callable, Iterable, Mapping +from collections.abc import Callable, Iterable, Mapping, Sequence from typing import TYPE_CHECKING, Any if TYPE_CHECKING: @@ -21,12 +21,15 @@ from .._common import ( _SESSION_ENDPOINT, + AUTH_PROXY_REQUIRED_HEADER, + AUTH_REASON_HEADER, ECHO_HEADER_PREFIX, EXTERNALIZATION_ENABLED_HEADER, MAX_EXTERNALIZED_RESPONSE_BYTES_HEADER, MAX_REQUEST_BYTES_HEADER, MAX_RESPONSE_BYTES_HEADER, MAX_UPLOAD_BYTES_HEADER, + PROOF_HEADER, PROOF_REQUIRED_HEADER, RPC_ERROR_HEADER, SESSION_CLOSE_HEADER, @@ -39,8 +42,9 @@ Encoding, available_encodings, ) +from .._unauthorized import build_proxy_hint, proxy_headers_of from ._app import _HttpRpcApp -from ._errors import _error_serializer, _make_not_found_sink +from ._errors import _make_error_serializer, _make_not_found_sink from ._middleware import ( _REQUEST_ID_HEADER, _AccessLogContextMiddleware, @@ -86,6 +90,7 @@ def make_wsgi_app( max_request_bytes: int | None = None, authenticate: Callable[[falcon.Request], AuthContext] | None = None, proxy_proof_required: bool = False, + proxy_auth_headers: Sequence[str] | None = None, cors_origins: str | Iterable[str] | None = None, cors_max_age: int | None = 7200, upload_url_provider: UploadUrlProvider | None = None, @@ -157,7 +162,19 @@ def make_wsgi_app( ``authenticate`` (see :func:`vgi_rpc.http.require_all`), which is an opaque callable — the factory cannot introspect it, so the operator states the posture here. Purely advertisement: it does not enable - or enforce anything. + or enforce anything. Also contributes ``VGI-Proxy-Proof`` to the + proxy note described under ``proxy_auth_headers``. + proxy_auth_headers: Names of headers a trusted reverse proxy must + inject for authentication to succeed. When any are known, every + 401 this app emits carries ``VGI-Auth-Proxy-Required: true`` and a + note telling the reader to check the proxy configuration before + blaming the credential — the failure mode operators actually hit. + The built-in mTLS and proxy-proof authenticators declare their own + headers (see :func:`vgi_rpc.http.declare_proxy_headers`) and are + discovered automatically, so this is only needed for a custom + ``authenticate`` that reads a proxy-injected header. The note is + fixed for the life of the app and identical on every 401, so it + discloses nothing about what failed on a given request. cors_origins: Allowed origins for CORS. Pass ``"*"`` to allow all origins, a single origin string like ``"https://example.com"``, or an iterable of origin strings. ``None`` (the default) @@ -313,6 +330,19 @@ def make_wsgi_app( token_ttl, max_externalized_response_bytes=max_externalized_response_bytes, ) + # Resolve the proxy-header dependency now, from the `authenticate` the + # caller passed. The PKCE branch below rebinds `authenticate` to a chained + # wrapper, and reading it after that point would work only because + # chain_authenticate propagates declarations — depending on that here + # would be a trap for anyone who later adds a wrapper that does not. + proxy_hint = build_proxy_hint( + [ + *(proxy_auth_headers or ()), + *proxy_headers_of(authenticate), + *((PROOF_HEADER,) if proxy_proof_required else ()), + ] + ) + middleware: list[Any] = [ _TransportNotifyMiddleware(server), _DrainRequestMiddleware(), @@ -448,6 +478,14 @@ def make_wsgi_app( capability_headers[PROOF_REQUIRED_HEADER] = "true" cors_expose.append(PROOF_REQUIRED_HEADER) + # 401 explanation headers. Exposed but never advertised as capabilities: + # they describe a rejection, so emitting them on a 200 would be noise. + # Without the CORS exposure a browser client could not read the reason + # code off a cross-origin 401 and would be back to guessing from the body. + cors_expose.append(AUTH_REASON_HEADER) + if proxy_hint: + cors_expose.append(AUTH_PROXY_REQUIRED_HEADER) + if enable_sticky: capability_headers[STICKY_ENABLED_HEADER] = "true" capability_headers[STICKY_DEFAULT_TTL_HEADER] = str(int(sticky_default_ttl)) @@ -595,7 +633,7 @@ def make_wsgi_app( if capability_headers: middleware.append(_CapabilitiesMiddleware(capability_headers)) app: falcon.App[falcon.Request, falcon.Response] = falcon.App(middleware=middleware or None) - app.set_error_serializer(_error_serializer) + app.set_error_serializer(_make_error_serializer(proxy_hint)) # OAuth well-known endpoint (must be before RPC routes) if _validated_oauth_metadata is not None: diff --git a/vgi_rpc/http/server/_middleware.py b/vgi_rpc/http/server/_middleware.py index cfc478f..70cea7f 100644 --- a/vgi_rpc/http/server/_middleware.py +++ b/vgi_rpc/http/server/_middleware.py @@ -53,6 +53,7 @@ from .._common import ( decompress as _decompress_with_encoding, ) +from .._unauthorized import classify_auth_failure _logger = logging.getLogger("vgi_rpc.http") @@ -228,6 +229,13 @@ class _AuthMiddleware: credentials) or ``PermissionError`` (forbidden) on failure. Other exceptions propagate as 500s so that bugs in the callback are not silently swallowed as 401s. + + A failure is classified into an :class:`AuthReason` and stashed on + ``req.context`` for the error serializer, which turns it into the + standardized 401 of ``docs/unauthorized-spec.md``. Raising + :class:`vgi_rpc.http.AuthFailure` picks the code explicitly; a bare + ``ValueError`` lands on the unclassified fallback rather than being + guessed at from its message text. """ __slots__ = ("_authenticate", "_exempt_prefixes", "_on_auth_failure", "_www_authenticate") @@ -271,6 +279,11 @@ def process_request(self, req: falcon.Request, resp: falcon.Response) -> None: try: auth = self._authenticate(req) except (ValueError, PermissionError) as exc: + # Classify before raising so the error serializer can put a code on + # the response. It reads req.context rather than re-deriving from + # the Falcon error, which by then has lost the original type. + reason = classify_auth_failure(exc) + req.context.vgi_auth_reason = reason _logger.warning( "Auth failure from %s: %s", req.remote_addr, @@ -279,6 +292,7 @@ def process_request(self, req: falcon.Request, resp: falcon.Response) -> None: "remote_addr": req.remote_addr or "", "error_type": type(exc).__name__, "auth_error": str(exc), + "auth_reason": reason.value, }, ) if self._on_auth_failure is not None: