stacklok / toolhive / 25403856720

65%

Build Type

push

github

Committed by web-flow

Commit Message

Return *oauth2.RetrieveError from tokenexchange (#5082)

* Return *oauth2.RetrieveError from tokenexchange

Replace the private oAuthError type with *oauth2.RetrieveError from
golang.org/x/oauth2 so token exchange errors expose RFC 6749 §5.2
fields (error, error_description, error_uri) as structured data via
errors.As. This is the library-standard surface for non-2xx token
endpoint responses, and aligns the error shape with the JWT Bearer
grant that will share helpers in pkg/oauth.

Behavior changes:

- validateResponseStatus takes *http.Response so it can attach the
  full response to the returned error and parse the body as RFC
  6749 §5.2 best-effort.

- When the body is non-conformant (no "error" field, e.g. a proxy
  HTML 5xx), the raw body is logged at debug level and cleared from
  the returned error. This prevents oauth2.RetrieveError.Error()
  from interpolating arbitrary upstream content (HTML, hostnames,
  stack traces) into wrapped error strings — same two-tier pattern
  used by formatOAuth2Error in pkg/authserver.

- parseTokenExchangeResponse wraps json.Unmarshal failures with %w.

The error type change is isolated from code movement so a future
bisect can distinguish "error shape regressed" from "plumbing
regressed".

* Always clear RetrieveError.Body in tokenexchange

The previous commit cleared the body only when the response was
non-conformant (no RFC 6749 §5.2 "error" field), on the theory that
a structured-error body is bounded and harmless. PR review pointed
out the asymmetry, and the simpler answer is to clear Body in both
branches:

- The structured fields (ErrorCode, ErrorDescription, ErrorURI) are
  already extracted onto *oauth2.RetrieveError, so callers using
  errors.As lose nothing.

- Full body content is preserved in slog.Debug for ops, regardless
  of which branch is taken.

- No caller in this repo reads retrieveErr.Body for any non-debug
  purpose (verified by grep on .RetrieveError\b).

- Removes a special case futur... (continued)

Coverage Stats

29 of 29 new or added lines in 1 file covered. (100.0%)

2 existing lines in 1 file now uncovered.

62912 of 97202 relevant lines covered (64.72%)

59.09 hits per line