-
Notifications
You must be signed in to change notification settings - Fork 392
fix(clerk-js): unset domain cookies #6953
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughUpdates session cookie management to compute and use a domain value when setting and removing cookies. Removal now clears both with-domain and without-domain variants for backward compatibility. Tests were updated to mock domain resolution and assert domain-aware set/remove sequences for cross-origin and same-origin contexts. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant SessionCookies as SessionCookies (module)
participant Domain as getCookieDomain()
participant Store as CookieStore
rect rgb(240,245,255)
note right of App: Set(token)
App->>SessionCookies: set(token, opts)
SessionCookies->>Domain: resolve domain
Domain-->>SessionCookies: domain
SessionCookies->>Store: remove(sessionCookie) (no domain)
SessionCookies->>Store: remove(suffixedSessionCookie) (no domain)
SessionCookies->>Store: set(sessionCookie, token, {domain, partitioned, sameSite})
SessionCookies->>Store: set(suffixedSessionCookie, token, {domain, partitioned, sameSite})
end
rect rgb(245,240,255)
note right of App: Remove()
App->>SessionCookies: remove(opts)
SessionCookies->>Domain: resolve domain
Domain-->>SessionCookies: domain
SessionCookies->>Store: remove(sessionCookie, {domain})
SessionCookies->>Store: remove(suffixedSessionCookie, {domain})
SessionCookies->>Store: remove(sessionCookie) (no domain)
SessionCookies->>Store: remove(suffixedSessionCookie) (no domain)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (9)**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
packages/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
packages/**/*.{ts,tsx,d.ts}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
packages/**/*.{test,spec}.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Files:
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Files:
**/*.{js,ts,tsx,jsx}📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Files:
**/__tests__/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Files:
🧬 Code graph analysis (2)packages/clerk-js/src/core/auth/cookies/__tests__/session.test.ts (1)
packages/clerk-js/src/core/auth/cookies/session.ts (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (8)
Comment |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
Description
Evaluating this change.. not ready for merge
Checklist
pnpm test
runs as expected.pnpm build
runs as expected.Type of change
Summary by CodeRabbit
New Features
Bug Fixes
Tests