From 1265c9fe2ab3c082b74a1e2af3af58589644603d Mon Sep 17 00:00:00 2001 From: Rob Zehner Date: Sun, 5 Jul 2026 02:40:50 -0700 Subject: [PATCH 1/2] fix(cli): implement PKCE for secretless OAuth authorization-code login MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generated auth login sent a bare authorization_code grant when no client secret was configured — no client authentication and no code_verifier — which token endpoints reject. Every printed CLI with authorization-code OAuth inherited the broken CLIENT_ID-only path. When no secret is set, the emitted flow now generates a per-attempt RFC 7636 verifier, sends an S256 code_challenge on the authorize request, and exchanges with code_verifier in place of client_secret. Secret-based flows are unchanged. The loopback redirect moves from localhost to 127.0.0.1 per RFC 8252 §7.3, and the token exchange gets its own 30s client instead of the shared default. Adds a generated-output test (compiled binary, verify-env URL assertions both ways, injected -race runtime tests incl. the RFC 7636 appendix B vector) and a golden case locking the authorization-code + PKCE shape, which previously had no golden coverage. Closes #3457. Refs mvanhorn/printing-press-library#1287. Co-Authored-By: Claude Fable 5 --- internal/generator/auth_oauth_pkce_test.go | 117 ++ internal/generator/templates/auth.go.tmpl | 51 +- .../artifacts.txt | 19 + .../command.txt | 2 + .../exit.txt | 1 + .../internal/cli/auth.go | 429 +++++++ .../internal/client/client.go | 1078 +++++++++++++++++ .../stderr.txt | 2 + .../stdout.txt | 0 .../fixtures/golden-api-oauth2-authcode.yaml | 47 + 10 files changed, 1742 insertions(+), 4 deletions(-) create mode 100644 internal/generator/auth_oauth_pkce_test.go create mode 100644 testdata/golden/cases/generate-golden-api-oauth2-authcode/artifacts.txt create mode 100644 testdata/golden/cases/generate-golden-api-oauth2-authcode/command.txt create mode 100644 testdata/golden/expected/generate-golden-api-oauth2-authcode/exit.txt create mode 100644 testdata/golden/expected/generate-golden-api-oauth2-authcode/printing-press-oauth2-authcode/internal/cli/auth.go create mode 100644 testdata/golden/expected/generate-golden-api-oauth2-authcode/printing-press-oauth2-authcode/internal/client/client.go create mode 100644 testdata/golden/expected/generate-golden-api-oauth2-authcode/stderr.txt create mode 100644 testdata/golden/expected/generate-golden-api-oauth2-authcode/stdout.txt create mode 100644 testdata/golden/fixtures/golden-api-oauth2-authcode.yaml diff --git a/internal/generator/auth_oauth_pkce_test.go b/internal/generator/auth_oauth_pkce_test.go new file mode 100644 index 000000000..10227797a --- /dev/null +++ b/internal/generator/auth_oauth_pkce_test.go @@ -0,0 +1,117 @@ +package generator + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/mvanhorn/cli-printing-press/v4/internal/spec" + "github.com/stretchr/testify/require" +) + +func TestOAuthLoginUsesPKCEWhenNoClientSecret(t *testing.T) { + t.Parallel() + + apiSpec := minimalSpec("oauth-pkce-login") + apiSpec.Auth = spec.AuthConfig{ + Type: "oauth2", + Header: "Authorization", + Format: "Bearer {token}", + OAuth2Grant: spec.OAuth2GrantAuthorizationCode, + AuthorizationURL: "https://accounts.example.com/oauth/authorize", + TokenURL: "https://accounts.example.com/oauth/token", + KeyURL: "https://console.example.com/oauth", + } + + outputDir := filepath.Join(t.TempDir(), "oauth-pkce-login-pp-cli") + require.NoError(t, New(apiSpec, outputDir).Generate()) + + authSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "auth.go")) + require.NoError(t, err) + auth := string(authSrc) + + // Authorize request carries an S256 challenge on the secretless path. + require.Contains(t, auth, "codeVerifier, err = generatePKCEVerifier()") + require.Contains(t, auth, `params.Set("code_challenge", pkceCodeChallengeS256(codeVerifier))`) + require.Contains(t, auth, `params.Set("code_challenge_method", "S256")`) + + // Token exchange authenticates with exactly one of client_secret or + // code_verifier; the bare authorization_code grant is gone. + require.Contains(t, auth, "tokenParams.Set(\"client_secret\", clientSecret)\n\t} else {\n\t\ttokenParams.Set(\"code_verifier\", codeVerifier)\n\t}") + + // RFC 8252 loopback redirect: IP literal, never "localhost". + require.Contains(t, auth, `redirectURI := fmt.Sprintf("http://127.0.0.1:%d/callback"`) + require.NotContains(t, auth, `http://localhost:%d/callback`) + + binPath := filepath.Join(outputDir, "oauth-pkce-login-pp-cli") + runGoCommand(t, outputDir, "build", "-o", binPath, "./cmd/oauth-pkce-login-pp-cli") + + // Under verify-env the command prints the authorize URL it would launch; + // use that to assert live flag→params behavior without opening a browser + // or binding assumptions about the callback port. + runVerifyLogin := func(extraArgs ...string) string { + args := append([]string{"--config", filepath.Join(t.TempDir(), "config.toml"), "login", "--client-id", "test-client"}, extraArgs...) + cmd := exec.Command(binPath, args...) + cmd.Env = append(os.Environ(), "PRINTING_PRESS_VERIFY=1") + out, err := cmd.CombinedOutput() + require.NoError(t, err, "verify-env login failed: %s", string(out)) + return string(out) + } + + secretless := runVerifyLogin() + require.Contains(t, secretless, "would launch:") + require.Contains(t, secretless, "code_challenge=") + require.Contains(t, secretless, "code_challenge_method=S256") + require.Contains(t, secretless, "127.0.0.1%3A8085%2Fcallback") + require.NotContains(t, secretless, "localhost") + + withSecret := runVerifyLogin("--client-secret", "test-secret") + require.Contains(t, withSecret, "would launch:") + require.NotContains(t, withSecret, "code_challenge") + + const runtimeTest = `package cli + +import ( + "strings" + "testing" +) + +// Vector from RFC 7636 appendix B. +func TestPKCECodeChallengeS256MatchesRFC7636Vector(t *testing.T) { + const verifier = "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk" + const want = "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM" + if got := pkceCodeChallengeS256(verifier); got != want { + t.Fatalf("pkceCodeChallengeS256() = %q, want %q", got, want) + } +} + +func TestGeneratePKCEVerifierShapeAndUniqueness(t *testing.T) { + const unreserved = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~" + first, err := generatePKCEVerifier() + if err != nil { + t.Fatalf("generatePKCEVerifier() error = %v", err) + } + if len(first) != 43 { + t.Fatalf("verifier length = %d, want 43", len(first)) + } + for _, r := range first { + if !strings.ContainsRune(unreserved, r) { + t.Fatalf("verifier contains %q, outside RFC 7636 unreserved set", r) + } + } + second, err := generatePKCEVerifier() + if err != nil { + t.Fatalf("generatePKCEVerifier() second call error = %v", err) + } + if first == second { + t.Fatalf("two verifiers are identical: %q", first) + } +} +` + require.NoError(t, os.WriteFile(filepath.Join(outputDir, "internal", "cli", "oauth_pkce_test.go"), []byte(runtimeTest), 0o644)) + runGoCommand(t, outputDir, "test", "-race", "./internal/cli", "-run", "Test(PKCECodeChallengeS256|GeneratePKCEVerifier)") + + require.False(t, strings.Contains(auth, "http.PostForm(tokenURL"), "token exchange should use a client with its own timeout, not the shared default client") +} diff --git a/internal/generator/templates/auth.go.tmpl b/internal/generator/templates/auth.go.tmpl index ceb6d1f23..b490ad469 100644 --- a/internal/generator/templates/auth.go.tmpl +++ b/internal/generator/templates/auth.go.tmpl @@ -7,6 +7,8 @@ import ( "bufio" "context" "crypto/rand" + "crypto/sha256" + "encoding/base64" "encoding/hex" "encoding/json" "fmt" @@ -153,6 +155,18 @@ func runOAuthLogin(cmd *cobra.Command, flags *rootFlags, clientID, clientSecret } state := hex.EncodeToString(stateBytes) + // RFC 7636 PKCE: with no client secret this is a public client, and a + // bare authorization_code grant reaches the token endpoint with no + // client authentication at all — providers reject that. When a secret + // is configured, keep the plain Authorization Code flow unchanged. + codeVerifier := "" + if clientSecret == "" { + codeVerifier, err = generatePKCEVerifier() + if err != nil { + return err + } + } + authURL := "" {{- if .Auth.AuthorizationURL}} authURL = cfg.AuthorizationURL @@ -166,6 +180,10 @@ func runOAuthLogin(cmd *cobra.Command, flags *rootFlags, clientID, clientSecret "response_type": {"code"}, "state": {state}, } + if codeVerifier != "" { + params.Set("code_challenge", pkceCodeChallengeS256(codeVerifier)) + params.Set("code_challenge_method", "S256") + } {{- if eq $refreshMech.Kind "query"}} params.Set({{printf "%q" $refreshMech.Key}}, {{printf "%q" $refreshMech.Value}}) {{- end}} @@ -184,7 +202,7 @@ func runOAuthLogin(cmd *cobra.Command, flags *rootFlags, clientID, clientSecret // derived from the configured --port; the live flow below uses the actual // bound port, which the OS assigns when --port 0 is passed. if cliutil.IsVerifyEnv() { - params.Set("redirect_uri", fmt.Sprintf("http://localhost:%d/callback", port)) + params.Set("redirect_uri", fmt.Sprintf("http://127.0.0.1:%d/callback", port)) fmt.Fprintf(w, "would launch: %s\n", authURL+"?"+params.Encode()) return nil } @@ -197,8 +215,10 @@ func runOAuthLogin(cmd *cobra.Command, flags *rootFlags, clientID, clientSecret // Derive the redirect URI from the live listener address so an OS-assigned // ephemeral port (--port 0) is reflected in both the auth request and the - // token exchange below. - redirectURI := fmt.Sprintf("http://localhost:%d/callback", listener.Addr().(*net.TCPAddr).Port) + // token exchange below. Use 127.0.0.1, not "localhost": RFC 8252 §7.3 + // prescribes the loopback IP literal, and some providers reject + // "localhost" redirect URIs outright. + redirectURI := fmt.Sprintf("http://127.0.0.1:%d/callback", listener.Addr().(*net.TCPAddr).Port) params.Set("redirect_uri", redirectURI) fullURL := authURL + "?" + params.Encode() @@ -261,9 +281,14 @@ func runOAuthLogin(cmd *cobra.Command, flags *rootFlags, clientID, clientSecret } if clientSecret != "" { tokenParams.Set("client_secret", clientSecret) + } else { + tokenParams.Set("code_verifier", codeVerifier) } - resp, err := http.PostForm(tokenURL, tokenParams) + // The browser-wait timeout above does not apply here; the token exchange + // is a plain server-to-server POST and gets its own short network timeout. + tokenClient := &http.Client{Timeout: 30 * time.Second} + resp, err := tokenClient.PostForm(tokenURL, tokenParams) if err != nil { return fmt.Errorf("exchanging code for token: %w", err) } @@ -446,6 +471,24 @@ func newAuthLogoutCmd(flags *rootFlags) *cobra.Command { } } +// generatePKCEVerifier returns a fresh RFC 7636 code verifier: 32 bytes of +// CSPRNG output, base64url-encoded without padding (43 chars, within the +// 43-128 range the RFC requires). +func generatePKCEVerifier() (string, error) { + verifierBytes := make([]byte, 32) + if _, err := rand.Read(verifierBytes); err != nil { + return "", fmt.Errorf("generating PKCE verifier: %w", err) + } + return base64.RawURLEncoding.EncodeToString(verifierBytes), nil +} + +// pkceCodeChallengeS256 derives the S256 code challenge for a verifier: +// base64url(SHA-256(verifier)) without padding. +func pkceCodeChallengeS256(verifier string) string { + sum := sha256.Sum256([]byte(verifier)) + return base64.RawURLEncoding.EncodeToString(sum[:]) +} + func openBrowser(url string) { var cmd *exec.Cmd switch runtime.GOOS { diff --git a/testdata/golden/cases/generate-golden-api-oauth2-authcode/artifacts.txt b/testdata/golden/cases/generate-golden-api-oauth2-authcode/artifacts.txt new file mode 100644 index 000000000..28defecc5 --- /dev/null +++ b/testdata/golden/cases/generate-golden-api-oauth2-authcode/artifacts.txt @@ -0,0 +1,19 @@ +# Locks the OAuth2 authorization_code + PKCE auth shape contract: +# +# parser.go → AuthConfig.OAuth2Grant=authorization_code, +# AuthorizationURL + TokenURL set +# generator.go → auth.go.tmpl selected (browser loopback login) +# auth.go.tmpl → RFC 8252 loopback redirect (127.0.0.1, not localhost), +# PKCE on the secretless path: generatePKCEVerifier / +# pkceCodeChallengeS256 / code_challenge_method=S256 on +# the authorize request, and exactly one of client_secret +# or code_verifier on the token exchange +# +# If a refactor regresses template selection or drops the PKCE branch, +# auth.go either disappears or loses the PKCE helpers and the harness +# fails with a content diff. +printing-press-oauth2-authcode/internal/cli/auth.go + +# client.go carries refreshAccessToken, which must keep the public-client +# refresh shape (client_id always sent, client_secret only when present). +printing-press-oauth2-authcode/internal/client/client.go diff --git a/testdata/golden/cases/generate-golden-api-oauth2-authcode/command.txt b/testdata/golden/cases/generate-golden-api-oauth2-authcode/command.txt new file mode 100644 index 000000000..071aa5d2a --- /dev/null +++ b/testdata/golden/cases/generate-golden-api-oauth2-authcode/command.txt @@ -0,0 +1,2 @@ +set -euo pipefail +"$BINARY" generate --spec testdata/golden/fixtures/golden-api-oauth2-authcode.yaml --output "$CASE_ACTUAL_DIR/printing-press-oauth2-authcode" --force --spec-url file://testdata/golden/fixtures/golden-api-oauth2-authcode.yaml --validate=false diff --git a/testdata/golden/expected/generate-golden-api-oauth2-authcode/exit.txt b/testdata/golden/expected/generate-golden-api-oauth2-authcode/exit.txt new file mode 100644 index 000000000..573541ac9 --- /dev/null +++ b/testdata/golden/expected/generate-golden-api-oauth2-authcode/exit.txt @@ -0,0 +1 @@ +0 diff --git a/testdata/golden/expected/generate-golden-api-oauth2-authcode/printing-press-oauth2-authcode/internal/cli/auth.go b/testdata/golden/expected/generate-golden-api-oauth2-authcode/printing-press-oauth2-authcode/internal/cli/auth.go new file mode 100644 index 000000000..d082c2a92 --- /dev/null +++ b/testdata/golden/expected/generate-golden-api-oauth2-authcode/printing-press-oauth2-authcode/internal/cli/auth.go @@ -0,0 +1,429 @@ +// Copyright 2026 printing-press-golden and contributors. Licensed under Apache-2.0. See LICENSE. +// Generated by CLI Printing Press (https://github.com/mvanhorn/cli-printing-press). DO NOT EDIT. + +package cli + +import ( + "bufio" + "context" + "crypto/rand" + "crypto/sha256" + "encoding/base64" + "encoding/hex" + "encoding/json" + "fmt" + "io" + "net" + "net/http" + "net/url" + "os" + "os/exec" + "runtime" + "strings" + "time" + + "github.com/spf13/cobra" + "printing-press-oauth2-pp-cli/internal/cliutil" + "printing-press-oauth2-pp-cli/internal/config" +) + +func newAuthCmd(flags *rootFlags) *cobra.Command { + cmd := &cobra.Command{ + Use: "auth", + Short: "Manage authentication for Printing Press Oauth2", + RunE: parentNoSubcommandRunE(flags), + } + + cmd.AddCommand(newAuthSetupCmd(flags)) + cmd.AddCommand(newAuthLoginCmd(flags)) + cmd.AddCommand(newAuthStatusCmd(flags)) + cmd.AddCommand(newAuthLogoutCmd(flags)) + + return cmd +} + +// newAuthSetupCmd prints concrete steps for registering an OAuth app and +// obtaining client credentials. Side-effect rule: print by default, --launch +// to open the registration URL, short-circuit when the verifier is running. +func newAuthSetupCmd(_ *rootFlags) *cobra.Command { + var launch bool + cmd := &cobra.Command{ + Use: "setup", + Short: "Print steps for registering an OAuth app (use --launch to open the URL)", + Example: " printing-press-oauth2-pp-cli auth setup\n printing-press-oauth2-pp-cli auth setup --launch", + RunE: func(cmd *cobra.Command, args []string) error { + w := cmd.OutOrStdout() + fmt.Fprintln(w, "No setup URL is configured for this CLI; check the API's docs.") + fmt.Fprintln(w, "") + fmt.Fprintln(w, "Then run:") + fmt.Fprintln(w, " printing-press-oauth2-pp-cli login") + if !launch { + return nil + } + fmt.Fprintln(cmd.ErrOrStderr(), "no setup URL configured; cannot launch") + return nil + }, + } + cmd.Flags().BoolVar(&launch, "launch", false, "Open the setup URL in your default browser") + return cmd +} + +func newAuthLoginCmd(flags *rootFlags) *cobra.Command { + var clientID string + var clientSecret string + var port int + + cmd := &cobra.Command{ + Use: "login", + Short: "Authenticate via OAuth2", + Annotations: map[string]string{ + "mcp:hidden": "true", + }, + RunE: func(cmd *cobra.Command, args []string) error { + return runOAuthLogin(cmd, flags, clientID, clientSecret, port) + }, + } + + cmd.Flags().StringVar(&clientID, "client-id", os.Getenv("PRINTING_PRESS_OAUTH2_CLIENT_ID"), "OAuth2 client ID") + cmd.Flags().StringVar(&clientSecret, "client-secret", os.Getenv("PRINTING_PRESS_OAUTH2_CLIENT_SECRET"), "OAuth2 client secret") + cmd.Flags().IntVar(&port, "port", 8085, "Local callback server port") + + return cmd +} + +func runOAuthLogin(cmd *cobra.Command, flags *rootFlags, clientID, clientSecret string, port int) error { + w := cmd.OutOrStdout() + cfg, err := config.Load(flags.configPath) + if err != nil { + return err + } + clientID, clientSecret, err = resolveOAuthCredentials(cmd, flags, cfg, clientID, clientSecret) + if err != nil { + return err + } + + stateBytes := make([]byte, 16) + if _, err := rand.Read(stateBytes); err != nil { + return fmt.Errorf("generating state: %w", err) + } + state := hex.EncodeToString(stateBytes) + + // RFC 7636 PKCE: with no client secret this is a public client, and a + // bare authorization_code grant reaches the token endpoint with no + // client authentication at all — providers reject that. When a secret + // is configured, keep the plain Authorization Code flow unchanged. + codeVerifier := "" + if clientSecret == "" { + codeVerifier, err = generatePKCEVerifier() + if err != nil { + return err + } + } + + authURL := "" + authURL = cfg.AuthorizationURL + if authURL == "" { + authURL = "https://accounts.authcode.example/oauth/authorize" + } + params := url.Values{ + "client_id": {clientID}, + "response_type": {"code"}, + "state": {state}, + } + if codeVerifier != "" { + params.Set("code_challenge", pkceCodeChallengeS256(codeVerifier)) + params.Set("code_challenge_method", "S256") + } + scopes := []string{"read", "write"} + if len(scopes) > 0 { + params.Set("scope", strings.Join(scopes, " ")) + } + + // Short-circuit BEFORE binding the callback port or opening a browser, so + // verify / validate-narrative neither binds 127.0.0.1: (which would + // EADDRINUSE on a parallel run or occupied port) nor launches a browser + // (which would time out). The redirect_uri shown here is informational and + // derived from the configured --port; the live flow below uses the actual + // bound port, which the OS assigns when --port 0 is passed. + if cliutil.IsVerifyEnv() { + params.Set("redirect_uri", fmt.Sprintf("http://127.0.0.1:%d/callback", port)) + fmt.Fprintf(w, "would launch: %s\n", authURL+"?"+params.Encode()) + return nil + } + + listener, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) + if err != nil { + return fmt.Errorf("starting callback server: %w", err) + } + defer listener.Close() + + // Derive the redirect URI from the live listener address so an OS-assigned + // ephemeral port (--port 0) is reflected in both the auth request and the + // token exchange below. Use 127.0.0.1, not "localhost": RFC 8252 §7.3 + // prescribes the loopback IP literal, and some providers reject + // "localhost" redirect URIs outright. + redirectURI := fmt.Sprintf("http://127.0.0.1:%d/callback", listener.Addr().(*net.TCPAddr).Port) + params.Set("redirect_uri", redirectURI) + fullURL := authURL + "?" + params.Encode() + + fmt.Fprintf(os.Stderr, "Opening browser for authentication...\n") + fmt.Fprintf(os.Stderr, "If the browser doesn't open, visit:\n%s\n\n", fullURL) + openBrowser(fullURL) + + codeCh := make(chan string, 1) + errCh := make(chan error, 1) + + mux := http.NewServeMux() + mux.HandleFunc("/callback", func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("state") != state { + errCh <- fmt.Errorf("state mismatch") + http.Error(w, "State mismatch", http.StatusBadRequest) + return + } + if errMsg := r.URL.Query().Get("error"); errMsg != "" { + errCh <- fmt.Errorf("auth error: %s", errMsg) + http.Error(w, errMsg, http.StatusBadRequest) + return + } + code := r.URL.Query().Get("code") + if code == "" { + errCh <- fmt.Errorf("no code in callback") + http.Error(w, "No code", http.StatusBadRequest) + return + } + w.Header().Set("Content-Type", "text/html") + fmt.Fprint(w, "

Authentication successful!

You can close this tab.

") + codeCh <- code + }) + + server := &http.Server{Handler: mux} + go server.Serve(listener) + + var code string + select { + case code = <-codeCh: + case err := <-errCh: + return err + case <-time.After(2 * time.Minute): + return fmt.Errorf("authentication timed out after 2 minutes") + } + + server.Shutdown(context.Background()) + + tokenURL := "" + tokenURL = cfg.TokenURL + if tokenURL == "" { + tokenURL = "https://accounts.authcode.example/oauth/token" + } + tokenParams := url.Values{ + "grant_type": {"authorization_code"}, + "code": {code}, + "redirect_uri": {redirectURI}, + "client_id": {clientID}, + } + if clientSecret != "" { + tokenParams.Set("client_secret", clientSecret) + } else { + tokenParams.Set("code_verifier", codeVerifier) + } + + // The browser-wait timeout above does not apply here; the token exchange + // is a plain server-to-server POST and gets its own short network timeout. + tokenClient := &http.Client{Timeout: 30 * time.Second} + resp, err := tokenClient.PostForm(tokenURL, tokenParams) + if err != nil { + return fmt.Errorf("exchanging code for token: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode >= 400 { + var body map[string]any + if err := json.NewDecoder(resp.Body).Decode(&body); err == nil { + return fmt.Errorf("exchanging code for token: HTTP %d: %v", resp.StatusCode, body) + } + return fmt.Errorf("exchanging code for token: HTTP %d", resp.StatusCode) + } + + var tokenResp struct { + AccessToken string `json:"access_token"` + RefreshToken string `json:"refresh_token"` + ExpiresIn int `json:"expires_in"` + TokenType string `json:"token_type"` + } + if err := json.NewDecoder(resp.Body).Decode(&tokenResp); err != nil { + return fmt.Errorf("parsing token response: %w", err) + } + if tokenResp.AccessToken == "" { + return fmt.Errorf("no access token in response") + } + + // Guard against non-conformant servers that return expires_in: 0. + // A zero-duration expiry resolves to time.Now(), which authHeader() + // then treats as expired and triggers a refresh on every call. + expiry := time.Time{} + if tokenResp.ExpiresIn > 0 { + expiry = time.Now().Add(time.Duration(tokenResp.ExpiresIn) * time.Second) + } + if err := cfg.SaveTokens(clientID, clientSecret, tokenResp.AccessToken, tokenResp.RefreshToken, expiry); err != nil { + return fmt.Errorf("saving tokens: %w", err) + } + + if expiry.IsZero() { + fmt.Fprintf(os.Stderr, "%s Authentication successful! Token expiry not provided.\n", green("OK")) + } else { + fmt.Fprintf(os.Stderr, "%s Authentication successful! Token expires at %s\n", green("OK"), expiry.Format(time.RFC3339)) + } + return nil +} + +func resolveOAuthCredentials(cmd *cobra.Command, flags *rootFlags, cfg *config.Config, clientID, clientSecret string) (string, string, error) { + explicitClientID := clientID + if clientID == "" && cfg != nil { + clientID = cfg.ClientID + } + if clientSecret == "" && cfg != nil && (explicitClientID == "" || explicitClientID == cfg.ClientID) { + clientSecret = cfg.ClientSecret + } + if clientID != "" { + return clientID, clientSecret, nil + } + if flags.noInput { + return "", "", fmt.Errorf("OAuth2 client ID is required; pass --client-id, set PRINTING_PRESS_OAUTH2_CLIENT_ID, or run 'printing-press-oauth2-pp-cli login' without --no-input") + } + reader := bufio.NewReader(cmd.InOrStdin()) + printOAuthCredentialHint(cmd.ErrOrStderr()) + clientID, err := promptOAuthCredential(cmd, reader, "OAuth2 Client ID") + if err != nil { + return "", "", err + } + if clientID == "" { + return "", "", fmt.Errorf("OAuth2 client ID is required") + } + if clientSecret == "" { + clientSecret, err = promptOAuthCredential(cmd, reader, "OAuth2 Client Secret (press Enter if not required)") + if err != nil { + return "", "", err + } + } + return clientID, clientSecret, nil +} + +func printOAuthCredentialHint(w io.Writer) { + fmt.Fprintln(w, "No setup URL is configured for this CLI; check the API's docs.") +} + +func promptOAuthCredential(cmd *cobra.Command, reader *bufio.Reader, label string) (string, error) { + fmt.Fprintf(cmd.ErrOrStderr(), "%s: ", label) + line, err := reader.ReadString('\n') + if err != nil && err != io.EOF { + return "", fmt.Errorf("reading %s: %w", strings.ToLower(label), err) + } + return strings.TrimSpace(line), nil +} + +func newAuthStatusCmd(flags *rootFlags) *cobra.Command { + return &cobra.Command{ + Use: "status", + Short: "Show authentication status", + RunE: func(cmd *cobra.Command, args []string) error { + cfg, err := config.Load(flags.configPath) + if err != nil { + return err + } + + w := cmd.OutOrStdout() + authed := cfg.AccessToken != "" + // JSON envelope: {authenticated, verified, source, config}. + if flags.asJSON { + out := map[string]any{ + "authenticated": authed, + "verified": false, + "source": cfg.AuthSource, + "config": cfg.Path, + } + return printJSONFiltered(w, out, flags) + } + + if !authed { + fmt.Fprintf(w, " %s Not authenticated. Run 'printing-press-oauth2-pp-cli login' to authenticate.\n", red("FAIL")) + return nil + } + + if cfg.TokenExpiry.IsZero() { + fmt.Fprintf(w, " %s Credentials present (not verified; no expiry info)\n", green("OK")) + } else if time.Now().Before(cfg.TokenExpiry) { + fmt.Fprintf(w, " %s Credentials present (not verified; expires %s)\n", green("OK"), cfg.TokenExpiry.Format(time.RFC3339)) + } else { + if cfg.RefreshToken != "" { + fmt.Fprintf(w, " %s Token expired (will auto-refresh on next request)\n", yellow("WARN")) + } else { + fmt.Fprintf(w, " %s Token expired. Run 'printing-press-oauth2-pp-cli login' to re-authenticate.\n", red("FAIL")) + } + } + + if cfg.AuthSource != "" { + fmt.Fprintf(w, " source: %s\n", cfg.AuthSource) + } + return nil + }, + } +} + +func newAuthLogoutCmd(flags *rootFlags) *cobra.Command { + return &cobra.Command{ + Use: "logout", + Short: "Remove stored authentication tokens", + RunE: func(cmd *cobra.Command, args []string) error { + cfg, err := config.Load(flags.configPath) + if err != nil { + return err + } + if err := cfg.ClearTokens(); err != nil { + return fmt.Errorf("clearing tokens: %w", err) + } + // JSON envelope: {cleared: true}. The OAuth flavor does not + // surface a "note" key because env-var creds aren't part of + // this auth flow's contract. + if flags.asJSON { + return printJSONFiltered(cmd.OutOrStdout(), map[string]any{ + "cleared": true, + }, flags) + } + fmt.Fprintf(os.Stderr, "Logged out. Tokens removed.\n") + return nil + }, + } +} + +// generatePKCEVerifier returns a fresh RFC 7636 code verifier: 32 bytes of +// CSPRNG output, base64url-encoded without padding (43 chars, within the +// 43-128 range the RFC requires). +func generatePKCEVerifier() (string, error) { + verifierBytes := make([]byte, 32) + if _, err := rand.Read(verifierBytes); err != nil { + return "", fmt.Errorf("generating PKCE verifier: %w", err) + } + return base64.RawURLEncoding.EncodeToString(verifierBytes), nil +} + +// pkceCodeChallengeS256 derives the S256 code challenge for a verifier: +// base64url(SHA-256(verifier)) without padding. +func pkceCodeChallengeS256(verifier string) string { + sum := sha256.Sum256([]byte(verifier)) + return base64.RawURLEncoding.EncodeToString(sum[:]) +} + +func openBrowser(url string) { + var cmd *exec.Cmd + switch runtime.GOOS { + case "darwin": + cmd = exec.Command("open", url) + case "linux": + cmd = exec.Command("xdg-open", url) + case "windows": + cmd = exec.Command("rundll32", "url.dll,FileProtocolHandler", url) + default: + return + } + cmd.Start() +} diff --git a/testdata/golden/expected/generate-golden-api-oauth2-authcode/printing-press-oauth2-authcode/internal/client/client.go b/testdata/golden/expected/generate-golden-api-oauth2-authcode/printing-press-oauth2-authcode/internal/client/client.go new file mode 100644 index 000000000..c81e9cde1 --- /dev/null +++ b/testdata/golden/expected/generate-golden-api-oauth2-authcode/printing-press-oauth2-authcode/internal/client/client.go @@ -0,0 +1,1078 @@ +// Copyright 2026 printing-press-golden and contributors. Licensed under Apache-2.0. See LICENSE. +// Generated by CLI Printing Press (https://github.com/mvanhorn/cli-printing-press). DO NOT EDIT. + +package client + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/base64" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "io" + "math" + "net/http" + "net/url" + "os" + "path/filepath" + "printing-press-oauth2-pp-cli/internal/cliutil" + "printing-press-oauth2-pp-cli/internal/config" + "sort" + "strings" + "time" +) + +const BinaryResponseHeader = "X-Printing-Press-Binary-Response" + +var ErrPlaceholderCredential = errors.New("auth placeholder credential") + +type Client struct { + BaseURL string + Config *config.Config + HTTPClient *http.Client + DryRun bool + NoCache bool + cacheDir string + limiter *cliutil.AdaptiveLimiter +} + +// RequestBaseURL returns the base URL used for requests. +// Novel commands that build request URLs by hand should use this instead of +// concatenating c.BaseURL directly. +func (c *Client) RequestBaseURL() string { + return c.BaseURL +} + +// APIError carries HTTP status information for structured exit codes. +type APIError struct { + Method string + Path string + StatusCode int + Body string +} + +func (e *APIError) Error() string { + return fmt.Sprintf("%s %s returned HTTP %d: %s", e.Method, e.Path, e.StatusCode, e.Body) +} + +func rejectUnresolvedPathParams(path string, allowedTemplateVars map[string]string) error { + for { + start := strings.IndexByte(path, '{') + if start < 0 { + return nil + } + rest := path[start+1:] + end := strings.IndexByte(rest, '}') + if end < 0 { + return nil + } + name := rest[:end] + if _, ok := allowedTemplateVars[name]; ok { + path = rest[end+1:] + continue + } + if isPathPlaceholderName(name) { + return fmt.Errorf("unresolved path parameter {%s}", name) + } + path = rest[end+1:] + } +} + +func isPathPlaceholderName(name string) bool { + if name == "" { + return false + } + for i, r := range name { + if r == '_' || r >= 'A' && r <= 'Z' || r >= 'a' && r <= 'z' || i > 0 && r >= '0' && r <= '9' { + continue + } + return false + } + return true +} + +func newHTTPClient(timeout time.Duration, jar http.CookieJar) *http.Client { + return &http.Client{Timeout: timeout, Jar: jar} +} + +func New(cfg *config.Config, timeout time.Duration, rateLimit float64) *Client { + cacheDir := "" + if dir, err := cliutil.CacheDir(); err == nil { + cacheDir = filepath.Join(dir, "http") + } + httpClient := newHTTPClient(timeout, nil) + c := &Client{ + BaseURL: strings.TrimRight(cfg.BaseURL, "/"), + Config: cfg, + HTTPClient: httpClient, + cacheDir: cacheDir, + limiter: cliutil.NewAdaptiveLimiter(rateLimit), + } + // CheckRedirect re-derives auth on each hop. Go's default replays the + // original Authorization header verbatim, which breaks nonce-bound + // schemes (OAuth 1.0a PLAINTEXT, SigV4, Hawk): the duplicate nonce + // trips the server's replay detector with a 401. c.authHeader() + // returns a fresh value for those schemes and the same static value + // for Bearer/api_key, so post-redirect headers are byte-identical for + // static auth and freshly-signed for nonce-bound auth. + httpClient.CheckRedirect = func(req *http.Request, via []*http.Request) error { + if len(via) >= 10 { + // Match Go's defaultCheckRedirect: a plain error so Client.Do + // returns it through do()'s err != nil branch. ErrUseLastResponse + // would cause Do to return the 3xx with nil error, which do() + // would then classify as a successful response and hand the HTML + // "Moved Permanently" body back to the caller. + return errors.New("stopped after 10 redirects") + } + // Same-host gate mirrors Go's shouldCopyHeaderOnRedirect: a + // cross-domain 3xx (open redirect or partner handoff) must not + // receive the auth credential, even though we are inside + // CheckRedirect where Go's automatic stripping has already run. + if req.URL.Host == via[0].URL.Host { + if h, err := c.authHeader(req.Context()); err == nil && h != "" { + req.Header.Set("Authorization", h) + } + } else { + // Cross-host hop: Go strips standard auth headers (Authorization, + // Cookie) but not custom ones, so a custom API-key header would be + // forwarded verbatim to the redirect target. Delete it explicitly. + req.Header.Del("Authorization") + } + return nil + } + return c +} + +// RateLimit returns the current effective rate limit in req/s. Returns 0 if disabled. +func (c *Client) RateLimit() float64 { + return c.limiter.Rate() +} + +func (c *Client) Get(ctx context.Context, path string, params map[string]string) (json.RawMessage, error) { + return c.GetWithHeaders(ctx, path, params, nil) +} + +func (c *Client) GetWithHeaders(ctx context.Context, path string, params map[string]string, headers map[string]string) (json.RawMessage, error) { + if err := c.validateCachedRequestAuth(ctx); err != nil { + return nil, err + } + binaryResponse := c.wantsBinaryResponse(headers) + cacheEnabled := c.responseCacheEnabled(binaryResponse) + // Check cache for GET requests + if cacheEnabled { + if cached, ok := c.readCache(path, params); ok { + return cached, nil + } + } + result, _, err := c.do(ctx, "GET", path, params, nil, headers) + if err == nil && cacheEnabled { + c.writeCache(path, params, result) + } + return result, err +} + +func (c *Client) GetWithHeadersValues(ctx context.Context, path string, params url.Values, headers map[string]string) (json.RawMessage, error) { + return c.GetWithHeaders(ctx, pathWithQueryValues(path, params), nil, headers) +} + +// GetNoCache issues a GET that bypasses the cache read for this call only, +// then refreshes the cache with the fresh response on success. Use for +// polling-until-terminal patterns where every call must reflect current +// server state; the same (path, params) pair returning a stale +// "in-progress" snapshot from cache would lock the poll loop on the +// initial response. Writing-back on success means subsequent c.Get calls +// (e.g. a follow-up `... get ` after WaitForJob returns) see the +// terminal value, not the stale non-terminal snapshot left behind by the +// first poll. +func (c *Client) GetNoCache(ctx context.Context, path string, params map[string]string) (json.RawMessage, error) { + return c.GetWithHeadersNoCache(ctx, path, params, nil) +} + +// GetWithHeadersNoCache is GetWithHeaders that skips the cache read but still +// writes cacheable fresh responses on success. See GetNoCache for when to +// prefer this over Get/GetWithHeaders. +func (c *Client) GetWithHeadersNoCache(ctx context.Context, path string, params map[string]string, headers map[string]string) (json.RawMessage, error) { + binaryResponse := c.wantsBinaryResponse(headers) + result, _, err := c.do(ctx, "GET", path, params, nil, headers) + if err == nil && c.responseCacheEnabled(binaryResponse) { + c.writeCache(path, params, result) + } + return result, err +} + +func (c *Client) GetWithHeadersNoCacheValues(ctx context.Context, path string, params url.Values, headers map[string]string) (json.RawMessage, error) { + return c.GetWithHeadersNoCache(ctx, pathWithQueryValues(path, params), nil, headers) +} + +func (c *Client) responseCacheEnabled(binaryResponse bool) bool { + return !binaryResponse && !c.NoCache && !c.DryRun && c.cacheDir != "" +} + +// The response cache stores text/JSON bodies as .json. Binary callers +// opt out so opaque blobs do not land under a misleading extension. +func (c *Client) wantsBinaryResponse(headers map[string]string) bool { + binaryResponse := false + if c != nil && c.Config != nil { + if value, ok := binaryResponseHeaderValue(c.Config.Headers); ok { + binaryResponse = value + } + } + if value, ok := binaryResponseHeaderValue(headers); ok { + binaryResponse = value + } + return binaryResponse +} + +func binaryResponseHeaderValue(headers map[string]string) (bool, bool) { + found := false + for k, v := range headers { + if strings.EqualFold(k, BinaryResponseHeader) { + found = true + if strings.EqualFold(v, "true") { + return true, true + } + } + } + return false, found +} + +func (c *Client) validateCachedRequestAuth(ctx context.Context) error { + if c == nil || c.Config == nil { + return nil + } + if authHeaderLooksLikePlaceholderCredential(c.Config.AuthHeader()) { + return authPlaceholderCredentialError(c.Config) + } + return nil +} + +func (c *Client) ProbeGet(ctx context.Context, path string) (int, error) { + _, status, err := c.do(ctx, "GET", path, nil, nil, nil) + return status, err +} + +func (c *Client) cacheKey(path string, params map[string]string) string { + key := path + key += "|base_url=" + c.BaseURL + if c.Config != nil { + key += "|auth_source=" + c.Config.AuthSource + if authHeader := c.Config.AuthHeader(); authHeader != "" { + authHash := sha256.Sum256([]byte(authHeader)) + key += "|auth=" + hex.EncodeToString(authHash[:8]) + } + if c.Config.Path != "" { + key += "|config_path=" + c.Config.Path + } + } + if len(params) > 0 { + query := url.Values{} + for k, v := range params { + query.Set(k, v) + } + key += "|query=" + query.Encode() + } + h := sha256.Sum256([]byte(key)) + return hex.EncodeToString(h[:8]) +} + +func pathWithQueryValues(path string, params url.Values) string { + if len(params) == 0 { + return path + } + encoded := params.Encode() + if encoded == "" { + return path + } + separator := "?" + if strings.Contains(path, "?") { + separator = "&" + } + return path + separator + encoded +} + +func (c *Client) readCache(path string, params map[string]string) (json.RawMessage, bool) { + cacheFile := filepath.Join(c.cacheDir, c.cacheKey(path, params)+".json") + info, err := os.Stat(cacheFile) + if err != nil || time.Since(info.ModTime()) > 5*time.Minute { + return nil, false + } + data, err := os.ReadFile(cacheFile) + if err != nil { + return nil, false + } + return json.RawMessage(data), true +} + +func (c *Client) writeCache(path string, params map[string]string, data json.RawMessage) { + os.MkdirAll(c.cacheDir, 0o700) + cacheFile := filepath.Join(c.cacheDir, c.cacheKey(path, params)+".json") + os.WriteFile(cacheFile, []byte(data), 0o600) +} + +// invalidateCache wholesale-removes the cache directory so the next read +// after a mutation cannot return a stale snapshot. Selective per-resource +// invalidation rejected: cache keys are opaque sha256 hashes. +func (c *Client) invalidateCache() { + if c.cacheDir == "" { + return + } + _ = os.RemoveAll(c.cacheDir) +} + +func (c *Client) Post(ctx context.Context, path string, body any) (json.RawMessage, int, error) { + return c.do(ctx, "POST", path, nil, body, nil) +} + +func (c *Client) PostWithParams(ctx context.Context, path string, params map[string]string, body any) (json.RawMessage, int, error) { + return c.do(ctx, "POST", path, params, body, nil) +} + +func (c *Client) PostWithHeaders(ctx context.Context, path string, body any, headers map[string]string) (json.RawMessage, int, error) { + return c.do(ctx, "POST", path, nil, body, headers) +} + +func (c *Client) PostWithParamsAndHeaders(ctx context.Context, path string, params map[string]string, body any, headers map[string]string) (json.RawMessage, int, error) { + return c.do(ctx, "POST", path, params, body, headers) +} + +// PostQueryWithParams is a POST that does not mutate remote state — used +// by read-only operations that ride a mutating verb on the wire (GraphQL +// queries, JSON-RPC reads, POST-based search endpoints). The verify-mode +// short-circuit does not fire for these calls; the request reaches the +// real transport even under PRINTING_PRESS_VERIFY=1 without LIVE_HTTP=1. +func (c *Client) PostQueryWithParams(ctx context.Context, path string, params map[string]string, body any) (json.RawMessage, int, error) { + return c.doRead(ctx, "POST", path, params, body, nil) +} + +// PostQueryWithParamsAndHeaders is the headers-aware counterpart to +// PostQueryWithParams. See PostQueryWithParams for the verify-mode rationale. +func (c *Client) PostQueryWithParamsAndHeaders(ctx context.Context, path string, params map[string]string, body any, headers map[string]string) (json.RawMessage, int, error) { + return c.doRead(ctx, "POST", path, params, body, headers) +} + +func (c *Client) Delete(ctx context.Context, path string) (json.RawMessage, int, error) { + return c.do(ctx, "DELETE", path, nil, nil, nil) +} + +func (c *Client) DeleteWithParams(ctx context.Context, path string, params map[string]string) (json.RawMessage, int, error) { + return c.do(ctx, "DELETE", path, params, nil, nil) +} + +func (c *Client) DeleteWithBody(ctx context.Context, path string, body any) (json.RawMessage, int, error) { + return c.do(ctx, "DELETE", path, nil, body, nil) +} + +func (c *Client) DeleteWithParamsAndBody(ctx context.Context, path string, params map[string]string, body any) (json.RawMessage, int, error) { + return c.do(ctx, "DELETE", path, params, body, nil) +} + +func (c *Client) DeleteWithHeaders(ctx context.Context, path string, headers map[string]string) (json.RawMessage, int, error) { + return c.do(ctx, "DELETE", path, nil, nil, headers) +} + +func (c *Client) DeleteWithParamsAndHeaders(ctx context.Context, path string, params map[string]string, headers map[string]string) (json.RawMessage, int, error) { + return c.do(ctx, "DELETE", path, params, nil, headers) +} + +func (c *Client) DeleteWithBodyAndHeaders(ctx context.Context, path string, body any, headers map[string]string) (json.RawMessage, int, error) { + return c.do(ctx, "DELETE", path, nil, body, headers) +} + +func (c *Client) DeleteWithParamsAndBodyAndHeaders(ctx context.Context, path string, params map[string]string, body any, headers map[string]string) (json.RawMessage, int, error) { + return c.do(ctx, "DELETE", path, params, body, headers) +} + +func (c *Client) Put(ctx context.Context, path string, body any) (json.RawMessage, int, error) { + return c.do(ctx, "PUT", path, nil, body, nil) +} + +func (c *Client) PutWithParams(ctx context.Context, path string, params map[string]string, body any) (json.RawMessage, int, error) { + return c.do(ctx, "PUT", path, params, body, nil) +} + +func (c *Client) PutWithHeaders(ctx context.Context, path string, body any, headers map[string]string) (json.RawMessage, int, error) { + return c.do(ctx, "PUT", path, nil, body, headers) +} + +func (c *Client) PutWithParamsAndHeaders(ctx context.Context, path string, params map[string]string, body any, headers map[string]string) (json.RawMessage, int, error) { + return c.do(ctx, "PUT", path, params, body, headers) +} + +func (c *Client) Patch(ctx context.Context, path string, body any) (json.RawMessage, int, error) { + return c.do(ctx, "PATCH", path, nil, body, nil) +} + +func (c *Client) PatchWithParams(ctx context.Context, path string, params map[string]string, body any) (json.RawMessage, int, error) { + return c.do(ctx, "PATCH", path, params, body, nil) +} + +func (c *Client) PatchWithHeaders(ctx context.Context, path string, body any, headers map[string]string) (json.RawMessage, int, error) { + return c.do(ctx, "PATCH", path, nil, body, headers) +} + +func (c *Client) PatchWithParamsAndHeaders(ctx context.Context, path string, params map[string]string, body any, headers map[string]string) (json.RawMessage, int, error) { + return c.do(ctx, "PATCH", path, params, body, headers) +} + +// isMutatingVerb reports whether the HTTP method writes server state. +// Used by do()'s verify-mode short-circuit to gate dial-out: under +// PRINTING_PRESS_VERIFY=1 (without LIVE_HTTP=1 opt-in), generated +// commands must not actually issue mutating requests, even if a +// handler-level dry-run check was missed. +func isMutatingVerb(method string) bool { + switch method { + case "DELETE", "POST", "PUT", "PATCH": + return true + } + return false +} + +// verifyShortCircuitEnvelope returns the synthetic JSON body that +// stands in for a real mutating response when do() short-circuits in +// verify mode. The __pp_verify_synthetic__ sentinel is namespace- +// reserved (no real API uses __pp_*) so downstream consumers +// (validate-narrative, agent inspections) can key on one obvious field +// instead of trying to disambiguate common literals like status:"noop". +// method and path are echoed back as diagnostic prose for human/agent +// inspection. +func verifyShortCircuitEnvelope(method, path string) json.RawMessage { + body, _ := json.Marshal(map[string]any{ + "__pp_verify_synthetic__": true, + "status": "noop", + "reason": "verify_short_circuit", + "method": method, + "path": path, + }) + return json.RawMessage(body) +} + +func sleepContext(ctx context.Context, wait time.Duration) error { + timer := time.NewTimer(wait) + defer timer.Stop() + select { + case <-timer.C: + return nil + case <-ctx.Done(): + return ctx.Err() + } +} + +// do executes an HTTP request. headerOverrides, when non-nil, override global +// RequiredHeaders for this specific request (used for per-endpoint API versioning). +func (c *Client) do(ctx context.Context, method, path string, params map[string]string, body any, headerOverrides map[string]string) (json.RawMessage, int, error) { + return c.doInternal(ctx, method, path, params, body, headerOverrides, false) +} + +// doRead is do() minus the verify-mode mutating-verb gate. Used by the +// PostQuery* family for read-only operations that ride a mutating verb on +// the wire (GraphQL queries, JSON-RPC reads, POST-based search endpoints). +// The wire verb is still POST/PUT/PATCH so the server sees a real request, +// but the verify-mode short-circuit does not fire because the operation +// does not mutate remote state. +func (c *Client) doRead(ctx context.Context, method, path string, params map[string]string, body any, headerOverrides map[string]string) (json.RawMessage, int, error) { + return c.doInternal(ctx, method, path, params, body, headerOverrides, true) +} + +// doInternal is the shared implementation behind do() and doRead(). The +// readOnlyIntent flag is set by doRead() callers (read-only POST/PUT/PATCH +// operations like GraphQL queries) to skip the mutating-verb verify-mode +// gate. Plain do() callers leave it false and get the usual short-circuit. +func (c *Client) doInternal(ctx context.Context, method, path string, params map[string]string, body any, headerOverrides map[string]string, readOnlyIntent bool) (json.RawMessage, int, error) { + // Verify-mode transport-layer gate. When the verifier (or any consumer + // that sets PRINTING_PRESS_VERIFY=1) drives a mutating verb without + // the LIVE_HTTP=1 opt-in, return a synthetic envelope without dialing, + // minting auth, or touching the cache. The verify pipeline itself + // sets both env vars in mock mode so its httptest server still sees + // real requests; every other consumer gets a safe no-op. + // + // readOnlyIntent suppresses the gate for read-only operations that + // happen to ride a mutating verb on the wire (GraphQL queries, JSON-RPC + // reads, POST-based search endpoints). The handler-level annotation + // `mcp:read-only: true` drives the codegen choice of doRead() vs do(). + // + // Placement note: this fires BEFORE URL building, auth header + // minting, and the success-branch invalidateCache() call below — so + // no cache invalidation runs (no remote state changed) and no + // client_credentials mint happens unnecessarily. + if !readOnlyIntent && isMutatingVerb(method) && cliutil.IsVerifyEnv() && !cliutil.IsVerifyLiveHTTPEnv() { + return verifyShortCircuitEnvelope(method, path), http.StatusOK, nil + } + if err := rejectUnresolvedPathParams(path, nil); err != nil { + return nil, 0, err + } + targetURL := c.BaseURL + path + + var bodyBytes []byte + if body != nil { + b, err := json.Marshal(body) + if err != nil { + return nil, 0, fmt.Errorf("marshaling body: %w", err) + } + bodyBytes = b + } + + // Resolve auth material before the dry-run branch so --dry-run can preview + // exactly what would be sent. Uses only cached credentials; a token that + // requires a network refresh will be re-fetched on the live request path, + // not during dry-run. + authHeader, err := c.authHeader(ctx) + if err != nil { + return nil, 0, err + } + + // Build the request for dry-run display or actual execution + if c.DryRun { + return c.dryRun(method, targetURL, path, params, bodyBytes, headerOverrides, authHeader) + } + + maxRetries := clientMaxRetries() + var lastErr error + refreshedAfterUnauthorized := false + + for attempt := 0; attempt <= maxRetries; attempt++ { + // Proactive rate limiting — wait before sending + c.limiter.Wait() + var bodyReader io.Reader + if bodyBytes != nil { + bodyReader = strings.NewReader(string(bodyBytes)) + } + + req, err := http.NewRequestWithContext(ctx, method, targetURL, bodyReader) + if err != nil { + return nil, 0, fmt.Errorf("creating request: %w", err) + } + if bodyBytes != nil { + req.Header.Set("Content-Type", "application/json") + } + + if params != nil { + q := req.URL.Query() + for k, v := range params { + if v != "" { + q.Set(k, v) + } + } + req.URL.RawQuery = q.Encode() + } + + if authHeader != "" { + req.Header.Set("Authorization", authHeader) + } + if c.Config != nil { + for k, v := range c.Config.Headers { + req.Header.Set(k, v) + } + } + // Per-endpoint header overrides (e.g., different API version per resource) + for k, v := range headerOverrides { + req.Header.Set(k, v) + } + binaryResponse := strings.EqualFold(req.Header.Get(BinaryResponseHeader), "true") + if binaryResponse { + req.Header.Del(BinaryResponseHeader) + } + if req.Header.Get("User-Agent") == "" { + req.Header.Set("User-Agent", "printing-press-oauth2-pp-cli/1.0.0") + } + // Go's net/http omits Accept by default; browsers, curl, and other + // stdlibs always send it. Fingerprint-checking WAFs (Imperva, Akamai, + // Cloudflare bot-mode, DataDome) flag the absence as a bot signal + // and answer with empty-body 5xx, 403, or a challenge redirect + // depending on vendor and rule tier. The value is application/json + // rather than */* because strict-JSON APIs (Zendesk, Atlassian REST, + // Salesforce) return 415 on anything that isn't literally + // application/json; specs that need a different content type + // (vendor mediatypes, XML, HTML) declare it via RequiredHeaders or + // per-endpoint headerOverrides, both of which run before this + // if-empty default. + if req.Header.Get("Accept") == "" { + if binaryResponse { + req.Header.Set("Accept", "*/*") + } else { + req.Header.Set("Accept", "application/json") + } + } + + resp, err := c.HTTPClient.Do(req) + if err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, 0, ctxErr + } + lastErr = fmt.Errorf("%s %s: %w", method, c.displayURL(path, authHeader), c.maskError(err, authHeader)) + continue + } + + respBody, err := io.ReadAll(resp.Body) + resp.Body.Close() + if err != nil { + return nil, 0, fmt.Errorf("reading response: %w", err) + } + + // Success + if resp.StatusCode < 400 { + c.limiter.OnSuccess() + if method != http.MethodGet && !c.DryRun { + c.invalidateCache() + } + // Non-textual bodies (PDF, zip, image, octet-stream) must not be + // run through the JSON sanitizer or returned as raw json.RawMessage + // — return a self-describing base64 envelope instead. Textual and + // JSON responses fall through to the unchanged path. + if isBinaryResponseContentType(resp.Header.Get("Content-Type")) { + env, encErr := wrapBinaryResponse(resp.Header.Get("Content-Type"), respBody) + if encErr != nil { + return nil, 0, encErr + } + return env, resp.StatusCode, nil + } + return json.RawMessage(sanitizeJSONResponse(respBody)), resp.StatusCode, nil + } + + if !binaryResponse { + respBody = sanitizeJSONResponse(respBody) + } + + apiErr := &APIError{ + Method: method, + Path: c.displayURL(path, authHeader), + StatusCode: resp.StatusCode, + Body: c.maskCredentialText(truncateBody(respBody), authHeader), + } + // OAuth providers can expire tokens early, omit expires_in, or disagree + // with the local clock. Retry one unauthorized response after refreshing. + if resp.StatusCode == http.StatusUnauthorized && !refreshedAfterUnauthorized && attempt < maxRetries && c.Config != nil && c.Config.RefreshToken != "" && !(cliutil.IsVerifyEnv() && !cliutil.IsVerifyLiveHTTPEnv()) { + if authHeaderLooksLikePlaceholderCredential(c.Config.AccessToken) || authHeaderLooksLikePlaceholderCredential(c.Config.RefreshToken) || authHeaderLooksLikePlaceholderCredential(c.Config.ClientID) || authHeaderLooksLikePlaceholderCredential(c.Config.ClientSecret) { + return nil, resp.StatusCode, authPlaceholderCredentialError(c.Config) + } + if err := c.refreshAccessToken(ctx); err != nil { + return nil, resp.StatusCode, fmt.Errorf("refreshing access token after 401: %w", err) + } + authHeader = c.Config.AuthHeader() + if authHeaderLooksLikePlaceholderCredential(authHeader) { + return nil, resp.StatusCode, authPlaceholderCredentialError(c.Config) + } + refreshedAfterUnauthorized = true + lastErr = apiErr + continue + } + + // Rate limited - adjust adaptive limiter and retry + if resp.StatusCode == 429 && attempt < maxRetries { + c.limiter.OnRateLimit() + wait := cliutil.RetryAfter(resp) + fmt.Fprintf(os.Stderr, "rate limited, waiting %s (attempt %d/%d, rate adjusted to %.1f req/s)\n", wait, attempt+1, maxRetries, c.limiter.Rate()) + if err := sleepContext(ctx, wait); err != nil { + return nil, 0, err + } + lastErr = apiErr + continue + } + + // Server error - retry with backoff + if resp.StatusCode >= 500 && attempt < maxRetries { + wait := time.Duration(math.Pow(2, float64(attempt))) * time.Second + fmt.Fprintf(os.Stderr, "server error %d, retrying in %s (attempt %d/%d)\n", resp.StatusCode, wait, attempt+1, maxRetries) + if err := sleepContext(ctx, wait); err != nil { + return nil, 0, err + } + lastErr = apiErr + continue + } + + // Client error or retries exhausted - return the error + return nil, resp.StatusCode, apiErr + } + + return nil, 0, lastErr +} + +// dryRun prints the outgoing request exactly as the live path would send it, +// using the auth material already resolved in `do()`. Never triggers a network +// call — the caller is responsible for passing cached auth material only. +func (c *Client) dryRun(method, targetURL, path string, params map[string]string, body []byte, headerOverrides map[string]string, authHeader string) (json.RawMessage, int, error) { + fmt.Fprintf(os.Stderr, "%s %s\n", method, c.displayURL(targetURL, authHeader)) + queryPrinted := false + if params != nil { + keys := make([]string, 0, len(params)) + for k := range params { + if params[k] != "" { + keys = append(keys, k) + } + } + sort.Strings(keys) + for _, k := range keys { + sep := "?" + if queryPrinted { + sep = "&" + } + fmt.Fprintf(os.Stderr, " %s%s=%s\n", sep, k, c.maskCredentialText(params[k], authHeader)) + queryPrinted = true + } + } + _ = queryPrinted + if body != nil { + var pretty json.RawMessage + if json.Unmarshal(body, &pretty) == nil { + enc := json.NewEncoder(os.Stderr) + enc.SetIndent(" ", " ") + fmt.Fprintf(os.Stderr, " Body:\n") + enc.Encode(pretty) + } + } + if authHeader != "" { + fmt.Fprintf(os.Stderr, " %s: %s\n", "Authorization", maskToken(authHeader)) + } + fmt.Fprintf(os.Stderr, "\n(dry run - no request sent)\n") + return json.RawMessage(`{"dry_run": true}`), 0, nil +} + +func (c *Client) ConfiguredTimeout() time.Duration { + if c.HTTPClient != nil && c.HTTPClient.Timeout > 0 { + return c.HTTPClient.Timeout + } + return 60 * time.Second +} + +func (c *Client) authHeader(ctx context.Context) (string, error) { + if c.Config == nil { + return "", nil + } + if c.Config.AccessToken != "" && !c.Config.TokenExpiry.IsZero() && time.Now().After(c.Config.TokenExpiry) && c.Config.RefreshToken != "" && !(cliutil.IsVerifyEnv() && !cliutil.IsVerifyLiveHTTPEnv()) { + if authHeaderLooksLikePlaceholderCredential(c.Config.AccessToken) || authHeaderLooksLikePlaceholderCredential(c.Config.RefreshToken) || authHeaderLooksLikePlaceholderCredential(c.Config.ClientID) || authHeaderLooksLikePlaceholderCredential(c.Config.ClientSecret) { + return "", authPlaceholderCredentialError(c.Config) + } + if err := c.refreshAccessToken(ctx); err != nil { + return "", err + } + } + authHeader := c.Config.AuthHeader() + if authHeaderLooksLikePlaceholderCredential(authHeader) { + return "", authPlaceholderCredentialError(c.Config) + } + return authHeader, nil +} + +func authHeaderLooksLikePlaceholderCredential(header string) bool { + if scheme, encoded, ok := strings.Cut(strings.TrimSpace(header), " "); ok && strings.EqualFold(scheme, "Basic") { + encoded = strings.TrimSpace(encoded) + decoded, err := base64.StdEncoding.DecodeString(encoded) + if err == nil && authHeaderLooksLikePlaceholderCredential(string(decoded)) { + return true + } + } + if !strings.Contains(header, "<") && !strings.Contains(header, "YOUR_TOKEN_HERE") && !strings.Contains(header, "your-token") && !strings.Contains(header, "your-key") { + return false + } + for _, field := range strings.Fields(header) { + field = strings.Trim(field, `"'`) + if idx := strings.LastIndex(field, "="); idx >= 0 { + field = field[idx+1:] + } + if idx := strings.Index(field, ":"); idx >= 0 { + if looksLikeCredentialPlaceholder(field[:idx]) || looksLikeCredentialPlaceholder(field[idx+1:]) { + return true + } + } + if looksLikeCredentialPlaceholder(field) { + return true + } + } + return looksLikeCredentialPlaceholder(header) +} + +func looksLikeCredentialPlaceholder(value string) bool { + value = strings.Trim(strings.TrimSpace(value), `"'`) + switch value { + case "", "", "", "YOUR_TOKEN_HERE", "your-token-here": + return true + } + if len(value) < 3 || value[0] != '<' || value[len(value)-1] != '>' { + return false + } + for _, r := range value[1 : len(value)-1] { + if r != '_' && (r < 'A' || r > 'Z') { + return false + } + } + return true +} + +func authPlaceholderCredentialError(cfg *config.Config) error { + return authPlaceholderCredentialErrorWithSetup(cfg, "printing-press-oauth2-pp-cli auth login") +} + +func authPlaceholderCredentialErrorWithSetup(cfg *config.Config, setup string) error { + location := "config file" + if cfg != nil && cfg.Path != "" { + location = cfg.Path + } + return fmt.Errorf("%w configured in %s; set a real token with: %s", ErrPlaceholderCredential, location, setup) +} + +func (c *Client) refreshAccessToken(ctx context.Context) error { + if c.Config == nil { + return nil + } + if c.Config.RefreshToken == "" { + return nil + } + if strings.TrimSpace(c.Config.ClientID) == "" { + return fmt.Errorf("refreshing access token: OAuth2 client ID is required when a refresh token is configured; set client_id in config or the client ID environment variable") + } + + tokenURL := c.Config.TokenURL + if tokenURL == "" { + tokenURL = "https://accounts.authcode.example/oauth/token" + } + + params := url.Values{ + "grant_type": {"refresh_token"}, + "refresh_token": {c.Config.RefreshToken}, + "client_id": {c.Config.ClientID}, + } + if c.Config.ClientSecret != "" { + params.Set("client_secret", c.Config.ClientSecret) + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, tokenURL, strings.NewReader(params.Encode())) + if err != nil { + return fmt.Errorf("building refresh request: %w", err) + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + resp, err := c.HTTPClient.Do(req) + if err != nil { + return fmt.Errorf("refreshing access token: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode >= 400 { + body, _ := io.ReadAll(resp.Body) + return fmt.Errorf("refreshing access token: HTTP %d: %s", resp.StatusCode, cliutil.SanitizeErrorBody(truncateBody(body))) + } + + var tokenResp struct { + AccessToken string `json:"access_token"` + RefreshToken string `json:"refresh_token"` + ExpiresIn int `json:"expires_in"` + } + if err := json.NewDecoder(resp.Body).Decode(&tokenResp); err != nil { + return fmt.Errorf("parsing refresh response: %w", err) + } + if tokenResp.AccessToken == "" { + return fmt.Errorf("refreshing access token: no access token in response") + } + + refreshToken := c.Config.RefreshToken + if tokenResp.RefreshToken != "" { + refreshToken = tokenResp.RefreshToken + } + + expiry := time.Time{} + if tokenResp.ExpiresIn > 0 { + expiry = time.Now().Add(time.Duration(tokenResp.ExpiresIn) * time.Second) + } + + c.Config.AuthHeaderVal = "" // force AuthHeader() to use the refreshed AccessToken path + if err := c.Config.SaveTokens(c.Config.ClientID, c.Config.ClientSecret, tokenResp.AccessToken, refreshToken, expiry); err != nil { + return fmt.Errorf("saving refreshed token: %w", err) + } + + return nil +} + +// binaryResponseEnvelope wraps a non-textual success body so it survives the +// json.RawMessage contract every consumer (CLI output, --json, MCP tools) +// depends on. Without it, raw bytes (PDF, zip, image) are corrupted by +// sanitizeJSONResponse and emitted as invalid JSON. The _pp_binary +// discriminator lets callers and agents detect and base64-decode the payload. +type binaryResponseEnvelope struct { + PPBinary bool `json:"_pp_binary"` + ContentType string `json:"content_type"` + Encoding string `json:"encoding"` + Bytes int `json:"bytes"` + Data string `json:"data"` +} + +// isBinaryResponseContentType reports whether a successful response with this +// Content-Type must be base64-wrapped instead of treated as text/JSON. It is +// deliberately narrow: JSON, */*, XML, and every text/* type (including +// text/html, so response_format:html CLIs are untouched) pass through +// unchanged. Only genuinely binary payloads are wrapped. +func isBinaryResponseContentType(ct string) bool { + mt := strings.ToLower(strings.TrimSpace(ct)) + if i := strings.IndexByte(mt, ';'); i >= 0 { + mt = strings.TrimSpace(mt[:i]) + } + if mt == "" { + return false + } + switch { + case mt == "application/json", mt == "text/json", mt == "*/*": + return false + case strings.HasPrefix(mt, "text/"): + return false + case strings.HasSuffix(mt, "+json"), strings.HasSuffix(mt, "+xml"): + return false + case mt == "application/xml", mt == "application/xhtml+xml": + return false + case mt == "application/javascript", mt == "application/ecmascript", + mt == "application/x-www-form-urlencoded", mt == "application/graphql": + return false + } + return true +} + +// wrapBinaryResponse marshals body into a self-describing base64 envelope. +func wrapBinaryResponse(ct string, body []byte) (json.RawMessage, error) { + out, err := json.Marshal(binaryResponseEnvelope{ + PPBinary: true, + ContentType: ct, + Encoding: "base64", + Bytes: len(body), + Data: base64.StdEncoding.EncodeToString(body), + }) + if err != nil { + return nil, fmt.Errorf("encoding binary response: %w", err) + } + return json.RawMessage(out), nil +} + +// sanitizeJSONResponse strips known JSONP/XSSI prefixes and UTF-8 BOM from +// response bodies so that downstream JSON parsing succeeds. For clean JSON +// responses these checks are no-ops. +func sanitizeJSONResponse(body []byte) []byte { + // UTF-8 BOM + body = bytes.TrimPrefix(body, []byte("\xEF\xBB\xBF")) + + // JSONP/XSSI prefixes, ordered longest-first where prefixes overlap + prefixes := [][]byte{ + []byte(")]}'\n"), + []byte(")]}'"), + []byte("{}&&"), + []byte("for(;;);"), + []byte("while(1);"), + } + for _, p := range prefixes { + if bytes.HasPrefix(body, p) { + body = bytes.TrimPrefix(body, p) + body = bytes.TrimLeft(body, " \t\r\n") + break + } + } + return body +} + +// maskToken redacts all but the last 4 characters of a token for safe display. +func maskToken(token string) string { + if token == "" { + return "" + } + if len(token) <= 4 { + return "****" + } + return "****" + token[len(token)-4:] +} + +type maskedError struct { + msg string +} + +func (e maskedError) Error() string { + return e.msg +} + +func (c *Client) displayURL(rawURL string, extraCredentials ...string) string { + return c.maskCredentialText(rawURL, extraCredentials...) +} + +func (c *Client) maskError(err error, extraCredentials ...string) error { + if err == nil { + return nil + } + raw := err.Error() + msg := c.maskCredentialText(raw, extraCredentials...) + if msg == raw { + return err + } + return maskedError{msg: msg} +} + +func (c *Client) maskCredentialText(text string, extraCredentials ...string) string { + if text == "" { + return text + } + type credentialMask struct { + needle string + replacement string + } + var masks []credentialMask + seen := map[string]struct{}{} + addValue := func(value string) { + value = strings.TrimSpace(value) + if value == "" { + return + } + replacement := maskToken(value) + addMask := func(needle string) { + if needle == "" { + return + } + if _, ok := seen[needle]; ok { + return + } + seen[needle] = struct{}{} + masks = append(masks, credentialMask{needle: needle, replacement: replacement}) + } + addMask(value) + if escaped := url.QueryEscape(value); escaped != value { + addMask(escaped) + } + if escaped := url.PathEscape(value); escaped != value { + addMask(escaped) + } + } + addCredential := func(value string) { + value = strings.TrimSpace(value) + addValue(value) + if _, token, ok := strings.Cut(value, " "); ok { + addValue(token) + } + } + for _, value := range extraCredentials { + addCredential(value) + } + if c != nil && c.Config != nil { + addCredential(c.Config.AuthHeaderVal) + addCredential(c.Config.AuthHeader()) + addCredential(c.Config.AccessToken) + addCredential(c.Config.RefreshToken) + addCredential(c.Config.ClientSecret) + addCredential(c.Config.PrintingPressOauth2Oauth2AuthCode) + } + sort.SliceStable(masks, func(i, j int) bool { + return len(masks[i].needle) > len(masks[j].needle) + }) + masked := text + for _, mask := range masks { + masked = strings.ReplaceAll(masked, mask.needle, mask.replacement) + } + return masked +} + +func truncateBody(b []byte) string { + const maxBytes = 4096 + if len(b) <= maxBytes { + return string(b) + } + return strings.ToValidUTF8(string(b[:maxBytes]), "") + "..." +} + +func clientMaxRetries() int { + if cliutil.IsVerifyEnv() || cliutil.IsDogfoodEnv() { + return 0 + } + return 3 +} diff --git a/testdata/golden/expected/generate-golden-api-oauth2-authcode/stderr.txt b/testdata/golden/expected/generate-golden-api-oauth2-authcode/stderr.txt new file mode 100644 index 000000000..6122332b2 --- /dev/null +++ b/testdata/golden/expected/generate-golden-api-oauth2-authcode/stderr.txt @@ -0,0 +1,2 @@ +warning: could not derive run_id from --research-dir; phase5 dogfood acceptance will refuse to write without it +Generated printing-press-oauth2 at /generate-golden-api-oauth2-authcode/printing-press-oauth2-authcode diff --git a/testdata/golden/expected/generate-golden-api-oauth2-authcode/stdout.txt b/testdata/golden/expected/generate-golden-api-oauth2-authcode/stdout.txt new file mode 100644 index 000000000..e69de29bb diff --git a/testdata/golden/fixtures/golden-api-oauth2-authcode.yaml b/testdata/golden/fixtures/golden-api-oauth2-authcode.yaml new file mode 100644 index 000000000..d50099fdc --- /dev/null +++ b/testdata/golden/fixtures/golden-api-oauth2-authcode.yaml @@ -0,0 +1,47 @@ +openapi: "3.0.3" +info: + title: Printing Press OAuth2 AuthCode API + version: "1.0.0" + description: Purpose-built fixture for the OAuth2 authorization_code + PKCE auth shape. +servers: + - url: https://api.authcode.example/v1 +security: + - OAuth2AuthCode: [] +components: + securitySchemes: + # authorizationCode flow declared. The OpenAPI parser detects this + # branch and sets AuthConfig.OAuth2Grant=authorization_code with + # AuthorizationURL + TokenURL, which gates generator template + # selection onto auth.go.tmpl (the browser loopback login flow). + OAuth2AuthCode: + type: oauth2 + flows: + authorizationCode: + authorizationUrl: https://accounts.authcode.example/oauth/authorize + tokenUrl: https://accounts.authcode.example/oauth/token + scopes: + read: Read access + write: Write access + schemas: + Item: + type: object + properties: + id: + type: string + name: + type: string +paths: + /items: + get: + tags: [items] + operationId: listItems + summary: List items + responses: + "200": + description: OK + content: + application/json: + schema: + type: array + items: + $ref: "#/components/schemas/Item" From 82ff4b057d2661837ec0c7214231ee20deb5410e Mon Sep 17 00:00:00 2001 From: Rob Zehner Date: Sun, 5 Jul 2026 20:49:09 -0700 Subject: [PATCH 2/2] =?UTF-8?q?fix(cli):=20address=20review=20=E2=80=94=20?= =?UTF-8?q?context-cancellable=20token=20exchange,=20robust=20test=20asser?= =?UTF-8?q?tions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Token exchange now uses http.NewRequestWithContext(cmd.Context(), ...) so Ctrl+C cancels an in-flight exchange, matching the refreshAccessToken idiom in client.go.tmpl. Split the whitespace-sensitive combined assertion into independent Contains checks; mutual exclusivity stays covered by the verify-env behavioral checks. Co-Authored-By: Claude Fable 5 --- internal/generator/auth_oauth_pkce_test.go | 7 +++++-- internal/generator/templates/auth.go.tmpl | 10 ++++++++-- .../internal/cli/auth.go | 10 ++++++++-- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/internal/generator/auth_oauth_pkce_test.go b/internal/generator/auth_oauth_pkce_test.go index 10227797a..b94229a48 100644 --- a/internal/generator/auth_oauth_pkce_test.go +++ b/internal/generator/auth_oauth_pkce_test.go @@ -38,8 +38,10 @@ func TestOAuthLoginUsesPKCEWhenNoClientSecret(t *testing.T) { require.Contains(t, auth, `params.Set("code_challenge_method", "S256")`) // Token exchange authenticates with exactly one of client_secret or - // code_verifier; the bare authorization_code grant is gone. - require.Contains(t, auth, "tokenParams.Set(\"client_secret\", clientSecret)\n\t} else {\n\t\ttokenParams.Set(\"code_verifier\", codeVerifier)\n\t}") + // code_verifier (mutual exclusivity is validated behaviorally below); + // the bare authorization_code grant is gone. + require.Contains(t, auth, `tokenParams.Set("client_secret", clientSecret)`) + require.Contains(t, auth, `tokenParams.Set("code_verifier", codeVerifier)`) // RFC 8252 loopback redirect: IP literal, never "localhost". require.Contains(t, auth, `redirectURI := fmt.Sprintf("http://127.0.0.1:%d/callback"`) @@ -113,5 +115,6 @@ func TestGeneratePKCEVerifierShapeAndUniqueness(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(outputDir, "internal", "cli", "oauth_pkce_test.go"), []byte(runtimeTest), 0o644)) runGoCommand(t, outputDir, "test", "-race", "./internal/cli", "-run", "Test(PKCECodeChallengeS256|GeneratePKCEVerifier)") + require.Contains(t, auth, "http.NewRequestWithContext(cmd.Context(), http.MethodPost, tokenURL", "token exchange should be context-cancellable") require.False(t, strings.Contains(auth, "http.PostForm(tokenURL"), "token exchange should use a client with its own timeout, not the shared default client") } diff --git a/internal/generator/templates/auth.go.tmpl b/internal/generator/templates/auth.go.tmpl index b490ad469..88dd1e111 100644 --- a/internal/generator/templates/auth.go.tmpl +++ b/internal/generator/templates/auth.go.tmpl @@ -286,9 +286,15 @@ func runOAuthLogin(cmd *cobra.Command, flags *rootFlags, clientID, clientSecret } // The browser-wait timeout above does not apply here; the token exchange - // is a plain server-to-server POST and gets its own short network timeout. + // is a plain server-to-server POST and gets its own short network timeout, + // while cmd.Context() keeps it cancellable from the terminal. + tokenReq, err := http.NewRequestWithContext(cmd.Context(), http.MethodPost, tokenURL, strings.NewReader(tokenParams.Encode())) + if err != nil { + return fmt.Errorf("building token request: %w", err) + } + tokenReq.Header.Set("Content-Type", "application/x-www-form-urlencoded") tokenClient := &http.Client{Timeout: 30 * time.Second} - resp, err := tokenClient.PostForm(tokenURL, tokenParams) + resp, err := tokenClient.Do(tokenReq) if err != nil { return fmt.Errorf("exchanging code for token: %w", err) } diff --git a/testdata/golden/expected/generate-golden-api-oauth2-authcode/printing-press-oauth2-authcode/internal/cli/auth.go b/testdata/golden/expected/generate-golden-api-oauth2-authcode/printing-press-oauth2-authcode/internal/cli/auth.go index d082c2a92..c9427c2a4 100644 --- a/testdata/golden/expected/generate-golden-api-oauth2-authcode/printing-press-oauth2-authcode/internal/cli/auth.go +++ b/testdata/golden/expected/generate-golden-api-oauth2-authcode/printing-press-oauth2-authcode/internal/cli/auth.go @@ -228,9 +228,15 @@ func runOAuthLogin(cmd *cobra.Command, flags *rootFlags, clientID, clientSecret } // The browser-wait timeout above does not apply here; the token exchange - // is a plain server-to-server POST and gets its own short network timeout. + // is a plain server-to-server POST and gets its own short network timeout, + // while cmd.Context() keeps it cancellable from the terminal. + tokenReq, err := http.NewRequestWithContext(cmd.Context(), http.MethodPost, tokenURL, strings.NewReader(tokenParams.Encode())) + if err != nil { + return fmt.Errorf("building token request: %w", err) + } + tokenReq.Header.Set("Content-Type", "application/x-www-form-urlencoded") tokenClient := &http.Client{Timeout: 30 * time.Second} - resp, err := tokenClient.PostForm(tokenURL, tokenParams) + resp, err := tokenClient.Do(tokenReq) if err != nil { return fmt.Errorf("exchanging code for token: %w", err) }