fix: Safari mobile auth - change sameSite to none for cross-site OAuth - #116
fix: Safari mobile auth - change sameSite to none for cross-site OAuth#116adrianhajdin wants to merge 1 commit into
Conversation
Fixes #247 Safari on iOS 17+ blocks cookies with sameSite='strict' during OAuth redirects from external providers (Google). This causes the session cookie to be dropped, resulting in a redirect loop back to /sign-in. Changes: - Update cookie sameSite from 'strict' to 'none' in signIn() - Update cookie sameSite from 'strict' to 'none' in signUp() The cookies already use httpOnly=true and secure=true, making sameSite=none safe for cross-site authentication flows.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe pull request modifies cookie security settings in user authentication actions. The sameSite attribute for the appwrite-session cookie is changed from "strict" to "none" in two locations where cookies are set during sign-in and sign-up operations. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/actions/user.actions.ts`:
- Around line 95-100: Update the cookie's SameSite setting from "none" to "lax"
in the cookies().set call that sets the "appwrite-session" cookie; locate the
cookies().set(...) invocation in lib/actions/user.actions.ts (the block that
sets path, httpOnly, sameSite, secure) and change sameSite: "none" to sameSite:
"lax" so it matches the signIn behavior and maintains consistent cookie policy.
- Around line 40-45: Update the cookie SameSite policy in the session setter to
use "lax" instead of "none": locate the cookies().set(...) call in
user.actions.ts (the session creation/assignment logic where session.secret is
stored) and change sameSite from "none" to "lax" while preserving path,
httpOnly, and secure flags so OAuth top-level redirects still work but
cross-site subrequests are blocked.
| cookies().set("appwrite-session", session.secret, { | ||
| path: "/", | ||
| httpOnly: true, | ||
| sameSite: "strict", | ||
| sameSite: "none", | ||
| secure: true, | ||
| }); |
There was a problem hiding this comment.
Prefer sameSite: "lax" over "none" — it fixes Safari OAuth redirects without opening up CSRF surface.
sameSite: "none" causes the browser to attach the session cookie to every cross-site request (iframes, fetch, form POSTs from third-party pages, etc.), which is unnecessary exposure for a banking app. OAuth redirect flows are top-level navigations, and sameSite: "lax" already allows cookies on those while still blocking cross-origin subrequests.
"lax" is the recommended setting for exactly this scenario — surviving OAuth/SSO redirects without degrading CSRF protection.
Proposed fix
cookies().set("appwrite-session", session.secret, {
path: "/",
httpOnly: true,
- sameSite: "none",
+ sameSite: "lax",
secure: true,
});📝 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.
| cookies().set("appwrite-session", session.secret, { | |
| path: "/", | |
| httpOnly: true, | |
| sameSite: "strict", | |
| sameSite: "none", | |
| secure: true, | |
| }); | |
| cookies().set("appwrite-session", session.secret, { | |
| path: "/", | |
| httpOnly: true, | |
| sameSite: "lax", | |
| secure: true, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/actions/user.actions.ts` around lines 40 - 45, Update the cookie SameSite
policy in the session setter to use "lax" instead of "none": locate the
cookies().set(...) call in user.actions.ts (the session creation/assignment
logic where session.secret is stored) and change sameSite from "none" to "lax"
while preserving path, httpOnly, and secure flags so OAuth top-level redirects
still work but cross-site subrequests are blocked.
| cookies().set("appwrite-session", session.secret, { | ||
| path: "/", | ||
| httpOnly: true, | ||
| sameSite: "strict", | ||
| sameSite: "none", | ||
| secure: true, | ||
| }); |
There was a problem hiding this comment.
Same sameSite concern as in signIn — use "lax" here too.
Apply the same change as suggested for signIn above.
Proposed fix
cookies().set("appwrite-session", session.secret, {
path: "/",
httpOnly: true,
- sameSite: "none",
+ sameSite: "lax",
secure: true,
});📝 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.
| cookies().set("appwrite-session", session.secret, { | |
| path: "/", | |
| httpOnly: true, | |
| sameSite: "strict", | |
| sameSite: "none", | |
| secure: true, | |
| }); | |
| cookies().set("appwrite-session", session.secret, { | |
| path: "/", | |
| httpOnly: true, | |
| sameSite: "lax", | |
| secure: true, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/actions/user.actions.ts` around lines 95 - 100, Update the cookie's
SameSite setting from "none" to "lax" in the cookies().set call that sets the
"appwrite-session" cookie; locate the cookies().set(...) invocation in
lib/actions/user.actions.ts (the block that sets path, httpOnly, sameSite,
secure) and change sameSite: "none" to sameSite: "lax" so it matches the signIn
behavior and maintains consistent cookie policy.
1 participant
Fixes #247
Problem
Safari on iOS 17+ blocks cookies with
sameSite="strict"during OAuth redirects from external providers (Google). This causes the session cookie to be dropped, resulting in users being redirected back to/sign-inin a loop after completing authentication.Solution
Changed
sameSitefrom"strict"to"none"in bothsignIn()andsignUp()functions.Safety
httpOnly=trueandsecure=truesameSite="none"is safe when paired withsecure=trueTesting
Should be tested on:
/dashboardafter authcc @NicholasDev
Summary by CodeRabbit