mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
fix: support path-aware OAuth discovery for remote MCP (#30168)
## Problem VS Code discovers Coder's MCP endpoint as a path-qualified OAuth resource, but Coder advertises the deployment root. Accepting MCP-bound tokens also requires keeping REST-backed tool calls within the originating MCP resource rather than granting those tokens direct REST access. ## Fix - Publish path-aware protected resource metadata and authentication challenges; keep authorization-server metadata at the root. - Validate the MCP audience and delegate tool requests through a private in-process transport that retains identity, scopes, expiry, revocation, and streaming cancellation. - Reject unsupported MCP path aliases and validate SDK path components before URL resolution. - Add discovery, audience, authorization, endpoint-isolation, and transport lifecycle regression tests and update integration docs. ## Validation VS Code completed OAuth and returned the authenticated user through a real MCP tool. Official MCP SDK 1.30.1 completed discovery, registration, PKCE, tool execution, and refresh. These live checks preceded the final path hardening; the hardened paths are covered by automated regressions. Affected Go package tests, targeted race tests, backend/SDK lint, and targeted documentation checks passed during self-review. The exact PR commit was also checked separately from the uncommitted consent-scope fix. > [!NOTE] > Tokens are MCP-audience-bound, but grants still include `coder:all`; least-privilege scopes remain a separate requirement ([PLAT-639](https://linear.app/codercom/issue/PLAT-639)). Preview: https://coder.com/docs/@fix/mcp-internal-delegation/ai-coder/mcp-server <details> <summary>Implementation decisions and review context</summary> 1. Treat `/api/experimental/mcp/http` and its single-trailing-slash form as supported MCP resource paths; reject other aliases and descendants before route authentication. 2. Preserve deployment-root discovery and authorization-server metadata for existing REST integrations. 3. Bind private delegation to the authenticated API-key ID. Revalidate the original credential and enforce normal RBAC/scopes on every internal REST request; do not derive delegation from HTTP headers. 4. Keep internal HTTP, streaming, and WebSocket traffic in-process with no network fallback. Close the transport on outer-request cancellation and normal completion. 5. Validate workspace owner/name path segments before SDK URL construction, including tool-level tests that malformed arguments cannot select a different REST endpoint. 6. Keep compatibility and delegated tool execution together: discovery-only changes would allow authentication while leaving real MCP tool calls broken. </details> > 🤖 This PR was created with the help of Coder Agents, and needs a human review. 🧑💻
This commit is contained in:
+25
-4
@@ -74,6 +74,7 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/httpmw"
|
||||
"github.com/coder/coder/v2/coderd/httpmw/loggermw"
|
||||
"github.com/coder/coder/v2/coderd/idpsync"
|
||||
"github.com/coder/coder/v2/coderd/mcp"
|
||||
"github.com/coder/coder/v2/coderd/metricscache"
|
||||
"github.com/coder/coder/v2/coderd/notifications"
|
||||
"github.com/coder/coder/v2/coderd/oauth2provider"
|
||||
@@ -1106,7 +1107,7 @@ func New(options *Options) *API {
|
||||
// and in the build info response and the AI bridge config, so a runtime
|
||||
// toggle would have to update all three.
|
||||
oauth2ProviderEnabled := api.DeploymentValues.OAuth2.Provider.Enable.Value()
|
||||
apiKeyMiddleware := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{
|
||||
apiKeyConfig := httpmw.ExtractAPIKeyConfig{
|
||||
DB: options.Database,
|
||||
ActivateDormantUser: ActivateDormantUser(options.Logger, &api.Auditor, options.Database),
|
||||
OAuth2Configs: oauthConfigs,
|
||||
@@ -1117,7 +1118,8 @@ func New(options *Options) *API {
|
||||
PostAuthAdditionalHeadersFunc: options.PostAuthAdditionalHeadersFunc,
|
||||
Logger: options.Logger,
|
||||
AccessURL: options.AccessURL,
|
||||
})
|
||||
}
|
||||
apiKeyMiddleware := httpmw.ExtractAPIKeyMW(apiKeyConfig)
|
||||
// Same as above but it redirects to the login page.
|
||||
apiKeyMiddlewareRedirect := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{
|
||||
DB: options.Database,
|
||||
@@ -1405,13 +1407,32 @@ func New(options *Options) *API {
|
||||
})
|
||||
|
||||
r.Route("/mcp", func(r chi.Router) {
|
||||
r.Use(apiKeyMiddleware)
|
||||
// Providers pin the redirect URI when a session is established,
|
||||
// so the callback URL cannot change for existing sessions without
|
||||
// breaking token refresh and forcing a re-auth.
|
||||
r.Get("/servers/{mcpServer}/oauth2/callback", api.mcpServerOAuth2Callback)
|
||||
r.With(apiKeyMiddleware).Get("/servers/{mcpServer}/oauth2/callback", api.mcpServerOAuth2Callback)
|
||||
// MCP HTTP transport endpoint with mandatory authentication.
|
||||
r.Route("/http", func(r chi.Router) {
|
||||
r.Use(func(next http.Handler) http.Handler {
|
||||
cfg := apiKeyConfig
|
||||
cfg.ResourceURI = options.AccessURL.JoinPath(mcp.MCPEndpoint).String()
|
||||
cfg.ResourceMetadataURL = options.AccessURL.JoinPath("/.well-known/oauth-protected-resource", mcp.MCPEndpoint).String()
|
||||
canonical := httpmw.ExtractAPIKeyMW(cfg)(next)
|
||||
cfg.ResourceMetadataURL = options.AccessURL.JoinPath("/.well-known/oauth-protected-resource", mcp.MCPEndpoint+"/").String()
|
||||
trailingSlash := httpmw.ExtractAPIKeyMW(cfg)(next)
|
||||
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
|
||||
// Routing normalizes slashes, but OAuth resource identity
|
||||
// must not admit aliases or arbitrary descendant paths.
|
||||
switch r.URL.EscapedPath() {
|
||||
case mcp.MCPEndpoint:
|
||||
canonical.ServeHTTP(rw, r)
|
||||
case mcp.MCPEndpoint + "/":
|
||||
trailingSlash.ServeHTTP(rw, r)
|
||||
default:
|
||||
httpapi.RouteNotFound(rw)
|
||||
}
|
||||
})
|
||||
})
|
||||
r.Use(
|
||||
httpmw.RequireOAuth2Provider(oauth2ProviderEnabled),
|
||||
httpmw.RequireExperiment(api.Experiments, codersdk.ExperimentMCPServerHTTP),
|
||||
|
||||
+49
-10
@@ -34,8 +34,25 @@ import (
|
||||
type (
|
||||
apiKeyContextKey struct{}
|
||||
apiKeyPrecheckedContextKey struct{}
|
||||
apiKeyDelegationContextKey struct{}
|
||||
)
|
||||
|
||||
type apiKeyDelegation struct {
|
||||
resourceURI string
|
||||
apiKeyID string
|
||||
}
|
||||
|
||||
// WithAPIKeyDelegation binds server-owned, in-process work to the originating
|
||||
// resource URI and authenticated API key. Never derive these values from HTTP
|
||||
// headers or query parameters. The original credential must still pass
|
||||
// validation, including its audience, scopes, and expiry.
|
||||
func WithAPIKeyDelegation(ctx context.Context, resourceURI, apiKeyID string) context.Context {
|
||||
return context.WithValue(ctx, apiKeyDelegationContextKey{}, apiKeyDelegation{
|
||||
resourceURI: resourceURI,
|
||||
apiKeyID: apiKeyID,
|
||||
})
|
||||
}
|
||||
|
||||
// ValidateAPIKeyConfig holds the settings needed for API key
|
||||
// validation at the top of the request lifecycle. Unlike
|
||||
// ExtractAPIKeyConfig it omits route-specific fields
|
||||
@@ -175,9 +192,17 @@ type ExtractAPIKeyConfig struct {
|
||||
PostAuthAdditionalHeadersFunc func(a rbac.Subject, header http.Header)
|
||||
|
||||
// AccessURL is the configured access URL for this Coder deployment.
|
||||
// Used for generating OAuth2 resource metadata URLs in WWW-Authenticate headers.
|
||||
// It supplies the default OAuth2 resource URI and metadata URL.
|
||||
AccessURL *url.URL
|
||||
|
||||
// ResourceURI overrides the OAuth2 audience expected by this route.
|
||||
// Empty uses the deployment access URL, or the request origin if unset.
|
||||
ResourceURI string
|
||||
|
||||
// ResourceMetadataURL overrides the OAuth2 discovery URL advertised in
|
||||
// WWW-Authenticate. Empty uses the deployment's root metadata endpoint.
|
||||
ResourceMetadataURL string
|
||||
|
||||
// Logger is used for logging middleware operations.
|
||||
Logger slog.Logger
|
||||
}
|
||||
@@ -265,6 +290,13 @@ func ValidateAPIKey(ctx context.Context, cfg ValidateAPIKeyConfig, r *http.Reque
|
||||
return nil, valErr
|
||||
}
|
||||
|
||||
if delegation, ok := ctx.Value(apiKeyDelegationContextKey{}).(apiKeyDelegation); ok && delegation.apiKeyID != key.ID {
|
||||
return nil, &ValidateAPIKeyError{
|
||||
Code: http.StatusUnauthorized,
|
||||
Response: codersdk.Response{Message: "Delegated requests must use the credential that authenticated the originating request."},
|
||||
}
|
||||
}
|
||||
|
||||
// Log the API key ID for all requests that have a valid key
|
||||
// format and secret, regardless of whether subsequent validation
|
||||
// (expiry, user status, etc.) succeeds.
|
||||
@@ -628,7 +660,7 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon
|
||||
|
||||
// Add WWW-Authenticate header for 401/403 responses (RFC 6750 + RFC 9728)
|
||||
if code == http.StatusUnauthorized || code == http.StatusForbidden {
|
||||
rw.Header().Set("WWW-Authenticate", buildWWWAuthenticateHeader(cfg.AccessURL, r, code, response))
|
||||
rw.Header().Set("WWW-Authenticate", buildWWWAuthenticateHeader(cfg.AccessURL, cfg.ResourceMetadataURL, r, code, response))
|
||||
}
|
||||
|
||||
httpapi.Write(ctx, rw, code, response)
|
||||
@@ -701,7 +733,11 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon
|
||||
|
||||
// Validate OAuth2 provider app token audience (RFC 8707) if applicable.
|
||||
if key.LoginType == database.LoginTypeOAuth2ProviderApp {
|
||||
if err := validateOAuth2ProviderAppTokenAudience(ctx, cfg.DB, *key, cfg.AccessURL, r); err != nil {
|
||||
resourceURI := cfg.ResourceURI
|
||||
if resourceURI == "" {
|
||||
resourceURI = extractExpectedAudience(cfg.AccessURL, r)
|
||||
}
|
||||
if err := validateOAuth2ProviderAppTokenAudience(ctx, cfg.DB, *key, resourceURI); err != nil {
|
||||
// Log the detailed error for debugging but don't expose it to the client.
|
||||
cfg.Logger.Debug(ctx, "oauth2 token audience validation failed", slog.Error(err))
|
||||
return optionalWrite(http.StatusForbidden, codersdk.Response{
|
||||
@@ -742,7 +778,7 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon
|
||||
|
||||
// validateOAuth2ProviderAppTokenAudience validates that an OAuth2 provider app token
|
||||
// is being used with the correct audience/resource server (RFC 8707).
|
||||
func validateOAuth2ProviderAppTokenAudience(ctx context.Context, db database.Store, key database.APIKey, accessURL *url.URL, r *http.Request) error {
|
||||
func validateOAuth2ProviderAppTokenAudience(ctx context.Context, db database.Store, key database.APIKey, expectedAudience string) error {
|
||||
// Get the OAuth2 provider app token to check its audience
|
||||
//nolint:gocritic // OAuth2 system context — audience validation for provider app tokens
|
||||
token, err := db.GetOAuth2ProviderAppTokenByAPIKeyID(dbauthz.AsSystemOAuth2(ctx), key.ID)
|
||||
@@ -755,8 +791,11 @@ func validateOAuth2ProviderAppTokenAudience(ctx context.Context, db database.Sto
|
||||
return nil
|
||||
}
|
||||
|
||||
// Extract the expected audience from the access URL
|
||||
expectedAudience := extractExpectedAudience(accessURL, r)
|
||||
if delegation, ok := ctx.Value(apiKeyDelegationContextKey{}).(apiKeyDelegation); ok && delegation.apiKeyID == key.ID {
|
||||
// Server-owned delegation validates the originating resource rather
|
||||
// than the resource implementing the internal request.
|
||||
expectedAudience = delegation.resourceURI
|
||||
}
|
||||
|
||||
// Normalize both audience values for RFC 3986 compliant comparison
|
||||
normalizedTokenAudience := normalizeAudienceURI(token.Audience.String)
|
||||
@@ -878,7 +917,7 @@ func normalizePathSegments(path string) string {
|
||||
// Test export functions for testing package access
|
||||
|
||||
// buildWWWAuthenticateHeader constructs RFC 6750 + RFC 9728 compliant WWW-Authenticate header
|
||||
func buildWWWAuthenticateHeader(accessURL *url.URL, r *http.Request, code int, response codersdk.Response) string {
|
||||
func buildWWWAuthenticateHeader(accessURL *url.URL, resourceMetadata string, r *http.Request, code int, response codersdk.Response) string {
|
||||
// Use the configured access URL for resource metadata
|
||||
if accessURL == nil {
|
||||
scheme := "https"
|
||||
@@ -893,7 +932,9 @@ func buildWWWAuthenticateHeader(accessURL *url.URL, r *http.Request, code int, r
|
||||
}
|
||||
}
|
||||
|
||||
resourceMetadata := accessURL.JoinPath("/.well-known/oauth-protected-resource").String()
|
||||
if resourceMetadata == "" {
|
||||
resourceMetadata = accessURL.JoinPath("/.well-known/oauth-protected-resource").String()
|
||||
}
|
||||
|
||||
switch code {
|
||||
case http.StatusUnauthorized:
|
||||
@@ -920,8 +961,6 @@ func buildWWWAuthenticateHeader(accessURL *url.URL, r *http.Request, code int, r
|
||||
// extractExpectedAudience determines the expected audience for the current request.
|
||||
// This should match the resource parameter used during authorization.
|
||||
func extractExpectedAudience(accessURL *url.URL, r *http.Request) string {
|
||||
// For MCP compliance, the audience should be the canonical URI of the resource server
|
||||
// This typically matches the access URL of the Coder deployment
|
||||
var audience string
|
||||
|
||||
if accessURL != nil {
|
||||
|
||||
@@ -3,11 +3,13 @@ package httpmw_test
|
||||
import (
|
||||
"context"
|
||||
"crypto/sha256"
|
||||
"database/sql"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
@@ -39,6 +41,131 @@ import (
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
)
|
||||
|
||||
func TestAPIKeyResource(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db, _ := dbtestutil.NewDB(t)
|
||||
user := dbgen.User(t, db, database.User{})
|
||||
app := dbgen.OAuth2ProviderApp(t, db, database.OAuth2ProviderApp{})
|
||||
origin, err := url.Parse("https://coder.example.com")
|
||||
require.NoError(t, err)
|
||||
const (
|
||||
reports = "https://coder.example.com/reports"
|
||||
jobs = "https://coder.example.com/jobs"
|
||||
metadata = "https://discovery.example.com/reports"
|
||||
)
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
audience string
|
||||
resourceURI string
|
||||
delegatedResource string
|
||||
wrongKey bool
|
||||
invalidSecret bool
|
||||
missingToken bool
|
||||
queryToken bool
|
||||
status int
|
||||
}{
|
||||
{name: "DefaultRoot", audience: origin.String(), status: http.StatusNoContent},
|
||||
{name: "DefaultRejectsPathAudience", audience: reports, status: http.StatusForbidden},
|
||||
{name: "ReportsResource", audience: reports, resourceURI: reports, status: http.StatusNoContent},
|
||||
{name: "JobsResource", audience: jobs, resourceURI: jobs, status: http.StatusNoContent},
|
||||
{name: "NormalizedResource", audience: "https://CODER.example.com:443/reports/", resourceURI: reports, status: http.StatusNoContent},
|
||||
{name: "ResourceRejectsRoot", audience: origin.String(), resourceURI: reports, status: http.StatusForbidden},
|
||||
{name: "ResourceRejectsOtherResource", audience: jobs, resourceURI: reports, status: http.StatusForbidden},
|
||||
{name: "ResourceRejectsOtherDeployment", audience: "https://other.example.com/reports", resourceURI: reports, status: http.StatusForbidden},
|
||||
{name: "ReportsDelegation", audience: reports, delegatedResource: reports, status: http.StatusNoContent},
|
||||
{name: "JobsDelegation", audience: jobs, delegatedResource: jobs, status: http.StatusNoContent},
|
||||
{name: "DelegationOverridesDestinationResource", audience: reports, resourceURI: jobs, delegatedResource: reports, status: http.StatusNoContent},
|
||||
{name: "DelegationRejectsDestinationAudience", audience: jobs, resourceURI: jobs, delegatedResource: reports, status: http.StatusForbidden},
|
||||
{name: "DelegationRejectsRoot", audience: origin.String(), delegatedResource: reports, status: http.StatusForbidden},
|
||||
{name: "DelegationRejectsOtherResource", audience: jobs, delegatedResource: reports, status: http.StatusForbidden},
|
||||
{name: "DelegationRejectsOtherDeployment", audience: "https://other.example.com/reports", delegatedResource: reports, status: http.StatusForbidden},
|
||||
{name: "DelegationRejectsSubstitutedKey", audience: reports, delegatedResource: reports, wrongKey: true, status: http.StatusUnauthorized},
|
||||
{name: "DelegationRejectsInvalidSecret", audience: reports, delegatedResource: reports, invalidSecret: true, status: http.StatusUnauthorized},
|
||||
{name: "DelegationRequiresCredential", audience: reports, delegatedResource: reports, missingToken: true, status: http.StatusUnauthorized},
|
||||
{name: "DelegationRejectsQueryToken", audience: reports, delegatedResource: reports, queryToken: true, status: http.StatusUnauthorized},
|
||||
{name: "ResourceDiscoveryWithoutCredential", resourceURI: reports, missingToken: true, status: http.StatusUnauthorized},
|
||||
{name: "UnboundLegacyResource", resourceURI: reports, status: http.StatusNoContent},
|
||||
{name: "UnboundLegacyDelegation", delegatedResource: reports, status: http.StatusNoContent},
|
||||
{name: "UnboundLegacyRejectsSubstitutedKey", delegatedResource: reports, wrongKey: true, status: http.StatusUnauthorized},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
key, token := dbgen.APIKey(t, db, database.APIKey{
|
||||
UserID: user.ID, LoginType: database.LoginTypeOAuth2ProviderApp,
|
||||
Scopes: database.APIKeyScopes{database.ApiKeyScopeUserRead},
|
||||
})
|
||||
dbgen.OAuth2ProviderAppToken(t, db, database.OAuth2ProviderAppToken{
|
||||
AppID: app.ID, UserID: user.ID, APIKeyID: key.ID, HashPrefix: []byte(key.ID),
|
||||
Audience: sql.NullString{String: tc.audience, Valid: tc.audience != ""},
|
||||
})
|
||||
delegatedID := key.ID
|
||||
if tc.wrongKey {
|
||||
otherKey, _ := dbgen.APIKey(t, db, database.APIKey{UserID: user.ID})
|
||||
delegatedID = otherKey.ID
|
||||
}
|
||||
if tc.invalidSecret {
|
||||
token = key.ID + "-" + strings.Repeat("0", 22)
|
||||
}
|
||||
|
||||
for _, precheck := range []bool{false, true} {
|
||||
t.Run(fmt.Sprintf("Precheck=%t", precheck), func(t *testing.T) {
|
||||
t.Parallel()
|
||||
// Request-controlled resource and delegation hints must have no
|
||||
// effect, whether or not server-owned delegation is present.
|
||||
req := httptest.NewRequest(http.MethodGet, "https://untrusted.example.com/api/v2/users/me?resource="+url.QueryEscape(reports), nil)
|
||||
req.Header.Set("X-Coder-API-Key-Delegation", key.ID)
|
||||
req.Header.Set("X-Coder-MCP-Delegation", "true")
|
||||
req.Header.Set("X-Forwarded-Uri", "/reports")
|
||||
req.Header.Set("X-Forwarded-Host", "coder.example.com")
|
||||
req.Header.Set("Referer", reports)
|
||||
if tc.queryToken {
|
||||
q := req.URL.Query()
|
||||
q.Set("access_token", token)
|
||||
req.URL.RawQuery = q.Encode()
|
||||
} else if !tc.missingToken {
|
||||
req.Header.Set("Authorization", "Bearer "+token)
|
||||
}
|
||||
if tc.delegatedResource != "" {
|
||||
req = req.WithContext(httpmw.WithAPIKeyDelegation(req.Context(), tc.delegatedResource, delegatedID))
|
||||
}
|
||||
originalURL := req.URL.String()
|
||||
handler := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{
|
||||
DB: db, AccessURL: origin, Logger: testutil.Logger(t),
|
||||
ResourceURI: tc.resourceURI, ResourceMetadataURL: metadata,
|
||||
})(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
assert.Equal(t, originalURL, r.URL.String())
|
||||
assert.Equal(t, key.ID, httpmw.APIKey(r).ID)
|
||||
actor := httpmw.UserAuthorization(r.Context())
|
||||
assert.Equal(t, user.ID.String(), actor.ID)
|
||||
assert.Equal(t, key.ScopeSet(), actor.Scope)
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}))
|
||||
if precheck {
|
||||
handler = httpmw.PrecheckAPIKey(httpmw.ValidateAPIKeyConfig{DB: db, Logger: testutil.Logger(t)})(handler)
|
||||
}
|
||||
rw := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rw, req)
|
||||
require.Equal(t, tc.status, rw.Code)
|
||||
if tc.wrongKey {
|
||||
var response codersdk.Response
|
||||
require.NoError(t, json.NewDecoder(rw.Body).Decode(&response))
|
||||
require.Equal(t, "Delegated requests must use the credential that authenticated the originating request.", response.Message)
|
||||
require.NotContains(t, response.Message, key.ID)
|
||||
require.NotContains(t, response.Message, delegatedID)
|
||||
require.NotContains(t, rw.Header().Get("WWW-Authenticate"), "expired")
|
||||
}
|
||||
require.Equal(t, originalURL, req.URL.String())
|
||||
if tc.status != http.StatusNoContent {
|
||||
require.Contains(t, rw.Header().Get("WWW-Authenticate"), `resource_metadata="`+metadata+`"`)
|
||||
}
|
||||
})
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func randomAPIKeyParts() (id string, secret string, hashedSecret []byte) {
|
||||
id, _ = cryptorand.String(10)
|
||||
secret, hashedSecret, _ = apikey.GenerateSecret(22)
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"testing"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
@@ -209,7 +210,51 @@ func TestNormalizePathSegments(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestExtractExpectedAudience tests audience extraction from HTTP requests
|
||||
func TestBuildWWWAuthenticateHeaderResourceMetadata(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
accessURL, err := url.Parse("https://coder.example.com")
|
||||
require.NoError(t, err)
|
||||
for _, tc := range []struct {
|
||||
path string
|
||||
metadataURL string
|
||||
expectedURL string
|
||||
}{
|
||||
{path: "/api/experimental/mcp/http"},
|
||||
{path: "/api/experimental/mcp/http/"},
|
||||
{path: "/api/experimental/mcp/http/child"},
|
||||
{path: "/api/experimental/mcp/http?toolset=chatgpt"},
|
||||
{path: "/api/experimental/mcp/http-extra"},
|
||||
{path: "/api/v2/users/me"},
|
||||
{
|
||||
path: "/reports", metadataURL: "https://discovery.example.com/reports",
|
||||
expectedURL: "https://discovery.example.com/reports",
|
||||
},
|
||||
{
|
||||
path: "/reports/", metadataURL: "https://discovery.example.com/reports/",
|
||||
expectedURL: "https://discovery.example.com/reports/",
|
||||
},
|
||||
{
|
||||
path: "/reports?resource_metadata=https://untrusted.example.com", metadataURL: "https://discovery.example.com/reports",
|
||||
expectedURL: "https://discovery.example.com/reports",
|
||||
},
|
||||
} {
|
||||
t.Run(tc.path, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
req := httptest.NewRequest(http.MethodPost, "https://untrusted.example.com"+tc.path, nil)
|
||||
expectedURL := tc.expectedURL
|
||||
if expectedURL == "" {
|
||||
expectedURL = "https://coder.example.com/.well-known/oauth-protected-resource"
|
||||
}
|
||||
for _, code := range []int{http.StatusUnauthorized, http.StatusForbidden} {
|
||||
header := buildWWWAuthenticateHeader(accessURL, tc.metadataURL, req, code, codersdk.Response{})
|
||||
require.Contains(t, header, `resource_metadata="`+expectedURL+`"`)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestExtractExpectedAudience tests audience extraction from HTTP requests.
|
||||
func TestExtractExpectedAudience(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -234,6 +279,27 @@ func TestExtractExpectedAudience(t *testing.T) {
|
||||
path: "/v1/users",
|
||||
expected: "https://api.example.com/",
|
||||
},
|
||||
{
|
||||
name: "MCPWithoutOverride",
|
||||
scheme: "https",
|
||||
host: "example.com",
|
||||
path: "/api/experimental/mcp/http",
|
||||
expected: "https://example.com/",
|
||||
},
|
||||
{
|
||||
name: "MCPWithTrailingSlashWithoutOverride",
|
||||
scheme: "https",
|
||||
host: "example.com",
|
||||
path: "/api/experimental/mcp/http/",
|
||||
expected: "https://example.com/",
|
||||
},
|
||||
{
|
||||
name: "MCPPrefixLookalike",
|
||||
scheme: "https",
|
||||
host: "example.com",
|
||||
path: "/api/experimental/mcp/http-extra",
|
||||
expected: "https://example.com/",
|
||||
},
|
||||
{
|
||||
name: "WithPort",
|
||||
scheme: "http",
|
||||
@@ -256,6 +322,11 @@ func TestExtractExpectedAudience(t *testing.T) {
|
||||
|
||||
result := extractExpectedAudience(nil, req)
|
||||
assert.Equal(t, tc.expected, result)
|
||||
|
||||
accessURL, err := url.Parse(tc.scheme + "://" + tc.host)
|
||||
require.NoError(t, err)
|
||||
req.Host = "untrusted.example.com"
|
||||
assert.Equal(t, tc.expected, extractExpectedAudience(accessURL, req))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
+2
-2
@@ -22,8 +22,8 @@ const (
|
||||
// MCPServerInstructions is the instructions text for the MCP server.
|
||||
MCPServerInstructions = "Coder MCP Server providing workspace and template management tools"
|
||||
|
||||
// Used in tests and aibridge.
|
||||
MCPEndpoint = "/api/experimental/mcp/http"
|
||||
// MCPEndpoint is the remote MCP HTTP endpoint path.
|
||||
MCPEndpoint = codersdk.MCPEndpoint
|
||||
)
|
||||
|
||||
// Server represents an MCP HTTP server instance
|
||||
|
||||
@@ -3,6 +3,7 @@ package mcp_test
|
||||
import (
|
||||
"context"
|
||||
"crypto/sha256"
|
||||
"database/sql"
|
||||
"encoding/base64"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
@@ -15,6 +16,7 @@ import (
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/google/uuid"
|
||||
"github.com/modelcontextprotocol/go-sdk/mcp"
|
||||
"github.com/spf13/afero"
|
||||
@@ -27,6 +29,8 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/coderdtest"
|
||||
"github.com/coder/coder/v2/coderd/database"
|
||||
"github.com/coder/coder/v2/coderd/database/dbfake"
|
||||
"github.com/coder/coder/v2/coderd/database/dbgen"
|
||||
"github.com/coder/coder/v2/coderd/database/dbtestutil"
|
||||
mcpserver "github.com/coder/coder/v2/coderd/mcp"
|
||||
"github.com/coder/coder/v2/coderd/oauth2provider/oauth2providertest"
|
||||
"github.com/coder/coder/v2/coderd/rbac"
|
||||
@@ -170,6 +174,118 @@ func TestMCPHTTP_E2E_ClientIntegration(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestMCPHTTP_E2E_CancelLogStream(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
db, _ := dbtestutil.NewDB(t)
|
||||
logsDB := &mcpLogStreamStore{Store: db, queried: make(chan struct{}, 1)}
|
||||
outerDone := make(chan struct{})
|
||||
type outerContextKey struct{}
|
||||
coderClient, closer, api := coderdtest.NewWithAPI(t, &coderdtest.Options{
|
||||
Database: logsDB,
|
||||
DeploymentValues: mcpDeploymentValues(t),
|
||||
APIMiddleware: func(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path == mcpserver.MCPEndpoint {
|
||||
defer close(outerDone)
|
||||
r = r.WithContext(context.WithValue(r.Context(), outerContextKey{}, "outer-request-state"))
|
||||
}
|
||||
next.ServeHTTP(w, r)
|
||||
})
|
||||
},
|
||||
})
|
||||
t.Cleanup(func() { _ = closer.Close() })
|
||||
user := coderdtest.CreateFirstUser(t, coderClient)
|
||||
build := dbfake.WorkspaceBuild(t, api.Database, database.WorkspaceTable{
|
||||
OrganizationID: user.OrganizationID, OwnerID: user.UserID,
|
||||
}).Starting().Do().Build
|
||||
key, token := dbgen.APIKey(t, api.Database, database.APIKey{
|
||||
UserID: user.UserID, LoginType: database.LoginTypeOAuth2ProviderApp,
|
||||
})
|
||||
app := dbgen.OAuth2ProviderApp(t, api.Database, database.OAuth2ProviderApp{})
|
||||
dbgen.OAuth2ProviderAppToken(t, api.Database, database.OAuth2ProviderAppToken{
|
||||
AppID: app.ID, UserID: user.UserID, APIKeyID: key.ID,
|
||||
Audience: sql.NullString{String: api.AccessURL.String() + mcpserver.MCPEndpoint, Valid: true},
|
||||
})
|
||||
|
||||
innerDone := make(chan struct{})
|
||||
cancelInner := make(chan context.CancelFunc, 1)
|
||||
root := api.RootHandler
|
||||
api.RootHandler = chi.NewRouter()
|
||||
api.RootHandler.Use(func(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path == fmt.Sprintf("/api/v2/workspacebuilds/%s/logs", build.ID) {
|
||||
defer close(innerDone)
|
||||
assert.Nil(t, r.Context().Value(outerContextKey{}), "outer request values must not reach internal authentication")
|
||||
// Allow cleanup to release the log follower even if the
|
||||
// cancellation regression fails.
|
||||
innerCtx, cancel := context.WithCancel(r.Context())
|
||||
defer cancel()
|
||||
cancelInner <- cancel
|
||||
r = r.WithContext(innerCtx)
|
||||
}
|
||||
next.ServeHTTP(w, r)
|
||||
})
|
||||
})
|
||||
api.RootHandler.Mount("/", root)
|
||||
t.Cleanup(func() {
|
||||
select {
|
||||
case cancel := <-cancelInner:
|
||||
cancel()
|
||||
default:
|
||||
}
|
||||
})
|
||||
|
||||
// Send a raw stateless MCP call so canceling it disconnects the HTTP
|
||||
// request without an SDK cancellation notification masking the leak.
|
||||
body, err := json.Marshal(map[string]any{
|
||||
"jsonrpc": "2.0", "id": 1, "method": "tools/call",
|
||||
"params": map[string]any{
|
||||
"name": toolsdk.ToolNameGetWorkspaceBuildLogs,
|
||||
"arguments": map[string]any{"workspace_build_id": build.ID.String()},
|
||||
},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
requestCtx, cancel := context.WithCancel(ctx)
|
||||
defer cancel()
|
||||
req, err := http.NewRequestWithContext(requestCtx, http.MethodPost, api.AccessURL.String()+mcpserver.MCPEndpoint, strings.NewReader(string(body)))
|
||||
require.NoError(t, err)
|
||||
req.Header.Set("Authorization", "Bearer "+token)
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
req.Header.Set("Accept", "application/json, text/event-stream")
|
||||
req.Header.Set("MCP-Protocol-Version", "2025-03-26")
|
||||
requestDone := make(chan error, 1)
|
||||
go func() {
|
||||
resp, err := coderClient.HTTPClient.Do(req)
|
||||
if resp != nil {
|
||||
_ = resp.Body.Close()
|
||||
}
|
||||
requestDone <- err
|
||||
}()
|
||||
|
||||
// The log query runs after WebSocket acceptance and pubsub subscription.
|
||||
testutil.TryReceive(ctx, t, logsDB.queried)
|
||||
cancel()
|
||||
require.ErrorIs(t, testutil.TryReceive(ctx, t, requestDone), context.Canceled)
|
||||
waitCtx := testutil.Context(t, testutil.WaitShort)
|
||||
testutil.TryReceive(waitCtx, t, innerDone)
|
||||
testutil.TryReceive(waitCtx, t, outerDone)
|
||||
}
|
||||
|
||||
type mcpLogStreamStore struct {
|
||||
database.Store
|
||||
queried chan struct{}
|
||||
}
|
||||
|
||||
func (s *mcpLogStreamStore) GetProvisionerLogsAfterID(ctx context.Context, params database.GetProvisionerLogsAfterIDParams) ([]database.ProvisionerJobLog, error) {
|
||||
logs, err := s.Store.GetProvisionerLogsAfterID(ctx, params)
|
||||
select {
|
||||
case s.queried <- struct{}{}:
|
||||
default:
|
||||
}
|
||||
return logs, err
|
||||
}
|
||||
|
||||
func TestMCPHTTP_E2E_UnauthenticatedAccess(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -198,12 +314,120 @@ func TestMCPHTTP_E2E_UnauthenticatedAccess(t *testing.T) {
|
||||
|
||||
// Verify we get 401 Unauthorized
|
||||
require.Equal(t, http.StatusUnauthorized, resp.StatusCode, "Should get HTTP 401 for unauthenticated access")
|
||||
require.Contains(t, resp.Header.Get("WWW-Authenticate"), "Bearer ")
|
||||
require.Contains(t, resp.Header.Get("WWW-Authenticate"), fmt.Sprintf(`resource_metadata=%q`,
|
||||
api.AccessURL.String()+"/.well-known/oauth-protected-resource"+mcpserver.MCPEndpoint))
|
||||
require.Empty(t, resp.Header.Get("Location"))
|
||||
|
||||
// Also test with MCP client to ensure it handles the error gracefully
|
||||
_, err = newIsolatedMCPClient(ctx, mcpURL, "test-client-unauth", nil)
|
||||
require.Error(t, err, "Should fail during MCP initialization without authentication")
|
||||
}
|
||||
|
||||
func TestMCPHTTP_E2E_ResourcePaths(t *testing.T) {
|
||||
t.Parallel()
|
||||
client := coderdtest.New(t, &coderdtest.Options{DeploymentValues: mcpDeploymentValues(t)})
|
||||
coderdtest.CreateFirstUser(t, client)
|
||||
type credential struct {
|
||||
name string
|
||||
token string
|
||||
wantStatus int
|
||||
}
|
||||
tokens := []credential{
|
||||
{name: "Unauthenticated", wantStatus: http.StatusUnauthorized},
|
||||
{name: "SessionToken", token: client.SessionToken(), wantStatus: http.StatusOK},
|
||||
}
|
||||
httpClient := &http.Client{CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }}
|
||||
for _, resource := range []struct {
|
||||
name string
|
||||
audience string
|
||||
wantStatus int
|
||||
}{
|
||||
{name: "RootAudience", audience: client.URL.String(), wantStatus: http.StatusForbidden},
|
||||
{name: "MCPAudience", audience: client.URL.String() + mcpserver.MCPEndpoint, wantStatus: http.StatusOK},
|
||||
} {
|
||||
app, secret := oauth2providertest.CreateTestOAuth2App(t, client)
|
||||
verifier, challenge := oauth2providertest.GeneratePKCE(t)
|
||||
code := oauth2providertest.AuthorizeOAuth2App(t, client, client.URL.String(), oauth2providertest.AuthorizeParams{
|
||||
ClientID: app.ID.String(), ResponseType: "code", RedirectURI: oauth2providertest.TestRedirectURI,
|
||||
State: oauth2providertest.GenerateState(t), CodeChallenge: challenge, CodeChallengeMethod: "S256",
|
||||
Resource: resource.audience,
|
||||
})
|
||||
token := oauth2providertest.ExchangeCodeForToken(t, client.URL.String(), oauth2providertest.TokenExchangeParams{
|
||||
GrantType: "authorization_code", Code: code, ClientID: app.ID.String(), ClientSecret: secret,
|
||||
RedirectURI: oauth2providertest.TestRedirectURI, CodeVerifier: verifier, Resource: resource.audience,
|
||||
})
|
||||
tokens = append(tokens, credential{name: resource.name, token: token.AccessToken, wantStatus: resource.wantStatus})
|
||||
}
|
||||
for _, path := range []string{
|
||||
"/api//experimental/mcp/http", "/api/experimental//mcp/http",
|
||||
"/api/experimental/mcp//http", "/api/experimental/mcp/http//",
|
||||
"/api/experimental/mcp/http/child", "/api/experimental/mcp/http/../http",
|
||||
"/api/experimental/mcp/%68ttp", "/api/experimental/mcp/http%2f",
|
||||
} {
|
||||
t.Run(path, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
for _, token := range tokens {
|
||||
t.Run(token.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
req, err := http.NewRequestWithContext(t.Context(), http.MethodPost, client.URL.String()+path,
|
||||
strings.NewReader(`{"jsonrpc":"2.0","id":1,"method":"tools/list","params":{}}`))
|
||||
require.NoError(t, err)
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
req.Header.Set("Accept", "application/json, text/event-stream")
|
||||
if token.token != "" {
|
||||
req.Header.Set("Authorization", "Bearer "+token.token)
|
||||
}
|
||||
resp, err := httpClient.Do(req)
|
||||
require.NoError(t, err)
|
||||
_ = resp.Body.Close()
|
||||
require.Equal(t, http.StatusNotFound, resp.StatusCode)
|
||||
require.Empty(t, resp.Header.Get("WWW-Authenticate"))
|
||||
})
|
||||
}
|
||||
})
|
||||
}
|
||||
t.Run("CanonicalPaths", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
for _, path := range []string{mcpserver.MCPEndpoint, mcpserver.MCPEndpoint + "/"} {
|
||||
for _, token := range tokens {
|
||||
t.Run(path+"/"+token.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
req, err := http.NewRequestWithContext(t.Context(), http.MethodPost, client.URL.String()+path,
|
||||
strings.NewReader(`{"jsonrpc":"2.0","id":1,"method":"tools/list","params":{}}`))
|
||||
require.NoError(t, err)
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
req.Header.Set("Accept", "application/json, text/event-stream")
|
||||
if token.token != "" {
|
||||
req.Header.Set("Authorization", "Bearer "+token.token)
|
||||
}
|
||||
resp, err := httpClient.Do(req)
|
||||
require.NoError(t, err)
|
||||
_ = resp.Body.Close()
|
||||
require.Equal(t, token.wantStatus, resp.StatusCode)
|
||||
if token.wantStatus == http.StatusUnauthorized {
|
||||
metadataURL := client.URL.String() + "/.well-known/oauth-protected-resource" + path
|
||||
require.Contains(t, resp.Header.Get("WWW-Authenticate"), fmt.Sprintf(`resource_metadata=%q`, metadataURL))
|
||||
var metadata codersdk.OAuth2ProtectedResourceMetadata
|
||||
testutil.RequireEventuallyResponseOK(testutil.Context(t, testutil.WaitLong), t, metadataURL, &metadata)
|
||||
require.Equal(t, client.URL.String()+path, metadata.Resource)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
})
|
||||
t.Run("CallbackStillRequiresAuthentication", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
req, err := http.NewRequestWithContext(t.Context(), http.MethodGet,
|
||||
client.URL.String()+"/api/experimental/mcp/servers/"+uuid.NewString()+"/oauth2/callback", nil)
|
||||
require.NoError(t, err)
|
||||
resp, err := httpClient.Do(req)
|
||||
require.NoError(t, err)
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusUnauthorized, resp.StatusCode)
|
||||
})
|
||||
}
|
||||
|
||||
func TestMCPHTTP_E2E_ToolWithWorkspace(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -520,6 +744,205 @@ func TestMCPHTTP_E2E_OAuth2_EndToEnd(t *testing.T) {
|
||||
t.Logf("OAuth2 Bearer token MCP test successful: Found %d tools", len(tools.Tools))
|
||||
})
|
||||
|
||||
t.Run("PathQualifiedResource", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
mcpURL := api.AccessURL.String() + mcpserver.MCPEndpoint
|
||||
app, secret := oauth2providertest.CreateTestOAuth2App(t, coderClient)
|
||||
verifier, challenge := oauth2providertest.GeneratePKCE(t)
|
||||
code := oauth2providertest.AuthorizeOAuth2App(t, coderClient, api.AccessURL.String(), oauth2providertest.AuthorizeParams{
|
||||
ClientID: app.ID.String(), ResponseType: "code",
|
||||
RedirectURI: oauth2providertest.TestRedirectURI,
|
||||
State: oauth2providertest.GenerateState(t),
|
||||
CodeChallenge: challenge, CodeChallengeMethod: "S256",
|
||||
Resource: mcpURL,
|
||||
})
|
||||
token := oauth2providertest.ExchangeCodeForToken(t, api.AccessURL.String(), oauth2providertest.TokenExchangeParams{
|
||||
GrantType: "authorization_code", Code: code,
|
||||
ClientID: app.ID.String(), ClientSecret: secret,
|
||||
RedirectURI: oauth2providertest.TestRedirectURI,
|
||||
CodeVerifier: verifier, Resource: mcpURL,
|
||||
})
|
||||
mcpClient, err := newIsolatedMCPClient(ctx, mcpURL, "path-resource-client", map[string]string{
|
||||
"Authorization": "Bearer " + token.AccessToken,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = mcpClient.Close() })
|
||||
require.Equal(t, mcpserver.MCPServerName, mcpClient.InitializeResult().ServerInfo.Name)
|
||||
tools, err := mcpClient.ListTools(ctx, nil)
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, tools.Tools)
|
||||
|
||||
result, err := mcpClient.CallTool(ctx, &mcp.CallToolParams{
|
||||
Name: toolsdk.ToolNameGetAuthenticatedUser,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.False(t, result.IsError, "REST-backed tool failed: %+v", result.Content)
|
||||
require.Len(t, result.Content, 1)
|
||||
text, ok := result.Content[0].(*mcp.TextContent)
|
||||
require.True(t, ok)
|
||||
var user codersdk.User
|
||||
require.NoError(t, json.Unmarshal([]byte(text.Text), &user))
|
||||
me, err := coderClient.User(ctx, codersdk.Me)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, me.ID, user.ID)
|
||||
|
||||
// Exercise SDK URL construction through the delegated transport. A
|
||||
// workspace owner must not select the token-creation endpoint.
|
||||
before, err := coderClient.Tokens(ctx, codersdk.Me, codersdk.TokensFilter{})
|
||||
require.NoError(t, err)
|
||||
for _, injectedUser := range []string{
|
||||
"me/keys/tokens?", "me/keys/tokens#", "me/keys/tokens/../tokens?",
|
||||
"me%2fkeys%2ftokens%3f", "../users/me/keys/tokens?", ".", "..",
|
||||
} {
|
||||
result, err := mcpClient.CallTool(ctx, &mcp.CallToolParams{
|
||||
Name: toolsdk.ToolNameCreateWorkspace,
|
||||
Arguments: map[string]any{
|
||||
"user": injectedUser, "name": "unused",
|
||||
"template_id": uuid.NewString(), "rich_parameters": map[string]string{},
|
||||
},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.True(t, result.IsError, "injected owner %q must fail", injectedUser)
|
||||
require.Len(t, result.Content, 1)
|
||||
text, ok := result.Content[0].(*mcp.TextContent)
|
||||
require.True(t, ok)
|
||||
assert.Contains(t, text.Text, "invalid user")
|
||||
after, err := coderClient.Tokens(ctx, codersdk.Me, codersdk.TokensFilter{})
|
||||
require.NoError(t, err)
|
||||
require.ElementsMatch(t, before, after, "workspace tool must not create API keys")
|
||||
}
|
||||
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, api.AccessURL.String()+"/api/v2/users/me", nil)
|
||||
require.NoError(t, err)
|
||||
req.Header.Set("Authorization", "Bearer "+token.AccessToken)
|
||||
// Neither a claimed origin nor a delegation-looking header may
|
||||
// turn an external REST request into trusted MCP work.
|
||||
req.Header.Set("X-Coder-MCP-Delegation", "true")
|
||||
req.Header.Set("X-Forwarded-Uri", mcpserver.MCPEndpoint)
|
||||
req.Header.Set("Referer", mcpURL)
|
||||
resp, err := http.DefaultClient.Do(req)
|
||||
require.NoError(t, err)
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusForbidden, resp.StatusCode)
|
||||
var response codersdk.Response
|
||||
require.NoError(t, json.NewDecoder(resp.Body).Decode(&response))
|
||||
require.Equal(t, "Token audience validation failed", response.Message)
|
||||
|
||||
refreshed := oauth2providertest.ExchangeCodeForToken(t, api.AccessURL.String(), oauth2providertest.TokenExchangeParams{
|
||||
GrantType: "refresh_token", RefreshToken: token.RefreshToken,
|
||||
ClientID: app.ID.String(), ClientSecret: secret, Resource: mcpURL,
|
||||
})
|
||||
refreshedClient, err := newIsolatedMCPClient(ctx, mcpURL, "refreshed-path-resource", map[string]string{
|
||||
"Authorization": "Bearer " + refreshed.AccessToken,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = refreshedClient.Close() })
|
||||
result, err = refreshedClient.CallTool(ctx, &mcp.CallToolParams{Name: toolsdk.ToolNameGetAuthenticatedUser})
|
||||
require.NoError(t, err)
|
||||
require.False(t, result.IsError)
|
||||
require.NoError(t, coderClient.RevokeOAuth2Token(ctx, app.ID, secret, refreshed.AccessToken))
|
||||
_, err = refreshedClient.CallTool(ctx, &mcp.CallToolParams{Name: toolsdk.ToolNameGetAuthenticatedUser})
|
||||
require.Error(t, err, "revoked token must not execute tools")
|
||||
})
|
||||
|
||||
t.Run("PathQualifiedAuthorization", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
me, err := coderClient.User(ctx, codersdk.Me)
|
||||
require.NoError(t, err)
|
||||
orgID := me.OrganizationIDs[0]
|
||||
memberClient, member := coderdtest.CreateAnotherUser(t, coderClient, orgID)
|
||||
orgAdminClient, orgAdmin := coderdtest.CreateAnotherUser(t, coderClient, orgID, rbac.ScopedRoleOrgAdmin(orgID))
|
||||
otherOrg := dbgen.Organization(t, api.Database, database.Organization{})
|
||||
ownWorkspace := dbfake.WorkspaceBuild(t, api.Database, database.WorkspaceTable{
|
||||
OrganizationID: orgID, OwnerID: member.ID,
|
||||
}).Do().Workspace
|
||||
peerWorkspace := dbfake.WorkspaceBuild(t, api.Database, database.WorkspaceTable{
|
||||
OrganizationID: orgID, OwnerID: me.ID,
|
||||
}).Do().Workspace
|
||||
otherOrgWorkspace := dbfake.WorkspaceBuild(t, api.Database, database.WorkspaceTable{
|
||||
OrganizationID: otherOrg.ID, OwnerID: me.ID,
|
||||
}).Do().Workspace
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
client *codersdk.Client
|
||||
userID uuid.UUID
|
||||
scope string
|
||||
ownAllowed bool
|
||||
peerAllowed bool
|
||||
}{
|
||||
{name: "Member", client: memberClient, userID: member.ID, scope: "coder:all", ownAllowed: true},
|
||||
{name: "OrganizationAdmin", client: orgAdminClient, userID: orgAdmin.ID, scope: "coder:all", ownAllowed: true, peerAllowed: true},
|
||||
{name: "UserReadOnlyScope", client: memberClient, userID: member.ID, scope: "user:read"},
|
||||
{name: "OwnerReadOnlyScope", client: coderClient, userID: me.ID, scope: "user:read"},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
mcpURL := api.AccessURL.String() + mcpserver.MCPEndpoint
|
||||
app, secret := oauth2providertest.CreateTestOAuth2App(t, coderClient)
|
||||
verifier, challenge := oauth2providertest.GeneratePKCE(t)
|
||||
code := oauth2providertest.AuthorizeOAuth2App(t, tc.client, api.AccessURL.String(), oauth2providertest.AuthorizeParams{
|
||||
ClientID: app.ID.String(), ResponseType: "code",
|
||||
RedirectURI: oauth2providertest.TestRedirectURI, State: oauth2providertest.GenerateState(t),
|
||||
CodeChallenge: challenge, CodeChallengeMethod: "S256", Resource: mcpURL, Scope: tc.scope,
|
||||
})
|
||||
token := oauth2providertest.ExchangeCodeForToken(t, api.AccessURL.String(), oauth2providertest.TokenExchangeParams{
|
||||
GrantType: "authorization_code", Code: code, ClientID: app.ID.String(), ClientSecret: secret,
|
||||
RedirectURI: oauth2providertest.TestRedirectURI, CodeVerifier: verifier, Resource: mcpURL,
|
||||
})
|
||||
client, err := newIsolatedMCPClient(ctx, mcpURL, tc.name, map[string]string{
|
||||
"Authorization": "Bearer " + token.AccessToken,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = client.Close() })
|
||||
result, err := client.CallTool(ctx, &mcp.CallToolParams{Name: toolsdk.ToolNameGetAuthenticatedUser})
|
||||
require.NoError(t, err)
|
||||
require.False(t, result.IsError)
|
||||
var user codersdk.User
|
||||
require.NoError(t, json.Unmarshal([]byte(result.Content[0].(*mcp.TextContent).Text), &user))
|
||||
require.Equal(t, tc.userID, user.ID)
|
||||
|
||||
if tc.scope == "user:read" {
|
||||
visible, err := tc.client.Workspaces(ctx, codersdk.WorkspaceFilter{Owner: "me"})
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, visible.Workspaces, "unrestricted credential must see fixture workspaces")
|
||||
result, err := client.CallTool(ctx, &mcp.CallToolParams{Name: toolsdk.ToolNameListWorkspaces})
|
||||
require.NoError(t, err)
|
||||
require.False(t, result.IsError)
|
||||
var workspaces []toolsdk.MinimalWorkspace
|
||||
require.NoError(t, json.Unmarshal([]byte(result.Content[0].(*mcp.TextContent).Text), &workspaces))
|
||||
require.Empty(t, workspaces, "OAuth scope must filter workspace visibility even for an Owner")
|
||||
}
|
||||
|
||||
for _, ws := range []struct {
|
||||
id uuid.UUID
|
||||
allowed bool
|
||||
}{
|
||||
{id: ownWorkspace.ID, allowed: tc.ownAllowed},
|
||||
{id: peerWorkspace.ID, allowed: tc.peerAllowed},
|
||||
{id: otherOrgWorkspace.ID},
|
||||
} {
|
||||
result, err := client.CallTool(ctx, &mcp.CallToolParams{
|
||||
Name: toolsdk.ToolNameGetWorkspace, Arguments: map[string]any{"workspace_id": ws.id.String()},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, !ws.allowed, result.IsError)
|
||||
text := result.Content[0].(*mcp.TextContent).Text
|
||||
if ws.allowed {
|
||||
var workspace codersdk.Workspace
|
||||
require.NoError(t, json.Unmarshal([]byte(text), &workspace))
|
||||
require.Equal(t, ws.id, workspace.ID)
|
||||
} else {
|
||||
require.Contains(t, text, "404", "must fail RBAC, not audience validation")
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
// Test 3: Full OAuth2 Authorization Code Flow with Token Refresh
|
||||
t.Run("OAuth2FullFlowWithTokenRefresh", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
+10
-2
@@ -39,9 +39,17 @@ func (api *API) mcpHTTPHandler() http.Handler {
|
||||
})
|
||||
return
|
||||
}
|
||||
// Extract the original session token from the request
|
||||
// Keep tool requests in-process. The private transport carries the
|
||||
// originating MCP audience without granting external REST access.
|
||||
transport, closeTransport := newMCPDelegatedTransport(api.Logger, api.RootHandler, api.AccessURL, httpmw.APIKey(r).ID)
|
||||
defer closeTransport()
|
||||
// The MCP SDK detaches tool cancellation and waits for in-flight tools
|
||||
// before returning, so close their transport independently on disconnect.
|
||||
stopClose := context.AfterFunc(r.Context(), closeTransport)
|
||||
defer stopClose()
|
||||
authenticatedClient := codersdk.New(api.AccessURL,
|
||||
codersdk.WithSessionToken(httpmw.APITokenFromRequest(r)))
|
||||
codersdk.WithSessionToken(httpmw.APITokenFromRequest(r)),
|
||||
codersdk.WithHTTPClient(&http.Client{Transport: transport}))
|
||||
|
||||
// Wrap the agent connection function to enforce ActionSSH
|
||||
// on the workspace. Without this check, a user who can read
|
||||
|
||||
@@ -0,0 +1,138 @@
|
||||
package coderd
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"path"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"cdr.dev/slog/v3"
|
||||
"github.com/coder/coder/v2/coderd/httpmw"
|
||||
"github.com/coder/coder/v2/coderd/mcp"
|
||||
)
|
||||
|
||||
// mcpDelegatedReadHeaderTimeout bounds header reads on the in-process delegation
|
||||
// server to satisfy gosec G112. The only client is the local net.Pipe transport,
|
||||
// so this is not tuned for a slow network client.
|
||||
const mcpDelegatedReadHeaderTimeout = 10 * time.Second
|
||||
|
||||
// newMCPDelegatedTransport delegates same-origin REST requests over net.Pipe,
|
||||
// preserving streaming and WebSockets without inheriting caller context values.
|
||||
// Middleware revalidates the original credential, binds it to apiKeyID, and
|
||||
// checks its MCP audience. Call cleanup to release the server and connections.
|
||||
func newMCPDelegatedTransport(logger slog.Logger, handler http.Handler, accessURL *url.URL, apiKeyID string) (http.RoundTripper, func()) {
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
listener := &mcpListener{ctx: ctx, cancel: cancel, connections: make(chan net.Conn)}
|
||||
server := &http.Server{
|
||||
Handler: handler,
|
||||
ReadHeaderTimeout: mcpDelegatedReadHeaderTimeout,
|
||||
// Pipe connections carry no TLS, so net/http errors here are handler
|
||||
// faults; route them to the Coder logger instead of stderr.
|
||||
ErrorLog: slog.Stdlib(ctx, logger.Named("mcp_delegated_transport"), slog.LevelError),
|
||||
BaseContext: func(net.Listener) context.Context {
|
||||
return httpmw.WithAPIKeyDelegation(ctx, accessURL.JoinPath(mcp.MCPEndpoint).String(), apiKeyID)
|
||||
},
|
||||
}
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
defer close(done)
|
||||
_ = server.Serve(listener)
|
||||
}()
|
||||
transport := &http.Transport{DialContext: listener.dialContext}
|
||||
return &mcpDelegatedTransport{origin: *accessURL, transport: transport}, sync.OnceFunc(func() {
|
||||
cancel()
|
||||
transport.CloseIdleConnections()
|
||||
_ = server.Close()
|
||||
<-done
|
||||
})
|
||||
}
|
||||
|
||||
type mcpDelegatedTransport struct {
|
||||
origin url.URL
|
||||
transport *http.Transport
|
||||
}
|
||||
|
||||
func (t *mcpDelegatedTransport) RoundTrip(req *http.Request) (*http.Response, error) {
|
||||
reject := func(message string) (*http.Response, error) {
|
||||
if req.Body != nil {
|
||||
_ = req.Body.Close()
|
||||
}
|
||||
return nil, xerrors.New(message)
|
||||
}
|
||||
// Reject other origins, including redirect targets, so the bearer token
|
||||
// never leaves the process.
|
||||
if req.URL.Scheme != t.origin.Scheme || req.URL.Host != t.origin.Host || req.Host != t.origin.Host || req.URL.User != nil {
|
||||
return reject("MCP tool request must target the Coder deployment")
|
||||
}
|
||||
if err := validateMCPDelegatedPath(req.URL.Path); err != nil {
|
||||
return reject(err.Error())
|
||||
}
|
||||
clone := req.Clone(req.Context())
|
||||
// Even for HTTPS deployments, these bytes only travel over net.Pipe.
|
||||
clone.URL.Scheme = "http"
|
||||
return t.transport.RoundTrip(clone)
|
||||
}
|
||||
|
||||
func validateMCPDelegatedPath(requestPath string) error {
|
||||
// Reject aliases before routing can normalize them into another endpoint.
|
||||
if path.Clean(requestPath) != strings.TrimSuffix(requestPath, "/") {
|
||||
return xerrors.New("MCP tool request must use a canonical REST path")
|
||||
}
|
||||
if (!strings.HasPrefix(requestPath, "/api/v2/") && !strings.HasPrefix(requestPath, "/api/experimental/")) ||
|
||||
strings.HasPrefix(requestPath, "/api/experimental/mcp/") {
|
||||
return xerrors.New("MCP tool request must target an internal REST endpoint")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
type mcpListener struct {
|
||||
ctx context.Context
|
||||
cancel context.CancelFunc
|
||||
connections chan net.Conn
|
||||
}
|
||||
|
||||
func (l *mcpListener) dialContext(ctx context.Context, _, _ string) (net.Conn, error) {
|
||||
client, server := net.Pipe()
|
||||
// Closing the MCP transport also closes hijacked WebSocket connections,
|
||||
// which http.Server.Close does not own after the upgrade.
|
||||
context.AfterFunc(l.ctx, func() {
|
||||
_ = client.Close()
|
||||
_ = server.Close()
|
||||
})
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
_ = client.Close()
|
||||
_ = server.Close()
|
||||
return nil, ctx.Err()
|
||||
case <-l.ctx.Done():
|
||||
_ = client.Close()
|
||||
_ = server.Close()
|
||||
return nil, net.ErrClosed
|
||||
case l.connections <- server:
|
||||
return client, nil
|
||||
}
|
||||
}
|
||||
|
||||
func (l *mcpListener) Accept() (net.Conn, error) {
|
||||
select {
|
||||
case <-l.ctx.Done():
|
||||
return nil, net.ErrClosed
|
||||
case conn := <-l.connections:
|
||||
return conn, nil
|
||||
}
|
||||
}
|
||||
|
||||
func (l *mcpListener) Close() error {
|
||||
l.cancel()
|
||||
return nil
|
||||
}
|
||||
|
||||
func (*mcpListener) Addr() net.Addr {
|
||||
return &net.TCPAddr{IP: net.IPv4(127, 0, 0, 1)}
|
||||
}
|
||||
@@ -0,0 +1,149 @@
|
||||
package coderd
|
||||
|
||||
import (
|
||||
"context"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"runtime"
|
||||
"sync"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
)
|
||||
|
||||
// BenchmarkMCPDelegatedTransport isolates transport overhead with a trivial
|
||||
// handler, excluding MCP, authentication, database work, and request creation.
|
||||
func BenchmarkMCPDelegatedTransport(b *testing.B) {
|
||||
origin := &url.URL{Scheme: "https", Host: "coder.example.com"}
|
||||
req, err := http.NewRequestWithContext(b.Context(), http.MethodGet, origin.String()+"/api/v2/users/me", nil)
|
||||
require.NoError(b, err)
|
||||
logger := testutil.Logger(b)
|
||||
handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
})
|
||||
|
||||
b.Run("Reuse", func(b *testing.B) {
|
||||
transport, closeTransport := newMCPDelegatedTransport(logger, handler, origin, "benchmark-key")
|
||||
defer closeTransport()
|
||||
benchmarkMCPNoContent(b, transport, req)
|
||||
b.ReportAllocs()
|
||||
for b.Loop() {
|
||||
benchmarkMCPNoContent(b, transport, req)
|
||||
}
|
||||
})
|
||||
b.Run("SetupTeardown", func(b *testing.B) {
|
||||
b.ReportAllocs()
|
||||
for b.Loop() {
|
||||
_, closeTransport := newMCPDelegatedTransport(logger, handler, origin, "benchmark-key")
|
||||
closeTransport()
|
||||
}
|
||||
})
|
||||
b.Run("SetupFirstCallTeardown", func(b *testing.B) {
|
||||
b.ReportAllocs()
|
||||
for b.Loop() {
|
||||
func() {
|
||||
transport, closeTransport := newMCPDelegatedTransport(logger, handler, origin, "benchmark-key")
|
||||
defer closeTransport()
|
||||
benchmarkMCPNoContent(b, transport, req)
|
||||
}()
|
||||
}
|
||||
})
|
||||
b.Run("BufferedRecorderBaseline", func(b *testing.B) {
|
||||
// This direct buffered call is not production-equivalent: it has no
|
||||
// streaming, hijacking, HTTP serialization, or delegation isolation.
|
||||
b.ReportAllocs()
|
||||
for b.Loop() {
|
||||
recorder := httptest.NewRecorder()
|
||||
handler.ServeHTTP(recorder, req)
|
||||
resp := recorder.Result()
|
||||
if err := resp.Body.Close(); err != nil {
|
||||
b.Fatal(err)
|
||||
}
|
||||
if resp.StatusCode != http.StatusNoContent {
|
||||
b.Fatalf("unexpected status: %d", resp.StatusCode)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func benchmarkMCPNoContent(b *testing.B, transport http.RoundTripper, req *http.Request) {
|
||||
resp, err := transport.RoundTrip(req)
|
||||
if err != nil {
|
||||
b.Fatal(err)
|
||||
}
|
||||
if err := resp.Body.Close(); err != nil {
|
||||
b.Fatal(err)
|
||||
}
|
||||
if resp.StatusCode != http.StatusNoContent {
|
||||
b.Fatalf("unexpected status: %d", resp.StatusCode)
|
||||
}
|
||||
}
|
||||
|
||||
// BenchmarkMCPDelegatedTransportActiveStreams measures one cohort per iteration,
|
||||
// not per-request latency. Run standalone with -run '^$' -benchtime=1x so global
|
||||
// goroutine counts are not obscured by other tests. Each stream owns a transport,
|
||||
// modeling separate outer MCP requests rather than one shared connection pool.
|
||||
func BenchmarkMCPDelegatedTransportActiveStreams(b *testing.B) {
|
||||
for b.Loop() {
|
||||
benchmarkMCPActiveStreamCohort(b)
|
||||
}
|
||||
}
|
||||
|
||||
func benchmarkMCPActiveStreamCohort(b *testing.B) {
|
||||
const streams = 1024
|
||||
ctx, cancel := context.WithTimeout(b.Context(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
origin := &url.URL{Scheme: "https", Host: "coder.example.com"}
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, origin.String()+"/api/v2/logs", nil)
|
||||
require.NoError(b, err)
|
||||
logger := testutil.Logger(b)
|
||||
baseline := runtime.NumGoroutine()
|
||||
completed := make(chan struct{}, streams)
|
||||
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
defer func() { completed <- struct{}{} }()
|
||||
w.Header().Set("Content-Type", "text/event-stream")
|
||||
_, _ = io.WriteString(w, "data: ready\n\n")
|
||||
w.(http.Flusher).Flush()
|
||||
<-r.Context().Done()
|
||||
})
|
||||
closers := make([]func(), 0, streams)
|
||||
cleanup := sync.OnceFunc(func() {
|
||||
for _, closeTransport := range closers {
|
||||
closeTransport()
|
||||
}
|
||||
})
|
||||
defer cleanup()
|
||||
for range streams {
|
||||
transport, closeTransport := newMCPDelegatedTransport(logger, handler, origin, "benchmark-key")
|
||||
closers = append(closers, closeTransport)
|
||||
//nolint:bodyclose // Bodies are closed with their transports in cleanup.
|
||||
resp, err := transport.RoundTrip(req)
|
||||
require.NoError(b, err)
|
||||
closers[len(closers)-1] = func() {
|
||||
closeTransport()
|
||||
_ = resp.Body.Close()
|
||||
}
|
||||
require.Equal(b, http.StatusOK, resp.StatusCode)
|
||||
var first [len("data: ready\n\n")]byte
|
||||
_, err = io.ReadFull(resp.Body, first[:])
|
||||
require.NoError(b, err)
|
||||
}
|
||||
active := runtime.NumGoroutine()
|
||||
cleanup()
|
||||
for range streams {
|
||||
testutil.TryReceive(ctx, b, completed)
|
||||
}
|
||||
// Connection goroutines can exit after Server.Close returns. Report rather
|
||||
// than assert the process-wide baseline, which unrelated work can change.
|
||||
for runtime.NumGoroutine() > baseline && ctx.Err() == nil {
|
||||
runtime.Gosched()
|
||||
}
|
||||
b.ReportMetric(float64(streams), "streams")
|
||||
b.ReportMetric(float64(baseline), "goroutines-baseline")
|
||||
b.ReportMetric(float64(active), "goroutines-active")
|
||||
b.ReportMetric(float64(runtime.NumGoroutine()), "goroutines-cleaned")
|
||||
}
|
||||
@@ -0,0 +1,227 @@
|
||||
package coderd
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/database"
|
||||
"github.com/coder/coder/v2/coderd/database/dbgen"
|
||||
"github.com/coder/coder/v2/coderd/database/dbtestutil"
|
||||
"github.com/coder/coder/v2/coderd/database/dbtime"
|
||||
"github.com/coder/coder/v2/coderd/httpmw"
|
||||
"github.com/coder/coder/v2/coderd/mcp"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
"github.com/coder/websocket"
|
||||
)
|
||||
|
||||
func TestMCPTransportAuthentication(t *testing.T) {
|
||||
t.Parallel()
|
||||
db, _ := dbtestutil.NewDB(t)
|
||||
user := dbgen.User(t, db, database.User{})
|
||||
app := dbgen.OAuth2ProviderApp(t, db, database.OAuth2ProviderApp{})
|
||||
origin, err := url.Parse("https://coder.example.com")
|
||||
require.NoError(t, err)
|
||||
handler := httpmw.PrecheckAPIKey(httpmw.ValidateAPIKeyConfig{DB: db})(
|
||||
httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{DB: db, AccessURL: origin, Logger: testutil.Logger(t)})(
|
||||
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
assert.Equal(t, user.ID.String(), httpmw.UserAuthorization(r.Context()).ID)
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
})))
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
audience string
|
||||
status int
|
||||
expired bool
|
||||
deleted bool
|
||||
wrongKeyID bool
|
||||
}{
|
||||
{name: "MCPAudience", audience: origin.String() + mcp.MCPEndpoint, status: http.StatusNoContent},
|
||||
{name: "TrailingSlash", audience: origin.String() + mcp.MCPEndpoint + "/", status: http.StatusNoContent},
|
||||
{name: "UnboundLegacyToken", status: http.StatusNoContent},
|
||||
{name: "RootAudience", audience: origin.String(), status: http.StatusForbidden},
|
||||
{name: "OtherDeployment", audience: "https://other.example.com" + mcp.MCPEndpoint, status: http.StatusForbidden},
|
||||
{name: "WrongKey", audience: origin.String() + mcp.MCPEndpoint, wrongKeyID: true, status: http.StatusUnauthorized},
|
||||
{name: "Expired", audience: origin.String() + mcp.MCPEndpoint, expired: true, status: http.StatusUnauthorized},
|
||||
{name: "Deleted", audience: origin.String() + mcp.MCPEndpoint, deleted: true, status: http.StatusUnauthorized},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
key, token := dbgen.APIKey(t, db, database.APIKey{UserID: user.ID, LoginType: database.LoginTypeOAuth2ProviderApp})
|
||||
dbgen.OAuth2ProviderAppToken(t, db, database.OAuth2ProviderAppToken{
|
||||
AppID: app.ID, UserID: user.ID, APIKeyID: key.ID, HashPrefix: []byte(key.ID),
|
||||
Audience: sql.NullString{String: tc.audience, Valid: tc.audience != ""},
|
||||
})
|
||||
delegatedID := key.ID
|
||||
if tc.wrongKeyID {
|
||||
delegatedID = "other-key"
|
||||
}
|
||||
transport, closeTransport := newMCPDelegatedTransport(testutil.Logger(t), handler, origin, delegatedID)
|
||||
t.Cleanup(closeTransport)
|
||||
client := codersdk.New(origin, codersdk.WithSessionToken(token), codersdk.WithHTTPClient(&http.Client{Transport: transport}))
|
||||
|
||||
if tc.expired || tc.deleted {
|
||||
// Validate once before mutation, then reuse the same private
|
||||
// transport. Internal calls must not cache authentication.
|
||||
resp, err := client.Request(ctx, http.MethodGet, "/api/v2/users/me", nil)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, resp.Body.Close())
|
||||
require.Equal(t, http.StatusNoContent, resp.StatusCode)
|
||||
if tc.deleted {
|
||||
require.NoError(t, db.DeleteAPIKeyByID(ctx, key.ID))
|
||||
} else {
|
||||
require.NoError(t, db.UpdateAPIKeyByID(ctx, database.UpdateAPIKeyByIDParams{
|
||||
ID: key.ID, LastUsed: key.LastUsed, ExpiresAt: dbtime.Now().Add(-time.Hour), IPAddress: key.IPAddress,
|
||||
}))
|
||||
}
|
||||
}
|
||||
resp, err := client.Request(ctx, http.MethodGet, "/api/v2/users/me", nil)
|
||||
require.NoError(t, err)
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, tc.status, resp.StatusCode)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestMCPTransportIsolation(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
origin, err := url.Parse("https://coder.example.com")
|
||||
require.NoError(t, err)
|
||||
type callerContextKey struct{}
|
||||
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
assert.Nil(t, r.Context().Value(callerContextKey{}), "caller auth and routing state must not cross the connection")
|
||||
if r.URL.Path == "/api/v2/redirect" {
|
||||
http.Redirect(w, r, "https://other.example.com/api/v2/users/me", http.StatusFound)
|
||||
return
|
||||
}
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
})
|
||||
transport, closeTransport := newMCPDelegatedTransport(testutil.Logger(t), handler, origin, "test-key")
|
||||
t.Cleanup(closeTransport)
|
||||
client := &http.Client{Transport: transport}
|
||||
|
||||
for _, target := range []string{
|
||||
"https://other.example.com/api/v2/users/me",
|
||||
"http://coder.example.com/api/v2/users/me",
|
||||
"https://coder.example.com/oauth2/tokens",
|
||||
"https://coder.example.com/api/experimental/mcp/http",
|
||||
"https://coder.example.com/api/experimental//mcp/http",
|
||||
"https://coder.example.com/api/v2/../../oauth2/tokens",
|
||||
"https://coder.example.com/api/v2/redirect",
|
||||
} {
|
||||
body := &mcpRequestBody{Reader: strings.NewReader("test")}
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodPost, target, body)
|
||||
require.NoError(t, err)
|
||||
resp, err := client.Do(req)
|
||||
if resp != nil {
|
||||
_ = resp.Body.Close()
|
||||
}
|
||||
require.Error(t, err, "untrusted destination must not be reachable")
|
||||
require.True(t, body.closed.Load(), "rejected request body must be closed")
|
||||
}
|
||||
req, err := http.NewRequestWithContext(context.WithValue(ctx, callerContextKey{}, "outer-auth"), http.MethodGet, origin.String()+"/api/v2/users/me", nil)
|
||||
require.NoError(t, err)
|
||||
resp, err := client.Do(req)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, resp.Body.Close())
|
||||
require.Equal(t, http.StatusNoContent, resp.StatusCode)
|
||||
|
||||
closeTransport()
|
||||
resp, err = client.Do(req)
|
||||
if resp != nil {
|
||||
_ = resp.Body.Close()
|
||||
}
|
||||
require.Error(t, err, "transport must not outlive its MCP request")
|
||||
}
|
||||
|
||||
type mcpRequestBody struct {
|
||||
io.Reader
|
||||
closed atomic.Bool
|
||||
}
|
||||
|
||||
func (b *mcpRequestBody) Close() error {
|
||||
b.closed.Store(true)
|
||||
return nil
|
||||
}
|
||||
|
||||
func TestMCPTransportStreaming(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
origin, err := url.Parse("https://coder.example.com")
|
||||
require.NoError(t, err)
|
||||
handlerDone := make(chan struct{})
|
||||
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
defer close(handlerDone)
|
||||
_, _ = io.WriteString(w, "first\n")
|
||||
w.(http.Flusher).Flush()
|
||||
<-r.Context().Done()
|
||||
})
|
||||
transport, closeTransport := newMCPDelegatedTransport(testutil.Logger(t), handler, origin, "test-key")
|
||||
t.Cleanup(closeTransport)
|
||||
client := &http.Client{Transport: transport}
|
||||
requestCtx, cancel := context.WithCancel(ctx)
|
||||
defer cancel()
|
||||
req, err := http.NewRequestWithContext(requestCtx, http.MethodGet, origin.String()+"/api/v2/logs", nil)
|
||||
require.NoError(t, err)
|
||||
resp, err := client.Do(req)
|
||||
require.NoError(t, err)
|
||||
defer resp.Body.Close()
|
||||
first := make([]byte, len("first\n"))
|
||||
_, err = io.ReadFull(resp.Body, first)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "first\n", string(first))
|
||||
cancel()
|
||||
_, err = resp.Body.Read(first)
|
||||
require.Error(t, err)
|
||||
testutil.TryReceive(ctx, t, handlerDone)
|
||||
}
|
||||
|
||||
func TestMCPTransportWebSocket(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
origin, err := url.Parse("https://coder.example.com")
|
||||
require.NoError(t, err)
|
||||
handlerDone := make(chan struct{})
|
||||
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
defer close(handlerDone)
|
||||
conn, err := websocket.Accept(w, r, nil)
|
||||
if !assert.NoError(t, err) {
|
||||
return
|
||||
}
|
||||
defer conn.CloseNow()
|
||||
kind, data, err := conn.Read(ctx)
|
||||
if !assert.NoError(t, err) {
|
||||
return
|
||||
}
|
||||
assert.NoError(t, conn.Write(ctx, kind, data))
|
||||
_, _, err = conn.Read(ctx)
|
||||
assert.Error(t, err, "transport cleanup must close hijacked connections")
|
||||
})
|
||||
transport, closeTransport := newMCPDelegatedTransport(testutil.Logger(t), handler, origin, "test-key")
|
||||
t.Cleanup(closeTransport)
|
||||
conn, resp, err := websocket.Dial(ctx, "wss://coder.example.com/api/v2/chats/watch", &websocket.DialOptions{HTTPClient: &http.Client{Transport: transport}})
|
||||
if resp != nil && resp.Body != nil {
|
||||
defer resp.Body.Close()
|
||||
}
|
||||
require.NoError(t, err)
|
||||
defer conn.CloseNow()
|
||||
require.NoError(t, conn.Write(ctx, websocket.MessageText, []byte("ping")))
|
||||
_, data, err := conn.Read(ctx)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "ping", string(data))
|
||||
closeTransport()
|
||||
testutil.TryReceive(ctx, t, handlerDone)
|
||||
}
|
||||
@@ -3,6 +3,7 @@ package oauth2provider
|
||||
import (
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/database"
|
||||
"github.com/coder/coder/v2/coderd/database/dbauthz"
|
||||
@@ -51,8 +52,20 @@ func GetAuthorizationServerMetadata(db database.Store, accessURL *url.URL) http.
|
||||
func GetProtectedResourceMetadata(accessURL *url.URL) http.HandlerFunc {
|
||||
return func(rw http.ResponseWriter, r *http.Request) {
|
||||
ctx := r.Context()
|
||||
resource := accessURL.String()
|
||||
suffix := strings.TrimPrefix(r.URL.Path, "/.well-known/oauth-protected-resource")
|
||||
switch suffix {
|
||||
case "", "/":
|
||||
case codersdk.MCPEndpoint, codersdk.MCPEndpoint + "/":
|
||||
resource = accessURL.JoinPath(suffix).String()
|
||||
default:
|
||||
// Match before joining paths so malformed or unknown resources
|
||||
// cannot be normalized into a supported resource (RFC 9728 §3.1).
|
||||
httpapi.RouteNotFound(rw)
|
||||
return
|
||||
}
|
||||
metadata := codersdk.OAuth2ProtectedResourceMetadata{
|
||||
Resource: accessURL.String(),
|
||||
Resource: resource,
|
||||
AuthorizationServers: []string{accessURL.String()},
|
||||
ScopesSupported: rbac.ExternalScopeNames(),
|
||||
// RFC 6750 Bearer Token methods supported as fallback methods in api key middleware
|
||||
|
||||
@@ -119,6 +119,31 @@ func TestGetAuthorizationServerMetadata_DCREnabled(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestOAuth2ProtectedResourceMetadataUnknownPath(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
for _, suffix := range []string{
|
||||
"/unknown", "/api/v2/users/me", "/api/experimental/mcp/http/unknown",
|
||||
"/api/experimental/mcp/http-extra", "/../api/experimental/mcp/http",
|
||||
"/api/experimental/mcp/../mcp/http", "/api//experimental/mcp/http",
|
||||
"//api/experimental/mcp/http", "/api/experimental/mcp/http//",
|
||||
"/api/experimental/mcp/%2e%2e/mcp/http", "/%ff",
|
||||
} {
|
||||
t.Run(suffix, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet,
|
||||
client.URL.String()+"/.well-known/oauth-protected-resource"+suffix, nil)
|
||||
require.NoError(t, err)
|
||||
resp, err := client.HTTPClient.Do(req)
|
||||
require.NoError(t, err)
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusNotFound, resp.StatusCode)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestOAuth2ProtectedResourceMetadata(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -135,10 +160,17 @@ func TestOAuth2ProtectedResourceMetadata(t *testing.T) {
|
||||
testutil.RequireEventuallyResponseOK(ctx, t, endpoint, &metadata)
|
||||
|
||||
// Verify the metadata
|
||||
require.NotEmpty(t, metadata.Resource)
|
||||
require.Equal(t, serverURL.String(), metadata.Resource)
|
||||
require.NotEmpty(t, metadata.AuthorizationServers)
|
||||
require.Len(t, metadata.AuthorizationServers, 1)
|
||||
require.Equal(t, metadata.Resource, metadata.AuthorizationServers[0])
|
||||
|
||||
for _, suffix := range []string{"/api/experimental/mcp/http", "/api/experimental/mcp/http/"} {
|
||||
var pathMetadata codersdk.OAuth2ProtectedResourceMetadata
|
||||
testutil.RequireEventuallyResponseOK(ctx, t, endpoint+suffix, &pathMetadata)
|
||||
require.Equal(t, serverURL.String()+suffix, pathMetadata.Resource)
|
||||
require.Equal(t, []string{serverURL.String()}, pathMetadata.AuthorizationServers)
|
||||
}
|
||||
// RFC 6750 bearer tokens are now supported as fallback methods
|
||||
require.Contains(t, metadata.BearerMethodsSupported, "header")
|
||||
require.Contains(t, metadata.BearerMethodsSupported, "query")
|
||||
|
||||
@@ -168,7 +168,7 @@ func TestOAuth2RateLimit(t *testing.T) {
|
||||
requireLimited(t, func() *http.Response {
|
||||
uri := baseURL + "/.well-known/oauth-protected-resource/" + uuid.NewString()
|
||||
return doRequest(ctx, t, http.MethodGet, uri, nil)
|
||||
}, http.StatusOK)
|
||||
}, http.StatusNotFound)
|
||||
})
|
||||
|
||||
t.Run("SeparateBucketPerEndpoint", func(t *testing.T) {
|
||||
|
||||
@@ -9,6 +9,9 @@ import (
|
||||
"github.com/google/uuid"
|
||||
)
|
||||
|
||||
// MCPEndpoint is the remote MCP HTTP endpoint path.
|
||||
const MCPEndpoint = "/api/experimental/mcp/http"
|
||||
|
||||
// MCPServerOAuth2ConnectURL returns the URL the user should visit to
|
||||
// start the OAuth2 flow for an MCP server. The frontend opens this
|
||||
// in a new window/popup.
|
||||
|
||||
@@ -674,6 +674,10 @@ func (c *Client) CreateWorkspace(ctx context.Context, _ uuid.UUID, user string,
|
||||
|
||||
// CreateUserWorkspace creates a new workspace for the template specified.
|
||||
func (c *Client) CreateUserWorkspace(ctx context.Context, user string, request CreateWorkspaceRequest) (Workspace, error) {
|
||||
// Validate before URL resolution can interpret user input as a path or query.
|
||||
if err := validateWorkspacePathSegment(user); err != nil {
|
||||
return Workspace{}, xerrors.Errorf("invalid user: %w", err)
|
||||
}
|
||||
res, err := c.Request(ctx, http.MethodPost, fmt.Sprintf("/api/v2/users/%s/workspaces", user), request)
|
||||
if err != nil {
|
||||
return Workspace{}, err
|
||||
|
||||
@@ -1135,15 +1135,23 @@ func TestTools(t *testing.T) {
|
||||
tb, err := toolsdk.NewDeps(client)
|
||||
require.NoError(t, err)
|
||||
t.Run("WithoutPreset", func(t *testing.T) {
|
||||
res, err := testTool(t, toolsdk.CreateWorkspace, tb, toolsdk.CreateWorkspaceArgs{
|
||||
User: "me",
|
||||
TemplateVersionID: r.TemplateVersion.ID.String(),
|
||||
Name: testutil.GetRandomNameHyphenated(t),
|
||||
RichParameters: map[string]string{},
|
||||
})
|
||||
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, res.ID, "expected a workspace ID")
|
||||
for _, user := range []string{"", codersdk.Me, member.Username, member.ID.String()} {
|
||||
t.Run(user, func(t *testing.T) {
|
||||
res, err := testTool(t, toolsdk.CreateWorkspace, tb, toolsdk.CreateWorkspaceArgs{
|
||||
User: user,
|
||||
TemplateVersionID: r.TemplateVersion.ID.String(),
|
||||
Name: testutil.GetRandomNameHyphenated(t),
|
||||
RichParameters: map[string]string{},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.NotEqual(t, uuid.Nil, res.ID)
|
||||
if user == member.Username || user == member.ID.String() {
|
||||
require.Equal(t, member.ID, res.OwnerID)
|
||||
} else {
|
||||
require.Equal(t, owner.UserID, res.OwnerID)
|
||||
}
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("WithPreset", func(t *testing.T) {
|
||||
|
||||
@@ -651,6 +651,13 @@ func (c *Client) Workspaces(ctx context.Context, filter WorkspaceFilter) (Worksp
|
||||
|
||||
// WorkspaceByOwnerAndName returns a workspace by the owner's UUID and the workspace's name.
|
||||
func (c *Client) WorkspaceByOwnerAndName(ctx context.Context, owner string, name string, params WorkspaceOptions) (Workspace, error) {
|
||||
// These identifiers must remain individual path segments during URL resolution.
|
||||
if err := validateWorkspacePathSegment(owner); err != nil {
|
||||
return Workspace{}, xerrors.Errorf("invalid workspace owner: %w", err)
|
||||
}
|
||||
if err := validateWorkspacePathSegment(name); err != nil {
|
||||
return Workspace{}, xerrors.Errorf("invalid workspace name: %w", err)
|
||||
}
|
||||
res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/users/%s/workspace/%s", owner, name), nil, func(r *http.Request) {
|
||||
q := r.URL.Query()
|
||||
q.Set("include_deleted", fmt.Sprintf("%t", params.IncludeDeleted))
|
||||
@@ -669,6 +676,15 @@ func (c *Client) WorkspaceByOwnerAndName(ctx context.Context, owner string, name
|
||||
return workspace, ReadBodyAsJSON(res, &workspace)
|
||||
}
|
||||
|
||||
// validateWorkspacePathSegment prevents workspace and user identifiers from
|
||||
// changing the endpoint during URL resolution, without restricting legacy names.
|
||||
func validateWorkspacePathSegment(identifier string) error {
|
||||
if identifier == "" || identifier == "." || identifier == ".." || strings.ContainsAny(identifier, `/\%?#`) {
|
||||
return xerrors.New("must be a non-empty path segment without slashes, escapes, query, fragment, or dot segments")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// SplitWorkspaceIdentifier splits an identifier into owner and
|
||||
// workspace name. A bare name defaults the owner to Me ("me"). An
|
||||
// "owner/name" pair is accepted, and identifiers with more than one
|
||||
|
||||
+68
-15
@@ -19,6 +19,53 @@ import (
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
)
|
||||
|
||||
func TestWorkspacePathSegments(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
for _, identifier := range []string{
|
||||
codersdk.Me, "alice", "legacy_name", uuid.NewString(),
|
||||
"me/keys/tokens?", "me/keys/tokens#", "../users/me", "me%2fkeys",
|
||||
"me%252fkeys", `me\keys`, "", ".", "..",
|
||||
} {
|
||||
t.Run(identifier, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
valid := identifier == codersdk.Me || identifier == "alice" || identifier == "legacy_name"
|
||||
if _, err := uuid.Parse(identifier); err == nil {
|
||||
valid = true
|
||||
}
|
||||
origin, err := url.Parse("https://coder.example.com")
|
||||
require.NoError(t, err)
|
||||
var calls int
|
||||
client := codersdk.New(origin, codersdk.WithHTTPClient(&http.Client{
|
||||
Transport: testutil.RoundTripperFunc(func(req *http.Request) (*http.Response, error) {
|
||||
calls++
|
||||
assert.Equal(t, origin.Host, req.URL.Host)
|
||||
assert.Empty(t, req.URL.Fragment)
|
||||
if req.Method == http.MethodPost {
|
||||
assert.Equal(t, "/api/v2/users/"+identifier+"/workspaces", req.URL.Path)
|
||||
assert.Empty(t, req.URL.RawQuery)
|
||||
} else {
|
||||
assert.Equal(t, "/api/v2/users/"+identifier+"/workspace/legacy_name", req.URL.Path)
|
||||
assert.Equal(t, "include_deleted=false", req.URL.RawQuery)
|
||||
}
|
||||
return nil, xerrors.New("request reached transport")
|
||||
}),
|
||||
}))
|
||||
_, createErr := client.CreateUserWorkspace(t.Context(), identifier, codersdk.CreateWorkspaceRequest{})
|
||||
_, lookupErr := client.WorkspaceByOwnerAndName(t.Context(), identifier, "legacy_name", codersdk.WorkspaceOptions{})
|
||||
if valid {
|
||||
require.ErrorContains(t, createErr, "request reached transport")
|
||||
require.ErrorContains(t, lookupErr, "request reached transport")
|
||||
require.Equal(t, 2, calls)
|
||||
} else {
|
||||
require.ErrorContains(t, createErr, "invalid user")
|
||||
require.ErrorContains(t, lookupErr, "invalid workspace owner")
|
||||
require.Zero(t, calls)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveWorkspace(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -293,20 +340,26 @@ func TestResolveWorkspace(t *testing.T) {
|
||||
t.Run("InvalidIdentifier", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var hits atomic.Int64
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
|
||||
hits.Add(1)
|
||||
t.Errorf("unexpected HTTP request for invalid identifier: %s", req.URL.Path)
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
u, err := url.Parse(srv.URL)
|
||||
require.NoError(t, err)
|
||||
client := codersdk.New(u)
|
||||
|
||||
_, err = client.ResolveWorkspace(t.Context(), "a/b/c")
|
||||
require.Error(t, err)
|
||||
require.ErrorContains(t, err, "invalid workspace identifier: \"a/b/c\"")
|
||||
require.EqualValues(t, 0, hits.Load(), "invalid identifiers should fail before any HTTP request")
|
||||
for _, identifier := range []string{
|
||||
"a/b/c", "me?/workspace", "me#/workspace", "me%3f/workspace",
|
||||
"../workspace", "./workspace", "workspace?after=1", "workspace#fragment",
|
||||
"%2e%2e", "me%252fkeys/workspace", `me\keys/workspace`, "..", ".", "", "/workspace", "me/",
|
||||
} {
|
||||
t.Run(identifier, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
var hits atomic.Int64
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
|
||||
hits.Add(1)
|
||||
t.Errorf("unexpected HTTP request for invalid identifier: %s", req.URL.Path)
|
||||
}))
|
||||
t.Cleanup(srv.Close)
|
||||
u, err := url.Parse(srv.URL)
|
||||
require.NoError(t, err)
|
||||
client := codersdk.New(u)
|
||||
_, err = client.ResolveWorkspace(t.Context(), identifier)
|
||||
require.ErrorContains(t, err, "invalid workspace")
|
||||
require.EqualValues(t, 0, hits.Load(), "invalid identifiers should fail before any HTTP request")
|
||||
})
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -121,9 +121,13 @@ Coder enforces PKCE in compliance with the OAuth 2.1 specification, for both pub
|
||||
Coder provides OAuth2 discovery endpoints for programmatic integration:
|
||||
|
||||
- **Authorization Server Metadata**: `GET /.well-known/oauth-authorization-server`
|
||||
- **Protected Resource Metadata**: `GET /.well-known/oauth-protected-resource`
|
||||
- **REST API protected resource metadata**: `GET /.well-known/oauth-protected-resource`
|
||||
- **MCP protected resource metadata**: `GET /.well-known/oauth-protected-resource/api/experimental/mcp/http`
|
||||
|
||||
These endpoints return server capabilities and endpoint URLs according to [RFC 8414](https://datatracker.ietf.org/doc/html/rfc8414) and [RFC 9728](https://datatracker.ietf.org/doc/html/rfc9728).
|
||||
The REST API metadata identifies the deployment URL as its `resource`, while the MCP metadata identifies the full MCP endpoint URL.
|
||||
Both list the deployment URL in `authorization_servers` and use the same authorization server metadata endpoint.
|
||||
OAuth2 tokens issued for the MCP resource work with MCP tools.
|
||||
|
||||
`token_endpoint_auth_methods_supported` lists every method the token endpoint accepts, including `none`.
|
||||
It is not gated on [Dynamic Client Registration](./index.md#dynamic-client-registration), since existing public clients still exchange tokens when new registrations are disabled.
|
||||
|
||||
@@ -178,15 +178,16 @@ The MCP server supports two authentication methods:
|
||||
|
||||
### OAuth2 (Recommended for Interactive Clients)
|
||||
|
||||
MCP clients that support [RFC 9728](https://datatracker.ietf.org/doc/html/rfc9728)
|
||||
(Protected Resource Metadata) can authenticate automatically using OAuth2. The
|
||||
server advertises its OAuth2 capabilities via the `WWW-Authenticate` header and
|
||||
`/.well-known/oauth-protected-resource` endpoint.
|
||||
|
||||
MCP clients that support [RFC 9728](https://datatracker.ietf.org/doc/html/rfc9728) (Protected Resource Metadata) can authenticate automatically using OAuth2.
|
||||
This enables a seamless connect-and-authenticate experience where users sign in through their browser without manually managing tokens.
|
||||
|
||||
The MCP endpoint's `WWW-Authenticate` header points to `/.well-known/oauth-protected-resource/api/experimental/mcp/http` through its `resource_metadata` parameter.
|
||||
This metadata identifies the full MCP endpoint URL as the `resource`, for example, `https://coder.example.com/api/experimental/mcp/http`.
|
||||
|
||||
> [!NOTE]
|
||||
> OAuth2 requires `CODER_OAUTH2_PROVIDER_ENABLE=true` on your Coder deployment.
|
||||
>
|
||||
> [Dynamic Client Registration (DCR)](../admin/integrations/oauth2-provider/index.md#dynamic-client-registration) is off by default and must be enabled before clients can self-register.
|
||||
|
||||
### Session Token (For Programmatic Access)
|
||||
|
||||
@@ -252,5 +253,6 @@ them for you to invoke, for example as slash commands:
|
||||
### OAuth2 authentication not working
|
||||
|
||||
- Ensure your Coder deployment has `CODER_OAUTH2_PROVIDER_ENABLE=true` set
|
||||
- If your client registers itself automatically, ensure [Dynamic Client Registration](../admin/integrations/oauth2-provider/index.md#dynamic-client-registration) is enabled
|
||||
- Verify your MCP client supports RFC 9728 Protected Resource Metadata
|
||||
- Check that your browser can reach the Coder authorization endpoint
|
||||
|
||||
Generated
+6
@@ -6701,6 +6701,12 @@ export interface LoginWithPasswordResponse {
|
||||
readonly session_token: string;
|
||||
}
|
||||
|
||||
// From codersdk/mcp.go
|
||||
/**
|
||||
* MCPEndpoint is the remote MCP HTTP endpoint path.
|
||||
*/
|
||||
export const MCPEndpoint = "/api/experimental/mcp/http";
|
||||
|
||||
// From codersdk/mcp.go
|
||||
/**
|
||||
* MCPServerConfig represents an admin-configured MCP server.
|
||||
|
||||
Reference in New Issue
Block a user