Delegate tokenexchange HTTP plumbing to pkg/oauthproto

[jhrozek]

Summary

• Why: pkg/auth/tokenexchange carried its own copies of token-endpoint request construction, body draining, size capping, and RFC 6749 response parsing. Earlier commits in this series moved those into pkg/oauthproto for the upcoming JWT-bearer grant. Keeping the local duplicates around meant the two paths would drift, so this PR completes the migration and fulfils the TODO: drop when executeTokenExchangeRequest is replaced by oauthproto.DoTokenRequest planted in Use shared pkg/oauthproto helpers in tokenexchange #5212.
• What:
  • exchangeToken now collapses to buildFormData + oauthproto.NewFormRequest + oauthproto.DoTokenRequest. The 5 local helpers (createTokenExchangeRequest, executeTokenExchangeRequest, parseTokenExchangeResponse, validateResponseStatus, the local response struct, and the maxResponseBodySize constant) are deleted, along with the now-unused imports encoding/json, io, log/slog, strconv, time.
  • RFC 8693 §2.2.1 enforcement (non-empty token_type and issued_token_type) is preserved at the call site — oauthproto.ParseTokenResponse is intentionally permissive on token_type to match x/oauth2/Google, so the exchange grant tightens it back. The redundant access_token empty check is gone; oauthproto.ParseTokenResponse emits "oauth: token response missing access_token (RFC 6749 Section 5.1)" centrally.
  • The historical SubjectTokenType short-form normalization in Validate() is preserved and pinned by TestExchangeConfig_Validate_NormalizesSubjectTokenType.
  • Test fixtures migrate from the in-file responseBuilder to the shared oauthtest.ResponseBuilder from pkg/oauthproto/oauthtest. Three literal response{...} constructions in middleware_test.go get the same treatment.
  • RetrieveError.Body scrubbing is preserved as an explicit opt-in: pkg/oauthproto.parseRetrieveError keeps Body populated for general-purpose callers, but tokenexchange.exchangeToken does an errors.As + Body = nil step right after DoTokenRequest. TestExchangeToken_HTTPErrorResponses re-asserts Body == nil so the property cannot drift again.

Type of change

• Refactoring (no behavior change)

Test plan

• Unit tests (task test) — pkg/auth/tokenexchange/... and pkg/vmcp/auth/strategies/... pass with -race.
• Linting (task lint-fix) — 0 issues.
• E2E tests (task test-e2e via PR CI on kindest/node v1.33/v1.34/v1.35).

API Compatibility

• This PR does not break the v1beta1 API.

Does this introduce a user-facing change?

Yes — operators or alerts keyed on the old tokenexchange error strings need to pick up the new substrings:

• "token exchange request failed" → "oauth: token request failed"
• "failed to parse token exchange response" → "oauth: cannot parse token response"
• "failed to create token exchange request" → "tokenexchange: build request"
• "empty access_token" (vmcp strategy path) → "missing access_token"

Error types are unchanged: *oauth2.RetrieveError still surfaces on non-2xx responses (and now on 2xx responses that carry an RFC 6749 §5.2 error field), and errors.As chains still work.

Special notes for reviewers

• Third PR in the pkg/oauthproto consolidation series (after Use shared pkg/oauthproto helpers in tokenexchange #5212 and the CIMD refactor). The shared NewFormRequest / DoTokenRequest / ParseTokenResponse helpers landed in earlier commits; this PR is purely the consumer migration.
• The body-clearing on RetrieveError (commit body section "RetrieveError.Body preservation") is an explicit opt-in to keep the pre-refactor security property — pkg/oauthproto deliberately preserves Body so general-purpose callers can surface server replies verbatim, but the tokenexchange errors flow through paths that may log err.Error(), so the package opts back into the stricter behavior.
• Diff is dominated by test churn (most of exchange_test.go is migrated to oauthtest.ResponseBuilder); the production-code delta in exchange.go is the substantive change.
• An initial draft of this PR also tightened ExchangeConfig.Validate() on TokenURL to reject plain-HTTP non-localhost endpoints. That hardening was reverted (force-pushed) after the e2e suite — which spins up cluster-local plain-HTTP mock auth servers — surfaced it as a behavior change. A refactor PR shouldn't carry it; the tightening will be proposed separately with appropriate plumbing for in-cluster test setups.

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.83%. Comparing base (8d9eb28) to head (2e9af0c).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/auth/tokenexchange/exchange.go	84.61%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5226      +/-   ##
==========================================
- Coverage   67.87%   67.83%   -0.04%     
==========================================
  Files         610      610              
  Lines       62383    62340      -43     
==========================================
- Hits        42340    42287      -53     
- Misses      16871    16874       +3     
- Partials     3172     3179       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.