mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 20:42:43 +00:00
fix(auth): allow narrow-scope tokens on unscoped routes — unblocks v1.5 iOS endpoints
The previous `requireAuth()` policy 403'd every narrow-scope Bearer token on routes that did not declare a required permission. The intent was to keep wildcard tokens from acting as session-equivalents on sensitive routes that never gated on scope, but the side effect blocked four W8d routes (`/api/measurements/by-external-ids`, `/api/personal-records`, `/api/medications/[id]/glp1`, `/api/dashboard/glp1`) for the v1.5 iOS app. Restate the contract: a route that does not declare a `requiredPermission` is content with any authenticated token; a route that declares one gates on it. Narrow-scope tokens still 403 against scoped routes they don't list. Add four cases to the Bearer-auth test suite covering all token- × route-scope combinations.
This commit is contained in:
1 parent
db5e07a944
commit
c38a2c8a2a
2 files changed
+69
-31
No files matched your search
@@ -198,28 +198,67 @@ describe("requireAuth — Bearer token path", () => {
|
||||
expect(prisma.user.findUnique).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a scoped Bearer token when the route did not declare a scope", async () => {
|
||||
// v1.4.25 W10 reconcile (code-review H2): a route that does NOT declare
|
||||
// a scope is content with any authenticated token. The previous policy
|
||||
// 403'd every narrow-scope token here and broke `/api/measurements/by-
|
||||
// external-ids`, `/api/personal-records`, `/api/medications/[id]/glp1`,
|
||||
// and `/api/dashboard/glp1` — all four were called without a scope.
|
||||
it("accepts a narrow-scope Bearer token when the route did not declare a scope", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(null);
|
||||
setBearerHeader(`Bearer ${RAW_TOKEN}`);
|
||||
|
||||
vi.mocked(prisma.apiToken.findUnique).mockResolvedValue({
|
||||
id: "token-5",
|
||||
id: "token-5a",
|
||||
userId: "user-1",
|
||||
permissions: ["medication:ingest"],
|
||||
revoked: false,
|
||||
expiresAt: null,
|
||||
} as never);
|
||||
vi.mocked(prisma.user.findUnique).mockResolvedValue(FAKE_USER as never);
|
||||
|
||||
await expect(requireAuth()).rejects.toMatchObject({
|
||||
statusCode: 403,
|
||||
});
|
||||
const ctx = await requireAuth();
|
||||
expect(ctx.user.id).toBe("user-1");
|
||||
expect(ctx.session.id).toBe("token-5a");
|
||||
expect(auditLog).toHaveBeenCalledWith(
|
||||
"auth.bearer.failure",
|
||||
expect.objectContaining({
|
||||
details: expect.objectContaining({ reason: "scope_required" }),
|
||||
}),
|
||||
"auth.bearer.success",
|
||||
expect.objectContaining({ userId: "user-1" }),
|
||||
);
|
||||
expect(prisma.user.findUnique).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("accepts a wildcard Bearer token when the route did not declare a scope", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(null);
|
||||
setBearerHeader(`Bearer ${RAW_TOKEN}`);
|
||||
|
||||
vi.mocked(prisma.apiToken.findUnique).mockResolvedValue({
|
||||
id: "token-5b",
|
||||
userId: "user-1",
|
||||
permissions: ["*"],
|
||||
revoked: false,
|
||||
expiresAt: null,
|
||||
} as never);
|
||||
vi.mocked(prisma.user.findUnique).mockResolvedValue(FAKE_USER as never);
|
||||
|
||||
const ctx = await requireAuth();
|
||||
expect(ctx.user.id).toBe("user-1");
|
||||
expect(ctx.session.id).toBe("token-5b");
|
||||
});
|
||||
|
||||
it("accepts a narrow-scope Bearer token when the route's required scope matches", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(null);
|
||||
setBearerHeader(`Bearer ${RAW_TOKEN}`);
|
||||
|
||||
vi.mocked(prisma.apiToken.findUnique).mockResolvedValue({
|
||||
id: "token-5c",
|
||||
userId: "user-1",
|
||||
permissions: ["medication:ingest"],
|
||||
revoked: false,
|
||||
expiresAt: null,
|
||||
} as never);
|
||||
vi.mocked(prisma.user.findUnique).mockResolvedValue(FAKE_USER as never);
|
||||
|
||||
const ctx = await requireAuth("medication:ingest");
|
||||
expect(ctx.user.id).toBe("user-1");
|
||||
expect(ctx.session.id).toBe("token-5c");
|
||||
});
|
||||
|
||||
it("returns 401 when neither cookie nor Bearer is present", async () => {
|
||||
|
||||
+20
-21
@@ -255,29 +255,28 @@ async function authenticateBearer(
|
||||
throw new HttpError(401, "Token expired");
|
||||
}
|
||||
|
||||
// Audit V3 NEW-V3-1 fix: `["*"]` is a real wildcard — it grants the
|
||||
// session-equivalent scope (the iOS app receives this on login). Without
|
||||
// the wildcard branch, EVERY future requireAuth("scope:name") call would
|
||||
// 403 every iOS-issued token because string-literal `.includes("*"...)`
|
||||
// never matches. Worse: today many sensitive routes call requireAuth()
|
||||
// *without* a requiredPermission, so a leaked iOS token can act as a
|
||||
// full-scope token (account delete, settings wipe). Once those routes
|
||||
// adopt requireAuth("scope:name"), the wildcard handling here keeps
|
||||
// the iOS app working while narrower-scoped tokens (e.g. ["medication:
|
||||
// ingest"]) get correctly 403'd.
|
||||
// `["*"]` is the wildcard scope: granted to the iOS app on login so it
|
||||
// can call any authenticated route. Narrow scopes (e.g.
|
||||
// `["medication:ingest"]`) gate specific routes.
|
||||
//
|
||||
// Authorisation contract:
|
||||
// - Route declares no `requiredPermission`: any valid token passes
|
||||
// (both wildcard and narrow-scope). The route does not gate on
|
||||
// scope, so authentication alone is sufficient.
|
||||
// - Route declares a `requiredPermission`: wildcard tokens pass;
|
||||
// narrow-scope tokens must list the required permission.
|
||||
//
|
||||
// v1.4.25 W10 reconcile (code-review H2): the previous "no
|
||||
// requiredPermission ⇒ wildcard only" rule misread the contract.
|
||||
// Routes without a declared scope (`/api/measurements/by-external-ids`,
|
||||
// `/api/personal-records`, `/api/medications/[id]/glp1`,
|
||||
// `/api/dashboard/glp1`) 403'd every narrow-scope token even though
|
||||
// those routes did not intend to restrict by scope at all. Allowing
|
||||
// any authenticated token through when the route does not declare a
|
||||
// scope unblocks the v1.5 iOS endpoints while keeping the scoped-
|
||||
// route gating below intact.
|
||||
const hasWildcardPermission = apiToken.permissions.includes("*");
|
||||
|
||||
if (!requiredPermission && !hasWildcardPermission) {
|
||||
auditLog("auth.bearer.failure", {
|
||||
userId: apiToken.userId,
|
||||
details: {
|
||||
reason: "scope_required",
|
||||
tokenId: apiToken.id,
|
||||
},
|
||||
}).catch(() => {});
|
||||
throw new HttpError(403, "Insufficient permissions");
|
||||
}
|
||||
|
||||
if (
|
||||
requiredPermission &&
|
||||
!hasWildcardPermission &&
|
||||
|
||||
Reference in new issue
Block a user