GeezerVison

Coder's MCP OAuth2 client never sent the RFC 8707 resource parameter, which the MCP authorization spec requires on authorization and token requests. Authorization servers that enforce it cannot be connected: with Dagster+, discovery and dynamic client registration succeed, then the authorize request fails with "MCP OAuth requests must include resource".

MCP server configs get an oauth2_resource field (migration 000618, admin-only in API responses, audited). Coder sends it on the authorize redirect, the authorization-code exchange, and token refresh. Refresh now goes through clientcredentials with grant_type=refresh_token, because oauth2.Config cannot add parameters to refresh requests; it keeps the same auth-style detection and error types, and adds no dependency.

Auto-discovery fills the field with the most specific Protected Resource Metadata resource that covers the server URL, stored as advertised. As in the MCP SDKs, a resource covers URLs on its origin at or under its path, so /mcp covers /mcp/ but /mcp/ does not cover /mcp. URLs compare in RFC 3986 normal form (default ports, percent-encodings, dot segments, and an empty path as /), ports ignore leading zeros, IPv6 literals compare by address, and %2F stays inside its segment. Unlike the SDKs, a resource with a query, even an empty one, only covers a server URL with the same query. When none covers it, Coder uses the server URL with the scheme and host lowercased and user information and fragment removed, or sends no resource when that is not a valid resource indicator. An explicit value in the create request wins.

Manual configurations send no resource unless an admin sets one, because some providers reject it (Microsoft Entra ID v2 fails with AADSTS901002). Values must be absolute RFC 3986 URIs without a fragment, user information, or an IPv6 zone identifier.

The resource binds grants like the other OAuth settings: changing it deletes stored user tokens, and a callback whose config resource changed after the user started connecting returns 409 before the code is redeemed. The connect step records the resource in a callback-scoped cookie for that check.

Discovery only runs when a configuration is created, so existing auto-discovered configurations, including the Dagster+ one that prompted this, keep an empty resource until an admin sets the new "Resource URL" field or re-creates the server.

Testing

• Go: discovery (most specific match, path and trailing-slash boundaries, query matching including empty queries, equivalent ports, IPv6 literals, escapes, and dot segments including escaped ones, the normalized fallback and an invalid one, explicit override), validation (URI characters, brackets, IP literals, user information, trimming), grant deletion on change, clearing on an auth type change, redaction for members, refresh with and without a resource, the 409 when the resource changes mid-flow (the authorization server never receives the code), and a connect and callback flow against an authorization server that rejects requests without the resource. Each new test was checked to fail when the behavior it covers is broken, except two malformed IP literal hosts that Go's URL parser already rejects.
• Vitest: the form round-trips the stored value, and clearing the field sends "" on update.
• Remote UAT through Coder Agents (chat) on a test deployment with loopback MCP and authorization server mocks:
  • Round 1 on 2b87506: literal verdict FAIL. When the resource changed between connect and callback, the callback redeemed the code for the new resource and stored a grant. The 409 binding fixes it.
  • Round 2 on 4b54ffb: PASS. The mid-flow change returned 409 with no token request in every variant (strict and lenient servers, set, cleared, abandoned flows, parallel configs); auto-discovery, connect, refresh, validation, and the docs matched. Minor observations: the binding cookie is an unkeyed digest, so a member who edits their own HttpOnly cookie can redeem a code with the current resource, which a fresh connect also gives them; other settings changed mid-flow still reach the token endpoint, as on main; the 409 popup shows raw JSON; the UI does not warn that a resource change disconnects users; validation toasts omit the server's detail.
  • Round 3 on the final head 2d6abfa: PASS. The head moved during the round, so it ran in four parts, the last on 2d6abfa. That part covered the matching rules (queries including empty ones, default and zero-padded ports, %2F, escaped dot segments, trailing slashes, IPv6 literals), the invalid fallback (stored empty, no resource sent), every validation value through the API and the UI, and the discovery, connect, refresh, and mid-flow 409 flows. The equivalent-encoding cases last ran live on 5c23fe0; unit tests cover them on the final head. Observation: the path-aware metadata URL is built from the decoded path, so /mcp%2Ftenant fetches metadata for /mcp/tenant, as on main.
• frontend-review and the eight-role coder-frontend-review ran on the site/ diff; every role passes after the label, help text, row layout, API doc wording, and form test fixes. Declined: linking field descriptions with aria-describedby, because the form's shared Field primitive renders every description without one, so it belongs in a change to all fields.

Review record

Xum opened this PR on behalf of @ibetitsmike.