# ADR: Session ownership check and AuthenticatedRequest type ## Status Accepted ## Date 2026-04-28 ## Context `evaluateAnswer` accepted any `sessionId` without verifying it belonged to the requesting user. The only protection was the unguessability of a UUID — security through obscurity. If a user intercepted or guessed another user's `sessionId`, they could submit answers on their behalf. Additionally, protected controller handlers typed their `req` parameter as `Request`, making `session` optional even though `requireAuth` middleware guarantees it is present. This required non-null assertions (`req.session!`) in business logic — a type assertion that could cause a runtime crash if middleware ordering ever changed. ## Decision Store `userId` in `GameSessionData`. Pass `userId` from the controller into both `createGameSession` and `evaluateAnswer`. Assert ownership on evaluation — if the session's `userId` doesn't match the requesting user's ID, throw `NotFoundError`. Introduce `AuthenticatedRequest` to eliminate non-null assertions in protected handlers. ## Options considered ### Option A — AuthenticatedRequest type ✅ Define `AuthenticatedRequest = Request & { session: { session: Session; user: User } }` in `types/express.d.ts`. Use it in protected controller handlers instead of `Request`. Requires a single `as express.RequestHandler` cast at route registration due to Express's type limitations. Chosen because: eliminates dangerous non-null assertions in business logic. The cast at route registration is a necessary cast caused by a third-party library limitation, not uncertain logic. ### Option B — Non-null assertion (`req.session!`) Keep `Request` on all handlers. Assert `req.session!` at every usage. Rejected because: non-null assertions in business logic are dangerous — if middleware ordering ever changes, the assertion silently passes and crashes at runtime. --- ### Option C — NotFoundError (404) on ownership failure ✅ When a session exists but belongs to a different user, throw `NotFoundError` with the same message as a missing session. Chosen because: session IDs are opaque secrets. Returning 403 would confirm to the caller that the session ID is valid and belongs to someone else — information they shouldn't have. This pattern is used by GitHub, AWS, and most security-conscious APIs. ### Option D — ForbiddenError (403) on ownership failure Explicit error that distinguishes "not found" from "not allowed". Rejected because: for user-owned resources identified by opaque IDs, confirming existence to an unauthorised caller is an information leak. 404 is the industry standard for this case. ## Consequences - Alice cannot submit answers for Bob's session — ownership is verified at the service layer - `req.session.user.id` is accessible without non-null assertions in protected handlers - `GameSessionData` now carries `userId` — any future `GameSessionStore` implementation must store and return it - Route registration requires `as express.RequestHandler` cast for protected handlers — one cast per route, in wiring code only - `ValKeyGameSessionStore` must serialise and deserialise `userId` alongside `answers` ## Affected files - `apps/api/src/types/express.d.ts` — `AuthenticatedRequest` type added - `apps/api/src/gameSessionStore/GameSessionStore.ts` — `userId` added to `GameSessionData` - `apps/api/src/gameSessionStore/InMemoryGameSessionStore.test.ts` — updated data fixtures - `apps/api/src/services/gameService.ts` — `userId` parameter added to both functions, ownership assertion in `evaluateAnswer` - `apps/api/src/services/gameService.test.ts` — updated all calls, ownership test added - `apps/api/src/controllers/gameController.ts` — extracts `userId` from `req.session.user.id`, passes to service calls - `apps/api/src/routes/gameRouter.ts` — `as express.RequestHandler` cast at route registration ## References - [OWASP: Insecure Direct Object Reference](https://owasp.org/www-community/attacks/Insecure_Direct_Object_Reference) - [HTTP 403 vs 404 for authorization failures](https://stackoverflow.com/questions/3297048/403-forbidden-vs-401-unauthorized-http-responses) --- ## Setup guide / implementation notes 1. `types/express.d.ts` — add: ```ts export type AuthenticatedRequest = Request & { session: { session: Session; user: User }; }; ``` 2. `GameSessionStore.ts` — add `userId` to `GameSessionData`: ```ts export type GameSessionData = { answers: Map; userId: string }; ``` 3. `gameService.ts` — add `userId` to both function signatures: ```ts export const createGameSession = async ( request: GameRequest, store: GameSessionStore, userId: string, ): Promise ``` Store it on create: ```ts await store.create(sessionId, { answers: answerKey, userId }, 30 * 60 * 1000); ``` Assert on evaluate: ```ts if (!session || session.userId !== userId) { throw new NotFoundError(`Game session not found: ${submission.sessionId}`); } ``` 4. `gameController.ts` — extract from authenticated request: ```ts req.session.user.id ``` 5. `gameRouter.ts` — cast at registration: ```ts router.post("/start", controller.createGame as express.RequestHandler); router.post("/answer", controller.submitAnswer as express.RequestHandler); ```