extauth: add ClientCredentialsHandler for OAuth client credentials grant#895
extauth: add ClientCredentialsHandler for OAuth client credentials grant#895ravyg wants to merge 2 commits intomodelcontextprotocol:mainfrom
Conversation
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
ravyg
force-pushed
the
feat/627-oauth-client-credentials
branch
from
013c785 to
0977382
Compare
| // Use of this source code is governed by an MIT-style | ||
| // license that can be found in the LICENSE file. |
There was a problem hiding this comment.
| // Use of this source code is governed by an MIT-style | |
| // license that can be found in the LICENSE file. | |
| // Use of this source code is governed by the license | |
| // that can be found in the LICENSE file. |
Let's make sure we use the new version of the header in newly added files.
| return nil, fmt.Errorf("config must be provided") | ||
| } | ||
| if config.Credentials == nil { | ||
| return nil, fmt.Errorf("credentials is required") |
There was a problem hiding this comment.
| return nil, fmt.Errorf("credentials is required") | |
| return nil, fmt.Errorf("credentials are required") |
| httpClient = http.DefaultClient | ||
| } | ||
|
|
||
| tokenEndpoint := h.config.TokenEndpoint |
There was a problem hiding this comment.
When reading the specification for this method, my understanding is that it extends the general Authorization guidance from MCP. This would mean that we shouldn't have TokenEndpoint or AuthServerURL, but rather use the request URL to fetch PRM and then ASM like in `auth/authorization_code.go. The sequence diagram there confirms that fetching PRM is expected: https://github.com/modelcontextprotocol/ext-auth/blob/main/specification/draft/oauth-client-credentials.mdx#flow-steps.
In that scenario scopes are also coming from PRM and don't need to be specified explicitly on the handler config.
| } | ||
|
|
||
| creds := h.config.Credentials | ||
| cfg := &clientcredentials.Config{ |
There was a problem hiding this comment.
We should set AuthStyle based on ASM's token_endpoint_auth_methods_supported and our priority order (client_secret_post is preferred over client_secret_basic).
| // token endpoint auth methods from the authorization server metadata. | ||
| // Returns [oauth2.AuthStyleInHeader] (HTTP Basic) by default, which is the | ||
| // recommended method per RFC 6749 Section 2.3.1. | ||
| func AuthStyle(methods []string) oauth2.AuthStyle { |
There was a problem hiding this comment.
I assume you forgot to use this function to set the style in the config. Either way, please un-export this function and take a look at a similar logic we've introduced for authorization code handler. I believe it's slightly more correct.
go-sdk/auth/authorization_code.go
Lines 369 to 396 in 15e93a2
| }{ | ||
| { | ||
| name: "nil config", | ||
| config: nil, |
There was a problem hiding this comment.
Please take a look how the corresponding test cases are built for EnterpriseHandler: there is a reusable validXConfig function that produces configuration that is correct and each test case precisely overwrites the part that it targets. You don't have to rely on any ordering of validations with this strategy.
| if err == nil { | ||
| t.Fatalf("expected error containing %q, got nil", tc.wantError) | ||
| } | ||
| if got := err.Error(); !contains(got, tc.wantError) { |
There was a problem hiding this comment.
Please use strings.Contains.
| t.Fatalf("expected error containing %q, got nil", tc.wantError) | ||
| } | ||
| if got := err.Error(); !contains(got, tc.wantError) { | ||
| t.Fatalf("error %q does not contain %q", got, tc.wantError) |
There was a problem hiding this comment.
Nit: You can use Errorf here and in line 104.
| } | ||
|
|
||
| func TestClientCredentialsHandler_Authorize(t *testing.T) { | ||
| ctx := context.Background() |
|
|
||
| // ClientCredentialsEnabled enables support for the client_credentials | ||
| // grant type (RFC 6749 Section 4.4) on a [FakeAuthorizationServer]. | ||
| // When true, the /token endpoint accepts grant_type=client_credentials |
There was a problem hiding this comment.
This part should be moved to the doc comment of Enabled.
2 participants
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