diff --git a/middlewareNode/src/routes/puzzles.js b/middlewareNode/src/routes/puzzles.js index f8f21ce6..caa7d042 100644 --- a/middlewareNode/src/routes/puzzles.js +++ b/middlewareNode/src/routes/puzzles.js @@ -1,9 +1,9 @@ /** * Puzzles Routes - * + * * API endpoints for retrieving chess puzzles from the database. * Puzzles are sourced from Lichess and stored in MongoDB. - * + * * Features: * - Get all puzzles * - Get random selection of puzzles @@ -11,15 +11,17 @@ */ const express = require("express"); +const passport = require("passport"); const router = express.Router(); const puzzles = require("../models/puzzles"); +const { emitPuzzleSolved } = require("../services/currencyEvents"); /** * GET /puzzles/list - * + * * Retrieves all chess puzzles from the database. * Returns array of puzzle objects without MongoDB _id field. - * + * * @returns {Array} Array of all puzzles */ router.get("/list", async (req, res) => { @@ -34,19 +36,21 @@ router.get("/list", async (req, res) => { /** * GET /puzzles/random - * + * * Retrieves a random selection of chess puzzles. * Useful for providing variety in puzzle practice sessions. - * + * * Query Parameters: * - limit: Number of random puzzles to return (default: 20) - * + * * @returns {Array} Array of randomly selected puzzles */ router.get("/random", async (req, res) => { try { const limit = parseInt(req.query.limit) || 20; - const puzzlesArray = await puzzles.aggregate([{ $sample: { size: limit } }]); + const puzzlesArray = await puzzles.aggregate([ + { $sample: { size: limit } }, + ]); res.status(200).json(puzzlesArray); } catch (error) { console.error(error.message); @@ -54,4 +58,118 @@ router.get("/random", async (req, res) => { } }); -module.exports = router; \ No newline at end of file +/** + * True if a submitted move matches the puzzle's expected move for that + * position, using the exact same leniency Puzzles.tsx's handlePlayerMove + * already applies client-side: + * + * playerAttemptedMove === expectedPlayerMove || + * playerAttemptedMove === expectedPlayerMove.substring(0, 4) + * + * i.e. a 4-character move (no promotion piece specified) is accepted even + * when the real solution is 5 characters (e.g. "e7e8q") — the client + * already lets a student complete a promotion puzzle without explicitly + * picking a piece, so the server has to accept that same shape or it + * would reject a move the client UI already treated as solved. Comparison + * is case-insensitive to match how the client builds its own move string. + */ +function moveMatches(submitted, expected) { + const s = (submitted || "").toLowerCase(); + const e = (expected || "").toLowerCase(); + return s === e || s === e.substring(0, 4); +} + +/** + * Verifies a client-submitted move sequence against a puzzle's stored + * answer key (space-separated UCI moves). Returns true only if the + * sequences have the same length and every move matches positionally. + * + * This isn't a complete defense — a client that reads the puzzle's own + * `moves` field (already sent to the client to drive the interactive + * play-through) and submits it verbatim without actually playing will + * still pass. What it does stop is the cheaper, more likely attack this + * route originally allowed outright: looping over puzzleIds with no + * solve data at all. Raising the bar to "must know the real solution + * string," not "must have nothing." + */ +function movesMatch(submitted, expected) { + if (!Array.isArray(submitted)) return false; + + const expectedMoves = (expected || "").trim().split(/\s+/).filter(Boolean); + if (submitted.length === 0 || submitted.length !== expectedMoves.length) { + return false; + } + + return expectedMoves.every((expectedMove, i) => moveMatches(submitted[i], expectedMove)); +} + +/** + * POST /puzzles/solved + * + * Records a successfully completed puzzle as a currency-earning action — + * but only once the submitted move sequence is verified against the + * puzzle's stored answer key. A bare puzzleId is no longer sufficient: + * without this, any authenticated caller could loop over every puzzleId + * in the catalog and farm currency for puzzles never attempted. See the + * limitation noted on movesMatch() above for what this does and doesn't + * defend against. + * + * Body: + * - puzzleId: ID of the completed puzzle + * - moves: array of UCI move strings the player submitted, in order, + * matching the puzzle's own `moves` answer key exactly + * + * Only authenticated users can earn currency. + * Guests can still play puzzles, but there is no account to credit. + */ +router.post( + "/solved", + passport.authenticate("jwt", { session: false }), + async (req, res) => { + try { + const { puzzleId, moves } = req.body; + + if (!puzzleId) { + return res.status(400).json({ + error: "puzzleId is required", + }); + } + if (!Array.isArray(moves) || moves.length === 0) { + return res.status(400).json({ + error: "moves is required and must be a non-empty array of UCI move strings", + }); + } + + const puzzle = await puzzles.findOne({ puzzleId }); + + if (!puzzle) { + return res.status(404).json({ + error: "Puzzle not found", + }); + } + + if (!movesMatch(moves, puzzle.moves)) { + return res.status(400).json({ + error: "Submitted moves do not match the puzzle's solution", + }); + } + + const result = await emitPuzzleSolved({ + userId: req.user._id, + puzzleId, + }); + + return res.status(200).json(result); + } catch (error) { + console.error( + "currencyEvents: failed to emit puzzle.solved:", + error.message, + ); + return res.status(500).json({ + error: "Failed to record puzzle completion", + }); + } + }, +); + +module.exports = router; diff --git a/middlewareNode/src/services/currencyEvents.js b/middlewareNode/src/services/currencyEvents.js index 087408aa..3a7e1cde 100644 --- a/middlewareNode/src/services/currencyEvents.js +++ b/middlewareNode/src/services/currencyEvents.js @@ -37,7 +37,13 @@ const DUPLICATE_KEY_ERROR = 11000; * fresh insert, { emitted: false, duplicate: true } for an already-seen * eventId. */ -async function emit({ eventId, actionKey, userId, metadata = {}, occurredAt = new Date() }) { +async function emit({ + eventId, + actionKey, + userId, + metadata = {}, + occurredAt = new Date(), +}) { try { await ActionEvent.create({ eventId, @@ -79,7 +85,24 @@ async function emitLessonCompleted({ userId, piece, lessonNum, occurredAt }) { }); } +/** + * Emits puzzle.solved after a student successfully completes a puzzle. + * + * eventId is `puzzle::` — deterministic per + * (student, puzzle) pair so a retry cannot award the same puzzle twice. + */ +async function emitPuzzleSolved({ userId, puzzleId, occurredAt }) { + return emit({ + eventId: `puzzle:${userId}:${puzzleId}`, + actionKey: "puzzle.solved", + userId, + metadata: { puzzleId }, + occurredAt, + }); +} + module.exports = { emit, emitLessonCompleted, + emitPuzzleSolved, }; diff --git a/middlewareNode/tests/lessons.currencyEmit.test.js b/middlewareNode/tests/lessons.currencyEmit.test.js index 98ec5a64..a5add64d 100644 --- a/middlewareNode/tests/lessons.currencyEmit.test.js +++ b/middlewareNode/tests/lessons.currencyEmit.test.js @@ -25,7 +25,9 @@ jest.setTimeout(60000); let mongod; beforeAll(async () => { - mongod = await MongoMemoryServer.create({ instance: { launchTimeout: 30000 } }); + mongod = await MongoMemoryServer.create({ + instance: { launchTimeout: 30000 }, + }); await mongoose.connect(mongod.getUri() + "ystem"); }); @@ -65,7 +67,10 @@ app.use(express.json()); // simulates "this is the already-authenticated user" without a real JWT. app.use((req, _res, next) => { if (req.headers["x-test-user-id"]) { - req.__testUser = { _id: req.headers["x-test-user-id"], username: req.headers["x-test-username"] }; + req.__testUser = { + _id: req.headers["x-test-user-id"], + username: req.headers["x-test-username"], + }; } next(); }); @@ -80,9 +85,29 @@ async function seedUser({ username, piece, lessonNumber }) { return result.insertedId; } +async function waitForActionEvent(query, timeoutMs = 2000) { + const start = Date.now(); + + while (Date.now() - start < timeoutMs) { + const event = await ActionEvent.findOne(query); + + if (event) { + return event; + } + + await new Promise((resolve) => setTimeout(resolve, 10)); + } + + return null; +} + describe("GET /lessons/updateLessonCompletion — currency emit", () => { it("emits lesson.completed when the write is genuine forward progress", async () => { - const userId = await seedUser({ username: "alice", piece: "The Fork", lessonNumber: 0 }); + const userId = await seedUser({ + username: "alice", + piece: "The Fork", + lessonNumber: 0, + }); const res = await request(app) .get("/lessons/updateLessonCompletion") @@ -96,7 +121,7 @@ describe("GET /lessons/updateLessonCompletion — currency emit", () => { // — give it a tick to land before asserting. await new Promise((r) => setImmediate(r)); - const stored = await ActionEvent.findOne({ actionKey: "lesson.completed" }); + const stored = await waitForActionEvent({ actionKey: "lesson.completed" }); expect(stored).not.toBeNull(); expect(stored.userId.toString()).toBe(userId.toString()); expect(stored.metadata).toEqual({ piece: "The Fork", lessonNum: 1 }); @@ -105,7 +130,11 @@ describe("GET /lessons/updateLessonCompletion — currency emit", () => { it("does NOT emit when the request doesn't advance progress (304 branch)", async () => { // lessonNumber already at 5; requesting lessonNum=2 (index of an // earlier lesson) can't win the $lt comparison, so modifiedCount is 0. - const userId = await seedUser({ username: "bob", piece: "The Fork", lessonNumber: 5 }); + const userId = await seedUser({ + username: "bob", + piece: "The Fork", + lessonNumber: 5, + }); const res = await request(app) .get("/lessons/updateLessonCompletion") @@ -117,27 +146,48 @@ describe("GET /lessons/updateLessonCompletion — currency emit", () => { await new Promise((r) => setImmediate(r)); - expect(await ActionEvent.countDocuments({ actionKey: "lesson.completed" })).toBe(0); + expect( + await ActionEvent.countDocuments({ actionKey: "lesson.completed" }), + ).toBe(0); }); it("does NOT emit for a repeated request that already succeeded once (idempotent, not double-counted)", async () => { - const userId = await seedUser({ username: "carol", piece: "The Fork", lessonNumber: 0 }); + const userId = await seedUser({ + username: "carol", + piece: "The Fork", + lessonNumber: 0, + }); const query = { piece: "The Fork", lessonNum: "1" }; - const headers = { "x-test-user-id": userId.toString(), "x-test-username": "carol" }; + const headers = { + "x-test-user-id": userId.toString(), + "x-test-username": "carol", + }; - const first = await request(app).get("/lessons/updateLessonCompletion").query(query).set(headers); + const first = await request(app) + .get("/lessons/updateLessonCompletion") + .query(query) + .set(headers); expect(first.status).toBe(200); - await new Promise((r) => setImmediate(r)); + + const firstEvent = await waitForActionEvent({ + actionKey: "lesson.completed", + }); + expect(firstEvent).not.toBeNull(); // Re-sending the exact same request: the DB write itself is now a // no-op (lessonNumber is already 1, not < 1), so this hits the 304 // branch and must not add a second event even if it somehow did. - const second = await request(app).get("/lessons/updateLessonCompletion").query(query).set(headers); + const second = await request(app) + .get("/lessons/updateLessonCompletion") + .query(query) + .set(headers); expect(second.status).toBe(304); await new Promise((r) => setImmediate(r)); - expect(await ActionEvent.countDocuments({ actionKey: "lesson.completed" })).toBe(1); + expect( + await ActionEvent.countDocuments({ actionKey: "lesson.completed" }), + ).toBe(1); }); it("never emits for a guest (unauthenticated) request", async () => { diff --git a/middlewareNode/tests/puzzles.currencyEmit.test.js b/middlewareNode/tests/puzzles.currencyEmit.test.js new file mode 100644 index 00000000..c0583d52 --- /dev/null +++ b/middlewareNode/tests/puzzles.currencyEmit.test.js @@ -0,0 +1,281 @@ +/** + * Real-database integration test for routes/puzzles.js's + * POST /puzzles/solved — specifically the currency-event emit. + * + * Scoped to the property this session's change needs proven: + * authenticated puzzle completion creates a puzzle.solved ActionEvent, + * while invalid/guest requests do not — AND, critically, that completion + * actually requires submitting the puzzle's real solution. The route + * originally accepted a bare puzzleId with no proof of solving it; these + * tests include the regression coverage for that (see "does NOT emit when + * submitted moves don't match the puzzle's solution" below). + */ + +const { MongoMemoryServer } = require("mongodb-memory-server"); +const mongoose = require("mongoose"); + +jest.setTimeout(60000); + +let mongod; + +beforeAll(async () => { + mongod = await MongoMemoryServer.create({ + instance: { launchTimeout: 30000 }, + }); + await mongoose.connect(mongod.getUri() + "ystem"); +}); + +afterAll(async () => { + await mongoose.disconnect(); + await mongod.stop(); +}); + +afterEach(async () => { + const collections = mongoose.connection.collections; + await Promise.all(Object.values(collections).map((c) => c.deleteMany({}))); + jest.restoreAllMocks(); +}); + +jest.mock("passport", () => ({ + authenticate: (_strategy, _opts, callback) => (req, res, next) => { + if (req.headers["x-test-guest"] === "true") { + return callback(null, false, null); + } + + req.user = req.__testUser; + return next(); + }, +})); + +const express = require("express"); +const request = require("supertest"); +const puzzlesRouter = require("../src/routes/puzzles"); +const ActionEvent = require("../src/models/actionEvent"); +const puzzles = require("../src/models/puzzles"); + +const app = express(); +app.use(express.json()); + +app.use((req, _res, next) => { + if (req.headers["x-test-user-id"]) { + req.__testUser = { + _id: req.headers["x-test-user-id"], + username: req.headers["x-test-username"], + }; + } + next(); +}); + +app.use("/puzzles", puzzlesRouter); + +async function waitForActionEvent(query, timeoutMs = 2000) { + const start = Date.now(); + + while (Date.now() - start < timeoutMs) { + const event = await ActionEvent.findOne(query); + + if (event) { + return event; + } + + await new Promise((resolve) => setTimeout(resolve, 10)); + } + + return null; +} + +const SOLUTION_MOVES = "e1g1 e8g8"; + +async function seedPuzzle(puzzleId, moves = SOLUTION_MOVES) { + await puzzles.create({ + puzzleId, + FEN: "r3k2r/ppp2ppp/2n5/1B1p4/3P4/2P5/PP3PPP/R3K2R w KQkq - 0 1", + moves, + }); +} + +describe("POST /puzzles/solved — currency emit", () => { + it("emits puzzle.solved when the submitted moves match the puzzle's solution", async () => { + const userId = new mongoose.Types.ObjectId(); + const puzzleId = "test-puzzle-1"; + + await seedPuzzle(puzzleId); + + const res = await request(app) + .post("/puzzles/solved") + .send({ puzzleId, moves: ["e1g1", "e8g8"] }) + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "alice"); + + expect(res.status).toBe(200); + + const stored = await waitForActionEvent({ + actionKey: "puzzle.solved", + }); + + expect(stored).not.toBeNull(); + expect(stored.eventId).toBe(`puzzle:${userId}:${puzzleId}`); + expect(stored.userId.toString()).toBe(userId.toString()); + expect(stored.metadata).toEqual({ puzzleId }); + }); + + it("accepts a 4-char submitted move against a 5-char (promotion) solution, matching the client's own leniency", async () => { + const userId = new mongoose.Types.ObjectId(); + const puzzleId = "test-puzzle-promo-lenient"; + + // Solution requires promoting to a queen; Puzzles.tsx's + // handlePlayerMove accepts the move even if the player didn't specify + // a promotion piece (playerAttemptedMove === expectedMove.substring(0,4)), + // so the server has to accept that same shape or it would reject a + // move the client UI itself already treated as solved. + await seedPuzzle(puzzleId, "e7e8q"); + + const res = await request(app) + .post("/puzzles/solved") + .send({ puzzleId, moves: ["e7e8"] }) + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "dave"); + + expect(res.status).toBe(200); + }); + + it("rejects the wrong promotion piece even though the first 4 chars match", async () => { + const userId = new mongoose.Types.ObjectId(); + const puzzleId = "test-puzzle-wrong-promo"; + + await seedPuzzle(puzzleId, "e7e8q"); // solution promotes to queen + + const res = await request(app) + .post("/puzzles/solved") + .send({ puzzleId, moves: ["e7e8r"] }) // submitted promotes to rook + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "ivan"); + + expect(res.status).toBe(400); + }); + + it("does NOT emit when the submitted moves don't match the puzzle's solution", async () => { + const userId = new mongoose.Types.ObjectId(); + const puzzleId = "test-puzzle-wrong-moves"; + + await seedPuzzle(puzzleId); + + // This is the regression case: a caller who knows (or guesses) a real + // puzzleId but never actually played — or played incorrectly — must + // be rejected, not credited. + const res = await request(app) + .post("/puzzles/solved") + .send({ puzzleId, moves: ["a1a2", "a7a8"] }) + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "eve"); + + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/do not match/i); + + await new Promise((resolve) => setImmediate(resolve)); + expect(await ActionEvent.countDocuments({})).toBe(0); + }); + + it("does NOT emit when fewer moves are submitted than the solution requires", async () => { + const userId = new mongoose.Types.ObjectId(); + const puzzleId = "test-puzzle-partial"; + + await seedPuzzle(puzzleId); // solution is two moves + + const res = await request(app) + .post("/puzzles/solved") + .send({ puzzleId, moves: ["e1g1"] }) // only the first move + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "frank"); + + expect(res.status).toBe(400); + + await new Promise((resolve) => setImmediate(resolve)); + expect(await ActionEvent.countDocuments({})).toBe(0); + }); + + it("does NOT emit when moves is missing — a bare puzzleId is no longer sufficient", async () => { + const userId = new mongoose.Types.ObjectId(); + const puzzleId = "test-puzzle-no-moves"; + + await seedPuzzle(puzzleId); + + const res = await request(app) + .post("/puzzles/solved") + .send({ puzzleId }) + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "grace"); + + expect(res.status).toBe(400); + + await new Promise((resolve) => setImmediate(resolve)); + expect(await ActionEvent.countDocuments({})).toBe(0); + }); + + it("does NOT emit when moves is an empty array", async () => { + const userId = new mongoose.Types.ObjectId(); + const puzzleId = "test-puzzle-empty-moves"; + + await seedPuzzle(puzzleId); + + const res = await request(app) + .post("/puzzles/solved") + .send({ puzzleId, moves: [] }) + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "heidi"); + + expect(res.status).toBe(400); + + await new Promise((resolve) => setImmediate(resolve)); + expect(await ActionEvent.countDocuments({})).toBe(0); + }); + + it("does NOT emit when the puzzle does not exist", async () => { + const userId = new mongoose.Types.ObjectId(); + + const res = await request(app) + .post("/puzzles/solved") + .send({ puzzleId: "does-not-exist", moves: ["e1g1", "e8g8"] }) + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "bob"); + + expect(res.status).toBe(404); + + await new Promise((resolve) => setImmediate(resolve)); + + expect(await ActionEvent.countDocuments({})).toBe(0); + }); + + it("does NOT emit when puzzleId is missing", async () => { + const userId = new mongoose.Types.ObjectId(); + + const res = await request(app) + .post("/puzzles/solved") + .send({ moves: ["e1g1", "e8g8"] }) + .set("x-test-user-id", userId.toString()) + .set("x-test-username", "carol"); + + expect(res.status).toBe(400); + + await new Promise((resolve) => setImmediate(resolve)); + + expect(await ActionEvent.countDocuments({})).toBe(0); + }); + + it("does NOT emit for a guest request", async () => { + const puzzleId = "guest-puzzle"; + + await seedPuzzle(puzzleId); + + const res = await request(app) + .post("/puzzles/solved") + .send({ puzzleId, moves: ["e1g1", "e8g8"] }) + .set("x-test-guest", "true"); + + expect(res.status).not.toBe(200); + + await new Promise((resolve) => setImmediate(resolve)); + + expect(await ActionEvent.countDocuments({})).toBe(0); + }); +}); diff --git a/react-ystemandchess/src/features/puzzles/Puzzles.tsx b/react-ystemandchess/src/features/puzzles/Puzzles.tsx index 3a22153a..fa64ae49 100644 --- a/react-ystemandchess/src/features/puzzles/Puzzles.tsx +++ b/react-ystemandchess/src/features/puzzles/Puzzles.tsx @@ -56,6 +56,12 @@ const Puzzles: React.FC = ({ // Refs const chessBoardRef = useRef(null); const moveListRef = useRef([]); + // Accumulates the player's own moves as they're confirmed correct, + // separately from moveListRef (which shrinks via .shift() as the + // remaining solution is consumed, so it's empty by the time a puzzle + // completes). This is what gets submitted to POST /puzzles/solved for + // server-side verification. + const playedMovesRef = useRef([]); const isPuzzleEndRef = useRef(false); const currentPuzzleRef = useRef(null); const isInitializingRef = useRef(false); @@ -161,6 +167,7 @@ const Puzzles: React.FC = ({ const firstPuzzle = puzzles[0]; currentPuzzleRef.current = firstPuzzle; moveListRef.current = firstPuzzle?.Moves?.split(" ") || []; + playedMovesRef.current = []; if (moveListRef.current.length === 0) { console.warn("No valid moves in initial puzzle:", firstPuzzle); @@ -206,6 +213,7 @@ const Puzzles: React.FC = ({ setCurrentFEN(normalizedFen); moveListRef.current = puzzle?.Moves?.split(" ") || []; + playedMovesRef.current = []; isPuzzleEndRef.current = false; setHighlightSquares([]); @@ -293,6 +301,45 @@ const Puzzles: React.FC = ({ } }; + /** + * Reports a completed puzzle to POST /puzzles/solved so the student + * earns currency for it (see middlewareNode/src/routes/puzzles.js). + * Submits the move sequence actually played — verified server-side + * against the puzzle's stored solution — rather than just the + * puzzleId, so a request can't claim credit for a puzzle it never + * solved. Guests never earn currency (no account to credit), so this + * is skipped entirely for them rather than sent and rejected. + */ + const reportPuzzleSolved = async () => { + if (status === "guest" || !cookies.login) return; + + const puzzleId = currentPuzzleRef.current?.puzzleId; + if (!puzzleId) return; + + try { + const res = await fetch(`${environment.urls.middlewareURL}/puzzles/solved`, { + method: "POST", + headers: { + Authorization: `Bearer ${cookies.login}`, + "Content-Type": "application/json", + }, + body: JSON.stringify({ + puzzleId, + moves: playedMovesRef.current, + }), + }); + if (!res.ok) { + const body = await res.json().catch(() => ({})); + console.error("Failed to record puzzle completion:", body.error || res.status); + } + } catch (error) { + // Currency crediting is a side effect of completing the puzzle, not + // a prerequisite for the UI's own success state — a network error + // here shouldn't block or roll back the "Puzzle completed" modal. + console.error("Failed to record puzzle completion:", error); + } + }; + const handlePlayerMove = (move: Move) => { if ( isPuzzleEndRef.current || @@ -310,6 +357,11 @@ const Puzzles: React.FC = ({ playerAttemptedMove === expectedPlayerMove.substring(0, 4); if (isCorrect) { + // Record the verified move (the expected UCI string, not the raw + // attempt, so promotion suffixes etc. match exactly what the + // puzzle's own solution — and the server-side check — expect) before + // shifting it off moveListRef. + playedMovesRef.current.push(expectedPlayerMove); moveListRef.current.shift(); setHighlightSquares([move.from, move.to]); @@ -326,6 +378,7 @@ const Puzzles: React.FC = ({ if (moveListRef.current.length === 0) { isPuzzleEndRef.current = true; socket.sendMessage("puzzle completed"); + reportPuzzleSolved(); setTimeout(() => { setModal({ type: "success", @@ -368,6 +421,7 @@ const Puzzles: React.FC = ({ if (data && data.type === "puzzle_data") { if (status === "guest") { moveListRef.current = data.moves?.split(" ") || []; + playedMovesRef.current = []; setThemeList(data.themes?.split(" ") || []); currentPuzzleRef.current = { FEN: data.fen,