auth: change OAuthHandler to take http Request and Response (#603)
Change OAuthHandler signature from func(context.Context, OAuthHandlerArgs) to func(req *http.Request, res *http.Response). - Remove OAuthHandlerArgs struct - Update HTTPTransport to pass req and resp to handler - Update tests to use new signature - Handler can now call oauthex.GetProtectedResourceMetadataFromHeader with proper validation against request URL This change fixes an impedance mismatch between OAuthHandler and the protected resource metadata functions of the oauthex package. The new signature allows handlers to properly validate resource metadata against the request URL, as required by RFC 9728. Fixes #600
This commit is contained in:
parent
d256a9c109
commit
72afbc91bd
2 changed files with 13 additions and 30 deletions
|
|
@ -8,26 +8,19 @@ package auth
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"bytes"
|
"bytes"
|
||||||
"context"
|
|
||||||
"errors"
|
"errors"
|
||||||
"io"
|
"io"
|
||||||
"net/http"
|
"net/http"
|
||||||
"sync"
|
"sync"
|
||||||
|
|
||||||
"github.com/modelcontextprotocol/go-sdk/oauthex"
|
|
||||||
"golang.org/x/oauth2"
|
"golang.org/x/oauth2"
|
||||||
)
|
)
|
||||||
|
|
||||||
// An OAuthHandler conducts an OAuth flow and returns a [oauth2.TokenSource] if the authorization
|
// An OAuthHandler conducts an OAuth flow and returns a [oauth2.TokenSource] if the authorization
|
||||||
// is approved, or an error if not.
|
// is approved, or an error if not.
|
||||||
type OAuthHandler func(context.Context, OAuthHandlerArgs) (oauth2.TokenSource, error)
|
// The handler receives the HTTP request and response that triggered the authentication flow.
|
||||||
|
// To obtain the protected resource metadata, call [oauthex.GetProtectedResourceMetadataFromHeader].
|
||||||
// OAuthHandlerArgs are arguments to an [OAuthHandler].
|
type OAuthHandler func(req *http.Request, res *http.Response) (oauth2.TokenSource, error)
|
||||||
type OAuthHandlerArgs struct {
|
|
||||||
// The URL to fetch protected resource metadata, extracted from the WWW-Authenticate header.
|
|
||||||
// Empty if not present or there was an error obtaining it.
|
|
||||||
ResourceMetadataURL string
|
|
||||||
}
|
|
||||||
|
|
||||||
// HTTPTransport is an [http.RoundTripper] that follows the MCP
|
// HTTPTransport is an [http.RoundTripper] that follows the MCP
|
||||||
// OAuth protocol when it encounters a 401 Unauthorized response.
|
// OAuth protocol when it encounters a 401 Unauthorized response.
|
||||||
|
|
@ -112,10 +105,7 @@ func (t *HTTPTransport) RoundTrip(req *http.Request) (*http.Response, error) {
|
||||||
// TODO: We hold the lock for the entire OAuth flow. This could be a long
|
// TODO: We hold the lock for the entire OAuth flow. This could be a long
|
||||||
// time. Is there a better way?
|
// time. Is there a better way?
|
||||||
if _, ok := t.opts.Base.(*oauth2.Transport); !ok {
|
if _, ok := t.opts.Base.(*oauth2.Transport); !ok {
|
||||||
authHeaders := resp.Header[http.CanonicalHeaderKey("WWW-Authenticate")]
|
ts, err := t.handler(req, resp)
|
||||||
ts, err := t.handler(req.Context(), OAuthHandlerArgs{
|
|
||||||
ResourceMetadataURL: extractResourceMetadataURL(authHeaders),
|
|
||||||
})
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
@ -131,11 +121,3 @@ func (t *HTTPTransport) RoundTrip(req *http.Request) (*http.Response, error) {
|
||||||
|
|
||||||
return t.opts.Base.RoundTrip(req)
|
return t.opts.Base.RoundTrip(req)
|
||||||
}
|
}
|
||||||
|
|
||||||
func extractResourceMetadataURL(authHeaders []string) string {
|
|
||||||
cs, err := oauthex.ParseWWWAuthenticate(authHeaders)
|
|
||||||
if err != nil {
|
|
||||||
return ""
|
|
||||||
}
|
|
||||||
return oauthex.ResourceMetadataURL(cs)
|
|
||||||
}
|
|
||||||
|
|
|
||||||
|
|
@ -7,7 +7,6 @@
|
||||||
package auth
|
package auth
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
|
|
@ -65,10 +64,11 @@ func TestHTTPTransport(t *testing.T) {
|
||||||
|
|
||||||
t.Run("successful auth flow", func(t *testing.T) {
|
t.Run("successful auth flow", func(t *testing.T) {
|
||||||
var handlerCalls int
|
var handlerCalls int
|
||||||
handler := func(ctx context.Context, args OAuthHandlerArgs) (oauth2.TokenSource, error) {
|
handler := func(req *http.Request, res *http.Response) (oauth2.TokenSource, error) {
|
||||||
handlerCalls++
|
handlerCalls++
|
||||||
if args.ResourceMetadataURL != "http://metadata.example.com" {
|
// Verify that the response has the expected WWW-Authenticate header
|
||||||
t.Errorf("handler got metadata URL %q, want %q", args.ResourceMetadataURL, "http://metadata.example.com")
|
if res.Header.Get("WWW-Authenticate") != `Bearer resource_metadata="http://metadata.example.com"` {
|
||||||
|
t.Errorf("handler got WWW-Authenticate header %q", res.Header.Get("WWW-Authenticate"))
|
||||||
}
|
}
|
||||||
return fakeTokenSource, nil
|
return fakeTokenSource, nil
|
||||||
}
|
}
|
||||||
|
|
@ -108,9 +108,10 @@ func TestHTTPTransport(t *testing.T) {
|
||||||
})
|
})
|
||||||
|
|
||||||
t.Run("request body is cloned", func(t *testing.T) {
|
t.Run("request body is cloned", func(t *testing.T) {
|
||||||
handler := func(ctx context.Context, args OAuthHandlerArgs) (oauth2.TokenSource, error) {
|
handler := func(req *http.Request, res *http.Response) (oauth2.TokenSource, error) {
|
||||||
if args.ResourceMetadataURL != "http://metadata.example.com" {
|
// Verify that the response has the expected WWW-Authenticate header
|
||||||
t.Errorf("handler got metadata URL %q, want %q", args.ResourceMetadataURL, "http://metadata.example.com")
|
if res.Header.Get("WWW-Authenticate") != `Bearer resource_metadata="http://metadata.example.com"` {
|
||||||
|
t.Errorf("handler got WWW-Authenticate header %q", res.Header.Get("WWW-Authenticate"))
|
||||||
}
|
}
|
||||||
return fakeTokenSource, nil
|
return fakeTokenSource, nil
|
||||||
}
|
}
|
||||||
|
|
@ -134,7 +135,7 @@ func TestHTTPTransport(t *testing.T) {
|
||||||
|
|
||||||
t.Run("handler returns error", func(t *testing.T) {
|
t.Run("handler returns error", func(t *testing.T) {
|
||||||
handlerErr := errors.New("user rejected auth")
|
handlerErr := errors.New("user rejected auth")
|
||||||
handler := func(ctx context.Context, args OAuthHandlerArgs) (oauth2.TokenSource, error) {
|
handler := func(req *http.Request, res *http.Response) (oauth2.TokenSource, error) {
|
||||||
return nil, handlerErr
|
return nil, handlerErr
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue