From 41676fa07214c46f18fc2c06eb981ef03f3f1aa5 Mon Sep 17 00:00:00 2001 From: jwinterm Date: Sun, 31 May 2026 10:15:08 -0400 Subject: [PATCH] refactor: unify identity resolution on stable user_id (anon_token) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There were two identity systems: SessionManager keyed on the stable, UNIQUE anon_token, and getOrCreateUser keyed on the mutable, non-unique socket_id. The latter could create a duplicate "orphan" user row (or resolve the wrong row) when the socket_id lookup raced the session's socket_id update — desyncing credits and payouts between the two rows for one human. - getOrCreateUser now resolves through SessionManager's stable identity first (anon_token -> users.id), re-selecting a fresh row by stable id, and only falls back to the legacy socket_id lookup when no session exists. The last-resort create now warns, since it indicates a missing session. - gameModeManager.sessionManager is wired in SocketHandlers so the session-first path is active in production. - anon_token is already UNIQUE (migration 003), so the stable-identity constraint already exists; no new migration needed. Re-keying the in-memory activeGames map by user_id is deferred (invasive; already mitigated by stamping dbUserId on the game in the disconnect fix). Adds identityResolution.test.js proving no duplicate row is created when a session exists. Full suite: 17 suites / 136 tests. --- docs/REMEDIATION_PLAN.md | 2 +- src/game/gameModeManager.js | 49 +++++++++++++++++++----- src/network/socketHandlers.js | 3 ++ test/identityResolution.test.js | 67 +++++++++++++++++++++++++++++++++ 4 files changed, 110 insertions(+), 11 deletions(-) create mode 100644 test/identityResolution.test.js diff --git a/docs/REMEDIATION_PLAN.md b/docs/REMEDIATION_PLAN.md index 50dcfa3..2346383 100644 --- a/docs/REMEDIATION_PLAN.md +++ b/docs/REMEDIATION_PLAN.md @@ -33,7 +33,7 @@ Key: `S` ≤ half day · `M` ~1–3 days · `L` ~1 week+ · 🔴 launch-blocker - [x] **1.6 S — `credits` → BIGINT; idempotent 010; atomic migration runner.** ✅ Migration 018 widens `users.credits` + `credit_transactions.amount/balance_after` to BIGINT. Migration 010 rewritten with conname-guarded `DO` blocks (re-runnable, no startup wedge). Runner now applies each migration **and** records the ledger row in one transaction (no applied-but-unrecorded state). Verified end-to-end: all 18 migrations applied cleanly to a throwaway Postgres DB, re-run skipped via the ledger, `credits` confirmed `bigint`, all CHECKs present. ## Phase 2 — Identity, abuse & concurrency (🟠) -- [ ] **2.1 L — Unify identity on `user_id`.** One `IdentityService.resolve(socket)`; kill `getOrCreateUser(socketId)` keying (`gameModeManager.js:926`); key `activeGames` by `user_id`. +- [x] **2.1 L — Unify identity resolution on stable `user_id`.** ✅ `getOrCreateUser` now resolves through SessionManager's stable `anon_token`→`id` identity first (re-selecting a fresh row by stable id), falling back to the legacy `socket_id` lookup only when no session exists — eliminating the duplicate "orphan" user row that caused credit/payout desync. `gameModeManager.sessionManager` wired in SocketHandlers. `anon_token` is already `UNIQUE` (migration 003), so the stable-identity constraint exists. New `identityResolution.test.js` proves no duplicate row is created when a session exists. _Deferred: re-keying the in-memory `activeGames` map by `user_id` (invasive; already mitigated in 0.5 by stamping `dbUserId` on the game) — tracked as a follow-up._ - [ ] **2.2 M — Reconnect-proof rate limiting.** Key on `user_id`/IP; IP-limit chat; set `trust proxy`; de-dup limit configs (`src/network/rateLimiter.js:80`). - [ ] **2.3 M — Input validation + leak fixes.** Validate socket payloads (`connectionHandler.js:90`); TTL-evict `mempoolNotified` (`socketHandlers.js:223`); evict `sessions` on disconnect; wire all `dispose()` into `shutdown()`; snapshot in `checkGamesTimeout` (`socketHandlers.js:1012`). diff --git a/src/game/gameModeManager.js b/src/game/gameModeManager.js index 6544449..ee38dbb 100644 --- a/src/game/gameModeManager.js +++ b/src/game/gameModeManager.js @@ -950,34 +950,63 @@ class GameModeManager { /** * Get or create user record */ + /** + * Resolve the database user for a socket. + * + * IDENTITY (Phase 2.1): The stable identity is `users.id`, established from the + * client's `anon_token` by SessionManager at connection time. This method resolves + * through that session identity FIRST, so every money/credit/payout path operates on + * the same stable row. It only falls back to the legacy `socket_id` lookup-or-create + * when no session exists (e.g. SessionManager not wired, or a REST call for a socket + * with no live session) — `socket_id` is mutable and non-unique and must never be the + * primary money key. + */ async getOrCreateUser(socketId, ipAddress = null) { try { - // Try to find existing user + // 1. Prefer the stable identity established by SessionManager (anon_token -> id). + if (this.sessionManager && typeof this.sessionManager.getBySocket === 'function') { + try { + const sessionUser = await this.sessionManager.getBySocket(socketId); + if (sessionUser && sessionUser.id != null) { + // Return a FRESH row by stable id (the cached session row may be stale). + const fresh = await this.db.query(`SELECT * FROM users WHERE id = $1`, [sessionUser.id]); + if (fresh.rows.length > 0) { + await this.db.query(`UPDATE users SET last_active = NOW() WHERE id = $1`, [fresh.rows[0].id]); + return fresh.rows[0]; + } + } + } catch (sessErr) { + // Fall through to legacy resolution on any session-lookup error. + } + } + + // 2. Legacy fallback: look up by socket_id. let userResult = await this.db.query(` SELECT * FROM users WHERE socket_id = $1 `, [socketId]); - + if (userResult.rows.length > 0) { - // Update last active await this.db.query(` - UPDATE users + UPDATE users SET last_active = NOW() WHERE id = $1 `, [userResult.rows[0].id]); - + return userResult.rows[0]; } - - // Create new user + + // 3. Last resort: create a new user. This only fires when no session was + // established for the socket (abnormal in normal flow, since every connection + // creates a session) — log it so orphan-row creation is visible. userResult = await this.db.query(` INSERT INTO users (socket_id, ip_address) VALUES ($1, $2) RETURNING * `, [socketId, ipAddress]); - - console.log(`👤 Created new user: ${socketId}`); + + console.warn(`👤 Created new user without a session (socket ${socketId}) — no anon_token identity resolved.`); return userResult.rows[0]; - + } catch (error) { const normalized = normalizeError(error, 'Failed to load user'); console.error('❌ Error getting/creating user:', normalized.message); diff --git a/src/network/socketHandlers.js b/src/network/socketHandlers.js index bb847bd..78ab4da 100644 --- a/src/network/socketHandlers.js +++ b/src/network/socketHandlers.js @@ -67,6 +67,9 @@ class SocketHandlers { debugManager: this.debugManager, gameModeManager: this.gameModeManager }); + // IDENTITY (Phase 2.1): give GameModeManager the session manager so getOrCreateUser + // resolves through the stable anon_token identity instead of the mutable socket_id. + this.gameModeManager.sessionManager = this.sessionManager; // Initialize cleanup timers this.sessionManager.initialize().catch(err => { console.error('Failed to initialize SessionManager:', err); diff --git a/test/identityResolution.test.js b/test/identityResolution.test.js new file mode 100644 index 0000000..a377d1b --- /dev/null +++ b/test/identityResolution.test.js @@ -0,0 +1,67 @@ +/** + * Identity resolution tests (Phase 2.1). + * + * getOrCreateUser must resolve through the stable anon_token identity (via SessionManager) + * rather than the mutable, non-unique socket_id — and must NOT create a duplicate "orphan" + * user row when a session already exists. + */ + +const GameModeManager = require('../src/game/gameModeManager'); + +const createMockPaymentConfig = () => ({ + getConfig: () => ({ + paymentsEnabled: true, + currency: { symbol: 'WOW', decimals: 11 }, + modes: { + direct: { enabled: true, price: 100000000000n }, + credits: { enabled: true, creditsPerGame: 1, packages: [{ id: 'small', credits: 10, price: '500000000000', bonus: 0 }] } + }, + payouts: { rules: { direct: { enabled: true, multipliers: { escape: 2, escapeWithTreasure: 3 } }, credits: { enabled: false, multipliers: { escape: 1.5, escapeWithTreasure: 2 }, baseValue: 50000000000n } } }, + preferences: { preferCreditsFirst: true }, + earlyEntry: { enabled: false } + }), + getLegacyGameMode: () => 'PAID_SINGLE', + eventBus: { on: () => {} } +}); + +function buildGmm() { + const calls = { byId: 0, bySocketSelect: 0, insert: 0 }; + const db = { + query: jest.fn(async (text, params = []) => { + if (/SELECT \* FROM users WHERE id = \$1/i.test(text)) { calls.byId++; return { rows: [{ id: params[0], credits: 5, socket_id: 'sockNEW' }] }; } + if (/UPDATE users SET last_active/i.test(text)) return { rows: [] }; + if (/SELECT \* FROM users WHERE socket_id = \$1/i.test(text)) { calls.bySocketSelect++; return { rows: [] }; } + if (/INSERT INTO users/i.test(text)) { calls.insert++; return { rows: [{ id: 99, socket_id: params[0] }] }; } + return { rows: [] }; + }) + }; + const gmm = new GameModeManager(db, { processPayout: jest.fn() }, { CONSOLE_LOGGING: false, getCurrentBlockHeight: () => 1 }, createMockPaymentConfig()); + return { gmm, db, calls }; +} + +describe('getOrCreateUser identity resolution', () => { + test('resolves through the session stable id and does not create a duplicate', async () => { + const { gmm, calls } = buildGmm(); + // Session resolves the socket to stable user id 1 (cache populated at connect). + gmm.sessionManager = { getBySocket: jest.fn().mockResolvedValue({ id: 1 }) }; + + const user = await gmm.getOrCreateUser('sockNEW'); + + expect(user.id).toBe(1); + expect(gmm.sessionManager.getBySocket).toHaveBeenCalledWith('sockNEW'); + expect(calls.byId).toBe(1); // resolved by stable id + expect(calls.insert).toBe(0); // NO duplicate orphan row created + expect(calls.bySocketSelect).toBe(0); // did not fall back to socket_id lookup + }); + + test('falls back to socket_id lookup when no session manager is wired', async () => { + const { gmm, calls } = buildGmm(); + // No gmm.sessionManager (legacy path). + const user = await gmm.getOrCreateUser('sockLEGACY'); + + expect(calls.bySocketSelect).toBe(1); // used socket_id lookup + // socket_id lookup returned no rows in this mock -> created as last resort + expect(calls.insert).toBe(1); + expect(user.id).toBe(99); + }); +});