From ac0bf80eca8e086da1e0c3263ed36e41590f092c Mon Sep 17 00:00:00 2001 From: Danijel Martinek Date: Sun, 12 Jul 2026 23:53:43 +0200 Subject: [PATCH] fix(auth): port server-side session revocation and sign-in hardening Ports the upstream auth audit fixes onto the kept auth feature: - revoke sessions server-side via an in-memory jti denylist (B5): createSession embeds the session id as the JWT jti, invalidateSession denylists it for the max token lifetime, validateSession rejects denylisted and jti-less (fail-closed) tokens; constant-time signature comparison (B4). Adds session-denylist.ts + test. - cover signToken/verifyToken/validateSession crypto paths without a running Payload by stubbing the payload module (B8). - derive clientIp server-side from trusted proxy headers and drop it from the public sign-in input schema; thread it as a server-only request context argument so a client can no longer spoof its rate-limit bucket (B2). - declare the auth-injected email (and displayName) in the users collection-level DSR pii map so Art. 15 export and Art. 17 soft delete cover them (A5). Adapted to our collection set (no username field). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_016j8z4VHjedXDTjEDNg7qHK --- .../application/use-cases/sign-in.use-case.ts | 16 +- .../services/authentication.service.test.ts | 161 +++++++++++++++++- .../services/authentication.service.ts | 62 +++++-- .../services/session-denylist.test.ts | 44 +++++ .../services/session-denylist.ts | 46 +++++ .../auth/src/integrations/api/router.test.ts | 41 ++++- packages/auth/src/integrations/api/router.ts | 21 ++- .../src/integrations/cms/collections/users.ts | 21 +++ .../controllers/sign-in.controller.test.ts | 40 +++++ .../controllers/sign-in.controller.ts | 17 +- 10 files changed, 437 insertions(+), 32 deletions(-) create mode 100644 packages/auth/src/infrastructure/services/session-denylist.test.ts create mode 100644 packages/auth/src/infrastructure/services/session-denylist.ts diff --git a/packages/auth/src/application/use-cases/sign-in.use-case.ts b/packages/auth/src/application/use-cases/sign-in.use-case.ts index bf8ef72..dadb7b1 100644 --- a/packages/auth/src/application/use-cases/sign-in.use-case.ts +++ b/packages/auth/src/application/use-cases/sign-in.use-case.ts @@ -11,15 +11,27 @@ import type { IUsersRepository } from "../repositories/users.repository.interfac import type { IAuthenticationService } from "../services/authentication.service.interface"; // ── Input ──────────────────────────────────────────────────────────────── +// `.strict()` + no clientIp field: a client submitting clientIp is rejected +// at the procedure boundary (audit finding B2). export const signInInputSchema = z .object({ username: z.string().min(3).max(31), password: z.string().min(6).max(255), - clientIp: z.string().optional(), }) .strict(); export type SignInInput = z.infer; +/** + * Server-derived per-request context, typed OUTSIDE the public input schema + * so it can never be client-supplied (audit finding B2). The tRPC adapter + * derives `clientIp` from trusted proxy headers and the controller threads + * it through; `undefined` means "no proxy header present" and falls into a + * shared bucket. + */ +export type SignInRequestContext = { + clientIp?: string; +}; + // ── Output ─────────────────────────────────────────────────────────────── export const signInOutputSchema = z.object({ session: sessionSchema, @@ -36,7 +48,7 @@ export const signInUseCase = authenticationService: IAuthenticationService, rateLimit: IRateLimit, ) => - async (input: SignInInput): Promise => { + async (input: SignInInput & SignInRequestContext): Promise => { const { allowed: ipAllowed } = await rateLimit.consume( "ip", `signIn:ip:${input.clientIp ?? ""}`, diff --git a/packages/auth/src/infrastructure/services/authentication.service.test.ts b/packages/auth/src/infrastructure/services/authentication.service.test.ts index 3007fc6..a207a69 100644 --- a/packages/auth/src/infrastructure/services/authentication.service.test.ts +++ b/packages/auth/src/infrastructure/services/authentication.service.test.ts @@ -1,7 +1,38 @@ -import { describe, it, expect } from "vitest"; +import crypto from "node:crypto"; +import { describe, it, expect, vi, afterEach } from "vitest"; import { AuthenticationService } from "@/infrastructure/services/authentication.service"; import { stubPayloadConfig } from "@repo/core-testing/payload/stub-config"; +// The session methods only need `payload.secret` + `payload.findByID`, so a +// module-level stub covers the pure crypto paths without booting Payload. +const payloadStub = vi.hoisted(() => ({ + secret: "test-secret", + findByID: vi.fn( + async ({ id }: { collection: string; id: string }): Promise => ({ + id, + username: "alice", + passwordHash: "stored-hash", + }), + ), +})); + +vi.mock("payload", () => ({ + getPayload: vi.fn(async () => payloadStub), +})); + +/** Craft a HS256 JWT directly so tests can control every claim (B8). */ +function craftToken(payload: Record, secret: string): string { + const header = Buffer.from( + JSON.stringify({ alg: "HS256", typ: "JWT" }), + ).toString("base64url"); + const body = Buffer.from(JSON.stringify(payload)).toString("base64url"); + const signature = crypto + .createHmac("sha256", secret) + .update(`${header}.${body}`) + .digest("base64url"); + return `${header}.${body}.${signature}`; +} + describe("AuthenticationService", () => { const service = new AuthenticationService(stubPayloadConfig); @@ -42,11 +73,16 @@ describe("AuthenticationService", () => { }); }); - describe("session methods (require Payload)", () => { - // createSession and validateSession call getPayload() internally, - // so they require a running Payload instance. These are exercised - // by the mock service in use-case tests and by integration tests. - // Here we only test invalidateSession (no Payload dependency). + describe("session methods", () => { + // getPayload is mocked module-wide (secret + findByID only), so these + // exercise the real signToken/verifyToken/validateSession crypto paths + // without a running Payload instance (audit finding B8). + const user = { id: "u1", username: "alice", passwordHash: "stored-hash" }; + + afterEach(() => { + vi.useRealTimers(); + payloadStub.secret = "test-secret"; + }); it("invalidateSession returns a blank cookie with maxAge 0", async () => { const { blankCookie } = await service.invalidateSession("any-token"); @@ -56,5 +92,118 @@ describe("AuthenticationService", () => { expect(blankCookie.attributes.httpOnly).toBe(true); expect(blankCookie.attributes.path).toBe("/"); }); + + it("createSession then validateSession round-trips (jti = session.id)", async () => { + const svc = new AuthenticationService(stubPayloadConfig); + const { session, cookie } = await svc.createSession(user); + const validated = await svc.validateSession(cookie.value); + expect(validated.user.id).toBe("u1"); + expect(validated.session.userId).toBe("u1"); + // The session id is the JWT jti, minted once at createSession (B5). + expect(validated.session.id).toBe(session.id); + }); + + it("rejects a token with a tampered signature", async () => { + const svc = new AuthenticationService(stubPayloadConfig); + const { cookie } = await svc.createSession(user); + const [header, body] = cookie.value.split("."); + const forged = `${header}.${body}.${Buffer.from("forged-signature").toString("base64url")}`; + await expect(svc.validateSession(forged)).rejects.toThrow( + /invalid or expired/i, + ); + }); + + it("rejects a token whose payload was swapped after signing", async () => { + const svc = new AuthenticationService(stubPayloadConfig); + const { cookie } = await svc.createSession(user); + const [header, , signature] = cookie.value.split("."); + const swappedBody = Buffer.from( + JSON.stringify({ + id: "attacker", + collection: "users", + exp: Math.floor(Date.now() / 1000) + 9999, + jti: "attacker-jti", + }), + ).toString("base64url"); + await expect( + svc.validateSession(`${header}.${swappedBody}.${signature}`), + ).rejects.toThrow(/invalid or expired/i); + }); + + it("rejects an expired token", async () => { + vi.useFakeTimers(); + const svc = new AuthenticationService(stubPayloadConfig); + const { cookie } = await svc.createSession(user); + vi.advanceTimersByTime(3 * 60 * 60 * 1000); // 3h > 2h session duration + await expect(svc.validateSession(cookie.value)).rejects.toThrow( + /invalid or expired/i, + ); + }); + + it.each([ + ["empty string", ""], + ["one segment", "not-a-jwt"], + ["two segments", "aaaa.bbbb"], + ["four segments", "a.b.c.d"], + ["garbage segments", "!!.??.%%"], + ])("rejects a malformed token (%s)", async (_label, token) => { + const svc = new AuthenticationService(stubPayloadConfig); + await expect(svc.validateSession(token)).rejects.toThrow( + /invalid or expired/i, + ); + }); + + it("rejects a token signed with the wrong secret", async () => { + const svc = new AuthenticationService(stubPayloadConfig); + const token = craftToken( + { + id: "u1", + collection: "users", + exp: Math.floor(Date.now() / 1000) + 600, + jti: "jti-1", + }, + "some-other-secret", + ); + await expect(svc.validateSession(token)).rejects.toThrow( + /invalid or expired/i, + ); + }); + + it("rejects a correctly signed token without a jti (fail closed)", async () => { + const svc = new AuthenticationService(stubPayloadConfig); + const token = craftToken( + { + id: "u1", + collection: "users", + exp: Math.floor(Date.now() / 1000) + 600, + }, + "test-secret", + ); + await expect(svc.validateSession(token)).rejects.toThrow( + /invalid or expired/i, + ); + }); + + it("rejects a valid token after its session is invalidated (B5)", async () => { + const svc = new AuthenticationService(stubPayloadConfig); + const { session, cookie } = await svc.createSession(user); + // Sanity: valid before revocation. + await expect(svc.validateSession(cookie.value)).resolves.toBeDefined(); + + await svc.invalidateSession(session.id); + await expect(svc.validateSession(cookie.value)).rejects.toThrow( + /revoked/i, + ); + }); + + it("revocation is per-service-instance (documented single-process limit)", async () => { + const svcA = new AuthenticationService(stubPayloadConfig); + const svcB = new AuthenticationService(stubPayloadConfig); + const { session, cookie } = await svcA.createSession(user); + await svcA.invalidateSession(session.id); + // A separate instance (≈ another process) still accepts the token — + // this pins the documented in-memory denylist limitation. + await expect(svcB.validateSession(cookie.value)).resolves.toBeDefined(); + }); }); }); diff --git a/packages/auth/src/infrastructure/services/authentication.service.ts b/packages/auth/src/infrastructure/services/authentication.service.ts index 8451824..11372a2 100644 --- a/packages/auth/src/infrastructure/services/authentication.service.ts +++ b/packages/auth/src/infrastructure/services/authentication.service.ts @@ -4,6 +4,7 @@ import type { IAuthenticationService } from "../../application/services/authenti import type { Cookie } from "../../entities/models/cookie"; import type { Session } from "../../entities/models/session"; import type { User } from "../../entities/models/user"; +import { InMemorySessionDenylist } from "./session-denylist"; const SALT_LENGTH = 16; const KEY_LENGTH = 64; @@ -15,7 +16,12 @@ const COOKIE_NAME = "payload-token"; const SESSION_DURATION_SECONDS = 7200; // 2 hours (matches Payload default) export class AuthenticationService implements IAuthenticationService { - constructor(private config: SanitizedConfig) {} + constructor( + private config: SanitizedConfig, + // Server-side revocation (audit finding B5). In-memory: revocations are + // per-process — see session-denylist.ts for the limitation write-up. + private denylist: InMemorySessionDenylist = new InMemorySessionDenylist(), + ) {} generateUserId(): string { return crypto.randomUUID(); @@ -69,10 +75,13 @@ export class AuthenticationService implements IAuthenticationService { const payload = await getPayload({ config: this.config }); const expiresAt = new Date(Date.now() + SESSION_DURATION_SECONDS * 1000); - const token = this.signToken(user.id, payload.secret); + // The session id doubles as the JWT `jti` so the token can be revoked + // server-side via the denylist (audit finding B5). + const sessionId = crypto.randomUUID(); + const token = this.signToken(user.id, sessionId, payload.secret); const session: Session = { - id: crypto.randomUUID(), + id: sessionId, userId: user.id, expiresAt, }; @@ -97,6 +106,11 @@ export class AuthenticationService implements IAuthenticationService { const payload = await getPayload({ config: this.config }); const decoded = this.verifyToken(token, payload.secret); if (!decoded) throw new Error("Invalid or expired session token"); + // Server-side revocation check (audit finding B5): a signed, unexpired + // token is still rejected once its jti has been invalidated. + if (this.denylist.isRevoked(decoded.jti)) { + throw new Error("Session has been revoked"); + } const userDoc = await payload.findByID({ collection: "users" as const, @@ -111,7 +125,7 @@ export class AuthenticationService implements IAuthenticationService { }; const session: Session = { - id: token, + id: decoded.jti, userId: user.id, expiresAt: new Date(decoded.exp * 1000), }; @@ -119,9 +133,11 @@ export class AuthenticationService implements IAuthenticationService { return { user, session }; } - async invalidateSession( - _sessionId: string, - ): Promise<{ blankCookie: Cookie }> { + async invalidateSession(sessionId: string): Promise<{ blankCookie: Cookie }> { + // `sessionId` is the JWT `jti` (the `session.id` returned by + // createSession/validateSession). Denylist it for the maximum token + // lifetime — beyond that, the token's own `exp` rejects it (B5). + this.denylist.revoke(sessionId, SESSION_DURATION_SECONDS); return { blankCookie: { name: COOKIE_NAME, @@ -138,13 +154,13 @@ export class AuthenticationService implements IAuthenticationService { } /** Sign a HS256 JWT using Payload's instance secret. No external dependency. */ - private signToken(userId: string, secret: string): string { + private signToken(userId: string, jti: string, secret: string): string { const header = Buffer.from( JSON.stringify({ alg: "HS256", typ: "JWT" }), ).toString("base64url"); const exp = Math.floor(Date.now() / 1000) + SESSION_DURATION_SECONDS; const body = Buffer.from( - JSON.stringify({ id: userId, collection: "users", exp }), + JSON.stringify({ id: userId, collection: "users", exp, jti }), ).toString("base64url"); const signature = crypto .createHmac("sha256", secret) @@ -157,22 +173,38 @@ export class AuthenticationService implements IAuthenticationService { private verifyToken( token: string, secret: string, - ): { id: string; exp: number } | null { + ): { id: string; exp: number; jti: string } | null { const parts = token.split("."); if (parts.length !== 3) return null; const [header, body, signature] = parts as [string, string, string]; const expected = crypto .createHmac("sha256", secret) .update(`${header}.${body}`) - .digest("base64url"); - if (signature !== expected) return null; + .digest(); + const provided = Buffer.from(signature, "base64url"); + // Constant-time comparison, mirroring verifyPassword (audit finding B4). + // timingSafeEqual requires equal-length buffers; a length mismatch is + // already an invalid signature, and the guard leaks nothing an attacker + // does not know (the expected HMAC-SHA256 length is public). + if (provided.length !== expected.length) return null; + if (!crypto.timingSafeEqual(provided, expected)) return null; try { const decoded = JSON.parse(Buffer.from(body, "base64url").toString()) as { - id: string; - exp: number; + id?: unknown; + exp?: unknown; + jti?: unknown; }; + // Fail closed: tokens without a jti cannot be revoked, so they are + // not accepted (audit finding B5). + if ( + typeof decoded.id !== "string" || + typeof decoded.exp !== "number" || + typeof decoded.jti !== "string" + ) { + return null; + } if (decoded.exp < Math.floor(Date.now() / 1000)) return null; - return decoded; + return { id: decoded.id, exp: decoded.exp, jti: decoded.jti }; } catch { return null; } diff --git a/packages/auth/src/infrastructure/services/session-denylist.test.ts b/packages/auth/src/infrastructure/services/session-denylist.test.ts new file mode 100644 index 0000000..8b308aa --- /dev/null +++ b/packages/auth/src/infrastructure/services/session-denylist.test.ts @@ -0,0 +1,44 @@ +import { describe, it, expect } from "vitest"; +import { InMemorySessionDenylist } from "@/infrastructure/services/session-denylist"; + +describe("InMemorySessionDenylist", () => { + it("reports a revoked jti as revoked", () => { + const denylist = new InMemorySessionDenylist(); + denylist.revoke("jti-1", 60); + expect(denylist.isRevoked("jti-1")).toBe(true); + }); + + it("does not report unknown jtis as revoked", () => { + const denylist = new InMemorySessionDenylist(); + expect(denylist.isRevoked("never-seen")).toBe(false); + }); + + it("prunes entries after their ttl elapses", () => { + let now = 1_000_000; + const denylist = new InMemorySessionDenylist(() => now); + denylist.revoke("jti-1", 60); + expect(denylist.isRevoked("jti-1")).toBe(true); + + now += 60_000; // exactly at expiry — entry is prunable + expect(denylist.isRevoked("jti-1")).toBe(false); + }); + + it("keeps entries alive until the ttl elapses", () => { + let now = 1_000_000; + const denylist = new InMemorySessionDenylist(() => now); + denylist.revoke("jti-1", 60); + now += 59_999; + expect(denylist.isRevoked("jti-1")).toBe(true); + }); + + it("prunes expired entries on revoke, not just on reads", () => { + let now = 1_000_000; + const denylist = new InMemorySessionDenylist(() => now); + denylist.revoke("old", 1); + now += 5_000; + denylist.revoke("new", 60); + // Reach into nothing — observable via isRevoked semantics only. + expect(denylist.isRevoked("old")).toBe(false); + expect(denylist.isRevoked("new")).toBe(true); + }); +}); diff --git a/packages/auth/src/infrastructure/services/session-denylist.ts b/packages/auth/src/infrastructure/services/session-denylist.ts new file mode 100644 index 0000000..6f40655 --- /dev/null +++ b/packages/auth/src/infrastructure/services/session-denylist.ts @@ -0,0 +1,46 @@ +/** + * In-memory JWT `jti` denylist backing server-side session revocation + * (audit finding B5). + * + * `AuthenticationService.createSession` mints a session id and embeds it in + * the JWT as `jti`; `invalidateSession(jti)` records it here and + * `validateSession` rejects any token whose `jti` is denylisted. Entries + * expire with the token they revoke (max session lifetime), so the map is + * self-pruning and cannot grow past the number of sign-outs per lifetime + * window. + * + * SINGLE-PROCESS LIMITATION: this denylist lives in process memory. It is + * correct for a single server process (the template's deployment shape) but + * revocations are NOT shared across processes/instances and do not survive + * restarts — a restarted process accepts a signed, unexpired token again. + * Multi-instance deployments must swap this for a shared store (Redis, DB) + * behind the same two methods. + */ +export class InMemorySessionDenylist { + /** jti -> epoch-ms after which the entry may be pruned. */ + private readonly revoked = new Map(); + + constructor(private readonly clock: () => number = () => Date.now()) {} + + /** + * Record a revoked `jti`. `ttlSeconds` should be the maximum remaining + * token lifetime — after that, the token's own `exp` rejects it anyway. + */ + revoke(jti: string, ttlSeconds: number): void { + this.prune(); + this.revoked.set(jti, this.clock() + ttlSeconds * 1000); + } + + isRevoked(jti: string): boolean { + this.prune(); + return this.revoked.has(jti); + } + + /** Expiry-based pruning — runs on every access; the map stays small. */ + private prune(): void { + const now = this.clock(); + for (const [jti, expiresAt] of this.revoked) { + if (expiresAt <= now) this.revoked.delete(jti); + } + } +} diff --git a/packages/auth/src/integrations/api/router.test.ts b/packages/auth/src/integrations/api/router.test.ts index ad9af3a..745f297 100644 --- a/packages/auth/src/integrations/api/router.test.ts +++ b/packages/auth/src/integrations/api/router.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach } from "vitest"; +import { describe, it, expect, beforeEach, vi } from "vitest"; import { TRPCError } from "@trpc/server"; import { authRouter } from "@/integrations/api/router"; @@ -27,6 +27,45 @@ describe("authRouter", () => { }); expect(result.name).toBe("session"); }); + + it("rejects a client-supplied clientIp at the procedure boundary (B2)", async () => { + const caller = authRouter.createCaller({}); + try { + await caller.signIn({ + username: "alice", + password: "password_alice", + clientIp: "6.6.6.6", + } as never); + throw new Error("expected throw"); + } catch (e) { + expect(e).toBeInstanceOf(TRPCError); + expect((e as TRPCError).code).toBe("BAD_REQUEST"); + } + }); + + it("threads ctx.clientIp (server-derived) into the controller (B2)", async () => { + const original = authContainer.get(AUTH_SYMBOLS.ISignInController); + authContainer.unbind(AUTH_SYMBOLS.ISignInController); + const spy = vi.fn(async () => ({ + name: "session", + value: "tok", + attributes: {}, + })); + authContainer.bind(AUTH_SYMBOLS.ISignInController).toConstantValue(spy); + try { + const caller = authRouter.createCaller({ clientIp: "203.0.113.7" }); + await caller.signIn({ username: "alice", password: "password_alice" }); + expect(spy).toHaveBeenCalledWith( + { username: "alice", password: "password_alice" }, + { clientIp: "203.0.113.7" }, + ); + } finally { + authContainer.unbind(AUTH_SYMBOLS.ISignInController); + authContainer + .bind(AUTH_SYMBOLS.ISignInController) + .toConstantValue(original); + } + }); }); describe("authRouter error mapping", () => { diff --git a/packages/auth/src/integrations/api/router.ts b/packages/auth/src/integrations/api/router.ts index 5ebf5e2..aca7d8e 100644 --- a/packages/auth/src/integrations/api/router.ts +++ b/packages/auth/src/integrations/api/router.ts @@ -14,18 +14,29 @@ import type { ISignOutController } from "../../interface-adapters/controllers/si import { authProcedure } from "./procedures"; export const authRouter = router({ - signIn: authProcedure.input(signInInputSchema).mutation(({ input }) => { - const ctrl = authContainer.get(AUTH_SYMBOLS.ISignInController); - return ctrl(input); + signIn: authProcedure.input(signInInputSchema).mutation(({ input, ctx }) => { + const ctrl = authContainer.get( + AUTH_SYMBOLS.ISignInController, + ); + // clientIp is derived server-side by the adapter's createContext (from + // trusted proxy headers) — never from the client payload; the strict + // input schema rejects a client-supplied clientIp outright (B2). Same + // ctx-cast pattern as the dsr router until the shared t is context-typed. + const { clientIp } = ctx as { clientIp?: string }; + return ctrl(input, { clientIp }); }), signUp: authProcedure.input(signUpInputSchema).mutation(({ input }) => { - const ctrl = authContainer.get(AUTH_SYMBOLS.ISignUpController); + const ctrl = authContainer.get( + AUTH_SYMBOLS.ISignUpController, + ); return ctrl(input); }), signOut: authProcedure.input(signOutInputSchema).mutation(({ input }) => { - const ctrl = authContainer.get(AUTH_SYMBOLS.ISignOutController); + const ctrl = authContainer.get( + AUTH_SYMBOLS.ISignOutController, + ); return ctrl(input); }), }); diff --git a/packages/auth/src/integrations/cms/collections/users.ts b/packages/auth/src/integrations/cms/collections/users.ts index 7fd69b2..0b3227d 100644 --- a/packages/auth/src/integrations/cms/collections/users.ts +++ b/packages/auth/src/integrations/cms/collections/users.ts @@ -16,6 +16,27 @@ export const users: CollectionConfig = { }, }, subject: { kind: "self", field: "id" }, + // Collection-level PII map consumed by the DSR walkers (audit finding + // A5): export includes fields marked exportable; the soft-delete path + // redacts them. `email` is auto-added by Payload's `auth: true` and has + // no explicit field entry below, so it MUST be declared here or Art. 15 + // export misses it and Art. 17 soft delete leaves it behind. `displayName` + // is declared here (in addition to its field-level tag) because the DSR + // walkers read only this collection-level map. + pii: { + email: { + category: "contact-email", + purpose: ["account-authentication", "transactional-notifications"], + exportable: true, + restrictable: true, + }, + displayName: { + category: "identification-username", + purpose: ["service-delivery"], + exportable: true, + restrictable: true, + }, + }, }, fields: [ { diff --git a/packages/auth/src/interface-adapters/controllers/sign-in.controller.test.ts b/packages/auth/src/interface-adapters/controllers/sign-in.controller.test.ts index f98c20f..0148805 100644 --- a/packages/auth/src/interface-adapters/controllers/sign-in.controller.test.ts +++ b/packages/auth/src/interface-adapters/controllers/sign-in.controller.test.ts @@ -6,6 +6,7 @@ import { MockAuthenticationService } from "@/infrastructure/services/authenticat import { InputParseError } from "@/entities/errors/common"; import { userFactory } from "@/__factories__/user.factory"; import { NoopRateLimit } from "@repo/core-shared/rate-limit"; +import { RecordingRateLimit } from "@repo/core-testing/rate-limit"; describe("signInController", () => { it("returns a cookie on successful sign-in", async () => { @@ -28,6 +29,45 @@ describe("signInController", () => { expect(result.value).toBeTruthy(); }); + it("threads the server-derived clientIp into the use case (B2)", async () => { + const users = new MockUsersRepository([]); + const auth = new MockAuthenticationService(users); + const rl = new RecordingRateLimit(); + await users.createUser( + userFactory.build({ + username: "alice", + passwordHash: "hashed_testpassword", + }), + ); + + const controller = signInController(signInUseCase(users, auth, rl)); + await controller( + { username: "alice", password: "testpassword" }, + { clientIp: "203.0.113.7" }, + ); + + expect(rl.consumeCalls[0]).toMatchObject({ + budgetName: "ip", + key: "signIn:ip:203.0.113.7", + }); + }); + + it("rejects clientIp inside the client payload (strict schema, B2)", async () => { + const users = new MockUsersRepository([]); + const auth = new MockAuthenticationService(users); + const controller = signInController( + signInUseCase(users, auth, new NoopRateLimit()), + ); + + await expect( + controller({ + username: "alice", + password: "testpassword", + clientIp: "6.6.6.6", + }), + ).rejects.toBeInstanceOf(InputParseError); + }); + it("throws InputParseError on invalid input", async () => { const users = new MockUsersRepository([]); const auth = new MockAuthenticationService(users); diff --git a/packages/auth/src/interface-adapters/controllers/sign-in.controller.ts b/packages/auth/src/interface-adapters/controllers/sign-in.controller.ts index 88f555e..e25fffa 100644 --- a/packages/auth/src/interface-adapters/controllers/sign-in.controller.ts +++ b/packages/auth/src/interface-adapters/controllers/sign-in.controller.ts @@ -3,6 +3,7 @@ import { signInInputSchema, type ISignInUseCase, type SignInOutput, + type SignInRequestContext, } from "../../application/use-cases/sign-in.use-case"; function presenter(value: SignInOutput) { @@ -13,11 +14,21 @@ export type ISignInController = ReturnType; export const signInController = (signInUseCase: ISignInUseCase) => - async (input: unknown): Promise> => { + async ( + input: unknown, + // Server-derived, never part of the client-facing input schema (B2): + // the tRPC adapter builds it from trusted proxy headers. + requestContext?: SignInRequestContext, + ): Promise> => { const parsed = signInInputSchema.safeParse(input); if (!parsed.success) { - throw new InputParseError("Invalid sign-in input", { cause: parsed.error }); + throw new InputParseError("Invalid sign-in input", { + cause: parsed.error, + }); } - const result = await signInUseCase(parsed.data); + const result = await signInUseCase({ + ...parsed.data, + clientIp: requestContext?.clientIp, + }); return presenter(result); };