Skip to content

[ATL-763] [LUM-1951] fix(ios): set max-age on session cookie so it survives cold reopen #32529

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.

Sign up for GitHub

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

Merged
clopen-set merged 1 commit into from
May 29, 2026
Merged

[ATL-763] [LUM-1951] fix(ios): set max-age on session cookie so it survives cold reopen #32529

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion apps/web/src/runtime/native-auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,10 @@ export async function waitForNativeSessionCookie(): Promise<void> {
* same code works across environments without runtime host sniffing.
*/
export function installSessionCookies(sessionToken: string): void {
const cookieAttrs = "path=/; domain=.vellum.ai; secure; samesite=lax";
// `max-age` makes the cookie persistent. If unspecified, the cookie
// expires at the end of the session, and users will be required to
// login again.
const cookieAttrs = "path=/; domain=.vellum.ai; secure; samesite=lax; max-age=1209600";

@chatgpt-codex-connector chatgpt-codex-connector Bot May 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Clear the persistent session cookie on logout failures

This makes the JS-installed native session cookie survive cold restarts, but the web logout path only calls allauthLogout() best-effort and then clears local state in finally (apps/web/src/stores/auth-store.ts:243-251), even if the DELETE fails/returns non-ok and therefore no server Set-Cookie expiry is applied. In that offline/server-error scenario an iOS user appears signed out and is redirected to login, but after reopening the app the now-persistent valid cookie logs them back in; either explicitly expire these cookie names during logout cleanup or avoid persisting them without a matching client-side clear path.

Useful? React with 👍 / 👎.

@clopen-set clopen-set May 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor edge case that I'm not gonna address here

@clopen-set clopen-set May 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like that we're manually reinstalling cookies. Why not just use X-Session-Token for auth, like with macOS?

As is, this requires us to keep all of these attributes in sync with the actual cookie, but we're not doing properly right now:

  • domain is wrong. This should be same-host and not be made available on all subdomains
  • max-age also needs to be kept in sync with backend in case we tweak default expiry

@clopen-set clopen-set May 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @noanflaherty in case this has implications for electron

document.cookie = `sessionid=${sessionToken}; ${cookieAttrs}`;
document.cookie = `__Secure-sessionid=${sessionToken}; ${cookieAttrs}`;
Comment on lines 187 to 188

@clopen-set clopen-set May 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should only set one of these cookies, not both. Public SPA is served over HTTPS and should use the __Secure prefix

}
Expand Down