feat(console): add AUTH_COOKIE_DOMAIN to scope the Firebase auth cookie#1349
feat(console): add AUTH_COOKIE_DOMAIN to scope the Firebase auth cookie#1349vklimontovich wants to merge 4 commits into
Conversation
The jitsu-auth session cookie is scoped to the request host, so sibling subdomains (e.g. a marketing site on the apex domain) can't read the logged-in session. Add an optional AUTH_COOKIE_DOMAIN env var to widen the cookie to a parent domain; unset keeps the current host-only behaviour. Applied to both the set (create-session) and clear (revoke-session) paths so logout still works.
![jitsu-code-review[bot]](https://avatars.githubusercontent.com/in/3391383?s=60&v=4){kind=small}
| const domain = getRequestHost(req).split(":")[0]; | ||
| // Host-only by default; AUTH_COOKIE_DOMAIN can widen it to a parent domain | ||
| // so sibling subdomains share the session. | ||
| const domain = getAuthCookieDomain(req); |
There was a problem hiding this comment.
Potential correctness issue: this path is described as "host-only by default", but we always send a domain attribute (domain is always set in options). Per cookie semantics, host-only cookies require omitting Domain entirely. As written, default behavior is a domain cookie for the current host (and can include subdomains). If host-only is intended, can we make domain optional and only set it when AUTH_COOKIE_DOMAIN is configured?
| httpOnly: true, | ||
| secure, | ||
| // Must match the domain used when the cookie was set, or it won't clear. | ||
| domain: getAuthCookieDomain(req), |
There was a problem hiding this comment.
Possible logout bug: delete uses matching domain now, but it still omits path: "/". The session cookie is created with path: "/" in create-session.ts; clearing with a different/default path may fail to remove the real auth cookie. Can we set path: "/" here too so name+domain+path all match?
Address review on #1349: - omit the Domain attribute unless AUTH_COOKIE_DOMAIN is set (a true host-only cookie must not send Domain), so getAuthCookieDomain returns string | undefined - clear the cookie with path "/" in revoke-session so name+path+domain all match create-session and logout actually removes it
There was a problem hiding this comment.
Reviewed the auth-cookie domain changes across create-session, revoke-session, and server env wiring. The domain/path matching logic now looks consistent for set/clear. I left two inline comments for follow-up: one security concern (session token value logged) and one correctness/maintainability issue (duplicate AUTH_COOKIE_DOMAIN schema key).
| const domain = getAuthCookieDomain(); | ||
| getLog() | ||
| .atDebug() | ||
| .log(`Setting firebase auth cookie (domain: ${domain ?? "host-only"}): ${cookie}`); |
There was a problem hiding this comment.
Potential security issue: this debug statement logs the full Firebase session cookie value (${cookie}), which is a bearer credential. If debug logs are enabled or exported, this can leak active sessions. Can we avoid logging the raw token and log only non-sensitive metadata instead?
| // request host (e.g. "use.jitsu.com"). Set this to a parent domain (e.g. | ||
| // "jitsu.com") to share the session across sibling subdomains — for example so a | ||
| // marketing site can read the logged-in user. Leave unset for host-only scope. | ||
| AUTH_COOKIE_DOMAIN: z.string().optional(), |
There was a problem hiding this comment.
AUTH_COOKIE_DOMAIN is already declared earlier in the same schema (line 112). Duplicating object keys means the later entry silently overrides the earlier one, which can cause comment/schema drift. Can we keep a single declaration and one canonical description?
AUTH_COOKIE_DOMAIN is already declared in serverEnv.ts (used by nextauth); this PR only needs to wire it into the firebase auth cookie. Remove the duplicate that broke the build (TS1117).
| const domain = getAuthCookieDomain(); | ||
| getLog() | ||
| .atDebug() | ||
| .log(`Setting firebase auth cookie (domain: ${domain ?? "host-only"}): ${cookie}`); |
There was a problem hiding this comment.
Security issue: this logs the full Firebase session cookie token ($cookie), which is a bearer credential. Even in debug level, this can leak active sessions via log aggregation or support dumps. Can we remove the token from logs and keep only non-sensitive metadata (e.g., domain + expiry)?
The debug statement logged the full session-cookie value, a bearer credential that could leak active sessions via log aggregation. Log only non-sensitive metadata (domain + expiry) and use the file's named logger.
There was a problem hiding this comment.
Reviewed the Firebase auth cookie changes in:\n- webapps/console/lib/server/firebase-server.ts\n- webapps/console/pages/api/fb-auth/create-session.ts\n- webapps/console/pages/api/fb-auth/revoke-session.ts\n\nChecked cookie domain/path matching on set+clear, host-only behavior when is unset, and token logging safety. I did not find new actionable correctness or security issues in this diff.
1 participant
What
The
jitsu-authFirebase session cookie is set withdomain = getRequestHost(req)— i.e. host-only (use.jitsu.com). Sibling subdomains on the same apex (e.g. a marketing site atjitsu.com) therefore never receive it and can't read the logged-in session server-side.This adds an optional
AUTH_COOKIE_DOMAINenv var. When set (e.g.jitsu.com), the auth cookie is scoped to that parent domain and is shared across its subdomains. Unset preserves the current host-only behaviour.Why
So a separate app on a sibling subdomain can resolve the logged-in user from the cookie (verify the session-cookie JWT) and, for example, attach identity to analytics — without a cross-origin
/api/meround-trip.Changes
AUTH_COOKIE_DOMAIN.getAuthCookieDomain(req)helper:AUTH_COOKIE_DOMAINif set, else the request host.Notes