security: implement server-side session management - #6
Open
isaJrKai wants to merge 1 commit into
Open
Conversation
Replaces JWT-only authentication with server-side sessions. JWTs now contain a sessionId that must exist in the Session table AND must not be revoked AND must not be expired. Changes: 1. Schema: add revokedAt + lastSeenAt columns to Session model 2. auth.ts: sessionId in JWT, createSessionRecord on every login, isSessionValid check in getAuthUser, revokeSession on logout, clearSessionCookie with matching RFC 6265 attributes, debounced lastSeenAt refresh 3. auth-edge.ts: sessionId in SessionPayload 4. logout route: revokes session by sessionId before clearing cookie 5. child-login route: creates session record 6. login page: redirects to dashboard if valid session exists 7. admin/security: preserves sessionId when re-signing JWT 8. admin/sessions (NEW): list + revoke sessions endpoint 9. api/state + api/mutations: add getAuthUser auth gates All login paths (parent, 2FA, founder setup, child) now create session records. Logout revokes only the current device session. Multi-device sessions are independent. BREAKING: All existing JWTs (without sessionId) are auto-rejected on deploy. Every user must log in again. This is expected and correct — the old JWTs were issued under the less-secure model. Migration: prisma/migrations/20260625000000_add_session_revocation.sql must be run against Neon BEFORE merging this PR.
📝 WalkthroughWalkthroughIntroduces server-side session revocation by adding ChangesServer-side Session Revocation
Sequence Diagram(s)sequenceDiagram
participant Browser
participant LoginRoute
participant createSessionRecord
participant DB
participant signSession
participant getAuthUser
participant isSessionValid
Browser->>LoginRoute: POST credentials
LoginRoute->>DB: verify user
LoginRoute->>createSessionRecord: ip, userAgent, expires
createSessionRecord->>DB: INSERT session row
DB-->>createSessionRecord: { id, sessionToken }
createSessionRecord-->>LoginRoute: sessionId
LoginRoute->>signSession: payload + sessionId
signSession-->>LoginRoute: JWT
LoginRoute-->>Browser: Set-Cookie JWT
Browser->>getAuthUser: request with cookie
getAuthUser->>isSessionValid: sessionId from JWT
isSessionValid->>DB: SELECT session (not revoked, not expired)
DB-->>isSessionValid: row or null
isSessionValid-->>getAuthUser: true | false
getAuthUser-->>Browser: user or 401
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/api/mutations/route.ts (1)
32-42: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftAdd per-action authorization before dispatching mutations.
Line 34 only checks that a session is valid, but this route then accepts any
action; child sessions can reach parent/admin-style mutations likesetFamilySettings,createParent,createChild, andgiveTokens. Add an explicit role/action policy or passuserinto mutation helpers that enforce ownership and role checks.🛡️ Suggested shape
try { const body = await req.json(); const { action, payload } = body as { action: string; payload: any }; + + if (!canDispatchMutation(user, action, payload)) { + return NextResponse.json({ error: "Forbidden" }, { status: 403 }); + } switch (action) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/mutations/route.ts` around lines 32 - 42, The POST handler in mutations/route.ts only validates that getAuthUser() returns a session, but then dispatches any requested action without role checks. Add per-action authorization in POST before the switch on action, or pass the user context into the mutation helpers so setFamilySettings, createParent, createChild, and giveTokens can enforce ownership and role-based access. Keep the policy close to the action dispatch so only allowed actions for the current user can execute.src/app/api/admin/security/route.ts (1)
91-105: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winValidate the current session before committing the email change.
Lines 101-104 run after the transaction, so the email and
superAdminEmailcan already be updated before returning401, and the success audit log is skipped. Read and validate the sessionId before the transaction whenupdates.emailis set.🐛 Proposed fix
+ const emailSession = updates.email ? await getSession() : null; + if (updates.email && !emailSession?.sessionId) { + return NextResponse.json({ ok: false, error: "Session expired — please log in again" }, { status: 401 }); + } + await db.$transaction(async (tx) => { await tx.user.update({ where: { id: user.id }, data: updates }); if (updates.email) { await tx.systemSettings.update({ where: { id: "singleton" }, data: { superAdminEmail: updates.email } }); } }); - if (updates.email) { - // Preserve the existing sessionId — the user's session is still valid, - // only the email claim in the JWT needs to be refreshed. - const currentSession = await getSession(); - if (!currentSession?.sessionId) { - return NextResponse.json({ ok: false, error: "Session expired — please log in again" }, { status: 401 }); - } - const token = await signSession({ sub: user.id, email: updates.email, platformRole: user.platformRole, familyRole: user.familyRole, sessionId: currentSession.sessionId }); + if (updates.email && emailSession?.sessionId) { + const token = await signSession({ sub: user.id, email: updates.email, platformRole: user.platformRole, familyRole: user.familyRole, sessionId: emailSession.sessionId }); await setSessionCookie(token); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/admin/security/route.ts` around lines 91 - 105, The session check for email changes is happening after the database transaction in the security admin route, so a missing/expired session can still allow `tx.user.update` and `tx.systemSettings.update` to commit before returning 401. Move the `getSession()`/`sessionId` validation ahead of the `db.$transaction` inside the `route.ts` handler, gated by `updates.email`, and only proceed to the transaction and `signSession` after confirming the current session is valid.
🧹 Nitpick comments (2)
src/app/api/auth/logout/route.ts (1)
7-16: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPass request metadata into logout audit logging.
logout()already recordsipAddress/userAgentwhen provided; wiring them here keeps logout audit entries as useful as login entries.Proposed change
-import { NextResponse } from "next/server"; +import { NextRequest, NextResponse } from "next/server"; import { logout, getSession } from "`@/lib/auth`"; @@ -export async function POST() { +export async function POST(req: NextRequest) { @@ const session = await getSession(); + const ip = req.headers.get("x-forwarded-for")?.split(",")[0]?.trim() ?? "unknown"; + const ua = req.headers.get("user-agent") ?? "unknown"; await logout({ userId: session?.sub, sessionId: session?.sessionId, + ipAddress: ip, + userAgent: ua, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/auth/logout/route.ts` around lines 7 - 16, The POST handler in logout/route.ts calls logout() without passing request metadata, so logout audit entries miss IP and user agent. Update the POST function to accept/use the incoming request object and extract the same metadata already supported by logout() (ipAddress and userAgent), then include those fields in the logout call alongside userId and sessionId. Keep the fix localized to POST and the logout() invocation so logout logging stays consistent with the login flow.src/app/api/auth/child-login/route.ts (1)
31-37: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse
SESSION_TTL_SECONDSfor child session expiry.
This route hard-codes7 dayswhilesrc/lib/auth.tsderives session expiry fromSESSION_TTL_SECONDS; that can make the persisted session lifetime drift from the token/cookie lifetime. Reuse the shared constant here, or factor the session creation into a shared helper so all login paths stay aligned.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/auth/child-login/route.ts` around lines 31 - 37, The child-login session expiry is hard-coded to 7 days instead of using the shared SESSION_TTL_SECONDS setting. Update the session creation in route.ts to derive expires from SESSION_TTL_SECONDS like src/lib/auth.ts does, or move the session-building logic into a shared helper used by the child login flow and other login paths so the persisted session lifetime stays aligned with the token/cookie lifetime.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/api/admin/sessions/route.ts`:
- Around line 36-39: The single-session revoke path in the admin sessions
handler is too broad because revokeSession(sessionId) only checks the session
id, allowing this founder-only endpoint to revoke any known session. Update the
route in the sessions API so the revocation is scoped to the current user.id,
either by passing user.id into revokeSession or by adding an ownership check
before revoking, and keep the auditLog call unchanged once the session is
confirmed to belong to that founder.
In `@src/lib/auth-edge.ts`:
- Line 20: The cookie parsing in auth-edge’s guarded flow can throw before the
existing try/catch because `decodeURIComponent` is evaluated while building
`cookies` from `cookieHeader`. Move the
`cookieHeader.split(...).map(...decodeURIComponent...)` logic inside the
protected path’s try block in `getAuthData`/the auth middleware code so
malformed cookie values are handled as auth misses instead of bubbling up as
500s. Keep the same cookie extraction behavior, just ensure the decode step is
wrapped by the guard.
In `@src/lib/auth.ts`:
- Around line 199-223: The isSessionValid debounce cache never evicts successful
session entries, so lastSeenCache grows indefinitely in src/lib/auth.ts. Update
the cache management around LAST_SEEN_REFRESH_INTERVAL_MS and the
session.lastSeenAt refresh path so entries are removed after they are no longer
needed, not only on update failure. Use the existing isSessionValid and
lastSeenCache symbols to add expiration/cleanup for stale sessionId keys while
keeping the one-minute debounce behavior intact.
- Around line 164-170: The self-service session revocation in revokeSession
currently targets only session id, which lets a disclosed sessionId revoke
another user’s session. Update revokeSession in src/lib/auth.ts to include the
owning user identifier in the where filter (for example by accepting userId and
matching both id and userId) so only the session owner can revoke their own
session; if cross-user revocation is needed, keep that behavior in a separate
admin-only path used by the admin sessions route.
---
Outside diff comments:
In `@src/app/api/admin/security/route.ts`:
- Around line 91-105: The session check for email changes is happening after the
database transaction in the security admin route, so a missing/expired session
can still allow `tx.user.update` and `tx.systemSettings.update` to commit before
returning 401. Move the `getSession()`/`sessionId` validation ahead of the
`db.$transaction` inside the `route.ts` handler, gated by `updates.email`, and
only proceed to the transaction and `signSession` after confirming the current
session is valid.
In `@src/app/api/mutations/route.ts`:
- Around line 32-42: The POST handler in mutations/route.ts only validates that
getAuthUser() returns a session, but then dispatches any requested action
without role checks. Add per-action authorization in POST before the switch on
action, or pass the user context into the mutation helpers so setFamilySettings,
createParent, createChild, and giveTokens can enforce ownership and role-based
access. Keep the policy close to the action dispatch so only allowed actions for
the current user can execute.
---
Nitpick comments:
In `@src/app/api/auth/child-login/route.ts`:
- Around line 31-37: The child-login session expiry is hard-coded to 7 days
instead of using the shared SESSION_TTL_SECONDS setting. Update the session
creation in route.ts to derive expires from SESSION_TTL_SECONDS like
src/lib/auth.ts does, or move the session-building logic into a shared helper
used by the child login flow and other login paths so the persisted session
lifetime stays aligned with the token/cookie lifetime.
In `@src/app/api/auth/logout/route.ts`:
- Around line 7-16: The POST handler in logout/route.ts calls logout() without
passing request metadata, so logout audit entries miss IP and user agent. Update
the POST function to accept/use the incoming request object and extract the same
metadata already supported by logout() (ipAddress and userAgent), then include
those fields in the logout call alongside userId and sessionId. Keep the fix
localized to POST and the logout() invocation so logout logging stays consistent
with the login flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6515a431-5119-4180-ad95-a0af5aefcbe8
📒 Files selected for processing (11)
prisma/migrations/20260625000000_add_session_revocation.sqlprisma/schema.prismasrc/app/api/admin/security/route.tssrc/app/api/admin/sessions/route.tssrc/app/api/auth/child-login/route.tssrc/app/api/auth/logout/route.tssrc/app/api/mutations/route.tssrc/app/api/state/route.tssrc/app/login/page.tsxsrc/lib/auth-edge.tssrc/lib/auth.ts
Comment on lines
+36
to
+39
| if (!sessionId) return NextResponse.json({ ok: false, error: "sessionId required (or all=true)" }, { status: 400 }); | ||
| const ok = await revokeSession(sessionId); | ||
| if (!ok) return NextResponse.json({ ok: false, error: "Session not found or already revoked" }, { status: 404 }); | ||
| await auditLog({ userId: user.id, action: "SESSION_REVOKED", entityType: "session", entityId: sessionId, ipAddress: ip, userAgent: ua, success: true }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Scope single-session revocation to the current founder.
Line 37 calls revokeSession(sessionId), whose upstream contract updates by session id only. That lets this founder-only endpoint revoke any known session id, not just one of user.id’s sessions.
🛡️ Proposed localized fix
if (!sessionId) return NextResponse.json({ ok: false, error: "sessionId required (or all=true)" }, { status: 400 });
+ const ownsSession = (await listUserSessions(user.id)).some((session) => session.id === sessionId);
+ if (!ownsSession) return NextResponse.json({ ok: false, error: "Session not found" }, { status: 404 });
const ok = await revokeSession(sessionId);
if (!ok) return NextResponse.json({ ok: false, error: "Session not found or already revoked" }, { status: 404 });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!sessionId) return NextResponse.json({ ok: false, error: "sessionId required (or all=true)" }, { status: 400 }); | |
| const ok = await revokeSession(sessionId); | |
| if (!ok) return NextResponse.json({ ok: false, error: "Session not found or already revoked" }, { status: 404 }); | |
| await auditLog({ userId: user.id, action: "SESSION_REVOKED", entityType: "session", entityId: sessionId, ipAddress: ip, userAgent: ua, success: true }); | |
| if (!sessionId) return NextResponse.json({ ok: false, error: "sessionId required (or all=true)" }, { status: 400 }); | |
| const ownsSession = (await listUserSessions(user.id)).some((session) => session.id === sessionId); | |
| if (!ownsSession) return NextResponse.json({ ok: false, error: "Session not found" }, { status: 404 }); | |
| const ok = await revokeSession(sessionId); | |
| if (!ok) return NextResponse.json({ ok: false, error: "Session not found or already revoked" }, { status: 404 }); | |
| await auditLog({ userId: user.id, action: "SESSION_REVOKED", entityType: "session", entityId: sessionId, ipAddress: ip, userAgent: ua, success: true }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/api/admin/sessions/route.ts` around lines 36 - 39, The single-session
revoke path in the admin sessions handler is too broad because
revokeSession(sessionId) only checks the session id, allowing this founder-only
endpoint to revoke any known session. Update the route in the sessions API so
the revocation is scoped to the current user.id, either by passing user.id into
revokeSession or by adding an ownership check before revoking, and keep the
auditLog call unchanged once the session is confirmed to belong to that founder.
| // getAuthUser() at the page/route level. | ||
| export async function getSessionFromCookieHeader(cookieHeader: string | null | undefined): Promise<SessionPayload | null> { | ||
| if (!cookieHeader) return null; | ||
| const cookies = Object.fromEntries(cookieHeader.split(";").map((c) => c.trim().split("=")).filter(([k]) => k).map(([k, ...rest]) => [k, decodeURIComponent(rest.join("="))])); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Parse the cookie header inside the guarded path.
Line 20 can throw before the try when decodeURIComponent sees a malformed cookie value. Because middleware calls this on every protected request, a bad Cookie header can turn an auth miss into a 500.
Proposed fix
export async function getSessionFromCookieHeader(cookieHeader: string | null | undefined): Promise<SessionPayload | null> {
if (!cookieHeader) return null;
- const cookies = Object.fromEntries(cookieHeader.split(";").map((c) => c.trim().split("=")).filter(([k]) => k).map(([k, ...rest]) => [k, decodeURIComponent(rest.join("="))]));
- const token = cookies[SESSION_COOKIE];
- if (!token) return null;
try {
+ const cookies = Object.fromEntries(cookieHeader.split(";").map((c) => c.trim().split("=")).filter(([k]) => k).map(([k, ...rest]) => [k, decodeURIComponent(rest.join("="))]));
+ const token = cookies[SESSION_COOKIE];
+ if (!token) return null;
const { payload } = await jwtVerify(token, JWT_SECRET, { algorithms: ["HS256"] });
if (typeof payload.sub !== "string" || typeof payload.email !== "string" || typeof payload.platformRole !== "string" || typeof payload.sessionId !== "string") return null;
return { sub: payload.sub, email: payload.email, platformRole: payload.platformRole as string, familyRole: (payload.familyRole as string) || "PARENT", sessionId: payload.sessionId as string, iat: payload.iat, exp: payload.exp };
} catch { return null; }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const cookies = Object.fromEntries(cookieHeader.split(";").map((c) => c.trim().split("=")).filter(([k]) => k).map(([k, ...rest]) => [k, decodeURIComponent(rest.join("="))])); | |
| export async function getSessionFromCookieHeader(cookieHeader: string | null | undefined): Promise<SessionPayload | null> { | |
| if (!cookieHeader) return null; | |
| try { | |
| const cookies = Object.fromEntries(cookieHeader.split(";").map((c) => c.trim().split("=")).filter(([k]) => k).map(([k, ...rest]) => [k, decodeURIComponent(rest.join("="))])); | |
| const token = cookies[SESSION_COOKIE]; | |
| if (!token) return null; | |
| const { payload } = await jwtVerify(token, JWT_SECRET, { algorithms: ["HS256"] }); | |
| if (typeof payload.sub !== "string" || typeof payload.email !== "string" || typeof payload.platformRole !== "string" || typeof payload.sessionId !== "string") return null; | |
| return { sub: payload.sub, email: payload.email, platformRole: payload.platformRole as string, familyRole: (payload.familyRole as string) || "PARENT", sessionId: payload.sessionId as string, iat: payload.iat, exp: payload.exp }; | |
| } catch { return null; } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/auth-edge.ts` at line 20, The cookie parsing in auth-edge’s guarded
flow can throw before the existing try/catch because `decodeURIComponent` is
evaluated while building `cookies` from `cookieHeader`. Move the
`cookieHeader.split(...).map(...decodeURIComponent...)` logic inside the
protected path’s try block in `getAuthData`/the auth middleware code so
malformed cookie values are handled as auth misses instead of bubbling up as
500s. Keep the same cookie extraction behavior, just ensure the decode step is
wrapped by the guard.
Comment on lines
+164
to
+170
| export async function revokeSession(sessionId: string): Promise<boolean> { | ||
| if (!sessionId) return false; | ||
| try { | ||
| // Soft-revoke (set revokedAt). Keeps audit trail; allows admin to see history. | ||
| const result = await db.session.updateMany({ | ||
| where: { id: sessionId, revokedAt: null }, | ||
| data: { revokedAt: new Date() }, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Scope single-session revocation to the session owner.
Lines 168-170 revoke by raw id only. In src/app/api/admin/sessions/route.ts:20-41, the caller passes a user-supplied sessionId while the corresponding list API is scoped to user.id, so this endpoint can revoke another user's session if its id is disclosed. Require userId in the where clause for the self-service path (or keep a separate explicitly-admin path if cross-user revocation is intentional).
Proposed fix
-export async function revokeSession(sessionId: string): Promise<boolean> {
+export async function revokeSession(sessionId: string, userId?: string): Promise<boolean> {
if (!sessionId) return false;
try {
// Soft-revoke (set revokedAt). Keeps audit trail; allows admin to see history.
const result = await db.session.updateMany({
- where: { id: sessionId, revokedAt: null },
+ where: { id: sessionId, revokedAt: null, ...(userId ? { userId } : {}) },
data: { revokedAt: new Date() },
});
return result.count > 0;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export async function revokeSession(sessionId: string): Promise<boolean> { | |
| if (!sessionId) return false; | |
| try { | |
| // Soft-revoke (set revokedAt). Keeps audit trail; allows admin to see history. | |
| const result = await db.session.updateMany({ | |
| where: { id: sessionId, revokedAt: null }, | |
| data: { revokedAt: new Date() }, | |
| export async function revokeSession(sessionId: string, userId?: string): Promise<boolean> { | |
| if (!sessionId) return false; | |
| try { | |
| // Soft-revoke (set revokedAt). Keeps audit trail; allows admin to see history. | |
| const result = await db.session.updateMany({ | |
| where: { id: sessionId, revokedAt: null, ...(userId ? { userId } : {}) }, | |
| data: { revokedAt: new Date() }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/auth.ts` around lines 164 - 170, The self-service session revocation
in revokeSession currently targets only session id, which lets a disclosed
sessionId revoke another user’s session. Update revokeSession in src/lib/auth.ts
to include the owning user identifier in the where filter (for example by
accepting userId and matching both id and userId) so only the session owner can
revoke their own session; if cross-user revocation is needed, keep that behavior
in a separate admin-only path used by the admin sessions route.
Comment on lines
+199
to
+223
| const LAST_SEEN_REFRESH_INTERVAL_MS = 60 * 1000; | ||
| const lastSeenCache = new Map<string, number>(); | ||
|
|
||
| async function isSessionValid(sessionId: string): Promise<boolean> { | ||
| if (!sessionId) return false; | ||
| try { | ||
| const session = await db.session.findUnique({ where: { id: sessionId } }); | ||
| if (!session) return false; | ||
| if (session.revokedAt) return false; | ||
| if (session.expires < new Date()) return false; | ||
|
|
||
| // Debounced refresh of lastSeenAt — at most once per minute per session. | ||
| // This powers future admin UI ("last active 2 min ago") without DB churn. | ||
| const now = Date.now(); | ||
| const lastRefresh = lastSeenCache.get(sessionId) ?? 0; | ||
| if (now - lastRefresh > LAST_SEEN_REFRESH_INTERVAL_MS) { | ||
| lastSeenCache.set(sessionId, now); | ||
| // Fire-and-forget — don't block the auth check on this write | ||
| db.session.update({ | ||
| where: { id: sessionId }, | ||
| data: { lastSeenAt: new Date() }, | ||
| }).catch((err) => { | ||
| logger.error("Failed to refresh lastSeenAt", { err, sessionId }); | ||
| lastSeenCache.delete(sessionId); // retry on next request | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Expire old lastSeenCache entries.
This cache only needs the last minute of history, but successful sessions never evict their key. On a long-lived process, the Map grows with total historical logins rather than currently active debounce windows.
Proposed fix
const LAST_SEEN_REFRESH_INTERVAL_MS = 60 * 1000;
const lastSeenCache = new Map<string, number>();
+
+function pruneLastSeenCache(now: number): void {
+ for (const [id, ts] of lastSeenCache) {
+ if (now - ts > LAST_SEEN_REFRESH_INTERVAL_MS) lastSeenCache.delete(id);
+ }
+}
async function isSessionValid(sessionId: string): Promise<boolean> {
if (!sessionId) return false;
try {
const session = await db.session.findUnique({ where: { id: sessionId } });
- if (!session) return false;
- if (session.revokedAt) return false;
- if (session.expires < new Date()) return false;
+ if (!session || session.revokedAt || session.expires < new Date()) {
+ lastSeenCache.delete(sessionId);
+ return false;
+ }
// Debounced refresh of lastSeenAt — at most once per minute per session.
// This powers future admin UI ("last active 2 min ago") without DB churn.
const now = Date.now();
+ pruneLastSeenCache(now);
const lastRefresh = lastSeenCache.get(sessionId) ?? 0;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const LAST_SEEN_REFRESH_INTERVAL_MS = 60 * 1000; | |
| const lastSeenCache = new Map<string, number>(); | |
| async function isSessionValid(sessionId: string): Promise<boolean> { | |
| if (!sessionId) return false; | |
| try { | |
| const session = await db.session.findUnique({ where: { id: sessionId } }); | |
| if (!session) return false; | |
| if (session.revokedAt) return false; | |
| if (session.expires < new Date()) return false; | |
| // Debounced refresh of lastSeenAt — at most once per minute per session. | |
| // This powers future admin UI ("last active 2 min ago") without DB churn. | |
| const now = Date.now(); | |
| const lastRefresh = lastSeenCache.get(sessionId) ?? 0; | |
| if (now - lastRefresh > LAST_SEEN_REFRESH_INTERVAL_MS) { | |
| lastSeenCache.set(sessionId, now); | |
| // Fire-and-forget — don't block the auth check on this write | |
| db.session.update({ | |
| where: { id: sessionId }, | |
| data: { lastSeenAt: new Date() }, | |
| }).catch((err) => { | |
| logger.error("Failed to refresh lastSeenAt", { err, sessionId }); | |
| lastSeenCache.delete(sessionId); // retry on next request | |
| }); | |
| const LAST_SEEN_REFRESH_INTERVAL_MS = 60 * 1000; | |
| const lastSeenCache = new Map<string, number>(); | |
| function pruneLastSeenCache(now: number): void { | |
| for (const [id, ts] of lastSeenCache) { | |
| if (now - ts > LAST_SEEN_REFRESH_INTERVAL_MS) lastSeenCache.delete(id); | |
| } | |
| } | |
| async function isSessionValid(sessionId: string): Promise<boolean> { | |
| if (!sessionId) return false; | |
| try { | |
| const session = await db.session.findUnique({ where: { id: sessionId } }); | |
| if (!session || session.revokedAt || session.expires < new Date()) { | |
| lastSeenCache.delete(sessionId); | |
| return false; | |
| } | |
| // Debounced refresh of lastSeenAt — at most once per minute per session. | |
| // This powers future admin UI ("last active 2 min ago") without DB churn. | |
| const now = Date.now(); | |
| pruneLastSeenCache(now); | |
| const lastRefresh = lastSeenCache.get(sessionId) ?? 0; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/auth.ts` around lines 199 - 223, The isSessionValid debounce cache
never evicts successful session entries, so lastSeenCache grows indefinitely in
src/lib/auth.ts. Update the cache management around
LAST_SEEN_REFRESH_INTERVAL_MS and the session.lastSeenAt refresh path so entries
are removed after they are no longer needed, not only on update failure. Use the
existing isSessionValid and lastSeenCache symbols to add expiration/cleanup for
stale sessionId keys while keeping the one-minute debounce behavior intact.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
Replaces JWT-only authentication with server-side sessions. JWTs now contain a
sessionIdthat must exist in theSessiontable AND must not be revoked AND must not be expired.This closes the "founder returns after logout" bug: even if a browser cache or another tab holds the JWT, the revoked session row makes
getAuthUser()return null.Before merging this PR, run the migration SQL against Neon:
prisma/migrations/20260625000000_add_session_revocation.sqlThis is safe to run while the app is live — adds nullable/defaulted columns, no table lock.
If you merge without running the migration first, the app will 500 on every authenticated request until you run it.
All existing JWTs (without
sessionId) are auto-rejected on deploy. Every user must log in again. This is expected — the old JWTs were issued under the less-secure model.Changes
1. Schema (
prisma/schema.prisma)revokedAt DateTime?— NULL = active, set = revokedlastSeenAt DateTime @default(now())— refreshed on each request (debounced)2.
src/lib/auth.ts(core fix)SessionPayloadnow includessessionIdsignSession()requiressessionId(throws if missing)verifySession()rejects JWTs withoutsessionId(kills legacy tokens)clearSessionCookie()uses matching RFC 6265 attributes (HttpOnly, SameSite, Secure, Path, Expires, Max-Age)createSessionRecord(),revokeSession(),revokeAllUserSessions(),isSessionValid(),listUserSessions()getAuthUser()now callsisSessionValid(sessionId)— JWT alone is not enoughlogout()revokes the server-side session FIRST, then clears cookieattemptLogin,completeLoginWith2FA,performFounderSetup) create session recordslastSeenAtrefreshed with 60-second debounce (powers future admin UI)3.
src/lib/auth-edge.tssessionIdadded toSessionPayloadgetAuthUser()does the full DB check4.
src/app/api/auth/logout/route.tssessionIdfrom JWT, passes tologout()so the session is revoked5.
src/app/api/auth/child-login/route.tsSessionrecord on child login (was missing)6.
src/app/login/page.tsx7.
src/app/api/admin/security/route.tssessionIdwhen re-signing JWT after email change8.
src/app/api/admin/sessions/route.ts(NEW)GET /api/admin/sessions— list all sessions for current founderPOST /api/admin/sessions— revoke bysessionIdorall: true9.
src/app/api/state/route.ts+src/app/api/mutations/route.tsgetAuthUser()auth gates (were relying on middleware alone)Acceptance tests
All 5 tests pass locally (12/12 sub-checks):
Files changed (11 files)
prisma/schema.prisma— Session model: +2 columns, +2 indexesprisma/migrations/20260625000000_add_session_revocation.sql(new) — Neon migrationsrc/lib/auth.ts— core session lifecyclesrc/lib/auth-edge.ts— sessionId in payloadsrc/app/api/auth/logout/route.ts— revokes sessionsrc/app/api/auth/child-login/route.ts— creates sessionsrc/app/login/page.tsx— redirect if valid sessionsrc/app/api/admin/security/route.ts— preserve sessionIdsrc/app/api/admin/sessions/route.ts(new) — list/revoke endpointsrc/app/api/state/route.ts— auth gatesrc/app/api/mutations/route.ts— auth gateVerification
npx tsc --noEmit— cleannpx eslint— cleannpx prisma generate— cleanVercel env vars
None needed.
JWT_SECRETis already set (verified).Deployment steps (in order)
Summary by CodeRabbit
New Features
Bug Fixes