fix: repair sanitized redirect_uri test and prettier formatting#2
Conversation
The new test was missing storage setup (state + codeVerifier) and a fetchMock response, causing exchangeAuthCode to bail out early before reaching fetch. Also fixes prettier formatting in types.ts and the test file. https://claude.ai/code/session_01MfwF59xe6Rs5kyxnQuPRAR
This file was never tracked in the project and is generated locally by npm install. https://claude.ai/code/session_01MfwF59xe6Rs5kyxnQuPRAR
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44dd08ca95
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| code_verifier: codeVerifier, | ||
| grant_type: "authorization_code", | ||
| redirect_uri: redirectURL, | ||
| redirect_uri: sanitizeUrl(redirectURL), |
There was a problem hiding this comment.
When callers use generateAuthUrl(..., { disableUrlSanitization: true }), the authorize request deliberately sends the raw redirectURL via mapLoginMethodParamsForUrl (for example preserving a trailing slash). This new unconditional sanitizeUrl(redirectURL) changes the value sent to /oauth2/token, so any flow that opted out of sanitization or relies on an exact trailing/double-slash redirect URI will exchange a different redirect_uri than the one used at authorization time and be rejected by the provider. The exchange path needs the same sanitization decision/value that was used to build the authorize URL.
Useful? React with 👍 / 👎.
terfex
changed the base branch from
main
to
fix/exchange-auth-code-sanitize-redirect-uri
When callers pass disableUrlSanitization: true to generateAuthUrl the authorize request sends the raw redirectURL. The token exchange must use the same value; unconditionally calling sanitizeUrl() would produce a mismatched redirect_uri and cause the provider to reject the exchange. Adds the disableUrlSanitization option (default false) to ExchangeAuthCodeParams and mirrors the same conditional used in mapLoginMethodParamsForUrl. Also adds a test covering the raw-URI path. https://claude.ai/code/session_01MfwF59xe6Rs5kyxnQuPRAR
|
@codex please check |
|
To use Codex here, create an environment for this repo. |
2 participants
$(cat <<'EOF'
Summary
sends a sanitized redirect_uri so /token matches the /authorize valuetest inlib/utils/exchangeAuthCode.test.tslib/sessionManager/types.tsandlib/utils/exchangeAuthCode.test.tsRoot Cause
The new test was missing two things required before
exchangeAuthCodereaches thefetchcall:MemoryStoragewith matchingstateandcodeVerifiersession items — without these the function returns early with an "Invalid state" errorfetchMockresponse — without it the mock call list is empty, causing the destructureconst [, requestInit] = ...to throwTypeError: undefined is not iterableIt also used
(global.fetch as jest.Mock)inconsistently with the surrounding tests that usefetchMock, and tried to parse the body as a string whenexchangeAuthCodepasses aURLSearchParamsobject directly.Changes
lib/utils/exchangeAuthCode.test.ts: AddedMemoryStorage+setActiveStoragesetup,fetchMock.mockResponseOnce, and corrected body access pattern to match the rest of the test suitelib/sessionManager/types.ts: Prettier reformatting of theSessionBaseclass generic constraintTest plan
npm run lint— passes (no prettier or eslint errors)npm test -- --run— 619 passed, 20 skipped, 0 failedhttps://claude.ai/code/session_01MfwF59xe6Rs5kyxnQuPRAR
EOF
)
Generated by Claude Code