From c38a2c8a2a5b561e7b08397ad79fa1f778915c59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 14 May 2026 16:46:35 +0200 Subject: [PATCH] =?UTF-8?q?fix(auth):=20allow=20narrow-scope=20tokens=20on?= =?UTF-8?q?=20unscoped=20routes=20=E2=80=94=20unblocks=20v1.5=20iOS=20endp?= =?UTF-8?q?oints?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/lib/__tests__/require-auth-bearer.test.ts | 59 +++++++++++++++---- src/lib/api-handler.ts | 41 +++++++------ 2 files changed, 69 insertions(+), 31 deletions(-) diff --git a/src/lib/__tests__/require-auth-bearer.test.ts b/src/lib/__tests__/require-auth-bearer.test.ts index 326f57fc6..a4ad1f1a2 100644 --- a/src/lib/__tests__/require-auth-bearer.test.ts +++ b/src/lib/__tests__/require-auth-bearer.test.ts @@ -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 () => { diff --git a/src/lib/api-handler.ts b/src/lib/api-handler.ts index 8ca6e40f0..4b9b39b7b 100644 --- a/src/lib/api-handler.ts +++ b/src/lib/api-handler.ts @@ -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 &&