feat: send RFC 8707 resource parameter on MCP server OAuth2 flows (#30651)

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.

![MCP server edit form showing the Resource URL field filled by
auto-discovery](https://github.com/user-attachments/assets/319bc8a0-f6e1-4b7d-8878-8e341c94e590)

## 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](https://dogfood.cdr.dev/agents/58695d5b-2112-400b-88f7-ab64c678f756))
on a test deployment with loopback MCP and authorization server mocks:
- Round 1 on 2b875068e0: 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 4b54ffb5f9: 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 2d6abfac49: PASS. The head moved during the
round, so it ran in four parts, the last on 2d6abfac49. 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 5c23fe00dca; 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

| Review | Commit | Findings | Disposition |
|---|---|---|---|
| [Codex round
1](https://github.com/coder/coder/pull/30651#pullrequestreview-5492085359)
| 4b54ffb5f9 | P2: a resource with another query was treated as
covering the server URL | Reproduced with a failing test, fixed in
bf0879560c, resolved |
| [Codex round
2](https://github.com/coder/coder/pull/30651#pullrequestreview-5492305363)
| bf0879560c | P2: explicit default ports; P2: `%2F` treated as a
segment boundary; P2: `url.Parse` accepts spaces and invalid escapes |
Each reproduced with a failing test, fixed in 16a1d783fa, resolved |
| [Codex round
3](https://github.com/coder/coder/pull/30651#pullrequestreview-5492677517)
| 16a1d783fa | P1: docs wrapped mid-sentence (DOC1); P2: equivalent
percent-encodings compared unequal; P2: brackets accepted outside an IP
literal host | Fixed in 8d617f0528 (the P2s reproduced with failing
tests; the same pass also covers the query, dot segment, and user
information cases), resolved |
| [Codex round
4](https://github.com/coder/coder/pull/30651#pullrequestreview-5493033885)
| 8d617f0528 | P2: the server URL fallback was not validated; P2:
escaped dot segments were compared before decoding | Each reproduced
with a failing test, fixed in 5c23fe00dc, resolved |
| [Codex round
5](https://github.com/coder/coder/pull/30651#pullrequestreview-5493342889)
| 5c23fe00dc | P2: an empty query (`?`) was treated as no query |
Reproduced with a failing test, fixed in 72c019ff5e, resolved |
| [Codex round
6](https://github.com/coder/coder/pull/30651#pullrequestreview-5493524846)
| 72c019ff5e | P2: `/mcp/` covered `/mcp`; P2: equivalent IPv6 literals
compared unequal; P2: malformed IP literals accepted | The first two
reproduced with failing tests. Go 1.26.5 `url.Parse` already rejects the
hosts named in the third, but it accepted an IPv6 zone identifier. All
fixed in ef2df2c4e7, resolved |
| [Codex round
7](https://github.com/coder/coder/pull/30651#pullrequestreview-5493668131)
| ef2df2c4e7 | P2: leading zeros in a port made equal origins differ |
Reproduced with a failing test, fixed in 2d6abfac49, resolved |
| [Codex round
8](https://github.com/coder/coder/pull/30651#issuecomment-6128743972) |
2d6abfac49 | None: "Didn't find any major issues" | Clean verdict on
the current head |

> Xum opened this PR on behalf of @ibetitsmike.
This commit is contained in:
Michael Suchacz
2026-10-13 23:03:08 +02:00
committed by GitHub
parent b7d63121b0
commit fa7cd62c8b
26 changed files with 1198 additions and 83 deletions
+11
View File
@@ -23788,6 +23788,10 @@ const docTemplate = `{
"oauth2_client_secret": {
"type": "string"
},
"oauth2_resource": {
"description": "OAuth2Resource is the RFC 8707 resource indicator sent on OAuth2\nrequests when set. Auto-discovery fills it if empty.",
"type": "string"
},
"oauth2_revocation_url": {
"description": "OAuth2RevocationURL is the provider's RFC 7009 revocation\nendpoint; auto-populated by OAuth2 discovery when omitted.",
"type": "string"
@@ -26483,6 +26487,9 @@ const docTemplate = `{
"description": "OAuth2 fields (only populated for admins).",
"type": "string"
},
"oauth2_resource": {
"type": "string"
},
"oauth2_revocation_url": {
"type": "string"
},
@@ -31979,6 +31986,10 @@ const docTemplate = `{
"oauth2_client_secret": {
"type": "string"
},
"oauth2_resource": {
"description": "OAuth2Resource is the RFC 8707 resource indicator. Omit it to keep\nthe stored value; send \"\" to clear it.",
"type": "string"
},
"oauth2_revocation_url": {
"description": "OAuth2RevocationURL is validated in the handler because a\nvalidate tag would reject the pointer to \"\" that clears it.",
"type": "string"
+11
View File
@@ -21606,6 +21606,10 @@
"oauth2_client_secret": {
"type": "string"
},
"oauth2_resource": {
"description": "OAuth2Resource is the RFC 8707 resource indicator sent on OAuth2\nrequests when set. Auto-discovery fills it if empty.",
"type": "string"
},
"oauth2_revocation_url": {
"description": "OAuth2RevocationURL is the provider's RFC 7009 revocation\nendpoint; auto-populated by OAuth2 discovery when omitted.",
"type": "string"
@@ -24192,6 +24196,9 @@
"description": "OAuth2 fields (only populated for admins).",
"type": "string"
},
"oauth2_resource": {
"type": "string"
},
"oauth2_revocation_url": {
"type": "string"
},
@@ -29464,6 +29471,10 @@
"oauth2_client_secret": {
"type": "string"
},
"oauth2_resource": {
"description": "OAuth2Resource is the RFC 8707 resource indicator. Omit it to keep\nthe stored value; send \"\" to clear it.",
"type": "string"
},
"oauth2_revocation_url": {
"description": "OAuth2RevocationURL is validated in the handler because a\nvalidate tag would reject the pointer to \"\" that clears it.",
"type": "string"
+1
View File
@@ -424,6 +424,7 @@ func MCPServerConfig(t testing.TB, db database.Store, seed database.MCPServerCon
OAuth2AuthURL: seed.OAuth2AuthURL,
OAuth2TokenURL: seed.OAuth2TokenURL,
OAuth2RevocationURL: seed.OAuth2RevocationURL,
OAuth2Resource: seed.OAuth2Resource,
OAuth2Scopes: seed.OAuth2Scopes,
APIKeyHeader: seed.APIKeyHeader,
APIKeyValue: seed.APIKeyValue,
+1
View File
@@ -2758,6 +2758,7 @@ CREATE TABLE mcp_server_configs (
user_acl jsonb DEFAULT '{}'::jsonb NOT NULL,
signing_secret text DEFAULT ''::text NOT NULL,
signing_secret_key_id text,
oauth2_resource text DEFAULT ''::text NOT NULL,
CONSTRAINT mcp_server_configs_auth_type_check CHECK ((auth_type = ANY (ARRAY['none'::text, 'oauth2'::text, 'api_key'::text, 'custom_headers'::text, 'user_oidc'::text]))),
CONSTRAINT mcp_server_configs_availability_check CHECK ((availability = ANY (ARRAY['force_on'::text, 'default_on'::text, 'default_off'::text]))),
CONSTRAINT mcp_server_configs_group_acl_is_object CHECK ((jsonb_typeof(group_acl) = 'object'::text)),
@@ -0,0 +1 @@
ALTER TABLE mcp_server_configs DROP COLUMN oauth2_resource;
@@ -0,0 +1 @@
ALTER TABLE mcp_server_configs ADD COLUMN oauth2_resource text NOT NULL DEFAULT '';
+1
View File
@@ -1339,6 +1339,7 @@ func (q *sqlQuerier) GetAuthorizedMCPServerConfigs(ctx context.Context, organiza
&i.UserACL,
&i.SigningSecret,
&i.SigningSecretKeyID,
&i.OAuth2Resource,
); err != nil {
return nil, err
}
+1
View File
@@ -5757,6 +5757,7 @@ type MCPServerConfig struct {
UserACL ChatACL `db:"user_acl" json:"user_acl"`
SigningSecret string `db:"signing_secret" json:"signing_secret"`
SigningSecretKeyID sql.NullString `db:"signing_secret_key_id" json:"signing_secret_key_id"`
OAuth2Resource string `db:"oauth2_resource" json:"oauth2_resource"`
}
type MCPServerUserToken struct {
+48 -32
View File
@@ -19495,7 +19495,7 @@ func (q *sqlQuerier) DeleteMCPServerUserTokensByConfigID(ctx context.Context, mc
const getEnabledMCPServerConfigsByOrganization = `-- name: GetEnabledMCPServerConfigsByOrganization :many
SELECT
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl, signing_secret, signing_secret_key_id
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl, signing_secret, signing_secret_key_id, oauth2_resource
FROM
mcp_server_configs
WHERE
@@ -19551,6 +19551,7 @@ func (q *sqlQuerier) GetEnabledMCPServerConfigsByOrganization(ctx context.Contex
&i.UserACL,
&i.SigningSecret,
&i.SigningSecretKeyID,
&i.OAuth2Resource,
); err != nil {
return nil, err
}
@@ -19567,7 +19568,7 @@ func (q *sqlQuerier) GetEnabledMCPServerConfigsByOrganization(ctx context.Contex
const getEnabledMCPServerConfigsByOrganizationAndIDs = `-- name: GetEnabledMCPServerConfigsByOrganizationAndIDs :many
SELECT
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl, signing_secret, signing_secret_key_id
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl, signing_secret, signing_secret_key_id, oauth2_resource
FROM
mcp_server_configs
WHERE
@@ -19629,6 +19630,7 @@ func (q *sqlQuerier) GetEnabledMCPServerConfigsByOrganizationAndIDs(ctx context.
&i.UserACL,
&i.SigningSecret,
&i.SigningSecretKeyID,
&i.OAuth2Resource,
); err != nil {
return nil, err
}
@@ -19645,7 +19647,7 @@ func (q *sqlQuerier) GetEnabledMCPServerConfigsByOrganizationAndIDs(ctx context.
const getForcedMCPServerConfigsByOrganization = `-- name: GetForcedMCPServerConfigsByOrganization :many
SELECT
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl, signing_secret, signing_secret_key_id
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl, signing_secret, signing_secret_key_id, oauth2_resource
FROM
mcp_server_configs
WHERE
@@ -19702,6 +19704,7 @@ func (q *sqlQuerier) GetForcedMCPServerConfigsByOrganization(ctx context.Context
&i.UserACL,
&i.SigningSecret,
&i.SigningSecretKeyID,
&i.OAuth2Resource,
); err != nil {
return nil, err
}
@@ -19718,7 +19721,7 @@ func (q *sqlQuerier) GetForcedMCPServerConfigsByOrganization(ctx context.Context
const getMCPServerConfigByID = `-- name: GetMCPServerConfigByID :one
SELECT
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl, signing_secret, signing_secret_key_id
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl, signing_secret, signing_secret_key_id, oauth2_resource
FROM
mcp_server_configs
WHERE
@@ -19765,13 +19768,14 @@ func (q *sqlQuerier) GetMCPServerConfigByID(ctx context.Context, id uuid.UUID) (
&i.UserACL,
&i.SigningSecret,
&i.SigningSecretKeyID,
&i.OAuth2Resource,
)
return i, err
}
const getMCPServerConfigByIDForUpdate = `-- name: GetMCPServerConfigByIDForUpdate :one
SELECT
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl, signing_secret, signing_secret_key_id
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl, signing_secret, signing_secret_key_id, oauth2_resource
FROM
mcp_server_configs
WHERE
@@ -19819,13 +19823,14 @@ func (q *sqlQuerier) GetMCPServerConfigByIDForUpdate(ctx context.Context, id uui
&i.UserACL,
&i.SigningSecret,
&i.SigningSecretKeyID,
&i.OAuth2Resource,
)
return i, err
}
const getMCPServerConfigByOrganizationAndSlug = `-- name: GetMCPServerConfigByOrganizationAndSlug :one
SELECT
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl, signing_secret, signing_secret_key_id
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl, signing_secret, signing_secret_key_id, oauth2_resource
FROM
mcp_server_configs
WHERE
@@ -19878,13 +19883,14 @@ func (q *sqlQuerier) GetMCPServerConfigByOrganizationAndSlug(ctx context.Context
&i.UserACL,
&i.SigningSecret,
&i.SigningSecretKeyID,
&i.OAuth2Resource,
)
return i, err
}
const getMCPServerConfigsByOrganization = `-- name: GetMCPServerConfigsByOrganization :many
SELECT
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl, signing_secret, signing_secret_key_id
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl, signing_secret, signing_secret_key_id, oauth2_resource
FROM
mcp_server_configs
WHERE
@@ -19941,6 +19947,7 @@ func (q *sqlQuerier) GetMCPServerConfigsByOrganization(ctx context.Context, orga
&i.UserACL,
&i.SigningSecret,
&i.SigningSecretKeyID,
&i.OAuth2Resource,
); err != nil {
return nil, err
}
@@ -20081,6 +20088,7 @@ INSERT INTO mcp_server_configs (
oauth2_auth_url,
oauth2_token_url,
oauth2_revocation_url,
oauth2_resource,
oauth2_scopes,
api_key_header,
api_key_value,
@@ -20124,20 +20132,21 @@ INSERT INTO mcp_server_configs (
$21::text,
$22::text,
$23::text,
$24::text[],
$24::text,
$25::text[],
$26::text,
$27::boolean,
$26::text[],
$27::text,
$28::boolean,
$29::boolean,
$30::boolean,
$31,
$31::boolean,
$32,
$33::uuid,
$34::uuid
$33,
$34::uuid,
$35::uuid
)
RETURNING
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl, signing_secret, signing_secret_key_id
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl, signing_secret, signing_secret_key_id, oauth2_resource
`
type InsertMCPServerConfigParams struct {
@@ -20156,6 +20165,7 @@ type InsertMCPServerConfigParams struct {
OAuth2AuthURL string `db:"oauth2_auth_url" json:"oauth2_auth_url"`
OAuth2TokenURL string `db:"oauth2_token_url" json:"oauth2_token_url"`
OAuth2RevocationURL string `db:"oauth2_revocation_url" json:"oauth2_revocation_url"`
OAuth2Resource string `db:"oauth2_resource" json:"oauth2_resource"`
OAuth2Scopes string `db:"oauth2_scopes" json:"oauth2_scopes"`
APIKeyHeader string `db:"api_key_header" json:"api_key_header"`
APIKeyValue string `db:"api_key_value" json:"api_key_value"`
@@ -20194,6 +20204,7 @@ func (q *sqlQuerier) InsertMCPServerConfig(ctx context.Context, arg InsertMCPSer
arg.OAuth2AuthURL,
arg.OAuth2TokenURL,
arg.OAuth2RevocationURL,
arg.OAuth2Resource,
arg.OAuth2Scopes,
arg.APIKeyHeader,
arg.APIKeyValue,
@@ -20252,6 +20263,7 @@ func (q *sqlQuerier) InsertMCPServerConfig(ctx context.Context, arg InsertMCPSer
&i.UserACL,
&i.SigningSecret,
&i.SigningSecretKeyID,
&i.OAuth2Resource,
)
return i, err
}
@@ -20321,27 +20333,28 @@ SET
oauth2_auth_url = $11::text,
oauth2_token_url = $12::text,
oauth2_revocation_url = $13::text,
oauth2_scopes = $14::text,
api_key_header = $15::text,
api_key_value = $16::text,
api_key_value_key_id = $17::text,
custom_headers = $18::text,
custom_headers_key_id = $19::text,
signing_secret = $20::text,
signing_secret_key_id = $21::text,
tool_allow_list = $22::text[],
tool_deny_list = $23::text[],
availability = $24::text,
enabled = $25::boolean,
model_intent = $26::boolean,
allow_in_plan_mode = $27::boolean,
forward_coder_headers = $28::boolean,
updated_by = $29::uuid,
oauth2_resource = $14::text,
oauth2_scopes = $15::text,
api_key_header = $16::text,
api_key_value = $17::text,
api_key_value_key_id = $18::text,
custom_headers = $19::text,
custom_headers_key_id = $20::text,
signing_secret = $21::text,
signing_secret_key_id = $22::text,
tool_allow_list = $23::text[],
tool_deny_list = $24::text[],
availability = $25::text,
enabled = $26::boolean,
model_intent = $27::boolean,
allow_in_plan_mode = $28::boolean,
forward_coder_headers = $29::boolean,
updated_by = $30::uuid,
updated_at = NOW()
WHERE
id = $30::uuid
id = $31::uuid
RETURNING
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl, signing_secret, signing_secret_key_id
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl, signing_secret, signing_secret_key_id, oauth2_resource
`
type UpdateMCPServerConfigParams struct {
@@ -20358,6 +20371,7 @@ type UpdateMCPServerConfigParams struct {
OAuth2AuthURL string `db:"oauth2_auth_url" json:"oauth2_auth_url"`
OAuth2TokenURL string `db:"oauth2_token_url" json:"oauth2_token_url"`
OAuth2RevocationURL string `db:"oauth2_revocation_url" json:"oauth2_revocation_url"`
OAuth2Resource string `db:"oauth2_resource" json:"oauth2_resource"`
OAuth2Scopes string `db:"oauth2_scopes" json:"oauth2_scopes"`
APIKeyHeader string `db:"api_key_header" json:"api_key_header"`
APIKeyValue string `db:"api_key_value" json:"api_key_value"`
@@ -20392,6 +20406,7 @@ func (q *sqlQuerier) UpdateMCPServerConfig(ctx context.Context, arg UpdateMCPSer
arg.OAuth2AuthURL,
arg.OAuth2TokenURL,
arg.OAuth2RevocationURL,
arg.OAuth2Resource,
arg.OAuth2Scopes,
arg.APIKeyHeader,
arg.APIKeyValue,
@@ -20448,6 +20463,7 @@ func (q *sqlQuerier) UpdateMCPServerConfig(ctx context.Context, arg UpdateMCPSer
&i.UserACL,
&i.SigningSecret,
&i.SigningSecretKeyID,
&i.OAuth2Resource,
)
return i, err
}
@@ -88,6 +88,7 @@ INSERT INTO mcp_server_configs (
oauth2_auth_url,
oauth2_token_url,
oauth2_revocation_url,
oauth2_resource,
oauth2_scopes,
api_key_header,
api_key_value,
@@ -123,6 +124,7 @@ INSERT INTO mcp_server_configs (
@oauth2_auth_url::text,
@oauth2_token_url::text,
@oauth2_revocation_url::text,
@oauth2_resource::text,
@oauth2_scopes::text,
@api_key_header::text,
@api_key_value::text,
@@ -163,6 +165,7 @@ SET
oauth2_auth_url = @oauth2_auth_url::text,
oauth2_token_url = @oauth2_token_url::text,
oauth2_revocation_url = @oauth2_revocation_url::text,
oauth2_resource = @oauth2_resource::text,
oauth2_scopes = @oauth2_scopes::text,
api_key_header = @api_key_header::text,
api_key_value = @api_key_value::text,
+1
View File
@@ -315,6 +315,7 @@ sql:
oauth2_auth_url: OAuth2AuthURL
oauth2_token_url: OAuth2TokenURL
oauth2_revocation_url: OAuth2RevocationURL
oauth2_resource: OAuth2Resource
oauth2_scopes: OAuth2Scopes
api_key_header: APIKeyHeader
api_key_value: APIKeyValue
+329 -9
View File
@@ -3,12 +3,15 @@ package coderd
import (
"bytes"
"context"
"crypto/sha256"
"database/sql"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
"net/netip"
"net/url"
"slices"
"strings"
@@ -330,6 +333,10 @@ func (api *API) createMCPServerConfig(rw http.ResponseWriter, r *http.Request) {
}
}
if !validateMCPOAuth2ResourceRequest(ctx, rw, req.OAuth2Resource) {
return
}
configID := uuid.New()
// Validate auth-type-dependent fields.
@@ -414,6 +421,9 @@ func (api *API) createMCPServerConfig(rw http.ResponseWriter, r *http.Request) {
req.OAuth2AuthURL = result.authURL
req.OAuth2TokenURL = result.tokenURL
req.OAuth2RevocationURL = oauth2RevocationURL
if strings.TrimSpace(req.OAuth2Resource) == "" {
req.OAuth2Resource = result.resource
}
req.OAuth2Scopes = oauth2Scopes
} else if req.OAuth2ClientID == "" || req.OAuth2AuthURL == "" || req.OAuth2TokenURL == "" {
// Partial manual config: all three fields are required together.
@@ -463,6 +473,7 @@ func (api *API) createMCPServerConfig(rw http.ResponseWriter, r *http.Request) {
OAuth2AuthURL: strings.TrimSpace(req.OAuth2AuthURL),
OAuth2TokenURL: strings.TrimSpace(req.OAuth2TokenURL),
OAuth2RevocationURL: strings.TrimSpace(req.OAuth2RevocationURL),
OAuth2Resource: strings.TrimSpace(req.OAuth2Resource),
OAuth2Scopes: strings.TrimSpace(req.OAuth2Scopes),
APIKeyHeader: strings.TrimSpace(req.APIKeyHeader),
APIKeyValue: strings.TrimSpace(req.APIKeyValue),
@@ -666,6 +677,10 @@ func (api *API) updateMCPServerConfig(rw http.ResponseWriter, r *http.Request) {
}
}
if req.OAuth2Resource != nil && !validateMCPOAuth2ResourceRequest(ctx, rw, *req.OAuth2Resource) {
return
}
// Pre-validate custom headers before entering the transaction.
var customHeadersJSON string
if req.CustomHeaders != nil {
@@ -762,6 +777,11 @@ func (api *API) updateMCPServerConfig(rw http.ResponseWriter, r *http.Request) {
oauth2RevocationURL = strings.TrimSpace(*req.OAuth2RevocationURL)
}
oauth2Resource := existing.OAuth2Resource
if req.OAuth2Resource != nil {
oauth2Resource = strings.TrimSpace(*req.OAuth2Resource)
}
oauth2Scopes := existing.OAuth2Scopes
if req.OAuth2Scopes != nil {
oauth2Scopes = strings.TrimSpace(*req.OAuth2Scopes)
@@ -841,6 +861,7 @@ func (api *API) updateMCPServerConfig(rw http.ResponseWriter, r *http.Request) {
oauth2AuthURL = ""
oauth2TokenURL = ""
oauth2RevocationURL = ""
oauth2Resource = ""
oauth2Scopes = ""
apiKeyHeader = ""
apiKeyValue = ""
@@ -860,6 +881,7 @@ func (api *API) updateMCPServerConfig(rw http.ResponseWriter, r *http.Request) {
oauth2AuthURL = ""
oauth2TokenURL = ""
oauth2RevocationURL = ""
oauth2Resource = ""
oauth2Scopes = ""
customHeaders = "{}"
customHeadersKeyID = sql.NullString{}
@@ -870,6 +892,7 @@ func (api *API) updateMCPServerConfig(rw http.ResponseWriter, r *http.Request) {
oauth2AuthURL = ""
oauth2TokenURL = ""
oauth2RevocationURL = ""
oauth2Resource = ""
oauth2Scopes = ""
apiKeyHeader = ""
apiKeyValue = ""
@@ -884,6 +907,7 @@ func (api *API) updateMCPServerConfig(rw http.ResponseWriter, r *http.Request) {
oauth2AuthURL = ""
oauth2TokenURL = ""
oauth2RevocationURL = ""
oauth2Resource = ""
oauth2Scopes = ""
apiKeyHeader = ""
apiKeyValue = ""
@@ -898,11 +922,12 @@ func (api *API) updateMCPServerConfig(rw http.ResponseWriter, r *http.Request) {
}
// User grants are bound to the destination, auth flow, token and revocation
// endpoints, and OAuth client. Invalidate them when any of these change so
// stored tokens cannot be sent to another endpoint or client.
// endpoints, OAuth client, and resource indicator. Invalidate them when any
// of these change so stored tokens cannot be sent to another endpoint,
// client, or resource.
if serverURL != existing.Url || authType != existing.AuthType ||
oauth2TokenURL != existing.OAuth2TokenURL || oauth2RevocationURL != existing.OAuth2RevocationURL ||
oauth2ClientID != existing.OAuth2ClientID {
oauth2ClientID != existing.OAuth2ClientID || oauth2Resource != existing.OAuth2Resource {
if err := tx.DeleteMCPServerUserTokensByConfigID(ctx, existing.ID); err != nil {
return xerrors.Errorf("invalidate MCP server user tokens: %w", err)
}
@@ -922,6 +947,7 @@ func (api *API) updateMCPServerConfig(rw http.ResponseWriter, r *http.Request) {
OAuth2AuthURL: oauth2AuthURL,
OAuth2TokenURL: oauth2TokenURL,
OAuth2RevocationURL: oauth2RevocationURL,
OAuth2Resource: oauth2Resource,
OAuth2Scopes: oauth2Scopes,
APIKeyHeader: apiKeyHeader,
APIKeyValue: apiKeyValue,
@@ -1114,6 +1140,16 @@ func (api *API) mcpServerOAuth2Connect(rw http.ResponseWriter, r *http.Request)
HttpOnly: true,
SameSite: http.SameSiteLaxMode,
}))
// Always overwrite the binding so a cookie left by an abandoned flow
// cannot fail this one.
http.SetCookie(rw, api.DeploymentValues.HTTPCookies.Apply(&http.Cookie{
Name: "mcp_oauth2_resource_" + config.ID.String(),
Value: mcpOAuth2ResourceBinding(config.OAuth2Resource),
Path: callbackPath,
MaxAge: 600, // 10 minutes
HttpOnly: true,
SameSite: http.SameSiteLaxMode,
}))
oauth2Config := &oauth2.Config{
ClientID: config.OAuth2ClientID,
@@ -1129,7 +1165,11 @@ func (api *API) mcpServerOAuth2Connect(rw http.ResponseWriter, r *http.Request)
scopes = strings.Split(config.OAuth2Scopes, " ")
}
oauth2Config.Scopes = scopes
authURL := oauth2Config.AuthCodeURL(state, oauth2.S256ChallengeOption(verifier))
authOpts := []oauth2.AuthCodeOption{oauth2.S256ChallengeOption(verifier)}
if config.OAuth2Resource != "" {
authOpts = append(authOpts, oauth2.SetAuthURLParam("resource", config.OAuth2Resource))
}
authURL := oauth2Config.AuthCodeURL(state, authOpts...)
http.Redirect(rw, r, authURL, http.StatusTemporaryRedirect)
}
@@ -1240,6 +1280,9 @@ func (api *API) mcpServerOAuth2Callback(rw http.ResponseWriter, r *http.Request)
if verifierCookie, err := r.Cookie("mcp_oauth2_verifier_" + config.ID.String()); err == nil {
exchangeOpts = append(exchangeOpts, oauth2.VerifierOption(verifierCookie.Value))
}
if config.OAuth2Resource != "" {
exchangeOpts = append(exchangeOpts, oauth2.SetAuthURLParam("resource", config.OAuth2Resource))
}
// Clear the verifier cookie regardless of whether it was present.
http.SetCookie(rw, api.DeploymentValues.HTTPCookies.Apply(&http.Cookie{
Name: "mcp_oauth2_verifier_" + config.ID.String(),
@@ -1250,6 +1293,27 @@ func (api *API) mcpServerOAuth2Callback(rw http.ResponseWriter, r *http.Request)
SameSite: http.SameSiteLaxMode,
}))
// RFC 8707 binds the code to the resource named in the authorization
// request. When an admin changed the resource since connect, fail before
// the exchange so the code is never redeemed for a resource the user did
// not authorize.
resourceBinding := ""
if resourceCookie, err := r.Cookie("mcp_oauth2_resource_" + config.ID.String()); err == nil {
resourceBinding = resourceCookie.Value
}
http.SetCookie(rw, api.DeploymentValues.HTTPCookies.Apply(&http.Cookie{
Name: "mcp_oauth2_resource_" + config.ID.String(),
Value: "",
Path: callbackPath,
MaxAge: -1,
HttpOnly: true,
SameSite: http.SameSiteLaxMode,
}))
if resourceBinding != mcpOAuth2ResourceBinding(config.OAuth2Resource) {
writeMCPConfigSupersededDuringAuth(ctx, rw)
return
}
// Exchange the authorization code for tokens.
oauth2Config := &oauth2.Config{
ClientID: config.OAuth2ClientID,
@@ -1302,7 +1366,7 @@ func (api *API) mcpServerOAuth2Callback(rw http.ResponseWriter, r *http.Request)
}
if current.Url != config.Url || current.AuthType != config.AuthType ||
current.OAuth2TokenURL != config.OAuth2TokenURL || current.OAuth2RevocationURL != config.OAuth2RevocationURL ||
current.OAuth2ClientID != config.OAuth2ClientID {
current.OAuth2ClientID != config.OAuth2ClientID || current.OAuth2Resource != config.OAuth2Resource {
return errMCPConfigSupersededDuringAuth
}
_, err = tx.UpsertMCPServerUserToken(ctx, database.UpsertMCPServerUserTokenParams{
@@ -1318,10 +1382,7 @@ func (api *API) mcpServerOAuth2Callback(rw http.ResponseWriter, r *http.Request)
return err
}, nil)
if errors.Is(err, errMCPConfigSupersededDuringAuth) {
httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{
Message: "MCP server configuration changed during authorization.",
Detail: "The server's destination or OAuth settings were updated while the connection was in progress. Reconnect to authorize against the current configuration.",
})
writeMCPConfigSupersededDuringAuth(ctx, rw)
return
}
if err != nil {
@@ -1346,6 +1407,13 @@ func (api *API) mcpServerOAuth2Callback(rw http.ResponseWriter, r *http.Request)
</script></body></html>`))
}
func writeMCPConfigSupersededDuringAuth(ctx context.Context, rw http.ResponseWriter) {
httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{
Message: "MCP server configuration changed during authorization.",
Detail: "The server's destination or OAuth settings were updated while the connection was in progress. Reconnect to authorize against the current configuration.",
})
}
// Removes the user's stored OAuth2 token for an MCP server.
// Provider revocation is best-effort and cannot block local deletion.
//
@@ -1629,6 +1697,7 @@ func convertMCPServerConfig(config database.MCPServerConfig) codersdk.MCPServerC
OAuth2AuthURL: config.OAuth2AuthURL,
OAuth2TokenURL: config.OAuth2TokenURL,
OAuth2RevocationURL: config.OAuth2RevocationURL,
OAuth2Resource: config.OAuth2Resource,
OAuth2Scopes: config.OAuth2Scopes,
APIKeyHeader: config.APIKeyHeader,
@@ -1666,6 +1735,7 @@ func convertMCPServerConfigRedacted(config database.MCPServerConfig) codersdk.MC
c.OAuth2AuthURL = ""
c.OAuth2TokenURL = ""
c.OAuth2RevocationURL = ""
c.OAuth2Resource = ""
c.OAuth2Scopes = ""
c.APIKeyHeader = ""
return c
@@ -1709,6 +1779,254 @@ func coalesceStringSlice(ss []string) []string {
return ss
}
// parseMCPOAuth2Resource parses an RFC 8707 resource indicator, which
// must be an absolute URI without a fragment (RFC 8707 section 2).
func parseMCPOAuth2Resource(resource string) (*url.URL, error) {
if strings.Contains(resource, "#") {
return nil, xerrors.New("oauth2_resource must not contain a fragment")
}
if !isURIText(resource) {
return nil, xerrors.New("oauth2_resource must be a valid URI")
}
parsed, err := url.Parse(resource)
if err != nil {
return nil, xerrors.New("oauth2_resource must be a valid URI")
}
// url.Parse validates an IP literal host, but it also accepts "[" and
// "]" elsewhere and a zone identifier in an IP literal, neither of
// which RFC 3986 allows.
ipLiteral := strings.HasPrefix(parsed.Host, "[")
if strings.ContainsAny(parsed.Opaque+parsed.EscapedPath()+parsed.RawQuery, "[]") ||
(!ipLiteral && strings.ContainsAny(parsed.Host, "[]")) ||
(ipLiteral && strings.Contains(parsed.Host, "%")) {
return nil, xerrors.New("oauth2_resource must be a valid URI")
}
if !parsed.IsAbs() {
return nil, xerrors.New("oauth2_resource must be an absolute URI")
}
// A resource indicator names a resource for the authorization server,
// so it must not carry credentials.
if parsed.User != nil {
return nil, xerrors.New("oauth2_resource must not contain user information")
}
return parsed, nil
}
// isURIText reports whether s contains only characters RFC 3986 allows
// in a URI, with every "%" starting a percent-encoded octet. url.Parse
// alone accepts spaces and other characters a URI cannot contain.
func isURIText(s string) bool {
for i := 0; i < len(s); i++ {
c := s[i]
switch {
case c == '%':
if i+2 >= len(s) || !isHexDigit(s[i+1]) || !isHexDigit(s[i+2]) {
return false
}
i += 2
case !isUnreserved(c) && strings.IndexByte(":/?#[]@!$&'()*+,;=", c) < 0:
return false
}
}
return true
}
// normalizePercentEncoding decodes percent-encoded unreserved characters
// and uppercases the hex digits of the remaining percent-encodings (RFC
// 3986 sections 6.2.2.1 and 6.2.2.2). Invalid escapes are left as they are.
func normalizePercentEncoding(s string) string {
out := make([]byte, 0, len(s))
for i := 0; i < len(s); i++ {
if s[i] == '%' && i+2 < len(s) && isHexDigit(s[i+1]) && isHexDigit(s[i+2]) {
if c := unhex(s[i+1])<<4 | unhex(s[i+2]); isUnreserved(c) {
out = append(out, c)
} else {
out = append(out, strings.ToUpper(s[i:i+3])...)
}
i += 2
continue
}
out = append(out, s[i])
}
return string(out)
}
func isUnreserved(c byte) bool {
return ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z') || ('0' <= c && c <= '9') ||
c == '-' || c == '.' || c == '_' || c == '~'
}
func isHexDigit(c byte) bool {
return ('0' <= c && c <= '9') || ('a' <= c && c <= 'f') || ('A' <= c && c <= 'F')
}
// unhex returns the value of a hex digit that isHexDigit accepted.
func unhex(c byte) byte {
switch {
case c >= 'a':
return c - 'a' + 10
case c >= 'A':
return c - 'A' + 10
default:
return c - '0'
}
}
// validateMCPOAuth2ResourceRequest writes a 400 response and returns
// false when a non-empty oauth2_resource is invalid. An empty value is
// valid because it means no resource parameter is sent.
func validateMCPOAuth2ResourceRequest(ctx context.Context, rw http.ResponseWriter, resource string) bool {
trimmed := strings.TrimSpace(resource)
if trimmed == "" {
return true
}
if _, err := parseMCPOAuth2Resource(trimmed); err != nil {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Invalid OAuth2 resource.",
Detail: err.Error(),
})
return false
}
return true
}
// mcpOAuth2ResourceBinding returns the callback cookie value that ties an
// authorization flow to the resource indicator it requested. A digest keeps
// the cookie short and cookie-safe for any URI. No resource maps to an empty
// value, which a missing cookie also matches.
func mcpOAuth2ResourceBinding(resource string) string {
if resource == "" {
return ""
}
sum := sha256.Sum256([]byte(resource))
return hex.EncodeToString(sum[:])
}
// mcpOAuth2ResourceIndicator picks the RFC 8707 resource indicator for
// an MCP server: the most specific advertised Protected Resource
// Metadata resource that covers the server URL, stored verbatim, else
// the server URL in canonical form, or "" when that is not a valid
// resource indicator. 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". Paths and queries compare in RFC 3986 normal form,
// where "%2F" does not end a segment, and IP addresses compare by value.
// Unlike the SDKs, a resource with a query, even an empty one, covers
// only URLs with the same query, which can select a tenant.
func mcpOAuth2ResourceIndicator(serverURL *url.URL, advertised []string) string {
serverPath := comparableURIPath(serverURL)
serverQuery, serverHasQuery := comparableURIQuery(serverURL)
best := ""
bestPathLen := -1
bestHasQuery := false
for _, raw := range advertised {
candidate := strings.TrimSpace(raw)
parsed, err := parseMCPOAuth2Resource(candidate)
if err != nil {
continue
}
if !sameOrigin(parsed, serverURL) {
continue
}
query, hasQuery := comparableURIQuery(parsed)
if hasQuery && (!serverHasQuery || query != serverQuery) {
continue
}
entryPath := comparableURIPath(parsed)
if entryPath != serverPath && !strings.HasPrefix(serverPath, withTrailingSlash(entryPath)) {
continue
}
if len(entryPath) > bestPathLen || (len(entryPath) == bestPathLen && hasQuery && !bestHasQuery) {
best = candidate
bestPathLen = len(entryPath)
bestHasQuery = hasQuery
}
}
if best != "" {
return best
}
canonical := *serverURL
canonical.Scheme = strings.ToLower(canonical.Scheme)
canonical.Host = strings.ToLower(canonical.Host)
canonical.User = nil
canonical.Fragment = ""
canonical.RawFragment = ""
fallback := canonical.String()
// url.Parse accepts server URLs that are not valid resource
// indicators, and sending none beats sending an invalid one.
if _, err := parseMCPOAuth2Resource(fallback); err != nil {
return ""
}
return fallback
}
// comparableURIPath returns u's escaped path with percent-encodings
// normalized and then dot segments removed, so "%2E%2E" is a dot segment.
// An empty path after an authority is "/" (RFC 3986 section 6.2.3).
func comparableURIPath(u *url.URL) string {
normalized := *u
// Normalizing escapes leaves the unescaped Path unchanged, so this
// RawPath stays a valid encoding of it.
normalized.RawPath = normalizePercentEncoding(u.EscapedPath())
// Resolving a URL against itself removes dot segments and keeps escapes.
path := normalized.ResolveReference(&normalized).EscapedPath()
if path == "" && u.Host != "" {
return "/"
}
return path
}
// comparableURIQuery returns u's query with percent-encodings normalized
// and whether u has a query. A trailing "?" is an empty query, which RFC
// 3986 distinguishes from no query.
func comparableURIQuery(u *url.URL) (string, bool) {
return normalizePercentEncoding(u.RawQuery), u.RawQuery != "" || u.ForceQuery
}
// sameOrigin reports whether two URLs have the same scheme, host, and
// port, where an omitted port is the scheme's default.
func sameOrigin(a, b *url.URL) bool {
return strings.EqualFold(a.Scheme, b.Scheme) &&
sameHost(a.Hostname(), b.Hostname()) &&
portOrDefault(a) == portOrDefault(b)
}
// sameHost compares IP addresses by value, so "::1" equals
// "0:0:0:0:0:0:0:1", and other hosts case-insensitively.
func sameHost(a, b string) bool {
addrA, errA := netip.ParseAddr(a)
addrB, errB := netip.ParseAddr(b)
if errA == nil && errB == nil {
return addrA == addrB
}
return strings.EqualFold(a, b)
}
func portOrDefault(u *url.URL) string {
if port := u.Port(); port != "" {
// An origin's port has no leading zeros (RFC 9110 section 4.3.1).
if trimmed := strings.TrimLeft(port, "0"); trimmed != "" {
return trimmed
}
return "0"
}
switch strings.ToLower(u.Scheme) {
case "http":
return "80"
case "https":
return "443"
default:
return ""
}
}
func withTrailingSlash(path string) string {
if strings.HasSuffix(path, "/") {
return path
}
return path + "/"
}
// mcpOAuth2Discovery holds the result of MCP OAuth2 auto-discovery
// and Dynamic Client Registration.
type mcpOAuth2Discovery struct {
@@ -1717,6 +2035,7 @@ type mcpOAuth2Discovery struct {
authURL string
tokenURL string
revocationURL string
resource string // RFC 8707 resource indicator
scopes string // space-separated
}
@@ -2081,6 +2400,7 @@ func discoverAndRegisterMCPOAuth2(ctx context.Context, httpClient *http.Client,
authURL: asMeta.AuthorizationEndpoint,
tokenURL: asMeta.TokenEndpoint,
revocationURL: asMeta.RevocationEndpoint,
resource: mcpOAuth2ResourceIndicator(parsed, prm.Resource),
scopes: scopes,
}, nil
}
+199
View File
@@ -3,6 +3,7 @@ package coderd
import (
"context"
"encoding/json"
"net/url"
"sync/atomic"
"testing"
"time"
@@ -267,3 +268,201 @@ func TestProtectedResourceMetadataResourceUnmarshal(t *testing.T) {
})
}
}
func TestMCPOAuth2ResourceIndicator(t *testing.T) {
t.Parallel()
cases := []struct {
name string
serverURL string
advertised []string
want string
}{
{
name: "ExactMatch",
serverURL: "https://mcp.agent.dagster.cloud/mcp",
advertised: []string{"https://mcp.agent.dagster.cloud/mcp"},
want: "https://mcp.agent.dagster.cloud/mcp",
},
{
name: "OriginCoversPath",
serverURL: "https://mcp.example.com/v1/mcp",
advertised: []string{"https://mcp.example.com"},
want: "https://mcp.example.com",
},
{
name: "CaseInsensitiveOriginKeptVerbatim",
serverURL: "https://mcp.example.com/mcp",
advertised: []string{"HTTPS://MCP.Example.com/mcp"},
want: "HTTPS://MCP.Example.com/mcp",
},
{
name: "PathCoversSubpath",
serverURL: "https://mcp.example.com/mcp/x",
advertised: []string{"https://mcp.example.com/mcp"},
want: "https://mcp.example.com/mcp",
},
{
name: "TrailingSlashResourceDoesNotCoverBarePath",
serverURL: "https://mcp.example.com/mcp",
advertised: []string{"https://mcp.example.com/mcp/"},
want: "https://mcp.example.com/mcp",
},
{
name: "EmptyPathEqualsSlash",
serverURL: "https://mcp.example.com",
advertised: []string{"https://mcp.example.com/"},
want: "https://mcp.example.com/",
},
{
name: "PathPrefixIsNotSegment",
serverURL: "https://mcp.example.com/mcp2",
advertised: []string{"https://mcp.example.com/mcp"},
want: "https://mcp.example.com/mcp2",
},
{
name: "OtherOriginFallsBack",
serverURL: "https://mcp.example.com/mcp",
advertised: []string{"https://other.example.com/mcp", "http://mcp.example.com/mcp", "https://mcp.example.com:8443/mcp"},
want: "https://mcp.example.com/mcp",
},
{
name: "MostSpecificWins",
serverURL: "https://mcp.example.com/a/b/c",
advertised: []string{
"https://mcp.example.com/",
"https://mcp.example.com/a/b",
"https://mcp.example.com/a",
},
want: "https://mcp.example.com/a/b",
},
{
name: "QueryMustMatch",
serverURL: "https://mcp.example.com/mcp?tenant=b",
advertised: []string{
"https://mcp.example.com/mcp?tenant=a",
"https://mcp.example.com/mcp?tenant=b",
},
want: "https://mcp.example.com/mcp?tenant=b",
},
{
name: "MatchingQueryBeatsNoQuery",
serverURL: "https://mcp.example.com/mcp?tenant=b",
advertised: []string{"https://mcp.example.com/mcp", "https://mcp.example.com/mcp?tenant=b"},
want: "https://mcp.example.com/mcp?tenant=b",
},
{
name: "NoQueryCoversQuery",
serverURL: "https://mcp.example.com/mcp?tenant=a",
advertised: []string{"https://mcp.example.com/mcp"},
want: "https://mcp.example.com/mcp",
},
{
name: "EmptyQueryIsNotOtherQuery",
serverURL: "https://mcp.example.com/mcp?tenant=b",
advertised: []string{"https://mcp.example.com/mcp?"},
want: "https://mcp.example.com/mcp?tenant=b",
},
{
name: "EmptyQueryIsNotNoQuery",
serverURL: "https://mcp.example.com/mcp",
advertised: []string{"https://mcp.example.com/mcp?"},
want: "https://mcp.example.com/mcp",
},
{
name: "ExplicitDefaultPortOnServer",
serverURL: "https://mcp.example.com:443/mcp",
advertised: []string{"https://mcp.example.com/mcp"},
want: "https://mcp.example.com/mcp",
},
{
name: "ExplicitDefaultPortOnResource",
serverURL: "http://mcp.example.com/mcp",
advertised: []string{"http://mcp.example.com:80/mcp"},
want: "http://mcp.example.com:80/mcp",
},
{
name: "PortLeadingZerosAreIgnored",
serverURL: "https://mcp.example.com:0443/mcp",
advertised: []string{"https://mcp.example.com/mcp"},
want: "https://mcp.example.com/mcp",
},
{
name: "EscapedSlashIsNotSegmentBoundary",
serverURL: "https://mcp.example.com/mcp%2Ftenant",
advertised: []string{"https://mcp.example.com/mcp"},
want: "https://mcp.example.com/mcp%2Ftenant",
},
{
name: "EscapeHexCaseIsEquivalent",
serverURL: "https://mcp.example.com/mcp%2ftenant",
advertised: []string{"https://mcp.example.com/mcp%2Ftenant"},
want: "https://mcp.example.com/mcp%2Ftenant",
},
{
name: "EscapedUnreservedIsEquivalent",
serverURL: "https://mcp.example.com/mcp",
advertised: []string{"https://mcp.example.com/%6Dcp"},
want: "https://mcp.example.com/%6Dcp",
},
{
name: "EscapedQueryIsEquivalent",
serverURL: "https://mcp.example.com/mcp?tenant=%61",
advertised: []string{"https://mcp.example.com/mcp?tenant=a"},
want: "https://mcp.example.com/mcp?tenant=a",
},
{
name: "DotSegmentsAreRemoved",
serverURL: "https://mcp.example.com/a/../mcp",
advertised: []string{"https://mcp.example.com/mcp"},
want: "https://mcp.example.com/mcp",
},
{
name: "EscapedDotSegmentsAreRemoved",
serverURL: "https://mcp.example.com/mcp",
advertised: []string{"https://mcp.example.com/a/%2E%2E/mcp"},
want: "https://mcp.example.com/a/%2E%2E/mcp",
},
{
name: "InvalidFallbackIsEmpty",
serverURL: "https://mcp.example.com/a[b]",
want: "",
},
{
name: "IPLiteralHost",
serverURL: "https://[::1]:8443/mcp",
advertised: []string{"https://[::1]:8443/"},
want: "https://[::1]:8443/",
},
{
name: "EquivalentIPv6Literal",
serverURL: "https://[0:0:0:0:0:0:0:1]/mcp",
advertised: []string{"https://[::1]/mcp"},
want: "https://[::1]/mcp",
},
{
name: "NoEntriesFallsBack",
serverURL: "https://mcp.example.com/mcp",
want: "https://mcp.example.com/mcp",
},
{
name: "FallbackIsCanonical",
serverURL: "HTTPS://user:pass@MCP.Example.COM:8443/Path/MCP?tenant=Acme#frag",
want: "https://mcp.example.com:8443/Path/MCP?tenant=Acme",
},
{
name: "InvalidEntriesIgnored",
serverURL: "https://mcp.example.com/mcp",
advertised: []string{"https://mcp.example.com/#frag", "/mcp", "", " "},
want: "https://mcp.example.com/mcp",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
serverURL, err := url.Parse(tc.serverURL)
require.NoError(t, err)
require.Equal(t, tc.want, mcpOAuth2ResourceIndicator(serverURL, tc.advertised))
})
}
}
+312 -1
View File
@@ -1122,6 +1122,7 @@ func TestMCPServerConfigsSecretsNeverLeaked(t *testing.T) {
OAuth2AuthURL: "https://auth.example.com/authorize",
OAuth2TokenURL: "https://auth.example.com/token",
OAuth2RevocationURL: "https://auth.example.com/revoke",
OAuth2Resource: "https://mcp.example.com/secrets",
OAuth2Scopes: "read write",
APIKeyHeader: "X-Api-Key",
APIKeyValue: "THIS-IS-A-SECRET-API-KEY",
@@ -1156,6 +1157,7 @@ func TestMCPServerConfigsSecretsNeverLeaked(t *testing.T) {
// Verify boolean indicators are set correctly.
require.True(t, created.HasOAuth2Secret, "HasOAuth2Secret should be true")
require.Equal(t, "https://mcp.example.com/secrets", created.OAuth2Resource)
require.True(t, created.HasAPIKey, "HasAPIKey should be true")
require.True(t, created.HasCustomHeaders, "HasCustomHeaders should be true")
@@ -1183,6 +1185,7 @@ func TestMCPServerConfigsSecretsNeverLeaked(t *testing.T) {
assert.Empty(t, cfg.OAuth2AuthURL, "member should not see OAuth2AuthURL")
assert.Empty(t, cfg.OAuth2TokenURL, "member should not see OAuth2TokenURL")
assert.Empty(t, cfg.OAuth2RevocationURL, "member should not see OAuth2RevocationURL")
assert.Empty(t, cfg.OAuth2Resource, "member should not see OAuth2Resource")
assert.Empty(t, cfg.APIKeyHeader, "member should not see APIKeyHeader")
assert.Empty(t, cfg.OAuth2Scopes, "member should not see OAuth2Scopes")
assert.Empty(t, cfg.URL, "member should not see URL")
@@ -1197,6 +1200,7 @@ func TestMCPServerConfigsSecretsNeverLeaked(t *testing.T) {
assert.Empty(t, memberSingle.OAuth2AuthURL, "member should not see OAuth2AuthURL")
assert.Empty(t, memberSingle.OAuth2TokenURL, "member should not see OAuth2TokenURL")
assert.Empty(t, memberSingle.OAuth2Scopes, "member should not see OAuth2Scopes")
assert.Empty(t, memberSingle.OAuth2Resource, "member should not see OAuth2Resource")
assert.Empty(t, memberSingle.APIKeyHeader, "member should not see APIKeyHeader")
assert.Empty(t, memberSingle.URL, "member should not see URL")
assert.Empty(t, memberSingle.Transport, "member should not see Transport")
@@ -1289,6 +1293,7 @@ func TestMCPServerConfigsUserOIDCClearsFields(t *testing.T) {
OAuth2AuthURL: "https://auth.example.com/authorize",
OAuth2TokenURL: "https://auth.example.com/token",
OAuth2RevocationURL: "https://auth.example.com/revoke",
OAuth2Resource: "https://mcp.example.com/v1",
OAuth2Scopes: "read write",
Availability: "default_off",
Enabled: true,
@@ -1356,6 +1361,67 @@ func TestMCPServerConfigsUserOIDCClearsFields(t *testing.T) {
require.NoError(t, err)
require.Equal(t, newRevocationURL, updated.OAuth2RevocationURL)
// RFC 8707 resource indicators must be valid absolute URIs without a
// fragment, on both create and update. Create rejects them before
// auto-discovery so an invalid value causes no client registration.
var discoveryRequests atomic.Int64
discoveryProbe := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
discoveryRequests.Add(1)
http.NotFound(w, r)
}))
t.Cleanup(discoveryProbe.Close)
for _, invalidResource := range []string{
"https://mcp.example.com/v1#frag",
"/v1/mcp",
"https://mcp.example.com/a b",
"https://mcp.example.com/mcp?tenant=%zz",
"https://mcp.example.com/a[b]",
"https://a]b.example.com/mcp",
"https://[x]/mcp",
"https://[::1][x]/mcp",
"https://[fe80::1%25en0]/mcp",
"https://user@mcp.example.com/mcp",
} {
_, err = client.UpdateMCPServerConfig(ctx, created.OrganizationID, created.ID, codersdk.UpdateMCPServerConfigRequest{
OAuth2Resource: &invalidResource,
})
require.ErrorAs(t, err, &sdkErr, invalidResource)
require.Equal(t, http.StatusBadRequest, sdkErr.StatusCode(), invalidResource)
require.Equal(t, "Invalid OAuth2 resource.", sdkErr.Message, invalidResource)
_, err = client.CreateMCPServerConfig(ctx, firstUser.OrganizationID, codersdk.CreateMCPServerConfigRequest{
DisplayName: "Invalid Resource",
Slug: "invalid-resource",
Transport: "streamable_http",
URL: discoveryProbe.URL + "/mcp",
AuthType: "oauth2",
OAuth2Resource: invalidResource,
Availability: "default_on",
Enabled: true,
ToolAllowList: []string{},
ToolDenyList: []string{},
})
require.ErrorAs(t, err, &sdkErr, invalidResource)
require.Equal(t, http.StatusBadRequest, sdkErr.StatusCode(), invalidResource)
require.Equal(t, "Invalid OAuth2 resource.", sdkErr.Message, invalidResource)
}
require.Zero(t, discoveryRequests.Load(), "invalid resource must be rejected before discovery")
emptyResource := ""
updated, err = client.UpdateMCPServerConfig(ctx, created.OrganizationID, created.ID, codersdk.UpdateMCPServerConfigRequest{
OAuth2Resource: &emptyResource,
})
require.NoError(t, err)
require.Empty(t, updated.OAuth2Resource)
newResource := "https://mcp.example.com/v2"
paddedResource := " " + newResource + " "
updated, err = client.UpdateMCPServerConfig(ctx, created.OrganizationID, created.ID, codersdk.UpdateMCPServerConfigRequest{
OAuth2Resource: &paddedResource,
})
require.NoError(t, err)
require.Equal(t, newResource, updated.OAuth2Resource)
newAuth := "user_oidc"
updated, err = client.UpdateMCPServerConfig(ctx, created.OrganizationID, created.ID, codersdk.UpdateMCPServerConfigRequest{
AuthType: &newAuth,
@@ -1369,10 +1435,65 @@ func TestMCPServerConfigsUserOIDCClearsFields(t *testing.T) {
require.Empty(t, updated.OAuth2AuthURL)
require.Empty(t, updated.OAuth2TokenURL)
require.Empty(t, updated.OAuth2RevocationURL)
require.Empty(t, updated.OAuth2Resource)
require.Empty(t, updated.OAuth2Scopes)
require.Empty(t, updated.APIKeyHeader)
}
// TestMCPServerConfigsAuthTypeChangeClearsOAuth2Resource covers the
// non-OIDC auth types; TestMCPServerConfigsUserOIDCClearsFields covers
// user_oidc.
func TestMCPServerConfigsAuthTypeChangeClearsOAuth2Resource(t *testing.T) {
t.Parallel()
client := newMCPClient(t)
firstUser := coderdtest.CreateFirstUser(t, client)
header := "X-Api-Key"
value := "api-key-value"
customHeaders := map[string]string{"X-Custom": "custom-value"}
cases := []struct {
authType string
req codersdk.UpdateMCPServerConfigRequest
}{
{authType: "none"},
{authType: "api_key", req: codersdk.UpdateMCPServerConfigRequest{APIKeyHeader: &header, APIKeyValue: &value}},
{authType: "custom_headers", req: codersdk.UpdateMCPServerConfigRequest{CustomHeaders: &customHeaders}},
}
for _, tc := range cases {
t.Run(tc.authType, func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
slug := "clear-resource-" + strings.ReplaceAll(tc.authType, "_", "-")
created, err := client.CreateMCPServerConfig(ctx, firstUser.OrganizationID, codersdk.CreateMCPServerConfigRequest{
DisplayName: "Clear Resource " + tc.authType,
Slug: slug,
Transport: "streamable_http",
URL: "https://mcp.example.com/" + slug,
AuthType: "oauth2",
OAuth2ClientID: "cid",
OAuth2AuthURL: "https://auth.example.com/authorize",
OAuth2TokenURL: "https://auth.example.com/token",
OAuth2Resource: "https://mcp.example.com/" + slug,
Availability: "default_off",
Enabled: true,
ToolAllowList: []string{},
ToolDenyList: []string{},
})
require.NoError(t, err)
require.NotEmpty(t, created.OAuth2Resource)
req := tc.req
req.AuthType = &tc.authType
updated, err := client.UpdateMCPServerConfig(ctx, created.OrganizationID, created.ID, req)
require.NoError(t, err)
require.Equal(t, tc.authType, updated.AuthType)
require.Empty(t, updated.OAuth2Resource)
})
}
}
func TestMCPServerConfigsAPIKeyRequiresHeaderAndValue(t *testing.T) {
t.Parallel()
@@ -1609,6 +1730,20 @@ func TestMCPServerConfigsUpdateInvalidatesUserGrants(t *testing.T) {
require.False(t, tokenExists(ctx, t, config.ID))
})
t.Run("ResourceChangeDeletesGrants", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
config := newConfig(ctx, t, "grant-resource-change")
seedToken(ctx, t, config.ID)
resource := "https://mcp.example.com/grant-resource-change"
_, err := adminClient.UpdateMCPServerConfig(ctx, config.OrganizationID, config.ID, codersdk.UpdateMCPServerConfigRequest{
OAuth2Resource: &resource,
})
require.NoError(t, err)
require.False(t, tokenExists(ctx, t, config.ID))
})
t.Run("UnrelatedChangeKeepsGrants", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
@@ -1734,6 +1869,15 @@ func TestMCPServerConfigsOAuth2CallbackRejectsSupersededRevocationURL(t *testing
})
}
func TestMCPServerConfigsOAuth2CallbackRejectsSupersededResource(t *testing.T) {
t.Parallel()
movedResource := "https://attacker.example.com/mcp"
runMCPServerConfigsOAuth2CallbackSupersessionTest(t, "superseded-resource-callback", codersdk.UpdateMCPServerConfigRequest{
OAuth2Resource: &movedResource,
})
}
func runMCPServerConfigsOAuth2CallbackSupersessionTest(
t *testing.T,
slug string,
@@ -2447,8 +2591,10 @@ func TestMCPServerConfigsOAuth2AutoDiscovery(t *testing.T) {
require.Equal(t, authServer.URL+"/token", created.OAuth2TokenURL)
require.Equal(t, authServer.URL+"/revoke", created.OAuth2RevocationURL)
require.Equal(t, "read write", created.OAuth2Scopes)
// The origin-only PRM resource covers the /v1/mcp server URL.
require.Equal(t, mcpServer.URL, created.OAuth2Resource)
// An explicit revocation URL wins over the discovered one.
// Explicit values win over the discovered ones.
overridden, err := client.CreateMCPServerConfig(ctx, firstUser.OrganizationID, codersdk.CreateMCPServerConfigRequest{
DisplayName: "Auto-Discovery Override",
Slug: "auto-discovery-override",
@@ -2456,6 +2602,7 @@ func TestMCPServerConfigsOAuth2AutoDiscovery(t *testing.T) {
URL: mcpServer.URL + "/v1/mcp",
AuthType: "oauth2",
OAuth2RevocationURL: "https://override.example.com/revoke",
OAuth2Resource: "https://override.example.com/mcp",
Availability: "default_on",
Enabled: true,
ToolAllowList: []string{},
@@ -2463,6 +2610,7 @@ func TestMCPServerConfigsOAuth2AutoDiscovery(t *testing.T) {
})
require.NoError(t, err)
require.Equal(t, "https://override.example.com/revoke", overridden.OAuth2RevocationURL)
require.Equal(t, "https://override.example.com/mcp", overridden.OAuth2Resource)
})
t.Run("CreateOnlyScopeAllowed", func(t *testing.T) {
@@ -3100,6 +3248,169 @@ func TestMCPServerConfigsOAuth2AutoDiscoverySSRF(t *testing.T) {
}
}
// TestMCPServerOAuth2ResourceIndicator runs the connect and callback flow
// against an authorization server that, like Dagster+, rejects requests
// without the expected RFC 8707 resource parameter.
func TestMCPServerOAuth2ResourceIndicator(t *testing.T) {
t.Parallel()
const resource = "https://mcp.example.com/mcp"
invalidTarget := func(w http.ResponseWriter) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusBadRequest)
_, _ = w.Write([]byte(`{"error":"invalid_target","error_description":"MCP OAuth requests must include resource"}`))
}
var redeemedCodes sync.Map
authServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/authorize":
if r.URL.Query().Get("resource") != resource {
invalidTarget(w)
return
}
redirect, err := url.Parse(r.URL.Query().Get("redirect_uri"))
if !assert.NoError(t, err) {
http.Error(w, "bad redirect_uri", http.StatusBadRequest)
return
}
q := redirect.Query()
q.Set("code", "code-"+r.URL.Query().Get("state"))
q.Set("state", r.URL.Query().Get("state"))
redirect.RawQuery = q.Encode()
http.Redirect(w, r, redirect.String(), http.StatusFound)
case "/token":
if err := r.ParseForm(); err != nil {
invalidTarget(w)
return
}
redeemedCodes.Store(r.PostForm.Get("code"), struct{}{})
if r.PostForm.Get("resource") != resource {
invalidTarget(w)
return
}
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`{"access_token":"resource-access-token","token_type":"Bearer","expires_in":3600,"refresh_token":"resource-refresh-token"}`))
default:
http.NotFound(w, r)
}
}))
t.Cleanup(authServer.Close)
adminClient := newMCPClient(t)
firstUser := coderdtest.CreateFirstUser(t, adminClient)
memberClient, _ := coderdtest.CreateAnotherUser(t, adminClient, firstUser.OrganizationID)
memberClient.HTTPClient.CheckRedirect = func(_ *http.Request, _ []*http.Request) error {
return http.ErrUseLastResponse
}
createConfig := func(ctx context.Context, t *testing.T, slug, oauth2Resource string) codersdk.MCPServerConfig {
t.Helper()
created, err := adminClient.CreateMCPServerConfig(ctx, firstUser.OrganizationID, codersdk.CreateMCPServerConfigRequest{
DisplayName: "Resource " + slug,
Slug: slug,
Transport: "streamable_http",
URL: "https://mcp.example.com/mcp",
AuthType: "oauth2",
OAuth2ClientID: "resource-client",
OAuth2AuthURL: authServer.URL + "/authorize",
OAuth2TokenURL: authServer.URL + "/token",
OAuth2Resource: oauth2Resource,
Availability: "default_on",
Enabled: true,
ToolAllowList: []string{},
ToolDenyList: []string{},
})
require.NoError(t, err)
return created
}
connect := func(ctx context.Context, t *testing.T, config codersdk.MCPServerConfig) (*url.URL, []*http.Cookie) {
t.Helper()
req, err := http.NewRequestWithContext(ctx, http.MethodGet, memberClient.MCPServerOAuth2ConnectURL(config.OrganizationID, config.ID), nil)
require.NoError(t, err)
req.AddCookie(&http.Cookie{Name: codersdk.SessionTokenCookie, Value: memberClient.SessionToken()})
res, err := memberClient.HTTPClient.Do(req)
require.NoError(t, err)
defer res.Body.Close()
require.Equal(t, http.StatusTemporaryRedirect, res.StatusCode)
location, err := res.Location()
require.NoError(t, err)
return location, res.Cookies()
}
authorize := func(ctx context.Context, t *testing.T, authorizeURL *url.URL) *url.URL {
t.Helper()
req, err := http.NewRequestWithContext(ctx, http.MethodGet, authorizeURL.String(), nil)
require.NoError(t, err)
res, err := memberClient.HTTPClient.Do(req)
require.NoError(t, err)
defer res.Body.Close()
require.Equal(t, http.StatusFound, res.StatusCode, "strict authorization server rejected the authorize request")
callbackURL, err := res.Location()
require.NoError(t, err)
return callbackURL
}
callback := func(ctx context.Context, t *testing.T, callbackURL *url.URL, cookies []*http.Cookie) int {
t.Helper()
req, err := http.NewRequestWithContext(ctx, http.MethodGet, callbackURL.String(), nil)
require.NoError(t, err)
req.AddCookie(&http.Cookie{Name: codersdk.SessionTokenCookie, Value: memberClient.SessionToken()})
for _, c := range cookies {
req.AddCookie(&http.Cookie{Name: c.Name, Value: c.Value})
}
res, err := memberClient.HTTPClient.Do(req)
require.NoError(t, err)
defer res.Body.Close()
return res.StatusCode
}
t.Run("ConfiguredResourceIsSent", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
config := createConfig(ctx, t, "resource-configured", resource)
authorizeURL, cookies := connect(ctx, t, config)
require.Equal(t, resource, authorizeURL.Query().Get("resource"))
callbackURL := authorize(ctx, t, authorizeURL)
require.Equal(t, http.StatusOK, callback(ctx, t, callbackURL, cookies), "callback must exchange the code for the authorized resource")
fetched, err := memberClient.MCPServerConfigByID(ctx, config.OrganizationID, config.ID)
require.NoError(t, err)
require.True(t, fetched.AuthConnected, "callback must store the exchanged token")
})
t.Run("ResourceChangedAfterConnectIsRejected", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
config := createConfig(ctx, t, "resource-changed", resource)
authorizeURL, cookies := connect(ctx, t, config)
callbackURL := authorize(ctx, t, authorizeURL)
movedResource := "https://mcp.example.com/moved"
_, err := adminClient.UpdateMCPServerConfig(ctx, config.OrganizationID, config.ID, codersdk.UpdateMCPServerConfigRequest{
OAuth2Resource: &movedResource,
})
require.NoError(t, err)
require.Equal(t, http.StatusConflict, callback(ctx, t, callbackURL, cookies))
_, redeemed := redeemedCodes.Load(callbackURL.Query().Get("code"))
require.False(t, redeemed, "a code authorized for one resource must not be redeemed for another")
})
t.Run("ManualConfigWithoutResourceSendsNone", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
config := createConfig(ctx, t, "resource-unset", "")
authorizeURL, _ := connect(ctx, t, config)
require.False(t, authorizeURL.Query().Has("resource"),
"manual configs without oauth2_resource must not send a resource parameter")
})
}
// nolint:bodyclose
func TestMCPServerOAuth2PKCE(t *testing.T) {
t.Parallel()
+37 -14
View File
@@ -24,6 +24,7 @@ import (
"github.com/google/uuid"
"github.com/modelcontextprotocol/go-sdk/mcp"
"golang.org/x/oauth2"
"golang.org/x/oauth2/clientcredentials"
"golang.org/x/sync/errgroup"
"golang.org/x/xerrors"
@@ -1475,8 +1476,9 @@ func RefreshFailureReason(err error) string {
// RefreshOAuth2Token checks whether the given MCP user token is
// expired (or within 10 seconds of expiry) and refreshes it using
// the OAuth2 credentials from the server config. If the token is
// still valid, no network call is made and Refreshed is false.
// the OAuth2 credentials from the server config, sending the config's
// RFC 8707 resource indicator when set. If the token is still valid,
// no network call is made and Refreshed is false.
//
// The caller is responsible for persisting the result when
// Refreshed is true.
@@ -1486,14 +1488,6 @@ func RefreshOAuth2Token(
cfg database.MCPServerConfig,
tok database.MCPServerUserToken,
) (RefreshResult, error) {
oauth2Cfg := &oauth2.Config{
ClientID: cfg.OAuth2ClientID,
ClientSecret: cfg.OAuth2ClientSecret,
Endpoint: oauth2.Endpoint{
TokenURL: cfg.OAuth2TokenURL,
},
}
oldToken := &oauth2.Token{
AccessToken: tok.AccessToken,
RefreshToken: tok.RefreshToken,
@@ -1502,6 +1496,38 @@ func RefreshOAuth2Token(
if tok.Expiry.Valid {
oldToken.Expiry = tok.Expiry.Time
}
// Valid uses a 10-second expiry window, so tokens about to
// expire are also refreshed proactively.
if oldToken.Valid() {
return RefreshResult{
AccessToken: tok.AccessToken,
RefreshToken: tok.RefreshToken,
TokenType: tok.TokenType,
Expiry: oldToken.Expiry,
}, nil
}
if tok.RefreshToken == "" {
return RefreshResult{}, xerrors.New("refresh oauth2 token: token expired and refresh token is not set")
}
params := url.Values{
"grant_type": {"refresh_token"},
"refresh_token": {tok.RefreshToken},
}
if cfg.OAuth2Resource != "" {
params.Set("resource", cfg.OAuth2Resource)
}
// oauth2.Config cannot add parameters to refresh requests. The
// clientcredentials token source sends EndpointParams verbatim,
// allows overriding grant_type, and shares oauth2.Config's token
// request code: auth style detection, *oauth2.RetrieveError, and
// keeping the request's refresh_token when the response omits it.
refreshCfg := &clientcredentials.Config{
ClientID: cfg.OAuth2ClientID,
ClientSecret: cfg.OAuth2ClientSecret,
TokenURL: cfg.OAuth2TokenURL,
EndpointParams: params,
}
// Cap the refresh HTTP call so a stalled token endpoint
// cannot block the entire MCP connection phase. The timeout
@@ -1515,10 +1541,7 @@ func RefreshOAuth2Token(
refreshClient.CheckRedirect = safedial.CheckSameOriginRedirect
refreshCtx = context.WithValue(refreshCtx, oauth2.HTTPClient, &refreshClient)
// TokenSource automatically refreshes expired tokens. It
// uses a 10-second expiry window, so tokens about to expire
// are also refreshed proactively.
newToken, err := oauth2Cfg.TokenSource(refreshCtx, oldToken).Token()
newToken, err := refreshCfg.Token(refreshCtx)
if err != nil {
return RefreshResult{}, xerrors.Errorf("refresh oauth2 token: %w", err)
}
+117
View File
@@ -6,12 +6,14 @@ import (
"encoding/json"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"sync/atomic"
"testing"
"time"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/oauth2"
"golang.org/x/xerrors"
@@ -167,3 +169,118 @@ func TestBuildAuthHeadersSkipsFailedToken(t *testing.T) {
require.Equal(t, "Bearer valid", headers["Authorization"])
})
}
func TestRefreshOAuth2TokenResourceIndicator(t *testing.T) {
t.Parallel()
const resource = "https://mcp.example.com/mcp"
cases := []struct {
name string
resource string
}{
{name: "Set", resource: resource},
{name: "Unset"},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
forms := make(chan url.Values, 1)
tokenSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if !assert.NoError(t, r.ParseForm()) {
http.Error(w, "bad form", http.StatusBadRequest)
return
}
// Auth style auto-detection may retry a rejected request.
select {
case forms <- r.PostForm:
default:
}
w.Header().Set("Content-Type", "application/json")
if r.PostForm.Get("resource") != tc.resource {
w.WriteHeader(http.StatusBadRequest)
_, _ = w.Write([]byte(`{"error":"invalid_target"}`))
return
}
_, _ = w.Write([]byte(`{"access_token":"fresh","token_type":"Bearer","expires_in":3600}`))
}))
defer tokenSrv.Close()
cfg := database.MCPServerConfig{
OAuth2ClientID: "cid",
OAuth2TokenURL: tokenSrv.URL,
OAuth2Resource: tc.resource,
}
tok := database.MCPServerUserToken{
AccessToken: "expired",
RefreshToken: "refresh",
TokenType: "Bearer",
Expiry: sql.NullTime{Time: time.Now().Add(-time.Hour), Valid: true},
}
result, err := mcpclient.RefreshOAuth2Token(context.Background(), testMCPHTTPClient(nil), cfg, tok)
require.NoError(t, err)
require.True(t, result.Refreshed)
require.Equal(t, "fresh", result.AccessToken)
// The provider did not rotate, so the stored refresh token is kept.
require.Equal(t, "refresh", result.RefreshToken)
form := <-forms
require.Equal(t, "refresh_token", form.Get("grant_type"))
require.Equal(t, "refresh", form.Get("refresh_token"))
require.False(t, form.Has("scope"))
if tc.resource == "" {
require.False(t, form.Has("resource"))
}
})
}
}
func TestRefreshOAuth2TokenWithoutRequest(t *testing.T) {
t.Parallel()
var hits atomic.Int64
tokenSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
hits.Add(1)
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`{"access_token":"fresh","token_type":"Bearer","expires_in":3600}`))
}))
t.Cleanup(tokenSrv.Close)
cfg := database.MCPServerConfig{
OAuth2ClientID: "cid",
OAuth2TokenURL: tokenSrv.URL,
OAuth2Resource: "https://mcp.example.com/mcp",
}
t.Run("ValidToken", func(t *testing.T) {
t.Parallel()
expiry := time.Now().Add(time.Hour)
result, err := mcpclient.RefreshOAuth2Token(context.Background(), testMCPHTTPClient(nil), cfg, database.MCPServerUserToken{
AccessToken: "still-valid",
RefreshToken: "refresh",
TokenType: "Bearer",
Expiry: sql.NullTime{Time: expiry, Valid: true},
})
require.NoError(t, err)
require.False(t, result.Refreshed)
require.Equal(t, "still-valid", result.AccessToken)
require.Equal(t, "refresh", result.RefreshToken)
require.Equal(t, "Bearer", result.TokenType)
require.True(t, expiry.Equal(result.Expiry))
})
t.Run("ExpiredWithoutRefreshToken", func(t *testing.T) {
t.Parallel()
_, err := mcpclient.RefreshOAuth2Token(context.Background(), testMCPHTTPClient(nil), cfg, database.MCPServerUserToken{
AccessToken: "expired",
TokenType: "Bearer",
Expiry: sql.NullTime{Time: time.Now().Add(-time.Hour), Valid: true},
})
require.Error(t, err)
})
t.Cleanup(func() {
require.Zero(t, hits.Load(), "no token request may be made")
})
}
+17 -10
View File
@@ -73,6 +73,7 @@ type MCPServerConfig struct {
OAuth2AuthURL string `json:"oauth2_auth_url,omitempty"`
OAuth2TokenURL string `json:"oauth2_token_url,omitempty"`
OAuth2RevocationURL string `json:"oauth2_revocation_url,omitempty"`
OAuth2Resource string `json:"oauth2_resource,omitempty"`
OAuth2Scopes string `json:"oauth2_scopes,omitempty"`
// API key fields (only populated for admins).
@@ -161,11 +162,14 @@ type CreateMCPServerConfigRequest struct {
OAuth2TokenURL string `json:"oauth2_token_url,omitempty" validate:"omitempty,url"`
// OAuth2RevocationURL is the provider's RFC 7009 revocation
// endpoint; auto-populated by OAuth2 discovery when omitted.
OAuth2RevocationURL string `json:"oauth2_revocation_url,omitempty" validate:"omitempty,url"`
OAuth2Scopes string `json:"oauth2_scopes,omitempty"`
APIKeyHeader string `json:"api_key_header,omitempty"`
APIKeyValue string `json:"api_key_value,omitempty"`
CustomHeaders map[string]string `json:"custom_headers,omitempty"`
OAuth2RevocationURL string `json:"oauth2_revocation_url,omitempty" validate:"omitempty,url"`
// OAuth2Resource is the RFC 8707 resource indicator sent on OAuth2
// requests when set. Auto-discovery fills it if empty.
OAuth2Resource string `json:"oauth2_resource,omitempty"`
OAuth2Scopes string `json:"oauth2_scopes,omitempty"`
APIKeyHeader string `json:"api_key_header,omitempty"`
APIKeyValue string `json:"api_key_value,omitempty"`
CustomHeaders map[string]string `json:"custom_headers,omitempty"`
ToolAllowList []string `json:"tool_allow_list,omitempty"`
ToolDenyList []string `json:"tool_deny_list,omitempty"`
@@ -200,11 +204,14 @@ type UpdateMCPServerConfigRequest struct {
OAuth2TokenURL *string `json:"oauth2_token_url,omitempty" validate:"omitempty,url"`
// OAuth2RevocationURL is validated in the handler because a
// validate tag would reject the pointer to "" that clears it.
OAuth2RevocationURL *string `json:"oauth2_revocation_url,omitempty"`
OAuth2Scopes *string `json:"oauth2_scopes,omitempty"`
APIKeyHeader *string `json:"api_key_header,omitempty"`
APIKeyValue *string `json:"api_key_value,omitempty"`
CustomHeaders *map[string]string `json:"custom_headers,omitempty"`
OAuth2RevocationURL *string `json:"oauth2_revocation_url,omitempty"`
// OAuth2Resource is the RFC 8707 resource indicator. Omit it to keep
// the stored value; send "" to clear it.
OAuth2Resource *string `json:"oauth2_resource,omitempty"`
OAuth2Scopes *string `json:"oauth2_scopes,omitempty"`
APIKeyHeader *string `json:"api_key_header,omitempty"`
APIKeyValue *string `json:"api_key_value,omitempty"`
CustomHeaders *map[string]string `json:"custom_headers,omitempty"`
ToolAllowList *[]string `json:"tool_allow_list,omitempty"`
ToolDenyList *[]string `json:"tool_deny_list,omitempty"`
+1
View File
@@ -518,6 +518,7 @@ Actions: `create`, `write`, `delete`
<tr><td><code>oauth2_client_id</code></td><td>Yes</td></tr>
<tr><td><code>oauth2_client_secret</code></td><td>Yes</td></tr>
<tr><td><code>oauth2_client_secret_key_id</code></td><td>No</td></tr>
<tr><td><code>oauth2_resource</code></td><td>Yes</td></tr>
<tr><td><code>oauth2_revocation_url</code></td><td>Yes</td></tr>
<tr><td><code>oauth2_scopes</code></td><td>Yes</td></tr>
<tr><td><code>oauth2_token_url</code></td><td>Yes</td></tr>
@@ -88,10 +88,16 @@ Optional fields:
| `oauth2_client_secret` | OAuth2 client secret. |
| `oauth2_scopes` | Space-separated list of scopes. |
| `oauth2_revocation_url` | Token revocation endpoint URL (RFC 7009). |
| `oauth2_resource` | Resource indicator URI (RFC 8707). |
The revocation endpoint must use HTTPS.
Loopback URLs may use HTTP for local development and tests.
When `oauth2_resource` is set, Coder sends it as the RFC 8707 `resource` parameter on authorization, token, and refresh requests.
The value must be an absolute URI without a fragment or user information.
A manual configuration sends no `resource` parameter unless you set this field.
Leave it empty for providers that reject the parameter, such as Microsoft Entra ID.
**Auto-discovery** — leave `oauth2_client_id`, `oauth2_auth_url`, and
`oauth2_token_url` empty. The server attempts discovery in this order:
@@ -99,17 +105,17 @@ Loopback URLs may use HTTP for local development and tests.
1. RFC 8414 — Authorization Server Metadata
1. RFC 7591 — Dynamic Client Registration
Auto-discovery also records the provider's `revocation_endpoint` from the
RFC 8414 metadata when advertised. An explicit `oauth2_revocation_url` in
the request takes precedence over the discovered value.
Auto-discovery also records the provider's `revocation_endpoint` from the RFC 8414 metadata when advertised, and sets `oauth2_resource` to the Protected Resource Metadata `resource` that matches the MCP server URL.
When none matches, it uses the server URL with the scheme and host lowercased and any user information or fragment removed.
Explicit `oauth2_revocation_url` and `oauth2_resource` values in the request take precedence over the discovered values.
Discovery runs only when a configuration is created, so to add a resource indicator to an existing configuration, set `oauth2_resource` on it or re-create the server.
Users connect through a popup that redirects through the OAuth2 provider.
Tokens are stored per-user and refreshed automatically. Users can disconnect
via the UI or API to remove stored tokens. When a revocation endpoint is
configured, disconnecting also asks the provider to revoke the token
(RFC 7009). Provider revocation is best-effort: the stored token is always
deleted from Coder, and the disconnect response reports whether provider
revocation succeeded via `token_revoked` and `token_revocation_error`.
Tokens are stored per-user and refreshed automatically.
Changing the server URL, authentication type, `oauth2_client_id`, `oauth2_token_url`, `oauth2_revocation_url`, or `oauth2_resource` deletes every user's stored token, so users must connect again.
Users can disconnect via the UI or API to remove stored tokens.
When a revocation endpoint is configured, disconnecting also asks the provider to revoke the token (RFC 7009).
Provider revocation is best-effort: the stored token is always deleted from Coder, and the disconnect response reports whether provider revocation succeeded via `token_revoked` and `token_revocation_error`.
### API key
+6
View File
@@ -6940,6 +6940,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in
"oauth2_auth_url": "string",
"oauth2_client_id": "string",
"oauth2_client_secret": "string",
"oauth2_resource": "string",
"oauth2_revocation_url": "string",
"oauth2_scopes": "string",
"oauth2_token_url": "string",
@@ -6976,6 +6977,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in
| `oauth2_auth_url` | string | false | | |
| `oauth2_client_id` | string | false | | |
| `oauth2_client_secret` | string | false | | |
| `oauth2_resource` | string | false | | Oauth2 resource is the RFC 8707 resource indicator sent on OAuth2 requests when set. Auto-discovery fills it if empty. |
| `oauth2_revocation_url` | string | false | | Oauth2 revocation URL is the provider's RFC 7009 revocation endpoint; auto-populated by OAuth2 discovery when omitted. |
| `oauth2_scopes` | string | false | | |
| `oauth2_token_url` | string | false | | |
@@ -10828,6 +10830,7 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith
"model_intent": true,
"oauth2_auth_url": "string",
"oauth2_client_id": "string",
"oauth2_resource": "string",
"oauth2_revocation_url": "string",
"oauth2_scopes": "string",
"oauth2_token_url": "string",
@@ -10868,6 +10871,7 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith
| `model_intent` | boolean | false | | |
| `oauth2_auth_url` | string | false | | |
| `oauth2_client_id` | string | false | | Oauth2 client ID fields (only populated for admins). |
| `oauth2_resource` | string | false | | |
| `oauth2_revocation_url` | string | false | | |
| `oauth2_scopes` | string | false | | |
| `oauth2_token_url` | string | false | | |
@@ -16954,6 +16958,7 @@ The cursor is owner-scoped, so only the chat owner may set this. Opening a chat'
"oauth2_auth_url": "string",
"oauth2_client_id": "string",
"oauth2_client_secret": "string",
"oauth2_resource": "string",
"oauth2_revocation_url": "string",
"oauth2_scopes": "string",
"oauth2_token_url": "string",
@@ -16990,6 +16995,7 @@ The cursor is owner-scoped, so only the chat owner may set this. Opening a chat'
| `oauth2_auth_url` | string | false | | |
| `oauth2_client_id` | string | false | | |
| `oauth2_client_secret` | string | false | | |
| `oauth2_resource` | string | false | | Oauth2 resource is the RFC 8707 resource indicator. Omit it to keep the stored value; send "" to clear it. |
| `oauth2_revocation_url` | string | false | | Oauth2 revocation URL is validated in the handler because a validate tag would reject the pointer to "" that clears it. |
| `oauth2_scopes` | string | false | | |
| `oauth2_token_url` | string | false | | |
+1
View File
@@ -589,6 +589,7 @@ var auditableResourcesTypes = map[any]map[string]Action{
"group_acl": ActionTrack,
"user_acl": ActionTrack,
"oauth2_revocation_url": ActionTrack,
"oauth2_resource": ActionTrack,
"organization_id": ActionIgnore,
},
&database.Skill{}: {
+11
View File
@@ -4389,6 +4389,11 @@ export interface CreateMCPServerConfigRequest {
* endpoint; auto-populated by OAuth2 discovery when omitted.
*/
readonly oauth2_revocation_url?: string;
/**
* OAuth2Resource is the RFC 8707 resource indicator sent on OAuth2
* requests when set. Auto-discovery fills it if empty.
*/
readonly oauth2_resource?: string;
readonly oauth2_scopes?: string;
readonly api_key_header?: string;
readonly api_key_value?: string;
@@ -6590,6 +6595,7 @@ export interface MCPServerConfig {
readonly oauth2_auth_url?: string;
readonly oauth2_token_url?: string;
readonly oauth2_revocation_url?: string;
readonly oauth2_resource?: string;
readonly oauth2_scopes?: string;
/**
* API key fields (only populated for admins).
@@ -10850,6 +10856,11 @@ export interface UpdateMCPServerConfigRequest {
* validate tag would reject the pointer to "" that clears it.
*/
readonly oauth2_revocation_url?: string;
/**
* OAuth2Resource is the RFC 8707 resource indicator. Omit it to keep
* the stored value; send "" to clear it.
*/
readonly oauth2_resource?: string;
readonly oauth2_scopes?: string;
readonly api_key_header?: string;
readonly api_key_value?: string;
@@ -153,6 +153,21 @@ const OAuth2Fields: React.FC<MCPServerAuthFieldsProps> = ({
/>
</Field>
</div>
<div className="grid gap-4 sm:grid-cols-2">
<Field
label="Resource URL"
htmlFor={`${formId}-oauth-resource`}
description="Sent to the OAuth2 provider as the RFC 8707 resource parameter. When empty, none is sent unless auto-discovery fills it in while registering the client."
>
<Input
id={`${formId}-oauth-resource`}
className="placeholder:text-content-disabled shadow-none"
{...form.getFieldProps("oauth2Resource")}
placeholder="https://"
disabled={disabled}
/>
</Field>
</div>
</div>
);
@@ -133,3 +133,32 @@ it("updates an API key server without re-entering the saved key", async () => {
);
});
});
it("clears a stored OAuth2 resource URL on update", async () => {
const user = userEvent.setup();
const onUpdateServer = vi.fn(async () => undefined);
const mockOAuth2Server = {
...MockCoderMCPServer,
oauth2_resource: "https://mcp.example.com/mcp",
};
render(
<MCPServerForm
server={mockOAuth2Server}
listPath="/servers"
isSaving={false}
isDeleting={false}
canSelectUserOIDC
onUpdateServer={onUpdateServer}
onCancel={vi.fn()}
/>,
);
await user.click(screen.getByRole("button", { name: /authentication/i }));
await user.clear(screen.getByLabelText("Resource URL"));
await user.click(screen.getByRole("button", { name: "Update server" }));
await waitFor(() => {
expect(onUpdateServer).toHaveBeenCalledWith(
mockOAuth2Server.id,
expect.objectContaining({ oauth2_resource: "" }),
);
});
});
@@ -115,16 +115,33 @@ describe("mcpServerFormLogic", () => {
expect(request.enabled).toBeUndefined();
});
it("sends an empty revocation URL on update so it can be cleared", () => {
const cleared = buildUpdateMCPServerConfigRequest(
validValues({ authType: "oauth2", oauth2RevocationURL: "" }),
);
it("sends empty revocation URL and resource on update so they can be cleared", () => {
const clearedOAuth2Values = validValues({
authType: "oauth2",
oauth2RevocationURL: "",
oauth2Resource: " ",
});
const cleared = buildUpdateMCPServerConfigRequest(clearedOAuth2Values);
expect(cleared.oauth2_revocation_url).toBe("");
expect(cleared.oauth2_resource).toBe("");
const created = buildCreateMCPServerConfigRequest(
validValues({ authType: "oauth2", oauth2RevocationURL: "" }),
);
const created = buildCreateMCPServerConfigRequest(clearedOAuth2Values);
expect(created.oauth2_revocation_url).toBeUndefined();
expect(created.oauth2_resource).toBeUndefined();
});
it("round-trips the stored OAuth2 resource", () => {
const values = buildInitialMCPServerFormValues({
...MockCoderMCPServer,
oauth2_resource: "https://mcp.example.com/mcp",
});
expect(values.oauth2Resource).toBe("https://mcp.example.com/mcp");
expect(buildUpdateMCPServerConfigRequest(values).oauth2_resource).toBe(
"https://mcp.example.com/mcp",
);
expect(buildCreateMCPServerConfigRequest(values).oauth2_resource).toBe(
"https://mcp.example.com/mcp",
);
});
it("initializes slugTouched true for edit and false for create", () => {
@@ -56,6 +56,7 @@ export type MCPServerFormValues = {
oauth2AuthURL: string;
oauth2TokenURL: string;
oauth2RevocationURL: string;
oauth2Resource: string;
oauth2Scopes: string;
apiKeyHeader: string;
apiKeyValue: string;
@@ -98,6 +99,7 @@ export const buildInitialMCPServerFormValues = (
oauth2AuthURL: server?.oauth2_auth_url ?? "",
oauth2TokenURL: server?.oauth2_token_url ?? "",
oauth2RevocationURL: server?.oauth2_revocation_url ?? "",
oauth2Resource: server?.oauth2_resource ?? "",
oauth2Scopes: server?.oauth2_scopes ?? "",
apiKeyHeader: server?.api_key_header || "Authorization",
apiKeyValue: server?.has_api_key ? SECRET_PLACEHOLDER : "",
@@ -186,6 +188,7 @@ export const buildCreateMCPServerConfigRequest = (
oauth2_auth_url: values.oauth2AuthURL.trim() || undefined,
oauth2_token_url: values.oauth2TokenURL.trim() || undefined,
oauth2_revocation_url: values.oauth2RevocationURL.trim() || undefined,
oauth2_resource: values.oauth2Resource.trim() || undefined,
oauth2_scopes: values.oauth2Scopes.trim() || undefined,
};
}
@@ -228,9 +231,10 @@ export const buildUpdateMCPServerConfigRequest = (
const { enabled: _enabled, ...updateFields } = base;
return {
...updateFields,
// Always sent: an omitted field keeps the stored value, "" clears it.
// Sent even when empty: an omitted field keeps the stored value, "" clears it.
...(values.authType === "oauth2" && {
oauth2_revocation_url: values.oauth2RevocationURL.trim(),
oauth2_resource: values.oauth2Resource.trim(),
}),
tool_allow_list: [...(base.tool_allow_list ?? [])],
tool_deny_list: [...(base.tool_deny_list ?? [])],