diff --git a/src/web/route-helpers.ts b/src/web/route-helpers.ts index 15966e7b..28f988b6 100644 --- a/src/web/route-helpers.ts +++ b/src/web/route-helpers.ts @@ -365,15 +365,18 @@ function lookupSessionOrFail(ctx: SessionPort, sessionId: string, req: FastifyRe * return, so this returns the persisted record instead. Same ownership * enforcement, same 404-not-403 leak protection — this is that function's * missing other half, not a separate check reimplemented inline. + * + * `req` is REQUIRED for the same reason as on `findSessionOrFail`: an optional + * one would let a caller that forgot it retire another user's row in multi-user mode. */ export function findPersistedSessionOrFail( store: { getSession(id: string): SessionState | null }, sessionId: string, - req?: FastifyRequest + req: FastifyRequest ): SessionState { const persisted = store.getSession(sessionId); if (!persisted) throw sessionNotFoundError(sessionId); - if (req && !canAccessOwned(getAuthUser(req), persisted.owner)) throw sessionNotFoundError(sessionId); + if (!canAccessOwned(getAuthUser(req), persisted.owner)) throw sessionNotFoundError(sessionId); return persisted; } diff --git a/test/ownership-scoping.test.ts b/test/ownership-scoping.test.ts index 62060c0b..dc1b3c31 100644 --- a/test/ownership-scoping.test.ts +++ b/test/ownership-scoping.test.ts @@ -166,18 +166,22 @@ describe('session-scoping helpers (multi-user)', () => { expect(() => findSessionUnscopedOrFail(ctxWith(map), 'missing')).toThrow(); }); - it('findSessionOrFail declares req as REQUIRED, so an unscoped call must use the explicit helper', async () => { - // tsc does not typecheck test/, so pin the signature from the source AST: an + it('findSessionOrFail and findPersistedSessionOrFail declare req as REQUIRED, so an unscoped call must use the explicit helper', async () => { + // tsc does not typecheck test/, so pin the signatures from the source AST: an // optional `req` would make forgetting it compile into an UNSCOPED lookup. const file = path.join(import.meta.dirname, '../src/web/route-helpers.ts'); const src = ts.createSourceFile(file, await fs.readFile(file, 'utf8'), ts.ScriptTarget.Latest, true); - const decl = src.statements.find( - (s): s is ts.FunctionDeclaration => ts.isFunctionDeclaration(s) && s.name?.text === 'findSessionOrFail' - ); - const req = decl?.parameters.find((p) => p.name.getText(src) === 'req'); - expect(req).toBeDefined(); - expect(req?.questionToken).toBeUndefined(); - expect(req?.initializer).toBeUndefined(); + for (const name of ['findSessionOrFail', 'findPersistedSessionOrFail']) { + const decl = src.statements.find( + (s): s is ts.FunctionDeclaration => ts.isFunctionDeclaration(s) && s.name?.text === name + ); + const req = decl?.parameters.find((p) => p.name.getText(src) === 'req'); + expect(req, name).toBeDefined(); + expect(req?.questionToken, name).toBeUndefined(); + expect(req?.initializer, name).toBeUndefined(); + // Exactly `FastifyRequest`: a later `FastifyRequest | undefined` would be optional in all but syntax. + expect(req?.type?.getText(src), name).toBe('FastifyRequest'); + } }); it('per-user session cap counts only the owner sessions', () => {