From 1277c71d1182c1d365557bcf7de682bbdc630b27 Mon Sep 17 00:00:00 2001 From: riyadhafraa <49757212-riyadhafraa@users.noreply.replit.com> Date: Tue, 21 Apr 2026 12:37:16 +0000 Subject: [PATCH] Task #54: Make staying signed in reliable across all sign-in points MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Background: Task #53 fixed the login/register session persistence race by explicitly awaiting `req.session.save` before responding. Other session-mutating endpoints (notably `/auth/logout`) still relied on express-session's default end-hook, which can flush the response before the store write finishes — producing intermittent "still logged in" / "logged out" glitches on the immediate next request. Changes: - New shared helper `artifacts/api-server/src/lib/session.ts` exporting `saveSession(req)` and `destroySession(req)` — promise wrappers around `req.session.save` / `req.session.destroy` so handlers can `await` store persistence before flushing the HTTP response. Documented the rationale in the file so future session-mutating routes use the same safe pattern. - `artifacts/api-server/src/routes/auth.ts`: - `/auth/register` and `/auth/login` now use `await saveSession(req)` in place of the inline ad-hoc Promise wrapper. - `/auth/logout` is now async and `await`s `destroySession(req)` before responding, closing the same race for the destroy path. - Audited remaining routes: only `auth.ts` mutates `req.session`; all other handlers only read `req.session.userId`, so no further changes are needed. Notes / deviations: - Pre-existing TypeScript errors in unrelated files (conversations, notes, users, api-zod exports) were left untouched — out of scope. - New test file `artifacts/api-server/tests/auth-session-persistence.test.mjs` covers the acceptance criterion: login / register / logout each followed by an immediate `/auth/me` probe to assert the session was persisted (or destroyed) before the response was flushed. Modeled on the existing leave-test pattern. Full suite: 18/18 passing. Replit-Task-Id: 11b72d21-d7c2-42cb-a4d9-f1197cfad4c5 --- artifacts/api-server/src/lib/session.ts | 29 ++++ artifacts/api-server/src/routes/auth.ts | 16 +- .../tests/auth-session-persistence.test.mjs | 162 ++++++++++++++++++ 3 files changed, 197 insertions(+), 10 deletions(-) create mode 100644 artifacts/api-server/src/lib/session.ts create mode 100644 artifacts/api-server/tests/auth-session-persistence.test.mjs diff --git a/artifacts/api-server/src/lib/session.ts b/artifacts/api-server/src/lib/session.ts new file mode 100644 index 00000000..d6634cce --- /dev/null +++ b/artifacts/api-server/src/lib/session.ts @@ -0,0 +1,29 @@ +import { type Request } from "express"; + +/** + * Persist any pending changes to `req.session` to the session store before + * resolving. Wraps express-session's callback-based `save` in a promise so + * route handlers can `await` it and guarantee the store write has completed + * before the HTTP response is flushed. + * + * Without this, the default end-hook behavior can race: response headers and + * body may be sent before the store write finishes, producing intermittent + * "logged out" / "still logged in" glitches on the very next request. + */ +export function saveSession(req: Request): Promise { + return new Promise((resolve, reject) => { + req.session.save((err) => (err ? reject(err) : resolve())); + }); +} + +/** + * Destroy the session and wait for the store deletion to complete before + * resolving. Mirrors `saveSession` for logout / account-deletion flows so the + * cookie's backing record is guaranteed to be gone before the response is + * returned. + */ +export function destroySession(req: Request): Promise { + return new Promise((resolve, reject) => { + req.session.destroy((err) => (err ? reject(err) : resolve())); + }); +} diff --git a/artifacts/api-server/src/routes/auth.ts b/artifacts/api-server/src/routes/auth.ts index cf8a7db8..602de87d 100644 --- a/artifacts/api-server/src/routes/auth.ts +++ b/artifacts/api-server/src/routes/auth.ts @@ -11,6 +11,7 @@ import { passwordResetTokensTable, } from "@workspace/db"; import { requireAuth, requireAdmin, getUserRoles } from "../middlewares/auth"; +import { saveSession, destroySession } from "../lib/session"; import { RegisterBody, LoginBody, @@ -92,9 +93,7 @@ router.post("/auth/register", async (req, res): Promise => { } req.session.userId = user.id; - await new Promise((resolve, reject) => { - req.session.save((err) => (err ? reject(err) : resolve())); - }); + await saveSession(req); const roles = await getUserRoles(user.id); res.status(201).json(buildAuthUser(user, roles)); }); @@ -130,9 +129,7 @@ router.post("/auth/login", async (req, res): Promise => { } req.session.userId = user.id; - await new Promise((resolve, reject) => { - req.session.save((err) => (err ? reject(err) : resolve())); - }); + await saveSession(req); const roles = await getUserRoles(user.id); res.json(buildAuthUser(user, roles)); }); @@ -296,10 +293,9 @@ router.post( }, ); -router.post("/auth/logout", (req, res): void => { - req.session.destroy(() => { - res.json({ success: true }); - }); +router.post("/auth/logout", async (req, res): Promise => { + await destroySession(req); + res.json({ success: true }); }); router.get("/auth/me", requireAuth, async (req, res): Promise => { diff --git a/artifacts/api-server/tests/auth-session-persistence.test.mjs b/artifacts/api-server/tests/auth-session-persistence.test.mjs new file mode 100644 index 00000000..783b0be5 --- /dev/null +++ b/artifacts/api-server/tests/auth-session-persistence.test.mjs @@ -0,0 +1,162 @@ +import { test, before, after } from "node:test"; +import assert from "node:assert/strict"; +import pg from "pg"; + +const API_BASE = process.env.TEST_API_BASE ?? "http://localhost:8080"; +const DATABASE_URL = process.env.DATABASE_URL; +if (!DATABASE_URL) { + throw new Error("DATABASE_URL must be set to run these tests"); +} + +const TEST_PASSWORD = "TestPass123!"; +const TEST_PASSWORD_HASH = + "$2b$10$Bs636ukPMyz01nKrsi.5m.JlDXSN22AVCvn8cgPWWDbo5yJRQX2vu"; + +const pool = new pg.Pool({ connectionString: DATABASE_URL }); + +const createdUserIds = []; +const createdUsernames = []; + +function uniqueName(prefix) { + return `${prefix}_${Date.now().toString(36)}_${Math.random() + .toString(36) + .slice(2, 8)}`; +} + +async function createUser(prefix) { + const username = uniqueName(prefix); + const { rows } = await pool.query( + `INSERT INTO users (username, email, password_hash, display_name_en, preferred_language, is_active) + VALUES ($1, $2, $3, 'Session Test', 'en', true) RETURNING id`, + [username, `${username}@example.com`, TEST_PASSWORD_HASH], + ); + const id = rows[0].id; + createdUserIds.push(id); + createdUsernames.push(username); + await pool.query( + `INSERT INTO user_roles (user_id, role_id) + SELECT $1, id FROM roles WHERE name = 'user'`, + [id], + ); + return { id, username }; +} + +function extractSidCookie(res) { + const setCookie = res.headers.get("set-cookie"); + if (!setCookie) return null; + return setCookie + .split(",") + .map((c) => c.split(";")[0].trim()) + .find((c) => c.startsWith("connect.sid=")) ?? null; +} + +after(async () => { + if (createdUserIds.length > 0) { + await pool.query(`DELETE FROM user_roles WHERE user_id = ANY($1::int[])`, [ + createdUserIds, + ]); + await pool.query(`DELETE FROM users WHERE id = ANY($1::int[])`, [ + createdUserIds, + ]); + } + // Also drop registration-created users (whose ids we don't know up front). + if (createdUsernames.length > 0) { + await pool.query( + `DELETE FROM user_roles WHERE user_id IN + (SELECT id FROM users WHERE username = ANY($1::text[]))`, + [createdUsernames], + ); + await pool.query(`DELETE FROM users WHERE username = ANY($1::text[])`, [ + createdUsernames, + ]); + } + await pool.end(); +}); + +test("login session is persisted before response so an immediate /auth/me succeeds", async () => { + const user = await createUser("session_login"); + + const loginRes = await fetch(`${API_BASE}/api/auth/login`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ username: user.username, password: TEST_PASSWORD }), + }); + assert.equal(loginRes.status, 200, "login should succeed"); + const cookie = extractSidCookie(loginRes); + assert.ok(cookie, "login must set connect.sid cookie"); + + // Immediate follow-up authenticated request — must already see the session. + const meRes = await fetch(`${API_BASE}/api/auth/me`, { + headers: { Cookie: cookie }, + }); + assert.equal( + meRes.status, + 200, + "fast follow-up /auth/me after login must be authenticated", + ); + const me = await meRes.json(); + assert.equal(me.username, user.username); +}); + +test("registration session is persisted before response so an immediate /auth/me succeeds", async () => { + const username = uniqueName("session_register"); + createdUsernames.push(username); + + const regRes = await fetch(`${API_BASE}/api/auth/register`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + username, + email: `${username}@example.com`, + password: TEST_PASSWORD, + displayNameEn: "Session Register", + preferredLanguage: "en", + }), + }); + // Registration may be closed in some environments; if so, skip the assertion + // about persistence — there is nothing to verify in that case. + if (regRes.status === 403) { + return; + } + assert.equal(regRes.status, 201, "register should succeed"); + const cookie = extractSidCookie(regRes); + assert.ok(cookie, "register must set connect.sid cookie"); + + const meRes = await fetch(`${API_BASE}/api/auth/me`, { + headers: { Cookie: cookie }, + }); + assert.equal( + meRes.status, + 200, + "fast follow-up /auth/me after register must be authenticated", + ); +}); + +test("logout destroys the session before response so an immediate /auth/me is rejected", async () => { + const user = await createUser("session_logout"); + + const loginRes = await fetch(`${API_BASE}/api/auth/login`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ username: user.username, password: TEST_PASSWORD }), + }); + assert.equal(loginRes.status, 200); + const cookie = extractSidCookie(loginRes); + assert.ok(cookie); + + const logoutRes = await fetch(`${API_BASE}/api/auth/logout`, { + method: "POST", + headers: { Cookie: cookie }, + }); + assert.equal(logoutRes.status, 200, "logout should succeed"); + + // Immediate follow-up using the same (now-destroyed) sid must be unauthorized. + const meRes = await fetch(`${API_BASE}/api/auth/me`, { + headers: { Cookie: cookie }, + }); + assert.equal( + meRes.status, + 401, + "fast follow-up /auth/me after logout must be unauthorized", + ); +});