From 73a92ccf8fb389fd37f7ff613f5275a19f8f31b2 Mon Sep 17 00:00:00 2001 From: Admin Date: Thu, 2 Apr 2026 22:22:17 +0500 Subject: [PATCH] fix: deduplicate sessions with device fingerprint upsert OAuth callbacks were creating a new session record on every login from the same device because user-agent/IP were hardcoded as empty strings, producing a pile-up of 6+ identical 'Unknown browser' sessions. - Add upsertUserSession(): looks up existing session by user_id + device_fingerprint (SHA-256 of ua::ip, first 16 hex chars); reuses and touches it (returning the same authSessionId) if found, creates a new record otherwise - Add device_fingerprint field to UserSession interface - Fix OAuth callback to read real user-agent/IP from request headers (they are available in RequestHandler via request.headers) - Switch both OAuth and password login to upsertUserSession so the returned authSessionId is used for the auth token - Extend pruneStaleUserSessions to also cap sessions at 10 per user - Keep createUserSession as a deprecated shim for gradual migration --- ui/src/lib/server/pocketbase.ts | 128 ++++++++++++++---- ui/src/routes/api/auth/login/+server.ts | 14 +- .../auth/[provider]/callback/+server.ts | 30 ++-- 3 files changed, 121 insertions(+), 51 deletions(-) diff --git a/ui/src/lib/server/pocketbase.ts b/ui/src/lib/server/pocketbase.ts index 6baca47..b7d4af4 100644 --- a/ui/src/lib/server/pocketbase.ts +++ b/ui/src/lib/server/pocketbase.ts @@ -542,7 +542,7 @@ export async function unsaveBook( // ─── Users ──────────────────────────────────────────────────────────────────── -import { scryptSync, randomBytes, timingSafeEqual } from 'node:crypto'; +import { scryptSync, randomBytes, timingSafeEqual, createHash } from 'node:crypto'; function hashPassword(password: string): string { const salt = randomBytes(16).toString('hex'); @@ -1003,12 +1003,79 @@ export interface UserSession { session_id: string; // the auth session ID embedded in the token user_agent: string; ip: string; + device_fingerprint: string; created_at: string; last_seen: string; } /** - * Create a new session record on login. Returns the record ID. + * Generate a short device fingerprint from user-agent + IP. + * SHA-256 of the concatenation, first 16 hex chars. + */ +function deviceFingerprint(userAgent: string, ip: string): string { + return createHash('sha256') + .update(`${userAgent}::${ip}`) + .digest('hex') + .slice(0, 16); +} + +/** + * Upsert a session record on login. + * - If a session already exists for this user + device fingerprint, touch it and + * return the existing authSessionId (so the caller can reuse the same token). + * - Otherwise create a new record. + * Returns `{ authSessionId, recordId }`. + */ +export async function upsertUserSession( + userId: string, + authSessionId: string, + userAgent: string, + ip: string +): Promise<{ authSessionId: string; recordId: string }> { + const fp = deviceFingerprint(userAgent, ip); + + // Look for an existing session from the same device + const existing = await listOne( + 'user_sessions', + `user_id="${userId}" && device_fingerprint="${fp}"` + ); + + if (existing) { + // Touch last_seen and return the existing authSessionId + const token = await getToken(); + await fetch(`${PB_URL}/api/collections/user_sessions/records/${existing.id}`, { + method: 'PATCH', + headers: { Authorization: `Bearer ${token}`, 'Content-Type': 'application/json' }, + body: JSON.stringify({ last_seen: new Date().toISOString() }) + }).catch(() => {}); + return { authSessionId: existing.session_id, recordId: existing.id }; + } + + // Create a new session record + const now = new Date().toISOString(); + const res = await pbPost('/api/collections/user_sessions/records', { + user_id: userId, + session_id: authSessionId, + user_agent: userAgent, + ip, + device_fingerprint: fp, + created_at: now, + last_seen: now + }); + if (!res.ok) { + const body = await res.text().catch(() => ''); + log.error('pocketbase', 'upsertUserSession POST failed', { userId, status: res.status, body }); + throw new Error(`Failed to create session: ${res.status}`); + } + const rec = (await res.json()) as { id: string }; + // Best-effort: prune stale/excess sessions in the background + pruneStaleUserSessions(userId).catch(() => {}); + return { authSessionId, recordId: rec.id }; +} + +/** + * @deprecated Use upsertUserSession instead. + * Kept temporarily so callers can be migrated incrementally. */ export async function createUserSession( userId: string, @@ -1016,24 +1083,8 @@ export async function createUserSession( userAgent: string, ip: string ): Promise { - const now = new Date().toISOString(); - const res = await pbPost('/api/collections/user_sessions/records', { - user_id: userId, - session_id: authSessionId, - user_agent: userAgent, - ip, - created_at: now, - last_seen: now - }); - if (!res.ok) { - const body = await res.text().catch(() => ''); - log.error('pocketbase', 'createUserSession POST failed', { userId, status: res.status, body }); - throw new Error(`Failed to create session: ${res.status}`); - } - const rec = (await res.json()) as { id: string }; - // Best-effort: prune stale sessions in the background so the list doesn't grow forever - pruneStaleUserSessions(userId).catch(() => {}); - return rec.id; + const { recordId } = await upsertUserSession(userId, authSessionId, userAgent, ip); + return recordId; } /** @@ -1070,20 +1121,37 @@ export async function listUserSessions(userId: string): Promise { } /** - * Delete sessions for a user that haven't been seen in the last `days` days. + * Delete sessions for a user that haven't been seen in the last `days` days, + * and cap the total number of sessions at `maxSessions` (pruning oldest first). * Called on login so the list self-cleans without a separate cron job. */ -async function pruneStaleUserSessions(userId: string, days = 30): Promise { - const cutoff = new Date(Date.now() - days * 24 * 60 * 60 * 1000).toISOString(); - const stale = await listAll( - 'user_sessions', - `user_id="${userId}" && last_seen<"${cutoff}"` - ); - if (stale.length === 0) return; +async function pruneStaleUserSessions( + userId: string, + days = 30, + maxSessions = 10 +): Promise { const token = await getToken(); + const all = await listAll('user_sessions', `user_id="${userId}"`, '-last_seen'); + + const cutoff = new Date(Date.now() - days * 24 * 60 * 60 * 1000).toISOString(); + const toDelete = new Set(); + + // Mark stale sessions + for (const s of all) { + if (s.last_seen < cutoff) toDelete.add(s.id); + } + + // Mark excess sessions beyond the cap (oldest first — list is sorted -last_seen) + const remaining = all.filter((s) => !toDelete.has(s.id)); + if (remaining.length > maxSessions) { + remaining.slice(maxSessions).forEach((s) => toDelete.add(s.id)); + } + + if (toDelete.size === 0) return; + await Promise.all( - stale.map((s) => - fetch(`${PB_URL}/api/collections/user_sessions/records/${s.id}`, { + [...toDelete].map((id) => + fetch(`${PB_URL}/api/collections/user_sessions/records/${id}`, { method: 'DELETE', headers: { Authorization: `Bearer ${token}` } }).catch(() => {}) diff --git a/ui/src/routes/api/auth/login/+server.ts b/ui/src/routes/api/auth/login/+server.ts index 5d04a36..a94f806 100644 --- a/ui/src/routes/api/auth/login/+server.ts +++ b/ui/src/routes/api/auth/login/+server.ts @@ -1,6 +1,6 @@ import { json, error } from '@sveltejs/kit'; import type { RequestHandler } from './$types'; -import { loginUser, mergeSessionProgress, createUserSession } from '$lib/server/pocketbase'; +import { loginUser, mergeSessionProgress, upsertUserSession } from '$lib/server/pocketbase'; import { createAuthToken } from '../../../../hooks.server'; import { log } from '$lib/server/logger'; import { randomBytes } from 'node:crypto'; @@ -48,16 +48,20 @@ export const POST: RequestHandler = async ({ request, cookies, locals }) => { log.warn('api/auth/login', 'mergeSessionProgress failed (non-fatal)', { err: String(e) }) ); - const authSessionId = randomBytes(16).toString('hex'); + const candidateSessionId = randomBytes(16).toString('hex'); const userAgent = request.headers.get('user-agent') ?? ''; const ip = request.headers.get('x-forwarded-for')?.split(',')[0]?.trim() ?? request.headers.get('x-real-ip') ?? ''; - createUserSession(user.id, authSessionId, userAgent, ip).catch((e) => - log.warn('api/auth/login', 'createUserSession failed (non-fatal)', { err: String(e) }) - ); + + let authSessionId = candidateSessionId; + try { + ({ authSessionId } = await upsertUserSession(user.id, candidateSessionId, userAgent, ip)); + } catch (e) { + log.warn('api/auth/login', 'upsertUserSession failed (non-fatal)', { err: String(e) }); + } const token = createAuthToken(user.id, user.username, user.role ?? 'user', authSessionId); diff --git a/ui/src/routes/auth/[provider]/callback/+server.ts b/ui/src/routes/auth/[provider]/callback/+server.ts index 7deb1d7..e7086e9 100644 --- a/ui/src/routes/auth/[provider]/callback/+server.ts +++ b/ui/src/routes/auth/[provider]/callback/+server.ts @@ -25,7 +25,7 @@ import { linkOAuthToUser } from '$lib/server/pocketbase'; import { createAuthToken } from '../../../../hooks.server'; -import { createUserSession, touchUserSession, mergeSessionProgress } from '$lib/server/pocketbase'; +import { upsertUserSession, mergeSessionProgress } from '$lib/server/pocketbase'; import { log } from '$lib/server/logger'; type Provider = 'google' | 'github'; @@ -159,7 +159,7 @@ function deriveUsername(name: string, email: string): string { // ─── Handler ────────────────────────────────────────────────────────────────── -export const GET: RequestHandler = async ({ params, url, cookies, locals }) => { +export const GET: RequestHandler = async ({ params, url, cookies, locals, request }) => { const provider = params.provider as Provider; if (provider !== 'google' && provider !== 'github') { error(404, 'Unknown OAuth provider'); @@ -226,21 +226,19 @@ export const GET: RequestHandler = async ({ params, url, cookies, locals }) => { log.warn('oauth', 'mergeSessionProgress failed (non-fatal)', { err: String(err) }) ); - // ── Create session + auth cookie ────────────────────────────────────────── + // ── Create / reuse session + auth cookie ───────────────────────────────── + const userAgent = request.headers.get('user-agent') ?? ''; + const ip = + request.headers.get('x-forwarded-for')?.split(',')[0]?.trim() ?? + request.headers.get('x-real-ip') ?? + ''; + const candidateSessionId = randomBytes(16).toString('hex'); let authSessionId: string; - - // Reuse existing session if the user is already logged in as the same user - if (locals.user?.id === user.id && locals.user?.authSessionId) { - authSessionId = locals.user.authSessionId; - // Just touch the existing session to update last_seen - touchUserSession(authSessionId).catch(() => {}); - } else { - authSessionId = randomBytes(16).toString('hex'); - const userAgent = ''; // not available in RequestHandler — omit - const ip = ''; - createUserSession(user.id, authSessionId, userAgent, ip).catch((err) => - log.warn('oauth', 'createUserSession failed (non-fatal)', { err: String(err) }) - ); + try { + ({ authSessionId } = await upsertUserSession(user.id, candidateSessionId, userAgent, ip)); + } catch (err) { + log.warn('oauth', 'upsertUserSession failed (non-fatal)', { err: String(err) }); + authSessionId = candidateSessionId; } const token = createAuthToken(user.id, user.username, user.role ?? 'user', authSessionId);