diff --git a/packages/rpc/src/identity.ts b/packages/rpc/src/identity.ts index 4ddd9e6a..0ce356b5 100644 --- a/packages/rpc/src/identity.ts +++ b/packages/rpc/src/identity.ts @@ -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."); } diff --git a/packages/rpc/test/identity.test.ts b/packages/rpc/test/identity.test.ts index 3ae54565..e439790b 100644 --- a/packages/rpc/test/identity.test.ts +++ b/packages/rpc/test/identity.test.ts @@ -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;