mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-10 05:12:14 +00:00
security: destroyAllSessions wipes API tokens + refresh tokens; tighten well-known proxy bypass
Two findings from the v1.4.34.3 security audit, both with the same
shape — every authenticated surface a user owns should rotate when a
credential-rotation path fires, and unfamiliar discovery sub-paths
must not inherit "no auth" status by default.
destroyAllSessions:
Previously only ran `prisma.session.deleteMany`, leaving every
long-lived `ApiToken` row (Bearer credentials issued via
/settings/api-tokens) and every native-client `RefreshToken`
intact. A stolen iOS Bearer kept working after the user rotated
their password or after an admin reset; only the browser cookie
flipped. Wraps the cleanup in a $transaction that flips
`apiToken.revoked = true` and `refreshToken.revokedAt = now()` in
the same atomic block so the audit trail survives. Both lookups at
`src/lib/auth/api-token.ts` and `src/lib/auth/refresh-token.ts`
already filter on these columns; no second code path needed.
Same destroyAllSessions call already runs from:
- POST /api/auth/password (user-initiated change)
- POST /api/admin/users/[id]/reset-password (admin reset)
- DELETE /api/settings/account (account deletion)
All three paths now wipe every transport in one shot.
Well-known proxy bypass:
`src/proxy.ts` PUBLIC_PATHS carried "/.well-known/" with a prefix
match — a future route at `/.well-known/foo` would inherit "no
auth" status silently. Tighten to an exact-match Set
(WELL_KNOWN_PUBLIC_PATHS) seeded with the one path we actually
serve (`apple-app-site-association`). New routes must be added
explicitly.
Also drops the stale `/api/auth/codex/callback` PUBLIC_PATHS
entry — that route doesn't exist in the codebase (the Codex
OAuth flow uses `/api/auth/codex/{device-poll,device-start,
disconnect}` instead).
Tests:
- New destroyAllSessions assertion in src/lib/auth/__tests__/
session.test.ts pins the three updateMany/deleteMany call
shapes inside a single $transaction.
- Existing proxy well-known + privacy + onboarding redirect
suites stay green.
This commit is contained in:
1 parent
8886c0ffdf
commit
58c5416bc9
3 files changed
+80
-11
No files matched your search
@@ -13,6 +13,13 @@ vi.mock("@/lib/db", () => ({
|
||||
deleteMany: vi.fn(),
|
||||
update: vi.fn(),
|
||||
},
|
||||
apiToken: {
|
||||
updateMany: vi.fn(),
|
||||
},
|
||||
refreshToken: {
|
||||
updateMany: vi.fn(),
|
||||
},
|
||||
$transaction: vi.fn(async (ops: Promise<unknown>[]) => Promise.all(ops)),
|
||||
},
|
||||
}));
|
||||
|
||||
@@ -36,7 +43,7 @@ vi.mock("next/headers", () => ({
|
||||
}));
|
||||
|
||||
import { prisma } from "@/lib/db";
|
||||
import { getSession } from "../session";
|
||||
import { getSession, destroyAllSessions } from "../session";
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
@@ -63,3 +70,32 @@ describe("getSession", () => {
|
||||
expect(cookieState.delete).toHaveBeenCalledWith("hl_onboarding");
|
||||
});
|
||||
});
|
||||
|
||||
describe("destroyAllSessions", () => {
|
||||
it("revokes web sessions, API tokens, and refresh tokens for the user", async () => {
|
||||
vi.mocked(prisma.session.deleteMany).mockResolvedValue({
|
||||
count: 2,
|
||||
} as never);
|
||||
vi.mocked(prisma.apiToken.updateMany).mockResolvedValue({
|
||||
count: 1,
|
||||
} as never);
|
||||
vi.mocked(prisma.refreshToken.updateMany).mockResolvedValue({
|
||||
count: 3,
|
||||
} as never);
|
||||
|
||||
await destroyAllSessions("user-rotated");
|
||||
|
||||
expect(prisma.session.deleteMany).toHaveBeenCalledWith({
|
||||
where: { userId: "user-rotated" },
|
||||
});
|
||||
expect(prisma.apiToken.updateMany).toHaveBeenCalledWith({
|
||||
where: { userId: "user-rotated", revoked: false },
|
||||
data: { revoked: true },
|
||||
});
|
||||
expect(prisma.refreshToken.updateMany).toHaveBeenCalledWith({
|
||||
where: { userId: "user-rotated", revokedAt: null },
|
||||
data: { revokedAt: expect.any(Date) },
|
||||
});
|
||||
expect(prisma.$transaction).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
+26
-1
@@ -156,8 +156,33 @@ export async function destroySession(): Promise<void> {
|
||||
cookieStore.delete(ONBOARDING_COOKIE);
|
||||
}
|
||||
|
||||
/**
|
||||
* Revoke every authenticated surface a user owns: web sessions, API
|
||||
* tokens (long-lived Bearer credentials issued via `/settings/api-tokens`),
|
||||
* and native-client refresh tokens. Called from every credential-rotation
|
||||
* path (`/api/auth/password`, `/api/admin/users/[id]/reset-password`,
|
||||
* `DELETE /api/settings/account`) so a remediated user wipes a stolen
|
||||
* credential across every transport, not just the browser cookie.
|
||||
*
|
||||
* `ApiToken` flips `revoked=true` (the token-hash lookup at
|
||||
* `src/lib/auth/api-token.ts` filters on this column). `RefreshToken`
|
||||
* sets `revokedAt` (the rotation lookup at
|
||||
* `src/lib/auth/refresh-token.ts` short-circuits on a non-null value).
|
||||
* Both run as `updateMany` rather than `deleteMany` so the audit trail
|
||||
* survives — the rows linger as evidence of the rotation event.
|
||||
*/
|
||||
export async function destroyAllSessions(userId: string): Promise<void> {
|
||||
await prisma.session.deleteMany({ where: { userId } });
|
||||
await prisma.$transaction([
|
||||
prisma.session.deleteMany({ where: { userId } }),
|
||||
prisma.apiToken.updateMany({
|
||||
where: { userId, revoked: false },
|
||||
data: { revoked: true },
|
||||
}),
|
||||
prisma.refreshToken.updateMany({
|
||||
where: { userId, revokedAt: null },
|
||||
data: { revokedAt: new Date() },
|
||||
}),
|
||||
]);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
+17
-9
@@ -23,7 +23,6 @@ const PUBLIC_PATHS = [
|
||||
"/api/withings/webhook",
|
||||
"/api/telegram/webhook",
|
||||
"/api/integrations/moodlog/webhook",
|
||||
"/api/auth/codex/callback",
|
||||
"/api/ingest/",
|
||||
// v1.4.26 — `/privacy` is a public legal page. iOS App Store Connect
|
||||
// requires a publicly reachable Privacy-Policy URL during submission;
|
||||
@@ -33,19 +32,25 @@ const PUBLIC_PATHS = [
|
||||
// alongside the project credits. The CC licence requires the
|
||||
// attribution to be reachable without a sign-in.
|
||||
"/about",
|
||||
// v1.4.33 — `/.well-known/*` covers IETF-registered discovery
|
||||
// endpoints (RFC 8615). Apple reads
|
||||
// `/.well-known/apple-app-site-association` without credentials to
|
||||
// wire Web Credentials (passkey sharing) and Universal Links to the
|
||||
// iOS bundle, so the path must answer 200 with the bare JSON body
|
||||
// before any auth gate runs. The trailing slash future-proofs the
|
||||
// namespace for `/security.txt`, `/openid-configuration`, etc.
|
||||
"/.well-known/",
|
||||
// `/onboarding` itself + its subroutes are matched exactly via
|
||||
// `isPublicPath()` so we don't admit `/onboarding-export` etc.
|
||||
"/robots.txt",
|
||||
];
|
||||
|
||||
/**
|
||||
* Exact-match allowlist for IETF-registered discovery endpoints
|
||||
* (RFC 8615). Apple reads `/.well-known/apple-app-site-association`
|
||||
* without credentials to wire passkey sharing and Universal Links;
|
||||
* the bare JSON body has to answer 200 before any auth gate runs.
|
||||
*
|
||||
* Exact match (not prefix) — a future `/.well-known/openid-configuration`
|
||||
* or `/.well-known/security.txt` must be added here explicitly so a new
|
||||
* sub-path doesn't auto-inherit "no auth" status.
|
||||
*/
|
||||
const WELL_KNOWN_PUBLIC_PATHS = new Set<string>([
|
||||
"/.well-known/apple-app-site-association",
|
||||
]);
|
||||
|
||||
/**
|
||||
* v1.4.22 W5 reconcile (Sec-MED-2) — `/onboarding` matches exact
|
||||
* page + subroutes (`/onboarding`, `/onboarding/*`) only, NOT the
|
||||
@@ -59,6 +64,9 @@ function isPublicPath(pathname: string): boolean {
|
||||
if (pathname === "/onboarding" || pathname.startsWith("/onboarding/")) {
|
||||
return true;
|
||||
}
|
||||
if (WELL_KNOWN_PUBLIC_PATHS.has(pathname)) {
|
||||
return true;
|
||||
}
|
||||
return PUBLIC_PATHS.some((p) => pathname.startsWith(p));
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user