Fix round 3 for Task 7 (N5, minor-to-important): fix round 2's encodeURI(options.redirectTo) fixed the non-ASCII crash but broke the most common real use of redirectTo -- a return-path query param that's already percent-encoded (e.g. /login?next=%2Fdash) -- because encodeURI also escapes "%", double-encoding it to %252Fdash. Replaced with headerSafePath(), a codepoint loop that encodes only codepoints above 0x7f (matching isLocalPath's style: no regex, no source escapes) and leaves "%" alone. Added tests: an already-percent-encoded target round-trips unchanged; a non-ASCII target still 303s without throwing and the location is ASCII-only; a plain ASCII target passes through byte-identical.
389 lines
15 KiB
TypeScript
389 lines
15 KiB
TypeScript
import { describe, expect, test } from "bun:test";
|
|
import type { Context } from "@wrnexus/core";
|
|
import { defineAuthz } from "../src/registry.ts";
|
|
import { mergeCatalogs } from "../src/catalog.ts";
|
|
import { memoryPermissionStore } from "../src/store.ts";
|
|
import { authzMiddleware, can, filterCan, guardPermission } from "../src/middleware.ts";
|
|
|
|
const catalog = mergeCatalogs([
|
|
{
|
|
source: "t.ts",
|
|
module: defineAuthz({
|
|
permissions: { "post:read": { public: true }, "post:write": {}, "post:delete": {} },
|
|
roles: { editor: ["post:write"] },
|
|
policies: {
|
|
ownsPost: async (s: { id?: string }, r?: { authorId?: string }) =>
|
|
r?.authorId === s?.id ? { allowed: true } : { allowed: false, reason: "not owner" },
|
|
},
|
|
bindings: { "post:delete": ["ownsPost"] },
|
|
}),
|
|
},
|
|
]);
|
|
|
|
/** Minimal Context stand-in; the middleware only touches user, tenant, locals. */
|
|
function makeCtx(user: unknown, tenantId?: string): Context {
|
|
return {
|
|
user,
|
|
tenant: tenantId ? { id: tenantId } : undefined,
|
|
locals: {},
|
|
url: new URL("http://localhost/x"),
|
|
req: new Request("http://localhost/x"),
|
|
} as unknown as Context;
|
|
}
|
|
|
|
const withMiddleware = async (ctx: Context, store = memoryPermissionStore()) => {
|
|
await authzMiddleware({ catalog, store, strict: false })(ctx, async () => new Response("ok"));
|
|
return store;
|
|
};
|
|
|
|
describe("authzMiddleware + can", () => {
|
|
test("can() resolves through the middleware-installed resolver", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
const store = memoryPermissionStore();
|
|
await store.assignRole("u1", "editor");
|
|
await withMiddleware(ctx, store);
|
|
expect(await can(ctx, "post:write")).toBe(true);
|
|
expect(await can(ctx, "post:delete", { authorId: "u1" })).toBe(false);
|
|
});
|
|
|
|
test("can() throws a clear setup error without the middleware", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
await expect(can(ctx, "post:read")).rejects.toThrow(/authzMiddleware/);
|
|
});
|
|
|
|
test("results are memoised per request", async () => {
|
|
const inner = memoryPermissionStore();
|
|
let reads = 0;
|
|
const counting = {
|
|
...inner,
|
|
assignmentsFor: (id: string, scope?: { tenantId?: string }) => {
|
|
reads++;
|
|
return inner.assignmentsFor(id, scope);
|
|
},
|
|
};
|
|
const ctx = makeCtx({ id: "u1" });
|
|
await authzMiddleware({ catalog, store: counting, strict: false })(
|
|
ctx,
|
|
async () => new Response("ok"),
|
|
);
|
|
await can(ctx, "post:write");
|
|
await can(ctx, "post:write");
|
|
expect(reads).toBe(1);
|
|
});
|
|
|
|
test("memoisation keys on the resource, not just the permission", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
const store = memoryPermissionStore();
|
|
await store.grant("u1", "post:delete", "allow");
|
|
await withMiddleware(ctx, store);
|
|
expect(await can(ctx, "post:delete", { authorId: "u1" })).toBe(true);
|
|
expect(await can(ctx, "post:delete", { authorId: "other" })).toBe(false);
|
|
});
|
|
|
|
test("the tenant on the context becomes the scope", async () => {
|
|
const ctx = makeCtx({ id: "u1" }, "t1");
|
|
const store = memoryPermissionStore();
|
|
await store.assignRole("u1", "editor", { tenantId: "t1" });
|
|
await withMiddleware(ctx, store);
|
|
expect(await can(ctx, "post:write")).toBe(true);
|
|
});
|
|
});
|
|
|
|
describe("guardPermission", () => {
|
|
test("calls next when allowed", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
const store = memoryPermissionStore();
|
|
await store.assignRole("u1", "editor");
|
|
await withMiddleware(ctx, store);
|
|
const res = await guardPermission("post:write")(ctx, async () => new Response("passed"));
|
|
expect(await res.text()).toBe("passed");
|
|
});
|
|
|
|
test("returns 403 without leaking the reason by default", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
await withMiddleware(ctx);
|
|
const res = await guardPermission("post:write")(ctx, async () => new Response("passed"));
|
|
expect(res.status).toBe(403);
|
|
const body = (await res.json()) as Record<string, unknown>;
|
|
expect(body).toEqual({ ok: false, error: "Forbidden" });
|
|
});
|
|
|
|
test("exposeReason opts into diagnostics", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
await withMiddleware(ctx);
|
|
const res = await guardPermission("post:write", { exposeReason: true })(
|
|
ctx,
|
|
async () => new Response("passed"),
|
|
);
|
|
const body = (await res.json()) as Record<string, unknown>;
|
|
expect(body.reason).toBe("Missing permission");
|
|
});
|
|
|
|
test("getResource feeds the bound policy", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
const store = memoryPermissionStore();
|
|
await store.grant("u1", "post:delete", "allow");
|
|
await withMiddleware(ctx, store);
|
|
const guard = guardPermission("post:delete", { getResource: () => ({ authorId: "u1" }) });
|
|
const res = await guard(ctx, async () => new Response("passed"));
|
|
expect(await res.text()).toBe("passed");
|
|
});
|
|
});
|
|
|
|
describe("filterCan", () => {
|
|
test("keeps only the items the subject may act on", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
const store = memoryPermissionStore();
|
|
await store.grant("u1", "post:delete", "allow");
|
|
await withMiddleware(ctx, store);
|
|
const posts = [{ authorId: "u1" }, { authorId: "other" }, { authorId: "u1" }];
|
|
expect(await filterCan(ctx, "post:delete", posts)).toHaveLength(2);
|
|
});
|
|
|
|
test("handles BigInt fields and circular references without leaking", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
const store = memoryPermissionStore();
|
|
await store.grant("u1", "post:delete", "allow");
|
|
await withMiddleware(ctx, store);
|
|
|
|
const mine = { authorId: "u1", views: 10n } as Record<string, unknown>;
|
|
const other = { authorId: "other", views: 11n } as Record<string, unknown>;
|
|
const circularMine = { authorId: "u1" } as Record<string, unknown>;
|
|
circularMine.self = circularMine;
|
|
const circularOther = { authorId: "other" } as Record<string, unknown>;
|
|
circularOther.self = circularOther;
|
|
|
|
const result = await filterCan(ctx, "post:delete", [mine, other, circularMine, circularOther]);
|
|
expect(result).toEqual([mine, circularMine]);
|
|
});
|
|
|
|
test("returns an empty array for an empty input", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
await withMiddleware(ctx);
|
|
expect(await filterCan(ctx, "post:delete", [])).toEqual([]);
|
|
});
|
|
});
|
|
|
|
describe("memoisation does not cross-authorize distinct resources", () => {
|
|
test("a numeric id and a string id on different resources do not collide", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
const store = memoryPermissionStore();
|
|
await store.grant("u1", "post:delete", "allow");
|
|
await withMiddleware(ctx, store);
|
|
expect(await can(ctx, "post:delete", { id: 7, authorId: "u1" })).toBe(true);
|
|
expect(await can(ctx, "post:delete", { id: "7", authorId: "other" })).toBe(false);
|
|
});
|
|
|
|
test("resources with object-shaped ids do not collide", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
const store = memoryPermissionStore();
|
|
await store.grant("u1", "post:delete", "allow");
|
|
await withMiddleware(ctx, store);
|
|
expect(await can(ctx, "post:delete", { id: { tenant: "A" }, authorId: "u1" })).toBe(true);
|
|
expect(await can(ctx, "post:delete", { id: { tenant: "B" }, authorId: "other" })).toBe(false);
|
|
});
|
|
|
|
test("two distinct resource objects sharing the same id value do not share a verdict", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
const store = memoryPermissionStore();
|
|
await store.grant("u1", "post:delete", "allow");
|
|
await withMiddleware(ctx, store);
|
|
expect(await can(ctx, "post:delete", { id: 1, authorId: "u1" })).toBe(true);
|
|
expect(await can(ctx, "post:delete", { id: 1, authorId: "other" })).toBe(false);
|
|
});
|
|
|
|
test("switching ctx.tenant mid-request changes the scope for subsequent checks", async () => {
|
|
const ctx = makeCtx({ id: "u1" }, "t1");
|
|
const store = memoryPermissionStore();
|
|
await store.assignRole("u1", "editor", { tenantId: "t1" });
|
|
await withMiddleware(ctx, store);
|
|
expect(await can(ctx, "post:write")).toBe(true);
|
|
(ctx as unknown as { tenant?: { id: string } }).tenant = { id: "t2" };
|
|
expect(await can(ctx, "post:write")).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe("memoisation does not cross-authorize distinct subjects", () => {
|
|
test("swapping ctx.user mid-request re-evaluates for the new subject", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
const store = memoryPermissionStore();
|
|
await store.grant("u1", "post:delete", "allow");
|
|
await withMiddleware(ctx, store);
|
|
const resource = { authorId: "u1" };
|
|
expect(await can(ctx, "post:delete", resource)).toBe(true);
|
|
(ctx as unknown as { user?: unknown }).user = { id: "u2" };
|
|
expect(await can(ctx, "post:delete", resource)).toBe(false);
|
|
});
|
|
|
|
test("clearing ctx.user mid-request denies rather than replaying the old verdict", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
const store = memoryPermissionStore();
|
|
await store.assignRole("u1", "editor");
|
|
await withMiddleware(ctx, store);
|
|
expect(await can(ctx, "post:write")).toBe(true);
|
|
(ctx as unknown as { user?: unknown }).user = null;
|
|
expect(await can(ctx, "post:write")).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe("memoisation identity edge cases", () => {
|
|
test("two distinct symbols with the same description do not share a verdict", async () => {
|
|
const approved = Symbol("row");
|
|
const other = Symbol("row");
|
|
const localCatalog = mergeCatalogs([
|
|
{
|
|
source: "symbol-identity-test.ts",
|
|
module: defineAuthz({
|
|
permissions: { "sym:pick": {} },
|
|
policies: {
|
|
isApproved: async (_s: unknown, r?: unknown) =>
|
|
r === approved ? { allowed: true } : { allowed: false, reason: "not approved" },
|
|
},
|
|
bindings: { "sym:pick": ["isApproved"] },
|
|
}),
|
|
},
|
|
]);
|
|
const ctx = makeCtx({ id: "u1" });
|
|
const store = memoryPermissionStore();
|
|
await store.grant("u1", "sym:pick", "allow");
|
|
await authzMiddleware({ catalog: localCatalog, store, strict: false })(
|
|
ctx,
|
|
async () => new Response("ok"),
|
|
);
|
|
expect(await can(ctx, "sym:pick", approved)).toBe(true);
|
|
expect(await can(ctx, "sym:pick", other)).toBe(false);
|
|
});
|
|
|
|
test("0 and -0 do not share a verdict", async () => {
|
|
const localCatalog = mergeCatalogs([
|
|
{
|
|
source: "negative-zero-test.ts",
|
|
module: defineAuthz({
|
|
permissions: { "zero:pick": {} },
|
|
policies: {
|
|
isPositiveZero: async (_s: unknown, r?: unknown) =>
|
|
Object.is(r, 0) ? { allowed: true } : { allowed: false, reason: "not +0" },
|
|
},
|
|
bindings: { "zero:pick": ["isPositiveZero"] },
|
|
}),
|
|
},
|
|
]);
|
|
const ctx = makeCtx({ id: "u1" });
|
|
const store = memoryPermissionStore();
|
|
await store.grant("u1", "zero:pick", "allow");
|
|
await authzMiddleware({ catalog: localCatalog, store, strict: false })(
|
|
ctx,
|
|
async () => new Response("ok"),
|
|
);
|
|
expect(await can(ctx, "zero:pick", 0)).toBe(true);
|
|
expect(await can(ctx, "zero:pick", -0)).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe("guardPermission hardening", () => {
|
|
test("throws the setup error and never calls next without the middleware", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
let called = false;
|
|
await expect(
|
|
guardPermission("post:write")(ctx, async () => {
|
|
called = true;
|
|
return new Response("passed");
|
|
}),
|
|
).rejects.toThrow(/authzMiddleware/);
|
|
expect(called).toBe(false);
|
|
});
|
|
|
|
test("a throwing getResource denies with the standard body, not the loader's message", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
await withMiddleware(ctx);
|
|
const guard = guardPermission("post:delete", {
|
|
getResource: () => {
|
|
throw new Error("SELECT * FROM posts WHERE id = 1 -- boom");
|
|
},
|
|
});
|
|
const res = await guard(ctx, async () => new Response("passed"));
|
|
expect(res.status).toBe(403);
|
|
const body = (await res.json()) as Record<string, unknown>;
|
|
expect(body).toEqual({ ok: false, error: "Forbidden" });
|
|
});
|
|
|
|
test("redirectTo issues a 303 for a page request", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
await withMiddleware(ctx);
|
|
const res = await guardPermission("post:write", { redirectTo: "/login" })(
|
|
ctx,
|
|
async () => new Response("passed"),
|
|
);
|
|
expect(res.status).toBe(303);
|
|
expect(res.headers.get("location")).toBe("/login");
|
|
expect(res.headers.get("cache-control")).toBe("private, no-store");
|
|
});
|
|
|
|
test("a non-ASCII redirectTo returns 303 without throwing, and the location is ASCII-only", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
await withMiddleware(ctx);
|
|
// Built at runtime via String.fromCodePoint (no non-ASCII characters
|
|
// typed into the source) per the repo-wide constraint.
|
|
const target = "/" + String.fromCodePoint(0x65e5) + String.fromCodePoint(0x672c);
|
|
const res = await guardPermission("post:write", { redirectTo: target })(
|
|
ctx,
|
|
async () => new Response("passed"),
|
|
);
|
|
expect(res.status).toBe(303);
|
|
const location = res.headers.get("location");
|
|
expect(location).not.toBeNull();
|
|
for (const ch of location ?? "") {
|
|
expect(ch.codePointAt(0)! <= 0x7f).toBe(true);
|
|
}
|
|
});
|
|
|
|
test("an already-percent-encoded redirectTo round-trips unchanged", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
await withMiddleware(ctx);
|
|
const res = await guardPermission("post:write", { redirectTo: "/login?next=%2Fdash" })(
|
|
ctx,
|
|
async () => new Response("passed"),
|
|
);
|
|
expect(res.status).toBe(303);
|
|
const location = res.headers.get("location");
|
|
expect(location).toBe("/login?next=%2Fdash");
|
|
expect(location).not.toContain("%25");
|
|
});
|
|
|
|
test("a plain ASCII redirectTo is passed through byte-identical", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
await withMiddleware(ctx);
|
|
const res = await guardPermission("post:write", { redirectTo: "/login?next=/dashboard" })(
|
|
ctx,
|
|
async () => new Response("passed"),
|
|
);
|
|
expect(res.status).toBe(303);
|
|
expect(res.headers.get("location")).toBe("/login?next=/dashboard");
|
|
});
|
|
|
|
test("redirectTo is ignored for an /api/ request, which gets 403 instead", async () => {
|
|
const ctx = {
|
|
user: { id: "u1" },
|
|
tenant: undefined,
|
|
locals: {},
|
|
url: new URL("http://localhost/api/x"),
|
|
req: new Request("http://localhost/api/x"),
|
|
} as unknown as Context;
|
|
await withMiddleware(ctx);
|
|
const res = await guardPermission("post:write", { redirectTo: "/login" })(
|
|
ctx,
|
|
async () => new Response("passed"),
|
|
);
|
|
expect(res.status).toBe(403);
|
|
});
|
|
|
|
test("an off-site redirectTo is refused and falls back to 403", async () => {
|
|
const ctx = makeCtx({ id: "u1" });
|
|
await withMiddleware(ctx);
|
|
const res = await guardPermission("post:write", {
|
|
redirectTo: "https://evil.example.com/harvest",
|
|
})(ctx, async () => new Response("passed"));
|
|
expect(res.status).toBe(403);
|
|
});
|
|
});
|