-
Notifications
You must be signed in to change notification settings - Fork 142
Description
Problem
ToolHive's dynamic client registration implementation violates RFC 7591 by sending the scope parameter as a JSON array instead of a space-delimited string.
Current Behavior
When performing dynamic client registration (RFC 7591), ToolHive sends:
{
"client_name": "ToolHive MCP Client",
"redirect_uris": ["http://localhost:8080/callback"],
"token_endpoint_auth_method": "none",
"grant_types": ["authorization_code"],
"response_types": ["code"],
"scope": ["openid", "profile", "email"]
}Expected Behavior per RFC 7591 Section 2
The scope field MUST be a space-delimited string:
{
"client_name": "ToolHive MCP Client",
"redirect_uris": ["http://localhost:8080/callback"],
"token_endpoint_auth_method": "none",
"grant_types": ["authorization_code"],
"response_types": ["code"],
"scope": "openid profile email"
}Specification Reference
RFC 7591, Section 2 states:
"String containing a space-separated list of scope values (as described in Section 3.3 of OAuth 2.0) that the client can use when requesting access tokens."
Impact
This affects users running:
- `thv proxy ` with OAuth/OIDC authentication
- `thv run` commands requiring OAuth to remote MCP servers
- Any scenario using dynamic client registration (when no `client_id`/`client_secret` are provided)
OAuth providers that strictly validate RFC 7591 compliance will reject the registration request.
Location in Code
The issue occurs in `pkg/auth/oauth/dynamic_registration.go`:
Line 40 - Request struct with array type:
```go
type DynamicClientRegistrationRequest struct {
// ...
Scopes []string `json:"scope,omitempty"`
}
```
Line 192 - Where the incorrect format is serialized and sent:
```go
requestBody, err := json.Marshal(request) // Marshals []string as JSON array
```
The tests already document this issue (lines 210-221, 243, 249 in `dynamic_registration_test.go`):
```go
// NOTE: This test documents current behavior where non-empty scopes are serialized
// as JSON arrays (e.g., ["openid", "profile"]), which violates RFC 7591 Section 2
// requirement for space-delimited strings (e.g., "openid profile").
//
// TODO: The RFC 7591 format violation for non-empty scopes should be addressed
```
Suggested Solution
Implement a custom `MarshalJSON` method for the `Scopes` field (similar to how `ScopeList` has a custom `UnmarshalJSON` for responses). This should:
- Convert `[]string{"openid", "profile"}` → `"openid profile"` when marshaling
- Handle empty/nil scopes correctly (omit field via `omitempty`)
Note on Response Handling
The response side is correctly implemented with the `ScopeList` type (lines 60-112) that flexibly handles both formats for compatibility with various OAuth providers.