Conversation
d40d41f to
ed54d46
Compare
ed54d46 to
f2e2ddb
Compare
|
Hi all, this proposal tries to address a long-standing gap in the SDK, which is client-side OAuth. I'm sure many people have worked around it and thus have an understanding what of the flow that will allow them to assess if this proposal would address their needs. Please review and provide your feedback! I will go through OAuth related issues and tag people that have been active there. Sorry for the noise if you're not interested. |
| h.tokenSource = ts | ||
| } | ||
|
|
||
| func (h *AuthorizationCodeOAuthHandler) FinalizeAuthorization(code, state string) error { |
There was a problem hiding this comment.
Developers are required to call this function to finalize the flow after they get the authorization code from the redirect. More details about the flow are present in the PR description.
|
The shape of this looks right to me, but I really have to defer to @jba, who has thought about this much more than me. |
e2bec4e to
21bf098
Compare
bd9ce4a to
3a9cbd0
Compare
3a9cbd0 to
18d5838
Compare
|
I have added an end-to-end example that uses the new APIs to showcase authorization. It's under I ran the example with a dev instance of the https://www.keycloak.org/ server. I was able to test a pre-registered client and dynamic client registration, as the server doesn't yet support client ID metadata documents. I would like to highlight my feedback request about the design of the part where an authorization URL is returned to the user. In this flow, the authorization server after getting user's consent, redirects to an URL specified by the client (likely a local URL) and this request contains the authorization code that our handler is able to exchange for a token. Currently this process happens with a return of control to the application: the SDK request fails with As mentioned before, the alternative could be to not return control to the caller and adjust |
|
Thanks for testing it for real! keycloak.org was a good find. I believe if you look at the TS or C# examples, they start a small local webserver that the auth server redirects to. That gets the code which can then be exchanged for a token, and then Authorize can return with a token. There should be a way for both flows to co-exist: one that returns control with an error, and one that does everything in a single call. |
It looks like you already have this. |
Hi @maciej-kisiel, I tried connecting example server with keycloak dev instance, it works from MCP inspector with connection type 'via Proxy', but does not when changed to 'direct'. From server logs I observed following - difference in logs: for 'via proxy': Apart from missing call to notifications/initialized, I did not see anything different. Is this expected behavior? |
Do you suggest we should adjust |
I read over the proposed API and example client before reading this comment, and this was going to be my top suggestion: The current control flow is quite complicated, and could be simplified by making some operations blocking instead of callback-based. Returning ErrRedirected from client.Connect means that the client user needs to not only retry the error, but must also synchronize with the authorization flow to identify when it has completed. This is necessary because the OAuthHandler interface provides a way to start authorization, but no way to tell when authorization has completed. I think OAuthHandler.Authorize should block until authorization has completed. Perhaps this leaves an additional goroutine running for the duration of authorization, but goroutines are cheap. Similarly, AuthorizationCodeOAuthHandler.AuthorizationURLHandler probably should block and return the code rather than returning it asynchronously. (But see the next section below.) AuthorizationCodeOAuthHandler.RedirectURL seems like it might be a problem to me: This must be set before AuthorizationURLHandler is called (because the redirect URL is part of the authorization URL), but the example client does not start the localhost HTTP server until AuthorizationURLHandler is called. This makes it impossible for the server to listen on a non-well-known port (notably, it can't listen on localhost:0) unless the listener is preemptively created even when the authorization flow does not need to execute. Perhaps there should be another type here, something along the lines of: type AuthorizationHandler interface {
// RedirectURL is the URL to redirect to after authorization.
RedirectURL() string
// Authorize handles the authorization URL.
Authorize(ctx context.Context, authorizationURL string) (code, state string, err error)
// Stop shuts down the localhost server (if one was started).
Stop()
}
type AuthorizationCodeOAuthHandler struct {
// NewAuthorizationHandler replaces RedirectURL and AuthorizationURLHandler.
NewAuthorizationHandler func() (AuthorizationHandler, error)
}Are most implementations of AuthorizationURLHandler going to use a localhost HTTP server? If so, it seems like we should provide a reasonable default implementation rather than requiring everyone to reimplement the same rather-subtle behavior. A few subtleties: The example client doesn't handle multiple visits to the localhost HTTP server. You can wedge the server by quickly sending it two requests, one of which will block on sending to r.authChan. If multiple clients copy the same example code, they'll all listen on the same port (localhost:3142) and potentially conflict with each other. I don't know if this is a problem in practice. The localhost server is started in a new goroutine, and there's a theoretical race condition where it hasn't started by the time the user visits it. (This seems fairly unlikely in practice.) This could be avoided by creating a net.Listener and then starting the server in a new goroutine with server.Serve instead of ListenAndServe. |

This PR introduces a proposal for how client-side OAuth support could look like in the SDK. It is open for community feedback.
The main goal of the PR is to validate the proposed APIs. The code may not be polished yet. In particular:
Overall design philosophy
The main new interface introduced to support client-side OAuth is the
OAuthHandler. Contrary to its counter-parts in some other SDKs, it is intentionally small and generic so that it can support most of the OAuth authorization flow variants. The mainmcppackage purposefully delegates almost all auth handling toOAuthHandlerimplementations. This way, themcppackage continues to be largely OAuth agnostic.The
authpackage comes with a defaultOAuthHandlerimplementation –AuthorizationCodeOAuthHandler, which implements the authorization flow for theauthorization_codegrant type, as defined in the MCP specification. It is expected that theOAuthHandlerimplementation granularity will be roughly per grant type, as this parameter influences the authorization flow the most.Detailed design
OAuthHandler(auth/client.go)The main interface for the integration consists of two methods:
StreamableClientTransportaccepts this interface as a field and uses theTokenSourcemethod to add anAuthorization: Bearerheader to outgoing requests to the MCP server. If the request fails with401: Unauthorizedor403: Forbidden, the transport callsAuthorizeto perform the authorization flow. IfAuthorizereturns a non-nil error, the request is retried once.Authorizeerrors do not result in termination of the client session (unless they happen during connection), as some OAuth flows are multi-legged and it should be acceptable to retry for example when the authorization grant was received.Note: There was already an
OAuthHandlertype defined as part of a previous iteration of OAuth support. It was renamed toOAuthHandlerLegacy. This is the only backwards incompatible API change in the experimental clientauthpart (protected by themcp_go_client_oauthGo build tag). The cost of replacing previous usages is deemed worthwhile to maintain clear naming, under the assumption that the experimental clientauthpart was not widely used.AuthorizationCodeOAuthHandler(auth/authorization_code.go)This is the
OAuthHandlerimplementation that fulfills the MCP specification. In particular, in supports:statefield generatorsnone,client_secret_post,client_secret_basictoken endpoint auth methodsAuthorization Server redirect
As part of the flow for the
authorization_codegrant type, the client needs to point the user to the authorization URL, so that they confirm the access request. This design proposes that it will be supported via a pluggable dependency that will initiate this process in an application-specific way:The function is supposed as soon as the user was pointed to the authorization URL, after which
Authorizewill finish withauth.ErrRedirected. The application is expected to wait for the callback and provide the required authorization code and state values to theAuthorizationCodeOAuthHandlerviaFinalizeAuthorizationcall. After that, the client call (or connection) can be retried.Token storage
AuthorizationCodeOAuthHandlerincludes aSetTokenSourcemethod that allows the application to pre-populate the handler with tokens, for example fetched from a persistent store. The token source may be overridden by the handler if the token from the existing token source does not authorize the user and the automatically initiatedAuthorizeflow succeeds. Optionally, aTokenStoreimplementation can be provided to the handler.When provided, the token source created by the handler on a successful token exchange will save each token it returns. The same effect can be achieved in a pre-populated token source thanks to an exposed
func NewPersistentTokenSource(ctx context.Context, wrapped oauth2.TokenSource, store TokenStore) oauth2.TokenSourceutility function.oauthexchangesSome additional/adjusted building blocks that may be useful when creating
OAuthHandlerimplementations are present in theoauthexpackage:auth_meta.go: Authorization Server Metadata utilities:GetAuthServerMeta: get Authorization Server Metadata from a URLAuthorizationServerMetadataURLs: generate ASM URL candidates based on MCP specificationdcr.go: Dynamic Client Registration utilities (unchanged)resource_meta.go: Protected Resource Metadata utilities:GetProtectedResourceMetadata: get Protected Resource Metadata from a URLProtectedResourceMetadataURLs: generate PRM URL candidates based on MCP specificationResourceMetadataURL,Scopes,Error– helpers to retrieve fields from theWWW-AuthenticatechallengesParseWWWAuthenticate– as per name (unchanged)GetProtectedResourceMetadataFromIDandGetProtectedResourceMetadataFromHeaderin favor of more genericGetProtectedResourceMetadata. They will be kept under the Go build tag and removed two versions of the SDK in the future.Input requested
Please provide any feedback/suggestions on the proposed API surface you might have. In particular:
ErrRedirectedand retrying the initial call at the application level. Some alternatives could be proposed, for example making the authorization URL handler blocking and returning authorization code and state directly (likely through a channel). It would simplify the control flow (no error returned fromAuthorizeand application-level retries) and make the implementation a bit cleaner, but would come at a cost of additional goroutines being created.Persisted token's refreshes: in order to create a refreshing token source based on a stored token it's required to create aoauth2.Configobject. Deriving the values to populate it is a significant part of theAuthorizationCodeOAuthHandlerimplementation and we can consider exposing some mechanisms to not require the users to replicate this logic.Exposed extension points forAuthorizationCodeOAuthHandler:TokenSourceandStateProvider– they were added in an anticipation of being useful and because other SDKs provide it, but it would be great to validate if that's really the case.Unauthorizederrors do not close the client session (with the exception of session connection that will fail). Is this the right approach? It's unclear what should happen when getting a handler's token source fails, feedback welcome.Suggest any real Authorization Server implementations that could be easily deployed in dev mode to facilitate end-to-end testing. So far the implementation was tested only with MCP conformance tests.Further steps:
mcp_go_client_oauthbuild tag to make CI passexamples/client(dev AS server would come in handy)Thank you!