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 the clean-slate collection set.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016j8z4VHjedXDTjEDNg7qHK
This commit is contained in:
2026-07-13 06:08:22 +02:00
parent f2f24f7bfa
commit 361b28da99
10 changed files with 425 additions and 28 deletions

View File

@@ -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<typeof signInInputSchema>;
/**
* 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<SignInOutput> => {
async (input: SignInInput & SignInRequestContext): Promise<SignInOutput> => {
const { allowed: ipAllowed } = await rateLimit.consume(
"ip",
`signIn:ip:${input.clientIp ?? ""}`,

View File

@@ -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<unknown> => ({
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<string, unknown>, 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();
});
});
});

View File

@@ -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;
}

View File

@@ -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);
});
});

View File

@@ -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<string, number>();
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);
}
}
}

View File

@@ -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", () => {

View File

@@ -14,11 +14,16 @@ import type { ISignOutController } from "../../interface-adapters/controllers/si
import { authProcedure } from "./procedures";
export const authRouter = router({
signIn: authProcedure.input(signInInputSchema).mutation(({ input }) => {
signIn: authProcedure.input(signInInputSchema).mutation(({ input, ctx }) => {
const ctrl = authContainer.get<ISignInController>(
AUTH_SYMBOLS.ISignInController,
);
return ctrl(input);
// 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 }) => {

View File

@@ -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: [
{

View File

@@ -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);

View File

@@ -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,13 +14,21 @@ export type ISignInController = ReturnType<typeof signInController>;
export const signInController =
(signInUseCase: ISignInUseCase) =>
async (input: unknown): Promise<ReturnType<typeof presenter>> => {
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<ReturnType<typeof presenter>> => {
const parsed = signInInputSchema.safeParse(input);
if (!parsed.success) {
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);
};