From 80bde2e79f6caa8d6e14be15a968a81691a73b33 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 16 Jun 2026 11:38:41 +0300 Subject: [PATCH] feat(cli): add server-edition `credential` command group (spec 074 T9) Add `mcpproxy credential list|status|connect|rm` to the server edition for managing per-user brokered upstream credentials over the T8 REST surfaces (GET/DELETE /api/v1/user/credentials, .../{server}/connect). - Responses are decoded into a non-secret typed view, so token material is never printed (FR-026) even if a response carries it. - Honors -o table|json|yaml and MCPPROXY_OUTPUT via the existing formatters. - Targets a server URL (--url / MCPPROXY_SERVER_URL) with a user JWT (--token / MCPPROXY_TOKEN): these surfaces sit behind session-or-Bearer auth, not the API-key/socket group, so a Bearer transport was added to the shared CLI client. - `connect` prints the browser URL (escaping slash-named servers); the flow binds to the user's browser session server-side. - Build-tagged (//go:build server) and registered via a server-edition command seam; the personal edition is unaffected (verified: `credential` is absent there). Docs: docs/cli/credential-commands.md. Related: spec 074 T9 / MCP-1042 --- cmd/mcpproxy/credential_cmd.go | 340 ++++++++++++++++++++ cmd/mcpproxy/credential_cmd_test.go | 117 +++++++ cmd/mcpproxy/main.go | 3 + cmd/mcpproxy/serveredition_commands.go | 12 + cmd/mcpproxy/serveredition_commands_stub.go | 8 + docs/cli/credential-commands.md | 98 ++++++ internal/cliclient/client.go | 22 +- internal/cliclient/credentials.go | 119 +++++++ internal/cliclient/credentials_test.go | 133 ++++++++ 9 files changed, 848 insertions(+), 4 deletions(-) create mode 100644 cmd/mcpproxy/credential_cmd.go create mode 100644 cmd/mcpproxy/credential_cmd_test.go create mode 100644 cmd/mcpproxy/serveredition_commands.go create mode 100644 cmd/mcpproxy/serveredition_commands_stub.go create mode 100644 docs/cli/credential-commands.md create mode 100644 internal/cliclient/credentials.go create mode 100644 internal/cliclient/credentials_test.go diff --git a/cmd/mcpproxy/credential_cmd.go b/cmd/mcpproxy/credential_cmd.go new file mode 100644 index 000000000..91b2bcf29 --- /dev/null +++ b/cmd/mcpproxy/credential_cmd.go @@ -0,0 +1,340 @@ +//go:build server + +package main + +import ( + "context" + "fmt" + "net/url" + "os" + "strings" + "time" + + "github.com/spf13/cobra" + "go.uber.org/zap" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/cliclient" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" +) + +// Server-edition flags for the credential command group. The credential broker +// surfaces (spec 074) live behind session-or-Bearer auth, so the CLI targets a +// server URL and presents a user JWT — unlike the API-key/socket commands. +var ( + credServerURL string + credToken string +) + +// newCredentialCommand builds the `mcpproxy credential` command tree (server +// edition only). It manages per-user brokered upstream credentials via the +// T8 REST surfaces and never prints secret values (FR-026). +func newCredentialCommand() *cobra.Command { + credentialCmd := &cobra.Command{ + Use: "credential", + Short: "Manage per-user brokered upstream credentials (server edition)", + Long: `Inspect and manage your brokered credentials for shared upstream servers. + +These commands talk to a running server-edition mcpproxy and require a user +token (a JWT obtained from the Web UI or POST /api/v1/auth/token). Provide it +with --token or the MCPPROXY_TOKEN environment variable, and point at the +server with --url or MCPPROXY_SERVER_URL. + +Secret values are never displayed (FR-026). + +Examples: + mcpproxy credential list + mcpproxy credential status github + mcpproxy credential connect github + mcpproxy credential rm github`, + } + + credentialCmd.PersistentFlags().StringVar(&credServerURL, "url", "", "Base URL of the server-edition mcpproxy (default: $MCPPROXY_SERVER_URL or local listen address)") + credentialCmd.PersistentFlags().StringVar(&credToken, "token", "", "User JWT bearer token (default: $MCPPROXY_TOKEN)") + + credentialCmd.AddCommand(newCredentialListCmd()) + credentialCmd.AddCommand(newCredentialStatusCmd()) + credentialCmd.AddCommand(newCredentialConnectCmd()) + credentialCmd.AddCommand(newCredentialRemoveCmd()) + + return credentialCmd +} + +func newCredentialListCmd() *cobra.Command { + return &cobra.Command{ + Use: "list", + Short: "List brokered upstreams with connection status (no secrets)", + Long: `List every brokered upstream and your connection status for it. + +Examples: + mcpproxy credential list + mcpproxy credential list -o json`, + Args: cobra.NoArgs, + RunE: runCredentialList, + } +} + +func newCredentialStatusCmd() *cobra.Command { + return &cobra.Command{ + Use: "status ", + Short: "Show one upstream's connection detail (no secrets)", + Long: `Show the connection detail for a single brokered upstream. + +Examples: + mcpproxy credential status github + mcpproxy credential status github -o yaml`, + Args: cobra.ExactArgs(1), + RunE: runCredentialStatus, + } +} + +func newCredentialConnectCmd() *cobra.Command { + return &cobra.Command{ + Use: "connect ", + Short: "Print the browser URL to connect an upstream credential", + Long: `Print the connect URL for a brokered upstream. Open it in a browser where +you are signed in to mcpproxy to complete the OAuth connect flow; the proxy +binds the flow to your user and stores the resulting credential server-side. + +Examples: + mcpproxy credential connect github`, + Args: cobra.ExactArgs(1), + RunE: runCredentialConnect, + } +} + +func newCredentialRemoveCmd() *cobra.Command { + return &cobra.Command{ + Use: "rm ", + Aliases: []string{"remove", "disconnect"}, + Short: "Disconnect (revoke) your credential for an upstream", + Long: `Disconnect and revoke your stored credential for a brokered upstream. + +Examples: + mcpproxy credential rm github`, + Args: cobra.ExactArgs(1), + RunE: runCredentialRemove, + } +} + +// --- client wiring --- + +// resolveCredentialBaseURL determines the server base URL from the --url flag, +// the MCPPROXY_SERVER_URL env var, or the local listen address in config. +func resolveCredentialBaseURL() string { + if credServerURL != "" { + return strings.TrimRight(credServerURL, "/") + } + if env := os.Getenv("MCPPROXY_SERVER_URL"); env != "" { + return strings.TrimRight(env, "/") + } + if cfg, err := config.Load(); err == nil && cfg.Listen != "" { + listen := cfg.Listen + if strings.HasPrefix(listen, ":") { + listen = "127.0.0.1" + listen + } + return "http://" + listen + } + return "http://127.0.0.1:8080" +} + +// resolveCredentialToken returns the user JWT from --token or MCPPROXY_TOKEN. +func resolveCredentialToken() string { + if credToken != "" { + return credToken + } + return os.Getenv("MCPPROXY_TOKEN") +} + +func newCredentialClient() (*cliclient.Client, string) { + baseURL := resolveCredentialBaseURL() + logger, _ := zap.NewProduction() + return cliclient.NewClientWithBearer(baseURL, resolveCredentialToken(), logger.Sugar()), baseURL +} + +// --- run functions --- + +func runCredentialList(_ *cobra.Command, _ []string) error { + client, _ := newCredentialClient() + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + creds, err := client.ListCredentials(ctx) + if err != nil { + return err + } + return emitCredentials(creds) +} + +func runCredentialStatus(_ *cobra.Command, args []string) error { + client, _ := newCredentialClient() + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + creds, err := client.ListCredentials(ctx) + if err != nil { + return err + } + cred, ok := findCredential(creds, args[0]) + if !ok { + return fmt.Errorf("brokered server %q not found", args[0]) + } + return emitCredentialDetail(cred) +} + +func runCredentialConnect(_ *cobra.Command, args []string) error { + baseURL := resolveCredentialBaseURL() + connectURL := credentialConnectURL(baseURL, args[0]) + + format := ResolveOutputFormat() + if format == "json" || format == "yaml" { + return emitFormatted(map[string]string{"server": args[0], "connect_url": connectURL}) + } + + fmt.Printf("Open this URL in a browser where you are signed in to mcpproxy:\n\n %s\n\n", connectURL) + fmt.Println("After authorizing, the credential is stored server-side and tied to your user.") + return nil +} + +func runCredentialRemove(_ *cobra.Command, args []string) error { + client, _ := newCredentialClient() + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + msg, err := client.DeleteCredential(ctx, args[0]) + if err != nil { + return err + } + if msg == "" { + msg = fmt.Sprintf("Disconnected credential for %q", args[0]) + } + fmt.Println(msg) + return nil +} + +// --- pure helpers (kept side-effect-free for testing) --- + +// findCredential looks up a credential by case-insensitive server name. +func findCredential(creds []cliclient.CredentialStatus, name string) (cliclient.CredentialStatus, bool) { + for _, c := range creds { + if strings.EqualFold(c.Server, name) { + return c, true + } + } + return cliclient.CredentialStatus{}, false +} + +// credentialConnectURL builds the browser connect URL for a brokered upstream. +// The server name is path-escaped so namespace/name registry identifiers do not +// inject extra path segments (cf. MCP-1111). +func credentialConnectURL(baseURL, server string) string { + return fmt.Sprintf("%s/api/v1/user/credentials/%s/connect", + strings.TrimRight(baseURL, "/"), url.PathEscape(server)) +} + +// renderCredentialsTable renders the list view. It only ever reads the +// non-secret fields of CredentialStatus, so no secret can appear (FR-026). +func renderCredentialsTable(creds []cliclient.CredentialStatus) string { + if len(creds) == 0 { + return "No brokered upstreams configured." + } + var b strings.Builder + fmt.Fprintf(&b, "%-24s %-16s %-15s %-10s %-20s\n", "SERVER", "MODE", "STATUS", "TOKEN", "EXPIRES") + b.WriteString(strings.Repeat("-", 90) + "\n") + needsConnect := false + for _, c := range creds { + expires := "-" + if c.ExpiresAt != nil { + expires = c.ExpiresAt.Format("2006-01-02 15:04") + } + tokenType := c.TokenType + if tokenType == "" { + tokenType = "-" + } + marker := "" + if c.ConnectPath != "" { + marker = " *" + needsConnect = true + } + fmt.Fprintf(&b, "%-24s %-16s %-15s %-10s %-20s\n", + truncateCell(c.Server, 24), truncateCell(c.Mode, 16), c.Status+marker, truncateCell(tokenType, 10), expires) + } + if needsConnect { + b.WriteString("\n* connectable: run 'mcpproxy credential connect '\n") + } + return strings.TrimRight(b.String(), "\n") +} + +// renderCredentialDetail renders the single-server status view. +func renderCredentialDetail(c cliclient.CredentialStatus) string { + var b strings.Builder + fmt.Fprintf(&b, "Server: %s\n", c.Server) + fmt.Fprintf(&b, "Mode: %s\n", c.Mode) + fmt.Fprintf(&b, "Status: %s\n", c.Status) + if c.TokenType != "" { + fmt.Fprintf(&b, "Token Type: %s\n", c.TokenType) + } + if len(c.Scopes) > 0 { + fmt.Fprintf(&b, "Scopes: %s\n", strings.Join(c.Scopes, ", ")) + } + if c.Audience != "" { + fmt.Fprintf(&b, "Audience: %s\n", c.Audience) + } + if c.ObtainedVia != "" { + fmt.Fprintf(&b, "Obtained Via: %s\n", c.ObtainedVia) + } + if c.ExpiresAt != nil { + fmt.Fprintf(&b, "Expires: %s\n", c.ExpiresAt.Format("2006-01-02 15:04:05")) + } + if c.UpdatedAt != nil { + fmt.Fprintf(&b, "Updated: %s\n", c.UpdatedAt.Format("2006-01-02 15:04:05")) + } + if c.ConnectPath != "" { + fmt.Fprintf(&b, "Connect: mcpproxy credential connect %s\n", c.Server) + } + return strings.TrimRight(b.String(), "\n") +} + +func truncateCell(s string, maxLen int) string { + if len(s) <= maxLen { + return s + } + if maxLen <= 3 { + return s[:maxLen] + } + return s[:maxLen-3] + "..." +} + +// emitCredentials prints the list in the resolved output format. +func emitCredentials(creds []cliclient.CredentialStatus) error { + switch ResolveOutputFormat() { + case "json", "yaml": + return emitFormatted(creds) + default: + fmt.Println(renderCredentialsTable(creds)) + return nil + } +} + +func emitCredentialDetail(c cliclient.CredentialStatus) error { + switch ResolveOutputFormat() { + case "json", "yaml": + return emitFormatted(c) + default: + fmt.Println(renderCredentialDetail(c)) + return nil + } +} + +// emitFormatted renders arbitrary data via the resolved structured formatter. +func emitFormatted(data interface{}) error { + formatter, err := GetOutputFormatter() + if err != nil { + return fmt.Errorf("failed to get output formatter: %w", err) + } + out, err := formatter.Format(data) + if err != nil { + return fmt.Errorf("failed to format output: %w", err) + } + fmt.Println(out) + return nil +} diff --git a/cmd/mcpproxy/credential_cmd_test.go b/cmd/mcpproxy/credential_cmd_test.go new file mode 100644 index 000000000..6cf6cee8c --- /dev/null +++ b/cmd/mcpproxy/credential_cmd_test.go @@ -0,0 +1,117 @@ +//go:build server + +package main + +import ( + "strings" + "testing" + "time" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/cliclient" +) + +func sampleCredentials() []cliclient.CredentialStatus { + exp := time.Date(2026, 7, 1, 12, 0, 0, 0, time.UTC) + return []cliclient.CredentialStatus{ + {Server: "github", Mode: "oauth_connect", Status: "connected", TokenType: "Bearer", Scopes: []string{"repo"}, ObtainedVia: "connect_flow", ExpiresAt: &exp}, + {Server: "jira", Mode: "oauth_connect", Status: "not_connected", ConnectPath: "/api/v1/user/credentials/jira/connect"}, + } +} + +func TestCredentialCommandTree(t *testing.T) { + cmd := newCredentialCommand() + want := map[string]bool{"list": false, "status": false, "connect": false, "rm": false} + for _, sub := range cmd.Commands() { + want[sub.Name()] = true + } + for name, found := range want { + if !found { + t.Errorf("credential command missing subcommand %q", name) + } + } +} + +func TestRenderCredentialsTable(t *testing.T) { + out := renderCredentialsTable(sampleCredentials()) + for _, want := range []string{"SERVER", "github", "connected", "not_connected", "oauth_connect"} { + if !strings.Contains(out, want) { + t.Errorf("table missing %q\n%s", want, out) + } + } + // The connectable marker + hint must surface for not_connected upstreams. + if !strings.Contains(out, "connectable") { + t.Errorf("expected connect hint in table:\n%s", out) + } +} + +func TestRenderCredentialsTable_Empty(t *testing.T) { + if got := renderCredentialsTable(nil); !strings.Contains(got, "No brokered upstreams") { + t.Errorf("unexpected empty render: %q", got) + } +} + +// FR-026: the rendered detail/table must never contain token-like field names +// or values. The render functions only read non-secret fields, so this is a +// regression guard on the struct contract. +func TestRenderCredential_NoSecretLeak(t *testing.T) { + out := renderCredentialDetail(sampleCredentials()[0]) + renderCredentialsTable(sampleCredentials()) + for _, banned := range []string{"access_token", "refresh_token", "AccessToken", "RefreshToken"} { + if strings.Contains(out, banned) { + t.Errorf("rendered output leaked secret-bearing field %q:\n%s", banned, out) + } + } +} + +func TestRenderCredentialDetail(t *testing.T) { + out := renderCredentialDetail(sampleCredentials()[0]) + for _, want := range []string{"Server:", "github", "Status:", "connected", "Scopes:", "repo", "Expires:"} { + if !strings.Contains(out, want) { + t.Errorf("detail missing %q\n%s", want, out) + } + } +} + +func TestFindCredential_CaseInsensitive(t *testing.T) { + creds := sampleCredentials() + got, ok := findCredential(creds, "GitHub") + if !ok || got.Server != "github" { + t.Errorf("expected case-insensitive match for github, got ok=%v server=%q", ok, got.Server) + } + if _, ok := findCredential(creds, "nope"); ok { + t.Errorf("expected no match for unknown server") + } +} + +func TestCredentialConnectURL_EscapesServer(t *testing.T) { + got := credentialConnectURL("http://localhost:8080/", "ns/name") + want := "http://localhost:8080/api/v1/user/credentials/ns%2Fname/connect" + if got != want { + t.Errorf("connect URL = %q, want %q", got, want) + } +} + +func TestResolveCredentialBaseURL_FlagAndEnv(t *testing.T) { + t.Setenv("MCPPROXY_SERVER_URL", "https://env.example.com/") + credServerURL = "" + if got := resolveCredentialBaseURL(); got != "https://env.example.com" { + t.Errorf("env base URL = %q", got) + } + credServerURL = "https://flag.example.com/" + defer func() { credServerURL = "" }() + if got := resolveCredentialBaseURL(); got != "https://flag.example.com" { + t.Errorf("flag base URL = %q (flag should win)", got) + } +} + +func TestResolveCredentialToken(t *testing.T) { + credToken = "" + t.Setenv("MCPPROXY_TOKEN", "env-tok") + if got := resolveCredentialToken(); got != "env-tok" { + t.Errorf("env token = %q", got) + } + credToken = "flag-tok" + defer func() { credToken = "" }() + if got := resolveCredentialToken(); got != "flag-tok" { + t.Errorf("flag token = %q (flag should win)", got) + } +} diff --git a/cmd/mcpproxy/main.go b/cmd/mcpproxy/main.go index ca6a7ab05..b1c920fa7 100644 --- a/cmd/mcpproxy/main.go +++ b/cmd/mcpproxy/main.go @@ -209,6 +209,9 @@ func main() { rootCmd.AddCommand(connectCmd) rootCmd.AddCommand(disconnectCmd) + // Server-edition-only commands (e.g. `credential`). No-op in personal edition. + registerServerEditionCommands(rootCmd) + // Setup --help-json for machine-readable help discovery // This must be called AFTER all commands are added clioutput.SetupHelpJSON(rootCmd) diff --git a/cmd/mcpproxy/serveredition_commands.go b/cmd/mcpproxy/serveredition_commands.go new file mode 100644 index 000000000..d85c14908 --- /dev/null +++ b/cmd/mcpproxy/serveredition_commands.go @@ -0,0 +1,12 @@ +//go:build server + +package main + +import "github.com/spf13/cobra" + +// registerServerEditionCommands adds CLI commands that only exist in the server +// edition. rootCmd is a local in main(), so the personal/server split is done +// via this build-tagged hook (mirrors collectServerEditionInfo). +func registerServerEditionCommands(root *cobra.Command) { + root.AddCommand(newCredentialCommand()) +} diff --git a/cmd/mcpproxy/serveredition_commands_stub.go b/cmd/mcpproxy/serveredition_commands_stub.go new file mode 100644 index 000000000..5734813db --- /dev/null +++ b/cmd/mcpproxy/serveredition_commands_stub.go @@ -0,0 +1,8 @@ +//go:build !server + +package main + +import "github.com/spf13/cobra" + +// registerServerEditionCommands is a no-op in the personal edition. +func registerServerEditionCommands(_ *cobra.Command) {} diff --git a/docs/cli/credential-commands.md b/docs/cli/credential-commands.md new file mode 100644 index 000000000..6ff385f8c --- /dev/null +++ b/docs/cli/credential-commands.md @@ -0,0 +1,98 @@ +# Credential CLI Commands (Server Edition) + +> **Server edition only.** These commands are built into `mcpproxy-server` +> (`go build -tags server`). They are not present in the personal edition. + +The `mcpproxy credential` command group manages your **per-user brokered +credentials** for shared upstream servers that use the credential broker +(spec 074). Brokered upstreams carry an `auth_broker` block in the server +config; each user connects their own credential, and the proxy injects it at +call time. + +Secret values (access/refresh tokens) are **never displayed** by these +commands (FR-026). The CLI decodes responses into a non-secret view, so even a +misbehaving server cannot cause a token to be printed. + +## Connecting to the server + +These surfaces sit behind session-or-Bearer authentication (not the API-key +group), so the CLI targets a server URL and presents a **user JWT**: + +| Setting | Flag | Environment variable | Default | +|---------|------|----------------------|---------| +| Server base URL | `--url` | `MCPPROXY_SERVER_URL` | local listen address (`http://`) | +| User token (JWT) | `--token` | `MCPPROXY_TOKEN` | _none_ | + +Obtain a token from the Web UI after signing in, or via +`POST /api/v1/auth/token`. The `connect` subcommand does **not** need a token — +it prints a URL you open in a browser where you are already signed in. + +```bash +export MCPPROXY_SERVER_URL=https://mcp.example.com +export MCPPROXY_TOKEN=eyJ... # your user JWT +``` + +## Commands + +### `credential list` + +List every brokered upstream with your connection status. No secrets. + +```bash +mcpproxy credential list +mcpproxy credential list -o json +``` + +``` +SERVER MODE STATUS TOKEN EXPIRES +------------------------------------------------------------------------------------------ +github oauth_connect connected Bearer 2026-07-01 12:00 +jira oauth_connect not_connected * - - + +* connectable: run 'mcpproxy credential connect ' +``` + +Status values: `connected`, `expired`, `not_connected`, `unavailable` +(the latter means the server's credential store is disabled). + +### `credential status ` + +Show the connection detail for one brokered upstream. + +```bash +mcpproxy credential status github +mcpproxy credential status github -o yaml +``` + +### `credential connect ` + +Print the browser URL that starts the per-user OAuth connect flow. Open it in a +browser where you are signed in to mcpproxy; the proxy binds the flow to your +user and stores the resulting credential server-side. + +```bash +mcpproxy credential connect github +``` + +### `credential rm ` + +Disconnect (revoke) your stored credential for an upstream. Aliases: `remove`, +`disconnect`. + +```bash +mcpproxy credential rm github +``` + +## Output formatting + +All read commands honor the global `-o table|json|yaml` flag and the +`MCPPROXY_OUTPUT` environment variable (table is the default). + +## Related REST endpoints (spec 074 T8) + +| Endpoint | Description | +|----------|-------------| +| `GET /api/v1/user/credentials` | List connection status (no secrets) | +| `DELETE /api/v1/user/credentials/{server}` | Disconnect/revoke | +| `GET /api/v1/user/credentials/{server}/connect` | Start the browser connect flow | +| `GET /api/v1/user/credentials/{server}/callback` | OAuth callback (browser) | diff --git a/internal/cliclient/client.go b/internal/cliclient/client.go index abaff9d04..a51afb928 100644 --- a/internal/cliclient/client.go +++ b/internal/cliclient/client.go @@ -19,10 +19,11 @@ import ( // Client provides HTTP API access for CLI commands. type Client struct { - baseURL string - apiKey string - httpClient *http.Client - logger *zap.SugaredLogger + baseURL string + apiKey string + bearerToken string + httpClient *http.Client + logger *zap.SugaredLogger } // clientVersion holds the build-time version reported in X-MCPProxy-Client. @@ -146,6 +147,16 @@ func NewClientWithAPIKey(endpoint, apiKey string, logger *zap.SugaredLogger) *Cl } } +// NewClientWithBearer creates a CLI HTTP client that authenticates with a JWT +// Bearer token. The server edition user/JWT endpoints (e.g. the per-user +// credential broker surfaces, spec 074) sit behind session-or-Bearer auth +// rather than the API-key group, so callers must present a user token. +func NewClientWithBearer(endpoint, bearerToken string, logger *zap.SugaredLogger) *Client { + c := NewClientWithAPIKey(endpoint, "", logger) + c.bearerToken = bearerToken + return c +} + // DoRaw performs a raw HTTP request to the API and returns the response. // The caller is responsible for closing the response body. // The body parameter can be nil for methods that don't require a request body. @@ -175,6 +186,9 @@ func (c *Client) prepareRequest(ctx context.Context, req *http.Request) { if c.apiKey != "" { req.Header.Set("X-API-Key", c.apiKey) } + if c.bearerToken != "" && req.Header.Get("Authorization") == "" { + req.Header.Set("Authorization", "Bearer "+c.bearerToken) + } } // parseAPIError creates an APIError from API response fields. diff --git a/internal/cliclient/credentials.go b/internal/cliclient/credentials.go new file mode 100644 index 000000000..a94bbcde5 --- /dev/null +++ b/internal/cliclient/credentials.go @@ -0,0 +1,119 @@ +//go:build server + +package cliclient + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "time" +) + +// CredentialStatus is the non-secret connection view for one brokered upstream, +// as returned by GET /api/v1/user/credentials (spec 074 T8). It deliberately +// mirrors the server's response shape but contains NO token fields: the CLI +// decodes into this typed struct precisely so that any secret material a +// response might carry is dropped rather than rendered (FR-026). +type CredentialStatus struct { + Server string `json:"server" yaml:"server"` + Mode string `json:"mode" yaml:"mode"` + Status string `json:"status" yaml:"status"` + TokenType string `json:"token_type,omitempty" yaml:"token_type,omitempty"` + Scopes []string `json:"scopes,omitempty" yaml:"scopes,omitempty"` + Audience string `json:"audience,omitempty" yaml:"audience,omitempty"` + ObtainedVia string `json:"obtained_via,omitempty" yaml:"obtained_via,omitempty"` + ExpiresAt *time.Time `json:"expires_at,omitempty" yaml:"expires_at,omitempty"` + UpdatedAt *time.Time `json:"updated_at,omitempty" yaml:"updated_at,omitempty"` + ConnectPath string `json:"connect_path,omitempty" yaml:"connect_path,omitempty"` +} + +// credentialListResponse wraps the per-user credential statuses. +type credentialListResponse struct { + Credentials []CredentialStatus `json:"credentials"` +} + +// credentialMessageResponse is the {"message": ...} shape returned by DELETE. +type credentialMessageResponse struct { + Message string `json:"message"` + Error string `json:"error"` +} + +// ListCredentials fetches the connection status of every brokered upstream for +// the authenticated user. The result never contains secret values (FR-026): +// the response is decoded into the typed CredentialStatus, which has no token +// fields, so any secret a response carries is discarded. +func (c *Client) ListCredentials(ctx context.Context) ([]CredentialStatus, error) { + resp, err := c.DoRaw(ctx, http.MethodGet, "/api/v1/user/credentials", nil) + if err != nil { + return nil, fmt.Errorf("failed to call credentials API: %w", err) + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response: %w", err) + } + if resp.StatusCode != http.StatusOK { + return nil, credentialHTTPError(resp.StatusCode, body) + } + + var out credentialListResponse + if err := json.Unmarshal(body, &out); err != nil { + return nil, fmt.Errorf("failed to parse response: %w", err) + } + return out.Credentials, nil +} + +// DeleteCredential disconnects (revokes) the authenticated user's credential for +// a brokered upstream and returns the server's confirmation message. +func (c *Client) DeleteCredential(ctx context.Context, server string) (string, error) { + path := "/api/v1/user/credentials/" + url.PathEscape(server) + resp, err := c.DoRaw(ctx, http.MethodDelete, path, nil) + if err != nil { + return "", fmt.Errorf("failed to call credentials API: %w", err) + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return "", fmt.Errorf("failed to read response: %w", err) + } + if resp.StatusCode != http.StatusOK { + return "", credentialHTTPError(resp.StatusCode, body) + } + + var out credentialMessageResponse + if err := json.Unmarshal(body, &out); err != nil { + return "", fmt.Errorf("failed to parse response: %w", err) + } + return out.Message, nil +} + +// credentialHTTPError builds a friendly error from a non-200 credential +// response, preferring the server's "message"/"error" field and adding a hint +// for the common unauthenticated case. +func credentialHTTPError(status int, body []byte) error { + msg := "" + var parsed credentialMessageResponse + if json.Unmarshal(body, &parsed) == nil { + if parsed.Message != "" { + msg = parsed.Message + } else if parsed.Error != "" { + msg = parsed.Error + } + } + if status == http.StatusUnauthorized { + hint := "authentication required: provide a user token via --token or MCPPROXY_TOKEN" + if msg != "" { + return fmt.Errorf("%s (%s)", hint, msg) + } + return fmt.Errorf("%s", hint) + } + if msg != "" { + return fmt.Errorf("credentials API returned status %d: %s", status, msg) + } + return fmt.Errorf("credentials API returned status %d: %s", status, string(body)) +} diff --git a/internal/cliclient/credentials_test.go b/internal/cliclient/credentials_test.go new file mode 100644 index 000000000..89282217a --- /dev/null +++ b/internal/cliclient/credentials_test.go @@ -0,0 +1,133 @@ +//go:build server + +package cliclient + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "go.uber.org/zap" +) + +func newTestClient(t *testing.T, baseURL, bearer string) *Client { + t.Helper() + return NewClientWithBearer(baseURL, bearer, zap.NewNop().Sugar()) +} + +func TestListCredentials_ParsesNonSecretFields(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/user/credentials" || r.Method != http.MethodGet { + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"credentials":[ + {"server":"github","mode":"oauth_connect","status":"connected","token_type":"Bearer","scopes":["repo"],"obtained_via":"connect_flow"}, + {"server":"jira","mode":"oauth_connect","status":"not_connected","connect_path":"/api/v1/user/credentials/jira/connect"} + ]}`)) + })) + defer srv.Close() + + creds, err := newTestClient(t, srv.URL, "tok").ListCredentials(context.Background()) + if err != nil { + t.Fatalf("ListCredentials: %v", err) + } + if len(creds) != 2 { + t.Fatalf("expected 2 credentials, got %d", len(creds)) + } + if creds[0].Server != "github" || creds[0].Status != "connected" || creds[0].TokenType != "Bearer" { + t.Errorf("unexpected first credential: %+v", creds[0]) + } + if creds[1].ConnectPath != "/api/v1/user/credentials/jira/connect" { + t.Errorf("expected connect_path on jira, got %q", creds[1].ConnectPath) + } +} + +// FR-026: even if a (mis-)behaving server returns secret token material, the +// CLI client must never surface it. Decoding into the typed struct drops it. +func TestListCredentials_DropsSecretMaterial(t *testing.T) { + const secret = "super-secret-access-token-value" + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"credentials":[{"server":"github","mode":"oauth_connect","status":"connected","access_token":"` + secret + `","refresh_token":"` + secret + `"}]}`)) + })) + defer srv.Close() + + creds, err := newTestClient(t, srv.URL, "tok").ListCredentials(context.Background()) + if err != nil { + t.Fatalf("ListCredentials: %v", err) + } + // Re-marshal the typed result and confirm the secret cannot appear. + blob, _ := json.Marshal(creds) + if strings.Contains(string(blob), secret) { + t.Fatalf("secret material leaked into client result: %s", blob) + } +} + +func TestListCredentials_SendsBearerToken(t *testing.T) { + var gotAuth string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotAuth = r.Header.Get("Authorization") + _, _ = w.Write([]byte(`{"credentials":[]}`)) + })) + defer srv.Close() + + if _, err := newTestClient(t, srv.URL, "my-jwt").ListCredentials(context.Background()); err != nil { + t.Fatalf("ListCredentials: %v", err) + } + if gotAuth != "Bearer my-jwt" { + t.Fatalf("expected Authorization 'Bearer my-jwt', got %q", gotAuth) + } +} + +func TestListCredentials_UnauthorizedHint(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"message":"Authentication required"}`)) + })) + defer srv.Close() + + _, err := newTestClient(t, srv.URL, "").ListCredentials(context.Background()) + if err == nil || !strings.Contains(err.Error(), "--token") { + t.Fatalf("expected unauthenticated hint mentioning --token, got %v", err) + } +} + +func TestDeleteCredential_ReturnsMessage(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodDelete { + t.Errorf("expected DELETE, got %s", r.Method) + } + if r.URL.Path != "/api/v1/user/credentials/github" { + t.Errorf("unexpected path %q", r.URL.Path) + } + _, _ = w.Write([]byte(`{"message":"Disconnected credential for \"github\""}`)) + })) + defer srv.Close() + + msg, err := newTestClient(t, srv.URL, "tok").DeleteCredential(context.Background(), "github") + if err != nil { + t.Fatalf("DeleteCredential: %v", err) + } + if !strings.Contains(msg, "Disconnected") { + t.Fatalf("unexpected message: %q", msg) + } +} + +func TestDeleteCredential_EscapesSlashInServerName(t *testing.T) { + var gotPath string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.EscapedPath() + _, _ = w.Write([]byte(`{"message":"ok"}`)) + })) + defer srv.Close() + + if _, err := newTestClient(t, srv.URL, "tok").DeleteCredential(context.Background(), "ns/name"); err != nil { + t.Fatalf("DeleteCredential: %v", err) + } + if !strings.Contains(gotPath, "ns%2Fname") { + t.Fatalf("expected slash to be escaped in path, got %q", gotPath) + } +}