diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index d215469..4fb7c83 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -7,9 +7,9 @@ {"id":"forgejo-mcp-broker-wgo","title":"Phase 2e: OAuth security review + attack-path tests","description":"Phase 2 exit gate. Review every handler for classic OAuth vulns (open redirect, code replay, mix-up, token leak in logs, host spoofing). Add at least one test per attack class. Update design.md §8 with findings.","acceptance_criteria":"Review checklist documented. Tests added for: PKCE mismatch, stale code, token absent from log attributes, bad redirect_uri, mismatched state, replay of used code.","status":"open","priority":1,"issue_type":"task","owner":"olemd@glemt.net","created_at":"2026-04-24T15:45:14Z","created_by":"Ole-Morten Duesund","updated_at":"2026-04-24T15:45:14Z","dependencies":[{"issue_id":"forgejo-mcp-broker-wgo","depends_on_id":"forgejo-mcp-broker-b2o","type":"blocks","created_at":"2026-04-24T17:45:26Z","created_by":"Ole-Morten Duesund","metadata":"{}"},{"issue_id":"forgejo-mcp-broker-wgo","depends_on_id":"forgejo-mcp-broker-pur","type":"blocks","created_at":"2026-04-24T17:45:25Z","created_by":"Ole-Morten Duesund","metadata":"{}"}],"dependency_count":2,"dependent_count":0,"comment_count":0} {"id":"forgejo-mcp-broker-zuq","title":"Phase 3a: internal/supervisor managed stdio subprocess","description":"Child type: Start, Stop(ctx) with SIGTERM -\u003e grace -\u003e SIGKILL, Wait+reap goroutine (no zombies), stderr drainer with prefix. Protocol-agnostic.","acceptance_criteria":"Unit tests against an echo-loop helper: round trip, graceful stop, kill-after-grace, child-exits-on-own detection, stderr capture. Manual spawn of real forgejo-mcp --transport stdio works.","status":"open","priority":1,"issue_type":"task","owner":"olemd@glemt.net","created_at":"2026-04-24T15:45:14Z","created_by":"Ole-Morten Duesund","updated_at":"2026-04-24T15:45:14Z","dependency_count":0,"dependent_count":3,"comment_count":0} {"id":"forgejo-mcp-broker-b2o","title":"Phase 2d: OAuth discovery endpoints (/.well-known/*)","description":"GET /.well-known/oauth-protected-resource and /.well-known/oauth-authorization-server. Issuer URLs MUST derive from cfg.PublicURL, never inbound headers (host-header attack defense per design.md §8).","acceptance_criteria":"Responses validate against RFC 8414/9728 shapes. Issuer URL sourced from config only. supported_scopes matches cfg.ForgejoOAuthScopes.","status":"open","priority":1,"issue_type":"task","owner":"olemd@glemt.net","created_at":"2026-04-24T15:45:13Z","created_by":"Ole-Morten Duesund","updated_at":"2026-04-24T15:45:13Z","dependencies":[{"issue_id":"forgejo-mcp-broker-b2o","depends_on_id":"forgejo-mcp-broker-pur","type":"blocks","created_at":"2026-04-24T17:45:25Z","created_by":"Ole-Morten Duesund","metadata":"{}"}],"dependency_count":1,"dependent_count":1,"comment_count":0} -{"id":"forgejo-mcp-broker-b9i","title":"Phase 2b: internal/forgejo OAuth client","description":"Broker-side OAuth client for upstream Forgejo: authorize URL builder, code-to-token exchange, refresh_token grant, userinfo fetch, revoke. Used by AS callback and refresh machinery. Stateless; caller owns persistence.","acceptance_criteria":"Unit tests with httptest.Server fake Forgejo cover each grant plus error paths (wrong code, expired refresh, revoked token). No state persisted in this package.","status":"open","priority":1,"issue_type":"task","owner":"olemd@glemt.net","created_at":"2026-04-24T15:45:12Z","created_by":"Ole-Morten Duesund","updated_at":"2026-04-24T15:45:12Z","dependency_count":0,"dependent_count":1,"comment_count":0} +{"id":"forgejo-mcp-broker-b9i","title":"Phase 2b: internal/forgejo OAuth client","description":"Broker-side OAuth client for upstream Forgejo: authorize URL builder, code-to-token exchange, refresh_token grant, userinfo fetch, revoke. Used by AS callback and refresh machinery. Stateless; caller owns persistence.","acceptance_criteria":"Unit tests with httptest.Server fake Forgejo cover each grant plus error paths (wrong code, expired refresh, revoked token). No state persisted in this package.","status":"in_progress","priority":1,"issue_type":"task","assignee":"Ole-Morten Duesund","owner":"olemd@glemt.net","created_at":"2026-04-24T15:45:12Z","created_by":"Ole-Morten Duesund","updated_at":"2026-04-27T11:29:17Z","started_at":"2026-04-27T11:29:17Z","dependency_count":0,"dependent_count":1,"comment_count":0} {"id":"forgejo-mcp-broker-pur","title":"Phase 2c: internal/oauth AS endpoints (register, authorize, callback, token, revoke)","description":"Five OAuth handlers per design.md §4.1. RFC 7591 DCR with ephemeral client IDs, authorize -\u003e Forgejo delegation, callback minting broker auth codes, token exchange with SHA-256 hashing at rest, revoke. PKCE S256 required.","acceptance_criteria":"End-to-end curl walkthrough from plan.md phase 2 passes. All tokens hashed at rest. Auth codes single-use, 10-min TTL. Rejects missing PKCE, non-S256, wrong verifier, expired codes/tokens. Handler coverage \u003e=80%.","status":"open","priority":1,"issue_type":"task","owner":"olemd@glemt.net","created_at":"2026-04-24T15:45:12Z","created_by":"Ole-Morten Duesund","updated_at":"2026-04-24T15:45:12Z","dependencies":[{"issue_id":"forgejo-mcp-broker-pur","depends_on_id":"forgejo-mcp-broker-b9i","type":"blocks","created_at":"2026-04-24T17:45:24Z","created_by":"Ole-Morten Duesund","metadata":"{}"},{"issue_id":"forgejo-mcp-broker-pur","depends_on_id":"forgejo-mcp-broker-cpb","type":"blocks","created_at":"2026-04-24T17:45:24Z","created_by":"Ole-Morten Duesund","metadata":"{}"}],"dependency_count":2,"dependent_count":5,"comment_count":0} -{"id":"forgejo-mcp-broker-cpb","title":"Phase 2a: OAuth tables migration","description":"Add migrations/0002_oauth_tables.sql creating clients, auth_codes, access_tokens, refresh_tokens per design.md §4.2. Broker tokens stored as SHA-256 hashes; Forgejo tokens cleartext (subprocess spawning requires them). See plan.md phase 2.","acceptance_criteria":"Migration applies on a fresh DB and is idempotent on reopen. Schema matches design.md §4.2. Tests verify every table and key column exists.","status":"in_progress","priority":1,"issue_type":"task","assignee":"Ole-Morten Duesund","owner":"olemd@glemt.net","created_at":"2026-04-24T15:45:04Z","created_by":"Ole-Morten Duesund","updated_at":"2026-04-27T11:26:17Z","started_at":"2026-04-27T11:26:17Z","dependency_count":0,"dependent_count":1,"comment_count":0} +{"id":"forgejo-mcp-broker-cpb","title":"Phase 2a: OAuth tables migration","description":"Add migrations/0002_oauth_tables.sql creating clients, auth_codes, access_tokens, refresh_tokens per design.md §4.2. Broker tokens stored as SHA-256 hashes; Forgejo tokens cleartext (subprocess spawning requires them). See plan.md phase 2.","acceptance_criteria":"Migration applies on a fresh DB and is idempotent on reopen. Schema matches design.md §4.2. Tests verify every table and key column exists.","status":"closed","priority":1,"issue_type":"task","assignee":"Ole-Morten Duesund","owner":"olemd@glemt.net","created_at":"2026-04-24T15:45:04Z","created_by":"Ole-Morten Duesund","updated_at":"2026-04-27T11:28:20Z","started_at":"2026-04-27T11:26:17Z","closed_at":"2026-04-27T11:28:20Z","close_reason":"0002_oauth_tables.sql shipped: clients/auth_codes/access_tokens/refresh_tokens with cascading FKs, indexes on hot-path columns, and an oauth_schema_version marker. PRAGMA-driven tests verify columns; FK cascade tested in both directions.","dependency_count":0,"dependent_count":1,"comment_count":0} {"id":"forgejo-mcp-broker-8ei","title":"Phase 1: internal/httpserver with /healthz and graceful shutdown","description":"Implement internal/httpserver: constructs a *http.Server bound to cfg.Listen, mounts GET /healthz (returns 200 with JSON build-info from the build-info package, including version, git revision, build date, and current store status), handles SIGTERM/SIGINT by initiating graceful shutdown with a configurable deadline (default 10s). Uses log/slog for structured JSON logs. Exposes a Run(ctx) error method that blocks until shutdown completes.","acceptance_criteria":"go test ./internal/httpserver passes; GET /healthz returns expected JSON; sending SIGTERM causes Run to return nil within 2 seconds after in-flight requests complete; slow in-flight request is allowed to finish within the deadline, then forcibly closed.","status":"closed","priority":1,"issue_type":"task","assignee":"Ole-Morten Duesund","owner":"olemd@glemt.net","created_at":"2026-04-24T14:46:20Z","created_by":"Ole-Morten Duesund","updated_at":"2026-04-24T15:26:43Z","started_at":"2026-04-24T15:24:09Z","closed_at":"2026-04-24T15:26:43Z","close_reason":"httpserver shipped: /healthz with store probe, graceful shutdown with force-close fallback, ExtraHandler extension point. 97.9% coverage. internal/log also implemented in the same commit (100% coverage).","dependencies":[{"issue_id":"forgejo-mcp-broker-8ei","depends_on_id":"forgejo-mcp-broker-n84","type":"blocks","created_at":"2026-04-24T16:46:19Z","created_by":"Ole-Morten Duesund","metadata":"{}"}],"dependency_count":1,"dependent_count":1,"comment_count":0} {"id":"forgejo-mcp-broker-t37","title":"Phase 1: wire cmd/broker/main.go and integration test","description":"Final phase 1 task: wire config → log → store → httpserver in cmd/broker/main.go. Parse config, init slog, open store, start httpserver, wait for shutdown signal, close store, exit. Add an integration test under cmd/broker/ that builds the binary, runs it with a valid env + temp store path, curls /healthz, sends SIGTERM, verifies clean exit within 2s. This is the acceptance gate for phase 1.","acceptance_criteria":"make build; make test (incl. integration) pass; running the binary with missing config fails with a clear error; running with valid config serves /healthz; SIGTERM shuts down cleanly within 2s; /healthz JSON includes version, git revision, build date, and store OK status.","status":"closed","priority":1,"issue_type":"task","assignee":"Ole-Morten Duesund","owner":"olemd@glemt.net","created_at":"2026-04-24T14:46:20Z","created_by":"Ole-Morten Duesund","updated_at":"2026-04-24T15:29:44Z","started_at":"2026-04-24T15:27:58Z","closed_at":"2026-04-24T15:29:44Z","close_reason":"Main wired, signal.NotifyContext triggers shutdown cascade, integration tests green. Phase 1 complete: binary starts with valid config, serves /healthz JSON, shuts down cleanly on SIGTERM within 2s.","dependencies":[{"issue_id":"forgejo-mcp-broker-t37","depends_on_id":"forgejo-mcp-broker-8ei","type":"blocks","created_at":"2026-04-24T16:48:29Z","created_by":"Ole-Morten Duesund","metadata":"{}"},{"issue_id":"forgejo-mcp-broker-t37","depends_on_id":"forgejo-mcp-broker-9jh","type":"blocks","created_at":"2026-04-24T16:48:29Z","created_by":"Ole-Morten Duesund","metadata":"{}"},{"issue_id":"forgejo-mcp-broker-t37","depends_on_id":"forgejo-mcp-broker-9nq","type":"blocks","created_at":"2026-04-24T16:48:28Z","created_by":"Ole-Morten Duesund","metadata":"{}"},{"issue_id":"forgejo-mcp-broker-t37","depends_on_id":"forgejo-mcp-broker-n84","type":"blocks","created_at":"2026-04-24T16:48:28Z","created_by":"Ole-Morten Duesund","metadata":"{}"}],"dependency_count":4,"dependent_count":0,"comment_count":0} {"id":"forgejo-mcp-broker-9jh","title":"Phase 1: internal/store with SQLite open and embedded schema migrations","description":"Implement internal/store: wraps a modernc.org/sqlite connection, applies embedded schema migrations in order via a schema_migrations table, exposes a *sql.DB and a Close method. Phase 1 schema is just the migrations table itself plus a health_check row — real tables (clients, auth_codes, access_tokens, refresh_tokens) ship in phase 2. Store_path from config; creates parent dirs if missing; fails fast on unwritable path. Migrations embedded via embed.FS under internal/store/migrations/.","acceptance_criteria":"go test ./internal/store passes; opening a fresh db file applies migrations; re-opening is idempotent (no re-application, no errors); corrupt/locked files yield a clear error; Close() leaves no file handles open.","status":"closed","priority":1,"issue_type":"task","assignee":"Ole-Morten Duesund","owner":"olemd@glemt.net","created_at":"2026-04-24T14:46:19Z","created_by":"Ole-Morten Duesund","updated_at":"2026-04-24T15:22:53Z","started_at":"2026-04-24T15:11:36Z","closed_at":"2026-04-24T15:22:53Z","close_reason":"Store package shipped: modernc.org/sqlite, embed.FS migrations, WAL + FK pragmas, idempotent reopen, 90.1% coverage including bad-SQL rollback and record-step PK conflict.","dependencies":[{"issue_id":"forgejo-mcp-broker-9jh","depends_on_id":"forgejo-mcp-broker-n84","type":"blocks","created_at":"2026-04-24T16:46:19Z","created_by":"Ole-Morten Duesund","metadata":"{}"}],"dependency_count":1,"dependent_count":1,"comment_count":0} diff --git a/internal/forgejo/forgejo.go b/internal/forgejo/forgejo.go new file mode 100644 index 0000000..2e2c235 --- /dev/null +++ b/internal/forgejo/forgejo.go @@ -0,0 +1,277 @@ +// Package forgejo is the broker's OAuth 2.1 client for upstream Forgejo. +// +// Scope is narrow on purpose: build the authorize URL, exchange a code for +// access+refresh tokens, refresh, and fetch user info. The package is +// stateless — callers own persistence (the OAuth AS in internal/oauth holds +// the token store, not us). +// +// Forgejo speaks OIDC at /login/oauth/* and exposes a userinfo endpoint +// returning standard OIDC claims. Token revocation is not yet supported by +// upstream Forgejo (as of this writing), so the broker handles "revoke" by +// dropping its own copy and letting the upstream token expire naturally. +package forgejo + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "time" +) + +// Default HTTP timeout. Conservative — Forgejo OAuth is a sub-second +// operation in healthy conditions; 30s leaves room for slow networks +// without letting a misbehaving upstream stall a request indefinitely. +const defaultHTTPTimeout = 30 * time.Second + +// Client talks to a Forgejo instance's OAuth 2.1 endpoints. +type Client struct { + baseURL *url.URL + clientID string + clientSecret string + userAgent string + httpClient *http.Client +} + +// ClientConfig collects the fields required to construct a Client. BaseURL, +// ClientID, and ClientSecret are required; the rest have sensible defaults. +type ClientConfig struct { + BaseURL string + ClientID string + ClientSecret string + UserAgent string // default "fjmcp-broker" + HTTPClient *http.Client // default with a 30s timeout +} + +// NewClient validates the config and returns a ready-to-use Client. +func NewClient(cfg ClientConfig) (*Client, error) { + if cfg.BaseURL == "" { + return nil, errors.New("forgejo: BaseURL is required") + } + if cfg.ClientID == "" { + return nil, errors.New("forgejo: ClientID is required") + } + if cfg.ClientSecret == "" { + return nil, errors.New("forgejo: ClientSecret is required") + } + u, err := url.Parse(cfg.BaseURL) + if err != nil { + return nil, fmt.Errorf("forgejo: parse BaseURL %q: %w", cfg.BaseURL, err) + } + if u.Scheme != "http" && u.Scheme != "https" { + return nil, fmt.Errorf("forgejo: BaseURL must use http(s), got %q", u.Scheme) + } + if u.Host == "" { + return nil, fmt.Errorf("forgejo: BaseURL missing host: %q", cfg.BaseURL) + } + httpClient := cfg.HTTPClient + if httpClient == nil { + httpClient = &http.Client{Timeout: defaultHTTPTimeout} + } + ua := cfg.UserAgent + if ua == "" { + ua = "fjmcp-broker" + } + return &Client{ + baseURL: u, + clientID: cfg.ClientID, + clientSecret: cfg.ClientSecret, + userAgent: ua, + httpClient: httpClient, + }, nil +} + +// AuthorizeURLOptions controls the redirect to Forgejo's authorize endpoint. +type AuthorizeURLOptions struct { + RedirectURI string // where Forgejo will send the user back (broker /oauth/callback) + State string // opaque CSRF token; broker stores and re-checks it + Scopes string // space-separated; mapped to Forgejo's coarse scope set + CodeChallenge string // PKCE challenge (S256) + CodeChallengeMethod string // must be "S256" +} + +// AuthorizeURL builds the URL to redirect the user-agent to so Forgejo can +// authenticate them and ask for consent. Required: RedirectURI, State, +// CodeChallenge, CodeChallengeMethod. Scopes is optional (Forgejo will use +// its app-default if empty). +func (c *Client) AuthorizeURL(opts AuthorizeURLOptions) string { + q := url.Values{} + q.Set("response_type", "code") + q.Set("client_id", c.clientID) + q.Set("redirect_uri", opts.RedirectURI) + q.Set("state", opts.State) + q.Set("code_challenge", opts.CodeChallenge) + q.Set("code_challenge_method", opts.CodeChallengeMethod) + if opts.Scopes != "" { + q.Set("scope", opts.Scopes) + } + u := *c.baseURL + u.Path = strings.TrimRight(u.Path, "/") + "/login/oauth/authorize" + u.RawQuery = q.Encode() + return u.String() +} + +// TokenResponse is the parsed body of a successful token endpoint response. +type TokenResponse struct { + AccessToken string `json:"access_token"` + TokenType string `json:"token_type"` + RefreshToken string `json:"refresh_token,omitempty"` + ExpiresIn int `json:"expires_in"` + Scope string `json:"scope,omitempty"` +} + +// Error is the structured form of an OAuth error response from the token +// or userinfo endpoint. Callers can `errors.As(err, &forgejo.Error{})` to +// decide how to react (e.g. invalid_grant → user must re-authenticate). +type Error struct { + Code string // OAuth error code, e.g. "invalid_grant" + Description string // optional human-readable description + HTTPStatus int // upstream HTTP status code +} + +func (e *Error) Error() string { + if e.Description != "" { + return fmt.Sprintf("forgejo oauth: %s (http %d): %s", e.Code, e.HTTPStatus, e.Description) + } + return fmt.Sprintf("forgejo oauth: %s (http %d)", e.Code, e.HTTPStatus) +} + +// ExchangeCode swaps an authorization code for access+refresh tokens. +// codeVerifier is the PKCE verifier matching the challenge sent on /authorize. +func (c *Client) ExchangeCode(ctx context.Context, code, codeVerifier, redirectURI string) (*TokenResponse, error) { + form := url.Values{} + form.Set("grant_type", "authorization_code") + form.Set("code", code) + form.Set("redirect_uri", redirectURI) + form.Set("client_id", c.clientID) + form.Set("client_secret", c.clientSecret) + form.Set("code_verifier", codeVerifier) + return c.postToken(ctx, form) +} + +// Refresh exchanges a refresh token for a new access token. Forgejo returns +// a new refresh token alongside (token rotation). +func (c *Client) Refresh(ctx context.Context, refreshToken string) (*TokenResponse, error) { + form := url.Values{} + form.Set("grant_type", "refresh_token") + form.Set("refresh_token", refreshToken) + form.Set("client_id", c.clientID) + form.Set("client_secret", c.clientSecret) + return c.postToken(ctx, form) +} + +func (c *Client) postToken(ctx context.Context, form url.Values) (*TokenResponse, error) { + endpoint := c.endpoint("/login/oauth/access_token") + req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, strings.NewReader(form.Encode())) + if err != nil { + return nil, fmt.Errorf("forgejo: build token request: %w", err) + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("Accept", "application/json") + req.Header.Set("User-Agent", c.userAgent) + + resp, err := c.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("forgejo: token request: %w", err) + } + defer resp.Body.Close() + + body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<16)) + if err != nil { + return nil, fmt.Errorf("forgejo: read token response: %w", err) + } + + if resp.StatusCode/100 != 2 { + return nil, parseOAuthError(body, resp.StatusCode) + } + + var tok TokenResponse + if err := json.Unmarshal(body, &tok); err != nil { + return nil, fmt.Errorf("forgejo: decode token response: %w", err) + } + if tok.AccessToken == "" { + return nil, fmt.Errorf("forgejo: token response missing access_token") + } + return &tok, nil +} + +// UserInfo is the subset of OIDC userinfo claims the broker cares about. +// Forgejo populates `sub` with the numeric user ID (as a string). +type UserInfo struct { + Sub string `json:"sub"` + PreferredUsername string `json:"preferred_username"` + Name string `json:"name,omitempty"` + Email string `json:"email,omitempty"` +} + +// FetchUserInfo calls the OIDC /login/oauth/userinfo endpoint with the given +// access token and returns the parsed claims. +func (c *Client) FetchUserInfo(ctx context.Context, accessToken string) (*UserInfo, error) { + endpoint := c.endpoint("/login/oauth/userinfo") + req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) + if err != nil { + return nil, fmt.Errorf("forgejo: build userinfo request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+accessToken) + req.Header.Set("Accept", "application/json") + req.Header.Set("User-Agent", c.userAgent) + + resp, err := c.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("forgejo: userinfo request: %w", err) + } + defer resp.Body.Close() + + body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<16)) + if err != nil { + return nil, fmt.Errorf("forgejo: read userinfo response: %w", err) + } + if resp.StatusCode/100 != 2 { + // userinfo errors aren't always shaped as RFC 6749 OAuth errors — + // some Forgejo versions return plain JSON like {"message": "..."} + // for 401. Try the OAuth shape first, fall back to a generic. + if oerr := parseOAuthError(body, resp.StatusCode); oerr.Code != "" { + return nil, oerr + } + return nil, &Error{ + Code: "userinfo_failed", + Description: strings.TrimSpace(string(body)), + HTTPStatus: resp.StatusCode, + } + } + var ui UserInfo + if err := json.Unmarshal(body, &ui); err != nil { + return nil, fmt.Errorf("forgejo: decode userinfo response: %w", err) + } + if ui.Sub == "" { + return nil, fmt.Errorf("forgejo: userinfo response missing sub") + } + return &ui, nil +} + +func (c *Client) endpoint(path string) string { + u := *c.baseURL + u.Path = strings.TrimRight(u.Path, "/") + path + u.RawQuery = "" + return u.String() +} + +// parseOAuthError extracts a structured Error from a 4xx/5xx body. If the +// body isn't valid JSON or doesn't carry an "error" field, returns an Error +// with Code="" so callers can fall back to a generic message. +func parseOAuthError(body []byte, status int) *Error { + var raw struct { + Code string `json:"error"` + Description string `json:"error_description"` + } + _ = json.Unmarshal(body, &raw) // best-effort + return &Error{ + Code: raw.Code, + Description: raw.Description, + HTTPStatus: status, + } +} diff --git a/internal/forgejo/forgejo_test.go b/internal/forgejo/forgejo_test.go new file mode 100644 index 0000000..dfb66d2 --- /dev/null +++ b/internal/forgejo/forgejo_test.go @@ -0,0 +1,390 @@ +package forgejo_test + +import ( + "errors" + "io" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "kode.naiv.no/olemd/forgejo-mcp-broker/internal/forgejo" +) + +// newTestClient returns a Client pointed at the given test server URL with +// well-known credentials. Callers can override individual ClientConfig +// fields by setting them on the returned config before calling NewClient +// themselves; this helper just keeps the boilerplate down. +func newTestClient(t *testing.T, baseURL string) *forgejo.Client { + t.Helper() + c, err := forgejo.NewClient(forgejo.ClientConfig{ + BaseURL: baseURL, + ClientID: "test-client-id", + ClientSecret: "test-client-secret", + UserAgent: "test-broker", + }) + if err != nil { + t.Fatalf("NewClient: %v", err) + } + return c +} + +func TestNewClient_ValidationErrors(t *testing.T) { + cases := []struct { + name string + cfg forgejo.ClientConfig + want string + }{ + {"no_base_url", forgejo.ClientConfig{ClientID: "id", ClientSecret: "s"}, "BaseURL"}, + {"no_client_id", forgejo.ClientConfig{BaseURL: "https://x", ClientSecret: "s"}, "ClientID"}, + {"no_client_secret", forgejo.ClientConfig{BaseURL: "https://x", ClientID: "id"}, "ClientSecret"}, + {"bad_scheme", forgejo.ClientConfig{BaseURL: "ftp://x", ClientID: "id", ClientSecret: "s"}, "http(s)"}, + {"no_host", forgejo.ClientConfig{BaseURL: "https://", ClientID: "id", ClientSecret: "s"}, "missing host"}, + {"unparseable_url", forgejo.ClientConfig{BaseURL: "://nope", ClientID: "id", ClientSecret: "s"}, "parse BaseURL"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := forgejo.NewClient(tc.cfg) + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Errorf("want error containing %q, got %v", tc.want, err) + } + }) + } +} + +func TestNewClient_DefaultsApplied(t *testing.T) { + c, err := forgejo.NewClient(forgejo.ClientConfig{ + BaseURL: "https://forgejo.example.com", ClientID: "id", ClientSecret: "s", + }) + if err != nil { + t.Fatalf("NewClient: %v", err) + } + // Smoke-test by building a URL — defaults don't have a public getter. + u := c.AuthorizeURL(forgejo.AuthorizeURLOptions{ + RedirectURI: "https://x/cb", State: "st", CodeChallenge: "cc", CodeChallengeMethod: "S256", + }) + if !strings.HasPrefix(u, "https://forgejo.example.com/login/oauth/authorize?") { + t.Errorf("authorize URL prefix wrong: %s", u) + } +} + +func TestAuthorizeURL_AllParamsPresent(t *testing.T) { + c := newTestClient(t, "https://forgejo.example.com") + out := c.AuthorizeURL(forgejo.AuthorizeURLOptions{ + RedirectURI: "https://broker.example.com/oauth/callback", + State: "csrf-token", + Scopes: "read:user write:repository", + CodeChallenge: "challenge-string", + CodeChallengeMethod: "S256", + }) + u, err := url.Parse(out) + if err != nil { + t.Fatalf("parse: %v", err) + } + q := u.Query() + want := map[string]string{ + "response_type": "code", + "client_id": "test-client-id", + "redirect_uri": "https://broker.example.com/oauth/callback", + "state": "csrf-token", + "scope": "read:user write:repository", + "code_challenge": "challenge-string", + "code_challenge_method": "S256", + } + for k, v := range want { + if q.Get(k) != v { + t.Errorf("query[%q] = %q, want %q", k, q.Get(k), v) + } + } + if u.Path != "/login/oauth/authorize" { + t.Errorf("path = %q, want /login/oauth/authorize", u.Path) + } +} + +func TestAuthorizeURL_OmitsScopeWhenEmpty(t *testing.T) { + c := newTestClient(t, "https://forgejo.example.com") + out := c.AuthorizeURL(forgejo.AuthorizeURLOptions{ + RedirectURI: "https://x/cb", State: "s", CodeChallenge: "c", CodeChallengeMethod: "S256", + }) + u, _ := url.Parse(out) + if u.Query().Has("scope") { + t.Errorf("scope should not appear when empty: %s", out) + } +} + +func TestAuthorizeURL_BaseWithTrailingSlash(t *testing.T) { + // Trailing slash on BaseURL must not cause a double-slash path. + c := newTestClient(t, "https://forgejo.example.com/") + out := c.AuthorizeURL(forgejo.AuthorizeURLOptions{ + RedirectURI: "https://x/cb", State: "s", CodeChallenge: "c", CodeChallengeMethod: "S256", + }) + if strings.Contains(out, "com//login") { + t.Errorf("double slash in URL: %s", out) + } +} + +// fakeForgejo wraps an httptest.Server with handler injection points so each +// test can shape the response without rewriting the boilerplate. +type fakeForgejo struct { + t *testing.T + server *httptest.Server + tokenStatus int + tokenBody string + userStatus int + userBody string + lastForm url.Values // populated after a token endpoint hit + lastAuth string // populated after a userinfo hit +} + +func newFakeForgejo(t *testing.T) *fakeForgejo { + t.Helper() + f := &fakeForgejo{ + t: t, + tokenStatus: http.StatusOK, + userStatus: http.StatusOK, + } + mux := http.NewServeMux() + mux.HandleFunc("/login/oauth/access_token", func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + form, _ := url.ParseQuery(string(body)) + f.lastForm = form + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(f.tokenStatus) + _, _ = io.WriteString(w, f.tokenBody) + }) + mux.HandleFunc("/login/oauth/userinfo", func(w http.ResponseWriter, r *http.Request) { + f.lastAuth = r.Header.Get("Authorization") + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(f.userStatus) + _, _ = io.WriteString(w, f.userBody) + }) + f.server = httptest.NewServer(mux) + t.Cleanup(f.server.Close) + return f +} + +func TestExchangeCode_Success(t *testing.T) { + f := newFakeForgejo(t) + f.tokenBody = `{"access_token":"a","refresh_token":"r","token_type":"bearer","expires_in":3600,"scope":"read:user"}` + c := newTestClient(t, f.server.URL) + + tok, err := c.ExchangeCode(t.Context(), "the-code", "the-verifier", "https://broker.example.com/oauth/callback") + if err != nil { + t.Fatalf("ExchangeCode: %v", err) + } + if tok.AccessToken != "a" || tok.RefreshToken != "r" || tok.ExpiresIn != 3600 { + t.Errorf("token mismatch: %+v", tok) + } + // Verify the form params Forgejo received. + expected := map[string]string{ + "grant_type": "authorization_code", + "code": "the-code", + "code_verifier": "the-verifier", + "redirect_uri": "https://broker.example.com/oauth/callback", + "client_id": "test-client-id", + "client_secret": "test-client-secret", + } + for k, v := range expected { + if got := f.lastForm.Get(k); got != v { + t.Errorf("form[%q] = %q, want %q", k, got, v) + } + } +} + +func TestExchangeCode_OAuthError(t *testing.T) { + f := newFakeForgejo(t) + f.tokenStatus = http.StatusBadRequest + f.tokenBody = `{"error":"invalid_grant","error_description":"code expired"}` + c := newTestClient(t, f.server.URL) + + _, err := c.ExchangeCode(t.Context(), "x", "v", "https://x/cb") + var e *forgejo.Error + if !errors.As(err, &e) { + t.Fatalf("want *forgejo.Error, got %T: %v", err, err) + } + if e.Code != "invalid_grant" { + t.Errorf("Code = %q, want invalid_grant", e.Code) + } + if e.HTTPStatus != http.StatusBadRequest { + t.Errorf("HTTPStatus = %d, want %d", e.HTTPStatus, http.StatusBadRequest) + } + if !strings.Contains(e.Error(), "expired") { + t.Errorf("Error string missing description: %v", e) + } +} + +func TestExchangeCode_OAuthErrorWithoutDescription(t *testing.T) { + f := newFakeForgejo(t) + f.tokenStatus = http.StatusBadRequest + f.tokenBody = `{"error":"invalid_request"}` + c := newTestClient(t, f.server.URL) + + _, err := c.ExchangeCode(t.Context(), "x", "v", "https://x/cb") + var e *forgejo.Error + if !errors.As(err, &e) || strings.Contains(e.Error(), ":") == false { + t.Errorf("expected formatted error, got %v", err) + } +} + +func TestExchangeCode_5xx_NoBody(t *testing.T) { + f := newFakeForgejo(t) + f.tokenStatus = http.StatusInternalServerError + f.tokenBody = "oops" + c := newTestClient(t, f.server.URL) + + _, err := c.ExchangeCode(t.Context(), "x", "v", "https://x/cb") + var e *forgejo.Error + if !errors.As(err, &e) { + t.Fatalf("want *forgejo.Error, got %v", err) + } + if e.HTTPStatus != http.StatusInternalServerError { + t.Errorf("HTTPStatus = %d", e.HTTPStatus) + } +} + +func TestExchangeCode_MalformedJSON(t *testing.T) { + f := newFakeForgejo(t) + f.tokenStatus = http.StatusOK + f.tokenBody = "not json" + c := newTestClient(t, f.server.URL) + + _, err := c.ExchangeCode(t.Context(), "x", "v", "https://x/cb") + if err == nil || !strings.Contains(err.Error(), "decode") { + t.Errorf("want decode error, got %v", err) + } +} + +func TestExchangeCode_MissingAccessToken(t *testing.T) { + f := newFakeForgejo(t) + f.tokenBody = `{"token_type":"bearer","expires_in":1}` + c := newTestClient(t, f.server.URL) + + _, err := c.ExchangeCode(t.Context(), "x", "v", "https://x/cb") + if err == nil || !strings.Contains(err.Error(), "access_token") { + t.Errorf("want missing access_token error, got %v", err) + } +} + +func TestExchangeCode_NetworkError(t *testing.T) { + f := newFakeForgejo(t) + c := newTestClient(t, f.server.URL) + f.server.Close() // force network error on next call + + _, err := c.ExchangeCode(t.Context(), "x", "v", "https://x/cb") + if err == nil { + t.Fatal("expected network error") + } + // Should not be a structured *forgejo.Error — those are for upstream + // rejections, not transport failures. + var e *forgejo.Error + if errors.As(err, &e) { + t.Errorf("network errors must not surface as *forgejo.Error: %v", err) + } +} + +func TestRefresh_Success(t *testing.T) { + f := newFakeForgejo(t) + f.tokenBody = `{"access_token":"a2","refresh_token":"r2","token_type":"bearer","expires_in":3600}` + c := newTestClient(t, f.server.URL) + + tok, err := c.Refresh(t.Context(), "old-refresh-token") + if err != nil { + t.Fatalf("Refresh: %v", err) + } + if tok.AccessToken != "a2" || tok.RefreshToken != "r2" { + t.Errorf("token mismatch: %+v", tok) + } + if f.lastForm.Get("grant_type") != "refresh_token" { + t.Errorf("grant_type = %q, want refresh_token", f.lastForm.Get("grant_type")) + } + if f.lastForm.Get("refresh_token") != "old-refresh-token" { + t.Errorf("refresh_token form param = %q", f.lastForm.Get("refresh_token")) + } +} + +func TestRefresh_InvalidGrant(t *testing.T) { + f := newFakeForgejo(t) + f.tokenStatus = http.StatusBadRequest + f.tokenBody = `{"error":"invalid_grant","error_description":"refresh token revoked"}` + c := newTestClient(t, f.server.URL) + + _, err := c.Refresh(t.Context(), "stale") + var e *forgejo.Error + if !errors.As(err, &e) || e.Code != "invalid_grant" { + t.Errorf("want invalid_grant, got %v", err) + } +} + +func TestFetchUserInfo_Success(t *testing.T) { + f := newFakeForgejo(t) + f.userBody = `{"sub":"42","preferred_username":"alice","name":"Alice Bee","email":"alice@example.com"}` + c := newTestClient(t, f.server.URL) + + ui, err := c.FetchUserInfo(t.Context(), "the-access-token") + if err != nil { + t.Fatalf("FetchUserInfo: %v", err) + } + if ui.Sub != "42" || ui.PreferredUsername != "alice" { + t.Errorf("user mismatch: %+v", ui) + } + if f.lastAuth != "Bearer the-access-token" { + t.Errorf("Authorization header = %q", f.lastAuth) + } +} + +func TestFetchUserInfo_OAuthError(t *testing.T) { + f := newFakeForgejo(t) + f.userStatus = http.StatusUnauthorized + f.userBody = `{"error":"invalid_token","error_description":"expired"}` + c := newTestClient(t, f.server.URL) + + _, err := c.FetchUserInfo(t.Context(), "x") + var e *forgejo.Error + if !errors.As(err, &e) || e.Code != "invalid_token" { + t.Errorf("want invalid_token, got %v", err) + } +} + +func TestFetchUserInfo_NonOAuthError(t *testing.T) { + // Some Forgejo versions return {"message": "..."} for 401 instead of the + // RFC 6749 oauth-error shape. Verify we still return a structured error. + f := newFakeForgejo(t) + f.userStatus = http.StatusUnauthorized + f.userBody = `{"message":"unauthenticated"}` + c := newTestClient(t, f.server.URL) + + _, err := c.FetchUserInfo(t.Context(), "x") + var e *forgejo.Error + if !errors.As(err, &e) { + t.Fatalf("want *forgejo.Error, got %v", err) + } + if e.Code != "userinfo_failed" { + t.Errorf("Code = %q, want userinfo_failed", e.Code) + } + if e.HTTPStatus != http.StatusUnauthorized { + t.Errorf("HTTPStatus = %d", e.HTTPStatus) + } +} + +func TestFetchUserInfo_MissingSub(t *testing.T) { + f := newFakeForgejo(t) + f.userBody = `{"preferred_username":"alice"}` + c := newTestClient(t, f.server.URL) + + _, err := c.FetchUserInfo(t.Context(), "x") + if err == nil || !strings.Contains(err.Error(), "sub") { + t.Errorf("want missing-sub error, got %v", err) + } +} + +func TestFetchUserInfo_MalformedJSON(t *testing.T) { + f := newFakeForgejo(t) + f.userBody = "not json" + c := newTestClient(t, f.server.URL) + + _, err := c.FetchUserInfo(t.Context(), "x") + if err == nil || !strings.Contains(err.Error(), "decode") { + t.Errorf("want decode error, got %v", err) + } +}