extauth: add ClientCredentialsHandler for OAuth client credentials grant#895
extauth: add ClientCredentialsHandler for OAuth client credentials grant#895ravyg wants to merge 1 commit intomodelcontextprotocol:mainfrom
Conversation
013c785 to
0977382
Compare
8814ca6 to
6383b63
Compare
| // scopesFromChallenges extracts scope values from WWW-Authenticate challenges. | ||
| func scopesFromChallenges(cs []oauthex.Challenge) []string { | ||
| for _, c := range cs { | ||
| if s := c.Params["scope"]; s != "" { |
There was a problem hiding this comment.
Why the difference in implementation from the AuthorizationCodeHandler? I believe we should split scopes into a slice instead of interpreting the space separated list as a single string.
| // getProtectedResourceMetadata discovers Protected Resource Metadata (RFC 9728) | ||
| // from the request URL. It tries the resource_metadata URL from WWW-Authenticate | ||
| // challenges first, then falls back to well-known discovery. | ||
| func getProtectedResourceMetadata(ctx context.Context, wwwChallenges []oauthex.Challenge, mcpServerURL string, httpClient *http.Client) (*oauthex.ProtectedResourceMetadata, error) { |
There was a problem hiding this comment.
Is there a particular reason we're changing the logic compared to the AuthorizationCodeHandler? Why not just copy the logic, do we want different behavior?
| } | ||
| u.Path = "" | ||
| rootURL := u.String() | ||
| prm, err := oauthex.GetProtectedResourceMetadata( |
There was a problem hiding this comment.
Please keep the call on a single line.
| func TestNewClientCredentialsHandler_Validation(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| modify func(*ClientCredentialsHandlerConfig) *ClientCredentialsHandlerConfig |
There was a problem hiding this comment.
This doesn't need to be a function, just the config. You can change the function signature to take 0 arguments, create valid config inside the function and call the function immediately in the test case definition. This way the config creation is local and easy to reason about.
|
|
||
| // Set up a fake MCP server that returns PRM pointing to the auth server. | ||
| mcpMux := http.NewServeMux() | ||
| mcpMux.HandleFunc("/.well-known/oauth-protected-resource", func(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
Please use auth.ProtectedResourceMetadataHandler like in the authorization_code_test.go.
|
taking a look |
6383b63 to
4d96e06
Compare
|
found one issue, I am fixing it. |
Add an implementation of auth.OAuthHandler that uses the OAuth 2.0 Client Credentials grant (RFC 6749 Section 4.4) for service-to-service authentication with pre-registered credentials. This bypasses both dynamic client registration and the authorization code flow. The handler supports two modes: - Direct token endpoint URL - Metadata discovery via AuthServerURL (RFC 8414) Also adds client_credentials grant type support to the fake authorization server in internal/oauthtest for testing. Refs modelcontextprotocol#627
4d96e06 to
2b46590
Compare
Summary
ClientCredentialsHandlerimplementingauth.OAuthHandlerusing the OAuth 2.0 Client Credentials grant (RFC 6749 Section 4.4) for service-to-service authentication with pre-registered credentials.TokenEndpointURL, or metadata discovery viaAuthServerURL(RFC 8414).client_credentialsgrant type support to the fake authorization server ininternal/oauthtestfor testing.Context
Per @jba's comment on #627:
The client-side OAuth scaffolding (#785) that this was blocked on is now complete, as @brkane noted.
Implements the client credentials variant of SEP-1046. The JWT Assertions variant (RFC 7523) is left for a follow-up as its API surface needs more design discussion.
Test plan
TestNewClientCredentialsHandler_Validation— validates all config error cases (nil config, missing fields, mutual exclusivity)TestClientCredentialsHandler_Authorize/direct_token_endpoint— end-to-end with fake auth serverTestClientCredentialsHandler_Authorize/metadata_discovery— discovers token endpoint via RFC 8414 metadataTestClientCredentialsHandler_Authorize/bad_credentials— verifies failure with wrong secretgo test ./... -count=1passesgo vet ./...cleanRefs #627