Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions internal/mcp/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,20 @@ func discoverAuthorizationServer(ctx context.Context, client *http.Client, baseU
// overrides. Configured endpoints take precedence over discovered ones, and act
// as a fallback when discovery fails or omits a value.
func resolveAuthorizationServer(ctx context.Context, client *http.Client, baseURL string, cfg OAuthConfig) (authServerMetadata, error) {
// When the config supplies both the authorization and token endpoints
// directly, skip network discovery entirely: there is nothing to discover,
// and a hung/blocked discovery call (e.g. offline, or an unreachable issuer
// in tests) must not gate an otherwise fully-specified login. Discovery stays
// the fallback for any endpoint the config leaves blank.
if strings.TrimSpace(cfg.AuthorizationEndpoint) != "" && strings.TrimSpace(cfg.TokenEndpoint) != "" {
metadata := authServerMetadata{
AuthorizationEndpoint: strings.TrimSpace(cfg.AuthorizationEndpoint),
TokenEndpoint: strings.TrimSpace(cfg.TokenEndpoint),
RegistrationEndpoint: strings.TrimSpace(cfg.RegistrationEndpoint),
}
return metadata, nil
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

discoveryBase := strings.TrimSpace(cfg.IssuerURL)
if discoveryBase == "" {
discoveryBase = baseURL
Expand Down
48 changes: 41 additions & 7 deletions internal/mcp/oauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"crypto/sha256"
"encoding/base64"
"encoding/json"
"errors"
"net/http"
"net/http/httptest"
"net/url"
Expand Down Expand Up @@ -51,25 +52,58 @@ func TestDiscoverParsesMetadata(t *testing.T) {

func TestResolveEndpointsFallsBackToConfig(t *testing.T) {
// Server with no metadata document: discovery must fail and the resolver must
// fall back to the explicitly configured endpoints.
// fall back to the explicitly configured authorization endpoint. Only one
// endpoint is configured so the skip-discovery fast path does not fire;
// the token endpoint comes from discovery (which fails here), so the
// resolver must return an error for the missing token endpoint.
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.NotFound(w, r)
}))
defer server.Close()

cfg := OAuthConfig{
AuthorizationEndpoint: "https://issuer.example/authorize",
TokenEndpoint: "https://issuer.example/token",
}
meta, err := resolveAuthorizationServer(context.Background(), http.DefaultClient, server.URL, cfg)
_, err := resolveAuthorizationServer(context.Background(), http.DefaultClient, server.URL, cfg)
if err == nil {
t.Fatal("expected error for missing token endpoint when discovery fails, got nil")
}
if !strings.Contains(err.Error(), "token endpoint") {
t.Fatalf("error = %q, want it to mention token endpoint", err)
}
}

// roundTripperFunc adapts a function to http.RoundTripper so a test can
// observe every outbound request.
type roundTripperFunc func(*http.Request) (*http.Response, error)

func (f roundTripperFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) }

func TestResolveEndpointsSkipsDiscoveryWhenBothConfigured(t *testing.T) {
// When both endpoints are configured, discovery must be skipped entirely.
// The client fails any outbound request, so this test distinguishes the
// fast path from a discovery failure rescued by the config fallback.
client := &http.Client{Transport: roundTripperFunc(func(r *http.Request) (*http.Response, error) {
t.Errorf("unexpected HTTP request to %s: discovery should be skipped", r.URL)
return nil, errors.New("discovery must be skipped")
})}
cfg := OAuthConfig{
AuthorizationEndpoint: " https://issuer.example/authorize ",
TokenEndpoint: " https://issuer.example/token ",
RegistrationEndpoint: " https://issuer.example/register ",
}
meta, err := resolveAuthorizationServer(context.Background(), client, "https://issuer.example", cfg)
if err != nil {
t.Fatalf("resolveAuthorizationServer() error = %v", err)
}
if meta.AuthorizationEndpoint != cfg.AuthorizationEndpoint {
t.Fatalf("authorization endpoint = %q, want config fallback", meta.AuthorizationEndpoint)
if meta.AuthorizationEndpoint != "https://issuer.example/authorize" {
t.Fatalf("authorization endpoint = %q, want trimmed config value", meta.AuthorizationEndpoint)
}
if meta.TokenEndpoint != cfg.TokenEndpoint {
t.Fatalf("token endpoint = %q, want config fallback", meta.TokenEndpoint)
if meta.TokenEndpoint != "https://issuer.example/token" {
t.Fatalf("token endpoint = %q, want trimmed config value", meta.TokenEndpoint)
}
if meta.RegistrationEndpoint != "https://issuer.example/register" {
t.Fatalf("registration endpoint = %q, want trimmed config value", meta.RegistrationEndpoint)
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

Expand Down
2 changes: 1 addition & 1 deletion internal/tui/picker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,7 @@ func TestSwitchProviderModelRecordsRecentHistory(t *testing.T) {
},
})

next, status, _ := m.switchProviderModel("ollama", "kimi-k2.7-code:cloud")
next, status, _, _ := m.switchProviderModel("ollama", "kimi-k2.7-code:cloud")
wantStatus := "Model\nSwitched to ollama · kimi-k2.7-code:cloud"
if status != wantStatus {
t.Fatalf("switchProviderModel() status = %q, want %q (a mismatch here means the switch itself failed, not the recentModels assertion below)", status, wantStatus)
Expand Down
Loading