mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 13:01:00 +00:00
fix(api): give the generic auth refusals a stable errorCode
`requireAuth`, `requireAdmin`, `requireCookieAuth`, `requireFreshMfa` and `requireBearerAuth` all refused with a bare prose string and no `meta`. So the single most important branch a native client makes — refresh the token and retry, drop the session and show the login screen, or stop retrying because this credential will never work — could only be made by string-matching the English sentence "Token expired". Any rewording of that string, or a localisation pass, silently turns a recoverable refresh into a forced logout for every already-shipped build. The mechanism was already here and these gates were the ones skipped: the assistant, consent, step-up, module and sharing refusals all carry `meta.errorCode`, and `/api/auth/refresh` emits `auth.refresh.*`. So `HttpError` takes an optional code and the wrapper puts it in the same field. Five values, dot.case like the majority: `auth.missing`, `auth.token.expired`, `auth.token.invalid`, `auth.scope.insufficient`, `auth.admin.required`. A revoked token reads invalid, not expired — refreshing a revoked token loops forever. Additive: an HttpError thrown without a code serialises byte-identically to before, which is most of them. The published 401 description now enumerates the codes and says what a client should do with each.
This commit is contained in:
1 parent
a7bdf14a4e
commit
21dd7c7e48
5 files changed
+322
-15
No files matched your search
@@ -70,7 +70,13 @@ paths:
|
||||
schema:
|
||||
$ref: "#/components/schemas/CycleCustomSymptomListEnvelope"
|
||||
"401": &a1
|
||||
description: Authentication required or invalid credentials.
|
||||
description: "Authentication required or invalid credentials. `meta.errorCode` says which, and it is the field to branch
|
||||
on — the English sentence beside it may be reworded or localised at any time. `auth.missing`: no session
|
||||
cookie and no Bearer header; send the user to sign in. `auth.token.expired`: the Bearer token's lifetime has
|
||||
passed; refresh and retry the request. `auth.token.invalid`: unknown, revoked, or the owning account is
|
||||
gone; discard the credential and sign in again — retrying will not help. The related 403 refusals carry
|
||||
`auth.scope.insufficient` (the credential is valid but its scope does not reach this route, including any
|
||||
Bearer token against an admin surface) and `auth.admin.required` (a cookie session that is not an admin's)."
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
|
||||
@@ -0,0 +1,249 @@
|
||||
/**
|
||||
* The generic auth gates refuse with a stable machine code.
|
||||
*
|
||||
* This decides the single most important branch a native client makes:
|
||||
* refresh the token and retry, drop the session and show the login screen, or
|
||||
* stop retrying because the credential will never work. Before these codes the
|
||||
* only way to tell those apart was to string-match the English sentence "Token
|
||||
* expired", so a rewording — or a localisation pass — would have turned a
|
||||
* recoverable refresh into a forced logout for every already-shipped build.
|
||||
*
|
||||
* So the assertions here are on the CODE, and deliberately not on the prose.
|
||||
* The one place the sentence is pinned is the pair of cases proving the
|
||||
* envelope is otherwise unchanged: `error` still carries a sentence, and an
|
||||
* HttpError thrown without a code still serialises without a `meta`.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: {
|
||||
apiToken: { findUnique: vi.fn(), update: vi.fn() },
|
||||
user: { findUnique: vi.fn() },
|
||||
session: { findUnique: vi.fn() },
|
||||
webauthnMfaCredential: { count: vi.fn() },
|
||||
passkey: { count: vi.fn() },
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() }));
|
||||
vi.mock("@/lib/auth/hmac", () => ({ hashToken: vi.fn() }));
|
||||
vi.mock("@/lib/auth/audit", () => ({
|
||||
auditLog: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() }));
|
||||
|
||||
const headersGet = vi.fn<(name: string) => string | null>();
|
||||
vi.mock("next/headers", () => ({
|
||||
headers: vi.fn(async () => ({ get: headersGet })),
|
||||
cookies: vi.fn(async () => ({
|
||||
get: () => undefined,
|
||||
set: () => {},
|
||||
delete: () => {},
|
||||
})),
|
||||
}));
|
||||
|
||||
import {
|
||||
apiHandler,
|
||||
AUTH_ERROR_CODES,
|
||||
HttpError,
|
||||
MFA_STEP_UP_MAX_AGE_SECONDS,
|
||||
requireAdmin,
|
||||
requireAuth,
|
||||
requireFreshMfa,
|
||||
} from "../api-handler";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { getSession } from "@/lib/auth/session";
|
||||
import { hashToken } from "@/lib/auth/hmac";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
|
||||
const FAKE_HASH = "deadbeefcafef00d";
|
||||
const RAW_TOKEN = "hlk_" + "a".repeat(64);
|
||||
const FAKE_USER = {
|
||||
id: "user-1",
|
||||
role: "USER" as const,
|
||||
username: "testuser",
|
||||
email: "user@example.com",
|
||||
};
|
||||
|
||||
function setBearerHeader(value: string | null): void {
|
||||
headersGet.mockReset();
|
||||
headersGet.mockImplementation((name: string) =>
|
||||
name.toLowerCase() === "authorization" ? value : null,
|
||||
);
|
||||
}
|
||||
|
||||
function mockSession(user: unknown) {
|
||||
vi.mocked(getSession).mockResolvedValue({
|
||||
session: { id: "sess-1", expiresAt: new Date(Date.now() + 1e6) },
|
||||
user,
|
||||
} as never);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
vi.mocked(hashToken).mockReturnValue(FAKE_HASH);
|
||||
vi.mocked(prisma.apiToken.update).mockResolvedValue({} as never);
|
||||
vi.mocked(auditLog).mockResolvedValue(undefined as never);
|
||||
});
|
||||
|
||||
/** Run a gate through `apiHandler` and read the envelope it produced. */
|
||||
async function envelopeFor(gate: () => Promise<unknown>): Promise<{
|
||||
status: number;
|
||||
body: { data: null; error: string; meta?: { errorCode?: string } };
|
||||
}> {
|
||||
const handler = apiHandler(async (_request: Request) => {
|
||||
await gate();
|
||||
return new Response(null, { status: 204 });
|
||||
});
|
||||
const res = (await handler(
|
||||
new Request("http://localhost/api/anything"),
|
||||
)) as Response;
|
||||
return {
|
||||
status: res.status,
|
||||
body: (await res.json()) as {
|
||||
data: null;
|
||||
error: string;
|
||||
meta?: { errorCode?: string };
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe("auth failures carry a machine-readable errorCode", () => {
|
||||
it("names auth.missing when no credential was presented at all", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(null);
|
||||
setBearerHeader(null);
|
||||
|
||||
const { status, body } = await envelopeFor(() => requireAuth());
|
||||
|
||||
expect(status).toBe(401);
|
||||
expect(body.meta?.errorCode).toBe(AUTH_ERROR_CODES.missing);
|
||||
// The envelope is otherwise what it always was.
|
||||
expect(body.data).toBeNull();
|
||||
expect(body.error).toBe("Not authenticated");
|
||||
});
|
||||
|
||||
it("names auth.token.expired so the client refreshes instead of logging out", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(null);
|
||||
setBearerHeader(`Bearer ${RAW_TOKEN}`);
|
||||
vi.mocked(prisma.apiToken.findUnique).mockResolvedValue({
|
||||
id: "token-expired",
|
||||
userId: "user-1",
|
||||
permissions: ["*"],
|
||||
revoked: false,
|
||||
expiresAt: new Date(Date.now() - 60_000),
|
||||
} as never);
|
||||
|
||||
const { status, body } = await envelopeFor(() => requireAuth());
|
||||
|
||||
expect(status).toBe(401);
|
||||
expect(body.meta?.errorCode).toBe(AUTH_ERROR_CODES.expired);
|
||||
});
|
||||
|
||||
it("names auth.token.invalid for an unknown token", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(null);
|
||||
setBearerHeader(`Bearer ${RAW_TOKEN}`);
|
||||
vi.mocked(prisma.apiToken.findUnique).mockResolvedValue(null);
|
||||
|
||||
const { status, body } = await envelopeFor(() => requireAuth());
|
||||
|
||||
expect(status).toBe(401);
|
||||
expect(body.meta?.errorCode).toBe(AUTH_ERROR_CODES.invalid);
|
||||
});
|
||||
|
||||
it("names auth.token.invalid for a revoked token — not the expired code", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(null);
|
||||
setBearerHeader(`Bearer ${RAW_TOKEN}`);
|
||||
vi.mocked(prisma.apiToken.findUnique).mockResolvedValue({
|
||||
id: "token-revoked",
|
||||
userId: "user-1",
|
||||
permissions: ["*"],
|
||||
revoked: true,
|
||||
expiresAt: null,
|
||||
} as never);
|
||||
|
||||
const { body } = await envelopeFor(() => requireAuth());
|
||||
|
||||
// Refreshing a revoked token loops forever; the two must not share a code.
|
||||
expect(body.meta?.errorCode).toBe(AUTH_ERROR_CODES.invalid);
|
||||
expect(body.meta?.errorCode).not.toBe(AUTH_ERROR_CODES.expired);
|
||||
});
|
||||
|
||||
it("names auth.scope.insufficient when the token's scope does not reach the route", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(null);
|
||||
setBearerHeader(`Bearer ${RAW_TOKEN}`);
|
||||
vi.mocked(prisma.apiToken.findUnique).mockResolvedValue({
|
||||
id: "token-narrow",
|
||||
userId: "user-1",
|
||||
permissions: ["something:else"],
|
||||
revoked: false,
|
||||
expiresAt: null,
|
||||
} as never);
|
||||
|
||||
const { status, body } = await envelopeFor(() =>
|
||||
requireAuth("medication:ingest"),
|
||||
);
|
||||
|
||||
expect(status).toBe(403);
|
||||
expect(body.meta?.errorCode).toBe(AUTH_ERROR_CODES.scope);
|
||||
});
|
||||
|
||||
it("names auth.scope.insufficient when a narrow token hits a route that declared none", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(null);
|
||||
setBearerHeader(`Bearer ${RAW_TOKEN}`);
|
||||
vi.mocked(prisma.apiToken.findUnique).mockResolvedValue({
|
||||
id: "token-undeclared",
|
||||
userId: "user-1",
|
||||
permissions: ["health:read"],
|
||||
revoked: false,
|
||||
expiresAt: null,
|
||||
} as never);
|
||||
|
||||
const { status, body } = await envelopeFor(() => requireAuth());
|
||||
|
||||
expect(status).toBe(403);
|
||||
expect(body.meta?.errorCode).toBe(AUTH_ERROR_CODES.scope);
|
||||
});
|
||||
|
||||
it("names auth.missing and auth.admin.required on the admin gate", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(null);
|
||||
const anonymous = await envelopeFor(() => requireAdmin());
|
||||
expect(anonymous.status).toBe(401);
|
||||
expect(anonymous.body.meta?.errorCode).toBe(AUTH_ERROR_CODES.missing);
|
||||
|
||||
mockSession(FAKE_USER);
|
||||
const nonAdmin = await envelopeFor(() => requireAdmin());
|
||||
expect(nonAdmin.status).toBe(403);
|
||||
expect(nonAdmin.body.meta?.errorCode).toBe(AUTH_ERROR_CODES.admin);
|
||||
});
|
||||
|
||||
it("names auth.missing on the step-up gate with no session", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(null);
|
||||
|
||||
const { status, body } = await envelopeFor(() =>
|
||||
requireFreshMfa(MFA_STEP_UP_MAX_AGE_SECONDS),
|
||||
);
|
||||
|
||||
expect(status).toBe(401);
|
||||
expect(body.meta?.errorCode).toBe(AUTH_ERROR_CODES.missing);
|
||||
});
|
||||
|
||||
it("leaves an HttpError thrown without a code exactly as it was", async () => {
|
||||
const handler = apiHandler(async (_request: Request) => {
|
||||
throw new HttpError(404, "Not found");
|
||||
});
|
||||
const res = (await handler(
|
||||
new Request("http://localhost/api/anything"),
|
||||
)) as Response;
|
||||
|
||||
expect(res.status).toBe(404);
|
||||
expect(await res.json()).toEqual({ data: null, error: "Not found" });
|
||||
});
|
||||
|
||||
it("keeps every code distinct and dot.case", () => {
|
||||
const codes = Object.values(AUTH_ERROR_CODES);
|
||||
expect(new Set(codes).size).toBe(codes.length);
|
||||
for (const code of codes) {
|
||||
expect(code).toMatch(/^auth(\.[a-z_]+)+$/);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -14,17 +14,49 @@ import { RECORD_FENCE_ERROR_CODE } from "./sharing/record-session-fence-contract
|
||||
/**
|
||||
* Custom error class for HTTP errors with status codes.
|
||||
* Throw inside apiHandler-wrapped routes to return a JSON error response.
|
||||
*
|
||||
* `errorCode` is optional and lands in `meta.errorCode` when set — the same
|
||||
* field the step-up, consent, module and sharing gates already use. An
|
||||
* HttpError without one serialises exactly as before.
|
||||
*/
|
||||
export class HttpError extends Error {
|
||||
constructor(
|
||||
public statusCode: number,
|
||||
message: string,
|
||||
public errorCode?: string,
|
||||
) {
|
||||
super(message);
|
||||
this.name = "HttpError";
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The stable codes the generic auth gates refuse with.
|
||||
*
|
||||
* These decide the single most important branch a native client makes:
|
||||
* refresh the token and retry, drop the session and show the login screen, or
|
||||
* stop retrying because this credential will never work. Until they existed
|
||||
* the only way to tell those apart was to string-match the English sentence
|
||||
* "Token expired", so a rewording — or a localisation pass — would have turned
|
||||
* a recoverable refresh into a forced logout for every shipped build.
|
||||
*
|
||||
* Dot.case, matching the majority convention and the `auth.refresh.*` /
|
||||
* `auth.stepup.*` families already on the wire. Add to this list rather than
|
||||
* inventing a second field.
|
||||
*/
|
||||
export const AUTH_ERROR_CODES = {
|
||||
/** No credential at all: no session cookie and no Bearer header. */
|
||||
missing: "auth.missing",
|
||||
/** The Bearer token was valid once and its expiry has passed. Refresh. */
|
||||
expired: "auth.token.expired",
|
||||
/** Unknown, revoked, or belonging to a user that no longer exists. */
|
||||
invalid: "auth.token.invalid",
|
||||
/** Authenticated, but the credential's scope does not reach this route. */
|
||||
scope: "auth.scope.insufficient",
|
||||
/** Authenticated, but not an admin. Cookie-only surface; never a token. */
|
||||
admin: "auth.admin.required",
|
||||
} as const;
|
||||
|
||||
/**
|
||||
* v1.36.0 — the two refusals the acting-account resolver can raise.
|
||||
*
|
||||
|
||||
+32
-13
@@ -81,6 +81,7 @@ export function isMcpAudienceToken(permissions: readonly string[]): boolean {
|
||||
// the api-handler ↔ record-session-fence import cycle) and are re-exported
|
||||
// here so the hundreds of existing importers read exactly as before.
|
||||
import {
|
||||
AUTH_ERROR_CODES,
|
||||
HttpError,
|
||||
SharingAuthError,
|
||||
SharingAccessDeniedError,
|
||||
@@ -90,6 +91,7 @@ import {
|
||||
} from "./api-errors";
|
||||
|
||||
export {
|
||||
AUTH_ERROR_CODES,
|
||||
HttpError,
|
||||
SharingAuthError,
|
||||
SharingAccessDeniedError,
|
||||
@@ -295,9 +297,17 @@ export function apiHandler<T extends (...args: any[]) => Promise<Response>>(
|
||||
{ status: error.statusCode },
|
||||
);
|
||||
} else if (error instanceof HttpError) {
|
||||
// `meta.errorCode` only when the throw site named one, so every
|
||||
// HttpError without a code serialises byte-identically to before.
|
||||
evt.setError(error);
|
||||
response = NextResponse.json(
|
||||
{ data: null, error: error.message },
|
||||
{
|
||||
data: null,
|
||||
error: error.message,
|
||||
...(error.errorCode
|
||||
? { meta: { errorCode: error.errorCode } }
|
||||
: {}),
|
||||
},
|
||||
{ status: error.statusCode },
|
||||
);
|
||||
} else if (error instanceof SyntaxError) {
|
||||
@@ -540,7 +550,7 @@ async function authenticateCaller(
|
||||
}
|
||||
|
||||
// 3. No credentials.
|
||||
throw new HttpError(401, "Not authenticated");
|
||||
throw new HttpError(401, "Not authenticated", AUTH_ERROR_CODES.missing);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -584,13 +594,15 @@ async function authenticateBearer(
|
||||
: {}),
|
||||
},
|
||||
}).catch(() => {});
|
||||
const message =
|
||||
// Sentence and code are decided together, so a future rewording of the
|
||||
// prose cannot silently change what a client does with the refusal.
|
||||
const [message, errorCode] =
|
||||
err.statusCode === 403
|
||||
? "Insufficient permissions"
|
||||
? (["Insufficient permissions", AUTH_ERROR_CODES.scope] as const)
|
||||
: err.reason === "expired"
|
||||
? "Token expired"
|
||||
: "Invalid token";
|
||||
throw new HttpError(err.statusCode, message);
|
||||
? (["Token expired", AUTH_ERROR_CODES.expired] as const)
|
||||
: (["Invalid token", AUTH_ERROR_CODES.invalid] as const);
|
||||
throw new HttpError(err.statusCode, message, errorCode);
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
@@ -626,7 +638,11 @@ async function authenticateBearer(
|
||||
method: method || "unknown",
|
||||
},
|
||||
}).catch(() => {});
|
||||
throw new HttpError(403, "Insufficient permissions");
|
||||
throw new HttpError(
|
||||
403,
|
||||
"Insufficient permissions",
|
||||
AUTH_ERROR_CODES.scope,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1114,7 +1130,8 @@ async function resolveSwitchedRecord(
|
||||
*/
|
||||
export async function requireAdmin(): Promise<AuthContext> {
|
||||
const sessionData = await getSession();
|
||||
if (!sessionData) throw new HttpError(401, "Not authenticated");
|
||||
if (!sessionData)
|
||||
throw new HttpError(401, "Not authenticated", AUTH_ERROR_CODES.missing);
|
||||
|
||||
const evt = getEvent();
|
||||
if (evt) {
|
||||
@@ -1126,7 +1143,7 @@ export async function requireAdmin(): Promise<AuthContext> {
|
||||
}
|
||||
|
||||
if (sessionData.user.role !== "ADMIN") {
|
||||
throw new HttpError(403, "Admin access required");
|
||||
throw new HttpError(403, "Admin access required", AUTH_ERROR_CODES.admin);
|
||||
}
|
||||
return { ...sessionData, authMethod: "cookie" };
|
||||
}
|
||||
@@ -1143,7 +1160,8 @@ export async function requireAdmin(): Promise<AuthContext> {
|
||||
*/
|
||||
export async function requireCookieAuth(): Promise<AuthContext> {
|
||||
const sessionData = await getSession();
|
||||
if (!sessionData) throw new HttpError(401, "Not authenticated");
|
||||
if (!sessionData)
|
||||
throw new HttpError(401, "Not authenticated", AUTH_ERROR_CODES.missing);
|
||||
|
||||
const evt = getEvent();
|
||||
if (evt) {
|
||||
@@ -1217,7 +1235,8 @@ export async function requireFreshMfa(
|
||||
proofSource: FreshFactorProofSource = "second-factor",
|
||||
): Promise<FreshMfaContext> {
|
||||
const sessionData = await getSession();
|
||||
if (!sessionData) throw new HttpError(401, "Not authenticated");
|
||||
if (!sessionData)
|
||||
throw new HttpError(401, "Not authenticated", AUTH_ERROR_CODES.missing);
|
||||
|
||||
const evt = getEvent();
|
||||
if (evt) {
|
||||
@@ -1448,7 +1467,7 @@ export async function requireBearerAuth(): Promise<BearerAuthContext> {
|
||||
authHeader = null;
|
||||
}
|
||||
if (!authHeader?.startsWith("Bearer ")) {
|
||||
throw new HttpError(401, "Not authenticated");
|
||||
throw new HttpError(401, "Not authenticated", AUTH_ERROR_CODES.missing);
|
||||
}
|
||||
const raw = authHeader.slice(7);
|
||||
const auth = await authenticateBearer(raw, undefined);
|
||||
|
||||
@@ -229,7 +229,8 @@ export const invalidBaseTokenResponse = {
|
||||
|
||||
export const stdResponses = {
|
||||
"401": {
|
||||
description: "Authentication required or invalid credentials.",
|
||||
description:
|
||||
"Authentication required or invalid credentials. `meta.errorCode` says which, and it is the field to branch on — the English sentence beside it may be reworded or localised at any time. `auth.missing`: no session cookie and no Bearer header; send the user to sign in. `auth.token.expired`: the Bearer token's lifetime has passed; refresh and retry the request. `auth.token.invalid`: unknown, revoked, or the owning account is gone; discard the credential and sign in again — retrying will not help. The related 403 refusals carry `auth.scope.insufficient` (the credential is valid but its scope does not reach this route, including any Bearer token against an admin surface) and `auth.admin.required` (a cookie session that is not an admin's).",
|
||||
content: { "application/json": { schema: errorEnvelope } },
|
||||
},
|
||||
"422": {
|
||||
|
||||
Reference in new issue
Block a user