diff --git a/coderd/mcp.go b/coderd/mcp.go index 8d407360a2..c4b58971f6 100644 --- a/coderd/mcp.go +++ b/coderd/mcp.go @@ -1,17 +1,20 @@ package coderd import ( + "bytes" "context" "database/sql" "encoding/json" "fmt" + "io" "net/http" "net/url" "strings" + "time" "github.com/go-chi/chi/v5" "github.com/google/uuid" - "github.com/mark3labs/mcp-go/client/transport" + "github.com/mark3labs/mcp-go/mcp" "golang.org/x/oauth2" "golang.org/x/xerrors" @@ -182,7 +185,11 @@ func (api *API) createMCPServerConfig(rw http.ResponseWriter, r *http.Request) { // Now build the callback URL with the actual ID. callbackURL := fmt.Sprintf("%s/api/experimental/mcp/servers/%s/oauth2/callback", api.AccessURL.String(), inserted.ID) - result, err := discoverAndRegisterMCPOAuth2(ctx, strings.TrimSpace(req.URL), callbackURL) + httpClient := api.HTTPClient + if httpClient == nil { + httpClient = &http.Client{Timeout: 30 * time.Second} + } + result, err := discoverAndRegisterMCPOAuth2(ctx, httpClient, strings.TrimSpace(req.URL), callbackURL) if err != nil { // Clean up: delete the partially created config. deleteErr := api.Database.DeleteMCPServerConfigByID(ctx, inserted.ID) @@ -1107,55 +1114,345 @@ type mcpOAuth2Discovery struct { scopes string // space-separated } -// discoverAndRegisterMCPOAuth2 uses the mcp-go library's OAuthHandler to -// perform the MCP OAuth2 discovery and Dynamic Client Registration flow: +// protectedResourceMetadata represents the response from a +// Protected Resource Metadata endpoint per RFC 9728 §2. +type protectedResourceMetadata struct { + Resource string `json:"resource"` + AuthorizationServers []string `json:"authorization_servers"` + ScopesSupported []string `json:"scopes_supported,omitempty"` +} + +// authServerMetadata represents the response from an Authorization +// Server Metadata endpoint per RFC 8414 §2. +type authServerMetadata struct { + Issuer string `json:"issuer"` + AuthorizationEndpoint string `json:"authorization_endpoint"` + TokenEndpoint string `json:"token_endpoint"` + RegistrationEndpoint string `json:"registration_endpoint,omitempty"` + ScopesSupported []string `json:"scopes_supported,omitempty"` +} + +// fetchJSON performs a GET request to the given URL with the +// standard MCP OAuth2 discovery headers and decodes the JSON +// response into dest. It returns nil on success or an error +// if the request fails or the server returns a non-200 status. +func fetchJSON(ctx context.Context, httpClient *http.Client, rawURL string, dest any) error { + req, err := http.NewRequestWithContext( + ctx, http.MethodGet, rawURL, nil, + ) + if err != nil { + return xerrors.Errorf("create request for %s: %w", rawURL, err) + } + req.Header.Set("Accept", "application/json") + req.Header.Set("MCP-Protocol-Version", mcp.LATEST_PROTOCOL_VERSION) + + resp, err := httpClient.Do(req) + if err != nil { + return xerrors.Errorf("GET %s: %w", rawURL, err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return xerrors.Errorf( + "GET %s returned HTTP %d", rawURL, resp.StatusCode, + ) + } + + body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + if err != nil { + return xerrors.Errorf( + "read response from %s: %w", rawURL, err, + ) + } + + if err := json.Unmarshal(body, dest); err != nil { + return xerrors.Errorf( + "decode JSON from %s: %w", rawURL, err, + ) + } + + return nil +} + +// discoverProtectedResource discovers the Protected Resource +// Metadata for the given MCP server per RFC 9728 §3.1. It +// tries the path-aware well-known URL first, then falls back +// to the root-level URL. // -// 1. Discover the authorization server via Protected Resource Metadata -// (RFC 9728) and Authorization Server Metadata (RFC 8414). -// 2. Register a client via Dynamic Client Registration (RFC 7591). -// 3. Return the discovered endpoints and generated credentials. -func discoverAndRegisterMCPOAuth2(ctx context.Context, mcpServerURL, callbackURL string) (*mcpOAuth2Discovery, error) { - // Per the MCP spec, the authorization base URL is the MCP server - // URL with the path component discarded (scheme + host only). +// Path-aware: GET {origin}/.well-known/oauth-protected-resource{path} +// Root: GET {origin}/.well-known/oauth-protected-resource +func discoverProtectedResource( + ctx context.Context, httpClient *http.Client, origin, path string, +) (*protectedResourceMetadata, error) { + var urls []string + + // Per RFC 9728 §3.1, when the resource URL contains a + // path component, the well-known URI is constructed by + // inserting the well-known prefix before the path. + if path != "" && path != "/" { + urls = append( + urls, + origin+"/.well-known/oauth-protected-resource"+path, + ) + } + // Always try the root-level URL as a fallback. + urls = append( + urls, origin+"/.well-known/oauth-protected-resource", + ) + + var lastErr error + for _, u := range urls { + var meta protectedResourceMetadata + if err := fetchJSON(ctx, httpClient, u, &meta); err != nil { + lastErr = err + continue + } + if len(meta.AuthorizationServers) == 0 { + lastErr = xerrors.Errorf( + "protected resource metadata at %s "+ + "has no authorization_servers", u, + ) + continue + } + return &meta, nil + } + + return nil, xerrors.Errorf( + "discover protected resource metadata: %w", lastErr, + ) +} + +// discoverAuthServerMetadata discovers the Authorization Server +// Metadata per RFC 8414 §3.1. When the authorization server +// issuer URL has a path component, the metadata URL is +// path-aware. Falls back to root-level and OpenID Connect +// discovery as a last resort. +// +// Path-aware: {origin}/.well-known/oauth-authorization-server{path} +// Root: {origin}/.well-known/oauth-authorization-server +// OpenID: {issuer}/.well-known/openid-configuration +func discoverAuthServerMetadata( + ctx context.Context, httpClient *http.Client, authServerURL string, +) (*authServerMetadata, error) { + parsed, err := url.Parse(authServerURL) + if err != nil { + return nil, xerrors.Errorf( + "parse auth server URL: %w", err, + ) + } + asOrigin := fmt.Sprintf( + "%s://%s", parsed.Scheme, parsed.Host, + ) + asPath := parsed.Path + + var urls []string + + // Per RFC 8414 §3.1, if the issuer URL has a path, + // insert the well-known prefix before the path. + if asPath != "" && asPath != "/" { + urls = append( + urls, + asOrigin+"/.well-known/oauth-authorization-server"+asPath, + ) + } + // Root-level fallback. + urls = append( + urls, + asOrigin+"/.well-known/oauth-authorization-server", + ) + // OpenID Connect discovery as a last resort. Note: this is + // tried after RFC 8414 (unlike the previous mcp-go code that + // tried OIDC first) because RFC 8414 is the MCP spec's + // recommended discovery mechanism. + // Per OpenID Connect Discovery 1.0 §4, the well-known URL + // is formed by appending to the full issuer (including + // path), not just the origin. + urls = append( + urls, + strings.TrimRight(authServerURL, "/")+ + "/.well-known/openid-configuration", + ) + + var lastErr error + for _, u := range urls { + var meta authServerMetadata + if err := fetchJSON(ctx, httpClient, u, &meta); err != nil { + lastErr = err + continue + } + if meta.AuthorizationEndpoint == "" || meta.TokenEndpoint == "" { + lastErr = xerrors.Errorf( + "auth server metadata at %s missing required "+ + "endpoints", u, + ) + continue + } + return &meta, nil + } + + return nil, xerrors.Errorf( + "discover auth server metadata: %w", lastErr, + ) +} + +// registerOAuth2Client performs Dynamic Client Registration per +// RFC 7591 by POSTing client metadata to the registration +// endpoint and returning the assigned client_id and optional +// client_secret. +func registerOAuth2Client( + ctx context.Context, httpClient *http.Client, + registrationEndpoint, callbackURL, clientName string, +) (clientID string, clientSecret string, err error) { + payload := map[string]any{ + "client_name": clientName, + "redirect_uris": []string{callbackURL}, + "token_endpoint_auth_method": "none", + "grant_types": []string{"authorization_code", "refresh_token"}, + "response_types": []string{"code"}, + } + + body, err := json.Marshal(payload) + if err != nil { + return "", "", xerrors.Errorf( + "marshal registration request: %w", err, + ) + } + + req, err := http.NewRequestWithContext( + ctx, http.MethodPost, + registrationEndpoint, bytes.NewReader(body), + ) + if err != nil { + return "", "", xerrors.Errorf( + "create registration request: %w", err, + ) + } + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json") + + resp, err := httpClient.Do(req) + if err != nil { + return "", "", xerrors.Errorf( + "POST %s: %w", registrationEndpoint, err, + ) + } + defer resp.Body.Close() + + respBody, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + if err != nil { + return "", "", xerrors.Errorf( + "read registration response: %w", err, + ) + } + + if resp.StatusCode != http.StatusOK && + resp.StatusCode != http.StatusCreated { + // Truncate to avoid leaking verbose upstream errors + // through the API. + const maxErrBody = 512 + errMsg := string(respBody) + if len(errMsg) > maxErrBody { + errMsg = errMsg[:maxErrBody] + "..." + } + return "", "", xerrors.Errorf( + "registration endpoint returned HTTP %d: %s", + resp.StatusCode, errMsg, + ) + } + + var result struct { + ClientID string `json:"client_id"` + ClientSecret string `json:"client_secret"` + } + if err := json.Unmarshal(respBody, &result); err != nil { + return "", "", xerrors.Errorf( + "decode registration response: %w", err, + ) + } + if result.ClientID == "" { + return "", "", xerrors.New( + "registration response missing client_id", + ) + } + + return result.ClientID, result.ClientSecret, nil +} + +// discoverAndRegisterMCPOAuth2 performs the full MCP OAuth2 +// discovery and Dynamic Client Registration flow: +// +// 1. Discover the authorization server via Protected Resource +// Metadata (RFC 9728). +// 2. Fetch Authorization Server Metadata (RFC 8414). +// 3. Register a client via Dynamic Client Registration +// (RFC 7591). +// 4. Return the discovered endpoints and credentials. +// +// Unlike a root-only approach, this implementation follows the +// path-aware well-known URI construction rules from RFC 9728 +// §3.1 and RFC 8414 §3.1, which is required for servers that +// serve metadata at path-specific URLs (e.g. +// https://api.githubcopilot.com/mcp/). +func discoverAndRegisterMCPOAuth2(ctx context.Context, httpClient *http.Client, mcpServerURL, callbackURL string) (*mcpOAuth2Discovery, error) { + // Parse the MCP server URL into origin and path. parsed, err := url.Parse(mcpServerURL) if err != nil { - return nil, xerrors.Errorf("parse MCP server URL: %w", err) + return nil, xerrors.Errorf( + "parse MCP server URL: %w", err, + ) } origin := fmt.Sprintf("%s://%s", parsed.Scheme, parsed.Host) + path := parsed.Path - oauthHandler := transport.NewOAuthHandler(transport.OAuthConfig{ - RedirectURI: callbackURL, - TokenStore: transport.NewMemoryTokenStore(), - }) - oauthHandler.SetBaseURL(origin) - - // Step 1: Discover authorization server metadata (RFC 9728 + RFC 8414). - metadata, err := oauthHandler.GetServerMetadata(ctx) + // Step 1: Discover the Protected Resource Metadata + // (RFC 9728) to find the authorization server. + prm, err := discoverProtectedResource(ctx, httpClient, origin, path) if err != nil { - return nil, xerrors.Errorf("discover authorization server: %w", err) - } - if metadata.AuthorizationEndpoint == "" { - return nil, xerrors.New("authorization server metadata missing authorization_endpoint") - } - if metadata.TokenEndpoint == "" { - return nil, xerrors.New("authorization server metadata missing token_endpoint") - } - if metadata.RegistrationEndpoint == "" { - return nil, xerrors.New("authorization server does not advertise a registration_endpoint (dynamic client registration may not be supported)") + return nil, xerrors.Errorf( + "protected resource discovery: %w", err, + ) } - // Step 2: Register a client via Dynamic Client Registration (RFC 7591). - if err := oauthHandler.RegisterClient(ctx, "Coder"); err != nil { - return nil, xerrors.Errorf("dynamic client registration: %w", err) + // Step 2: Fetch Authorization Server Metadata (RFC 8414) + // from the first advertised authorization server. + asMeta, err := discoverAuthServerMetadata( + ctx, httpClient, prm.AuthorizationServers[0], + ) + if err != nil { + return nil, xerrors.Errorf( + "auth server metadata discovery: %w", err, + ) } - scopes := strings.Join(metadata.ScopesSupported, " ") + // Only RegistrationEndpoint needs checking here; + // discoverAuthServerMetadata already validates that + // AuthorizationEndpoint and TokenEndpoint are present. + if asMeta.RegistrationEndpoint == "" { + return nil, xerrors.New( + "authorization server does not advertise a " + + "registration_endpoint (dynamic client " + + "registration may not be supported)", + ) + } + + // Step 3: Register via Dynamic Client Registration + // (RFC 7591). + clientID, clientSecret, err := registerOAuth2Client( + ctx, httpClient, asMeta.RegistrationEndpoint, callbackURL, "Coder", + ) + if err != nil { + return nil, xerrors.Errorf( + "dynamic client registration: %w", err, + ) + } + + scopes := strings.Join(asMeta.ScopesSupported, " ") return &mcpOAuth2Discovery{ - clientID: oauthHandler.GetClientID(), - clientSecret: oauthHandler.GetClientSecret(), - authURL: metadata.AuthorizationEndpoint, - tokenURL: metadata.TokenEndpoint, + clientID: clientID, + clientSecret: clientSecret, + authURL: asMeta.AuthorizationEndpoint, + tokenURL: asMeta.TokenEndpoint, scopes: scopes, }, nil } diff --git a/coderd/mcp_test.go b/coderd/mcp_test.go index b904a5eb91..5b791adf11 100644 --- a/coderd/mcp_test.go +++ b/coderd/mcp_test.go @@ -473,17 +473,21 @@ func TestMCPServerConfigsOAuth2AutoDiscovery(t *testing.T) { t.Cleanup(authServer.Close) // Stand up a mock MCP server that serves RFC 9728 Protected - // Resource Metadata pointing to the auth server above. + // Resource Metadata at the path-aware well-known URL. + // The URL used for the config ends with /v1/mcp, so the + // path-aware metadata URL is + // /.well-known/oauth-protected-resource/v1/mcp. mcpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/.well-known/oauth-protected-resource" { + switch r.URL.Path { + case "/.well-known/oauth-protected-resource/v1/mcp": w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`{ "resource": "` + "http://" + r.Host + `", "authorization_servers": ["` + authServer.URL + `"] }`)) - return + default: + http.NotFound(w, r) } - http.NotFound(w, r) })) t.Cleanup(mcpServer.Close) @@ -511,6 +515,275 @@ func TestMCPServerConfigsOAuth2AutoDiscovery(t *testing.T) { require.Equal(t, "read write", created.OAuth2Scopes) }) + // Verify that when both path-aware and root-level protected + // resource metadata are available, the path-aware URL takes + // priority. Each points to a different auth server so we can + // distinguish which one was actually used. + t.Run("PathAwareTakesPriority", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + + // Auth server that returns "path-scope" as the supported + // scope. + pathAuthServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-authorization-server": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "issuer": "` + "http://" + r.Host + `", + "authorization_endpoint": "` + "http://" + r.Host + `/authorize", + "token_endpoint": "` + "http://" + r.Host + `/token", + "registration_endpoint": "` + "http://" + r.Host + `/register", + "response_types_supported": ["code"], + "scopes_supported": ["path-scope"] + }`)) + case "/register": + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{ + "client_id": "path-client-id", + "client_secret": "path-client-secret" + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(pathAuthServer.Close) + + // Auth server that returns "root-scope" as the supported + // scope. + rootAuthServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-authorization-server": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "issuer": "` + "http://" + r.Host + `", + "authorization_endpoint": "` + "http://" + r.Host + `/authorize", + "token_endpoint": "` + "http://" + r.Host + `/token", + "registration_endpoint": "` + "http://" + r.Host + `/register", + "response_types_supported": ["code"], + "scopes_supported": ["root-scope"] + }`)) + case "/register": + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{ + "client_id": "root-client-id", + "client_secret": "root-client-secret" + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(rootAuthServer.Close) + + // MCP server serves different protected resource metadata at + // path-aware vs root URLs, each pointing to a different auth + // server. + mcpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-protected-resource/v1/mcp": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "resource": "` + "http://" + r.Host + `/v1/mcp", + "authorization_servers": ["` + pathAuthServer.URL + `"] + }`)) + case "/.well-known/oauth-protected-resource": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "resource": "` + "http://" + r.Host + `", + "authorization_servers": ["` + rootAuthServer.URL + `"] + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(mcpServer.Close) + + client := newMCPClient(t) + _ = coderdtest.CreateFirstUser(t, client) + + created, err := client.CreateMCPServerConfig(ctx, codersdk.CreateMCPServerConfigRequest{ + DisplayName: "Priority Test", + Slug: "priority-test", + Transport: "streamable_http", + URL: mcpServer.URL + "/v1/mcp", + AuthType: "oauth2", + Availability: "default_on", + Enabled: true, + ToolAllowList: []string{}, + ToolDenyList: []string{}, + }) + require.NoError(t, err) + // The path-aware auth server returns "path-scope", the root + // auth server returns "root-scope". If path-aware takes + // priority, we get "path-scope". + require.Equal(t, "path-client-id", created.OAuth2ClientID) + require.Equal(t, "path-scope", created.OAuth2Scopes) + }) + + // Verify discovery works when the protected resource metadata + // is only available at the root-level well-known URL (no path + // component). This covers servers that don't use path-aware + // metadata. + t.Run("RootLevelFallback", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + + authServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-authorization-server": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "issuer": "` + r.Host + `", + "authorization_endpoint": "` + "http://" + r.Host + `/authorize", + "token_endpoint": "` + "http://" + r.Host + `/token", + "registration_endpoint": "` + "http://" + r.Host + `/register", + "response_types_supported": ["code"], + "scopes_supported": ["all"] + }`)) + case "/register": + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{ + "client_id": "root-client-id", + "client_secret": "root-client-secret" + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(authServer.Close) + + // MCP server only serves metadata at the root well-known + // URL, NOT at the path-aware location. + mcpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-protected-resource": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "resource": "` + "http://" + r.Host + `", + "authorization_servers": ["` + authServer.URL + `"] + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(mcpServer.Close) + + client := newMCPClient(t) + _ = coderdtest.CreateFirstUser(t, client) + + created, err := client.CreateMCPServerConfig(ctx, codersdk.CreateMCPServerConfigRequest{ + DisplayName: "Root Fallback Server", + Slug: "root-fallback", + Transport: "streamable_http", + URL: mcpServer.URL + "/v1/mcp", + AuthType: "oauth2", + Availability: "default_on", + Enabled: true, + ToolAllowList: []string{}, + ToolDenyList: []string{}, + }) + require.NoError(t, err) + require.Equal(t, "root-client-id", created.OAuth2ClientID) + require.True(t, created.HasOAuth2Secret) + require.Equal(t, authServer.URL+"/authorize", created.OAuth2AuthURL) + require.Equal(t, authServer.URL+"/token", created.OAuth2TokenURL) + require.Equal(t, "all", created.OAuth2Scopes) + }) + + // Verify that when the authorization server issuer URL has a + // path component (e.g. https://github.com/login/oauth), the + // discovery uses the path-aware metadata URL per RFC 8414 §3.1. + t.Run("PathAwareAuthServerMetadata", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + + // Auth server that serves metadata at the path-aware URL. + // The issuer URL is http://host/login/oauth, so the + // metadata URL should be + // /.well-known/oauth-authorization-server/login/oauth. + authServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-authorization-server/login/oauth": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "issuer": "` + "http://" + r.Host + `/login/oauth", + "authorization_endpoint": "` + "http://" + r.Host + `/login/oauth/authorize", + "token_endpoint": "` + "http://" + r.Host + `/login/oauth/token", + "registration_endpoint": "` + "http://" + r.Host + `/register", + "response_types_supported": ["code"], + "scopes_supported": ["repo", "read:org"] + }`)) + case "/register": + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{ + "client_id": "path-aware-client-id" + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(authServer.Close) + + // MCP server that points to an auth server with a path + // in its issuer URL (like GitHub's /login/oauth). + mcpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-protected-resource/mcp": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "resource": "` + "http://" + r.Host + `/mcp", + "authorization_servers": ["` + authServer.URL + `/login/oauth"] + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(mcpServer.Close) + + client := newMCPClient(t) + _ = coderdtest.CreateFirstUser(t, client) + + created, err := client.CreateMCPServerConfig(ctx, codersdk.CreateMCPServerConfigRequest{ + DisplayName: "Path-Aware Auth", + Slug: "path-aware-auth", + Transport: "streamable_http", + URL: mcpServer.URL + "/mcp", + AuthType: "oauth2", + Availability: "default_on", + Enabled: true, + ToolAllowList: []string{}, + ToolDenyList: []string{}, + }) + require.NoError(t, err) + require.Equal(t, "path-aware-client-id", created.OAuth2ClientID) + require.Equal(t, authServer.URL+"/login/oauth/authorize", created.OAuth2AuthURL) + require.Equal(t, authServer.URL+"/login/oauth/token", created.OAuth2TokenURL) + require.Equal(t, "repo read:org", created.OAuth2Scopes) + }) + // Regression test: verify that during dynamic client registration // the redirect_uris sent to the authorization server contain the // real config UUID, NOT the literal string "{id}". Before the @@ -572,15 +845,17 @@ func TestMCPServerConfigsOAuth2AutoDiscovery(t *testing.T) { // Stand up a mock MCP server that returns RFC 9728 Protected // Resource Metadata pointing to the auth server. mcpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/.well-known/oauth-protected-resource" { + switch r.URL.Path { + case "/.well-known/oauth-protected-resource/v1/mcp", + "/.well-known/oauth-protected-resource": w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`{ "resource": "` + "http://" + r.Host + `", "authorization_servers": ["` + authServer.URL + `"] }`)) - return + default: + http.NotFound(w, r) } - http.NotFound(w, r) })) t.Cleanup(mcpServer.Close) @@ -1055,3 +1330,514 @@ func createChatModelConfigForMCP(t testing.TB, client *codersdk.ExperimentalClie require.NoError(t, err) return modelConfig } + +func TestMCPOAuth2DiscoveryEdgeCases(t *testing.T) { + t.Parallel() + + t.Run("EmptyAuthorizationServers", func(t *testing.T) { + t.Parallel() + + // When the path-aware PRM returns an empty + // authorization_servers array, discovery should fall + // back to the root-level PRM. + t.Run("RootFallback", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + + authServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-authorization-server": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "issuer": "` + "http://" + r.Host + `", + "authorization_endpoint": "` + "http://" + r.Host + `/authorize", + "token_endpoint": "` + "http://" + r.Host + `/token", + "registration_endpoint": "` + "http://" + r.Host + `/register", + "response_types_supported": ["code"], + "scopes_supported": ["fallback-scope"] + }`)) + case "/register": + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{ + "client_id": "fallback-client-id", + "client_secret": "fallback-client-secret" + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(authServer.Close) + + mcpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-protected-resource/v1/mcp": + // Path-aware: empty authorization_servers. + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "resource": "` + "http://" + r.Host + `/v1/mcp", + "authorization_servers": [] + }`)) + case "/.well-known/oauth-protected-resource": + // Root: valid authorization_servers. + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "resource": "` + "http://" + r.Host + `", + "authorization_servers": ["` + authServer.URL + `"] + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(mcpServer.Close) + + client := newMCPClient(t) + _ = coderdtest.CreateFirstUser(t, client) + + created, err := client.CreateMCPServerConfig(ctx, codersdk.CreateMCPServerConfigRequest{ + DisplayName: "Empty Auth Servers Fallback", + Slug: "empty-as-fallback", + Transport: "streamable_http", + URL: mcpServer.URL + "/v1/mcp", + AuthType: "oauth2", + Availability: "default_on", + Enabled: true, + ToolAllowList: []string{}, + ToolDenyList: []string{}, + }) + require.NoError(t, err) + require.Equal(t, "fallback-client-id", created.OAuth2ClientID) + require.Equal(t, authServer.URL+"/authorize", created.OAuth2AuthURL) + require.Equal(t, authServer.URL+"/token", created.OAuth2TokenURL) + require.Equal(t, "fallback-scope", created.OAuth2Scopes) + }) + + // When both path-aware and root PRM return empty + // authorization_servers, discovery should fail. + t.Run("BothEmpty", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + + mcpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-protected-resource/v1/mcp", + "/.well-known/oauth-protected-resource": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "resource": "` + "http://" + r.Host + `", + "authorization_servers": [] + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(mcpServer.Close) + + client := newMCPClient(t) + _ = coderdtest.CreateFirstUser(t, client) + + _, err := client.CreateMCPServerConfig(ctx, codersdk.CreateMCPServerConfigRequest{ + DisplayName: "Both Empty", + Slug: "both-empty-as", + Transport: "streamable_http", + URL: mcpServer.URL + "/v1/mcp", + AuthType: "oauth2", + Availability: "default_on", + Enabled: true, + ToolAllowList: []string{}, + ToolDenyList: []string{}, + }) + require.Error(t, err) + var sdkErr *codersdk.Error + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, http.StatusBadRequest, sdkErr.StatusCode()) + require.Contains(t, sdkErr.Message, "auto-discovery failed") + }) + }) + + // When the path-aware PRM returns malformed JSON, + // discovery should fall back to the root-level PRM. + t.Run("MalformedJSONFromDiscovery", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + + authServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-authorization-server": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "issuer": "` + "http://" + r.Host + `", + "authorization_endpoint": "` + "http://" + r.Host + `/authorize", + "token_endpoint": "` + "http://" + r.Host + `/token", + "registration_endpoint": "` + "http://" + r.Host + `/register", + "response_types_supported": ["code"], + "scopes_supported": ["json-fallback"] + }`)) + case "/register": + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{ + "client_id": "json-fallback-client", + "client_secret": "json-fallback-secret" + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(authServer.Close) + + mcpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-protected-resource/v1/mcp": + // Return valid HTTP 200 but invalid JSON. + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`not json`)) + case "/.well-known/oauth-protected-resource": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "resource": "` + "http://" + r.Host + `", + "authorization_servers": ["` + authServer.URL + `"] + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(mcpServer.Close) + + client := newMCPClient(t) + _ = coderdtest.CreateFirstUser(t, client) + + created, err := client.CreateMCPServerConfig(ctx, codersdk.CreateMCPServerConfigRequest{ + DisplayName: "Malformed JSON Fallback", + Slug: "malformed-json", + Transport: "streamable_http", + URL: mcpServer.URL + "/v1/mcp", + AuthType: "oauth2", + Availability: "default_on", + Enabled: true, + ToolAllowList: []string{}, + ToolDenyList: []string{}, + }) + require.NoError(t, err) + require.Equal(t, "json-fallback-client", created.OAuth2ClientID) + require.Equal(t, authServer.URL+"/authorize", created.OAuth2AuthURL) + require.Equal(t, authServer.URL+"/token", created.OAuth2TokenURL) + require.Equal(t, "json-fallback", created.OAuth2Scopes) + }) + + // When the path-aware auth server metadata is missing required + // endpoints, discovery should fall back to the root-level + // metadata URL. + t.Run("AuthServerMetadataMissingEndpoints", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + + // Auth server that returns incomplete metadata at the + // path-aware URL but complete metadata at the root URL. + authServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-authorization-server/auth": + // Path-aware: missing required endpoints. + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "issuer": "` + "http://" + r.Host + `/auth" + }`)) + case "/.well-known/oauth-authorization-server": + // Root-level: complete metadata. + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "issuer": "` + "http://" + r.Host + `", + "authorization_endpoint": "` + "http://" + r.Host + `/authorize", + "token_endpoint": "` + "http://" + r.Host + `/token", + "registration_endpoint": "` + "http://" + r.Host + `/register", + "response_types_supported": ["code"], + "scopes_supported": ["endpoint-fallback"] + }`)) + case "/register": + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{ + "client_id": "endpoint-fallback-client", + "client_secret": "endpoint-fallback-secret" + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(authServer.Close) + + // PRM points to auth server with a path (/auth) so that + // discoverAuthServerMetadata tries the path-aware URL first. + mcpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-protected-resource/v1/mcp": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "resource": "` + "http://" + r.Host + `/v1/mcp", + "authorization_servers": ["` + authServer.URL + `/auth"] + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(mcpServer.Close) + + client := newMCPClient(t) + _ = coderdtest.CreateFirstUser(t, client) + + created, err := client.CreateMCPServerConfig(ctx, codersdk.CreateMCPServerConfigRequest{ + DisplayName: "Missing Endpoints Fallback", + Slug: "missing-endpoints", + Transport: "streamable_http", + URL: mcpServer.URL + "/v1/mcp", + AuthType: "oauth2", + Availability: "default_on", + Enabled: true, + ToolAllowList: []string{}, + ToolDenyList: []string{}, + }) + require.NoError(t, err) + require.Equal(t, "endpoint-fallback-client", created.OAuth2ClientID) + require.Equal(t, authServer.URL+"/authorize", created.OAuth2AuthURL) + require.Equal(t, authServer.URL+"/token", created.OAuth2TokenURL) + require.Equal(t, "endpoint-fallback", created.OAuth2Scopes) + }) + + // When both RFC 8414 metadata URLs (path-aware and root) fail, + // discovery should fall back to the OIDC well-known URL. + // The auth server issuer has a path (/login/oauth) so the + // OIDC URL is {issuer}/.well-known/openid-configuration = + // /login/oauth/.well-known/openid-configuration. + t.Run("OIDCFallback", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + + authServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/login/oauth/.well-known/openid-configuration": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "issuer": "` + "http://" + r.Host + `/login/oauth", + "authorization_endpoint": "` + "http://" + r.Host + `/login/oauth/authorize", + "token_endpoint": "` + "http://" + r.Host + `/login/oauth/token", + "registration_endpoint": "` + "http://" + r.Host + `/register", + "response_types_supported": ["code"], + "scopes_supported": ["oidc-scope"] + }`)) + case "/register": + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{ + "client_id": "oidc-client-id", + "client_secret": "oidc-client-secret" + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(authServer.Close) + + // PRM points to auth server with a path (/login/oauth) + // so that RFC 8414 URLs are tried first and fail. + mcpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-protected-resource/v1/mcp": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "resource": "` + "http://" + r.Host + `/v1/mcp", + "authorization_servers": ["` + authServer.URL + `/login/oauth"] + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(mcpServer.Close) + + client := newMCPClient(t) + _ = coderdtest.CreateFirstUser(t, client) + + created, err := client.CreateMCPServerConfig(ctx, codersdk.CreateMCPServerConfigRequest{ + DisplayName: "OIDC Fallback", + Slug: "oidc-fallback", + Transport: "streamable_http", + URL: mcpServer.URL + "/v1/mcp", + AuthType: "oauth2", + Availability: "default_on", + Enabled: true, + ToolAllowList: []string{}, + ToolDenyList: []string{}, + }) + require.NoError(t, err) + require.Equal(t, "oidc-client-id", created.OAuth2ClientID) + require.Equal(t, authServer.URL+"/login/oauth/authorize", created.OAuth2AuthURL) + require.Equal(t, authServer.URL+"/login/oauth/token", created.OAuth2TokenURL) + require.Equal(t, "oidc-scope", created.OAuth2Scopes) + }) + + // When the registration endpoint returns a response + // without a client_id, the entire discovery flow should + // fail. + t.Run("RegistrationMissingClientID", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + + authServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-authorization-server": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "issuer": "` + "http://" + r.Host + `", + "authorization_endpoint": "` + "http://" + r.Host + `/authorize", + "token_endpoint": "` + "http://" + r.Host + `/token", + "registration_endpoint": "` + "http://" + r.Host + `/register", + "response_types_supported": ["code"] + }`)) + case "/register": + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + // Return response with client_secret but no + // client_id. + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{ + "client_secret": "secret-without-id" + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(authServer.Close) + + mcpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-protected-resource/v1/mcp": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "resource": "` + "http://" + r.Host + `/v1/mcp", + "authorization_servers": ["` + authServer.URL + `"] + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(mcpServer.Close) + + client := newMCPClient(t) + _ = coderdtest.CreateFirstUser(t, client) + + _, err := client.CreateMCPServerConfig(ctx, codersdk.CreateMCPServerConfigRequest{ + DisplayName: "Missing Client ID", + Slug: "missing-client-id", + Transport: "streamable_http", + URL: mcpServer.URL + "/v1/mcp", + AuthType: "oauth2", + Availability: "default_on", + Enabled: true, + ToolAllowList: []string{}, + ToolDenyList: []string{}, + }) + require.Error(t, err) + var sdkErr *codersdk.Error + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, http.StatusBadRequest, sdkErr.StatusCode()) + require.Contains(t, sdkErr.Message, "auto-discovery failed") + }) + + // Regression test for the exact scenario that motivated the PR: + // an MCP server URL with a trailing slash (like + // https://api.githubcopilot.com/mcp/). + t.Run("TrailingSlashURL", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + + authServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-authorization-server": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "issuer": "` + "http://" + r.Host + `", + "authorization_endpoint": "` + "http://" + r.Host + `/authorize", + "token_endpoint": "` + "http://" + r.Host + `/token", + "registration_endpoint": "` + "http://" + r.Host + `/register", + "response_types_supported": ["code"], + "scopes_supported": ["read"] + }`)) + case "/register": + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{ + "client_id": "trailing-slash-client", + "client_secret": "trailing-slash-secret" + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(authServer.Close) + + // Serve protected resource metadata at the path-aware URL + // WITH the trailing slash: /.well-known/oauth-protected-resource/mcp/ + mcpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-protected-resource/mcp/": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "resource": "` + "http://" + r.Host + `/mcp/", + "authorization_servers": ["` + authServer.URL + `"] + }`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(mcpServer.Close) + + client := newMCPClient(t) + _ = coderdtest.CreateFirstUser(t, client) + + // URL has a trailing slash, matching the GitHub Copilot URL + // pattern: https://api.githubcopilot.com/mcp/ + created, err := client.CreateMCPServerConfig(ctx, codersdk.CreateMCPServerConfigRequest{ + DisplayName: "Trailing Slash", + Slug: "trailing-slash", + Transport: "streamable_http", + URL: mcpServer.URL + "/mcp/", + AuthType: "oauth2", + Availability: "default_on", + Enabled: true, + ToolAllowList: []string{}, + ToolDenyList: []string{}, + }) + require.NoError(t, err) + require.Equal(t, "trailing-slash-client", created.OAuth2ClientID) + require.True(t, created.HasOAuth2Secret) + }) +}