Skip to content

fix: clear stale metadataFetchErr on successful OAuth discovery fallback #801

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ezynda3 merged 4 commits into from
Apr 15, 2026
Merged

fix: clear stale metadataFetchErr on successful OAuth discovery fallback #801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1d32830
fix
MariaChrysafis Apr 14, 2026
172eece
fix
MariaChrysafis Apr 14, 2026
bf663c8
fix
MariaChrysafis Apr 14, 2026
b9f2111
fix
MariaChrysafis Apr 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions client/transport/oauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ func (h *OAuthHandler) getServerMetadata(ctx context.Context) (*AuthServerMetada
}
h.fetchMetadataFromURL(ctx, authMetadataURL)
if h.serverMetadata != nil {
h.metadataFetchErr = nil
return
}
// If that also fails, fall back to default endpoints
Expand All @@ -419,6 +420,7 @@ func (h *OAuthHandler) getServerMetadata(ctx context.Context) (*AuthServerMetada
return
}
h.serverMetadata = metadata
h.metadataFetchErr = nil
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return
}

Expand Down Expand Up @@ -447,6 +449,7 @@ func (h *OAuthHandler) getServerMetadata(ctx context.Context) (*AuthServerMetada
return
}
h.serverMetadata = metadata
h.metadataFetchErr = nil
return
}

Expand All @@ -461,6 +464,7 @@ func (h *OAuthHandler) getServerMetadata(ctx context.Context) (*AuthServerMetada
}
h.fetchMetadataFromURL(ctx, authMetadataURL)
if h.serverMetadata != nil {
h.metadataFetchErr = nil
return
}

Expand All @@ -472,6 +476,7 @@ func (h *OAuthHandler) getServerMetadata(ctx context.Context) (*AuthServerMetada
}
h.fetchMetadataFromURL(ctx, openidMetadataURL)
if h.serverMetadata != nil {
h.metadataFetchErr = nil
return
}

Expand All @@ -482,6 +487,7 @@ func (h *OAuthHandler) getServerMetadata(ctx context.Context) (*AuthServerMetada
return
}
h.serverMetadata = metadata
h.metadataFetchErr = nil
})

if h.metadataFetchErr != nil {
Expand Down
51 changes: 51 additions & 0 deletions client/transport/oauth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1704,3 +1704,54 @@ func TestOAuthHandler_RFC8707_ResourceParameter(t *testing.T) {
assert.Equal(t, server.URL, capturedResource, "resource should fall back to baseURL")
})
}

// TestOAuthHandler_GetServerMetadata_AuthServerReturnsHTML tests that when the
// authorization server's .well-known endpoint returns 200 with HTML (e.g. a login
// page) instead of JSON, the fallback chain is not poisoned and default endpoints
// are used successfully.
func TestOAuthHandler_GetServerMetadata_AuthServerReturnsHTML(t *testing.T) {
// Create a separate "auth server" that returns HTML at its .well-known endpoints
authServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Return 200 with HTML for all requests (simulating a login page)
w.Header().Set("Content-Type", "text/html; charset=utf-8")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte("<html><body>Login Page</body></html>"))
}))
defer authServer.Close()

// Create the MCP server that returns valid protected resource metadata
// pointing to the auth server above
var mcpServerURL string
mcpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == "/.well-known/oauth-protected-resource" {
w.Header().Set("Content-Type", "application/json")
_ = json.NewEncoder(w).Encode(map[string]any{
"resource": mcpServerURL,
"authorization_servers": []string{authServer.URL},
})
return
}
w.WriteHeader(http.StatusUnauthorized)
}))
mcpServerURL = mcpServer.URL
defer mcpServer.Close()

config := OAuthConfig{
ClientID: "test-client",
RedirectURI: mcpServer.URL + "/callback",
Scopes: []string{"mcp.read"},
TokenStore: NewMemoryTokenStore(),
PKCEEnabled: true,
}

handler := NewOAuthHandler(config)
handler.SetBaseURL(mcpServer.URL)

metadata, err := handler.GetServerMetadata(context.Background())
require.NoError(t, err, "Should fall back to default endpoints when auth server returns HTML")

// Verify default endpoints were derived from the auth server URL
assert.Equal(t, authServer.URL+"/authorize", metadata.AuthorizationEndpoint)
assert.Equal(t, authServer.URL+"/token", metadata.TokenEndpoint)
assert.Equal(t, authServer.URL+"/register", metadata.RegistrationEndpoint)
}
Loading