← back to Cli Printing Press
fix(cli): refresh auth on redirects via CheckRedirect (#1497)
f3f7043558971d098960ee3f66c3548b7d0b4b15 · 2026-05-16 01:01:01 -0700 · Trevin Chow
* fix(cli): wire CheckRedirect to refresh auth on each redirect hop
Generated client.go now sets http.Client.CheckRedirect inside New() so
the closure can call c.authHeader() and re-stamp the auth header on
each redirect. Go's default follows redirects and replays the original
Authorization header verbatim, which trips nonce-bound auth (OAuth 1.0a
PLAINTEXT, SigV4, Hawk): the duplicate nonce hits the server's replay
detector and returns 401 on the second hop.
For static schemes (Bearer, api_key in header) c.authHeader() returns
the same value, so post-redirect headers are byte-identical. For
nonce-bound schemes the call recomputes a fresh signature. Tier-routing
selects header-vs-query at runtime, so its branch only caps depth and
leaves Go's default replay in place. Query-param auth is preserved by
Go's URL resolution on redirects, so its branch also just caps depth.
Closes #1410
* fix(cli): use plain error for redirect depth cap, not ErrUseLastResponse
Returning http.ErrUseLastResponse from CheckRedirect makes Client.Do
return the 3xx redirect response with a nil error. The generated do()
function then classifies the 3xx as a successful response and hands the
HTML "Moved Permanently" body back to the caller as if it were a valid
API response. Match Go's defaultCheckRedirect by returning a plain
error so Do() surfaces it through do()'s err != nil branch.
Updates the regression test to match.
* fix(cli): gate redirect auth re-stamp on same-host
Headers set inside CheckRedirect are sent to the redirect target
verbatim — Go's automatic Authorization stripping
(shouldCopyHeaderOnRedirect) only runs before CheckRedirect is invoked.
Re-stamping the auth header unconditionally would leak the user's API
token to a third-party host on cross-domain 3xx (open redirect or
partner handoff). Gate the re-stamp on req.URL.Host == via[0].URL.Host
so the nonce-replay fix still applies to same-host redirects (the
Schoology 303 case) while preserving Go's cross-domain credential
protection.
The session_handshake header-mode branch carries the same gate.
---------
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Files touched
A internal/generator/client_redirect_auth_test.goM internal/generator/templates/client.go.tmplM testdata/golden/expected/generate-golden-api-oauth2-cc/printing-press-oauth2-cc/internal/client/client.goM testdata/golden/expected/generate-golden-api/printing-press-golden/internal/client/client.goM testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/client/client.go
Diff
commit f3f7043558971d098960ee3f66c3548b7d0b4b15
Author: Trevin Chow <trevin@trevinchow.com>
Date: Sat May 16 01:01:01 2026 -0700
fix(cli): refresh auth on redirects via CheckRedirect (#1497)
* fix(cli): wire CheckRedirect to refresh auth on each redirect hop
Generated client.go now sets http.Client.CheckRedirect inside New() so
the closure can call c.authHeader() and re-stamp the auth header on
each redirect. Go's default follows redirects and replays the original
Authorization header verbatim, which trips nonce-bound auth (OAuth 1.0a
PLAINTEXT, SigV4, Hawk): the duplicate nonce hits the server's replay
detector and returns 401 on the second hop.
For static schemes (Bearer, api_key in header) c.authHeader() returns
the same value, so post-redirect headers are byte-identical. For
nonce-bound schemes the call recomputes a fresh signature. Tier-routing
selects header-vs-query at runtime, so its branch only caps depth and
leaves Go's default replay in place. Query-param auth is preserved by
Go's URL resolution on redirects, so its branch also just caps depth.
Closes #1410
* fix(cli): use plain error for redirect depth cap, not ErrUseLastResponse
Returning http.ErrUseLastResponse from CheckRedirect makes Client.Do
return the 3xx redirect response with a nil error. The generated do()
function then classifies the 3xx as a successful response and hands the
HTML "Moved Permanently" body back to the caller as if it were a valid
API response. Match Go's defaultCheckRedirect by returning a plain
error so Do() surfaces it through do()'s err != nil branch.
Updates the regression test to match.
* fix(cli): gate redirect auth re-stamp on same-host
Headers set inside CheckRedirect are sent to the redirect target
verbatim — Go's automatic Authorization stripping
(shouldCopyHeaderOnRedirect) only runs before CheckRedirect is invoked.
Re-stamping the auth header unconditionally would leak the user's API
token to a third-party host on cross-domain 3xx (open redirect or
partner handoff). Gate the re-stamp on req.URL.Host == via[0].URL.Host
so the nonce-replay fix still applies to same-host redirects (the
Schoology 303 case) while preserving Go's cross-domain credential
protection.
The session_handshake header-mode branch carries the same gate.
---------
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
internal/generator/client_redirect_auth_test.go | 104 +++++++++++++++++++++
internal/generator/templates/client.go.tmpl | 54 ++++++++++-
.../internal/client/client.go | 31 +++++-
.../internal/client/client.go | 31 +++++-
.../tier-routing-golden/internal/client/client.go | 25 ++++-
5 files changed, 240 insertions(+), 5 deletions(-)
diff --git a/internal/generator/client_redirect_auth_test.go b/internal/generator/client_redirect_auth_test.go
new file mode 100644
index 00000000..78f4ba91
--- /dev/null
+++ b/internal/generator/client_redirect_auth_test.go
@@ -0,0 +1,104 @@
+package generator
+
+import (
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+
+ "github.com/mvanhorn/cli-printing-press/v4/internal/spec"
+ "github.com/stretchr/testify/require"
+)
+
+// Regression for #1410: the generated client.go must wire CheckRedirect on
+// the http.Client so nonce-bound auth (OAuth 1.0a PLAINTEXT, SigV4, Hawk)
+// gets a fresh authHeader on each redirect hop instead of replaying the
+// original Authorization header. Go's default replays headers verbatim,
+// which trips replay-detection on the second hop (Schoology 303 -> 401).
+func TestClientCheckRedirectReappliesAuth(t *testing.T) {
+ t.Parallel()
+
+ t.Run("bearer in Authorization header re-sets Authorization", func(t *testing.T) {
+ t.Parallel()
+ apiSpec := minimalSpec("redirect-bearer")
+ apiSpec.Auth = spec.AuthConfig{
+ Type: "bearer_token",
+ EnvVars: []string{"REDIRECT_BEARER_TOKEN"},
+ }
+ client := generateClientSource(t, apiSpec)
+ closure := checkRedirectClosureBody(t, client)
+
+ require.Contains(t, closure, "if len(via) >= 10 {",
+ "CheckRedirect must cap depth at 10 to match Go's default policy")
+ require.Contains(t, closure, `return errors.New("stopped after 10 redirects")`,
+ "depth cap must return a plain error so Do() propagates it; ErrUseLastResponse would silently surface the 3xx body as a successful response")
+ require.Contains(t, closure, `c.authHeader()`,
+ "CheckRedirect must call c.authHeader() so nonce-bound schemes get a fresh signature")
+ require.Contains(t, closure, `req.Header.Set("Authorization", h)`,
+ "bearer auth must re-set Authorization on redirect to refresh nonce-bound headers")
+ require.Contains(t, closure, "req.URL.Host == via[0].URL.Host",
+ "auth re-stamp must be gated on same-host so a cross-domain 3xx (open redirect or partner handoff) does not leak the credential — Go's automatic Authorization stripping has already run by the time CheckRedirect is called, and any header set here is sent verbatim")
+ })
+
+ t.Run("api_key in custom header re-sets that header, not Authorization", func(t *testing.T) {
+ t.Parallel()
+ apiSpec := minimalSpec("redirect-apikey-header")
+ apiSpec.Auth = spec.AuthConfig{
+ Type: "api_key",
+ Header: "X-API-Key",
+ EnvVars: []string{"REDIRECT_APIKEY_HEADER_TOKEN"},
+ }
+ client := generateClientSource(t, apiSpec)
+ closure := checkRedirectClosureBody(t, client)
+
+ require.Contains(t, closure, `req.Header.Set("X-API-Key", h)`,
+ "api_key in custom header must re-set that header on redirect, not Authorization")
+ require.NotContains(t, closure, `req.Header.Set("Authorization", h)`,
+ "must not also stamp Authorization when the spec uses a custom header")
+ require.Contains(t, closure, "req.URL.Host == via[0].URL.Host",
+ "custom-header auth must also be same-host gated to avoid leaking credentials across domains")
+ })
+
+ t.Run("api_key in query parameter skips header re-set", func(t *testing.T) {
+ t.Parallel()
+ apiSpec := minimalSpec("redirect-apikey-query")
+ apiSpec.Auth = spec.AuthConfig{
+ Type: "api_key",
+ In: "query",
+ Header: "api_key",
+ EnvVars: []string{"REDIRECT_APIKEY_QUERY_TOKEN"},
+ }
+ client := generateClientSource(t, apiSpec)
+ closure := checkRedirectClosureBody(t, client)
+
+ require.Contains(t, closure, "if len(via) >= 10 {",
+ "CheckRedirect must still cap redirect depth for query-param auth")
+ require.NotContains(t, closure, `c.authHeader()`,
+ "query-param auth must not re-derive a header inside the closure; Go preserves the URL query on redirects")
+ require.NotContains(t, closure, "req.Header.Set(",
+ "query-param auth must not stamp any auth header on redirect")
+ })
+}
+
+// checkRedirectClosureBody extracts the body of the CheckRedirect closure
+// from the generated New() function so substring assertions don't match the
+// docstring above the closure (which mentions c.authHeader for context).
+func checkRedirectClosureBody(t *testing.T, content string) string {
+ t.Helper()
+ marker := "httpClient.CheckRedirect = func(req *http.Request, via []*http.Request) error {"
+ start := strings.Index(content, marker)
+ require.NotEqual(t, -1, start, "CheckRedirect closure must be emitted in New()")
+ body := content[start+len(marker):]
+ end := strings.Index(body, "\n\t}\n")
+ require.NotEqual(t, -1, end, "CheckRedirect closure must be properly closed")
+ return body[:end]
+}
+
+func generateClientSource(t *testing.T, apiSpec *spec.APISpec) string {
+ t.Helper()
+ outputDir := filepath.Join(t.TempDir(), apiSpec.Name+"-pp-cli")
+ require.NoError(t, New(apiSpec, outputDir).Generate())
+ src, err := os.ReadFile(filepath.Join(outputDir, "internal", "client", "client.go"))
+ require.NoError(t, err)
+ return string(src)
+}
diff --git a/internal/generator/templates/client.go.tmpl b/internal/generator/templates/client.go.tmpl
index f1ca56ff..77b75d82 100644
--- a/internal/generator/templates/client.go.tmpl
+++ b/internal/generator/templates/client.go.tmpl
@@ -11,6 +11,7 @@ import (
"crypto/sha256"
"encoding/hex"
"encoding/json"
+ "errors"
"fmt"
"io"
"math"
@@ -324,7 +325,7 @@ func New(cfg *config.Config, timeout time.Duration, rateLimit float64) *Client {
sess := newSessionManager(timeout)
httpClient := newHTTPClient(timeout, sess.CookieJar())
sess.AttachTo(httpClient)
- return &Client{
+ c := &Client{
BaseURL: strings.TrimRight(cfg.BaseURL, "/"),
{{- if .BasePath}}
BasePath: normalizeBasePath(cfg.BasePath),
@@ -350,7 +351,7 @@ func New(cfg *config.Config, timeout time.Duration, rateLimit float64) *Client {
}
{{- else}}
httpClient := newHTTPClient(timeout, nil)
- return &Client{
+ c := &Client{
BaseURL: strings.TrimRight(cfg.BaseURL, "/"),
{{- if .BasePath}}
BasePath: normalizeBasePath(cfg.BasePath),
@@ -374,6 +375,55 @@ func New(cfg *config.Config, timeout time.Duration, rateLimit float64) *Client {
{{- end}}
}
{{- end}}
+ // 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")
+ }
+{{- if .HasTierRouting}}
+ // Tier routing picks header vs query at runtime via authForRequest;
+ // re-deriving here would duplicate that selection logic. Cap depth
+ // only and let Go's default header replay handle static credentials.
+{{- else if eq .Auth.Type "session_handshake"}}
+{{- if eq .Auth.TokenParamIn "header"}}
+ // 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(); err == nil && h != "" {
+ req.Header.Set("{{.Auth.TokenParamName}}", h)
+ }
+ }
+{{- end}}
+{{- else if and .Auth .Auth.In (eq .Auth.In "query")}}
+ // Auth lives on URL query; Go preserves the query on relative
+ // redirects, so nothing to re-derive.
+{{- else}}
+ // 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(); err == nil && h != "" {
+ req.Header.Set("{{if .Auth.Header}}{{.Auth.Header}}{{else}}Authorization{{end}}", h)
+ }
+ }
+{{- end}}
+ return nil
+ }
+ return c
}
// RateLimit returns the current effective rate limit in req/s. Returns 0 if disabled.
diff --git a/testdata/golden/expected/generate-golden-api-oauth2-cc/printing-press-oauth2-cc/internal/client/client.go b/testdata/golden/expected/generate-golden-api-oauth2-cc/printing-press-oauth2-cc/internal/client/client.go
index 7ed57607..41e42318 100644
--- a/testdata/golden/expected/generate-golden-api-oauth2-cc/printing-press-oauth2-cc/internal/client/client.go
+++ b/testdata/golden/expected/generate-golden-api-oauth2-cc/printing-press-oauth2-cc/internal/client/client.go
@@ -8,6 +8,7 @@ import (
"crypto/sha256"
"encoding/hex"
"encoding/json"
+ "errors"
"fmt"
"io"
"math"
@@ -59,7 +60,7 @@ func New(cfg *config.Config, timeout time.Duration, rateLimit float64) *Client {
homeDir, _ := os.UserHomeDir()
cacheDir := filepath.Join(homeDir, ".cache", "printing-press-oauth2-pp-cli", "http")
httpClient := newHTTPClient(timeout, nil)
- return &Client{
+ c := &Client{
BaseURL: strings.TrimRight(cfg.BaseURL, "/"),
Config: cfg,
HTTPClient: httpClient,
@@ -67,6 +68,34 @@ func New(cfg *config.Config, timeout time.Duration, rateLimit float64) *Client {
limiter: cliutil.NewAdaptiveLimiter(rateLimit),
ccMu: &sync.Mutex{},
}
+ // 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(); err == nil && h != "" {
+ req.Header.Set("Authorization", h)
+ }
+ }
+ return nil
+ }
+ return c
}
// RateLimit returns the current effective rate limit in req/s. Returns 0 if disabled.
diff --git a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/client/client.go b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/client/client.go
index 62713388..29132e66 100644
--- a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/client/client.go
+++ b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/client/client.go
@@ -8,6 +8,7 @@ import (
"crypto/sha256"
"encoding/hex"
"encoding/json"
+ "errors"
"fmt"
"io"
"math"
@@ -54,13 +55,41 @@ func New(cfg *config.Config, timeout time.Duration, rateLimit float64) *Client {
homeDir, _ := os.UserHomeDir()
cacheDir := filepath.Join(homeDir, ".cache", "printing-press-golden-pp-cli", "http")
httpClient := newHTTPClient(timeout, nil)
- return &Client{
+ 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(); err == nil && h != "" {
+ req.Header.Set("X-API-Key", h)
+ }
+ }
+ return nil
+ }
+ return c
}
// RateLimit returns the current effective rate limit in req/s. Returns 0 if disabled.
diff --git a/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/client/client.go b/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/client/client.go
index e5f04e40..e12ed4b7 100644
--- a/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/client/client.go
+++ b/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/client/client.go
@@ -8,6 +8,7 @@ import (
"crypto/sha256"
"encoding/hex"
"encoding/json"
+ "errors"
"fmt"
"io"
"math"
@@ -147,7 +148,7 @@ func New(cfg *config.Config, timeout time.Duration, rateLimit float64) *Client {
homeDir, _ := os.UserHomeDir()
cacheDir := filepath.Join(homeDir, ".cache", "tier-routing-golden-pp-cli", "http")
httpClient := newHTTPClient(timeout, nil)
- return &Client{
+ c := &Client{
BaseURL: strings.TrimRight(cfg.BaseURL, "/"),
Config: cfg,
HTTPClient: httpClient,
@@ -155,6 +156,28 @@ func New(cfg *config.Config, timeout time.Duration, rateLimit float64) *Client {
limiter: cliutil.NewAdaptiveLimiter(rateLimit),
limiters: newTierLimiters(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")
+ }
+ // Tier routing picks header vs query at runtime via authForRequest;
+ // re-deriving here would duplicate that selection logic. Cap depth
+ // only and let Go's default header replay handle static credentials.
+ return nil
+ }
+ return c
}
// RateLimit returns the current effective rate limit in req/s. Returns 0 if disabled.
← dc0651ab fix(cli): exempt vendor OpenAPI/Swagger source under .manusc
·
back to Cli Printing Press
·
fix(cli): apply Bearer prefix in composed and cookie AuthHea ca4898a3 →