fix: fetch Crisp support token from the API instead of deriving it client-side - #2666
Conversation
…ient-side The token was computed in the browser as SHA-256 of a salt that ships in the client bundle plus the userId, both public. Anyone could reproduce any user's token and open their support conversation to read it and post as them. Fetch it from the new authenticated GET /user/crisp-token instead, which derives it server-side with a secret the browser never sees and issues it only to the authenticated caller. Same undefined-until-resolved contract and cache, so SupportDrawer's token gate is unchanged. Requires the peanut-api-ts endpoint to be deployed first.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe ChangesCrisp token retrieval
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Auth
participant useCrispTokenId
participant CrispTokenAPI
Auth->>useCrispTokenId: provide authenticated user
useCrispTokenId->>CrispTokenAPI: request /user/crisp-token
CrispTokenAPI-->>useCrispTokenId: return user ID and token type
useCrispTokenId->>useCrispTokenId: validate and cache token
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code-analysis diffPainscore total: 7137.94 → 7138.12 (+0.18) 🆕 New findings (1)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
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 `@src/hooks/useCrispTokenId.ts`:
- Around line 48-61: Update src/hooks/useCrispTokenId.ts lines 48-61 to
associate cached tokens with their owning userId, clear token state before
uncached requests and on invalid or failed responses, and return undefined
unless the stored token belongs to the current userId. Update
src/hooks/__tests__/useCrispTokenId.test.ts lines 29-49 by adding a rerender
scenario that switches users while the new token request is pending, failed, or
invalid, asserting undefined until the new user’s token resolves.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ec15b4aa-9825-4dd5-af90-44e64cb264e0
📒 Files selected for processing (2)
src/hooks/__tests__/useCrispTokenId.test.tssrc/hooks/useCrispTokenId.ts
…sient failures Address code review of the client hook: - Reset the token to the current user's cached value (or undefined) before fetching, so an account switch never serves the previous user's token while the new one loads (cross-user exposure window). - Verify the userId the route echoes matches the account we fetch for, so a stale auth bearer can't bind the widget to another user. - Retry the fetch a few times, so a transient blip doesn't strand the support drawer's loading gate for the whole session.
|
@coderabbitai review |
|
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 `@src/hooks/__tests__/useCrispTokenId.test.ts`:
- Around line 83-84: Update the retry test around the waitFor assertion to wait
for the configured three-request sequence to complete, rather than proceeding
once the second request starts. Use controlled timers or assert exactly three
API calls, then verify result.current remains undefined.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bee65b13-bfb6-413a-8fbb-470ba1e48bb1
📒 Files selected for processing (2)
src/hooks/__tests__/useCrispTokenId.test.tssrc/hooks/useCrispTokenId.ts
| await waitFor(() => expect(apiFetchMock.mock.calls.length).toBeGreaterThan(1), { timeout: 3000 }) | ||
| expect(result.current).toBeUndefined() |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Wait for the configured retry sequence to finish.
Line 83 passes after the second request starts. The test does not verify that all three attempts complete before the hook remains undefined.
Use controlled timers or wait for exactly three calls. Then assert that no token is returned.
🤖 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 `@src/hooks/__tests__/useCrispTokenId.test.ts` around lines 83 - 84, Update the
retry test around the waitFor assertion to wait for the configured three-request
sequence to complete, rather than proceeding once the second request starts. Use
controlled timers or assert exactly three API calls, then verify result.current
remains undefined.
Summary
Frontend half of the Crisp support-token IDOR hotfix. Pairs with peanut-api-ts #1325 — that API PR (and its migration) must deploy first.
The Crisp session token was derived in the browser as
SHA-256("peanut-crisp-session-v1:" + userId). The salt ships in the bundle anduserIdis readable, so anyone could reproduce any user's token and open their support thread via/crisp-proxy?crisp_token_id=<token>.This PR removes the client-side derivation.
useCrispTokenIdnow fetches the token from the authenticatedGET /user/crisp-token(the API issues a stored per-user random token — Crisp's documented safe pattern). The hook keeps the same contract —undefineduntil it resolves, in-memory cache per userId — soSupportDrawer'sisAwaitingTokengate and reset logic are unchanged.Hardening from code review is included:
userId; the hook discards any token whoseuserIddoesn't match the account it is fetching for (stale-bearer guard).Task
TASK link pending — user security report. Will attach.
Risks / breaking changes
GET /user/crisp-token404s and support chat stays on its loading state.QA
src/hooks/__tests__/useCrispTokenId.test.ts): fetches from the authed endpoint, passes no userId parameter, discards a token minted for a different user, drops the previous user's token on account switch, and stays undefined (with retry) on failure.SupportDrawersuite still green (it mocks the hook).Screenshots
N/A (no visible change) — the drawer renders identically; only the token's source changed.
Summary by CodeRabbit
New Features
Tests