fix(rpc): close the iat fail-open and tighten the identity guards
verifyJwt gates its maxAge check on iat being a number, so a token forged without iat was honoured at any maxAgeSeconds - the same shape as the audience and exp fail-opens closed in the previous round. A future-dated iat did the same via a negative age. Both refused now. The import side never checked aud was a single string, and verifyJwt compares with includes(), so a multi-audience token verified at several apps. The mint-side guard's invariant now holds where it is enforced. ctx.tenant present with a null id minted an authenticated credential with no tenant claim, which the callee reads as global. Absent ctx.tenant means untenanted; a present tenant with an unusable id is an error. Adds six tests pinning behaviours that mutation testing showed were free to delete without any test noticing: no-exp, no-iat, the 300s default max age, an array audience on import, a non-string tenant claim on import, and a null tenant id at mint. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -97,12 +97,12 @@ export async function exportSubjectContext(
|
||||
// A numeric tenant id is the common DB-backed case. Dropping it silently
|
||||
// would leave the callee reading "no tenant" as "global", which is a
|
||||
// cross-tenant exposure — so refuse it the same way a bad subject is refused.
|
||||
const rawTenant: unknown = ctx.tenant?.id;
|
||||
if (
|
||||
rawTenant !== undefined &&
|
||||
rawTenant !== null &&
|
||||
(typeof rawTenant !== "string" || rawTenant === "")
|
||||
) {
|
||||
// Absent ctx.tenant means untenanted (fine); a PRESENT tenant with an
|
||||
// unusable id (including null) is an error, not a silent downgrade — unlike
|
||||
// rawId === null, which mints no token at all, a bad tenant must not issue
|
||||
// an authenticated credential with silently widened scope.
|
||||
const rawTenant: unknown = ctx.tenant === undefined ? undefined : ctx.tenant.id;
|
||||
if (rawTenant !== undefined && (typeof rawTenant !== "string" || rawTenant === "")) {
|
||||
throw new Error(
|
||||
"WRN-RPC-TENANT: tenant id must be a non-empty string; coerce numeric ids with String(id).",
|
||||
);
|
||||
@@ -141,6 +141,8 @@ export async function importSubjectContext(
|
||||
tenant?: unknown;
|
||||
iss?: string;
|
||||
exp?: number;
|
||||
iat?: number;
|
||||
aud?: unknown;
|
||||
}>(token, rpcSecret(), {
|
||||
audience: selfApp,
|
||||
maxAge: options.maxAgeSeconds ?? DEFAULT_MAX_AGE_SECONDS,
|
||||
@@ -150,12 +152,26 @@ export async function importSubjectContext(
|
||||
if (typeof claims.exp !== "number") {
|
||||
throw new Error("WRN-RPC-IDENTITY: token has no expiry.");
|
||||
}
|
||||
// Same shape one level down: verifyJwt's maxAge check is gated on iat being
|
||||
// a number, so a token minted without iat silently defeats the age bound at
|
||||
// ANY maxAgeSeconds. A future-dated iat yields a negative age and does the
|
||||
// same. Both must be refused for maxAge to mean anything.
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
if (typeof claims.iat !== "number" || claims.iat > now + 60) {
|
||||
throw new Error("WRN-RPC-IDENTITY: token has no usable issued-at.");
|
||||
}
|
||||
if (typeof claims.sub !== "string" || claims.sub === "") {
|
||||
throw new Error("WRN-RPC-IDENTITY: token carries no usable subject.");
|
||||
}
|
||||
if (typeof claims.iss !== "string" || claims.iss === "") {
|
||||
throw new Error("WRN-RPC-IDENTITY: token names no calling app.");
|
||||
}
|
||||
// verifyJwt compares audience with includes(), so a token signed with
|
||||
// audience: ["billing", "reports"] verifies at BOTH — exactly what I3
|
||||
// exists to prevent. Require an exact single-audience match.
|
||||
if (claims.aud !== selfApp) {
|
||||
throw new Error("WRN-RPC-IDENTITY: token is addressed to more than this app.");
|
||||
}
|
||||
if (claims.tenant !== undefined && (typeof claims.tenant !== "string" || claims.tenant === "")) {
|
||||
throw new Error("WRN-RPC-IDENTITY: token carries an unusable tenant.");
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { afterEach, describe, expect, test } from "bun:test";
|
||||
import type { Context } from "@wrnexus/core";
|
||||
import { signJwt } from "@wrnexus/jwt";
|
||||
import { exportSubjectContext, importSubjectContext } from "../src/identity.ts";
|
||||
|
||||
const SECRET = "test-rpc-secret-at-least-32-chars-long";
|
||||
@@ -71,6 +72,28 @@ describe("subject context token", () => {
|
||||
await expect(importSubjectContext(token!, "billing")).rejects.toThrow();
|
||||
});
|
||||
|
||||
test("a forged token with no exp is refused", async () => {
|
||||
configure();
|
||||
// verifyJwt only checks exp when present, so a token minted without one
|
||||
// never expires unless importSubjectContext requires it explicitly.
|
||||
const token = await signJwt({ sub: "u1" }, SECRET, { issuer: "web", audience: "billing" });
|
||||
await expect(importSubjectContext(token, "billing")).rejects.toThrow(/expiry/i);
|
||||
});
|
||||
|
||||
test("a forged token with no iat is refused", async () => {
|
||||
configure();
|
||||
// signJwt always sets iat unless the payload explicitly overrides it with
|
||||
// undefined (JSON.stringify then drops the key). verifyJwt's maxAge check
|
||||
// is gated on iat being a number, so this would otherwise defeat the age
|
||||
// bound at ANY maxAgeSeconds.
|
||||
const token = await signJwt({ sub: "u1", iat: undefined }, SECRET, {
|
||||
issuer: "web",
|
||||
audience: "billing",
|
||||
expiresIn: 60,
|
||||
});
|
||||
await expect(importSubjectContext(token, "billing")).rejects.toThrow(/issued-at/i);
|
||||
});
|
||||
|
||||
test("a token signed with a different secret is rejected", async () => {
|
||||
configure();
|
||||
const token = await exportSubjectContext(ctxFor({ id: "u1" }), "billing");
|
||||
@@ -109,6 +132,27 @@ describe("subject context token", () => {
|
||||
await expect(importSubjectContext(token!, "billing", { maxAgeSeconds: -1 })).rejects.toThrow();
|
||||
});
|
||||
|
||||
test("a backdated iat is refused under the default max age, and accepted once maxAgeSeconds is raised", async () => {
|
||||
configure();
|
||||
// Age 400s, older than DEFAULT_MAX_AGE_SECONDS (300s). Calling with NO
|
||||
// options (the real default) must reject -- unlike the huge-ttl test
|
||||
// above, which always passes an explicit maxAgeSeconds: -1 and so never
|
||||
// actually exercises the 300s constant itself.
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
const token = await signJwt({ sub: "u1" }, SECRET, {
|
||||
issuer: "web",
|
||||
audience: "billing",
|
||||
expiresIn: 1000,
|
||||
now: now - 400,
|
||||
});
|
||||
await expect(importSubjectContext(token, "billing")).rejects.toThrow();
|
||||
// Same token, wider explicit bound: proves maxAgeSeconds is actually
|
||||
// wired through rather than the guard being a hardcoded rejection.
|
||||
await expect(
|
||||
importSubjectContext(token, "billing", { maxAgeSeconds: 1000 }),
|
||||
).resolves.toBeDefined();
|
||||
});
|
||||
|
||||
test("an array targetApp is refused, so no token is valid at two apps", async () => {
|
||||
configure();
|
||||
await expect(
|
||||
@@ -116,6 +160,20 @@ describe("subject context token", () => {
|
||||
).rejects.toThrow(/targetApp/);
|
||||
});
|
||||
|
||||
test("a token signed with an array audience is refused on import", async () => {
|
||||
configure();
|
||||
// verifyJwt compares audience with includes(), so a token signed with
|
||||
// audience: ["billing", "reports"] would verify at BOTH apps -- exactly
|
||||
// what the audience binding exists to prevent -- unless importSubjectContext
|
||||
// requires an exact single-audience match itself.
|
||||
const token = await signJwt({ sub: "u1" }, SECRET, {
|
||||
issuer: "web",
|
||||
audience: ["billing", "reports"],
|
||||
expiresIn: 60,
|
||||
});
|
||||
await expect(importSubjectContext(token, "billing")).rejects.toThrow();
|
||||
});
|
||||
|
||||
test("a non-string tenant id is refused rather than silently dropped", async () => {
|
||||
configure();
|
||||
// Silently dropping it leaves the callee reading "no tenant" as "global".
|
||||
@@ -129,6 +187,29 @@ describe("subject context token", () => {
|
||||
}
|
||||
});
|
||||
|
||||
test("ctx.tenant = { id: null } is refused at mint rather than minting untenanted", async () => {
|
||||
configure();
|
||||
// Unlike rawId === null (mints no token at all), a present tenant with an
|
||||
// unusable id must not silently issue an authenticated credential with
|
||||
// widened (untenanted/global) scope.
|
||||
await expect(
|
||||
exportSubjectContext(
|
||||
{ user: { id: "u1" }, tenant: { id: null }, locals: {} } as unknown as Context,
|
||||
"billing",
|
||||
),
|
||||
).rejects.toThrow(/tenant/i);
|
||||
});
|
||||
|
||||
test("a forged token whose tenant claim is a number is refused on import", async () => {
|
||||
configure();
|
||||
const token = await signJwt({ sub: "u1", tenant: 42 }, SECRET, {
|
||||
issuer: "web",
|
||||
audience: "billing",
|
||||
expiresIn: 60,
|
||||
});
|
||||
await expect(importSubjectContext(token, "billing")).rejects.toThrow(/tenant/i);
|
||||
});
|
||||
|
||||
test("a missing secret is a setup error, not a silent pass", async () => {
|
||||
process.env.WRNEXUS_APP_NAME = "web";
|
||||
delete process.env.WRNEXUS_RPC_SECRET;
|
||||
|
||||
Reference in New Issue
Block a user