mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 04:32:42 +00:00
fix(search): admit the search POST at read level through a declared, frozen readOverPost
This commit is contained in:
1 parent
1bf6108b37
commit
7ee72d0161
5 files changed
+83
-3
No files matched your search
@@ -0,0 +1,34 @@
|
||||
/**
|
||||
* `readOverPost` lets a delegable route answer a POST at the grant's read
|
||||
* level. That widens what a read-only grant reaches, so the set of routes
|
||||
* declaring it is frozen here: adding one is a reviewed diff, not an
|
||||
* accident. Today it is the record search alone, whose query is health data
|
||||
* that must stay out of URLs.
|
||||
*
|
||||
* The matcher tolerates whitespace and line breaks, and the guard asserts it
|
||||
* matched something, so a rewrite that hides the declaration from the regex
|
||||
* fails instead of passing on an empty set.
|
||||
*/
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { globSync } from "node:fs";
|
||||
import { join, relative, sep } from "node:path";
|
||||
|
||||
const ROOT = process.cwd();
|
||||
|
||||
const FROZEN = ["src/app/api/search/route.ts"];
|
||||
|
||||
describe("readOverPost declarations", () => {
|
||||
it("are exactly the frozen set", () => {
|
||||
const declaring = globSync("src/**/*.ts", { cwd: ROOT })
|
||||
.filter((file) => !file.includes(`${sep}__tests__${sep}`))
|
||||
.filter((file) => file !== join("src", "lib", "api-handler.ts"))
|
||||
.filter((file) =>
|
||||
/readOverPost\s*:\s*true/.test(readFileSync(join(ROOT, file), "utf8")),
|
||||
)
|
||||
.map((file) => relative(ROOT, join(ROOT, file)).split(sep).join("/"))
|
||||
.sort();
|
||||
expect(declaring.length).toBeGreaterThan(0);
|
||||
expect(declaring).toEqual(FROZEN);
|
||||
});
|
||||
});
|
||||
@@ -111,7 +111,10 @@ beforeEach(() => {
|
||||
describe("POST /api/search", () => {
|
||||
it("admits the whole record only, at read", async () => {
|
||||
await handler(request("q=schulter"));
|
||||
expect(mocks.requireRecordAuth).toHaveBeenCalledWith("read", "record");
|
||||
// A POST that only reads, declared as such.
|
||||
expect(mocks.requireRecordAuth).toHaveBeenCalledWith("read", "record", {
|
||||
readOverPost: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("propagates a refused admission without searching", async () => {
|
||||
|
||||
@@ -56,7 +56,11 @@ function displayLocale(value: string | null | undefined): Locale {
|
||||
}
|
||||
|
||||
export const POST = apiHandler(async (request: NextRequest) => {
|
||||
const { user, actor, grantId } = await requireRecordAuth("read", "record");
|
||||
// A POST that only reads: the query rides the body to stay out of URLs, and
|
||||
// a read grant on the whole record is what it needs, as it was over GET.
|
||||
const { user, actor, grantId } = await requireRecordAuth("read", "record", {
|
||||
readOverPost: true,
|
||||
});
|
||||
|
||||
const rl = await checkRateLimit(
|
||||
`search:${actor.id}`,
|
||||
|
||||
@@ -505,6 +505,29 @@ describe("requireRecordAuth — every way a grant can fail", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("reads a POST at read level only where the route declares readOverPost", async () => {
|
||||
signedInCookie(OWNER);
|
||||
liveGrant();
|
||||
const declared = route(() =>
|
||||
requireRecordAuth("read", "record", { readOverPost: true }),
|
||||
);
|
||||
expect((await declared("POST")).status).toBe(200);
|
||||
// The declaration covers POST alone: every other write method still
|
||||
// needs a write grant.
|
||||
for (const method of ["PUT", "PATCH", "DELETE"]) {
|
||||
expect((await declared(method)).status, method).toBe(403);
|
||||
}
|
||||
});
|
||||
|
||||
it("does not lower a declared write for a readOverPost route", async () => {
|
||||
signedInCookie(OWNER);
|
||||
liveGrant();
|
||||
const response = await route(() =>
|
||||
requireRecordAuth("write", "record", { readOverPost: true }),
|
||||
)("POST");
|
||||
expect(response.status).toBe(403);
|
||||
});
|
||||
|
||||
it("refuses a declared write under a read grant", async () => {
|
||||
signedInCookie(OWNER);
|
||||
liveGrant();
|
||||
|
||||
+17
-1
@@ -889,6 +889,19 @@ export interface RecordAuthOptions {
|
||||
* else.
|
||||
*/
|
||||
readonly scope?: string;
|
||||
/**
|
||||
* v1.42.2 — this route answers a POST without changing anything: the body
|
||||
* carries a query that must not stand in a URL (the record search, whose
|
||||
* text is health data a proxy's access log would otherwise keep). Under a
|
||||
* route declaring `"read"`, a POST is then read at the grant's read level
|
||||
* instead of being escalated to a write.
|
||||
*
|
||||
* Like `scope`, naming it is a WIDENING and is frozen by
|
||||
* `read-over-post-guard.test.ts`, so it lands in a reviewed diff. It does
|
||||
* nothing for any other method, and nothing for a route declaring a need
|
||||
* above `"read"`.
|
||||
*/
|
||||
readonly readOverPost?: true;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -980,8 +993,11 @@ export async function requireRecordAuth(
|
||||
}
|
||||
|
||||
const method = (getEvent()?.getHttpMethod() ?? "").toUpperCase();
|
||||
const readsOnly =
|
||||
READ_HTTP_METHODS.has(method) ||
|
||||
(options?.readOverPost === true && method === "POST");
|
||||
const effectiveNeed: GrantNeed =
|
||||
need !== "read" || !READ_HTTP_METHODS.has(method) ? escalate(need) : "read";
|
||||
need !== "read" || !readsOnly ? escalate(need) : "read";
|
||||
|
||||
return resolveSwitchedRecord(auth, carrier, (grant) => {
|
||||
if (!grantAllows(grant, effectiveNeed)) return "insufficient_access";
|
||||
|
||||
Reference in new issue
Block a user