Repository navigation
OAuth slice 3: dynamic client registration with rate limiting and unapproved-app expiry #797
Description
Activity
Correction: DCR is not what the MCP specification recommends.
This issue's rationale said dynamic client registration is "what the MCP spec recommends". An adversarial review checked the published specification and found that claim wrong.
The 2025-11-25 specification recommends Client ID Metadata Documents. It describes Dynamic Client Registration as optional, for backward compatibility or specific requirements.
Left uncorrected, the claim leads someone to build DCR-only support and expect the automatic interoperability the spec describes. A client that implements only the recommended metadata-document mechanism would not interoperate with a DCR-only server. The two are different mechanisms, and supporting one does not imply the other.
This does not cancel the slice. DCR remains a defensible choice, because clients in the field implement it and OpenIddict supports it well. But it is a deliberate compatibility decision rather than spec compliance, and the slice should record it as one.
This slice must now decide two things explicitly:
- Which protocol version it targets, and which clients it expects to connect.
- Whether it includes or defers Client ID Metadata Document support. If it defers that support, say so, so nobody is surprised when a spec-conformant client cannot connect.
The design document (
docs/plans/788-mcp-oauth/02-design.md) now labels DCR as a compatibility choice.Amendment, 2026-10-08: the expiry sweep also prunes old tokens and authorizations
The body and the 2026-09-13 correction stay as written. This adds one item to the scope.
#795 (PR #1135) ships OpenIddict with no cleanup of its tables. Redeemed and expired authorization codes, revoked tokens and revoked authorizations stay in the database indefinitely. Access tokens last until revoked (#788), so a live connection's rows are meant to stay; only dead rows are garbage. The rows grow once #798 turns the server on in Production.
In this slice:
- Build the unapproved-application expiry as a recurring sweep under
DurableJobWorker's leader gate, the same wayRefreshTokenPurgeSweepandIdempotencyRecordPurgeSweepwork. At most one instance runs it (Background worker has no single-runner guarantee — double-runs if scaled >1 instance #271). - In the same sweep, call OpenIddict's
PruneAsyncon the token and authorization managers, with a stated retention threshold. OpenIddict's default is 14 days. Pruning deletes only entries that are expired, revoked, redeemed or otherwise invalid, so connected apps are not touched. - Do not add OpenIddict's Quartz.NET integration. It would be a second scheduler, outside the leader gate.
- Tests: a live connection survives a sweep; a revoked token, a redeemed code and an expired unapproved application are removed after the threshold; the sweep runs only on the leader. Each test states its red mutation.
Done when, in addition to the body's criteria: dead OAuth rows older than the threshold disappear, and live connections remain.
- Build the unapproved-application expiry as a recurring sweep under
- added a commit that references this issue
on Oct 9, 2026
Slice 3 of #788. Depends on #795.
The MCP specification recommends that clients register themselves rather than being pre-configured. #788 chose automatic self-registration, which means an endpoint that any client on the internet can call.
Scope
IFixedWindowCounter(Shared-state ports: Redis implementations + in-process fallback #543/Distributed IP-keyed auth rate limiters with in-process fallback #544), not a process-local limiter.Why this is safe to leave open
Registering grants zero access. A registered client can do nothing until a user approves it on the consent screen (slice 4), and that approval requires re-entering a password. The real risk is junk rows, not unauthorized access, and the two controls above address it.
State that reasoning in the code, because "anyone on the internet can POST here" reads as alarming without it.
Worth deciding here
Decide whether a farm can opt out of assistant connections entirely. #788 considered an Owner-level switch and did not choose it, because each new required config key means updating two harnesses that CI does not run (the #370 sim harness and the #565 AppHost), and that update is easy to get wrong. If a switch is wanted, it belongs here, and it should be account data rather than configuration.
Done when
A client can register, the server rate-limits a client that hammers the endpoint, and an unapproved registration disappears after the expiry window.