stacklok / toolhive / 23193994670

push

github

Set AuthStyle to InParams for public PKCE OAuth clients (#4150)

* Set OAuth AuthStyle to InParams for PKCE clients

When oauth2.Endpoint.AuthStyle is unset (zero value), Go's oauth2
library uses AuthStyleAutoDetect, which tries HTTP Basic Auth first.
For public PKCE clients (token_endpoint_auth_method=none), this sends
an Authorization header with an empty password. Spec-compliant servers
reject this and consume the single-use authorization code, causing the
retry with client_id in POST body to fail with invalid_grant.

Set AuthStyleInParams explicitly in all three locations where
oauth2.Endpoint is constructed without AuthStyle:
- pkg/auth/oauth/flow.go (authorization code exchange)
- pkg/auth/remote/handler.go (token refresh from cached tokens)
- pkg/registry/auth/oauth_token_source.go (registry auth)

Add regression test with a strict mock server that rejects Basic Auth
for public clients. Without the fix: 2 requests (auto-detect probing).
With the fix: exactly 1 request.

Fixes #4149

Signed-off-by: Greg Katz <gkatz@indeed.com>

* Fix gci lint in flow_test.go

Signed-off-by: Greg Katz <gkatz@indeed.com>

* Address PR review feedback on OAuth AuthStyle fix

- Make AuthStyle conditional on ClientSecret: public clients (no secret)
  use AuthStyleInParams to avoid burning single-use auth codes on strict
  servers; confidential clients use AuthStyleAutoDetect so servers that
  mandate client_secret_basic are not broken
- Fix data race in regression test: replace plain int requestCount with
  atomic.Int32 so the httptest.Server goroutine and test goroutine access
  it safely under -race

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Signed-off-by: Greg Katz <gkatz@indeed.com>
Co-authored-by: amirejaz <amir@stacklok.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

10 of 28 new or added lines in 3 files covered. (35.71%)

49259 of 76106 relevant lines covered (64.72%)

123.88 hits per line