Issue 11 - PR 3 - Challenge.js tweaks

middlewareNode/src/routes/categorys.js

@@ -11,8 +11,6 @@
 
 const express = require("express");
 const router = express.Router();
-const crypto = require("crypto");
-const { check, validationResult } = require("express-validator");
 const categorys = require("../models/categorys");
 
 /**

middlewareNode/src/routes/challenge.js

@@ -13,37 +13,16 @@
  */
 
 const express = require('express');
-const crypto = require('crypto');
 const router = express.Router({ mergeParams: true });
 const requireAuth = require('../middleware/requireAuth');
-
-// challengeId -> { id, gameId, fromUsername, toUsername, status, createdAt }
-// status: "pending" | "accepted" | "declined"
-const challenges = new Map();
-
-// How long a pending/answered challenge lives before it's swept (ms).
-const CHALLENGE_TTL_MS = 2 * 60 * 1000;
-
-/**
- * Drops challenges older than the TTL so the map can't grow without bound.
- * Called opportunistically on each request — no background timer to leak.
- */
-function sweepExpired() {
-    const cutoff = Date.now() - CHALLENGE_TTL_MS;
-    for (const [id, c] of challenges) {
-        if (c.createdAt < cutoff) {
-            challenges.delete(id);
-        }
-    }
-}
+const challengeStore = require('../utils/challengeStore');
 
 /**
  * POST /challenge
  * Body: { fromUsername, toUsername }
  * Creates a pending challenge and returns its id + the reserved gameId.
  */
 router.post('/', requireAuth, (req, res) => {
-    sweepExpired();
     const { fromUsername, toUsername } = req.body || {};
 
     if (!fromUsername || !toUsername) {
@@ -58,59 +37,31 @@ router.post('/', requireAuth, (req, res) => {
         return res.status(403).json({ error: 'Forbidden: cannot create challenge for another user' });
     }
 
-    // Prevent stacking duplicate live challenges between the same pair.
-    for (const c of challenges.values()) {
-        if (
-            c.status === 'pending' &&
-            c.fromUsername === fromUsername &&
-            c.toUsername === toUsername
-        ) {
-            return res.status(200).json({ challengeId: c.id, gameId: c.gameId });
-        }
-    }
-
-    const challenge = {
-        id: crypto.randomUUID(),
-        gameId: crypto.randomUUID(),
-        fromUsername,
-        toUsername,
-        status: 'pending',
-        createdAt: Date.now(),
-    };
-    challenges.set(challenge.id, challenge);
-
-    return res.status(201).json({ challengeId: challenge.id, gameId: challenge.gameId });
+    const { challenge, created } = challengeStore.create(fromUsername, toUsername);
+    return res.status(created ? 201 : 200).json({ challengeId: challenge.id, gameId: challenge.gameId });
 });
 
 /**
  * GET /challenge/incoming/:username
  * Pending challenges addressed to this user (recipient short-poll).
  */
 router.get('/incoming/:username', requireAuth, (req, res) => {
-    sweepExpired();
     const { username } = req.params;
 
     // Enforce identity: caller can only inspect their own incoming challenges unless admin
     if (req.user.role !== 'admin' && req.user.username !== username) {
         return res.status(403).json({ error: "Forbidden: cannot read another user's challenges" });
     }
 
-    const incoming = [];
-    for (const c of challenges.values()) {
-        if (c.status === 'pending' && c.toUsername === username) {
-            incoming.push({ challengeId: c.id, fromUsername: c.fromUsername, gameId: c.gameId });
-        }
-    }
-    return res.status(200).json({ challenges: incoming });
+    return res.status(200).json({ challenges: challengeStore.listIncoming(username) });
 });
 
 /**
  * GET /challenge/:id
  * Current status of a challenge (challenger short-polls for acceptance).
  */
 router.get('/:id', requireAuth, (req, res) => {
-    sweepExpired();
-    const challenge = challenges.get(req.params.id);
+    const challenge = challengeStore.get(req.params.id);
     if (!challenge) {
         return res.status(404).json({ error: 'Challenge not found or expired' });
     }
@@ -138,8 +89,7 @@ router.get('/:id', requireAuth, (req, res) => {
  * Opponent accepts; both sides now share `gameId`.
  */
 router.post('/:id/accept', requireAuth, (req, res) => {
-    sweepExpired();
-    const challenge = challenges.get(req.params.id);
+    const challenge = challengeStore.get(req.params.id);
     if (!challenge) {
         return res.status(404).json({ error: 'Challenge not found or expired' });
     }
@@ -152,20 +102,19 @@ router.post('/:id/accept', requireAuth, (req, res) => {
     if (challenge.status !== 'pending') {
         return res.status(409).json({ error: `Challenge already ${challenge.status}` });
     }
-    challenge.status = 'accepted';
+    const acceptedChallenge = challengeStore.accept(challenge.id);
     return res.status(200).json({
-        gameId: challenge.gameId,
-        challenger: challenge.fromUsername,
-        opponent: challenge.toUsername,
+        gameId: acceptedChallenge.gameId,
+        challenger: acceptedChallenge.fromUsername,
+        opponent: acceptedChallenge.toUsername,
     });
 });
 
 /**
  * POST /challenge/:id/decline
  */
 router.post('/:id/decline', requireAuth, (req, res) => {
-    sweepExpired();
-    const challenge = challenges.get(req.params.id);
+    const challenge = challengeStore.get(req.params.id);
     if (!challenge) {
         return res.status(404).json({ error: 'Challenge not found or expired' });
     }
@@ -182,10 +131,10 @@ router.post('/:id/decline', requireAuth, (req, res) => {
     if (challenge.status !== 'pending') {
         return res.status(409).json({ error: `Challenge already ${challenge.status}` });
     }
-    challenge.status = 'declined';
+    challengeStore.decline(challenge.id);
     return res.status(200).json({ message: 'declined' });
 });
 
 module.exports = router;
 // Exported for unit tests — resets the in-memory store between cases.
-module.exports._reset = () => challenges.clear();
+module.exports._reset = () => challengeStore.reset();

middlewareNode/src/utils/challengeStore.js

@@ -0,0 +1,77 @@
+const crypto = require('crypto');
+
+const challenges = new Map();
+const CHALLENGE_TTL_MS = 2 * 60 * 1000;
+
+function sweepExpired() {
+    const cutoff = Date.now() - CHALLENGE_TTL_MS;
+    for (const [id, challenge] of challenges) {
+        if (challenge.createdAt < cutoff) {
+            challenges.delete(id);
+        }
+    }
+}
+
+function create(fromUsername, toUsername) {
+    sweepExpired();
+    for (const challenge of challenges.values()) {
+        if (
+            challenge.status === 'pending' &&
+            challenge.fromUsername === fromUsername &&
+            challenge.toUsername === toUsername
+        ) {
+            return { challenge, created: false };
+        }
+    }
+
+    const challenge = {
+        id: crypto.randomUUID(),
+        gameId: crypto.randomUUID(),
+        fromUsername,
+        toUsername,
+        status: 'pending',
+        createdAt: Date.now(),
+    };
+    challenges.set(challenge.id, challenge);
+    return { challenge, created: true };
+}
+
+function listIncoming(username) {
+    sweepExpired();
+    return Array.from(challenges.values())
+        .filter((challenge) => challenge.status === 'pending' && challenge.toUsername === username)
+        .map((challenge) => ({
+            challengeId: challenge.id,
+            fromUsername: challenge.fromUsername,
+            gameId: challenge.gameId,
+        }));
+}
+
+function get(id) {
+    sweepExpired();
+    return challenges.get(id);
+}
+
+function accept(id) {
+    const challenge = get(id);
+    if (!challenge || challenge.status !== 'pending') {
+        return null;
+    }
+    challenge.status = 'accepted';
+    return challenge;
+}
+
+function decline(id) {
+    const challenge = get(id);
+    if (!challenge || challenge.status !== 'pending') {
+        return null;
+    }
+    challenge.status = 'declined';
+    return challenge;
+}
+
+function reset() {
+    challenges.clear();
+}
+
+module.exports = { create, listIncoming, get, accept, decline, reset };

middlewareNode/tests/routeSecurity.test.js

@@ -482,6 +482,102 @@ describe("Security Audit Regression: Role & Ownership Authorization (403 Forbidd
       expect(res2.status).toBe(200);
     });
   });
+
+  describe("Challenge Matchmaking Lifecycle", () => {
+    beforeEach(() => {
+      challengeRouter._reset();
+      mockAuthUser = { _id: "u1", username: "alice", role: "student" };
+    });
+
+    test("creates a challenge, exposes it to the recipient, and shares the accepted gameId", async () => {
+      const createRes = await request(app)
+        .post("/challenge")
+        .send({ fromUsername: "alice", toUsername: "bob" });
+
+      expect(createRes.status).toBe(201);
+      expect(createRes.body.challengeId).toBeTruthy();
+      expect(createRes.body.gameId).toBeTruthy();
+
+      mockAuthUser = { _id: "u2", username: "bob", role: "student" };
+      const incomingRes = await request(app).get("/challenge/incoming/bob");
+      expect(incomingRes.status).toBe(200);
+      expect(incomingRes.body.challenges).toEqual([
+        {
+          challengeId: createRes.body.challengeId,
+          fromUsername: "alice",
+          gameId: createRes.body.gameId,
+        },
+      ]);
+
+      const acceptRes = await request(app).post(`/challenge/${createRes.body.challengeId}/accept`);
+      expect(acceptRes.status).toBe(200);
+      expect(acceptRes.body.gameId).toBe(createRes.body.gameId);
+
+      mockAuthUser = { _id: "u1", username: "alice", role: "student" };
+      const statusRes = await request(app).get(`/challenge/${createRes.body.challengeId}`);
+      expect(statusRes.status).toBe(200);
+      expect(statusRes.body).toMatchObject({
+        status: "accepted",
+        gameId: createRes.body.gameId,
+      });
+    });
+
+    test("allows a participant to decline a pending challenge", async () => {
+      const createRes = await request(app)
+        .post("/challenge")
+        .send({ fromUsername: "alice", toUsername: "bob" });
+
+      mockAuthUser = { _id: "u2", username: "bob", role: "student" };
+      const declineRes = await request(app).post(`/challenge/${createRes.body.challengeId}/decline`);
+      expect(declineRes.status).toBe(200);
+      expect(declineRes.body).toEqual({ message: "declined" });
+
+      const statusRes = await request(app).get(`/challenge/${createRes.body.challengeId}`);
+      expect(statusRes.body.status).toBe("declined");
+    });
+
+    test("rejects a second accept with 409", async () => {
+      const createRes = await request(app)
+        .post("/challenge")
+        .send({ fromUsername: "alice", toUsername: "bob" });
+      mockAuthUser = { _id: "u2", username: "bob", role: "student" };
+
+      const firstAccept = await request(app).post(`/challenge/${createRes.body.challengeId}/accept`);
+      const secondAccept = await request(app).post(`/challenge/${createRes.body.challengeId}/accept`);
+
+      expect(firstAccept.status).toBe(200);
+      expect(secondAccept.status).toBe(409);
+    });
+
+    test("rejects a self-challenge with 400", async () => {
+      const res = await request(app)
+        .post("/challenge")
+        .send({ fromUsername: "alice", toUsername: "alice" });
+
+      expect(res.status).toBe(400);
+      expect(res.body.error).toMatch(/cannot challenge yourself/i);
+    });
+
+    test("expires a challenge after the TTL", async () => {
+      const now = Date.now();
+      const dateNow = jest.spyOn(Date, "now").mockReturnValue(now);
+      try {
+        const createRes = await request(app)
+          .post("/challenge")
+          .send({ fromUsername: "alice", toUsername: "bob" });
+        dateNow.mockReturnValue(now + 2 * 60 * 1000 + 1);
+
+        mockAuthUser = { _id: "u2", username: "bob", role: "student" };
+        const incomingRes = await request(app).get("/challenge/incoming/bob");
+        const statusRes = await request(app).get(`/challenge/${createRes.body.challengeId}`);
+
+        expect(incomingRes.body.challenges).toEqual([]);
+        expect(statusRes.status).toBe(404);
+      } finally {
+        dateNow.mockRestore();
+      }
+    });
+  });
 });
 
 describe("Security Audit Regression: Auth Endpoint Body Credentials Hardening", () => {
@@ -534,6 +630,8 @@ describe("Security Audit Regression: Intentionally Public Routes (No Auth Requir
   test("GET /category/list returns 200 without authentication", async () => {
     const res = await request(app).get("/category/list");
     expect(res.status).toBe(200);
+    expect(res.body).toEqual([{ name: "fundamentals" }]);
+    expect(categorys.find).toHaveBeenCalledWith({});
   });
 
   test("GET /puzzles/list and /puzzles/random return 200 without 401", async () => {