diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index e829658..8e049b5 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -198,8 +198,10 @@ Two security-relevant details: 1. **Questions are sanitized before broadcast.** The `quiz:question` payload maps answers down to `{ id, text }` only. The client literally cannot know which option is correct, so you cannot cheat by reading the socket. -2. **Scoring happens server-side.** The client sends `timeMs`; the server - decides correctness and points. +2. **Scoring happens server-side.** The server decides correctness and + points, and measures the time used itself from the question's deadline in + Redis. A `timeMs` in the message is ignored. Only an answer for the + question that is currently open, received before its deadline, counts. 3. **The organizer role is decided by the server.** A `join` that asks for `ORGANIZER` must carry the host's JWT. The ws-server verifies it with the same `JWT_SECRET` as http-server and checks that the user is an accepted @@ -212,11 +214,12 @@ Two security-relevant details: Scoring rewards speed: ```js -points = isCorrect ? Math.max(10, 1000 - timeMs) : 0 +points = isCorrect ? Math.max(1, Math.ceil(10 * (1 - elapsedMs / limitMs))) : 0 ``` -A correct answer in 200ms scores 800; one at 3s floors at 10. Wrong answers -score nothing. +`elapsedMs` is how much of the question's time limit had gone when the answer +reached the server. A correct answer scores 10 down to 1 across the time +limit. Wrong answers score nothing. ### genAI — AI question generation diff --git a/apps/ws-server/src/utils/handleMessages.auth.test.ts b/apps/ws-server/src/utils/handleMessages.auth.test.ts index 34522fb..2b99053 100644 --- a/apps/ws-server/src/utils/handleMessages.auth.test.ts +++ b/apps/ws-server/src/utils/handleMessages.auth.test.ts @@ -20,6 +20,7 @@ const mocks = vi.hoisted(() => ({ drainSession: vi.fn(), endQuizSession: vi.fn(), startQuestionTimer: vi.fn(), + closeQuestionWindow: vi.fn(), })); vi.mock("@repo/db", () => ({ prisma: mocks.prisma })); @@ -41,6 +42,8 @@ vi.mock("./cache.js", () => ({ vi.mock("./timeManager.js", () => ({ startQuestionTimer: mocks.startQuestionTimer, timerDeadlineKey: (id: string) => `deadline:${id}`, + currentQuestionKey: (id: string) => `current:${id}`, + closeQuestionWindow: mocks.closeQuestionWindow, finishQuestionEarly: vi.fn(), })); vi.mock("./answerQueue.js", () => ({ @@ -127,6 +130,7 @@ beforeEach(() => { mocks.drainSession, mocks.endQuizSession, mocks.startQuestionTimer, + mocks.closeQuestionWindow, ]) { mock.mockReset(); } @@ -371,6 +375,11 @@ describe("a verified organizer can run their quiz", () => { await handleMessage(host, { type: "quiz:end", payload: { sessionId } }); + // answers stop being accepted before the buffer is drained + expect(mocks.closeQuestionWindow).toHaveBeenCalledWith(sessionId); + expect(mocks.closeQuestionWindow.mock.invocationCallOrder[0]).toBeLessThan( + mocks.drainSession.mock.invocationCallOrder[0], + ); expect(mocks.drainSession).toHaveBeenCalledWith(sessionId); expect(mocks.endQuizSession).toHaveBeenCalledWith(sessionId); expect(mocks.broadcastToSession).toHaveBeenCalledWith(sessionId, "quiz:leaderboard", { diff --git a/apps/ws-server/src/utils/handleMessages.submitAnswer.test.ts b/apps/ws-server/src/utils/handleMessages.submitAnswer.test.ts new file mode 100644 index 0000000..afcb97b --- /dev/null +++ b/apps/ws-server/src/utils/handleMessages.submitAnswer.test.ts @@ -0,0 +1,254 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import WebSocket from "ws"; + +const mocks = vi.hoisted(() => ({ + cache: { get: vi.fn() }, + getCachedQuestions: vi.fn(), + enqueueAnswer: vi.fn(), + countAnswered: vi.fn(), + finishQuestionEarly: vi.fn(), + closeQuestionWindow: vi.fn(), + drainSession: vi.fn(), + broadcastToSession: vi.fn(), +})); + +vi.mock("@repo/db", () => ({ prisma: {} })); +vi.mock("../redis.js", () => ({ cache: mocks.cache })); +vi.mock("./broadcastTosession.js", () => ({ broadcastToSession: mocks.broadcastToSession })); +vi.mock("./endQuizSession.js", () => ({ endQuizSession: vi.fn() })); +vi.mock("./leaderboard.js", () => ({ getLeaderboard: vi.fn() })); +vi.mock("./cache.js", () => ({ + getCachedSession: vi.fn(), + getCachedQuestions: mocks.getCachedQuestions, + getCachedParticipants: vi.fn(), + getCachedLeaderboard: vi.fn().mockResolvedValue([]), + invalidateParticipants: vi.fn(), + appendCachedParticipant: vi.fn(), + invalidateLeaderboard: vi.fn(), + invalidateSession: vi.fn(), + clearSessionCache: vi.fn(), +})); +vi.mock("./timeManager.js", () => ({ + startQuestionTimer: vi.fn(), + timerDeadlineKey: (id: string) => `deadline:${id}`, + currentQuestionKey: (id: string) => `current:${id}`, + closeQuestionWindow: mocks.closeQuestionWindow, + finishQuestionEarly: mocks.finishQuestionEarly, +})); +vi.mock("./answerQueue.js", () => ({ + enqueueAnswer: mocks.enqueueAnswer, + countAnswered: mocks.countAnswered, + drainSession: mocks.drainSession, +})); + +import { handleMessage } from "./handleMessages.js"; +import type { Client } from "../index.js"; + +const SESSION = "s-1"; +const START = new Date("2026-01-01T00:00:00Z").getTime(); +const LIMIT_S = 20; + +const questions = [ + { + id: "q1", + timeLimit: LIMIT_S, + answers: [ + { id: "right", isCorrect: true }, + { id: "wrong", isCorrect: false }, + ], + }, + { + id: "q2", + timeLimit: LIMIT_S, + answers: [{ id: "right2", isCorrect: true }], + }, +]; + +/** What the timer leaves in Redis while a question is open. */ +function openQuestion(index: number, startedAt = START) { + const redis: Record = { + [`deadline:${SESSION}`]: String(startedAt + LIMIT_S * 1_000), + [`current:${SESSION}`]: String(index), + }; + mocks.cache.get.mockImplementation(async (key: string) => redis[key] ?? null); +} + +function player(): Client { + return { + ws: { readyState: WebSocket.OPEN, send: vi.fn(), terminate: vi.fn() } as unknown as WebSocket, + sessionId: SESSION, + role: "PARTICIPANT", + participantId: "p-1", + isAlive: true, + } as Client; +} + +function submit(overrides: Record = {}) { + return handleMessage(player(), { + type: "quiz:submit-answer", + payload: { + sessionId: SESSION, + participantId: "p-1", + questionId: "q1", + answerId: "right", + timeMs: 0, + ...overrides, + }, + }); +} + +function enqueued() { + return mocks.enqueueAnswer.mock.calls.map(([, answer]) => answer); +} + +beforeEach(() => { + vi.useFakeTimers({ toFake: ["Date"] }); + vi.setSystemTime(START); + for (const mock of [ + mocks.cache.get, + mocks.getCachedQuestions, + mocks.enqueueAnswer, + mocks.countAnswered, + mocks.finishQuestionEarly, + ]) { + mock.mockReset(); + } + mocks.getCachedQuestions.mockResolvedValue(questions); + mocks.enqueueAnswer.mockReturnValue(true); + mocks.countAnswered.mockResolvedValue(0); +}); + +afterEach(() => { + vi.useRealTimers(); +}); + +describe("quiz:submit-answer timing", () => { + it("ignores a client timeMs of 0 and scores from the server clock", async () => { + openQuestion(0); + vi.setSystemTime(START + 15_000); // 15s of 20s used + + await submit({ timeMs: 0 }); + + expect(enqueued()).toEqual([ + expect.objectContaining({ timeMs: 15_000, points: 3, isCorrect: true }), + ]); + }); + + it("gives the same score whatever timeMs the client claims", async () => { + openQuestion(0); + vi.setSystemTime(START + 5_000); + + await submit({ timeMs: 0 }); + await submit({ timeMs: 19_999 }); + await submit({ timeMs: -500 }); + await submit({ timeMs: "nope" }); + + expect(enqueued().map((a) => [a.timeMs, a.points])).toEqual([ + [5_000, 8], + [5_000, 8], + [5_000, 8], + [5_000, 8], + ]); + }); + + it("still pays the maximum for an answer the instant the question opens", async () => { + openQuestion(0); + + await submit(); + + expect(enqueued()).toEqual([expect.objectContaining({ timeMs: 0, points: 10 })]); + }); + + it("scores a wrong answer 0", async () => { + openQuestion(0); + vi.setSystemTime(START + 1_000); + + await submit({ answerId: "wrong" }); + + expect(enqueued()).toEqual([expect.objectContaining({ isCorrect: false, points: 0 })]); + }); + + it("accepts an answer right on the deadline and floors it at 1 point", async () => { + openQuestion(0); + vi.setSystemTime(START + LIMIT_S * 1_000); + + await submit(); + + expect(enqueued()).toEqual([expect.objectContaining({ timeMs: 20_000, points: 1 })]); + }); +}); + +describe("quiz:submit-answer is only accepted for the live question", () => { + it("rejects an answer after the deadline", async () => { + openQuestion(0); + vi.setSystemTime(START + LIMIT_S * 1_000 + 1); + + await submit(); + + expect(mocks.enqueueAnswer).not.toHaveBeenCalled(); + }); + + it("rejects an answer once the question is closed (deadline key gone)", async () => { + mocks.cache.get.mockResolvedValue(null); + + await submit(); + + expect(mocks.enqueueAnswer).not.toHaveBeenCalled(); + expect(mocks.finishQuestionEarly).not.toHaveBeenCalled(); + }); + + it("rejects an answer for a question that is not the current one", async () => { + openQuestion(1); + + await submit({ questionId: "q1" }); + + expect(mocks.enqueueAnswer).not.toHaveBeenCalled(); + }); + + it("accepts the question that is current", async () => { + openQuestion(1); + + await submit({ questionId: "q2", answerId: "right2" }); + + expect(enqueued()).toEqual([expect.objectContaining({ questionId: "q2" })]); + }); + + it("rejects a question id that does not exist", async () => { + openQuestion(0); + + await submit({ questionId: "made-up" }); + + expect(mocks.enqueueAnswer).not.toHaveBeenCalled(); + }); + + it("rejects when the current-question marker is missing", async () => { + const redis: Record = { + [`deadline:${SESSION}`]: String(START + 10_000), + }; + mocks.cache.get.mockImplementation(async (key: string) => redis[key] ?? null); + + await submit(); + + expect(mocks.enqueueAnswer).not.toHaveBeenCalled(); + }); + + it("rejects a garbage deadline value", async () => { + const redis: Record = { + [`deadline:${SESSION}`]: "not-a-number", + [`current:${SESSION}`]: "0", + }; + mocks.cache.get.mockImplementation(async (key: string) => redis[key] ?? null); + + await submit(); + + expect(mocks.enqueueAnswer).not.toHaveBeenCalled(); + }); + + it("still ignores a message with no participant id", async () => { + openQuestion(0); + + await submit({ participantId: undefined }); + + expect(mocks.enqueueAnswer).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/ws-server/src/utils/handleMessages.ts b/apps/ws-server/src/utils/handleMessages.ts index 0e18385..2377da7 100644 --- a/apps/ws-server/src/utils/handleMessages.ts +++ b/apps/ws-server/src/utils/handleMessages.ts @@ -17,7 +17,13 @@ import { invalidateSession, clearSessionCache, } from './cache.js'; -import { startQuestionTimer, timerDeadlineKey, finishQuestionEarly } from "./timeManager.js"; +import { + startQuestionTimer, + timerDeadlineKey, + currentQuestionKey, + closeQuestionWindow, + finishQuestionEarly, +} from "./timeManager.js"; import { enqueueAnswer, countAnswered, drainSession } from "./answerQueue.js"; import { authorizeOrganizerAction, @@ -372,6 +378,7 @@ export const handleMessage = async (client: Client, data: any) => { // Buffered answers first: the quiz is over, so anything left // in the queue has no later flush to land in and the final // scores would be missing them permanently. + await closeQuestionWindow(sessionId); await drainSession(sessionId); await endQuizSession(sessionId); await clearSessionCache(sessionId); @@ -429,7 +436,9 @@ export const handleMessage = async (client: Client, data: any) => { } case 'quiz:submit-answer': { - const { sessionId, participantId, questionId, answerId, timeMs } = payload; + // timeMs is deliberately not read: the client's own stopwatch can + // say anything. Time used is measured here from the deadline. + const { sessionId, participantId, questionId, answerId } = payload; if (!participantId) break; @@ -438,8 +447,28 @@ export const handleMessage = async (client: Client, data: any) => { // handled below. Asking the database first would add a round-trip // to every answer and still not settle a race. const questions = await getCachedQuestions(sessionId); - const question = questions.find((q: any) => q.id === questionId); - const answerMeta = question?.answers.find((a: any) => a.id === answerId); + const questionIndex = questions.findIndex((q: any) => q.id === questionId); + const question = questions[questionIndex]; + if (!question) break; + + // Only the question that is open right now, and only before its + // deadline, can be answered. finishQuestion deletes both keys the + // moment a question closes and closeQuestionWindow does the same + // when the quiz ends, so a missing deadline means nothing is + // open. Without this, the correct answers broadcast in + // quiz:question-results could be replayed for full points by + // anyone who had skipped the question. + const [rawDeadline, rawCurrent] = await Promise.all([ + cache.get(timerDeadlineKey(sessionId)), + cache.get(currentQuestionKey(sessionId)), + ]); + if (!rawDeadline || rawCurrent === null || Number(rawCurrent) !== questionIndex) break; + + const now = Date.now(); + const deadline = Number(rawDeadline); + if (!Number.isFinite(deadline) || now > deadline) break; + + const answerMeta = question.answers.find((a: any) => a.id === answerId); const isCorrect = !!answerMeta?.isCorrect; @@ -452,11 +481,11 @@ export const handleMessage = async (client: Client, data: any) => { // Now a correct answer is worth MAX_POINTS down to 1, scaled over // the time actually allowed; a wrong answer is worth 0. Answering // at all still scores, so the floor is 1 rather than 0. - const limitMs = Math.max(1, (question?.timeLimit ?? 15) * 1_000); - // timeMs comes from the client and is not trusted for anything - // beyond ordering: clamp it so a negative or oversized value - // cannot mint points or push the score below the floor. - const elapsedMs = Math.min(Math.max(Number(timeMs) || 0, 0), limitMs); + const limitMs = Math.max(1, (question.timeLimit ?? 15) * 1_000); + // Time used = time allowed minus time that was left when the + // answer reached the server. Clamped because the timer and the + // limit are read separately and can disagree by a few ms. + const elapsedMs = Math.min(Math.max(limitMs - (deadline - now), 0), limitMs); const points = isCorrect ? Math.max(1, Math.ceil(MAX_POINTS * (1 - elapsedMs / limitMs))) : 0; @@ -471,7 +500,7 @@ export const handleMessage = async (client: Client, data: any) => { // A false return is a duplicate submission already buffered, // which is what the unique constraint would have rejected anyway. if (!enqueueAnswer(sessionId, { - participantId, questionId, answerId, timeMs, isCorrect, points, + participantId, questionId, answerId, timeMs: elapsedMs, isCorrect, points, })) break; // The leaderboard cache has a 10s TTL and is invalidated again @@ -530,8 +559,7 @@ export const handleMessage = async (client: Client, data: any) => { // session cache is not invalidated when the organizer // advances, so its currentQuestionIndex can be an hour // stale and would close the wrong question. - const idx = questions.findIndex((q: any) => q.id === questionId); - if (idx >= 0) await finishQuestionEarly(sessionId, idx); + await finishQuestionEarly(sessionId, questionIndex); } } @@ -549,6 +577,10 @@ export const handleMessage = async (client: Client, data: any) => { sessionTimers.delete(sessionId); } + // Stop accepting answers before draining the buffer, so nothing + // can be queued after the final board is built. + await closeQuestionWindow(sessionId); + // Buffered answers must land before the final standings are // read, or the last question's scores are missing from the board // everyone finishes on. diff --git a/apps/ws-server/src/utils/timeManager.ts b/apps/ws-server/src/utils/timeManager.ts index 867b741..b42fce5 100644 --- a/apps/ws-server/src/utils/timeManager.ts +++ b/apps/ws-server/src/utils/timeManager.ts @@ -20,7 +20,7 @@ export const timerDeadlineKey = (sessionId: string) => // Which question is in flight, so an early finish can prove it is closing the // question it meant to and not one that already moved on. -const currentQuestionKey = (sessionId: string) => +export const currentQuestionKey = (sessionId: string) => `quiz:current-q:${sessionId}`; // Set once when a question closes, by whichever path gets there first, so the @@ -111,6 +111,18 @@ async function finishQuestion( }, 2_000); } +/** + * Mark no question as open for this session. + * + * Answers are only accepted while the deadline key exists, so removing it is + * what stops late answers once the host ends the quiz. The normal close path + * does this itself in finishQuestion; this is for the paths that stop the + * countdown without going through it. + */ +export async function closeQuestionWindow(sessionId: string) { + await cache.del(timerDeadlineKey(sessionId), currentQuestionKey(sessionId)); +} + /** * End the current question early because everyone present has answered. *