mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-11 22:02:16 +00:00
fix(sharing): invitations need a fresh proof and end with sign out everywhere
Offering access to the record took a live session alone, and an invitation outlives the session that sent it: whoever held a stolen session could invite their own account and keep a way into the record after the owner signed everything out. POST /api/account/grants now takes the recent-proof gate at every level (a sign-in or re-proof within five minutes on the browser, with the second factor on an account that has one; a step-up elevation on a token, which no shipped client uses here), and the invite form opens the re-proof dialog and sends again. Sign out everywhere now withdraws every invitation the account offered that nobody has accepted. Accepted grants are left standing on purpose: each is a person the owner chose, often a carer who still needs the record, and a sign-out button must not quietly cut them off. The response lists them as grantsKept and the sessions card shows each with a one-click End access.
This commit is contained in:
1 parent
a4d27c618e
commit
a294d7f684
22 files changed
+585
-65
No files matched your search
@@ -27,11 +27,19 @@ vi.mock("@/lib/api-handler", async () => {
|
||||
await vi.importActual<typeof import("@/lib/api-handler")>(
|
||||
"@/lib/api-handler",
|
||||
);
|
||||
const requireAuth = vi.fn();
|
||||
return {
|
||||
...actual,
|
||||
apiHandler: <T extends (...args: never[]) => Promise<Response>>(h: T): T =>
|
||||
h,
|
||||
requireAuth: vi.fn(),
|
||||
requireAuth,
|
||||
// The invitation takes the fresh-proof gate; here it resolves the caller
|
||||
// the stubbed `requireAuth` names, so these cases stay about the row.
|
||||
// The gate itself has its own case in `invite-recent-proof.test.ts`.
|
||||
requireRecentProof: vi.fn(async () => ({
|
||||
...(await requireAuth()),
|
||||
commitElevation: async () => {},
|
||||
})),
|
||||
};
|
||||
});
|
||||
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
/**
|
||||
* Offering access to the record asks for a fresh proof.
|
||||
*
|
||||
* An invitation outlives the session that sent it, so a live session alone
|
||||
* was enough for a stolen one to leave a standing way into the record in the
|
||||
* thief's own account. The real gate runs here; only the session, the
|
||||
* database and the headers are stubbed.
|
||||
*/
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { NextRequest } from "next/server";
|
||||
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: {
|
||||
session: { findUnique: vi.fn() },
|
||||
webauthnMfaCredential: { count: vi.fn(async () => 0) },
|
||||
passkey: { count: vi.fn(async () => 0) },
|
||||
user: { findFirst: vi.fn() },
|
||||
accountGrant: { create: vi.fn(), findFirst: vi.fn() },
|
||||
},
|
||||
}));
|
||||
vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() }));
|
||||
vi.mock("@/lib/rate-limit", () => ({
|
||||
checkRateLimit: vi.fn(async () => ({ allowed: true })),
|
||||
}));
|
||||
vi.mock("@/lib/auth/audit", () => ({
|
||||
auditLog: vi.fn().mockResolvedValue(undefined),
|
||||
recordDelegatedAccess: vi.fn(),
|
||||
}));
|
||||
vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() }));
|
||||
vi.mock("@/lib/sharing/grants", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("@/lib/sharing/grants")>()),
|
||||
inviteGrant: vi.fn(),
|
||||
}));
|
||||
vi.mock("next/headers", () => ({
|
||||
headers: vi.fn(async () => ({ get: () => null })),
|
||||
cookies: vi.fn(async () => ({
|
||||
get: () => undefined,
|
||||
set: () => {},
|
||||
delete: () => {},
|
||||
})),
|
||||
}));
|
||||
|
||||
import { POST } from "../route";
|
||||
import { getSession } from "@/lib/auth/session";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { inviteGrant } from "@/lib/sharing/grants";
|
||||
|
||||
const OWNER = {
|
||||
id: "owner-1",
|
||||
role: "USER",
|
||||
username: "owner",
|
||||
totpConfirmedAt: null,
|
||||
passwordHash: "argon",
|
||||
};
|
||||
|
||||
function invite(): NextRequest {
|
||||
return new NextRequest("http://localhost/api/account/grants", {
|
||||
method: "POST",
|
||||
headers: { "content-type": "application/json" },
|
||||
body: JSON.stringify({ identifier: "housemate", access: "READ" }),
|
||||
});
|
||||
}
|
||||
|
||||
function signedIn(createdAt: Date) {
|
||||
vi.mocked(getSession).mockResolvedValue({
|
||||
session: { id: "sess-1", expiresAt: new Date(Date.now() + 1e6) },
|
||||
user: OWNER,
|
||||
} as never);
|
||||
vi.mocked(prisma.session.findUnique).mockResolvedValue({
|
||||
createdAt,
|
||||
mfaVerifiedAt: null,
|
||||
reproofAt: null,
|
||||
} as never);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
describe("POST /api/account/grants — fresh proof", () => {
|
||||
it("a session that signed in an hour ago is asked to confirm, and nothing is offered", async () => {
|
||||
signedIn(new Date(Date.now() - 60 * 60 * 1000));
|
||||
const res = await POST(invite());
|
||||
expect(res.status).toBe(401);
|
||||
const body = await res.json();
|
||||
expect(body.meta.errorCode).toBe("auth.reproof.required");
|
||||
expect(body.meta.methods).toEqual(["password"]);
|
||||
expect(prisma.user.findFirst).not.toHaveBeenCalled();
|
||||
expect(inviteGrant).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("a session that signed in a minute ago offers the invitation", async () => {
|
||||
signedIn(new Date(Date.now() - 60 * 1000));
|
||||
vi.mocked(prisma.user.findFirst).mockResolvedValue({
|
||||
id: "invitee-1",
|
||||
username: "housemate",
|
||||
displayName: null,
|
||||
} as never);
|
||||
vi.mocked(inviteGrant).mockResolvedValue({
|
||||
id: "grant-1",
|
||||
access: "READ",
|
||||
scopeJson: null,
|
||||
invitedAt: new Date(),
|
||||
acceptedAt: null,
|
||||
expiresAt: null,
|
||||
lastUsedAt: null,
|
||||
revokedAt: null,
|
||||
revokedBy: null,
|
||||
} as never);
|
||||
const res = await POST(invite());
|
||||
expect(res.status).toBe(201);
|
||||
expect(inviteGrant).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -31,11 +31,19 @@ vi.mock("@/lib/api-handler", async () => {
|
||||
await vi.importActual<typeof import("@/lib/api-handler")>(
|
||||
"@/lib/api-handler",
|
||||
);
|
||||
const requireAuth = vi.fn();
|
||||
return {
|
||||
...actual,
|
||||
apiHandler: <T extends (...args: never[]) => Promise<Response>>(h: T): T =>
|
||||
h,
|
||||
requireAuth: vi.fn(),
|
||||
requireAuth,
|
||||
// The invitation takes the fresh-proof gate; here it resolves the caller
|
||||
// the stubbed `requireAuth` names, so these cases stay about the row.
|
||||
// The gate itself has its own case in `invite-recent-proof.test.ts`.
|
||||
requireRecentProof: vi.fn(async () => ({
|
||||
...(await requireAuth()),
|
||||
commitElevation: async () => {},
|
||||
})),
|
||||
requireFreshMfaIfEnrolled: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
@@ -21,9 +21,15 @@
|
||||
* a code rather than leaving a token caller with an authentication error on a
|
||||
* request that authenticated fine.
|
||||
*
|
||||
* **Neither is delegable, and that is structural.** Both resolve through bare
|
||||
* `requireAuth()`, which refuses outright while the caller is acting on
|
||||
* another account. So a delegate cannot invite anybody into the record they
|
||||
* v1.39.3 — offering any level also asks for a fresh proof
|
||||
* (`requireRecentProof`), and "sign out everywhere" withdraws the invitations
|
||||
* nobody has accepted yet. An invitation outlives the session that sent it, so
|
||||
* one offered from a stolen session would otherwise stay live after the owner
|
||||
* signed everything else out.
|
||||
*
|
||||
* **Neither is delegable, and that is structural.** Both resolve through
|
||||
* `requireAuth()` (the invitation through `requireRecentProof`, which calls
|
||||
* it), which refuses outright while the caller is acting on another account. So a delegate cannot invite anybody into the record they
|
||||
* were given access to, cannot widen their own grant, and cannot pass it on:
|
||||
* the route does not have a re-delegation check that somebody could soften, it
|
||||
* has no code path that runs under a switch at all. The grantor is then taken
|
||||
@@ -45,6 +51,7 @@ import {
|
||||
apiHandler,
|
||||
requireAuth,
|
||||
requireFreshMfaIfEnrolled,
|
||||
requireRecentProof,
|
||||
MFA_STEP_UP_MAX_AGE_SECONDS,
|
||||
} from "@/lib/api-handler";
|
||||
import {
|
||||
@@ -122,7 +129,12 @@ export const GET = apiHandler(async () => {
|
||||
});
|
||||
|
||||
export const POST = apiHandler(async (request: NextRequest) => {
|
||||
const auth = await requireAuth();
|
||||
// Offering access leaves a standing way into the record in somebody else's
|
||||
// hands, one that outlives the session that offered it. So a live session is
|
||||
// not enough: a sign-in or re-proof within five minutes on the browser, with
|
||||
// the second factor on an account that has one, or a step-up elevation on a
|
||||
// token. No shipped client offers access with a token.
|
||||
const auth = await requireRecentProof({ bearer: "elevation" });
|
||||
const user = auth.user;
|
||||
|
||||
const rl = await checkRateLimit(
|
||||
@@ -194,6 +206,8 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
return apiError("No account with that name or e-mail", 404);
|
||||
}
|
||||
|
||||
await auth.commitElevation();
|
||||
|
||||
try {
|
||||
// `inviteGrant` decides everything about the row except the level: the
|
||||
// self-grant refusal, and the one-live-grant-per-pair rule the partial
|
||||
|
||||
@@ -12,6 +12,7 @@ vi.mock("@/lib/db", () => ({
|
||||
update: vi.fn(() => ({ catch: () => undefined })),
|
||||
},
|
||||
user: { findUnique: vi.fn() },
|
||||
accountGrant: { findMany: vi.fn(async () => []) },
|
||||
},
|
||||
}));
|
||||
|
||||
@@ -165,6 +166,7 @@ describe("DELETE /api/auth/me/sessions (sign out everywhere)", () => {
|
||||
accessTokensRevoked: 0,
|
||||
connectorsRevoked: 0,
|
||||
shareLinksRevoked: 0,
|
||||
pendingInvitesRevoked: 0,
|
||||
});
|
||||
|
||||
const res = await DELETE(del());
|
||||
@@ -205,6 +207,7 @@ describe("DELETE /api/auth/me/sessions (sign out everywhere)", () => {
|
||||
accessTokensRevoked: 1,
|
||||
connectorsRevoked: 0,
|
||||
shareLinksRevoked: 0,
|
||||
pendingInvitesRevoked: 0,
|
||||
});
|
||||
|
||||
const res = await DELETE(del());
|
||||
@@ -219,6 +222,53 @@ describe("DELETE /api/auth/me/sessions (sign out everywhere)", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("sign out everywhere and record sharing", () => {
|
||||
it("reports withdrawn invitations and lists the accepted grants it kept", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
vi.mocked(destroyOtherSessions).mockResolvedValue({
|
||||
sessionsRevoked: 1,
|
||||
accessTokensRevoked: 0,
|
||||
connectorsRevoked: 0,
|
||||
shareLinksRevoked: 0,
|
||||
pendingInvitesRevoked: 2,
|
||||
});
|
||||
const party = { id: "user-2", username: "carer", displayName: null };
|
||||
const live = {
|
||||
id: "grant-live",
|
||||
access: "READ",
|
||||
acceptedAt: new Date("2026-01-01"),
|
||||
revokedAt: null,
|
||||
expiresAt: null,
|
||||
grantee: party,
|
||||
};
|
||||
const lapsed = {
|
||||
...live,
|
||||
id: "grant-lapsed",
|
||||
expiresAt: new Date("2026-02-01"),
|
||||
};
|
||||
vi.mocked(prisma.accountGrant.findMany).mockResolvedValue([
|
||||
live,
|
||||
lapsed,
|
||||
] as never);
|
||||
|
||||
const res = await DELETE(del());
|
||||
const body = await res.json();
|
||||
expect(body.data.pendingInvitesRevoked).toBe(2);
|
||||
// The lapsed one confers nothing any more and is not listed.
|
||||
expect(body.data.grantsKept).toEqual([
|
||||
{ id: "grant-live", account: party, access: "READ" },
|
||||
]);
|
||||
expect(prisma.accountGrant.findMany).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
where: expect.objectContaining({
|
||||
grantorId: "user-1",
|
||||
revokedAt: null,
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("DELETE /api/auth/me/sessions/[id]", () => {
|
||||
function ctx(id: string) {
|
||||
return { params: Promise.resolve({ id }) };
|
||||
|
||||
@@ -5,7 +5,10 @@
|
||||
* connection and token, and (unless
|
||||
* `?keepShareLinks=1`) every clinician share
|
||||
* link, keeping the caller's current
|
||||
* credential. Closes #64.
|
||||
* credential. Closes #64. Also withdraws
|
||||
* record-sharing invitations nobody has
|
||||
* accepted yet, and lists the accepted
|
||||
* grants it left standing (`grantsKept`).
|
||||
*
|
||||
* v1.23 — the user-facing session/device-management surface. Distinct from
|
||||
* `/api/auth/me/devices`, which lists APNs / Web-Push notification devices;
|
||||
@@ -34,6 +37,8 @@ import {
|
||||
type CurrentCredential,
|
||||
} from "@/lib/auth/session";
|
||||
import { lookupIpLocation } from "@/lib/geo";
|
||||
import { GRANT_PARTY_SELECT } from "@/lib/sharing/grant-view";
|
||||
import { isGrantActive } from "@/lib/sharing/grants";
|
||||
import { coarseDeviceLabel, maskIp } from "@/lib/auth/device-fingerprint";
|
||||
|
||||
export const dynamic = "force-dynamic";
|
||||
@@ -131,11 +136,32 @@ export const DELETE = apiHandler(async (request: NextRequest) => {
|
||||
accessTokensRevoked,
|
||||
connectorsRevoked,
|
||||
shareLinksRevoked,
|
||||
pendingInvitesRevoked,
|
||||
} = await destroyOtherSessions(user.id, current, {
|
||||
reach: "everything",
|
||||
keepShareLinks,
|
||||
});
|
||||
|
||||
// The accepted grants this account gave, still live. Not ended here: each is
|
||||
// a person the owner chose, often a carer who still needs the record, and a
|
||||
// button labelled "sign out" must not quietly cut them off. They are listed
|
||||
// instead, so the person who just signed everything out can see who can
|
||||
// still read the record and end any of them with one click.
|
||||
const now = new Date();
|
||||
const grantsKept = (
|
||||
await prisma.accountGrant.findMany({
|
||||
where: {
|
||||
grantorId: user.id,
|
||||
acceptedAt: { not: null },
|
||||
revokedAt: null,
|
||||
},
|
||||
orderBy: { acceptedAt: "desc" },
|
||||
include: { grantee: { select: GRANT_PARTY_SELECT } },
|
||||
})
|
||||
)
|
||||
.filter((g) => isGrantActive(g, now))
|
||||
.map((g) => ({ id: g.id, account: g.grantee, access: g.access }));
|
||||
|
||||
await auditLog("auth.session.revoke_others", {
|
||||
userId: user.id,
|
||||
ipAddress: getClientIp(request),
|
||||
@@ -144,6 +170,8 @@ export const DELETE = apiHandler(async (request: NextRequest) => {
|
||||
accessTokensRevoked,
|
||||
connectorsRevoked,
|
||||
shareLinksRevoked,
|
||||
pendingInvitesRevoked,
|
||||
grantsKept: grantsKept.length,
|
||||
keepShareLinks,
|
||||
},
|
||||
});
|
||||
@@ -155,6 +183,8 @@ export const DELETE = apiHandler(async (request: NextRequest) => {
|
||||
access_tokens_revoked: accessTokensRevoked,
|
||||
connectors_revoked: connectorsRevoked,
|
||||
share_links_revoked: shareLinksRevoked,
|
||||
pending_invites_revoked: pendingInvitesRevoked,
|
||||
grants_kept: grantsKept.length,
|
||||
},
|
||||
});
|
||||
|
||||
@@ -163,6 +193,8 @@ export const DELETE = apiHandler(async (request: NextRequest) => {
|
||||
accessTokensRevoked,
|
||||
connectorsRevoked,
|
||||
shareLinksRevoked,
|
||||
pendingInvitesRevoked,
|
||||
grantsKept,
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in new issue
Block a user