mirror of
https://github.com/coder/coder.git
synced 2026-10-30 18:12:29 +08:00
fix(coderd): accept RFC 9728 resource metadata with array-valued resource (#28592)
GitLab's official MCP server returns `"resource"` as a JSON array in its `/.well-known/oauth-protected-resource` document, while RFC 9728 §2 defines it as a string. Coder's OAuth2 auto-discovery failed to decode the response (`json: cannot unmarshal array into Go struct field protectedResourceMetadata.resource of type string`), blocking zero-config `oauth2` DCR against GitLab. This makes `protectedResourceMetadata.Resource` tolerate both a single string and an array of strings. Part of [CODAGT-570](https://linear.app/codercom/issue/CODAGT-570/coder-agents-mcp-client-cannot-connect-to-gitlabs-official-mcp-server). The other blocker in that issue (GitLab replying 204 to `notifications/initialized`) was already fixed by the migration from `mark3labs/mcp-go` to `modelcontextprotocol/go-sdk`, whose streamable-HTTP client accepts 204. <details> <summary>Investigation notes</summary> CODAGT-570 reported two independent incompatibilities with GitLab CE's `/api/v4/mcp` server: 1. **204 vs 202 on `notifications/initialized` (blocked all auth modes):** the bundled `mark3labs/mcp-go` client only accepted 200/202 for notification POSTs. That library has since been removed entirely; `coderd/x/chatd/mcpclient` now uses `github.com/modelcontextprotocol/go-sdk v1.7.0`, which accepts both 204 and 202 (and only warns on other codes in non-strict mode, which is the only mode reachable from Coder). No further change needed. 2. **RFC 9728 `resource` array (blocked zero-config oauth2 DCR):** Coder's own parser in `coderd/mcp.go` declared `Resource string`. This lives outside the MCP library, so the SDK migration did not fix it. Fixed here with a custom `UnmarshalJSON` accepting string or array of strings. The field is only decoded, never consumed downstream, so behavior is otherwise unchanged. </details> --- Generated by Coder Agents on behalf of @f0ssel.
This commit is contained in:
+23
-3
@@ -1699,9 +1699,29 @@ type mcpOAuth2Discovery struct {
|
||||
// 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"`
|
||||
Resource resourceIdentifiers `json:"resource"`
|
||||
AuthorizationServers []string `json:"authorization_servers"`
|
||||
ScopesSupported []string `json:"scopes_supported,omitempty"`
|
||||
}
|
||||
|
||||
// resourceIdentifiers tolerates both a single JSON string and an
|
||||
// array of strings. RFC 9728 §2 defines "resource" as a string, but
|
||||
// some servers (e.g. GitLab's official MCP server) return an array
|
||||
// when the metadata document covers multiple resources.
|
||||
type resourceIdentifiers []string
|
||||
|
||||
func (r *resourceIdentifiers) UnmarshalJSON(data []byte) error {
|
||||
var single string
|
||||
if err := json.Unmarshal(data, &single); err == nil {
|
||||
*r = resourceIdentifiers{single}
|
||||
return nil
|
||||
}
|
||||
var many []string
|
||||
if err := json.Unmarshal(data, &many); err != nil {
|
||||
return xerrors.New("resource must be a string or an array of strings")
|
||||
}
|
||||
*r = resourceIdentifiers(many)
|
||||
return nil
|
||||
}
|
||||
|
||||
// authServerMetadata represents the response from an Authorization
|
||||
|
||||
@@ -2,6 +2,7 @@ package coderd
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -214,3 +215,55 @@ func TestOIDCMCPTokenSource(t *testing.T) {
|
||||
require.Empty(t, tok)
|
||||
})
|
||||
}
|
||||
|
||||
func TestProtectedResourceMetadataResourceUnmarshal(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
input string
|
||||
want resourceIdentifiers
|
||||
wantErr bool
|
||||
}{
|
||||
{
|
||||
name: "String",
|
||||
input: `{"resource": "https://gitlab.example.com/api/v4/mcp", "authorization_servers": ["https://gitlab.example.com"]}`,
|
||||
want: resourceIdentifiers{"https://gitlab.example.com/api/v4/mcp"},
|
||||
},
|
||||
{
|
||||
// GitLab's official MCP server returns an array of
|
||||
// resources despite RFC 9728 defining a string.
|
||||
name: "Array",
|
||||
input: `{"resource": ["https://gitlab.example.com/api/v4/mcp", "https://gitlab.example.com/api/v4/orbit/mcp"], "authorization_servers": ["https://gitlab.example.com"]}`,
|
||||
want: resourceIdentifiers{
|
||||
"https://gitlab.example.com/api/v4/mcp",
|
||||
"https://gitlab.example.com/api/v4/orbit/mcp",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Absent",
|
||||
input: `{"authorization_servers": ["https://gitlab.example.com"]}`,
|
||||
want: nil,
|
||||
},
|
||||
{
|
||||
name: "InvalidType",
|
||||
input: `{"resource": 42, "authorization_servers": ["https://gitlab.example.com"]}`,
|
||||
wantErr: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var meta protectedResourceMetadata
|
||||
err := json.Unmarshal([]byte(tt.input), &meta)
|
||||
if tt.wantErr {
|
||||
require.Error(t, err)
|
||||
return
|
||||
}
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, tt.want, meta.Resource)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user