Task #54: Make staying signed in reliable across all sign-in points
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
This commit is contained in:
@@ -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<void> {
|
||||||
|
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<void> {
|
||||||
|
return new Promise((resolve, reject) => {
|
||||||
|
req.session.destroy((err) => (err ? reject(err) : resolve()));
|
||||||
|
});
|
||||||
|
}
|
||||||
@@ -11,6 +11,7 @@ import {
|
|||||||
passwordResetTokensTable,
|
passwordResetTokensTable,
|
||||||
} from "@workspace/db";
|
} from "@workspace/db";
|
||||||
import { requireAuth, requireAdmin, getUserRoles } from "../middlewares/auth";
|
import { requireAuth, requireAdmin, getUserRoles } from "../middlewares/auth";
|
||||||
|
import { saveSession, destroySession } from "../lib/session";
|
||||||
import {
|
import {
|
||||||
RegisterBody,
|
RegisterBody,
|
||||||
LoginBody,
|
LoginBody,
|
||||||
@@ -92,9 +93,7 @@ router.post("/auth/register", async (req, res): Promise<void> => {
|
|||||||
}
|
}
|
||||||
|
|
||||||
req.session.userId = user.id;
|
req.session.userId = user.id;
|
||||||
await new Promise<void>((resolve, reject) => {
|
await saveSession(req);
|
||||||
req.session.save((err) => (err ? reject(err) : resolve()));
|
|
||||||
});
|
|
||||||
const roles = await getUserRoles(user.id);
|
const roles = await getUserRoles(user.id);
|
||||||
res.status(201).json(buildAuthUser(user, roles));
|
res.status(201).json(buildAuthUser(user, roles));
|
||||||
});
|
});
|
||||||
@@ -130,9 +129,7 @@ router.post("/auth/login", async (req, res): Promise<void> => {
|
|||||||
}
|
}
|
||||||
|
|
||||||
req.session.userId = user.id;
|
req.session.userId = user.id;
|
||||||
await new Promise<void>((resolve, reject) => {
|
await saveSession(req);
|
||||||
req.session.save((err) => (err ? reject(err) : resolve()));
|
|
||||||
});
|
|
||||||
const roles = await getUserRoles(user.id);
|
const roles = await getUserRoles(user.id);
|
||||||
res.json(buildAuthUser(user, roles));
|
res.json(buildAuthUser(user, roles));
|
||||||
});
|
});
|
||||||
@@ -296,10 +293,9 @@ router.post(
|
|||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
router.post("/auth/logout", (req, res): void => {
|
router.post("/auth/logout", async (req, res): Promise<void> => {
|
||||||
req.session.destroy(() => {
|
await destroySession(req);
|
||||||
res.json({ success: true });
|
res.json({ success: true });
|
||||||
});
|
|
||||||
});
|
});
|
||||||
|
|
||||||
router.get("/auth/me", requireAuth, async (req, res): Promise<void> => {
|
router.get("/auth/me", requireAuth, async (req, res): Promise<void> => {
|
||||||
|
|||||||
@@ -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",
|
||||||
|
);
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user