fix(auth): derive clientIp server-side, drop it from sign-in input
clientIp was part of the public signInInputSchema, so any client could spoof its own rate-limit bucket or dodge IP throttling entirely (audit finding B2). The schema no longer carries it (strict parsing rejects it with BAD_REQUEST); instead the web-next tRPC fetch adapter derives it in createTrpcContext from x-forwarded-for (first hop) / x-real-ip — trust caveat documented — and the router threads ctx.clientIp to the controller as a second, server-only argument typed outside the input schema (SignInRequestContext). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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 ?? ""}`,
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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<ISignInController>(AUTH_SYMBOLS.ISignInController);
|
||||
return ctrl(input);
|
||||
signIn: authProcedure.input(signInInputSchema).mutation(({ input, ctx }) => {
|
||||
const ctrl = authContainer.get<ISignInController>(
|
||||
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<ISignUpController>(AUTH_SYMBOLS.ISignUpController);
|
||||
const ctrl = authContainer.get<ISignUpController>(
|
||||
AUTH_SYMBOLS.ISignUpController,
|
||||
);
|
||||
return ctrl(input);
|
||||
}),
|
||||
|
||||
signOut: authProcedure.input(signOutInputSchema).mutation(({ input }) => {
|
||||
const ctrl = authContainer.get<ISignOutController>(AUTH_SYMBOLS.ISignOutController);
|
||||
const ctrl = authContainer.get<ISignOutController>(
|
||||
AUTH_SYMBOLS.ISignOutController,
|
||||
);
|
||||
return ctrl(input);
|
||||
}),
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<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 });
|
||||
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);
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user