diff --git a/cmd/mcp_auth.go b/cmd/mcp_auth.go new file mode 100644 index 000000000..01c3cc761 --- /dev/null +++ b/cmd/mcp_auth.go @@ -0,0 +1,218 @@ +package cmd + +import ( + "context" + "errors" + "fmt" + "html/template" + "net" + "net/http" + "sort" + "strings" + "time" + + "github.com/samsaffron/term-llm/internal/mcp" + mcpoauth "github.com/samsaffron/term-llm/internal/mcp/oauth" + internalauth "github.com/samsaffron/term-llm/internal/oauth" + "github.com/spf13/cobra" +) + +var ( + mcpLoginForce bool + mcpLoginNoBrowser bool + mcpLogoutLocal bool +) + +var mcpLoginCmd = &cobra.Command{ + Use: "login ", + Short: "Sign in to a remote MCP server", + Args: cobra.ExactArgs(1), + ValidArgsFunction: MCPServerArgCompletion, + RunE: mcpLogin, +} + +var mcpStatusCmd = &cobra.Command{ + Use: "status [name]", + Short: "Show MCP transport and authentication status", + Args: cobra.MaximumNArgs(1), + ValidArgsFunction: MCPServerArgCompletion, + RunE: mcpStatus, +} + +var mcpLogoutCmd = &cobra.Command{ + Use: "logout ", + Short: "Revoke and remove an MCP OAuth grant", + Args: cobra.ExactArgs(1), + ValidArgsFunction: MCPServerArgCompletion, + RunE: mcpLogout, +} + +func init() { + mcpLoginCmd.Flags().BoolVar(&mcpLoginForce, "force", false, "Sign in again even when the current grant is valid") + mcpLoginCmd.Flags().BoolVar(&mcpLoginNoBrowser, "no-browser", false, "Print the authorization URL without opening a browser") + mcpLogoutCmd.Flags().BoolVar(&mcpLogoutLocal, "local-only", false, "Remove local credentials without remote revocation") + mcpCmd.AddCommand(mcpLoginCmd, mcpStatusCmd, mcpLogoutCmd) +} + +func loadMCPAuthManager() (*mcp.Manager, error) { + manager := mcp.NewManager() + if err := manager.LoadConfig(); err != nil { + return nil, fmt.Errorf("load MCP config: %w", err) + } + return manager, nil +} + +func mcpLogin(cmd *cobra.Command, args []string) error { + name := args[0] + manager, err := loadMCPAuthManager() + if err != nil { + return err + } + statuses := manager.AuthStatuses() + status, ok := statuses[name] + if !ok { + return fmt.Errorf("unknown MCP server: %s", name) + } + if status.State == mcpoauth.AuthNotNeeded { + return fmt.Errorf("MCP server %s does not use automatic OAuth", name) + } + if status.State == mcpoauth.AuthSignedIn && !mcpLoginForce { + fmt.Fprintln(cmd.OutOrStdout(), "Already signed in") + return nil + } + + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + return fmt.Errorf("start OAuth callback listener: %w", err) + } + defer listener.Close() + redirectURL := "http://" + listener.Addr().String() + "/callback" + server := &http.Server{ReadHeaderTimeout: 5 * time.Second} + server.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/callback" { + http.NotFound(w, r) + return + } + _, accepted := mcpoauth.DefaultCoordinator().CompleteCallback( + r.URL.Query().Get("state"), r.URL.Query().Get("code"), + r.URL.Query().Get("iss"), r.URL.Query().Get("error"), + ) + if !accepted { + http.Error(w, "This authorization callback is invalid, expired, or was already used.", http.StatusBadRequest) + return + } + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.Header().Set("Cache-Control", "no-store") + _ = template.Must(template.New("done").Parse(`Connected

Connected

You can close this window.

`)).Execute(w, nil) + }) + go func() { _ = server.Serve(listener) }() + defer server.Shutdown(context.Background()) + + startCtx, cancel := context.WithTimeout(cmd.Context(), 45*time.Second) + defer cancel() + flow, err := manager.StartOAuth(startCtx, name, mcp.OAuthStartOptions{RedirectURL: redirectURL, Force: mcpLoginForce}) + if err != nil { + return fmt.Errorf("start MCP sign-in: %w", err) + } + fmt.Fprintf(cmd.OutOrStdout(), "Open this URL to authorize %s:\n%s\n", name, flow.AuthorizationURL) + if !mcpLoginNoBrowser { + if err := internalauth.OpenBrowser(flow.AuthorizationURL); err != nil { + fmt.Fprintf(cmd.ErrOrStderr(), "Could not open a browser: %v\n", err) + } + } + fmt.Fprintln(cmd.OutOrStdout(), "Waiting for authorization…") + + waitCtx, waitCancel := context.WithTimeout(cmd.Context(), 10*time.Minute) + defer waitCancel() + completed, err := mcpoauth.DefaultCoordinator().Wait(waitCtx, flow.ID) + if err != nil { + if errors.Is(err, context.DeadlineExceeded) { + return fmt.Errorf("authorization timed out") + } + return fmt.Errorf("wait for authorization: %w", err) + } + switch completed.State { + case mcpoauth.FlowSucceeded: + fmt.Fprintf(cmd.OutOrStdout(), "Signed in to %s\n", name) + return nil + case mcpoauth.FlowCanceled: + return fmt.Errorf("authorization canceled") + case mcpoauth.FlowExpired: + return fmt.Errorf("authorization timed out") + default: + if completed.Error == "" { + return fmt.Errorf("authorization failed") + } + return fmt.Errorf("authorization failed: %s", completed.Error) + } +} + +func mcpStatus(cmd *cobra.Command, args []string) error { + manager, err := loadMCPAuthManager() + if err != nil { + return err + } + statuses := manager.AuthStatuses() + names := manager.AvailableServers() + if len(args) == 1 { + if _, ok := statuses[args[0]]; !ok { + return fmt.Errorf("unknown MCP server: %s", args[0]) + } + names = []string{args[0]} + } + sort.Strings(names) + for i, name := range names { + cfg := manager.Config().Servers[name] + status := statuses[name] + if i > 0 { + fmt.Fprintln(cmd.OutOrStdout()) + } + fmt.Fprintf(cmd.OutOrStdout(), "%s\n transport: %s\n authentication: %s\n", name, cfg.TransportType(), authStateLabel(status.State)) + if status.Issuer != "" { + fmt.Fprintf(cmd.OutOrStdout(), " issuer: %s\n", status.Issuer) + } + if len(status.Scopes) > 0 { + fmt.Fprintf(cmd.OutOrStdout(), " scopes: %s\n", strings.Join(status.Scopes, " ")) + } + if !status.ExpiresAt.IsZero() { + fmt.Fprintf(cmd.OutOrStdout(), " expires: %s\n", status.ExpiresAt.Local().Format(time.RFC3339)) + } + if status.StoragePath != "" { + fmt.Fprintf(cmd.OutOrStdout(), " storage: %s\n", status.StoragePath) + } + } + return nil +} + +func authStateLabel(state mcpoauth.AuthState) string { + switch state { + case mcpoauth.AuthNotNeeded: + return "not needed" + case mcpoauth.AuthSignedIn: + return "signed in" + case mcpoauth.AuthExpired: + return "expired (refreshable)" + case mcpoauth.AuthRequired: + return "needs sign-in" + case mcpoauth.AuthWaiting: + return "waiting for browser" + case mcpoauth.AuthRetry: + return "temporary refresh failure (retry)" + default: + return "signed out" + } +} + +func mcpLogout(cmd *cobra.Command, args []string) error { + manager, err := loadMCPAuthManager() + if err != nil { + return err + } + ctx, cancel := context.WithTimeout(cmd.Context(), 30*time.Second) + defer cancel() + if err := manager.LogoutOAuth(ctx, args[0], mcpLogoutLocal); err != nil { + return fmt.Errorf("sign out of MCP server: %w", err) + } + fmt.Fprintf(cmd.OutOrStdout(), "Signed out of %s\n", args[0]) + return nil +} diff --git a/cmd/mcp_test.go b/cmd/mcp_test.go index 563a7033b..02abf9a11 100644 --- a/cmd/mcp_test.go +++ b/cmd/mcp_test.go @@ -12,6 +12,27 @@ import ( "github.com/spf13/cobra" ) +func TestMCPStatusReportsStaticAuthWithoutSecrets(t *testing.T) { + configHome := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", configHome) + cfg := &mcp.Config{Servers: map[string]mcp.ServerConfig{ + "static": {Type: "http", URL: "https://mcp.example/mcp", Headers: map[string]string{"Authorization": "Bearer must-not-print"}}, + }} + if err := cfg.Save(); err != nil { + t.Fatal(err) + } + var output bytes.Buffer + cmd := &cobra.Command{} + cmd.SetOut(&output) + if err := mcpStatus(cmd, []string{"static"}); err != nil { + t.Fatal(err) + } + got := output.String() + if !strings.Contains(got, "authentication: not needed") || strings.Contains(got, "must-not-print") { + t.Fatalf("status output = %q", got) + } +} + func TestMCPRunArgCompletionDoesNotStartServerOnCacheMiss(t *testing.T) { configHome := t.TempDir() t.Setenv("XDG_CONFIG_HOME", configHome) diff --git a/cmd/serve.go b/cmd/serve.go index 6805d2865..0a30357a5 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -9,6 +9,7 @@ import ( "io" "log" "net/http" + "net/url" "os" "strings" "sync" @@ -40,6 +41,7 @@ var ( serveAllowNoAuth bool serveAuthMode string serveBasePath string + servePublicURL string serveTitle string serveDisableLocationSharing bool serveCORSOrigins []string @@ -155,6 +157,7 @@ func init() { _ = serveCmd.Flags().MarkHidden("allow-no-auth") serveCmd.Flags().StringVar(&serveAuthMode, "auth", "bearer", "Auth mode: bearer or none") serveCmd.Flags().StringVar(&serveBasePath, "base-path", "/ui", "URL prefix the UI uses for session URLs (e.g. /chat)") + serveCmd.Flags().StringVar(&servePublicURL, "public-url", "", "Browser-visible URL for OAuth callbacks (defaults to $TERM_LLM_SERVE_PUBLIC_URL or the authenticated request origin)") serveCmd.Flags().StringVar(&serveTitle, "title", "", "Override the web UI sidebar title (defaults to agent name or Chat)") serveCmd.Flags().BoolVar(&serveDisableLocationSharing, "disable-location-sharing", false, "Hide the web UI action for sharing the browser's current location") serveCmd.Flags().StringArrayVar(&serveCORSOrigins, "cors-origin", nil, "Allowed CORS origin (repeatable, or '*' for all)") @@ -387,6 +390,13 @@ func runServeLegacy(parentCtx context.Context, cmd *cobra.Command, args []string if err != nil { return err } + if strings.TrimSpace(servePublicURL) == "" { + servePublicURL = strings.TrimSpace(os.Getenv("TERM_LLM_SERVE_PUBLIC_URL")) + } + servePublicURL, err = normalizeServePublicURL(servePublicURL) + if err != nil { + return fmt.Errorf("invalid serve --public-url: %w", err) + } resolvedTitle := strings.TrimSpace(serveTitle) if !cmd.Flags().Changed("title") { @@ -747,6 +757,7 @@ func runServeLegacy(parentCtx context.Context, cmd *cobra.Command, args []string suppressServerTools: serveFilterServerTools, verbose: serveVerbose, basePath: serveBasePath, + publicURL: servePublicURL, uiTitle: resolvedTitle, locationSharingDisabled: locationSharingDisabled, sidebarSessions: append([]string(nil), sidebarSessions...), @@ -1133,6 +1144,7 @@ type serveServerConfig struct { suppressServerTools bool verbose bool basePath string // e.g. "/ui" or "/chat", always without trailing slash + publicURL string // explicit browser-visible origin + optional prefix for OAuth callbacks uiTitle string locationSharingDisabled bool sidebarSessions []string @@ -1246,6 +1258,26 @@ func resolveServeWriteDirs(cliWriteDirs []string, cfg *config.Config) []string { return out } +func normalizeServePublicURL(raw string) (string, error) { + raw = strings.TrimSpace(raw) + if raw == "" { + return "", nil + } + u, err := url.Parse(raw) + if err != nil { + return "", err + } + if (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" || u.User != nil { + return "", fmt.Errorf("must be an http(s) URL with a host") + } + if u.RawQuery != "" || u.Fragment != "" { + return "", fmt.Errorf("must not contain a query or fragment") + } + u.Path = strings.TrimRight(u.Path, "/") + u.RawPath = "" + return strings.TrimRight(u.String(), "/"), nil +} + // normalizeBasePath validates and normalizes a base-path value. // It ensures a leading slash, strips trailing slashes, and rejects // empty or root-only paths (use the default "/ui" instead of "/"). @@ -1431,6 +1463,10 @@ func (s *serveServer) httpHandler() http.Handler { inner.HandleFunc("/v1/sidebar", s.auth(s.cors(s.handleSidebar))) inner.HandleFunc("/v1/events", s.auth(s.cors(s.handleEvents))) inner.HandleFunc("/v1/events/poll", s.auth(s.cors(s.handleEventPoll))) + inner.HandleFunc("/v1/mcp/oauth/flows/", s.auth(s.cors(s.handleMCPOAuthFlow))) + // OAuth callbacks cannot carry the serve bearer token. A high-entropy, + // single-use SDK state value is the capability checked by this handler. + inner.HandleFunc("/v1/mcp/oauth/callback", s.handleMCPOAuthCallback) inner.HandleFunc("/v1/sessions/status", s.auth(s.cors(s.handleSessionsStatus))) inner.HandleFunc("/v1/sessions/search", s.auth(s.cors(s.handleSessionsSearch))) inner.HandleFunc("/v1/worktrees/diff", s.auth(s.cors(s.handleWorktreeDiff))) diff --git a/cmd/serve_handlers.go b/cmd/serve_handlers.go index 050aba98a..5a8bae4af 100644 --- a/cmd/serve_handlers.go +++ b/cmd/serve_handlers.go @@ -2040,6 +2040,11 @@ func (s *serveServer) handleSessionByID(w http.ResponseWriter, r *http.Request) return } + if serverName, action, ok := parseSessionMCPOAuthSuffix(suffix); ok { + s.handleSessionMCPOAuth(w, r, sessionID, serverName, action) + return + } + if suffix == "mcp" { if r.Method != http.MethodGet && r.Method != http.MethodPatch { w.Header().Set("Allow", "GET, PATCH") diff --git a/cmd/serve_mcp_handler.go b/cmd/serve_mcp_handler.go index 63ee69c9e..50b984b33 100644 --- a/cmd/serve_mcp_handler.go +++ b/cmd/serve_mcp_handler.go @@ -12,6 +12,7 @@ import ( "github.com/samsaffron/term-llm/internal/llm" "github.com/samsaffron/term-llm/internal/mcp" + mcpoauth "github.com/samsaffron/term-llm/internal/mcp/oauth" "github.com/samsaffron/term-llm/internal/session" "github.com/samsaffron/term-llm/internal/tooldiscovery" ) @@ -30,6 +31,12 @@ type serveMCPServerView struct { Deferred int `json:"deferred,omitempty"` LoadingMode string `json:"loading_mode,omitempty"` LastRefresh time.Time `json:"last_refresh,omitempty"` + AuthState string `json:"auth_state"` + AuthIssuer string `json:"auth_issuer,omitempty"` + AuthScopes []string `json:"auth_scopes,omitempty"` + AuthExpiresAt time.Time `json:"auth_expires_at,omitempty"` + CanSignIn bool `json:"can_sign_in"` + CanSignOut bool `json:"can_sign_out"` } type serveMCPDiscoveryView struct { @@ -161,6 +168,7 @@ func buildServeMCPState(manager *mcp.Manager, engine *llm.Engine, mcpSetting, se enabled := normalizeMCPSelection(append(manager.EnabledServers(), parseServerList(mcpSetting)...)) enabledSet := stringSet(enabled) states := make(map[string]mcp.ServerState) + authStatuses := manager.AuthStatuses() for _, state := range manager.GetAllStates() { states[state.Name] = state } @@ -195,6 +203,16 @@ func buildServeMCPState(manager *mcp.Manager, engine *llm.Engine, mcpSetting, se if err != nil { view.Error = err.Error() } + if authStatus, ok := authStatuses[name]; ok { + view.AuthState = string(authStatus.State) + view.AuthIssuer = authStatus.Issuer + view.AuthScopes = append([]string(nil), authStatus.Scopes...) + view.AuthExpiresAt = authStatus.ExpiresAt + view.CanSignIn = authStatus.CanSignIn + view.CanSignOut = authStatus.CanSignOut + } else { + view.AuthState = string(mcpoauth.AuthNotNeeded) + } if state, ok := states[name]; ok { view.LastRefresh = state.LastToolRefresh if state.RefreshError != nil { diff --git a/cmd/serve_mcp_oauth.go b/cmd/serve_mcp_oauth.go new file mode 100644 index 000000000..6035389f2 --- /dev/null +++ b/cmd/serve_mcp_oauth.go @@ -0,0 +1,235 @@ +package cmd + +import ( + "context" + "encoding/json" + "fmt" + "html" + "net/http" + "net/url" + "strings" + "time" + + "github.com/samsaffron/term-llm/internal/mcp" + mcpoauth "github.com/samsaffron/term-llm/internal/mcp/oauth" + "github.com/samsaffron/term-llm/internal/terminaltext" +) + +type serveMCPOAuthStartRequest struct { + Force bool `json:"force"` +} + +type serveMCPOAuthCancelRequest struct { + FlowID string `json:"flow_id"` +} + +func parseSessionMCPOAuthSuffix(suffix string) (server, action string, ok bool) { + parts := strings.Split(suffix, "/") + if len(parts) < 3 || parts[0] != "mcp" || parts[1] == "" || parts[2] != "oauth" { + return "", "", false + } + decoded, err := url.PathUnescape(parts[1]) + if err != nil || decoded == "" || strings.Contains(decoded, "/") { + return "", "", false + } + if len(parts) == 3 { + return decoded, "logout", true + } + if len(parts) == 4 && (parts[3] == "start" || parts[3] == "cancel") { + return decoded, parts[3], true + } + return "", "", false +} + +func (s *serveServer) handleSessionMCPOAuth(w http.ResponseWriter, r *http.Request, sessionID, serverName, action string) { + wantMethod := http.MethodPost + if action == "logout" { + wantMethod = http.MethodDelete + } + if r.Method != wantMethod { + w.Header().Set("Allow", wantMethod) + writeOpenAIError(w, http.StatusMethodNotAllowed, "invalid_request_error", "method not allowed") + return + } + if s.sessionMgr == nil { + writeOpenAIError(w, http.StatusNotFound, "not_found_error", "session runtime is unavailable") + return + } + rt, err := s.sessionMgr.GetOrCreate(r.Context(), sessionID) + if err != nil || rt == nil { + status := http.StatusInternalServerError + if err != nil && strings.Contains(err.Error(), "busy") { + status = http.StatusConflict + } + writeOpenAIError(w, status, "server_error", "session runtime is unavailable") + return + } + if rt.hasActiveRun() || !rt.mu.TryLock() { + writeOpenAIError(w, http.StatusConflict, "conflict_error", "cannot change MCP authentication while a response is running") + return + } + defer rt.mu.Unlock() + if rt.hasActiveRun() { + writeOpenAIError(w, http.StatusConflict, "conflict_error", "cannot change MCP authentication while a response is running") + return + } + if err := rt.ensureMCPManagerLocked(); err != nil { + writeOpenAIError(w, http.StatusInternalServerError, "server_error", err.Error()) + return + } + if _, ok := rt.mcpManager.Config().Servers[serverName]; !ok { + writeOpenAIError(w, http.StatusNotFound, "not_found_error", "MCP server is not configured") + return + } + + switch action { + case "start": + var req serveMCPOAuthStartRequest + if r.Body != nil && r.ContentLength != 0 { + if err := decodeJSONBody(r, &req); err != nil { + writeOpenAIError(w, http.StatusBadRequest, "invalid_request_error", err.Error()) + return + } + } + redirectURL, err := s.mcpOAuthCallbackURL(r) + if err != nil { + writeOpenAIError(w, http.StatusBadRequest, "invalid_request_error", err.Error()) + return + } + startCtx, cancel := context.WithTimeout(r.Context(), 45*time.Second) + defer cancel() + selected := containsMCPServer(parseServerList(rt.mcpSetting), serverName) + if !selected && s.store != nil { + if session, getErr := s.store.Get(r.Context(), sessionID); getErr == nil && session != nil { + selected = containsMCPServer(parseServerList(session.MCP), serverName) + } + } + flow, err := rt.mcpManager.StartOAuth(startCtx, serverName, mcp.OAuthStartOptions{ + RedirectURL: redirectURL, Force: req.Force, SkipReconnect: true, + }) + if err != nil { + writeOpenAIError(w, http.StatusBadGateway, "oauth_error", safeServeOAuthError(err)) + return + } + go s.publishMCPOAuthCompletion(sessionID, serverName, flow.ID, rt.mcpManager, selected) + writeJSON(w, http.StatusAccepted, flow) + case "cancel": + var req serveMCPOAuthCancelRequest + if err := decodeJSONBody(r, &req); err != nil { + writeOpenAIError(w, http.StatusBadRequest, "invalid_request_error", err.Error()) + return + } + if req.FlowID == "" { + writeOpenAIError(w, http.StatusBadRequest, "invalid_request_error", "flow_id is required") + return + } + if err := rt.mcpManager.CancelOAuth(serverName, req.FlowID); err != nil { + writeOpenAIError(w, http.StatusConflict, "oauth_error", safeServeOAuthError(err)) + return + } + writeJSON(w, http.StatusOK, map[string]bool{"canceled": true}) + case "logout": + ctx, cancel := context.WithTimeout(r.Context(), 30*time.Second) + defer cancel() + if err := rt.mcpManager.LogoutOAuth(ctx, serverName, false); err != nil { + writeOpenAIError(w, http.StatusBadGateway, "oauth_error", safeServeOAuthError(err)) + return + } + s.publishEvent(serveEventInput{Type: serveEventSessionRuntimeChanged, SessionID: sessionID, Reason: "mcp_oauth"}) + writeJSON(w, http.StatusOK, map[string]bool{"signed_out": true}) + } +} + +func (s *serveServer) publishMCPOAuthCompletion(sessionID, serverName, flowID string, manager *mcp.Manager, selected bool) { + ctx, cancel := context.WithTimeout(context.Background(), 11*time.Minute) + defer cancel() + flow, err := mcpoauth.DefaultCoordinator().Wait(ctx, flowID) + if err == nil && flow != nil && flow.State == mcpoauth.FlowSucceeded { + if selected && manager != nil { + _ = manager.Restart(context.Background(), serverName) + } + s.publishEvent(serveEventInput{Type: serveEventSessionRuntimeChanged, SessionID: sessionID, Reason: "mcp_oauth"}) + } +} + +func containsMCPServer(names []string, target string) bool { + for _, name := range names { + if name == target { + return true + } + } + return false +} + +func (s *serveServer) handleMCPOAuthFlow(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + w.Header().Set("Allow", "GET") + writeOpenAIError(w, http.StatusMethodNotAllowed, "invalid_request_error", "method not allowed") + return + } + id := strings.TrimPrefix(r.URL.Path, "/v1/mcp/oauth/flows/") + if id == "" || strings.Contains(id, "/") { + http.NotFound(w, r) + return + } + flow, ok := mcpoauth.DefaultCoordinator().Flow(id) + if !ok { + writeOpenAIError(w, http.StatusNotFound, "not_found_error", "OAuth flow was not found") + return + } + writeJSON(w, http.StatusOK, flow) +} + +func (s *serveServer) handleMCPOAuthCallback(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + w.Header().Set("Allow", "GET") + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + flowID, accepted := mcpoauth.DefaultCoordinator().CompleteCallback( + r.URL.Query().Get("state"), r.URL.Query().Get("code"), + r.URL.Query().Get("iss"), r.URL.Query().Get("error"), + ) + if !accepted { + http.Error(w, "This authorization callback is invalid, expired, or was already used.", http.StatusBadRequest) + return + } + ok := r.URL.Query().Get("error") == "" && r.URL.Query().Get("code") != "" + flowJSON, _ := json.Marshal(flowID) + okJSON, _ := json.Marshal(ok) + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.Header().Set("Cache-Control", "no-store") + w.Header().Set("Content-Security-Policy", "default-src 'none'; script-src 'unsafe-inline'; style-src 'unsafe-inline'") + heading := "Connected" + detail := "You can close this window." + if !ok { + heading, detail = "Authorization not completed", "Return to term-llm and try again." + } + fmt.Fprintf(w, `%s

%s

%s

`, html.EscapeString(heading), html.EscapeString(heading), html.EscapeString(detail), flowJSON, okJSON) +} + +func (s *serveServer) mcpOAuthCallbackURL(r *http.Request) (string, error) { + const callbackPath = "/v1/mcp/oauth/callback" + if s.cfg.publicURL != "" { + return strings.TrimRight(s.cfg.publicURL, "/") + callbackPath, nil + } + if r.Host == "" { + return "", fmt.Errorf("request host is missing; configure serve --public-url") + } + scheme := "http" + if r.TLS != nil { + scheme = "https" + } + return scheme + "://" + r.Host + s.cfg.basePath + callbackPath, nil +} + +func safeServeOAuthError(err error) string { + if err == nil { + return "" + } + text := terminaltext.SanitizeSingleLine(err.Error()) + if len(text) > 300 { + text = text[:300] + "…" + } + return text +} diff --git a/cmd/serve_mcp_oauth_test.go b/cmd/serve_mcp_oauth_test.go new file mode 100644 index 000000000..6b94e4a74 --- /dev/null +++ b/cmd/serve_mcp_oauth_test.go @@ -0,0 +1,85 @@ +package cmd + +import ( + "crypto/tls" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestServeMCPOAuthRoutesProtectFlowButNotStateGatedCallback(t *testing.T) { + s := &serveServer{cfg: serveServerConfig{basePath: "/ui", requireAuth: true, token: "serve-token"}} + handler := s.httpHandler() + + flowReq := httptest.NewRequest("GET", "http://example.test/ui/v1/mcp/oauth/flows/unknown", nil) + flowRec := httptest.NewRecorder() + handler.ServeHTTP(flowRec, flowReq) + if flowRec.Code != http.StatusUnauthorized { + t.Fatalf("unauthenticated flow status = %d, want 401", flowRec.Code) + } + + callbackReq := httptest.NewRequest("GET", "http://hostile.test/ui/v1/mcp/oauth/callback?state=invalid&code=secret-code&redirect_uri=https://evil.example", nil) + callbackRec := httptest.NewRecorder() + handler.ServeHTTP(callbackRec, callbackReq) + if callbackRec.Code != http.StatusBadRequest { + t.Fatalf("state-gated callback status = %d, want 400 (not auth rejection)", callbackRec.Code) + } + body := callbackRec.Body.String() + if strings.Contains(body, "secret-code") || strings.Contains(body, "evil.example") { + t.Fatalf("callback reflected request secrets: %q", body) + } +} + +func TestServeMCPOAuthCallbackURL(t *testing.T) { + tests := []struct { + name string + publicURL string + basePath string + host string + tls bool + want string + }{ + {name: "derived http", basePath: "/ui", host: "127.0.0.1:8080", want: "http://127.0.0.1:8080/ui/v1/mcp/oauth/callback"}, + {name: "derived https", basePath: "/chat", host: "chat.example", tls: true, want: "https://chat.example/chat/v1/mcp/oauth/callback"}, + {name: "explicit hub mount", publicURL: "https://hub.example/node/demo", basePath: "/ui", host: "internal:8080", want: "https://hub.example/node/demo/v1/mcp/oauth/callback"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &serveServer{cfg: serveServerConfig{basePath: tt.basePath, publicURL: tt.publicURL}} + r := httptest.NewRequest("POST", "http://"+tt.host+"/", nil) + r.Host = tt.host + if tt.tls { + r.TLS = &tls.ConnectionState{} + } + got, err := s.mcpOAuthCallbackURL(r) + if err != nil { + t.Fatal(err) + } + if got != tt.want { + t.Fatalf("callback URL = %q, want %q", got, tt.want) + } + }) + } +} + +func TestNormalizeServePublicURL(t *testing.T) { + if got, err := normalizeServePublicURL(" https://example.com/node/demo/ "); err != nil || got != "https://example.com/node/demo" { + t.Fatalf("normalize = %q, %v", got, err) + } + for _, raw := range []string{"javascript:alert(1)", "https://example.com/path?redirect=evil", "https://user@example.com"} { + if _, err := normalizeServePublicURL(raw); err == nil { + t.Errorf("normalizeServePublicURL(%q) succeeded", raw) + } + } +} + +func TestParseSessionMCPOAuthSuffix(t *testing.T) { + server, action, ok := parseSessionMCPOAuthSuffix("mcp/github/oauth/start") + if !ok || server != "github" || action != "start" { + t.Fatalf("parse = %q, %q, %v", server, action, ok) + } + if _, _, ok := parseSessionMCPOAuthSuffix("mcp/github/oauth/start/extra"); ok { + t.Fatal("accepted extra path segment") + } +} diff --git a/docs-site/content/guides/mcp-servers.md b/docs-site/content/guides/mcp-servers.md index c09ffb100..f69ef1f82 100644 --- a/docs-site/content/guides/mcp-servers.md +++ b/docs-site/content/guides/mcp-servers.md @@ -34,6 +34,9 @@ term-llm chat --mcp playwright,filesystem |---------|-------------| | `mcp add ` | Add server from registry or URL | | `mcp list` | List configured servers | +| `mcp status [name]` | Show transport and safe authentication metadata | +| `mcp login ` | Sign in to a protected remote server | +| `mcp logout ` | Revoke and remove a stored grant | | `mcp info ` | Show server info and tools | | `mcp run [args]` | Run MCP tool(s) directly | | `mcp remove ` | Remove a server | @@ -62,6 +65,45 @@ term-llm mcp add exa # Exa web_search_exa and web_fetch_exa over https://m This adds Exa's free remote MCP endpoint. To use your own Exa key with this manually added MCP server, edit `mcp.json` and add an `x-api-key` header. The `search.exa_mcp.api_key` setting applies to term-llm's built-in `search.provider: exa_mcp` path. +### OAuth sign-in for remote servers + +Streamable HTTP servers can use MCP OAuth automatically. Adding a URL does not contact it or open a browser. Enable the server, then sign in when term-llm reports that authentication is required: + +```bash +term-llm mcp add https://mcp.example.com/mcp +term-llm mcp login example +term-llm mcp status example +``` + +`mcp login` performs protected-resource and authorization-server discovery, dynamic client registration when needed, PKCE S256, and the browser callback. It prints the authorization URL as a fallback. Use `--no-browser` when the browser is elsewhere; over SSH, forward the printed loopback callback port (for example with `ssh -L`) before opening the URL. Device-code authentication is not currently supported. + +The resulting registration and grant are stored in `$XDG_CONFIG_HOME/term-llm/mcp_oauth.json` (normally `~/.config/term-llm/mcp_oauth.json`). The directory is mode `0700`, the file and lock are mode `0600`, writes are atomic, and refresh-token rotation is serialized across term-llm processes. This private file contains credentials: do not copy it into a repository or expose it to a browser. Tokens and client secrets are never returned by the serve API or stored in browser storage. + +Use `term-llm mcp logout example` to attempt RFC 7009 revocation and remove the local grant. `--local-only` skips the remote attempt. Logout is safe to repeat. + +An explicit `Authorization` header remains authoritative and disables automatic OAuth for that server. Stdio servers continue to use their configured environment. Optional OAuth client configuration belongs in `mcp.json`, while the secret itself stays in the named environment variable: + +```json +{ + "servers": { + "private-remote": { + "type": "http", + "url": "https://mcp.example.com/mcp", + "oauth": { + "client_id": "registered-public-client", + "client_secret_env": "MCP_CLIENT_SECRET", + "scopes": ["read", "write"], + "client_id_metadata_url": "https://client.example/metadata.json" + } + } + } +} +``` + +Set `"disabled": true` under `oauth` to opt out without adding a static header. + +For web sign-in, `term-llm serve` derives the callback from the authenticated start request. Set `--public-url` or `TERM_LLM_SERVE_PUBLIC_URL` when the browser-visible URL differs. A node mounted behind `serve hub` must set this to its hub mount, such as `https://hub.example/node/`, because the hub deliberately strips forwarding headers. + ### Using MCP Tools The `--mcp` flag works with all commands (`ask`, `exec`, `edit`, `chat`): diff --git a/frontend/src/api/endpoints.ts b/frontend/src/api/endpoints.ts index 14779ad0d..08ab4aee0 100644 --- a/frontend/src/api/endpoints.ts +++ b/frontend/src/api/endpoints.ts @@ -1,5 +1,5 @@ import type { APIClient } from './client'; -import type { Goal, MCPResponse } from '../domain/types'; +import type { Goal, MCPOAuthFlow, MCPResponse } from '../domain/types'; import type { MentionSearchResponse } from '../domain/completions'; const encoded = (value: string): string => encodeURIComponent(value); @@ -207,6 +207,18 @@ export const endpoints = (api: APIClient) => ({ getMCP: (id: string) => api.get(`/v1/sessions/${encoded(id)}/mcp`), setMCP: (id: string, enabled: string[]) => api.patch(`/v1/sessions/${encoded(id)}/mcp`, { enabled }), + startMCPOAuth: (id: string, server: string, force = false) => + api.post(`/v1/sessions/${encoded(id)}/mcp/${encoded(server)}/oauth/start`, { + force, + }), + cancelMCPOAuth: (id: string, server: string, flowId: string) => + api.post(`/v1/sessions/${encoded(id)}/mcp/${encoded(server)}/oauth/cancel`, { + flow_id: flowId, + }), + logoutMCPOAuth: (id: string, server: string) => + api.delete(`/v1/sessions/${encoded(id)}/mcp/${encoded(server)}/oauth`), + getMCPOAuthFlow: (flowId: string) => + api.get(`/v1/mcp/oauth/flows/${encoded(flowId)}`), askUser: (id: string, body: unknown, operationId: string) => api.post(`/v1/sessions/${encoded(id)}/ask_user`, body, 'idempotent-mutation', { 'Idempotency-Key': `ask_user_${operationId}`, diff --git a/frontend/src/components/Modals.tsx b/frontend/src/components/Modals.tsx index 78c72f1f5..f2d7c0457 100644 --- a/frontend/src/components/Modals.tsx +++ b/frontend/src/components/Modals.tsx @@ -638,6 +638,7 @@ function MCP() { return `${server.tools} tool${server.tools === 1 ? '' : 's'} available`; } if (status === 'starting') return 'Starting server…'; + if (status === 'auth_required') return 'Server enabled · sign-in required'; if (server.error) return 'Failed to start'; return ''; }; @@ -698,8 +699,16 @@ function MCP() { const checked = state.enabled.includes(server.name); const status = server.configured ? server.status.toLocaleLowerCase() : 'failed'; const statusClass = status.replace(/[^a-z0-9_-]/g, '') || 'stopped'; + const oauth = state.oauth?.[server.name]; + const waiting = oauth?.state === 'starting' || oauth?.state === 'pending'; + const signInLabel = + server.authState === 'needs_sign_in' + ? 'Sign in again' + : server.authState === 'retry' + ? 'Retry' + : 'Sign in'; return ( - + ); }) )} diff --git a/frontend/src/components/components.test.tsx b/frontend/src/components/components.test.tsx index a5ed3f8f6..ee26b7650 100644 --- a/frontend/src/components/components.test.tsx +++ b/frontend/src/components/components.test.tsx @@ -3445,6 +3445,76 @@ describe('Preact-owned chat surfaces', () => { expect(store.toggleMCP).toHaveBeenCalledWith('discourse'); }); + it('separates MCP enablement from OAuth sign-in actions', async () => { + const store = createStore(); + store.modal.value = 'mcp'; + store.mcp.value = { + servers: [ + { + name: 'protected', + configured: true, + enabled: true, + status: 'auth_required', + error: '', + refreshWarning: '', + tools: 0, + active: 0, + deferred: 0, + loadingMode: '', + authState: 'needs_sign_in', + authIssuer: 'https://auth.example', + authScopes: ['read'], + authExpiresAt: '', + canSignIn: true, + canSignOut: false, + }, + ], + enabled: ['protected'], + loading: false, + pending: '', + error: '', + oauth: {}, + }; + store.startMCPOAuth = vi.fn(async () => undefined); + + const { rerender } = render( + + + , + ); + expect(screen.getByText('Server enabled · sign-in required')).toBeVisible(); + await userEvent.click(screen.getByRole('button', { name: 'Sign in again' })); + expect(store.startMCPOAuth).toHaveBeenCalledWith('protected', true); + expect(screen.getByRole('checkbox', { name: 'Disable protected' })).toBeChecked(); + + store.mcp.value = { + ...store.mcp.value, + oauth: { + protected: { + flowId: 'flow-safe-id', + authorizationURL: 'https://auth.example/authorize', + state: 'pending', + error: '', + popupBlocked: true, + }, + }, + }; + store.copyMCPOAuthLink = vi.fn(async () => undefined); + store.cancelMCPOAuth = vi.fn(async () => undefined); + rerender( + + + , + ); + await waitFor(() => + expect(screen.getByText('Popup blocked — copy the sign-in link.')).toBeVisible(), + ); + await userEvent.click(screen.getByRole('button', { name: 'Copy link' })); + await userEvent.click(screen.getByRole('button', { name: 'Cancel' })); + expect(store.copyMCPOAuthLink).toHaveBeenCalledWith('protected'); + expect(store.cancelMCPOAuth).toHaveBeenCalledWith('protected'); + }); + it('does not claim the MCP config is empty when loading fails', () => { const store = createStore(); store.modal.value = 'mcp'; diff --git a/frontend/src/domain/types.ts b/frontend/src/domain/types.ts index 974dac854..68f32284d 100644 --- a/frontend/src/domain/types.ts +++ b/frontend/src/domain/types.ts @@ -104,6 +104,20 @@ export interface MCPServer { active: number; deferred: number; loadingMode: string; + authState?: string; + authIssuer?: string; + authScopes?: string[]; + authExpiresAt?: string; + canSignIn?: boolean; + canSignOut?: boolean; +} + +export interface MCPOAuthFlow { + flow_id: string; + authorization_url?: string; + expires_at: string; + state: 'starting' | 'pending' | 'succeeded' | 'failed' | 'canceled' | 'expired'; + error?: string; } export interface MCPResponse { diff --git a/frontend/src/stores/app-store.test.ts b/frontend/src/stores/app-store.test.ts index ac1d27bb8..d7138aa7a 100644 --- a/frontend/src/stores/app-store.test.ts +++ b/frontend/src/stores/app-store.test.ts @@ -661,6 +661,75 @@ describe('AppStore compatibility behavior', () => { expect(store.activeSession.value?.mcpEnabled).toEqual(['github']); }); + it('starts MCP OAuth from a user popup and keeps flow data ephemeral', async () => { + const store = new AppStore(config); + store.sessions.value = [session()]; + store.activeSessionId.value = 's1'; + store.draftActive.value = false; + const assign = vi.fn(); + const close = vi.fn(); + const popup = { location: { assign }, close } as unknown as Window; + vi.spyOn(window, 'open').mockReturnValue(popup); + store.endpoints.startMCPOAuth = vi.fn(async () => ({ + flow_id: 'flow-id', + authorization_url: 'https://auth.example/authorize?state=capability', + expires_at: new Date(Date.now() + 60_000).toISOString(), + state: 'pending' as const, + })); + store.endpoints.cancelMCPOAuth = vi.fn(async () => ({})); + store.endpoints.getMCP = vi.fn(async () => ({ servers: [], enabled: [] })); + + await store.startMCPOAuth('protected'); + + expect(window.open).toHaveBeenCalledWith('', '_blank', 'popup=yes,width=560,height=720'); + expect(assign).toHaveBeenCalledWith('https://auth.example/authorize?state=capability'); + expect(store.mcp.value.oauth?.protected).toMatchObject({ + flowId: 'flow-id', + state: 'pending', + popupBlocked: false, + }); + const browserStorage = Array.from({ length: localStorage.length }, (_, index) => + localStorage.getItem(localStorage.key(index) || ''), + ).join(''); + expect(browserStorage).not.toContain('capability'); + + await store.cancelMCPOAuth('protected'); + expect(store.endpoints.cancelMCPOAuth).toHaveBeenCalledWith('s1', 'protected', 'flow-id'); + expect(close).toHaveBeenCalled(); + expect(store.mcp.value.oauth?.protected).toBeUndefined(); + }); + + it('stops MCP OAuth polling when the flow no longer exists server-side', async () => { + vi.useFakeTimers(); + try { + const store = new AppStore(config); + store.sessions.value = [session()]; + store.activeSessionId.value = 's1'; + store.draftActive.value = false; + vi.spyOn(window, 'open').mockReturnValue(null); + store.endpoints.startMCPOAuth = vi.fn(async () => ({ + flow_id: 'flow-id', + authorization_url: 'https://auth.example/authorize', + expires_at: new Date(Date.now() + 60_000).toISOString(), + state: 'pending' as const, + })); + const getFlow = vi.fn(async () => { + throw new APIError('flow not found', 404); + }); + store.endpoints.getMCPOAuthFlow = getFlow; + + await store.startMCPOAuth('protected'); + await vi.advanceTimersByTimeAsync(1000); + expect(getFlow).toHaveBeenCalledTimes(1); + expect(store.mcp.value.oauth?.protected).toMatchObject({ state: 'failed' }); + + await vi.advanceTimersByTimeAsync(10_000); + expect(getFlow).toHaveBeenCalledTimes(1); + } finally { + vi.useRealTimers(); + } + }); + it('rolls back an MCP toggle and exposes a recoverable save error', async () => { const store = new AppStore(config); store.sessions.value = [session()]; diff --git a/frontend/src/stores/app-store.ts b/frontend/src/stores/app-store.ts index 614c2242a..dc94343b7 100644 --- a/frontend/src/stores/app-store.ts +++ b/frontend/src/stores/app-store.ts @@ -33,7 +33,7 @@ import { AppStoreServices, type StoreDiagnostics } from './app-store-services'; import { RuntimeStore } from './runtime-store'; import { InteractionStore } from './interaction-store'; import { SideQuestionStore } from './side-question-store'; -import { MCPStore } from './mcp-store'; +import { MCPStore, type MCPOAuthUIState } from './mcp-store'; import { WorktreeStore } from './worktree-store'; import { GoalStore } from './goal-store'; import { PlanStore } from './plan-store'; @@ -159,6 +159,7 @@ export class AppStore { loading: boolean; pending: string; error: string; + oauth?: Record; }>; readonly worktrees: Signal[]>; readonly worktreeError: Signal; @@ -1101,6 +1102,18 @@ export class AppStore { async toggleMCP(name: string): Promise { await this.mcpStore.toggle(name); } + async startMCPOAuth(name: string, force = false): Promise { + await this.mcpStore.startOAuth(name, force); + } + async cancelMCPOAuth(name: string): Promise { + await this.mcpStore.cancelOAuth(name); + } + async logoutMCPOAuth(name: string): Promise { + await this.mcpStore.logoutOAuth(name); + } + async copyMCPOAuthLink(name: string): Promise { + await this.mcpStore.copyOAuthLink(name); + } async saveGoal(goal: Goal | { action: string }): Promise { await this.goalStore.save(goal); } diff --git a/frontend/src/stores/mcp-store.ts b/frontend/src/stores/mcp-store.ts index ebd80fbde..772e9eed0 100644 --- a/frontend/src/stores/mcp-store.ts +++ b/frontend/src/stores/mcp-store.ts @@ -1,15 +1,25 @@ import { signal, type ReadonlySignal } from '@preact/signals'; +import { APIError } from '../api/client'; import { errorMessage } from '../domain/text'; -import type { MCPServer, Session } from '../domain/types'; +import type { MCPOAuthFlow, MCPServer, Session } from '../domain/types'; import type { AppStoreServices } from './app-store-services'; import { normalizeMCPState } from './store-utils'; +export interface MCPOAuthUIState { + flowId: string; + authorizationURL: string; + state: MCPOAuthFlow['state']; + error: string; + popupBlocked: boolean; +} + export interface MCPState { servers: MCPServer[]; enabled: string[]; loading: boolean; pending: string; error: string; + oauth: Record; } export interface MCPStoreOptions { @@ -17,7 +27,7 @@ export interface MCPStoreOptions { patchSession: (id: string, patch: Partial) => void; } -/** Owns MCP server loading and optimistic enablement. */ +/** Owns MCP server loading, optimistic enablement, and ephemeral OAuth UI flows. */ export class MCPStore { readonly state = signal({ servers: [], @@ -25,8 +35,14 @@ export class MCPStore { loading: false, pending: '', error: '', + oauth: {}, }); + private readonly pollTimers = new Map>(); + private readonly popups = new Map(); + private readonly flowSessions = new Map(); + private readonly messageListeners = new Map void>(); + constructor( private readonly services: AppStoreServices, private readonly options: MCPStoreOptions, @@ -35,11 +51,18 @@ export class MCPStore { async load(): Promise { const session = this.options.activeSession.value; if (!session) return; + this.pruneOAuth(session.id); this.state.value = { ...this.state.value, loading: true, error: '' }; try { const data = await this.services.endpoints.getMCP(session.id); const state = normalizeMCPState(data); - this.state.value = { ...state, loading: false, pending: '', error: '' }; + this.state.value = { + ...state, + loading: false, + pending: '', + error: '', + oauth: this.state.value.oauth, + }; this.options.patchSession(session.id, { mcpEnabled: state.enabled }); } catch (error) { this.state.value = { @@ -61,7 +84,13 @@ export class MCPStore { try { const data = await this.services.endpoints.setMCP(session.id, enabled); const state = normalizeMCPState(data); - this.state.value = { ...state, loading: false, pending: '', error: '' }; + this.state.value = { + ...state, + loading: false, + pending: '', + error: '', + oauth: this.state.value.oauth, + }; this.options.patchSession(session.id, { mcpEnabled: state.enabled }); } catch (error) { this.state.value = { @@ -72,4 +101,198 @@ export class MCPStore { }; } } + + async startOAuth(name: string, force = false): Promise { + const session = this.options.activeSession.value; + const existing = this.state.value.oauth[name]; + if (!session || existing?.state === 'starting' || existing?.state === 'pending') return; + if (existing) this.removeOAuth(name); + + // Open synchronously from the click gesture. Navigation happens after the + // authenticated start response supplies the authorization URL. + const popup = window.open('', '_blank', 'popup=yes,width=560,height=720'); + this.flowSessions.set(name, session.id); + this.setOAuth(name, { + flowId: '', + authorizationURL: '', + state: 'starting', + error: '', + popupBlocked: !popup, + }); + try { + const flow = await this.services.endpoints.startMCPOAuth(session.id, name, force); + this.setOAuth(name, { + flowId: flow.flow_id, + authorizationURL: flow.authorization_url || '', + state: flow.state, + error: flow.error || '', + popupBlocked: !popup, + }); + if (popup && flow.authorization_url) { + this.popups.set(name, popup); + popup.location.assign(flow.authorization_url); + } + const onMessage = (event: MessageEvent) => { + const data = event.data as { type?: string; flow_id?: string } | null; + if ( + event.origin === window.location.origin && + data?.type === 'term-llm-mcp-oauth' && + data.flow_id === flow.flow_id + ) { + window.removeEventListener('message', onMessage); + this.messageListeners.delete(name); + void this.pollOAuth(name, flow.flow_id, true); + } + }; + this.clearMessageListener(name); + this.messageListeners.set(name, onMessage); + window.addEventListener('message', onMessage); + this.schedulePoll(name, flow.flow_id); + } catch (error) { + popup?.close(); + this.setOAuth(name, { + flowId: '', + authorizationURL: '', + state: 'failed', + error: errorMessage(error), + popupBlocked: !popup, + }); + } + } + + async cancelOAuth(name: string): Promise { + const session = this.options.activeSession.value; + const flow = this.state.value.oauth[name]; + if (!session || !flow?.flowId) return; + this.clearPoll(name); + try { + await this.services.endpoints.cancelMCPOAuth(session.id, name, flow.flowId); + this.popups.get(name)?.close(); + this.popups.delete(name); + this.removeOAuth(name); + await this.load(); + } catch (error) { + this.setOAuth(name, { ...flow, state: 'failed', error: errorMessage(error) }); + } + } + + async logoutOAuth(name: string): Promise { + const session = this.options.activeSession.value; + if (!session) return; + try { + await this.services.endpoints.logoutMCPOAuth(session.id, name); + this.removeOAuth(name); + await this.load(); + } catch (error) { + this.state.value = { ...this.state.value, error: errorMessage(error) }; + } + } + + async copyOAuthLink(name: string): Promise { + const url = this.state.value.oauth[name]?.authorizationURL; + if (url) await navigator.clipboard.writeText(url); + } + + private schedulePoll(name: string, flowId: string): void { + this.clearPoll(name); + this.pollTimers.set( + name, + setTimeout(() => void this.pollOAuth(name, flowId), 1000), + ); + } + + private async pollOAuth(name: string, flowId: string, immediate = false): Promise { + if (!immediate && this.state.value.oauth[name]?.flowId !== flowId) return; + try { + const flow = await this.services.endpoints.getMCPOAuthFlow(flowId); + const current = this.state.value.oauth[name]; + if (!current || current.flowId !== flowId) return; + if (flow.state === 'starting' || flow.state === 'pending') { + this.setOAuth(name, { + ...current, + state: flow.state, + authorizationURL: flow.authorization_url || current.authorizationURL, + }); + this.schedulePoll(name, flowId); + return; + } + this.clearPoll(name); + this.clearMessageListener(name); + this.popups.get(name)?.close(); + this.popups.delete(name); + if (flow.state === 'succeeded') { + this.removeOAuth(name); + await this.load(); + } else { + this.setOAuth(name, { + ...current, + state: flow.state, + authorizationURL: '', + error: flow.error || 'Authorization did not complete', + }); + } + } catch (error) { + const current = this.state.value.oauth[name]; + if (current?.flowId !== flowId) return; + if (error instanceof APIError && error.status === 404) { + // The flow no longer exists server-side (expired and pruned, or the + // server restarted). Stop polling instead of retrying forever. + this.clearPoll(name); + this.clearMessageListener(name); + this.popups.get(name)?.close(); + this.popups.delete(name); + this.setOAuth(name, { + ...current, + state: 'failed', + authorizationURL: '', + error: 'Authorization flow expired — try signing in again', + }); + return; + } + this.setOAuth(name, { ...current, error: errorMessage(error) }); + this.schedulePoll(name, flowId); + } + } + + private setOAuth(name: string, flow: MCPOAuthUIState): void { + this.state.value = { + ...this.state.value, + oauth: { ...this.state.value.oauth, [name]: flow }, + }; + } + + private removeOAuth(name: string): void { + this.clearPoll(name); + this.clearMessageListener(name); + const oauth = { ...this.state.value.oauth }; + delete oauth[name]; + this.flowSessions.delete(name); + this.state.value = { ...this.state.value, oauth }; + } + + private pruneOAuth(sessionId: string): void { + for (const [name, ownerSessionId] of this.flowSessions) { + if (ownerSessionId === sessionId) continue; + this.clearPoll(name); + this.clearMessageListener(name); + this.popups.get(name)?.close(); + this.popups.delete(name); + const oauth = { ...this.state.value.oauth }; + delete oauth[name]; + this.flowSessions.delete(name); + this.state.value = { ...this.state.value, oauth }; + } + } + + private clearMessageListener(name: string): void { + const listener = this.messageListeners.get(name); + if (listener) window.removeEventListener('message', listener); + this.messageListeners.delete(name); + } + + private clearPoll(name: string): void { + const timer = this.pollTimers.get(name); + if (timer) clearTimeout(timer); + this.pollTimers.delete(name); + } } diff --git a/frontend/src/stores/store-utils.ts b/frontend/src/stores/store-utils.ts index bd022cf9f..3ca3dbd71 100644 --- a/frontend/src/stores/store-utils.ts +++ b/frontend/src/stores/store-utils.ts @@ -62,6 +62,12 @@ export const normalizeMCPState = (value: unknown): { servers: MCPServer[]; enabl active: count('active'), deferred: count('deferred'), loadingMode: String(server.loading_mode || '').trim(), + authState: String(server.auth_state || 'not_needed').trim(), + authIssuer: String(server.auth_issuer || '').trim(), + authScopes: Array.isArray(server.auth_scopes) ? server.auth_scopes.map(String) : [], + authExpiresAt: String(server.auth_expires_at || '').trim(), + canSignIn: Boolean(server.can_sign_in), + canSignOut: Boolean(server.can_sign_out), }; }) .filter((server): server is MCPServer => server !== null); diff --git a/frontend/src/styles/features/approvals.css b/frontend/src/styles/features/approvals.css index 790eafe73..3e2daff38 100644 --- a/frontend/src/styles/features/approvals.css +++ b/frontend/src/styles/features/approvals.css @@ -188,7 +188,7 @@ .mcp-server-row { display: grid; - grid-template-columns: auto minmax(0, 1fr) auto; + grid-template-columns: auto minmax(0, 1fr) auto auto; align-items: center; gap: 0.72rem; border: 1px solid var(--border); @@ -306,6 +306,47 @@ word-break: break-word; } +.mcp-auth-actions { + display: inline-flex; + align-items: center; + justify-content: flex-end; + gap: 0.42rem; + max-width: 18rem; +} + +.mcp-auth-waiting { + color: var(--text-muted); + font-size: 0.76rem; + line-height: 1.25; +} + +.mcp-auth-action { + border: 1px solid var(--border-strong); + border-radius: 8px; + background: var(--surface-3); + color: var(--text); + padding: 0.34rem 0.52rem; + font: inherit; + font-size: 0.76rem; + white-space: nowrap; + cursor: pointer; +} + +.mcp-auth-action:hover, +.mcp-auth-action:focus-visible { + background: var(--surface-2); +} + +.mcp-auth-action.primary { + border-color: color-mix(in srgb, var(--accent-blue) 42%, var(--border)); + color: var(--accent-blue); +} + +.mcp-auth-action:disabled { + cursor: not-allowed; + opacity: 0.55; +} + .mcp-switch { position: relative; display: inline-flex; @@ -381,6 +422,13 @@ gap: 0.6rem; } + .mcp-auth-actions { + grid-column: 1 / -1; + grid-row: 2; + justify-content: flex-start; + max-width: none; + } + .mcp-server-icon { display: none; } diff --git a/frontend/vite.config.ts b/frontend/vite.config.ts index 1123274d0..9b23f1022 100644 --- a/frontend/vite.config.ts +++ b/frontend/vite.config.ts @@ -74,6 +74,7 @@ export default defineConfig({ manualChunks(id) { if (id.includes('/katex/')) return 'katex'; if (id.includes('/highlight.js/')) return 'highlight'; + if (id.endsWith('/src/stores/mcp-store.ts')) return 'mcp'; if ( id.includes('/node_modules/preact/') || id.includes('/node_modules/@preact/signals/') || diff --git a/go.mod b/go.mod index 46d8940d2..3dee1645f 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( github.com/go-webauthn/webauthn v0.17.4 github.com/gorilla/websocket v1.5.3 github.com/mattn/go-runewidth v0.0.23 - github.com/modelcontextprotocol/go-sdk v1.5.0 + github.com/modelcontextprotocol/go-sdk v1.7.0 github.com/muesli/cancelreader v0.2.2 github.com/muesli/reflow v0.3.0 github.com/openai/openai-go v1.12.0 @@ -41,6 +41,7 @@ require ( github.com/yuin/goldmark v1.8.2 golang.org/x/image v0.39.0 golang.org/x/net v0.54.0 + golang.org/x/oauth2 v0.35.0 golang.org/x/sync v0.20.0 golang.org/x/sys v0.45.0 golang.org/x/term v0.43.0 @@ -85,7 +86,7 @@ require ( github.com/go-webauthn/x v0.2.6 // indirect github.com/golang-jwt/jwt/v5 v5.3.1 // indirect github.com/google/go-tpm v0.9.8 // indirect - github.com/google/jsonschema-go v0.4.2 // indirect + github.com/google/jsonschema-go v0.4.3 // indirect github.com/google/uuid v1.6.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/lucasb-eyer/go-colorful v1.4.0 // indirect @@ -119,9 +120,8 @@ require ( go.yaml.in/yaml/v3 v3.0.4 // indirect golang.org/x/crypto v0.52.0 // indirect golang.org/x/exp v0.0.0-20260112195511-716be5621a96 // indirect - golang.org/x/oauth2 v0.35.0 // indirect golang.org/x/text v0.37.0 // indirect - golang.org/x/time v0.14.0 // indirect + golang.org/x/time v0.15.0 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect modernc.org/libc v1.72.0 // indirect modernc.org/mathutil v1.7.1 // indirect diff --git a/go.sum b/go.sum index f99258783..052a38bc3 100644 --- a/go.sum +++ b/go.sum @@ -117,8 +117,8 @@ github.com/google/go-tpm v0.9.8 h1:slArAR9Ft+1ybZu0lBwpSmpwhRXaa85hWtMinMyRAWo= github.com/google/go-tpm v0.9.8/go.mod h1:h9jEsEECg7gtLis0upRBQU+GhYVH6jMjrFxI8u6bVUY= github.com/google/go-tpm-tools v0.3.13-0.20230620182252-4639ecce2aba h1:qJEJcuLzH5KDR0gKc0zcktin6KSAwL7+jWKBYceddTc= github.com/google/go-tpm-tools v0.3.13-0.20230620182252-4639ecce2aba/go.mod h1:EFYHy8/1y2KfgTAsx7Luu7NGhoxtuVHnNo8jE7FikKc= -github.com/google/jsonschema-go v0.4.2 h1:tmrUohrwoLZZS/P3x7ex0WAVknEkBZM46iALbcqoRA8= -github.com/google/jsonschema-go v0.4.2/go.mod h1:r5quNTdLOYEz95Ru18zA0ydNbBuYoo9tgaYcxEYhJVE= +github.com/google/jsonschema-go v0.4.3 h1:/DBOLZTfDow7pe2GmaJNhltueGTtDKICi8V8p+DQPd0= +github.com/google/jsonschema-go v0.4.3/go.mod h1:r5quNTdLOYEz95Ru18zA0ydNbBuYoo9tgaYcxEYhJVE= github.com/google/pprof v0.0.0-20250317173921-a4b03ec1a45e h1:ijClszYn+mADRFY17kjQEVQ1XRhq2/JR1M3sGqeJoxs= github.com/google/pprof v0.0.0-20250317173921-a4b03ec1a45e/go.mod h1:boTsfXsheKC2y+lKOCMpSfarhxDeIzfZG1jqGcPl3cA= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= @@ -148,8 +148,8 @@ github.com/mattn/go-runewidth v0.0.23 h1:7ykA0T0jkPpzSvMS5i9uoNn2Xy3R383f9HDx3Ry github.com/mattn/go-runewidth v0.0.23/go.mod h1:XBkDxAl56ILZc9knddidhrOlY5R/pDhgLpndooCuJAs= github.com/mitchellh/hashstructure/v2 v2.0.2 h1:vGKWl0YJqUNxE8d+h8f6NJLcCJrgbhC4NcD46KavDd4= github.com/mitchellh/hashstructure/v2 v2.0.2/go.mod h1:MG3aRVU/N29oo/V/IhBX8GR/zz4kQkprJgF2EVszyDE= -github.com/modelcontextprotocol/go-sdk v1.5.0 h1:CHU0FIX9kpueNkxuYtfYQn1Z0slhFzBZuq+x6IiblIU= -github.com/modelcontextprotocol/go-sdk v1.5.0/go.mod h1:gggDIhoemhWs3BGkGwd1umzEXCEMMvAnhTrnbXJKKKA= +github.com/modelcontextprotocol/go-sdk v1.7.0 h1:yqjY2dsbKAC0LSuWZVBMrHgiG8ukXv6NRo0JiALay44= +github.com/modelcontextprotocol/go-sdk v1.7.0/go.mod h1:dL7u98E/zjJTGzEq+j30jQ8K2k1mb6LeAH4inEcSGts= github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELUXHmA= github.com/muesli/cancelreader v0.2.2/go.mod h1:3XuTXfFS2VjM+HTLZY9Ak0l6eUKfijIfMUZ4EgX0QYo= github.com/ncruces/go-strftime v1.0.0 h1:HMFp8mLCTPp341M/ZnA4qaf7ZlsbTc+miZjCLOFAw7w= @@ -324,8 +324,8 @@ golang.org/x/text v0.15.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ= golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= -golang.org/x/time v0.14.0 h1:MRx4UaLrDotUKUdCIqzPC48t1Y9hANFKIRpNx+Te8PI= -golang.org/x/time v0.14.0/go.mod h1:eL/Oa2bBBK0TkX57Fyni+NgnyQQN4LitPmob2Hjnqw4= +golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= +golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= diff --git a/internal/mcp/client.go b/internal/mcp/client.go index 2f0d7a832..139972f1e 100644 --- a/internal/mcp/client.go +++ b/internal/mcp/client.go @@ -19,6 +19,7 @@ import ( "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/samsaffron/term-llm/internal/llm" + mcpoauth "github.com/samsaffron/term-llm/internal/mcp/oauth" "github.com/samsaffron/term-llm/internal/procutil" ) @@ -87,6 +88,7 @@ type Client struct { stdioStderr *synchronizedLimitedBuffer maxToolsPerServer int onCatalogueChange func(oldSnapshot, newSnapshot *ToolSnapshot, err error) + oauthCoordinator *mcpoauth.Coordinator snapshot atomic.Pointer[ToolSnapshot] lifecycleMu sync.Mutex mu sync.RWMutex @@ -165,11 +167,9 @@ func (c *Client) start(ctx, processCtx context.Context) error { Version: "1.0.0", }, clientOpts) - var transport mcp.Transport - if c.config.TransportType() == "http" { - transport = c.createHTTPTransport() - } else { - transport = c.createStdioTransport(processCtx) + transport, err := c.createTransport(processCtx) + if err != nil { + return fmt.Errorf("configure MCP server %s transport: %w", c.name, err) } session, err := client.Connect(ctx, transport, nil) @@ -177,6 +177,9 @@ func (c *Client) start(ctx, processCtx context.Context) error { c.mu.Lock() c.cancelStdioProcessLocked() c.mu.Unlock() + if isClientAuthenticationRequired(err) { + return fmt.Errorf("connect to MCP server %s: %w; run `term-llm mcp login %s`", c.name, err, c.name) + } return fmt.Errorf("connect to MCP server %s: %w", c.name, c.withStdioStderr(err)) } @@ -245,8 +248,15 @@ func (c *Client) withStdioStderr(err error) error { return fmt.Errorf("%w\n\nMCP server stderr:\n%s", err, output) } +func (c *Client) createTransport(ctx context.Context) (mcp.Transport, error) { + if c.config.TransportType() == "http" { + return c.createHTTPTransport() + } + return c.createStdioTransport(ctx), nil +} + // createHTTPTransport creates an HTTP transport for URL-based servers. -func (c *Client) createHTTPTransport() mcp.Transport { +func (c *Client) createHTTPTransport() (mcp.Transport, error) { // Use a clone of the default transport so proxy, HTTP/2, and other standard // settings are preserved while avoiding a whole-request http.Client timeout. // Caller contexts control the full request lifetime, including long-running @@ -277,7 +287,52 @@ func (c *Client) createHTTPTransport() mcp.Transport { MaxRetries: 5, } - return transport + if c.automaticOAuthEnabled() { + oauthConfig := c.config.OAuth + // The OAuth handler talks to the authorization server (metadata, + // registration, token, refresh), not just the MCP endpoint. Custom + // per-server headers such as API keys must never be sent there, so it + // gets a client without the headerTransport wrapper. + options := mcpoauth.Options{HTTPClient: &http.Client{Transport: baseTransport}} + if oauthConfig != nil { + options.ClientID = oauthConfig.ClientID + options.Scopes = append([]string(nil), oauthConfig.Scopes...) + options.ClientIDMetadataURL = oauthConfig.ClientIDMetadataURL + if oauthConfig.ClientSecretEnv != "" { + options.ClientSecret = os.Getenv(oauthConfig.ClientSecretEnv) + if options.ClientSecret == "" { + return nil, fmt.Errorf("OAuth client secret environment variable %s is not set", oauthConfig.ClientSecretEnv) + } + } + } + coordinator := c.oauthCoordinator + if coordinator == nil { + coordinator = mcpoauth.DefaultCoordinator() + } + handler, err := coordinator.Handler(c.config.URL, options) + if err != nil { + return nil, err + } + transport.OAuthHandler = handler + } + + return transport, nil +} + +func (c *Client) automaticOAuthEnabled() bool { + if c.config.OAuth != nil && c.config.OAuth.Disabled { + return false + } + for key := range c.config.Headers { + if strings.EqualFold(key, "Authorization") { + return false + } + } + return true +} + +func isClientAuthenticationRequired(err error) bool { + return errors.Is(err, mcpoauth.ErrAuthenticationRequired) || errors.Is(err, mcpoauth.ErrRefreshRejected) } // headerTransport is an http.RoundTripper that adds custom headers to requests. diff --git a/internal/mcp/client_test.go b/internal/mcp/client_test.go index f4056f42f..0a47b4f5e 100644 --- a/internal/mcp/client_test.go +++ b/internal/mcp/client_test.go @@ -4,19 +4,24 @@ import ( "context" "encoding/base64" "errors" + "fmt" + "io" "net/http" + "net/http/httptest" "os" "path/filepath" "reflect" "runtime" "strconv" "strings" + "sync/atomic" "syscall" "testing" "time" sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/samsaffron/term-llm/internal/llm" + mcpoauth "github.com/samsaffron/term-llm/internal/mcp/oauth" ) type roundTripFunc func(*http.Request) (*http.Response, error) @@ -168,11 +173,15 @@ func TestCreateHTTPTransport_UsesTransportLevelTimeouts(t *testing.T) { client := &Client{ name: "test", config: ServerConfig{ - URL: "https://example.com/mcp", + URL: "https://example.com/mcp", + OAuth: &OAuthConfig{Disabled: true}, }, } - transport := client.createHTTPTransport() + transport, err := client.createHTTPTransport() + if err != nil { + t.Fatalf("createHTTPTransport: %v", err) + } st, ok := transport.(*sdkmcp.StreamableClientTransport) if !ok { t.Fatal("expected sdkmcp.StreamableClientTransport") @@ -217,7 +226,10 @@ func TestCreateHTTPTransport_HeadersWrapTimeoutTransport(t *testing.T) { }, } - transport := client.createHTTPTransport() + transport, err := client.createHTTPTransport() + if err != nil { + t.Fatalf("createHTTPTransport: %v", err) + } st := transport.(*sdkmcp.StreamableClientTransport) if st.HTTPClient.Timeout != 0 { t.Fatalf("HTTPClient.Timeout = %v, want 0 so context controls long-running calls", st.HTTPClient.Timeout) @@ -240,6 +252,107 @@ func TestCreateHTTPTransport_HeadersWrapTimeoutTransport(t *testing.T) { } } +func TestCreateHTTPTransport_OAuthWiringAndAuthorizationHeaderPrecedence(t *testing.T) { + tests := []struct { + name string + config ServerConfig + wantHandler bool + }{ + {name: "automatic OAuth", config: ServerConfig{URL: "https://example.com/mcp"}, wantHandler: true}, + {name: "explicit Authorization header", config: ServerConfig{URL: "https://example.com/mcp", Headers: map[string]string{"authorization": "Bearer static"}}}, + {name: "OAuth disabled", config: ServerConfig{URL: "https://example.com/mcp", OAuth: &OAuthConfig{Disabled: true}}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := &Client{ + name: "test", config: tt.config, + oauthCoordinator: mcpoauth.NewCoordinator(mcpoauth.NewFileStore(filepath.Join(t.TempDir(), "oauth.json"))), + } + transport, err := client.createHTTPTransport() + if err != nil { + t.Fatal(err) + } + streamable := transport.(*sdkmcp.StreamableClientTransport) + if got := streamable.OAuthHandler != nil; got != tt.wantHandler { + t.Fatalf("OAuthHandler attached = %v, want %v", got, tt.wantHandler) + } + }) + } +} + +func TestCreateHTTPTransport_OAuthClientOmitsCustomHeaders(t *testing.T) { + // The OAuth handler talks to authorization-server endpoints (metadata, + // registration, token). Custom per-server headers such as API keys must + // only reach the MCP endpoint, never OAuth discovery or registration. + var server *httptest.Server + var leakedHeader atomic.Bool + var oauthRequests atomic.Int32 + record := func(r *http.Request) { + oauthRequests.Add(1) + if r.Header.Get("X-Api-Key") != "" { + leakedHeader.Store(true) + } + } + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/oauth-protected-resource/mcp", func(w http.ResponseWriter, r *http.Request) { + record(r) + w.Header().Set("Content-Type", "application/json") + fmt.Fprintf(w, `{"resource":%q,"authorization_servers":[%q]}`, server.URL+"/mcp", server.URL) + }) + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, r *http.Request) { + record(r) + w.Header().Set("Content-Type", "application/json") + fmt.Fprintf(w, `{"issuer":%q,"authorization_endpoint":%q,"token_endpoint":%q,"registration_endpoint":%q,"response_types_supported":["code"],"code_challenge_methods_supported":["S256"]}`, + server.URL, server.URL+"/authorize", server.URL+"/token", server.URL+"/register") + }) + mux.HandleFunc("/register", func(w http.ResponseWriter, r *http.Request) { + record(r) + w.Header().Set("Content-Type", "application/json") + fmt.Fprintf(w, `{"client_id":"dynamic-client","redirect_uris":["http://127.0.0.1/callback"],"token_endpoint_auth_method":"none","grant_types":["authorization_code","refresh_token"],"response_types":["code"]}`) + }) + server = httptest.NewServer(mux) + defer server.Close() + + client := &Client{ + name: "test", + config: ServerConfig{ + URL: server.URL + "/mcp", + Headers: map[string]string{"X-Api-Key": "super-secret"}, + }, + oauthCoordinator: mcpoauth.NewCoordinator(mcpoauth.NewFileStore(filepath.Join(t.TempDir(), "oauth.json"))), + } + transport, err := client.createHTTPTransport() + if err != nil { + t.Fatal(err) + } + streamable := transport.(*sdkmcp.StreamableClientTransport) + if streamable.OAuthHandler == nil { + t.Fatal("expected OAuth handler") + } + + req, err := http.NewRequest(http.MethodPost, server.URL+"/mcp", nil) + if err != nil { + t.Fatal(err) + } + challenge := &http.Response{ + StatusCode: http.StatusUnauthorized, + Header: http.Header{"Www-Authenticate": []string{ + fmt.Sprintf(`Bearer resource_metadata=%q`, server.URL+"/.well-known/oauth-protected-resource/mcp"), + }}, + Body: io.NopCloser(strings.NewReader("")), + } + err = streamable.OAuthHandler.Authorize(context.Background(), req, challenge) + if !errors.Is(err, mcpoauth.ErrAuthenticationRequired) { + t.Fatalf("background Authorize error = %v, want ErrAuthenticationRequired", err) + } + if oauthRequests.Load() == 0 { + t.Fatal("expected OAuth discovery requests to reach the fake authorization server") + } + if leakedHeader.Load() { + t.Fatal("custom MCP header leaked to authorization-server endpoints") + } +} + func TestHeaderTransportAddsHeadersWithoutMutatingRequest(t *testing.T) { req, err := http.NewRequest("GET", "https://example.com/mcp", nil) if err != nil { diff --git a/internal/mcp/config.go b/internal/mcp/config.go index 9db57ed14..7e5c12ba5 100644 --- a/internal/mcp/config.go +++ b/internal/mcp/config.go @@ -27,6 +27,10 @@ type ServerConfig struct { URL string `json:"url,omitempty"` Headers map[string]string `json:"headers,omitempty"` + // OAuth configures automatic authorization-code authentication for HTTP + // transports. An explicit Authorization header remains authoritative. + OAuth *OAuthConfig `json:"oauth,omitempty"` + // Shared fields Env map[string]string `json:"env,omitempty"` AlwaysLoad []string `json:"always_load,omitempty"` @@ -35,6 +39,17 @@ type ServerConfig struct { Sampling *SamplingConfig `json:"sampling,omitempty"` } +// OAuthConfig customizes automatic OAuth for a remote MCP server. Client +// secrets are referenced through the environment and are never stored in +// mcp.json. +type OAuthConfig struct { + ClientID string `json:"client_id,omitempty"` + ClientSecretEnv string `json:"client_secret_env,omitempty"` + Scopes []string `json:"scopes,omitempty"` + ClientIDMetadataURL string `json:"client_id_metadata_url,omitempty"` + Disabled bool `json:"disabled,omitempty"` +} + // SamplingConfig configures MCP sampling behavior for a server. type SamplingConfig struct { // Enabled controls whether sampling is allowed for this server (default: true) diff --git a/internal/mcp/config_test.go b/internal/mcp/config_test.go index 56b99e505..3e747be37 100644 --- a/internal/mcp/config_test.go +++ b/internal/mcp/config_test.go @@ -52,3 +52,40 @@ func TestLoadConfigAlwaysLoad(t *testing.T) { t.Fatalf("always_load = %#v", got) } } + +func TestConfigOAuthRoundTrip(t *testing.T) { + path := filepath.Join(t.TempDir(), "mcp.json") + want := &OAuthConfig{ + ClientID: "public-client", ClientSecretEnv: "MCP_CLIENT_SECRET", + Scopes: []string{"read", "write"}, ClientIDMetadataURL: "https://client.example/metadata.json", + } + cfg := &Config{Servers: map[string]ServerConfig{ + "remote": {Type: "http", URL: "https://mcp.example/mcp", OAuth: want}, + }} + if err := cfg.SaveToPath(path); err != nil { + t.Fatal(err) + } + loaded, err := LoadConfigFromPath(path) + if err != nil { + t.Fatal(err) + } + if got := loaded.Servers["remote"].OAuth; !reflect.DeepEqual(got, want) { + t.Fatalf("OAuth after round trip = %#v, want %#v", got, want) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if stringsContainsTest(string(data), "client_secret\"") || stringsContainsTest(string(data), "access_token") { + t.Fatalf("mcp.json contains credential material: %s", data) + } +} + +func stringsContainsTest(value, part string) bool { + for i := 0; i+len(part) <= len(value); i++ { + if value[i:i+len(part)] == part { + return true + } + } + return false +} diff --git a/internal/mcp/manager.go b/internal/mcp/manager.go index f36472823..bcefa85df 100644 --- a/internal/mcp/manager.go +++ b/internal/mcp/manager.go @@ -5,24 +5,29 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "errors" "fmt" + "os" "sort" + "strings" "sync" "sync/atomic" "time" sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/samsaffron/term-llm/internal/llm" + mcpoauth "github.com/samsaffron/term-llm/internal/mcp/oauth" ) // ServerStatus represents the current state of an MCP server. type ServerStatus string const ( - StatusStopped ServerStatus = "stopped" - StatusStarting ServerStatus = "starting" - StatusReady ServerStatus = "ready" - StatusFailed ServerStatus = "failed" + StatusStopped ServerStatus = "stopped" + StatusStarting ServerStatus = "starting" + StatusReady ServerStatus = "ready" + StatusFailed ServerStatus = "failed" + StatusAuthRequired ServerStatus = "auth_required" ) var mcpStartupTimeout = 30 * time.Second @@ -59,6 +64,7 @@ type Manager struct { startups map[string]*serverStartup catalogues map[string]*ToolSnapshot maxToolsPerServer int + oauthCoordinator *mcpoauth.Coordinator mu sync.RWMutex aggregate atomic.Pointer[CatalogueSnapshot] @@ -119,11 +125,30 @@ func (m *Manager) Config() *Config { for name, server := range m.config.Servers { serverCopy := server serverCopy.AlwaysLoad = append([]string(nil), server.AlwaysLoad...) + serverCopy.Args = append([]string(nil), server.Args...) + serverCopy.Headers = cloneStringMap(server.Headers) + serverCopy.Env = cloneStringMap(server.Env) + if server.OAuth != nil { + oauthCopy := *server.OAuth + oauthCopy.Scopes = append([]string(nil), server.OAuth.Scopes...) + serverCopy.OAuth = &oauthCopy + } copy.Servers[name] = serverCopy } return copy } +func cloneStringMap(values map[string]string) map[string]string { + if values == nil { + return nil + } + copy := make(map[string]string, len(values)) + for key, value := range values { + copy[key] = value + } + return copy +} + // SetCatalogueChangeHandler sets the callback invoked after aggregate immutable // publication. The callback is never invoked while Manager.mu is held. func (m *Manager) SetCatalogueChangeHandler(handler func(CatalogueEvent)) { @@ -275,7 +300,7 @@ func (m *Manager) EnabledServers() []string { defer m.mu.RUnlock() var names []string for name, state := range m.statuses { - if state.Status == StatusStarting || state.Status == StatusReady { + if state.Status == StatusStarting || state.Status == StatusReady || state.Status == StatusAuthRequired { names = append(names, name) } } @@ -309,7 +334,7 @@ func (m *Manager) Enable(ctx context.Context, name string) error { // Check if already running or starting if state, ok := m.statuses[name]; ok { - if state.Status == StatusStarting || state.Status == StatusReady { + if state.Status == StatusStarting || state.Status == StatusReady || state.Status == StatusAuthRequired { m.mu.Unlock() return nil } @@ -317,6 +342,7 @@ func (m *Manager) Enable(ctx context.Context, name string) error { // Create client and set status to starting. client := NewClient(name, serverCfg) + client.oauthCoordinator = m.oauthCoordinator client.maxToolsPerServer = m.maxToolsPerServer client.SetCatalogueChangeHandler(func(oldSnapshot, newSnapshot *ToolSnapshot, err error) { m.handleCatalogueChange(name, client, oldSnapshot, newSnapshot, err) @@ -383,7 +409,11 @@ func (m *Manager) Enable(ctx context.Context, name string) error { var catalogueEvent *CatalogueEvent var catalogueHandler func(CatalogueEvent) if err != nil { - status = StatusFailed + if isAuthenticationRequired(err) { + status = StatusAuthRequired + } else { + status = StatusFailed + } state.Error = err } else { state.Error = nil @@ -438,10 +468,14 @@ func (m *Manager) watchSession(name string, client *Client, session *sdkmcp.Clie delete(m.catalogues, name) snapshot := m.publishAggregateLocked() handler := m.catalogueHandler - state.Status = StatusFailed + status := StatusFailed + if isAuthenticationRequired(err) { + status = StatusAuthRequired + } + state.Status = status state.Error = err state.ToolCount = 0 - m.sendStatusLocked(name, StatusFailed, err) + m.sendStatusLocked(name, status, err) m.mu.Unlock() if handler != nil { handler(CatalogueEvent{Server: name, Snapshot: copyCatalogueSnapshot(snapshot)}) @@ -495,6 +529,170 @@ func (m *Manager) Restart(ctx context.Context, name string) error { return m.Enable(ctx, name) } +// AuthStatus is safe OAuth account metadata for display surfaces. +type AuthStatus = mcpoauth.AuthStatus + +// OAuthStartOptions selects the redirect frontend and whether an existing valid +// grant should be replaced. +type OAuthStartOptions struct { + RedirectURL string + Force bool + SkipReconnect bool +} + +// AuthStatuses reports account state without starting any MCP server. +func (m *Manager) AuthStatuses() map[string]AuthStatus { + m.mu.RLock() + cfg := m.config + m.mu.RUnlock() + statuses := make(map[string]AuthStatus) + if cfg == nil { + return statuses + } + coordinator := m.oauthCoordinatorOrDefault() + for name, server := range cfg.Servers { + if server.TransportType() != "http" || !automaticOAuthForServer(server) { + statuses[name] = AuthStatus{State: mcpoauth.AuthNotNeeded} + continue + } + statuses[name] = coordinator.Status(server.URL) + } + return statuses +} + +// StartOAuth starts a browser authorization flow for one server. Callers should +// provide their callback URL; the default is only useful for custom native +// callback handlers. +func (m *Manager) StartOAuth(ctx context.Context, name string, startOptions ...OAuthStartOptions) (*mcpoauth.Flow, error) { + m.mu.RLock() + if m.config == nil { + m.mu.RUnlock() + return nil, fmt.Errorf("no MCP configuration loaded") + } + server, ok := m.config.Servers[name] + m.mu.RUnlock() + if !ok { + return nil, fmt.Errorf("unknown MCP server: %s", name) + } + if server.TransportType() != "http" || !automaticOAuthForServer(server) { + return nil, fmt.Errorf("MCP server %s does not use automatic OAuth", name) + } + options := OAuthStartOptions{RedirectURL: "http://127.0.0.1/callback"} + if len(startOptions) > 0 { + options = startOptions[0] + } + coordinator := m.oauthCoordinatorOrDefault() + oauthOptions := oauthOptionsForServer(server) + if server.OAuth != nil && server.OAuth.ClientSecretEnv != "" && oauthOptions.ClientSecret == "" { + return nil, fmt.Errorf("OAuth client secret environment variable %s is not set", server.OAuth.ClientSecretEnv) + } + flow, err := coordinator.Start(ctx, server.URL, oauthOptions, options.RedirectURL, options.Force) + if err != nil { + return nil, err + } + m.sendStatus(name, StatusAuthRequired, nil) + if !flow.Created || options.SkipReconnect { + return flow, nil + } + go func(flowID string) { + flow, waitErr := coordinator.Wait(context.Background(), flowID) + if waitErr != nil || flow == nil || flow.State != mcpoauth.FlowSucceeded { + m.sendStatus(name, StatusAuthRequired, waitErr) + return + } + m.mu.RLock() + state := m.statuses[name] + selected := state != nil && state.Status != StatusStopped + m.mu.RUnlock() + if selected { + _ = m.Restart(context.Background(), name) + } else { + m.sendStatus(name, StatusStopped, nil) + } + }(flow.ID) + return flow, nil +} + +func (m *Manager) CancelOAuth(name, flowID string) error { + m.mu.RLock() + if m.config == nil { + m.mu.RUnlock() + return fmt.Errorf("no MCP configuration loaded") + } + server, ok := m.config.Servers[name] + m.mu.RUnlock() + if !ok { + return fmt.Errorf("unknown MCP server: %s", name) + } + if !m.oauthCoordinatorOrDefault().Cancel(server.URL, flowID) { + return fmt.Errorf("OAuth flow is not pending") + } + m.sendStatus(name, StatusAuthRequired, nil) + return nil +} + +func (m *Manager) LogoutOAuth(ctx context.Context, name string, localOnly bool) error { + m.mu.RLock() + if m.config == nil { + m.mu.RUnlock() + return fmt.Errorf("no MCP configuration loaded") + } + server, ok := m.config.Servers[name] + state := m.statuses[name] + selected := state != nil && state.Status != StatusStopped + m.mu.RUnlock() + if !ok { + return fmt.Errorf("unknown MCP server: %s", name) + } + if server.TransportType() != "http" || !automaticOAuthForServer(server) { + return nil + } + if err := m.oauthCoordinatorOrDefault().Logout(ctx, server.URL, localOnly); err != nil { + return err + } + if selected { + return m.Restart(ctx, name) + } + m.sendStatus(name, StatusStopped, nil) + return nil +} + +func (m *Manager) oauthCoordinatorOrDefault() *mcpoauth.Coordinator { + if m.oauthCoordinator != nil { + return m.oauthCoordinator + } + return mcpoauth.DefaultCoordinator() +} + +func automaticOAuthForServer(server ServerConfig) bool { + if server.OAuth != nil && server.OAuth.Disabled { + return false + } + for key := range server.Headers { + if strings.EqualFold(key, "Authorization") { + return false + } + } + return true +} + +func oauthOptionsForServer(server ServerConfig) mcpoauth.Options { + options := mcpoauth.Options{} + if server.OAuth != nil { + options.ClientID = server.OAuth.ClientID + options.Scopes = append([]string(nil), server.OAuth.Scopes...) + options.ClientIDMetadataURL = server.OAuth.ClientIDMetadataURL + if server.OAuth.ClientSecretEnv != "" { + options.ClientSecret = os.Getenv(server.OAuth.ClientSecretEnv) + } + } + return options +} + +func isAuthenticationRequired(err error) bool { + return errors.Is(err, mcpoauth.ErrAuthenticationRequired) || errors.Is(err, mcpoauth.ErrRefreshRejected) +} + // StopAll stops all running MCP servers. func (m *Manager) StopAll() { m.mu.Lock() diff --git a/internal/mcp/manager_test.go b/internal/mcp/manager_test.go index e8c9e8579..7d03b26cf 100644 --- a/internal/mcp/manager_test.go +++ b/internal/mcp/manager_test.go @@ -8,8 +8,11 @@ import ( "fmt" "io/fs" "log" + "net/http" + "net/http/httptest" "os" "path/filepath" + "reflect" "runtime" "strings" "sync" @@ -18,6 +21,7 @@ import ( mcpSDK "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/samsaffron/term-llm/internal/llm" + mcpoauth "github.com/samsaffron/term-llm/internal/mcp/oauth" ) const runMCPManagerTestServerEnv = "TERM_LLM_MCP_MANAGER_TEST_SERVER" @@ -110,6 +114,45 @@ func runMCPManagerTestServer() { } } +func TestManagerMapsProtectedHTTPServerToAuthRequired(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/mcp" { + w.WriteHeader(http.StatusUnauthorized) + return + } + http.NotFound(w, r) + })) + defer server.Close() + manager := NewManagerWithConfig(&Config{Servers: map[string]ServerConfig{ + "protected": {Type: "http", URL: server.URL + "/mcp", OAuth: &OAuthConfig{ClientID: "public-client"}}, + }}) + manager.oauthCoordinator = mcpoauth.NewCoordinator(mcpoauth.NewFileStore(filepath.Join(t.TempDir(), "oauth.json"))) + updates := make(chan StatusUpdate, 4) + manager.SetStatusChannel(updates) + if err := manager.Enable(t.Context(), "protected"); err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + for { + select { + case update := <-updates: + if update.Status != StatusAuthRequired { + continue + } + if !errors.Is(update.Error, mcpoauth.ErrAuthenticationRequired) { + t.Fatalf("auth-required error = %v", update.Error) + } + if got := manager.EnabledServers(); !reflect.DeepEqual(got, []string{"protected"}) { + t.Fatalf("enabled servers = %v", got) + } + return + case <-ctx.Done(): + t.Fatal("timed out waiting for auth-required status") + } + } +} + func TestManagerEnable_TimesOutStartupWithBackgroundContext(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("requires sh") diff --git a/internal/mcp/oauth/coordinator.go b/internal/mcp/oauth/coordinator.go new file mode 100644 index 000000000..d488036ca --- /dev/null +++ b/internal/mcp/oauth/coordinator.go @@ -0,0 +1,861 @@ +package oauth + +import ( + "bytes" + "context" + "crypto/rand" + "encoding/base64" + "errors" + "fmt" + "io" + "net" + "net/http" + "net/url" + "sort" + "strings" + "sync" + "time" + "unicode" + + sdkauth "github.com/modelcontextprotocol/go-sdk/auth" + "github.com/modelcontextprotocol/go-sdk/oauthex" + "golang.org/x/oauth2" +) + +const ( + flowLifetime = 10 * time.Minute + refreshExpirySkew = 5 * time.Minute + refreshTimeout = 30 * time.Second + metadataTimeout = 15 * time.Second + preflightTimeout = 15 * time.Second + maximumSafeErrSize = 300 +) + +type callbackResult struct { + code string + state string + iss string + err error +} + +type flowRecord struct { + flow Flow + callbackState string + callbackDelivered bool + callback chan callbackResult + ready chan struct{} + done chan struct{} + cancel context.CancelFunc + readyOnce sync.Once + doneOnce sync.Once +} + +// Coordinator owns process-global flow deduplication and persistent token +// sources. Multiple Coordinator values remain safe through the store file lock. +type Coordinator struct { + store Store + client *http.Client + initErr error + + mu sync.Mutex + flows map[string]*flowRecord + byEndpoint map[string]string + byState map[string]string + refreshErrs map[string]error +} + +func NewCoordinator(store Store) *Coordinator { + return &Coordinator{ + store: store, client: http.DefaultClient, flows: make(map[string]*flowRecord), + byEndpoint: make(map[string]string), byState: make(map[string]string), + refreshErrs: make(map[string]error), + } +} + +var ( + defaultCoordinatorOnce sync.Once + defaultCoordinator *Coordinator +) + +func DefaultCoordinator() *Coordinator { + defaultCoordinatorOnce.Do(func() { + path, err := DefaultStorePath() + defaultCoordinator = NewCoordinator(NewFileStore(path)) + defaultCoordinator.initErr = err + }) + return defaultCoordinator +} + +// Handler constructs the SDK v1.7 authorization handler used by ordinary MCP +// connections. Its fetcher is deliberately non-interactive. +func (c *Coordinator) Handler(endpoint string, options Options) (sdkauth.OAuthHandler, error) { + return c.newHandler(endpoint, options, "http://127.0.0.1/callback", false, nil) +} + +func (c *Coordinator) newHandler(endpoint string, options Options, redirectURL string, force bool, interactive *flowRecord) (sdkauth.OAuthHandler, error) { + if c == nil || c.store == nil { + return nil, fmt.Errorf("MCP OAuth coordinator is unavailable") + } + if c.initErr != nil { + return nil, fmt.Errorf("resolve MCP OAuth store: %w", c.initErr) + } + canonical, err := CanonicalEndpoint(endpoint) + if err != nil { + return nil, err + } + client := options.HTTPClient + if client == nil { + client = c.client + } + var stored *Session + stored, err = c.store.Load(canonical) + if err != nil && !errors.Is(err, ErrNotFound) { + return nil, fmt.Errorf("load MCP OAuth credentials: %w", err) + } + if errors.Is(err, ErrNotFound) { + stored = nil + } + + var initial oauth2.TokenSource + if stored != nil && !force { + initial = &persistentTokenSource{ + store: c.store, endpoint: canonical, session: cloneSession(stored), + client: client, coordinator: c, + } + } + + var callbackIssuer string + fetcher := func(context.Context, *sdkauth.AuthorizationArgs) (*sdkauth.AuthorizationResult, error) { + return nil, ErrAuthenticationRequired + } + if interactive != nil { + fetcher = func(ctx context.Context, args *sdkauth.AuthorizationArgs) (*sdkauth.AuthorizationResult, error) { + u, err := url.Parse(args.URL) + if err != nil { + return nil, fmt.Errorf("invalid authorization URL") + } + state := u.Query().Get("state") + if state == "" { + return nil, fmt.Errorf("authorization server did not provide state") + } + c.mu.Lock() + if interactive.flow.State == FlowStarting { + interactive.flow.State = FlowPending + interactive.flow.AuthorizationURL = args.URL + interactive.callbackState = state + c.byState[state] = interactive.flow.ID + } + c.mu.Unlock() + interactive.readyOnce.Do(func() { close(interactive.ready) }) + + select { + case result := <-interactive.callback: + if result.err != nil { + return nil, result.err + } + callbackIssuer = result.iss + return &sdkauth.AuthorizationResult{Code: result.code, State: result.state, Iss: result.iss}, nil + case <-ctx.Done(): + return nil, ctx.Err() + } + } + } + + handlerConfig := &sdkauth.AuthorizationCodeHandlerConfig{ + RedirectURL: redirectURL, + AuthorizationCodeFetcher: fetcher, + RequestRefreshToken: true, + Client: client, + InitialTokenSource: initial, + } + if options.ClientIDMetadataURL != "" { + handlerConfig.ClientIDMetadataDocumentConfig = &sdkauth.ClientIDMetadataDocumentConfig{URL: options.ClientIDMetadataURL} + } + clientID, clientSecret, issuer := options.ClientID, options.ClientSecret, "" + if clientID == "" && stored != nil && stored.Config.ClientID != "" { + // Reuse the persisted (usually DCR-issued) client so refreshes and + // re-authorizations keep one registration. An interactive flow must + // present a redirect URI compatible with that registration: exact + // match, or RFC 8252 loopback redirects that differ only by port. + // Otherwise fall back to dynamic registration for the new redirect. + if interactive == nil || redirectCompatible(stored.Config.RedirectURL, redirectURL) { + clientID, clientSecret, issuer = stored.Config.ClientID, stored.Config.ClientSecret, stored.Issuer + } + } + if clientID != "" { + credentials := &oauthex.ClientCredentials{ClientID: clientID, Issuer: issuer} + if clientSecret != "" { + credentials.ClientSecretAuth = &oauthex.ClientSecretAuth{ClientSecret: clientSecret} + } + handlerConfig.PreregisteredClient = credentials + } + if clientID == "" { + handlerConfig.DynamicClientRegistrationConfig = &sdkauth.DynamicClientRegistrationConfig{ + Metadata: &oauthex.ClientRegistrationMetadata{ + RedirectURIs: []string{redirectURL}, TokenEndpointAuthMethod: "none", + GrantTypes: []string{"authorization_code", "refresh_token"}, + ResponseTypes: []string{"code"}, ClientName: "term-llm", + Scope: strings.Join(options.Scopes, " "), + }, + } + } + handlerConfig.NewTokenSource = func(ctx context.Context, cfg *oauth2.Config, token *oauth2.Token) (oauth2.TokenSource, error) { + resolvedIssuer := callbackIssuer + if resolvedIssuer == "" { + resolvedIssuer = inferIssuer(cfg.Endpoint.AuthURL) + } + revocationEndpoint := "" + if resolvedIssuer != "" { + metadataCtx, cancel := context.WithTimeout(ctx, metadataTimeout) + if metadata, metadataErr := sdkauth.GetAuthServerMetadata(metadataCtx, resolvedIssuer, client); metadataErr == nil && metadata != nil { + resolvedIssuer = metadata.Issuer + revocationEndpoint = metadata.RevocationEndpoint + } + cancel() + } + saved, err := c.store.Update(canonical, func(current *Session) (*Session, error) { + version := uint64(0) + if current != nil { + version = current.Version + } + return &Session{ + Version: version, Endpoint: canonical, Issuer: resolvedIssuer, + Config: configFromOAuth2(cfg), Token: cloneToken(token), + RevocationEndpoint: revocationEndpoint, + }, nil + }) + if err != nil { + return nil, fmt.Errorf("save MCP OAuth credentials: %w", err) + } + c.setRefreshError(canonical, nil) + return &persistentTokenSource{ + store: c.store, endpoint: canonical, session: saved, + client: client, coordinator: c, + }, nil + } + handler, err := sdkauth.NewAuthorizationCodeHandler(handlerConfig) + if err != nil { + return nil, err + } + configuredScopes := append([]string(nil), options.Scopes...) + if stored != nil { + configuredScopes = unionScopeStrings(configuredScopes, stored.Config.Scopes) + } + if len(configuredScopes) > 0 { + return &scopeOAuthHandler{OAuthHandler: handler, scopes: configuredScopes}, nil + } + return handler, nil +} + +type scopeOAuthHandler struct { + sdkauth.OAuthHandler + scopes []string +} + +func (h *scopeOAuthHandler) Authorize(ctx context.Context, req *http.Request, resp *http.Response) error { + challenges, err := oauthex.ParseWWWAuthenticate(resp.Header.Values("WWW-Authenticate")) + if err != nil { + return h.OAuthHandler.Authorize(ctx, req, resp) + } + foundBearer := false + for index := range challenges { + if challenges[index].Scheme != "bearer" { + continue + } + foundBearer = true + existing := strings.Fields(challenges[index].Params["scope"]) + challenges[index].Params["scope"] = strings.Join(unionScopeStrings(existing, h.scopes), " ") + break + } + if !foundBearer { + challenges = append(challenges, oauthex.Challenge{Scheme: "bearer", Params: map[string]string{"scope": strings.Join(h.scopes, " ")}}) + } + cloned := *resp + cloned.Header = resp.Header.Clone() + cloned.Header.Del("WWW-Authenticate") + for _, challenge := range challenges { + cloned.Header.Add("WWW-Authenticate", formatChallenge(challenge)) + } + return h.OAuthHandler.Authorize(ctx, req, &cloned) +} + +func unionScopeStrings(left, right []string) []string { + seen := make(map[string]bool, len(left)+len(right)) + result := make([]string, 0, len(left)+len(right)) + for _, scope := range append(append([]string(nil), left...), right...) { + if scope = strings.TrimSpace(scope); scope != "" && !seen[scope] { + seen[scope] = true + result = append(result, scope) + } + } + return result +} + +func formatChallenge(challenge oauthex.Challenge) string { + keys := make([]string, 0, len(challenge.Params)) + for key := range challenge.Params { + keys = append(keys, key) + } + sort.Strings(keys) + parts := make([]string, 0, len(keys)) + for _, key := range keys { + value := strings.NewReplacer(`\`, `\\`, `"`, `\"`).Replace(challenge.Params[key]) + parts = append(parts, key+`="`+value+`"`) + } + if len(parts) == 0 { + return challenge.Scheme + } + return challenge.Scheme + " " + strings.Join(parts, ", ") +} + +// Start begins or adopts an interactive authorization flow. It waits only for +// discovery to produce the authorization URL; completion continues in a +// background goroutine until callback, cancellation, or expiry. +func (c *Coordinator) Start(ctx context.Context, endpoint string, options Options, redirectURL string, force bool) (*Flow, error) { + canonical, err := CanonicalEndpoint(endpoint) + if err != nil { + return nil, err + } + if _, err := url.ParseRequestURI(redirectURL); err != nil { + return nil, fmt.Errorf("invalid OAuth redirect URL: %w", err) + } + if !force { + if status := c.Status(canonical); status.State == AuthSignedIn { + return nil, fmt.Errorf("already signed in") + } + } + + c.mu.Lock() + c.expireFlowsLocked(time.Now()) + if id := c.byEndpoint[canonical]; id != "" { + if record := c.flows[id]; record != nil && (record.flow.State == FlowStarting || record.flow.State == FlowPending) { + flow := record.flow + c.mu.Unlock() + return &flow, nil + } + } + id, err := randomID(24) + if err != nil { + c.mu.Unlock() + return nil, fmt.Errorf("create OAuth flow ID: %w", err) + } + flowCtx, cancel := context.WithTimeout(context.Background(), flowLifetime) + record := &flowRecord{ + flow: Flow{ID: id, Endpoint: canonical, ExpiresAt: time.Now().Add(flowLifetime).UTC(), State: FlowStarting}, + callback: make(chan callbackResult, 1), ready: make(chan struct{}), done: make(chan struct{}), cancel: cancel, + } + c.flows[id] = record + c.byEndpoint[canonical] = id + c.mu.Unlock() + + handler, err := c.newHandler(canonical, options, redirectURL, force, record) + if err != nil { + c.finishFlow(record, FlowFailed, err) + return nil, err + } + go c.runFlow(flowCtx, record, handler, options) + + select { + case <-record.ready: + flow, _ := c.Flow(id) + if flow.State == FlowFailed { + return flow, errors.New(flow.Error) + } + flow.Created = true + return flow, nil + case <-ctx.Done(): + c.Cancel(canonical, id) + return nil, ctx.Err() + case <-time.After(preflightTimeout + metadataTimeout): + c.Cancel(canonical, id) + return nil, fmt.Errorf("OAuth discovery timed out") + } +} + +func (c *Coordinator) runFlow(ctx context.Context, record *flowRecord, handler sdkauth.OAuthHandler, options Options) { + response := c.authorizationChallenge(ctx, record.flow.Endpoint, options) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, record.flow.Endpoint, nil) + if err == nil { + err = handler.Authorize(ctx, req, response) + } + if err == nil { + c.finishFlow(record, FlowSucceeded, nil) + return + } + if errors.Is(err, context.DeadlineExceeded) { + c.finishFlow(record, FlowExpired, err) + return + } + if errors.Is(err, context.Canceled) { + c.mu.Lock() + state := record.flow.State + c.mu.Unlock() + if state == FlowStarting || state == FlowPending { + c.finishFlow(record, FlowCanceled, err) + } + return + } + c.finishFlow(record, FlowFailed, err) +} + +func (c *Coordinator) authorizationChallenge(ctx context.Context, endpoint string, options Options) *http.Response { + client := options.HTTPClient + if client == nil { + client = c.client + } + preflightCtx, cancel := context.WithTimeout(ctx, preflightTimeout) + defer cancel() + req, err := http.NewRequestWithContext(preflightCtx, http.MethodPost, endpoint, bytes.NewReader([]byte("{}"))) + if err == nil { + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json, text/event-stream") + if resp, requestErr := client.Do(req); requestErr == nil { + if resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden { + if resp.Body == nil { + resp.Body = io.NopCloser(strings.NewReader("")) + } + if resp.Header.Get("WWW-Authenticate") == "" && len(options.Scopes) > 0 { + resp.Header.Set("WWW-Authenticate", `Bearer scope="`+strings.Join(options.Scopes, " ")+`"`) + } + return resp + } + _ = resp.Body.Close() + } + } + header := make(http.Header) + if len(options.Scopes) > 0 { + header.Set("WWW-Authenticate", `Bearer scope="`+strings.Join(options.Scopes, " ")+`"`) + } + return &http.Response{StatusCode: http.StatusUnauthorized, Header: header, Body: io.NopCloser(strings.NewReader(""))} +} + +// CompleteCallback atomically consumes a callback state capability. +func (c *Coordinator) CompleteCallback(state, code, issuer, oauthError string) (string, bool) { + c.mu.Lock() + id := c.byState[state] + record := c.flows[id] + if id == "" || record == nil || record.callbackState != state || record.flow.State != FlowPending || time.Now().After(record.flow.ExpiresAt) { + c.mu.Unlock() + return "", false + } + delete(c.byState, state) + record.callbackState = "" + record.callbackDelivered = true + c.mu.Unlock() + + result := callbackResult{code: code, state: state, iss: issuer} + if oauthError != "" { + result.err = fmt.Errorf("authorization was denied") + } else if code == "" { + result.err = fmt.Errorf("authorization callback did not include a code") + } + record.callback <- result + return id, true +} + +func (c *Coordinator) Cancel(endpoint, flowID string) bool { + canonical, _ := CanonicalEndpoint(endpoint) + c.mu.Lock() + record := c.flows[flowID] + if record == nil || record.callbackDelivered || (canonical != "" && record.flow.Endpoint != canonical) || (record.flow.State != FlowStarting && record.flow.State != FlowPending) { + c.mu.Unlock() + return false + } + record.flow.State = FlowCanceled + record.flow.Error = "authorization canceled" + if record.callbackState != "" { + delete(c.byState, record.callbackState) + record.callbackState = "" + } + delete(c.byEndpoint, record.flow.Endpoint) + cancel := record.cancel + record.readyOnce.Do(func() { close(record.ready) }) + record.doneOnce.Do(func() { close(record.done) }) + c.mu.Unlock() + cancel() + return true +} + +func (c *Coordinator) Flow(id string) (*Flow, bool) { + c.mu.Lock() + defer c.mu.Unlock() + c.expireFlowsLocked(time.Now()) + record := c.flows[id] + if record == nil { + return nil, false + } + flow := record.flow + return &flow, true +} + +func (c *Coordinator) Wait(ctx context.Context, id string) (*Flow, error) { + c.mu.Lock() + record := c.flows[id] + c.mu.Unlock() + if record == nil { + return nil, fmt.Errorf("unknown OAuth flow") + } + select { + case <-record.done: + flow, _ := c.Flow(id) + return flow, nil + case <-ctx.Done(): + return nil, ctx.Err() + } +} + +func (c *Coordinator) finishFlow(record *flowRecord, state FlowState, err error) { + c.mu.Lock() + if record.flow.State == FlowCanceled && state != FlowCanceled { + c.mu.Unlock() + return + } + record.flow.State = state + record.flow.AuthorizationURL = "" + record.flow.Error = safeError(err) + if state == FlowSucceeded { + record.flow.Error = "" + } + if record.callbackState != "" { + delete(c.byState, record.callbackState) + record.callbackState = "" + } + delete(c.byEndpoint, record.flow.Endpoint) + record.readyOnce.Do(func() { close(record.ready) }) + record.doneOnce.Do(func() { close(record.done) }) + c.mu.Unlock() + record.cancel() +} + +func (c *Coordinator) expireFlowsLocked(now time.Time) { + for id, record := range c.flows { + if now.After(record.flow.ExpiresAt.Add(flowLifetime)) && record.flow.State != FlowStarting && record.flow.State != FlowPending { + delete(c.flows, id) + continue + } + if now.After(record.flow.ExpiresAt) && (record.flow.State == FlowStarting || record.flow.State == FlowPending) { + record.flow.State = FlowExpired + record.flow.AuthorizationURL = "" + record.flow.Error = "authorization expired" + delete(c.byEndpoint, record.flow.Endpoint) + if record.callbackState != "" { + delete(c.byState, record.callbackState) + record.callbackState = "" + } + record.readyOnce.Do(func() { close(record.ready) }) + record.doneOnce.Do(func() { close(record.done) }) + record.cancel() + } + } +} + +func (c *Coordinator) Status(endpoint string) AuthStatus { + status := AuthStatus{State: AuthSignedOut, CanSignIn: true} + if c == nil || c.store == nil { + return status + } + canonical, err := CanonicalEndpoint(endpoint) + if err != nil { + return status + } + c.mu.Lock() + if id := c.byEndpoint[canonical]; id != "" { + if record := c.flows[id]; record != nil && (record.flow.State == FlowStarting || record.flow.State == FlowPending) { + status.State = AuthWaiting + c.mu.Unlock() + return status + } + } + refreshErr := c.refreshErrs[canonical] + c.mu.Unlock() + session, err := c.store.Load(canonical) + if err != nil { + return status + } + status.Issuer = session.Issuer + status.Scopes = append([]string(nil), session.Config.Scopes...) + status.StoragePath = c.store.Path() + status.CanSignOut = true + status.ExpiresAt = session.Token.Expiry + if errors.Is(refreshErr, ErrRefreshRejected) || (!session.Token.Expiry.IsZero() && time.Now().After(session.Token.Expiry) && session.Token.RefreshToken == "") { + status.State = AuthRequired + return status + } + if refreshErr != nil { + status.State = AuthRetry + return status + } + if tokenNeedsRefresh(session.Token) { + if session.Token.RefreshToken != "" { + status.State = AuthExpired + } else { + status.State = AuthRequired + } + return status + } + status.State = AuthSignedIn + return status +} + +func (c *Coordinator) Logout(ctx context.Context, endpoint string, localOnly bool) error { + session, err := c.store.Load(endpoint) + if errors.Is(err, ErrNotFound) { + return nil + } + if err != nil { + return fmt.Errorf("load MCP OAuth credentials: %w", err) + } + var revokeErr error + if !localOnly && session.RevocationEndpoint != "" { + revokeErr = c.revoke(ctx, session) + } + if _, err := c.store.Delete(endpoint); err != nil && !errors.Is(err, ErrNotFound) { + return fmt.Errorf("delete MCP OAuth credentials: %w", err) + } + canonical, _ := CanonicalEndpoint(endpoint) + c.setRefreshError(canonical, nil) + return revokeErr +} + +func (c *Coordinator) revoke(ctx context.Context, session *Session) error { + token := session.Token.RefreshToken + hint := "refresh_token" + if token == "" { + token, hint = session.Token.AccessToken, "access_token" + } + if token == "" { + return nil + } + values := url.Values{"token": {token}, "token_type_hint": {hint}} + useBasicAuth := session.Config.ClientSecret != "" && session.Config.Endpoint.AuthStyle == oauth2.AuthStyleInHeader + if !useBasicAuth { + values.Set("client_id", session.Config.ClientID) + if session.Config.ClientSecret != "" { + values.Set("client_secret", session.Config.ClientSecret) + } + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, session.RevocationEndpoint, strings.NewReader(values.Encode())) + if err != nil { + return fmt.Errorf("create OAuth revocation request: %w", err) + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + if useBasicAuth { + req.SetBasicAuth(session.Config.ClientID, session.Config.ClientSecret) + } + resp, err := c.client.Do(req) + if err != nil { + return fmt.Errorf("revoke MCP OAuth grant: %w", err) + } + defer resp.Body.Close() + _, _ = io.CopyN(io.Discard, resp.Body, 4096) + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return fmt.Errorf("revoke MCP OAuth grant: authorization server returned %s", resp.Status) + } + return nil +} + +func (c *Coordinator) setRefreshError(endpoint string, err error) { + c.mu.Lock() + defer c.mu.Unlock() + if err == nil { + delete(c.refreshErrs, endpoint) + } else { + c.refreshErrs[endpoint] = err + } +} + +type persistentTokenSource struct { + store Store + endpoint string + session *Session + client *http.Client + coordinator *Coordinator + mu sync.Mutex +} + +func (s *persistentTokenSource) Token() (*oauth2.Token, error) { + s.mu.Lock() + defer s.mu.Unlock() + var returned *oauth2.Token + var refreshRejected bool + updated, err := s.store.Update(s.endpoint, func(current *Session) (*Session, error) { + if current == nil || current.Token == nil { + return nil, ErrAuthenticationRequired + } + // A newer process may already have rotated the refresh token. Always + // adopt the disk generation before deciding whether network I/O is needed. + s.session = cloneSession(current) + if !tokenNeedsRefresh(current.Token) { + returned = cloneToken(current.Token) + return nil, nil + } + if current.Token.RefreshToken == "" { + if current.Token.Valid() { + returned = cloneToken(current.Token) + return nil, nil + } + return nil, ErrAuthenticationRequired + } + cfg := current.Config.oauth2Config() + // Refresh runs while holding the store lock so rotation is serialized + // across processes; bound it so a hung token endpoint cannot block + // every other credential operation indefinitely. + refreshCtx, cancelRefresh := context.WithTimeout(context.Background(), refreshTimeout) + defer cancelRefresh() + refreshCtx = context.WithValue(refreshCtx, oauth2.HTTPClient, s.client) + refreshSeed := cloneToken(current.Token) + // oauth2.Config.TokenSource otherwise reuses a token until its own small + // expiry delta. Mark only the in-memory seed expired so our five-minute + // proactive refresh skew is honored without changing the stored grant. + refreshSeed.Expiry = time.Now().Add(-time.Second) + token, refreshErr := cfg.TokenSource(refreshCtx, refreshSeed).Token() + if refreshErr != nil { + classified := classifyRefreshError(refreshErr) + s.coordinator.setRefreshError(s.endpoint, classified) + if errors.Is(classified, ErrRefreshRejected) { + current.Token.RefreshToken = "" + refreshRejected = true + return current, nil + } + return nil, classified + } + returned = cloneToken(token) + next := cloneSession(current) + next.Token = cloneToken(token) + return next, nil + }) + if err != nil { + return nil, err + } + if refreshRejected { + return nil, ErrRefreshRejected + } + if updated != nil { + s.session = updated + } + if returned == nil && updated != nil { + returned = cloneToken(updated.Token) + } + if returned == nil { + return nil, ErrAuthenticationRequired + } + s.coordinator.setRefreshError(s.endpoint, nil) + return returned, nil +} + +func tokenNeedsRefresh(token *oauth2.Token) bool { + if token == nil || token.AccessToken == "" { + return true + } + if token.Expiry.IsZero() { + return false + } + return time.Now().Add(refreshExpirySkew).After(token.Expiry) +} + +func classifyRefreshError(err error) error { + if err == nil { + return nil + } + var retrieveErr *oauth2.RetrieveError + if errors.As(err, &retrieveErr) && retrieveErr.ErrorCode == "invalid_grant" { + return ErrRefreshRejected + } + lower := strings.ToLower(err.Error()) + if strings.Contains(lower, "invalid_grant") || strings.Contains(lower, "revoked") { + return ErrRefreshRejected + } + return fmt.Errorf("temporarily unable to refresh MCP OAuth grant") +} + +// redirectCompatible reports whether an interactive authorization may reuse a +// client registration recorded for storedRedirect when redirecting to next. +func redirectCompatible(storedRedirect, next string) bool { + if storedRedirect == "" || next == "" { + return storedRedirect == next + } + if storedRedirect == next { + return true + } + a, errA := url.Parse(storedRedirect) + b, errB := url.Parse(next) + if errA != nil || errB != nil { + return false + } + // RFC 8252 §7.3: authorization servers must allow loopback redirect URIs + // to vary by port at request time. + return a.Scheme == b.Scheme && a.Path == b.Path && + isLoopbackHost(a.Hostname()) && isLoopbackHost(b.Hostname()) +} + +func isLoopbackHost(host string) bool { + if strings.EqualFold(host, "localhost") { + return true + } + ip := net.ParseIP(host) + return ip != nil && ip.IsLoopback() +} + +func inferIssuer(authURL string) string { + u, err := url.Parse(authURL) + if err != nil || u.Scheme == "" || u.Host == "" { + return "" + } + for _, suffix := range []string{"/oauth2/authorize", "/oauth/authorize", "/authorize"} { + if strings.HasSuffix(u.Path, suffix) { + u.Path = strings.TrimSuffix(u.Path, suffix) + break + } + } + u.RawQuery, u.Fragment = "", "" + return strings.TrimSuffix(u.String(), "/") +} + +func randomID(size int) (string, error) { + buf := make([]byte, size) + if _, err := rand.Read(buf); err != nil { + return "", err + } + return base64.RawURLEncoding.EncodeToString(buf), nil +} + +func cloneToken(token *oauth2.Token) *oauth2.Token { + if token == nil { + return nil + } + copy := *token + return © +} + +func safeError(err error) string { + if err == nil { + return "" + } + if errors.Is(err, ErrAuthenticationRequired) { + return "sign-in required" + } + if errors.Is(err, ErrRefreshRejected) { + return "stored authorization expired or was revoked; sign in again" + } + if errors.Is(err, context.DeadlineExceeded) { + return "authorization expired" + } + if errors.Is(err, context.Canceled) { + return "authorization canceled" + } + var retrieveErr *oauth2.RetrieveError + if errors.As(err, &retrieveErr) { + return "authorization server rejected the token request" + } + text := strings.Map(func(r rune) rune { + if unicode.IsControl(r) { + return -1 + } + return r + }, err.Error()) + text = strings.TrimSpace(text) + if len(text) > maximumSafeErrSize { + text = text[:maximumSafeErrSize] + "…" + } + return text +} diff --git a/internal/mcp/oauth/coordinator_test.go b/internal/mcp/oauth/coordinator_test.go new file mode 100644 index 000000000..3f928dba9 --- /dev/null +++ b/internal/mcp/oauth/coordinator_test.go @@ -0,0 +1,332 @@ +package oauth + +import ( + "context" + "crypto/sha256" + "encoding/base64" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/modelcontextprotocol/go-sdk/oauthex" + "golang.org/x/oauth2" +) + +func TestCoordinatorUsesSDKAuthorizationAndPersistsSession(t *testing.T) { + var server *httptest.Server + var registrations atomic.Int32 + var revocations atomic.Int32 + var sawResource atomic.Bool + var sawPKCE atomic.Bool + mux := http.NewServeMux() + mux.HandleFunc("/resource", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("WWW-Authenticate", fmt.Sprintf(`Bearer resource_metadata="%s/.well-known/oauth-protected-resource/resource", scope="read write"`, server.URL)) + w.WriteHeader(http.StatusUnauthorized) + }) + mux.HandleFunc("/.well-known/oauth-protected-resource/resource", func(w http.ResponseWriter, r *http.Request) { + writeTestJSON(w, map[string]any{ + "resource": server.URL + "/resource", "authorization_servers": []string{server.URL}, + "scopes_supported": []string{"read", "write"}, + }) + }) + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, r *http.Request) { + writeTestJSON(w, map[string]any{ + "issuer": server.URL, "authorization_endpoint": server.URL + "/authorize", + "token_endpoint": server.URL + "/token", "registration_endpoint": server.URL + "/register", + "revocation_endpoint": server.URL + "/revoke", "response_types_supported": []string{"code"}, + "grant_types_supported": []string{"authorization_code", "refresh_token"}, + "token_endpoint_auth_methods_supported": []string{"none"}, + "code_challenge_methods_supported": []string{"S256"}, + "authorization_response_iss_parameter_supported": true, + }) + }) + mux.HandleFunc("/register", func(w http.ResponseWriter, r *http.Request) { + registrations.Add(1) + var metadata oauthex.ClientRegistrationMetadata + if err := json.NewDecoder(r.Body).Decode(&metadata); err != nil { + t.Errorf("decode registration: %v", err) + } + if !containsString(metadata.GrantTypes, "refresh_token") { + t.Errorf("registration grant_types = %v, want refresh_token", metadata.GrantTypes) + } + writeTestJSON(w, map[string]any{ + "client_id": "dynamic-client", "redirect_uris": metadata.RedirectURIs, + "token_endpoint_auth_method": "none", "grant_types": metadata.GrantTypes, + "response_types": []string{"code"}, + }) + }) + mux.HandleFunc("/token", func(w http.ResponseWriter, r *http.Request) { + if err := r.ParseForm(); err != nil { + t.Errorf("parse token form: %v", err) + } + if r.Form.Get("resource") == server.URL+"/resource" { + sawResource.Store(true) + } + verifier := r.Form.Get("code_verifier") + sum := sha256.Sum256([]byte(verifier)) + if verifier != "" && base64.RawURLEncoding.EncodeToString(sum[:]) != "" { + sawPKCE.Store(true) + } + writeTestJSON(w, map[string]any{ + "access_token": "access-secret", "refresh_token": "refresh-secret", + "token_type": "Bearer", "expires_in": 3600, + }) + }) + mux.HandleFunc("/revoke", func(w http.ResponseWriter, r *http.Request) { + revocations.Add(1) + w.WriteHeader(http.StatusOK) + }) + server = httptest.NewServer(mux) + defer server.Close() + + store := NewFileStore(t.TempDir() + "/mcp_oauth.json") + coordinator := NewCoordinator(store) + flow, err := coordinator.Start(t.Context(), server.URL+"/resource", Options{HTTPClient: server.Client(), Scopes: []string{"configured"}}, "http://127.0.0.1/callback", false) + if err != nil { + t.Fatal(err) + } + authorizeURL, err := url.Parse(flow.AuthorizationURL) + if err != nil { + t.Fatal(err) + } + state := authorizeURL.Query().Get("state") + challenge := authorizeURL.Query().Get("code_challenge") + if state == "" || challenge == "" || authorizeURL.Query().Get("code_challenge_method") != "S256" { + t.Fatalf("authorization URL missing SDK PKCE/state: %s", flow.AuthorizationURL) + } + if authorizeURL.Query().Get("resource") != server.URL+"/resource" { + t.Fatalf("authorization resource = %q", authorizeURL.Query().Get("resource")) + } + if !containsString(strings.Fields(authorizeURL.Query().Get("scope")), "configured") { + t.Fatalf("authorization scope = %q, want configured scope", authorizeURL.Query().Get("scope")) + } + if _, ok := coordinator.CompleteCallback("wrong-state", "code", server.URL, ""); ok { + t.Fatal("mismatched state was accepted") + } + if id, ok := coordinator.CompleteCallback(state, "code", server.URL, ""); !ok || id != flow.ID { + t.Fatalf("callback accepted = %v, id = %q", ok, id) + } + completed, err := coordinator.Wait(t.Context(), flow.ID) + if err != nil { + t.Fatal(err) + } + if completed.State != FlowSucceeded { + t.Fatalf("flow state = %s, error = %s", completed.State, completed.Error) + } + if registrations.Load() != 1 || !sawResource.Load() || !sawPKCE.Load() { + t.Fatalf("DCR=%d resource=%v PKCE=%v", registrations.Load(), sawResource.Load(), sawPKCE.Load()) + } + status := coordinator.Status(server.URL + "/resource") + if status.State != AuthSignedIn || status.Issuer != server.URL || !containsString(status.Scopes, "read") { + t.Fatalf("status = %+v", status) + } + + // A new coordinator restores both the DCR client and token through v1.7's + // InitialTokenSource hook without another registration or browser flow. + restarted := NewCoordinator(store) + handler, err := restarted.Handler(server.URL+"/resource", Options{HTTPClient: server.Client()}) + if err != nil { + t.Fatal(err) + } + source, err := handler.TokenSource(t.Context()) + if err != nil || source == nil { + t.Fatalf("restored TokenSource = %v, err = %v", source, err) + } + token, err := source.Token() + if err != nil || token.AccessToken != "access-secret" { + t.Fatalf("restored token = %#v, err = %v", token, err) + } + if registrations.Load() != 1 { + t.Fatalf("registrations after restart = %d, want 1", registrations.Load()) + } + if err := restarted.Logout(t.Context(), server.URL+"/resource", false); err != nil { + t.Fatal(err) + } + if revocations.Load() != 1 { + t.Fatalf("revocations = %d, want 1", revocations.Load()) + } + if _, err := store.Load(server.URL + "/resource"); err != ErrNotFound { + t.Fatalf("Load after logout error = %v, want ErrNotFound", err) + } +} + +func TestConcurrentCoordinatorsAdoptRotatedRefresh(t *testing.T) { + var refreshes atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/token" { + http.NotFound(w, r) + return + } + refreshes.Add(1) + writeTestJSON(w, map[string]any{ + "access_token": "rotated-access", "refresh_token": "rotated-refresh", + "token_type": "Bearer", "expires_in": 3600, + }) + })) + defer server.Close() + endpoint := server.URL + "/resource" + store := NewFileStore(t.TempDir() + "/mcp_oauth.json") + _, err := store.Update(endpoint, func(*Session) (*Session, error) { + return &Session{ + Endpoint: endpoint, Issuer: server.URL, + Config: OAuth2Config{ClientID: "client", Endpoint: oauth2.Endpoint{AuthURL: server.URL + "/authorize", TokenURL: server.URL + "/token"}}, + Token: &oauth2.Token{AccessToken: "expired", RefreshToken: "original-refresh", Expiry: time.Now().Add(-time.Hour)}, + }, nil + }) + if err != nil { + t.Fatal(err) + } + + coordinators := []*Coordinator{NewCoordinator(store), NewCoordinator(NewFileStore(store.Path()))} + var wg sync.WaitGroup + errs := make(chan error, len(coordinators)) + for _, coordinator := range coordinators { + wg.Add(1) + go func(coordinator *Coordinator) { + defer wg.Done() + handler, err := coordinator.Handler(endpoint, Options{HTTPClient: server.Client()}) + if err != nil { + errs <- err + return + } + source, err := handler.TokenSource(context.Background()) + if err == nil { + var token *oauth2.Token + token, err = source.Token() + if err == nil && token.AccessToken != "rotated-access" { + err = fmt.Errorf("access token = %q", token.AccessToken) + } + } + errs <- err + }(coordinator) + } + wg.Wait() + close(errs) + for err := range errs { + if err != nil { + t.Fatal(err) + } + } + if refreshes.Load() != 1 { + t.Fatalf("refresh requests = %d, want 1", refreshes.Load()) + } + stored, err := store.Load(endpoint) + if err != nil { + t.Fatal(err) + } + if stored.Token.RefreshToken != "rotated-refresh" { + t.Fatalf("stored refresh token = %q", stored.Token.RefreshToken) + } +} + +func TestInteractiveStartHonorsStoredClientRedirectCompatibility(t *testing.T) { + var server *httptest.Server + var registrations atomic.Int32 + mux := http.NewServeMux() + mux.HandleFunc("/resource", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("WWW-Authenticate", fmt.Sprintf(`Bearer resource_metadata="%s/.well-known/oauth-protected-resource/resource"`, server.URL)) + w.WriteHeader(http.StatusUnauthorized) + }) + mux.HandleFunc("/.well-known/oauth-protected-resource/resource", func(w http.ResponseWriter, r *http.Request) { + writeTestJSON(w, map[string]any{"resource": server.URL + "/resource", "authorization_servers": []string{server.URL}}) + }) + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, r *http.Request) { + writeTestJSON(w, map[string]any{ + "issuer": server.URL, "authorization_endpoint": server.URL + "/authorize", + "token_endpoint": server.URL + "/token", "registration_endpoint": server.URL + "/register", + "response_types_supported": []string{"code"}, "code_challenge_methods_supported": []string{"S256"}, + }) + }) + mux.HandleFunc("/register", func(w http.ResponseWriter, r *http.Request) { + registrations.Add(1) + var metadata oauthex.ClientRegistrationMetadata + _ = json.NewDecoder(r.Body).Decode(&metadata) + writeTestJSON(w, map[string]any{ + "client_id": "dynamic-2", "redirect_uris": metadata.RedirectURIs, + "token_endpoint_auth_method": "none", "grant_types": metadata.GrantTypes, + "response_types": []string{"code"}, + }) + }) + server = httptest.NewServer(mux) + defer server.Close() + endpoint := server.URL + "/resource" + + tests := []struct { + name string + newRedirect string + wantClientID string + wantRegistrations int32 + }{ + { + // RFC 8252 loopback redirects vary by port between CLI logins. + name: "loopback port change reuses stored client", newRedirect: "http://127.0.0.1:41234/callback", + wantClientID: "dynamic-1", wantRegistrations: 0, + }, + { + // A serve callback is not registered for the stored loopback DCR + // client; a compliant AS would reject it, so re-register instead. + name: "web redirect re-registers", newRedirect: "https://app.example/ui/v1/mcp/oauth/callback", + wantClientID: "dynamic-2", wantRegistrations: 1, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + registrations.Store(0) + store := NewFileStore(t.TempDir() + "/mcp_oauth.json") + _, err := store.Update(endpoint, func(*Session) (*Session, error) { + return &Session{ + Endpoint: endpoint, Issuer: server.URL, + Config: OAuth2Config{ + ClientID: "dynamic-1", + Endpoint: oauth2.Endpoint{AuthURL: server.URL + "/authorize", TokenURL: server.URL + "/token"}, + RedirectURL: "http://127.0.0.1:39999/callback", + }, + Token: &oauth2.Token{AccessToken: "expired", Expiry: time.Now().Add(-time.Hour)}, + }, nil + }) + if err != nil { + t.Fatal(err) + } + coordinator := NewCoordinator(store) + flow, err := coordinator.Start(t.Context(), endpoint, Options{HTTPClient: server.Client()}, tt.newRedirect, false) + if err != nil { + t.Fatal(err) + } + defer coordinator.Cancel(endpoint, flow.ID) + authorizeURL, err := url.Parse(flow.AuthorizationURL) + if err != nil { + t.Fatal(err) + } + if got := authorizeURL.Query().Get("client_id"); got != tt.wantClientID { + t.Fatalf("client_id = %q, want %q", got, tt.wantClientID) + } + if got := authorizeURL.Query().Get("redirect_uri"); got != tt.newRedirect { + t.Fatalf("redirect_uri = %q, want %q", got, tt.newRedirect) + } + if got := registrations.Load(); got != tt.wantRegistrations { + t.Fatalf("registrations = %d, want %d", got, tt.wantRegistrations) + } + }) + } +} + +func writeTestJSON(w http.ResponseWriter, value any) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(value) +} + +func containsString(values []string, want string) bool { + for _, value := range values { + if strings.EqualFold(value, want) { + return true + } + } + return false +} diff --git a/internal/mcp/oauth/store.go b/internal/mcp/oauth/store.go new file mode 100644 index 000000000..4c272de8b --- /dev/null +++ b/internal/mcp/oauth/store.go @@ -0,0 +1,393 @@ +package oauth + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "net/url" + "os" + "path/filepath" + "strings" + "sync" + "time" + + "github.com/samsaffron/term-llm/internal/filelock" + "golang.org/x/oauth2" +) + +const storeSchemaVersion = 1 + +// OAuth2Config is the serializable subset of oauth2.Config required to reuse a +// registration and refresh a token after process restart. +type OAuth2Config struct { + ClientID string `json:"client_id"` + ClientSecret string `json:"client_secret,omitempty"` + Endpoint oauth2.Endpoint `json:"endpoint"` + RedirectURL string `json:"redirect_url"` + Scopes []string `json:"scopes,omitempty"` +} + +func configFromOAuth2(cfg *oauth2.Config) OAuth2Config { + return OAuth2Config{ + ClientID: cfg.ClientID, ClientSecret: cfg.ClientSecret, Endpoint: cfg.Endpoint, + RedirectURL: cfg.RedirectURL, Scopes: append([]string(nil), cfg.Scopes...), + } +} + +func (c OAuth2Config) oauth2Config() *oauth2.Config { + return &oauth2.Config{ + ClientID: c.ClientID, ClientSecret: c.ClientSecret, Endpoint: c.Endpoint, + RedirectURL: c.RedirectURL, Scopes: append([]string(nil), c.Scopes...), + } +} + +// Session is one persisted MCP OAuth grant. Credential-bearing values in this +// type must stay inside this package and the private store file. +type Session struct { + Version uint64 `json:"version"` + Endpoint string `json:"endpoint"` + Issuer string `json:"issuer"` + Config OAuth2Config `json:"config"` + Token *oauth2.Token `json:"token"` + RevocationEndpoint string `json:"revocation_endpoint,omitempty"` + UpdatedAt time.Time `json:"updated_at"` +} + +// Store is the persistence contract used by Coordinator. Update executes while +// holding both an in-process mutex and a cross-process file lock, allowing a +// refresh-token exchange and its rotated-token write to be serialized. +type Store interface { + Path() string + Load(endpoint string) (*Session, error) + Update(endpoint string, fn func(*Session) (*Session, error)) (*Session, error) + Delete(endpoint string) (*Session, error) +} + +type storeFile struct { + Version int `json:"version"` + Entries map[string]*Session `json:"entries"` +} + +// FileStore stores all MCP grants in one private, versioned JSON file. +type FileStore struct { + path string + mu sync.Mutex +} + +func DefaultStorePath() (string, error) { + configDir := os.Getenv("XDG_CONFIG_HOME") + if configDir == "" { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("resolve home directory: %w", err) + } + configDir = filepath.Join(home, ".config") + } + return filepath.Join(configDir, "term-llm", "mcp_oauth.json"), nil +} + +func NewFileStore(path string) *FileStore { return &FileStore{path: path} } +func (s *FileStore) Path() string { return s.path } + +// CanonicalEndpoint normalizes identity without changing path or query, which +// are part of the OAuth resource identity. +func CanonicalEndpoint(raw string) (string, error) { + u, err := url.Parse(raw) + if err != nil { + return "", fmt.Errorf("parse MCP endpoint: %w", err) + } + if u.Scheme != "http" && u.Scheme != "https" { + return "", fmt.Errorf("MCP endpoint must use http or https") + } + if u.Hostname() == "" { + return "", fmt.Errorf("MCP endpoint is missing a host") + } + u.Scheme = strings.ToLower(u.Scheme) + host := strings.ToLower(u.Hostname()) + port := u.Port() + if port != "" && !((u.Scheme == "https" && port == "443") || (u.Scheme == "http" && port == "80")) { + host = host + ":" + port + } + if strings.Contains(u.Hostname(), ":") { + host = "[" + strings.ToLower(u.Hostname()) + "]" + if port != "" && !((u.Scheme == "https" && port == "443") || (u.Scheme == "http" && port == "80")) { + host += ":" + port + } + } + u.Host = host + u.Fragment = "" + return u.String(), nil +} + +func entryKey(endpoint, issuer string) string { + sum := sha256.Sum256([]byte(endpoint + "\x00" + strings.TrimSuffix(issuer, "/"))) + return hex.EncodeToString(sum[:]) +} + +func cloneSession(in *Session) *Session { + if in == nil { + return nil + } + out := *in + out.Config.Scopes = append([]string(nil), in.Config.Scopes...) + if in.Token != nil { + tok := *in.Token + out.Token = &tok + } + return &out +} + +func (s *FileStore) Load(endpoint string) (*Session, error) { + canonical, err := CanonicalEndpoint(endpoint) + if err != nil { + return nil, err + } + var found *Session + err = s.withLock(func() error { + data, err := s.readLocked() + if err != nil { + return err + } + for _, entry := range data.Entries { + if entry.Endpoint == canonical && (found == nil || entry.Version > found.Version) { + found = cloneSession(entry) + } + } + return nil + }) + if err != nil { + return nil, err + } + if found == nil { + return nil, ErrNotFound + } + return found, nil +} + +func (s *FileStore) Update(endpoint string, fn func(*Session) (*Session, error)) (*Session, error) { + canonical, err := CanonicalEndpoint(endpoint) + if err != nil { + return nil, err + } + var result *Session + err = s.withLock(func() error { + data, err := s.readLocked() + if err != nil { + return err + } + var current *Session + var currentKey string + for key, entry := range data.Entries { + if entry.Endpoint == canonical && (current == nil || entry.Version > current.Version) { + current, currentKey = entry, key + } + } + next, err := fn(cloneSession(current)) + if err != nil { + return err + } + if next == nil { + result = cloneSession(current) + return nil + } + if next.Endpoint != "" { + normalized, err := CanonicalEndpoint(next.Endpoint) + if err != nil { + return err + } + if normalized != canonical { + return fmt.Errorf("credential endpoint changed during update") + } + } + next = cloneSession(next) + next.Endpoint = canonical + if current == nil { + if next.Version != 0 { + return ErrStaleVersion + } + next.Version = 1 + } else { + if next.Version != current.Version { + return ErrStaleVersion + } + next.Version = current.Version + 1 + delete(data.Entries, currentKey) + } + next.UpdatedAt = time.Now().UTC() + data.Entries[entryKey(canonical, next.Issuer)] = next + if err := s.writeLocked(data); err != nil { + return err + } + result = cloneSession(next) + return nil + }) + return result, err +} + +func (s *FileStore) Delete(endpoint string) (*Session, error) { + canonical, err := CanonicalEndpoint(endpoint) + if err != nil { + return nil, err + } + var deleted *Session + err = s.withLock(func() error { + data, err := s.readLocked() + if err != nil { + return err + } + changed := false + for key, entry := range data.Entries { + if entry.Endpoint == canonical { + if deleted == nil || entry.Version > deleted.Version { + deleted = cloneSession(entry) + } + delete(data.Entries, key) + changed = true + } + } + if changed { + return s.writeLocked(data) + } + return nil + }) + if err != nil { + return nil, err + } + if deleted == nil { + return nil, ErrNotFound + } + return deleted, nil +} + +func (s *FileStore) withLock(fn func() error) (err error) { + s.mu.Lock() + defer s.mu.Unlock() + if s.path == "" { + return fmt.Errorf("MCP OAuth store path is empty") + } + dir := filepath.Dir(s.path) + if err := os.MkdirAll(dir, 0o700); err != nil { + return fmt.Errorf("create MCP OAuth store directory: %w", err) + } + if err := os.Chmod(dir, 0o700); err != nil { + return fmt.Errorf("secure MCP OAuth store directory: %w", err) + } + if err := rejectUnsafeTarget(s.path); err != nil { + return err + } + lockPath := s.path + ".lock" + if err := rejectUnsafeTarget(lockPath); err != nil { + return err + } + unlock, err := filelock.Lock(lockPath) + if err != nil { + return fmt.Errorf("lock MCP OAuth store: %w", err) + } + defer func() { + if unlockErr := unlock(); err == nil && unlockErr != nil { + err = fmt.Errorf("unlock MCP OAuth store: %w", unlockErr) + } + }() + if err := os.Chmod(lockPath, 0o600); err != nil { + return fmt.Errorf("secure MCP OAuth lock file: %w", err) + } + if err := rejectUnsafeTarget(s.path); err != nil { + return err + } + if _, statErr := os.Stat(s.path); statErr == nil { + if err := os.Chmod(s.path, 0o600); err != nil { + return fmt.Errorf("secure MCP OAuth store: %w", err) + } + } else if !os.IsNotExist(statErr) { + return fmt.Errorf("inspect MCP OAuth store: %w", statErr) + } + return fn() +} + +func rejectUnsafeTarget(path string) error { + info, err := os.Lstat(path) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return fmt.Errorf("inspect MCP OAuth store target: %w", err) + } + if info.Mode()&os.ModeSymlink != 0 || !info.Mode().IsRegular() { + return fmt.Errorf("MCP OAuth store target must be a regular file") + } + return nil +} + +func (s *FileStore) readLocked() (*storeFile, error) { + data, err := os.ReadFile(s.path) + if os.IsNotExist(err) { + return &storeFile{Version: storeSchemaVersion, Entries: make(map[string]*Session)}, nil + } + if err != nil { + return nil, fmt.Errorf("read MCP OAuth store: %w", err) + } + var file storeFile + if err := json.Unmarshal(data, &file); err != nil { + return nil, fmt.Errorf("parse MCP OAuth store: %w", err) + } + if file.Version != storeSchemaVersion { + return nil, fmt.Errorf("unsupported MCP OAuth store version %d", file.Version) + } + if file.Entries == nil { + return nil, fmt.Errorf("invalid MCP OAuth store: missing entries") + } + for _, entry := range file.Entries { + if entry == nil || entry.Token == nil || entry.Config.ClientID == "" || entry.Endpoint == "" { + return nil, fmt.Errorf("invalid MCP OAuth store entry") + } + canonical, err := CanonicalEndpoint(entry.Endpoint) + if err != nil || canonical != entry.Endpoint { + return nil, fmt.Errorf("invalid MCP OAuth store endpoint") + } + } + return &file, nil +} + +func (s *FileStore) writeLocked(file *storeFile) (err error) { + data, err := json.MarshalIndent(file, "", " ") + if err != nil { + return fmt.Errorf("marshal MCP OAuth store: %w", err) + } + dir := filepath.Dir(s.path) + tmp, err := os.CreateTemp(dir, ".mcp_oauth-*") + if err != nil { + return fmt.Errorf("create MCP OAuth temporary store: %w", err) + } + tmpPath := tmp.Name() + defer func() { _ = os.Remove(tmpPath) }() + if err = tmp.Chmod(0o600); err == nil { + _, err = tmp.Write(data) + } + if err == nil { + err = tmp.Sync() + } + closeErr := tmp.Close() + if err != nil { + return fmt.Errorf("write MCP OAuth temporary store: %w", err) + } + if closeErr != nil { + return fmt.Errorf("close MCP OAuth temporary store: %w", closeErr) + } + if err := os.Rename(tmpPath, s.path); err != nil { + return fmt.Errorf("replace MCP OAuth store: %w", err) + } + if err := os.Chmod(s.path, 0o600); err != nil { + return fmt.Errorf("secure MCP OAuth store: %w", err) + } + if dirFile, err := os.Open(dir); err == nil { + if syncErr := dirFile.Sync(); syncErr != nil && !errors.Is(syncErr, context.Canceled) { + _ = dirFile.Close() + return fmt.Errorf("sync MCP OAuth store directory: %w", syncErr) + } + _ = dirFile.Close() + } + return nil +} diff --git a/internal/mcp/oauth/store_test.go b/internal/mcp/oauth/store_test.go new file mode 100644 index 000000000..05f6dffb0 --- /dev/null +++ b/internal/mcp/oauth/store_test.go @@ -0,0 +1,109 @@ +package oauth + +import ( + "errors" + "os" + "path/filepath" + "runtime" + "testing" + "time" + + "golang.org/x/oauth2" +) + +func testSession(endpoint string) *Session { + return &Session{ + Endpoint: endpoint, + Issuer: "https://auth.example", + Config: OAuth2Config{ + ClientID: "client", Endpoint: oauth2.Endpoint{AuthURL: "https://auth.example/authorize", TokenURL: "https://auth.example/token"}, + }, + Token: &oauth2.Token{AccessToken: "access", RefreshToken: "refresh", Expiry: time.Now().Add(time.Hour)}, + } +} + +func TestCanonicalEndpoint(t *testing.T) { + got, err := CanonicalEndpoint("HTTPS://EXAMPLE.COM:443/mcp?tenant=one#fragment") + if err != nil { + t.Fatal(err) + } + if want := "https://example.com/mcp?tenant=one"; got != want { + t.Fatalf("CanonicalEndpoint = %q, want %q", got, want) + } +} + +func TestFileStorePermissionsAndStaleWrite(t *testing.T) { + path := filepath.Join(t.TempDir(), "private", "mcp_oauth.json") + store := NewFileStore(path) + endpoint := "https://mcp.example/mcp" + saved, err := store.Update(endpoint, func(*Session) (*Session, error) { return testSession(endpoint), nil }) + if err != nil { + t.Fatal(err) + } + if saved.Version != 1 { + t.Fatalf("version = %d, want 1", saved.Version) + } + if runtime.GOOS != "windows" { + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if got := info.Mode().Perm(); got != 0o600 { + t.Fatalf("mode = %o, want 600", got) + } + dirInfo, err := os.Stat(filepath.Dir(path)) + if err != nil { + t.Fatal(err) + } + if got := dirInfo.Mode().Perm(); got != 0o700 { + t.Fatalf("directory mode = %o, want 700", got) + } + } + _, err = store.Update(endpoint, func(current *Session) (*Session, error) { + current.Version-- + return current, nil + }) + if !errors.Is(err, ErrStaleVersion) { + t.Fatalf("stale update error = %v, want ErrStaleVersion", err) + } + loaded, err := store.Load(endpoint) + if err != nil { + t.Fatal(err) + } + if loaded.Version != 1 || loaded.Token.RefreshToken != "refresh" { + t.Fatalf("stale write changed stored session: %+v", loaded) + } +} + +func TestFileStoreRejectsSymlinkAndCorruption(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "target") + if err := os.WriteFile(target, []byte(`{"version":1,"entries":{}}`), 0o600); err != nil { + t.Fatal(err) + } + link := filepath.Join(dir, "mcp_oauth.json") + if err := os.Symlink(target, link); err != nil { + t.Skipf("symlink unavailable: %v", err) + } + store := NewFileStore(link) + if _, err := store.Load("https://mcp.example/mcp"); err == nil || !stringsContains(err.Error(), "regular file") { + t.Fatalf("symlink Load error = %v, want regular file rejection", err) + } + + corrupt := filepath.Join(dir, "corrupt.json") + if err := os.WriteFile(corrupt, []byte(`{"version":1,"entries":`), 0o600); err != nil { + t.Fatal(err) + } + if _, err := NewFileStore(corrupt).Load("https://mcp.example/mcp"); err == nil || !stringsContains(err.Error(), "parse MCP OAuth store") { + t.Fatalf("corrupt Load error = %v", err) + } +} + +func stringsContains(value, part string) bool { + for i := 0; i+len(part) <= len(value); i++ { + if value[i:i+len(part)] == part { + return true + } + } + return false +} diff --git a/internal/mcp/oauth/types.go b/internal/mcp/oauth/types.go new file mode 100644 index 000000000..34d9cb40b --- /dev/null +++ b/internal/mcp/oauth/types.go @@ -0,0 +1,77 @@ +package oauth + +import ( + "errors" + "net/http" + "time" +) + +var ( + // ErrAuthenticationRequired indicates that an interactive authorization flow + // is required. Background MCP connections never open a browser themselves. + ErrAuthenticationRequired = errors.New("MCP authentication required") + // ErrRefreshRejected indicates that the authorization server rejected the + // stored refresh grant and the user must sign in again. + ErrRefreshRejected = errors.New("MCP OAuth refresh grant expired or revoked") + // ErrStaleVersion prevents an older process from replacing newer credentials. + ErrStaleVersion = errors.New("stale MCP OAuth credential version") + // ErrNotFound indicates that no grant is stored for an endpoint. + ErrNotFound = errors.New("MCP OAuth credentials not found") +) + +// Options configures the SDK authorization-code handler for an MCP endpoint. +type Options struct { + ClientID string + ClientSecret string + Scopes []string + ClientIDMetadataURL string + HTTPClient *http.Client +} + +// AuthState is the product-facing state of an MCP OAuth grant. +type AuthState string + +const ( + AuthNotNeeded AuthState = "not_needed" + AuthSignedOut AuthState = "signed_out" + AuthSignedIn AuthState = "signed_in" + AuthExpired AuthState = "expired" + AuthRequired AuthState = "needs_sign_in" + AuthWaiting AuthState = "waiting" + AuthRetry AuthState = "retry" +) + +// AuthStatus contains safe grant metadata. It never contains credentials. +type AuthStatus struct { + State AuthState `json:"state"` + Issuer string `json:"issuer,omitempty"` + Scopes []string `json:"scopes,omitempty"` + ExpiresAt time.Time `json:"expires_at,omitempty"` + StoragePath string `json:"storage_path,omitempty"` + CanSignIn bool `json:"can_sign_in"` + CanSignOut bool `json:"can_sign_out"` +} + +// FlowState is the state of an interactive browser authorization flow. +type FlowState string + +const ( + FlowStarting FlowState = "starting" + FlowPending FlowState = "pending" + FlowSucceeded FlowState = "succeeded" + FlowFailed FlowState = "failed" + FlowCanceled FlowState = "canceled" + FlowExpired FlowState = "expired" +) + +// Flow is the safe, public view of an OAuth flow. OAuth codes, PKCE values, +// tokens, client secrets, and the callback state capability are never exposed. +type Flow struct { + ID string `json:"flow_id"` + Endpoint string `json:"-"` + AuthorizationURL string `json:"authorization_url,omitempty"` + ExpiresAt time.Time `json:"expires_at"` + State FlowState `json:"state"` + Error string `json:"error,omitempty"` + Created bool `json:"-"` +} diff --git a/internal/oauth/chatgpt.go b/internal/oauth/chatgpt.go index 608cc72ba..9bae3510a 100644 --- a/internal/oauth/chatgpt.go +++ b/internal/oauth/chatgpt.go @@ -93,6 +93,9 @@ func buildAuthorizationURL(codeChallenge, state string) string { return ChatGPTAuthEndpoint + "?" + params.Encode() } +// OpenBrowser opens a URL in the user's default browser. +func OpenBrowser(url string) error { return openBrowser(url) } + // openBrowser opens a URL in the default browser func openBrowser(url string) error { var cmd *exec.Cmd diff --git a/internal/serveui/embed_test.go b/internal/serveui/embed_test.go index a5f33975f..58504043e 100644 --- a/internal/serveui/embed_test.go +++ b/internal/serveui/embed_test.go @@ -12,7 +12,7 @@ func TestGeneratedBundleAssets(t *testing.T) { for _, name := range []string{ "dist/app.js", "dist/app.css", "dist/chunks/vendor.js", "dist/chunks/rich-highlight.js", "dist/chunks/rich-katex.js", - "dist/chunks/highlight.js", "dist/chunks/katex.js", "dist/chunks/katex.css", "dist/chunks/webrtc.js", + "dist/chunks/highlight.js", "dist/chunks/katex.js", "dist/chunks/katex.css", "dist/chunks/webrtc.js", "dist/chunks/mcp.js", } { body, err := StaticAsset(name) if err != nil { @@ -108,7 +108,7 @@ func TestRenderServiceWorkerVersionsOnlyDirectShellAssets(t *testing.T) { t.Errorf("service worker missing %q", want) } } - for _, chunk := range []string{"vendor.js?v=", "webrtc.js?v=", "highlight.js?v=", "katex.js?v="} { + for _, chunk := range []string{"vendor.js?v=", "webrtc.js?v=", "highlight.js?v=", "katex.js?v=", "mcp.js?v="} { if strings.Contains(without, chunk) || strings.Contains(with, chunk) { t.Errorf("stable-named chunk %q must remain unversioned and network-first", chunk) } diff --git a/internal/tui/chat/chat.go b/internal/tui/chat/chat.go index e89fc72e2..6f9dd6fd5 100644 --- a/internal/tui/chat/chat.go +++ b/internal/tui/chat/chat.go @@ -2681,6 +2681,26 @@ func (m *Model) Update(msg tea.Msg) (model tea.Model, cmd tea.Cmd) { case promptHistoryLookupMsg: return m.handlePromptHistoryLookupMsg(msg) + case mcpOAuthResultMsg: + m.refreshMCPPickerIfOpen() + if msg.err != nil { + detail := safeMCPOAuthMessage(msg.err) + action := fmt.Sprintf("Try `/mcp login %s` again.", msg.name) + if msg.logout { + action = fmt.Sprintf("Try `/mcp logout %s` again.", msg.name) + } + m.dialog.ShowContent("MCP authentication", detail+"\n\n"+action) + _, footerCmd := m.showFooterMessageWithTone("MCP authentication failed: "+detail, "error") + cmds = append(cmds, footerCmd) + } else { + verb := "Signed in to" + if msg.logout { + verb = "Signed out of" + } + _, footerCmd := m.showFooterMessage(verb + " MCP server " + msg.name) + cmds = append(cmds, footerCmd) + } + case mcpStatusUpdateMsg: m.refreshMCPPickerIfOpen() cmds = append(cmds, m.listenForMCPStatusUpdates()) diff --git a/internal/tui/chat/commands.go b/internal/tui/chat/commands.go index a89a04ed6..dc25fa87a 100644 --- a/internal/tui/chat/commands.go +++ b/internal/tui/chat/commands.go @@ -249,10 +249,12 @@ func AllCommands() []Command { { Name: "mcp", Description: "MCP servers (browser, database, git tools)", - Usage: "/mcp [start|stop|add|list|status [tools]]", + Usage: "/mcp [start|stop|login|logout|add|list|status [tools]]", Subcommands: []Subcommand{ {Name: "start", Description: "Start a configured server"}, {Name: "stop", Description: "Stop a running server"}, + {Name: "login", Description: "Sign in to a remote server"}, + {Name: "logout", Description: "Sign out of a remote server"}, {Name: "add", Description: "Add a new server"}, {Name: "list", Description: "Show available servers"}, {Name: "status", Description: "Show server status"}, @@ -2741,6 +2743,35 @@ func (m *Model) cmdMcp(args []string) (tea.Model, tea.Cmd) { m.setTextareaValue("") return m.showSystemMessage(fmt.Sprintf("Restarting MCP server: %s", name)) + case "login": + if m.mcpManager == nil { + return m.showMCPQuickStart() + } + if len(subArgs) != 1 { + return m.showSystemMessage("Usage: `/mcp login `") + } + name, err := m.mcpFindServer(subArgs[0]) + if err != nil { + return m.showSystemMessage(err.Error()) + } + m.setTextareaValue("") + _, footerCmd := m.showFooterMessage("Waiting for MCP authorization in your browser…") + return m, tea.Batch(footerCmd, m.startMCPOAuthCmd(name, false)) + + case "logout": + if m.mcpManager == nil { + return m.showMCPQuickStart() + } + if len(subArgs) != 1 { + return m.showSystemMessage("Usage: `/mcp logout `") + } + name, err := m.mcpFindServer(subArgs[0]) + if err != nil { + return m.showSystemMessage(err.Error()) + } + m.setTextareaValue("") + return m, m.logoutMCPOAuthCmd(name) + case "status": if m.mcpManager == nil { return m.showMCPQuickStart() diff --git a/internal/tui/chat/dialog.go b/internal/tui/chat/dialog.go index 0df4ea190..b72371496 100644 --- a/internal/tui/chat/dialog.go +++ b/internal/tui/chat/dialog.go @@ -377,6 +377,7 @@ func (d *DialogModel) ShowMCPPicker(mcpManager *mcp.Manager, discovery ...llm.To available := mcpManager.AvailableServers() states := mcpManager.GetAllStates() + authStatuses := mcpManager.AuthStatuses() discoveryServers := make(map[string]llm.ToolDiscoveryServerDiagnostic) if len(discovery) > 0 { for _, server := range discovery[0].Servers { @@ -396,8 +397,20 @@ func (d *DialogModel) ShowMCPPicker(mcpManager *mcp.Manager, discovery ...llm.To if status == "" { status = "stopped" } - isRunning := status == "ready" || status == "starting" + isRunning := status == "ready" || status == "starting" || status == "auth_required" description := status + if authStatus, ok := authStatuses[name]; ok { + switch authStatus.State { + case "signed_in": + description += " · signed in" + case "waiting": + description += " · waiting for browser…" + case "signed_out", "needs_sign_in", "expired": + description += " · sign-in required" + case "retry": + description += " · sign-in retry available" + } + } if state.ToolCount > 0 { description += fmt.Sprintf(" · %d tools", state.ToolCount) } @@ -1165,6 +1178,8 @@ func (d *DialogModel) viewMCPPicker() string { statusIcon = successStyle.Render("●") case "starting": statusIcon = warningStyle.Render("◐") + case "auth_required": + statusIcon = warningStyle.Render("○") case "failed": statusIcon = errorStyle.Render("○") default: @@ -1186,9 +1201,20 @@ func (d *DialogModel) viewMCPPicker() string { statusText = warningStyle.Render(" starting...") case "failed": statusText = errorStyle.Render(" failed") + case "auth_required": + statusText = warningStyle.Render(" sign-in required") default: // No status text for stopped servers - cleaner look } + if strings.Contains(item.Description, "waiting for browser") { + statusText += warningStyle.Render(" · waiting for browser…") + } else if status != "auth_required" && strings.Contains(item.Description, "sign-in required") { + statusText += warningStyle.Render(" · sign-in required") + } else if strings.Contains(item.Description, "signed in") { + statusText += successStyle.Render(" · signed in") + } else if strings.Contains(item.Description, "sign-in retry available") { + statusText += warningStyle.Render(" · retry sign-in") + } line := cursor + statusIcon + " " + item.Label + statusText if actualIdx == d.cursor { diff --git a/internal/tui/chat/dialog_test.go b/internal/tui/chat/dialog_test.go index fb8d633db..724837517 100644 --- a/internal/tui/chat/dialog_test.go +++ b/internal/tui/chat/dialog_test.go @@ -17,11 +17,12 @@ func TestMCPPickerRendersStatusIndicatorsWithMetadata(t *testing.T) { {ID: "ready", Label: "ready-server", Description: "ready · 42 tools · refreshed 17:16:40"}, {ID: "starting", Label: "starting-server", Description: "starting"}, {ID: "failed", Label: "failed-server", Description: "failed · error: connection closed"}, + {ID: "auth", Label: "protected-server", Description: "auth_required · sign-in required"}, } d.filtered = d.items view := d.View() - for _, want := range []string{"●", "◐", "○", "ready", "starting", "failed"} { + for _, want := range []string{"●", "◐", "○", "ready", "starting", "failed", "sign-in required"} { if !strings.Contains(view, want) { t.Fatalf("MCP picker missing %q:\n%s", want, view) } diff --git a/internal/tui/chat/handlers.go b/internal/tui/chat/handlers.go index 3df9955e2..83adc72c3 100644 --- a/internal/tui/chat/handlers.go +++ b/internal/tui/chat/handlers.go @@ -744,6 +744,9 @@ func (m *Model) handleKeyMsg(msg tea.KeyPressMsg) (tea.Model, tea.Cmd) { // attached discovery planner. name := selected.ID status, _ := m.mcpManager.ServerStatus(name) + if status == mcp.StatusAuthRequired { + return m, m.startMCPOAuthCmd(name, false) + } if status == mcp.StatusReady || status == mcp.StatusStarting { if err := m.mcpManager.Disable(name); err == nil { m.setMCPServerSelected(name, false) @@ -1348,6 +1351,20 @@ func (m *Model) handleKeyMsg(msg tea.KeyPressMsg) (tea.Model, tea.Cmd) { return m, nil } + // Tab completion for /mcp login|logout . + for _, subcommand := range []string{"login", "logout"} { + prefix := "/mcp " + subcommand + " " + if strings.HasPrefix(valueLower, prefix) && m.mcpManager != nil { + partial := strings.TrimSpace(value[len(prefix):]) + if partial != "" { + if match := m.mcpFindServerMatch(partial); match != "" { + m.setTextareaValue(prefix + match) + } + } + return m, nil + } + } + return m, nil } diff --git a/internal/tui/chat/mcp_oauth.go b/internal/tui/chat/mcp_oauth.go new file mode 100644 index 000000000..1539240ae --- /dev/null +++ b/internal/tui/chat/mcp_oauth.go @@ -0,0 +1,93 @@ +package chat + +import ( + "context" + "fmt" + "html/template" + "net" + "net/http" + "time" + + tea "charm.land/bubbletea/v2" + "github.com/samsaffron/term-llm/internal/mcp" + mcpoauth "github.com/samsaffron/term-llm/internal/mcp/oauth" + internalauth "github.com/samsaffron/term-llm/internal/oauth" + "github.com/samsaffron/term-llm/internal/terminaltext" +) + +type mcpOAuthResultMsg struct { + name string + logout bool + err error +} + +func (m *Model) startMCPOAuthCmd(name string, force bool) tea.Cmd { + return func() tea.Msg { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + return mcpOAuthResultMsg{name: name, err: fmt.Errorf("start callback listener: %w", err)} + } + defer listener.Close() + server := &http.Server{ReadHeaderTimeout: 5 * time.Second} + server.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/callback" { + http.NotFound(w, r) + return + } + _, ok := mcpoauth.DefaultCoordinator().CompleteCallback( + r.URL.Query().Get("state"), r.URL.Query().Get("code"), + r.URL.Query().Get("iss"), r.URL.Query().Get("error"), + ) + if !ok { + http.Error(w, "This authorization callback is invalid, expired, or was already used.", http.StatusBadRequest) + return + } + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.Header().Set("Cache-Control", "no-store") + _ = template.Must(template.New("done").Parse(`Connected

Connected

You can close this window.

`)).Execute(w, nil) + }) + go func() { _ = server.Serve(listener) }() + defer server.Shutdown(context.Background()) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) + defer cancel() + flow, err := m.mcpManager.StartOAuth(ctx, name, mcp.OAuthStartOptions{ + RedirectURL: "http://" + listener.Addr().String() + "/callback", + Force: force, + }) + if err != nil { + return mcpOAuthResultMsg{name: name, err: err} + } + if err := internalauth.OpenBrowser(flow.AuthorizationURL); err != nil { + _ = m.mcpManager.CancelOAuth(name, flow.ID) + return mcpOAuthResultMsg{name: name, err: fmt.Errorf("open browser: %w; use `term-llm mcp login %s --no-browser`", err, name)} + } + completed, err := mcpoauth.DefaultCoordinator().Wait(ctx, flow.ID) + if err != nil { + return mcpOAuthResultMsg{name: name, err: err} + } + if completed.State != mcpoauth.FlowSucceeded { + if completed.Error == "" { + completed.Error = "authorization did not complete" + } + return mcpOAuthResultMsg{name: name, err: fmt.Errorf("%s", completed.Error)} + } + return mcpOAuthResultMsg{name: name} + } +} + +func (m *Model) logoutMCPOAuthCmd(name string) tea.Cmd { + return func() tea.Msg { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + err := m.mcpManager.LogoutOAuth(ctx, name, false) + return mcpOAuthResultMsg{name: name, logout: true, err: err} + } +} + +func safeMCPOAuthMessage(err error) string { + if err == nil { + return "" + } + return terminaltext.SanitizeSingleLine(err.Error()) +} diff --git a/internal/tui/chat/render.go b/internal/tui/chat/render.go index 36064b3ce..3d6657dd8 100644 --- a/internal/tui/chat/render.go +++ b/internal/tui/chat/render.go @@ -1852,7 +1852,9 @@ func (m *Model) updateCompletions() { // Check for "/mcp start ", "/mcp stop ", "/mcp restart " - show configured servers if strings.HasPrefix(lowerQuery, "mcp start ") || strings.HasPrefix(lowerQuery, "mcp stop ") || - strings.HasPrefix(lowerQuery, "mcp restart ") { + strings.HasPrefix(lowerQuery, "mcp restart ") || + strings.HasPrefix(lowerQuery, "mcp login ") || + strings.HasPrefix(lowerQuery, "mcp logout ") { if m.mcpManager != nil { // Extract the partial server name after the subcommand parts := strings.SplitN(query, " ", 3)