From 9b73fc732b1cf6b1d4e214c9499d9e11ce7eb915 Mon Sep 17 00:00:00 2001
From: Omkar Bhad <60899563+omkarbhad@users.noreply.github.com>
Date: Sat, 9 May 2026 09:37:49 -0400
Subject: [PATCH] fix(auth): logout clears guest cookie too
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Guests had no working logout. The client called GET /api/auth/logout
which routes to magnova-auth's federated signout — guests have no
magnova session, so they bounced through an external provider that
returned indeterminate state and landed back on graphini with their
graphini_guest_id cookie still set. validateSessionOrGuest then
re-created the same guest user on the next request, making logout a
no-op.

Two changes:

1. POST /api/auth/logout now clears both graphini_session AND
   graphini_guest_id (via the existing clearGuestCookieHeader helper).
   Idempotent — either or both may be present; clearing both is safe.
   GET still serves federated signout for OAuth users who specifically
   want their upstream session terminated.

2. authStore.logout() POSTs instead of GETs. The POST response carries
   the Set-Cookie that actually removes the cookies; we then
   window.location.href = '/' to force a clean reload. The previous
   client-side `document.cookie = 'graphini_session=; Path=/; Max-Age=0'`
   was redundant (server clears it) and never touched the guest cookie
   (which is HttpOnly, not deletable from JS anyway).

The federated-signout GET path stays for completeness; nothing in the
current UI calls it. If we want OAuth users to also terminate their
magnova session at logout, a separate authStore.federatedLogout() can
be added later.

Reuses clearGuestCookieHeader and clearLocalSessionCookie from
$lib/server/auth — no new server primitives.
---
 src/lib/client/stores/auth.svelte.ts  | 25 +++++++++++++++++++++----
 src/routes/api/auth/logout/+server.ts | 22 ++++++++++++----------
 2 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/src/lib/client/stores/auth.svelte.ts b/src/lib/client/stores/auth.svelte.ts
index a459b2d..a623949 100644
--- a/src/lib/client/stores/auth.svelte.ts
+++ b/src/lib/client/stores/auth.svelte.ts
@@ -187,14 +187,31 @@ async function register(
   }
 }
 
-function logout(): void {
+async function logout(): Promise<void> {
   state.user = null;
   state.credits = null;
   saveCachedAuth(null, null);
   setActiveUserId(null);
-  // Clear local session cookie by setting expired
-  document.cookie = 'graphini_session=; Path=/; Max-Age=0';
-  window.location.href = '/api/auth/logout';
+
+  // POST clears both graphini_session and graphini_guest_id server-side, then
+  // 302s to /. We don't follow that redirect via fetch (browsers won't apply
+  // its Set-Cookie to the document anyway when fetch follows redirects), so
+  // we read the response and force-navigate to / ourselves below. The
+  // Set-Cookie on the POST response is what actually removes the cookies.
+  //
+  // The earlier `window.location.href = '/api/auth/logout'` issued a GET,
+  // which routes to magnova-auth's federated signout — wrong for guests
+  // (they have no magnova session) and unnecessary for password users.
+  try {
+    await fetch('/api/auth/logout', {
+      method: 'POST',
+      credentials: 'include',
+      redirect: 'manual'
+    });
+  } catch {
+    /* non-fatal — client state is already cleared and we force-reload below. */
+  }
+  window.location.href = '/';
 }
 
 async function refreshCredits(): Promise<void> {
diff --git a/src/routes/api/auth/logout/+server.ts b/src/routes/api/auth/logout/+server.ts
index 2d6ef7f..c9d8e91 100644
--- a/src/routes/api/auth/logout/+server.ts
+++ b/src/routes/api/auth/logout/+server.ts
@@ -1,20 +1,22 @@
 import { redirect } from '@sveltejs/kit';
 import type { RequestHandler } from './$types';
-import { clearLocalSessionCookie, getSignoutUrl } from '$lib/server/auth';
+import { clearGuestCookieHeader, clearLocalSessionCookie, getSignoutUrl } from '$lib/server/auth';
 
 export const GET: RequestHandler = async () => {
-  // Clear local session cookie, then redirect to magnova-auth signout
+  // Federated (magnova-auth) signout flow. Reserved for OAuth users who
+  // want their upstream session terminated too. Local clients should use
+  // POST so guests and password-login users don't get bounced through an
+  // external auth provider that has no session for them.
   throw redirect(302, getSignoutUrl());
 };
 
 export const POST: RequestHandler = async ({ url }) => {
-  // Local logout — clear graphini_session cookie and redirect to home
+  // Local logout — clears both the password-session cookie and the guest
+  // cookie. Either may be present (or both, if a guest was migrated into
+  // a real account mid-session); clearing both is idempotent.
   const secureCookie = url.protocol === 'https:';
-  return new Response(null, {
-    status: 302,
-    headers: {
-      'Set-Cookie': clearLocalSessionCookie(secureCookie),
-      Location: '/'
-    }
-  });
+  const headers = new Headers({ Location: '/' });
+  headers.append('Set-Cookie', clearLocalSessionCookie(secureCookie));
+  headers.append('Set-Cookie', clearGuestCookieHeader(secureCookie));
+  return new Response(null, { status: 302, headers });
 };