Preserve Cloudflare OAuth token exchange errors - #325
Open
atsaplin wants to merge 1 commit into
Open
Conversation
Author
|
I have read the CLA Document and I hereby sign the CLA |
|
All contributors have signed the CLA ✍️ ✅ |
ndisidore
reviewed
Aug 25, 2026
| if (!resp.ok) { | ||
| resp.body?.cancel(); | ||
| return null; | ||
| const providerError: unknown = await resp.json().catch(() => null); |
Member
There was a problem hiding this comment.
redeem() is also used by refreshTokens(). Previously a rejected refresh returned null, allowing getAccessToken() to call credentialsExpired(). with this change it now throws so an expired/revoked refresh token will keep failing without prompting reconnection.
If you are able to fix this happy to merge!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #323
What does this change?
Cloudflare OAuth token redemption currently collapses every non-2xx response to
null. That makes invalid client credentials indistinguishable from a successful response that omitted an access token. This patch throws an error containing only the HTTP status and standard OAutherroranderror_descriptionfields, and adds a focused rejection-path test.Why is this obviously correct and trivially verifiable?
The change affects one non-2xx branch in the token exchange helper. The test supplies a 401
invalid_clientresponse and asserts the exact sanitized error. Credentials and token values are never read into the message. Successful response handling is unchanged.Verification
pnpm --filter @gadgets/cloudflare-gatekeeper test:runChecklist