fix(expo): Sanitize stray hash fragment in SSO callback URL - #9272
fix(expo): Sanitize stray hash fragment in SSO callback URL#9272wobsoriano wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: b0842c7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
📝 WalkthroughWalkthroughAdds a shared callback URL parameter helper that removes stray or encoded hash fragments and handles invalid or missing parameters. Updates SSO, hosted authentication, and OAuth flows to use it for callback values, including Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Review ran into problems🔥 ProblemsLinked repositories: Couldn't analyze
Errors logged to '/home/jailuser/git/.git/lfs/logs/20260729T181243.864989527.log'. Errors logged to '/home/jailuser/git/.git/lfs/logs/20260729T181403.276019622.log'. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/expo/src/hooks/useSSO.ts (1)
110-110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a hook-level regression test for nonce normalization.
The helper tests cover parsing, but they do not verify that
useSSOpasses the sanitized nonce tosignIn.reload. Add a test inpackages/expo/src/hooks/__tests__/useSSO.test.tscovering literal and encoded trailing#values and assertingrotatingTokenNonce: 'abc123'.As per coding guidelines, tests should cover new functionality, error handling, and edge cases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/expo/src/hooks/useSSO.ts` at line 110, Add a hook-level regression test in useSSO tests that exercises the sign-in flow with both literal and URL-encoded trailing # nonce values, then assert signIn.reload receives rotatingTokenNonce: 'abc123'.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/expo/src/hooks/useSSO.ts`:
- Line 110: Add a hook-level regression test in useSSO tests that exercises the
sign-in flow with both literal and URL-encoded trailing # nonce values, then
assert signIn.reload receives rotatingTokenNonce: 'abc123'.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: b9d360f3-c5be-4b18-868a-96298e535428
📒 Files selected for processing (4)
.changeset/expo-callback-nonce-hash.mdpackages/expo/src/hooks/useSSO.tspackages/expo/src/utils/__tests__/authSessionCallback.test.tspackages/expo/src/utils/authSessionCallback.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/cli(auto-detected)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/expo/src/hooks/useOAuth.ts`:
- Around line 102-103: Update the OAuth callback flow around
getAuthSessionCallbackParam and signIn.reload to reject a null or otherwise
missing rotating token nonce before invoking reload. Remove the empty-string
fallback, fail locally using the existing error-handling behavior used for
invalid auth callback data, and only pass a validated nonce to signIn.reload.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 0a12bfbe-65d8-4461-a000-9c9267333518
📒 Files selected for processing (2)
packages/expo/src/hooks/useHostedAuth.tspackages/expo/src/hooks/useOAuth.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/cli(auto-detected)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)
| const rotatingTokenNonce = getAuthSessionCallbackParam(url, 'rotating_token_nonce') || ''; | ||
| await signIn.reload({ rotatingTokenNonce }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject missing or invalid nonces before reloading sign-in.
getAuthSessionCallbackParam returns null for malformed URLs or absent parameters, but || '' causes Line 103 to call signIn.reload({ rotatingTokenNonce: '' }). Fail locally, as hosted auth does, instead of forwarding an invalid nonce to FAPI.
Proposed fix
- const rotatingTokenNonce = getAuthSessionCallbackParam(url, 'rotating_token_nonce') || '';
+ const rotatingTokenNonce = getAuthSessionCallbackParam(url, 'rotating_token_nonce');
+ if (!rotatingTokenNonce) {
+ return errorThrower.throw(
+ 'OAuth callback did not include a rotating token nonce. Please restart the OAuth flow.',
+ );
+ }
await signIn.reload({ rotatingTokenNonce });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const rotatingTokenNonce = getAuthSessionCallbackParam(url, 'rotating_token_nonce') || ''; | |
| await signIn.reload({ rotatingTokenNonce }); | |
| const rotatingTokenNonce = getAuthSessionCallbackParam(url, 'rotating_token_nonce'); | |
| if (!rotatingTokenNonce) { | |
| return errorThrower.throw( | |
| 'OAuth callback did not include a rotating token nonce. Please restart the OAuth flow.', | |
| ); | |
| } | |
| await signIn.reload({ rotatingTokenNonce }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/expo/src/hooks/useOAuth.ts` around lines 102 - 103, Update the OAuth
callback flow around getAuthSessionCallbackParam and signIn.reload to reject a
null or otherwise missing rotating token nonce before invoking reload. Remove
the empty-string fallback, fail locally using the existing error-handling
behavior used for invalid auth callback data, and only pass a validated nonce to
signIn.reload.
2 participants
Description
Native SSO sign-ins can fail with a 401 signed_out at the sign-in reload when the OAuth callback deep link carries a stray
#. Some iOS redirect chains leave a fragment on the redirect URL, either literal or percent-encoded into therotating_token_noncevalue, so the nonce reaches FAPI as<nonce>#and the exact-match check fails.useSSOnow extracts the nonce through a helper that strips fragments from the callback URL and trims a # and anything after it from the extracted value.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change