Merge pull request '[session 2] docs(agent-proxy): agent token validation resilience (#30, #46, #48)' (#67) from docs/agent-proxy-auth-resilience into develop

2026-07-26 07:52:18 -04:00
parent 540e30a77d 7c23aceadb
commit ab02c310da
1 changed files with 72 additions and 1 deletions
@@ -51,7 +51,7 @@ Not exhaustive — see the controller package for the full set.
 ## Gotchas

 - **Auth role is `ROLE_AGENT`, not the admin pattern.** `SecurityConfig` requires `hasRole("AGENT")` on every request except `/actuator/health`, `/actuator/info`, `/error`. Do NOT expect `hasAnyRole('MSP_ADMIN','BCOS_ADMIN')` here — that is for admin-facing services.
- **Two auth mechanisms, checked in order** (`AgentAuthFilter`): (1) `Authorization: Bearer agtkn_*` → validated against mgmt `/api/internal/agent-tokens/validate`, cached 5 min (`AGENT_TOKEN_CACHE_SECONDS`, default 300); (2) legacy `X-Institution-Key` header → provisional `ROLE_AGENT`, uppercased, incident re-validates. Principal is `agent:{institutionKey}` in both cases.
+- **Two auth mechanisms, checked in order** (`AgentAuthFilter`): (1) `Authorization: Bearer agtkn_*` → validated against mgmt `/api/internal/agent-tokens/validate`, cached 5 min (`AGENT_TOKEN_CACHE_SECONDS`, default 300); (2) legacy `X-Institution-Key` header → provisional `ROLE_AGENT`, uppercased, incident re-validates. Principal is `agent:{institutionKey}` in both cases. **An mgmt outage no longer 401s the fleet** — see *Agent token validation resilience* below before changing anything in `AgentTokenValidationService`.
 - **`INTERNAL_SERVICE_SECRET` is required.** All outbound calls to incident carry it as `X-Internal-Secret`; incident's `InternalSecretFilter` grants `ROLE_INTERNAL`. Empty by default in config.
 - **Uses `DB_HOST`/`DB_PORT`/`DB_NAME`, not `DB_URL`.** (Contrast with hiveops-journal, which expects a full `DB_URL`.) Dev profile has Flyway disabled and uses H2 `create-drop`; prod uses Flyway + `ddl-auto: validate`.
 - **`atm_id` / `device_id` dual-write.** V2 migration keeps both columns in sync via a BEFORE INSERT/UPDATE trigger (device-rename Phase 1). `MachineFingerprintService` should accept both `atmId` and `deviceId` JSON field names from heartbeats.
@@ -62,3 +62,74 @@ Not exhaustive — see the controller package for the full set.
 New agent-authed `POST /api/atms/hardware-events` (AgentAuthFilter) injects `institutionKey` and
 forwards to devices `/api/internal/atms/hardware-events`. Thin gateway, no Kafka here — mirrors the
 existing `hardware/sync` path. Not yet enabled. See [agent.xfs], [technical.devices].
+
+## Agent token validation resilience
+
+**Read this before touching `AgentTokenValidationService`.** Its shape is not accidental — an earlier
+version took the entire ATM fleet down.
+
+### What went wrong (prod, 2026-07-17)
+
+`hiveops-mgmt` briefly stopped answering. `validate()` caught every exception and returned
+"invalid", while annotated `@Cacheable`. So a transport error was stored as though mgmt had rendered
+a verdict, and **every ATM got 401 for the full 5-minute cache TTL** — long after mgmt recovered.
+Observed: 1455 / 1252 / 318 401s across three consecutive minutes.
+
+### Three outcomes, not two
+
+| Outcome | Trigger | Cached? |
+|---|---|---|
+| **VALID** | mgmt 2xx, `valid=true` | yes, plus recorded as last-known-good |
+| **INVALID** | mgmt 2xx, `valid=false` | yes — the **only** definitive negative |
+| **UNAVAILABLE** | I/O error, timeout, 4xx, 5xx | **never** — falls back to last-known-good |
+
+Only an answer mgmt actually rendered is cached. A 2xx with no body counts as UNAVAILABLE, not as a
+verdict. `@Cacheable` was replaced with explicit `CacheManager` use precisely because the return value
+must **not** always be cached.
+
+Two caches back this: `agentTokens` (hot verdicts, `AGENT_TOKEN_CACHE_SECONDS`, default 300s) and
+`agentTokensFallback` (last-known-good, `AGENT_TOKEN_GRACE_SECONDS`, default 24h). Only tokens mgmt has
+already accepted are eligible for the fallback, so an unknown or forged token is still rejected during
+an outage.
+
+### Bounded call, then a circuit breaker
+
+- **Timeouts** (`mgmtRestClient` bean in `RestConfig`): 2s connect / 5s read. Without them an
+  unreachable mgmt held a Tomcat worker until the OS timeout — verified pre-fix as an nginx **504**.
+- **Circuit breaker** (`MgmtCircuitBreaker`): opens after 5 consecutive UNAVAILABLE outcomes, stays
+  open 10s, then admits exactly one probe (half-open). While open, mgmt is not called at all.
+  **Only UNAVAILABLE counts as a failure** — mgmt rejecting a token is a *successful* call, so a fleet
+  presenting revoked tokens cannot trip the breaker and mask real revocations.
+
+Timeouts bound one call; the breaker bounds the aggregate. At the ~24 req/s seen during the incident,
+a 5s read timeout would otherwise hold ~120 workers against a 200-thread pool.
+
+### The trade-off
+
+While mgmt is unreachable, a token revoked **during** the outage keeps working until the grace window
+expires. That is deliberate and fits the staged revocation process (push a fleet-wide `CONFIG_UPDATE`
+before revoking). Two independent kill switches: `AGENT_TOKEN_GRACE_ENABLED=false` restores strict
+fail-closed, `AGENT_TOKEN_BREAKER_ENABLED=false` disables the breaker.
+
+### Config
+
+| Env var | Default | Purpose |
+|---|---|---|
+| `AGENT_TOKEN_CACHE_SECONDS` | 300 | hot verdict cache TTL |
+| `AGENT_TOKEN_GRACE_SECONDS` | 86400 | last-known-good window |
+| `AGENT_TOKEN_GRACE_ENABLED` | true | fail-open kill switch |
+| `AGENT_TOKEN_CONNECT_TIMEOUT_MS` | 2000 | mgmt connect budget |
+| `AGENT_TOKEN_READ_TIMEOUT_MS` | 5000 | mgmt read budget |
+| `AGENT_TOKEN_BREAKER_ENABLED` | true | breaker kill switch |
+| `AGENT_TOKEN_BREAKER_FAILURE_THRESHOLD` | 5 | consecutive UNAVAILABLE before opening |
+| `AGENT_TOKEN_BREAKER_OPEN_SECONDS` | 10 | open window before a probe |
+
+### How to test it
+
+Do **not** reuse a token across the control and the outage. mgmt answering `valid=false` for it while
+healthy caches a definitive negative for 300s, so every later request hits the cache, never calls mgmt,
+and returns a fast 401 with no log output — a false pass that looks clean. Use **distinct never-seen
+tokens** during the outage, and confirm the precondition (`/mgmt/...` returning 502) before trusting
+the result.
+
+Issues: agent-proxy#30 (negative caching), #46 (timeouts), #48 (breaker).