feat(auth): add CLIENT_REGISTRATION_TTL=0 never-expire opt-in

README.md

@@ -183,7 +183,7 @@ alone remain replayable within their TTL.
 
 | Flow state | Encrypted into | TTL |
 |---|---|---|
-| Client registration | `client_id` | 7d (configurable via `CLIENT_REGISTRATION_TTL`) |
+| Client registration | `client_id` | 7d (configurable via `CLIENT_REGISTRATION_TTL`; `0` = never expires) |
 | Authorize session | IdP `state` parameter | 10min |
 | Authorization code | `code` parameter | 60s |
 | Access token | Opaque bearer | 1h |

config/config.go

@@ -375,14 +375,21 @@ func Load() (*Config, error) {
 	// Hard cap at 90d — a longer envelope materially extends the
 	// window an exfiltrated client_id (which is unauthenticated
 	// metadata) can be reused.
+	//
+	// Set to 0 to disable expiry entirely (emits client_id_expires_at=0,
+	// "never expires" — mirroring RFC 7591 §3.2.1 client_secret_expires_at).
+	// Opt-in escape hatch for fleets whose MCP clients never re-run DCR
+	// on invalid_client (e.g. Azure APIM connectors) and would otherwise
+	// break on every TTL window. Trades away the reuse-window bound — see
+	// docs/runbooks/client-registration-expired.md.
 	c.ClientRegistrationTTL = 7 * 24 * time.Hour
 	if raw := os.Getenv("CLIENT_REGISTRATION_TTL"); raw != "" {
 		d, err := time.ParseDuration(raw)
 		if err != nil {
-			return nil, fmt.Errorf("CLIENT_REGISTRATION_TTL must be a Go duration (e.g. 168h, 24h, 720h): %w", err)
+			return nil, fmt.Errorf("CLIENT_REGISTRATION_TTL must be a Go duration (e.g. 168h, 24h, 720h, or 0 to never expire): %w", err)
 		}
-		if d <= 0 {
-			return nil, fmt.Errorf("CLIENT_REGISTRATION_TTL must be positive, got %s", d)
+		if d < 0 {
+			return nil, fmt.Errorf("CLIENT_REGISTRATION_TTL must not be negative (use 0 to never expire), got %s", d)
 		}
 		if d > 90*24*time.Hour {
 			return nil, fmt.Errorf("CLIENT_REGISTRATION_TTL exceeds 90d cap, got %s", d)

config/config_test.go

@@ -565,12 +565,32 @@ func TestLoad_ClientRegistrationTTL_Custom(t *testing.T) {
 	}
 }
 
-func TestLoad_ClientRegistrationTTL_RejectsNonPositive(t *testing.T) {
+// 0 is the documented opt-in for "never expires" (client_id_expires_at=0),
+// not an error: it must load cleanly as a zero duration. Both the
+// bare "0" (the natural operator input the docs imply) and "0s" are
+// accepted by time.ParseDuration and must both reach the sentinel.
+func TestLoad_ClientRegistrationTTL_ZeroMeansNever(t *testing.T) {
+	for _, raw := range []string{"0", "0s"} {
+		t.Run(raw, func(t *testing.T) {
+			setAllRequired(t)
+			t.Setenv("CLIENT_REGISTRATION_TTL", raw)
+			cfg, err := Load()
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if cfg.ClientRegistrationTTL != 0 {
+				t.Errorf("ClientRegistrationTTL = %v, want 0 (never expires)", cfg.ClientRegistrationTTL)
+			}
+		})
+	}
+}
+
+func TestLoad_ClientRegistrationTTL_RejectsNegative(t *testing.T) {
 	setAllRequired(t)
-	t.Setenv("CLIENT_REGISTRATION_TTL", "0s")
+	t.Setenv("CLIENT_REGISTRATION_TTL", "-1h")
 	_, err := Load()
-	if err == nil || !strings.Contains(err.Error(), "positive") {
-		t.Fatalf("want positive rejection, got %v", err)
+	if err == nil || !strings.Contains(err.Error(), "negative") {
+		t.Fatalf("want negative rejection, got %v", err)
 	}
 }
 

docs/configuration.md

@@ -45,7 +45,7 @@ secure production posture (`PROD_MODE=true`); flags listed here as
 |---|---|---|
 | `TOKEN_SIGNING_SECRETS_PREVIOUS` | (empty) | Whitespace-separated retired signing secrets accepted on Open during a rolling rotation. New seals always use the primary `TOKEN_SIGNING_SECRET`; Open tries primary first, then each previous. See [`runbooks/key-rotation.md`](./runbooks/key-rotation.md). |
 | `REVOKE_BEFORE` | (empty) | RFC3339 timestamp. Bulk revocation cutoff: tokens with `iat` before this are rejected. Applies to access AND refresh tokens. |
-| `CLIENT_REGISTRATION_TTL` | `168h` (7d) | Lifetime of a sealed `client_id` minted by `POST /register`. Default matches the 7-day refresh-token TTL so a client holding a still-valid refresh can always exchange it; a shorter value silently kills long-running MCP clients (which treat DCR as one-shot at startup) the moment their access token first expires. Go duration syntax (`168h`, `720h`, …); capped at 90d. **Rolling-deploy note:** the TTL is sealed into each `client_id` at registration time, so bumping this env var only affects newly-issued client_ids — existing registrations stay on whatever TTL was in effect when they were minted. See [`runbooks/client-registration-expired.md`](./runbooks/client-registration-expired.md). |
+| `CLIENT_REGISTRATION_TTL` | `168h` (7d) | Lifetime of a sealed `client_id` minted by `POST /register`. Default matches the 7-day refresh-token TTL so a client holding a still-valid refresh can always exchange it; a shorter value silently kills long-running MCP clients (which treat DCR as one-shot at startup) the moment their access token first expires. Go duration syntax (`168h`, `720h`, …); capped at 90d. **Set to `0` to disable expiry entirely** (`client_id_expires_at=0`, "never expires" — mirroring RFC 7591 §3.2.1 `client_secret_expires_at`) — opt-in escape hatch for clients that never re-run DCR on `invalid_client` (e.g. Azure APIM connectors); trades away the reuse-window bound. **Rolling-deploy note:** the TTL is sealed into each `client_id` at registration time, so bumping this env var only affects newly-issued client_ids — existing registrations stay on whatever TTL was in effect when they were minted. See [`runbooks/client-registration-expired.md`](./runbooks/client-registration-expired.md). |
 
 ## Replay store (Redis)
 
@@ -158,6 +158,21 @@ sum(mcp_auth_consent_decisions_total{decision="approved"})
   - `state_missing` — `/authorize` without state in strict mode.
   - `refresh_family_revoked` / `refresh_concurrent_submit` —
     refresh-rotation outcomes.
+  - `client_id_missing` — `/authorize` received a request with no
+    `client_id`. (`/token` folds a missing `client_id` into its generic
+    `invalid_request` "missing required parameters", unmetered.)
+  - `client_id_invalid` — `/authorize` or `/token` could not decode
+    the sealed `client_id` (tampered, truncated, or wrong key).
+  - `client_typ_mismatch` — the blob decoded but is not a client
+    registration (sealed-type-confusion defense; should not fire
+    absent tampering).
+  - `client_audience_mismatch` — sealed `client_id` was issued for a
+    different proxy deployment (distinct from `audience_mismatch`,
+    which is the access-token binding).
+  - `client_registration_expired` — sealed `client_id` outlived
+    `CLIENT_REGISTRATION_TTL`. Sustained counts = a client that
+    never re-runs DCR; see
+    [`runbooks/client-registration-expired.md`](./runbooks/client-registration-expired.md).
 - `mcp_auth_replay_detected_total{kind}` — `code` / `refresh` /
   `consent` / `callback_state` replays caught by the Redis-backed
   store. The `consent` kind answers with a re-rendered consent page

docs/runbooks/client-registration-expired.md

@@ -14,14 +14,31 @@ re-register and the user sees the error directly.
 
 ## Signals
 
-- Per-client log line: `client_registration_expired`
-  (`handlers/helpers.go`, fired by `openAndValidateClient`).
-- Counter: `mcp_auth_access_denied_total{reason="invalid_client"}`
-  with `error_description="client registration expired"` in the
-  log line that accompanies it.
+- Counter: `mcp_auth_access_denied_total{reason="client_registration_expired"}`
+  — incremented on every rejection by both `/authorize` and
+  `/token` (`handlers/authorize.go`, `handlers/helpers.go`
+  `openAndValidateClient`). A tight, sustained rise = a stuck
+  client (e.g. an Azure APIM connector) looping on the 400 instead
+  of re-running DCR.
+- Log line: `access_denied_client_registration_expired` at WARN with
+  the client's `internal_id` and `expired_at` (same `access_denied_*`
+  family as every other denial — catchable by a `level>=warn` or
+  `event=~"access_denied_.*"` filter).
+- HTTP access log: repeated `GET /authorize` (and/or `POST
+  /token`) at **status 400** with a **77-byte** response body —
+  the 76-byte JSON
+  `{"error":"invalid_client","error_description":"client registration expired"}`
+  plus the encoder's trailing newline (`resp_bytes=77` on the wire).
 - User report: "MCP server stopped working after a few days of
   uptime, restart fixes it."
 
+> **Log volume:** the `access_denied_*` WARN lines are volume-bounded
+> by the default per-IP rate limiting plus zap's production sampling.
+> A stuck single-client storm is capped well before it reaches the
+> handler. If you run with `RATE_LIMIT_ENABLED=false` **and** the dev
+> (TTY) logger config (no sampling), an attacker- or bug-driven
+> rejection loop logs unbounded — keep rate limiting on in production.
+
 ## Why it happens
 
 The sealed `client_id` returned by `POST /register` has a
@@ -33,6 +50,19 @@ rejects with `invalid_client: client registration expired`.
 The lifetime is the `CLIENT_REGISTRATION_TTL` env var, default
 **7 days** (matches `refreshTokenTTL` so a client holding a
 still-valid refresh can always exchange it). Cap is 90 days.
+Setting it to **`0`** disables expiry entirely (never expires).
+
+## Known client-side root cause
+
+This is the server-side symptom of a widespread MCP client bug:
+the client caches its DCR `client_id` and **does not re-run DCR
+when it gets `invalid_client`**. The proxy is spec-correct — it
+advertises `client_id_expires_at` (an RFC 7591 extension field) and
+rejects a lapsed handle — but most clients ignore the field and never
+re-register. Notably **Azure APIM-backed connectors** loop on the
+400 indefinitely instead of reconnecting. Tracked across Claude
+Code, Cursor, LibreChat, et al. The MCP spec is itself moving away
+from DCR toward URL-based client IDs to retire this failure mode.
 
 ## Response
 
@@ -57,7 +87,16 @@ suggests they should:
    actual cause may be the token cutoff. Logs disambiguate.
 3. **Lengthen the TTL** for long-running deployments by setting
    `CLIENT_REGISTRATION_TTL=720h` (30d) or up to the 90d cap.
-4. **Consider Option 4** (auto-extend `client_id` on each
+4. **Disable expiry** with `CLIENT_REGISTRATION_TTL=0` when the
+   client provably never re-registers (e.g. Azure APIM connectors
+   that loop on the 400). This emits `client_id_expires_at=0` and
+   skips the expiry check on every validation path. It is the
+   correct fix when the alternative is a permanently-broken
+   connector — at the cost of the reuse-window bound below.
+   Reversible: unset the env var to restore the 7d default (only
+   newly-issued client_ids are affected, per the rolling-deploy
+   note).
+5. **Consider Option 4** (auto-extend `client_id` on each
    `/token` use) — see `misc/next-steps.md`. Not yet
    implemented as of this writing.
 
@@ -76,11 +115,20 @@ no matter what the env var says now. Plan accordingly:
 
 ## What NOT to do
 
-- **Don't disable `CLIENT_REGISTRATION_TTL` checks.** The TTL
-  bounds the residual reach of an exfiltrated `client_id`
+- **Don't reach for `CLIENT_REGISTRATION_TTL=0` by default.** The
+  TTL bounds the residual reach of an exfiltrated `client_id`
   (which is unauthenticated metadata sent in the clear on
-  `/authorize`). A 0 or near-infinite value silently extends
-  that window.
+  `/authorize`). `0` removes that bound — a leaked `client_id`
+  stays openable forever. It is a deliberate opt-in (see Response
+  step 4), justified only when a client provably never
+  re-registers; reserve it for that case rather than as a blanket
+  fix. Note a leaked `client_id` alone grants nothing: a full
+  OAuth flow with IdP consent is still required. Under `0` the only
+  per-`client_id` revocation lever is rotating `TOKEN_SIGNING_SECRET`,
+  which invalidates **every** sealed blob fleet-wide (all clients,
+  codes, and refresh tokens) — there is no per-client deny-list, so
+  losing the TTL backstop means a single bad client can only be
+  evicted with a fleet-wide rotation.
 - **Don't try to extend an existing `client_id` server-side.**
   The sealed payload is immutable. The only way to extend is
   re-issuing a fresh `client_id`.

handlers/authorize.go

@@ -103,28 +103,39 @@ func Authorize(tm *token.Manager, logger *zap.Logger, baseURL string, oauth2Cfg
 		// not receive an `error=` redirect — we'd be forwarding to whatever
 		// host an attacker chose. JSON 400 keeps these errors visible to
 		// the resource owner instead.
+		// Error shapes here differ from the /token path (invalid_client /
+		// invalid_request, not invalid_grant) per RFC 6749 §4.1.2.1, so
+		// the block stays inline rather than sharing openAndValidateClient;
+		// recordClientDenial keeps the metric+log observable (see its doc).
 		if clientIDStr == "" {
+			recordClientDenial(logger, "client_id_missing")
 			writeOAuthError(w, http.StatusBadRequest, "invalid_request", "client_id is required")
 			return
 		}
 
 		var client sealedClient
 		if err := tm.OpenJSON(clientIDStr, &client, token.PurposeClient); err != nil {
+			recordClientDenial(logger, "client_id_invalid", zap.Error(err))
 			writeOAuthError(w, http.StatusBadRequest, "invalid_client", "unknown client_id")
 			return
 		}
 
 		if client.Typ != token.PurposeClient {
+			recordClientDenial(logger, "client_typ_mismatch")
 			writeOAuthError(w, http.StatusBadRequest, "invalid_client", "unknown client_id")
 			return
 		}
 
 		if client.Audience != baseURL {
+			recordClientDenial(logger, "client_audience_mismatch", zap.String("internal_id", client.ID))
 			writeOAuthError(w, http.StatusBadRequest, "invalid_client", "client registered for a different audience")
 			return
 		}
 
-		if time.Now().After(client.ExpiresAt) {
+		// clientExpired treats a zero ExpiresAt as never-expiring
+		// (CLIENT_REGISTRATION_TTL=0); shared with the /token path.
+		if clientExpired(&client) {
+			recordClientDenial(logger, "client_registration_expired", zap.String("internal_id", client.ID), zap.Time("expired_at", client.ExpiresAt))
 			writeOAuthError(w, http.StatusBadRequest, "invalid_client", "client registration expired")
 			return
 		}