Skip to content

Commit bfde902

Browse files
committed
oauth: return error instead of (nil, nil) from getServerMetadata
When metadata discovery hits a non-2xx response, fetchMetadataFromURL returns (nil, nil), leaving getServerMetadata returning (nil, nil) — no metadata and no error. Callers (RegisterClient, token/auth URL builders) then dereferenced the nil *AuthServerMetadata and panicked. Return an explicit error instead. Fixes #903.
1 parent d972f3f commit bfde902

2 files changed

Lines changed: 58 additions & 0 deletions

File tree

client/transport/oauth.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,13 @@ func (h *OAuthHandler) getServerMetadata(ctx context.Context) (*AuthServerMetada
577577
if h.metadataFetchErr != nil {
578578
return nil, h.metadataFetchErr
579579
}
580+
// A "no-op" discovery (e.g. a non-2xx response handled by
581+
// fetchMetadataFromURL) leaves serverMetadata nil without recording an
582+
// error. Returning (nil, nil) here makes every caller dereference a nil
583+
// *AuthServerMetadata and panic, so surface an explicit error instead.
584+
if h.serverMetadata == nil {
585+
return nil, fmt.Errorf("authorization server metadata unavailable: discovery returned no metadata")
586+
}
580587
return h.serverMetadata, nil
581588
}
582589

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package transport
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"testing"
7+
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
// Regression test for https://github.com/mark3labs/mcp-go/issues/903
12+
//
13+
// When metadata discovery hits a non-2xx response, fetchMetadataFromURL returns
14+
// (nil, nil), which previously left getServerMetadata returning (nil, nil) — no
15+
// metadata and no error. Callers (RegisterClient, token/auth URL builders) then
16+
// dereferenced the nil *AuthServerMetadata and panicked, crashing the process.
17+
// getServerMetadata must return a non-nil error instead of (nil, nil).
18+
func newOAuthHandlerWithUnavailableMetadata(t *testing.T) *OAuthHandler {
19+
t.Helper()
20+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
21+
w.WriteHeader(http.StatusNotFound)
22+
}))
23+
t.Cleanup(server.Close)
24+
25+
return NewOAuthHandler(OAuthConfig{
26+
ClientID: "test-client",
27+
RedirectURI: "http://localhost/callback",
28+
Scopes: []string{"mcp.read"},
29+
TokenStore: NewMemoryTokenStore(),
30+
AuthServerMetadataURL: server.URL + "/.well-known/oauth-authorization-server",
31+
})
32+
}
33+
34+
func TestOAuthHandler_GetServerMetadata_UnavailableReturnsError(t *testing.T) {
35+
handler := newOAuthHandlerWithUnavailableMetadata(t)
36+
37+
metadata, err := handler.GetServerMetadata(t.Context())
38+
39+
require.Error(t, err, "expected an error instead of (nil, nil) when discovery returns no metadata")
40+
require.Nil(t, metadata)
41+
}
42+
43+
func TestOAuthHandler_RegisterClient_UnavailableMetadataDoesNotPanic(t *testing.T) {
44+
handler := newOAuthHandlerWithUnavailableMetadata(t)
45+
46+
var err error
47+
require.NotPanics(t, func() {
48+
err = handler.RegisterClient(t.Context(), "test-client")
49+
})
50+
require.Error(t, err)
51+
}

0 commit comments

Comments
 (0)