Skip to content

oauth2: join both probe errors when auth style is unknown#803

Open
55728 wants to merge 1 commit into
golang:masterfrom
55728:fix-authstyle-probe-error-swallowing
Open

oauth2: join both probe errors when auth style is unknown#803
55728 wants to merge 1 commit into
golang:masterfrom
55728:fix-authstyle-probe-error-swallowing

Conversation

@55728
Copy link
Copy Markdown

@55728 55728 commented Apr 23, 2026

When Endpoint.AuthStyle is AuthStyleUnknown (the default),
RetrieveToken probes the token endpoint with two credential
delivery methods: first in the Authorization header, then in
form params if that fails. If both attempts fail, only the
second error was returned, silently discarding the first.

This is misleading when the first request fails for a reason
unrelated to auth style (e.g. misconfiguration or an expired
signing key) and the provider has already consumed the
authorization code. The caller then sees a confusing error
like "code already redeemed" instead of the real cause.

Fixes #786

Changes:

  • internal/token.go: Join both probe errors with errors.Join
    when both attempts fail.
  • token.go: Add convertRetrieveError helper that recursively
    converts *internal.RetrieveError values inside a joined
    error to the public *RetrieveError type, so errors.As
    keeps working.
  • Tests: Add cases for both-probes-fail and
    header-fails-params-succeeds. Update existing tests.

Compatibility notes:

  • Type assertion change: Code using err.(*RetrieveError)
    directly will get ok == false for the joined error in the
    both-probes-fail path. Use errors.As instead.
  • errors.As ordering: errors.As now unwraps to the first
    (header) probe error rather than the second (params) one.
    This is intentional, as the header error is typically the
    root cause.
  • Error string format: err.Error() will contain both error
    messages (newline-separated via errors.Join).

Credit: Thanks to @masonelmore for the thorough analysis
and reproduction repo
(https://github.com/masonelmore/authstyle-unknown).

When Endpoint.AuthStyle is AuthStyleUnknown (the default), RetrieveToken
probes the token endpoint by trying credentials in the Authorization
header first, then in form params if that fails. If both attempts
failed, only the second error was returned, silently discarding the
first.

This is misleading when the first request fails for a reason unrelated
to auth style (e.g. misconfiguration or an expired signing key) and the
provider has already consumed the authorization code. The second
request then fails with a different error (e.g. "code already
redeemed") that hides the real cause.

Join both errors with errors.Join so callers see the full picture, and
update retrieveToken to convert every wrapped *internal.RetrieveError
to the public *RetrieveError type so errors.As keeps working.

Note: errors.As now unwraps to the first (header) probe error
rather than the second (params) one. This is intentional, as the
header error is typically the root cause.

Fixes golang#786
@gopherbot
Copy link
Copy Markdown
Contributor

This PR (HEAD: 6250ccf) has been imported to Gerrit for code review.

Please visit Gerrit at https://go-review.googlesource.com/c/oauth2/+/769980.

Important tips:

  • Don't comment on this PR. All discussion takes place in Gerrit.
  • You need a Gmail or other Google account to log in to Gerrit.
  • To change your code in response to feedback:
    • Push a new commit to the branch used by your GitHub PR.
    • A new "patch set" will then appear in Gerrit.
    • Respond to each comment by marking as Done in Gerrit if implemented as suggested. You can alternatively write a reply.
    • Critical: you must click the blue Reply button near the top to publish your Gerrit responses.
    • Multiple commits in the PR will be squashed by GerritBot.
  • The title and description of the GitHub PR are used to construct the final commit message.
    • Edit these as needed via the GitHub web interface (not via Gerrit or git).
    • You should word wrap the PR description at ~76 characters unless you need longer lines (e.g., for tables or URLs).
  • See the Sending a change via GitHub and Reviews sections of the Contribution Guide as well as the FAQ for details.

@gopherbot
Copy link
Copy Markdown
Contributor

Message from Gopher Robot:

Patch Set 1:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/769980.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Copy Markdown
Contributor

Message from Gopher Robot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
A maintainer will review your change and provide feedback. See
https://go.dev/doc/contribute#review for more info and tips to get your
patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.


Please don’t reply on this GitHub thread. Visit golang.org/cl/769980.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Copy Markdown
Contributor

Message from Kenta Ishizaki:

Patch Set 2:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/769980.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Copy Markdown
Contributor

Message from Kenta Ishizaki:

Patch Set 2:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/769980.
After addressing review feedback, remember to publish your drafts!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Swallowed exception for header attempt if auth style is unknown

2 participants