fix(rpc): close identity-token fail-open and validation gaps
- importSubjectContext now rejects a non-string/empty selfApp before verifying. verifyJwt skips the audience check entirely when audience is undefined, so an unvalidated selfApp (the natural shape of currentAppName(): string | undefined) accepted every token from every app for every audience. - exportSubjectContext now rejects a non-string/empty targetApp, so an array can no longer mint one token valid at multiple apps. - Both directions now reject a present-but-non-string tenant id instead of silently dropping it (was: callee reads missing tenantId as global/unscoped -> cross-tenant exposure). - importSubjectContext now requires exp to be present and independently bounds accepted token age via a new maxAge/ImportOptions.maxAgeSeconds (default 300s), so a caller cannot mint a long-lived token via a huge ttlSeconds and have it honoured indefinitely. - SubjectContext.callerApp doc now states it is self-asserted (the signing secret is workspace-wide) and must never be an authz input. - index.ts also exports the new ImportOptions type. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -6,11 +6,17 @@ export const RPC_IDENTITY_HEADER = "x-wrnexus-rpc-identity";
|
||||
|
||||
const MIN_SECRET_LENGTH = 32;
|
||||
const DEFAULT_TTL_SECONDS = 60;
|
||||
/** Upper bound on accepted token age, whatever the token's own exp says. */
|
||||
const DEFAULT_MAX_AGE_SECONDS = 300;
|
||||
|
||||
export interface SubjectContext {
|
||||
subjectId: string;
|
||||
tenantId?: string;
|
||||
/** The app that minted the token. */
|
||||
/**
|
||||
* The app that CLAIMS to have minted the token. Self-asserted: the signing
|
||||
* secret is workspace-wide, so any app can set this to any name. Useful for
|
||||
* logs and tracing; NEVER an authorization input.
|
||||
*/
|
||||
callerApp: string;
|
||||
}
|
||||
|
||||
@@ -18,6 +24,15 @@ export interface ExportOptions {
|
||||
ttlSeconds?: number;
|
||||
}
|
||||
|
||||
export interface ImportOptions {
|
||||
/**
|
||||
* Reject a token older than this regardless of its own `exp`, so a caller
|
||||
* that mints with a huge ttlSeconds cannot create a long-lived
|
||||
* impersonation credential the callee will honour. Defaults to 300s.
|
||||
*/
|
||||
maxAgeSeconds?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* The workspace-wide RPC signing secret.
|
||||
*
|
||||
@@ -74,9 +89,26 @@ export async function exportSubjectContext(
|
||||
"WRN-RPC-SUBJECT: subject id must be a non-empty string; coerce numeric ids with String(id).",
|
||||
);
|
||||
}
|
||||
const tenantId = ctx.tenant?.id;
|
||||
// An array here would mint one token valid at SEVERAL apps, defeating the
|
||||
// audience binding that stops app B replaying A's token against app C.
|
||||
if (typeof targetApp !== "string" || targetApp === "") {
|
||||
throw new Error("WRN-RPC-AUDIENCE: targetApp must be a non-empty string.");
|
||||
}
|
||||
// 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 === "")
|
||||
) {
|
||||
throw new Error(
|
||||
"WRN-RPC-TENANT: tenant id must be a non-empty string; coerce numeric ids with String(id).",
|
||||
);
|
||||
}
|
||||
return signJwt(
|
||||
{ sub: rawId, ...(typeof tenantId === "string" && tenantId ? { tenant: tenantId } : {}) },
|
||||
{ sub: rawId, ...(rawTenant ? { tenant: rawTenant as string } : {}) },
|
||||
rpcSecret(),
|
||||
{
|
||||
issuer: callerAppName(),
|
||||
@@ -95,21 +127,41 @@ export async function exportSubjectContext(
|
||||
export async function importSubjectContext(
|
||||
token: string,
|
||||
selfApp: string,
|
||||
options: ImportOptions = {},
|
||||
): Promise<SubjectContext> {
|
||||
const claims = await verifyJwt<{ sub?: string; tenant?: string; iss?: string }>(
|
||||
token,
|
||||
rpcSecret(),
|
||||
{ audience: selfApp },
|
||||
);
|
||||
// verifyJwt SKIPS the audience check entirely when audience is undefined, so
|
||||
// an empty selfApp would disable the only cross-app binding in the system and
|
||||
// accept every token from every app. currentAppName() returns
|
||||
// `string | undefined`, which is exactly how that gets passed by accident.
|
||||
if (typeof selfApp !== "string" || selfApp === "") {
|
||||
throw new Error("WRN-RPC-AUDIENCE: selfApp must be a non-empty string.");
|
||||
}
|
||||
const claims = await verifyJwt<{
|
||||
sub?: string;
|
||||
tenant?: unknown;
|
||||
iss?: string;
|
||||
exp?: number;
|
||||
}>(token, rpcSecret(), {
|
||||
audience: selfApp,
|
||||
maxAge: options.maxAgeSeconds ?? DEFAULT_MAX_AGE_SECONDS,
|
||||
});
|
||||
// verifyJwt only checks exp when it is present, so a token minted without
|
||||
// one never expires. Require it.
|
||||
if (typeof claims.exp !== "number") {
|
||||
throw new Error("WRN-RPC-IDENTITY: token has no expiry.");
|
||||
}
|
||||
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.");
|
||||
}
|
||||
if (claims.tenant !== undefined && (typeof claims.tenant !== "string" || claims.tenant === "")) {
|
||||
throw new Error("WRN-RPC-IDENTITY: token carries an unusable tenant.");
|
||||
}
|
||||
return {
|
||||
subjectId: claims.sub,
|
||||
tenantId: typeof claims.tenant === "string" && claims.tenant ? claims.tenant : undefined,
|
||||
tenantId: claims.tenant as string | undefined,
|
||||
callerApp: claims.iss,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -29,4 +29,4 @@ export {
|
||||
importSubjectContext,
|
||||
rpcSecret,
|
||||
} from "./identity.ts";
|
||||
export type { ExportOptions, SubjectContext } from "./identity.ts";
|
||||
export type { ExportOptions, ImportOptions, SubjectContext } from "./identity.ts";
|
||||
|
||||
@@ -91,6 +91,44 @@ describe("subject context token", () => {
|
||||
).rejects.toThrow();
|
||||
});
|
||||
|
||||
test("an empty selfApp is refused rather than disabling the audience check", async () => {
|
||||
configure();
|
||||
const token = await exportSubjectContext(ctxFor({ id: "u1" }), "billing");
|
||||
// verifyJwt skips the audience check when audience is undefined, so this
|
||||
// would otherwise accept every token from every app.
|
||||
for (const bad of [undefined, "", null]) {
|
||||
await expect(importSubjectContext(token!, bad as never)).rejects.toThrow(/selfApp/);
|
||||
}
|
||||
});
|
||||
|
||||
test("a token with a huge ttl is still rejected once it exceeds maxAge", async () => {
|
||||
configure();
|
||||
const token = await exportSubjectContext(ctxFor({ id: "u1" }), "billing", {
|
||||
ttlSeconds: 31_536_000,
|
||||
});
|
||||
await expect(importSubjectContext(token!, "billing", { maxAgeSeconds: -1 })).rejects.toThrow();
|
||||
});
|
||||
|
||||
test("an array targetApp is refused, so no token is valid at two apps", async () => {
|
||||
configure();
|
||||
await expect(
|
||||
exportSubjectContext(ctxFor({ id: "u1" }), ["billing", "reports"] as never),
|
||||
).rejects.toThrow(/targetApp/);
|
||||
});
|
||||
|
||||
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".
|
||||
for (const tenant of [42, {}, ""]) {
|
||||
await expect(
|
||||
exportSubjectContext(
|
||||
{ user: { id: "u1" }, tenant: { id: tenant }, locals: {} } as unknown as Context,
|
||||
"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