mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
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.  ## 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 on2b875068e0: 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 on4b54ffb5f9: 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 head2d6abfac49: PASS. The head moved during the round, so it ran in four parts, the last on2d6abfac49. 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 inbf0879560c, 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 in16a1d783fa, 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 in8d617f0528(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 in5c23fe00dc, 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 in72c019ff5e, 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 inef2df2c4e7, 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 in2d6abfac49, 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:
Generated
+11
@@ -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"
|
||||
|
||||
Generated
+11
@@ -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"
|
||||
|
||||
@@ -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,
|
||||
|
||||
Generated
+1
@@ -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 '';
|
||||
@@ -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
|
||||
}
|
||||
|
||||
Generated
+1
@@ -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 {
|
||||
|
||||
Generated
+48
-32
@@ -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,
|
||||
|
||||
@@ -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
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
@@ -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()
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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
@@ -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"`
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
Generated
+6
@@ -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 | | |
|
||||
|
||||
@@ -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{}: {
|
||||
|
||||
Generated
+11
@@ -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 ?? [])],
|
||||
|
||||
Reference in New Issue
Block a user