mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 13:01:00 +00:00
fix: answer the managed-profile Guardian routes in one shape
- publish the roster's entry from the invitation and `{ grantId, revokedAt }`
from the removal, instead of the account-grant row Prisma returned
- drop the invitation's self refusal: it named a code no request can elicit,
because a Guardian grant's grantor is the profile and never the caller
- name the duplicate grant error rather than catching every grant refusal into
a sentence that would be wrong for any other one
- keep the rate limit and the unknown-identifier 404 free of an error code,
as their siblings elsewhere are, and say why at the call site
This commit is contained in:
1 parent
eb9b03a788
commit
26fceca219
5 files changed
+195
-24
No files matched your search
@@ -25,6 +25,32 @@ describe("managed-profile contract", () => {
|
||||
expect(source).not.toContain("requireFreshMfaIfEnrolled");
|
||||
});
|
||||
|
||||
it("publishes no Guardian refusal a request cannot produce", async () => {
|
||||
// `managed_profile.guardian.self` was written, shipped and never
|
||||
// reachable: `self_grant` needs the grantor to equal the grantee, and the
|
||||
// grantor of a Guardian grant is the PROFILE. Inviting yourself lands on
|
||||
// the duplicate refusal; naming the profile is refused a step earlier as
|
||||
// `managed_grantee`. Both doors are asserted in
|
||||
// `tests/integration/managed-profile-surface.test.ts`, so the arm was
|
||||
// known to be dead rather than assumed to be, and it was removed rather
|
||||
// than frozen into the published contract.
|
||||
const route = await readFile(
|
||||
path.join(root, "src/app/api/managed-profiles/[id]/guardians/route.ts"),
|
||||
"utf8",
|
||||
);
|
||||
const spec = await readFile(
|
||||
path.join(root, "docs/api/openapi.yaml"),
|
||||
"utf8",
|
||||
);
|
||||
|
||||
// Non-zero control: the codes that ARE reachable are still named, so this
|
||||
// cannot pass by matching an empty file or a renamed family.
|
||||
expect(route).toContain("managed_profile.guardian.duplicate");
|
||||
expect(route).toContain("managed_profile.guardian.managed_invitee");
|
||||
expect(route).not.toContain('errorCode: "managed_profile.guardian.self"');
|
||||
expect(spec).not.toContain("managed_profile.guardian.self");
|
||||
});
|
||||
|
||||
it("does not publish an emancipation API surface", async () => {
|
||||
const source = await readFile(
|
||||
path.join(root, "prisma/schema.prisma"),
|
||||
|
||||
@@ -31,7 +31,21 @@ export const DELETE = apiHandler(
|
||||
action: { name: "managed_profile.guardian.revoked" },
|
||||
meta: { profile_id: id, grant_id: grant.id },
|
||||
});
|
||||
return apiSuccess(grant);
|
||||
// Two fields, not the grant row this used to hand back whole. What ended
|
||||
// and when is the entire answer; the party who held it is already on the
|
||||
// roster the caller is about to re-read, and `grantorId`, `granteeId` and
|
||||
// `scopeJson` are a table's columns rather than a contract.
|
||||
const { revokedAt } = grant;
|
||||
if (!revokedAt) {
|
||||
// The service stamps it inside the transaction it returns from, so
|
||||
// this cannot happen — and if it ever does, an invented timestamp
|
||||
// would be worse than the operator hearing about it.
|
||||
throw new Error("A revoked Guardian grant carries no revocation time");
|
||||
}
|
||||
return apiSuccess({
|
||||
grantId: grant.id,
|
||||
revokedAt: revokedAt.toISOString(),
|
||||
});
|
||||
} catch (error) {
|
||||
if (error instanceof LastManagedGuardianError) {
|
||||
return apiError("Add another Guardian before ending this access", 409, {
|
||||
|
||||
@@ -15,9 +15,13 @@ import {
|
||||
} from "@/lib/api-response";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { listManagedProfileGuardians } from "@/lib/managed-profiles/guardian-list";
|
||||
import {
|
||||
listManagedProfileGuardians,
|
||||
toManagedProfileGuardianView,
|
||||
} from "@/lib/managed-profiles/guardian-list";
|
||||
import { ManagedProfileLifecycleError } from "@/lib/managed-profiles/lifecycle";
|
||||
import { checkRateLimit } from "@/lib/rate-limit";
|
||||
import { GRANT_PARTY_SELECT } from "@/lib/sharing/grant-view";
|
||||
import { GrantError, inviteManagedProfileGuardian } from "@/lib/sharing/grants";
|
||||
|
||||
type RouteParams = { params: Promise<{ id: string }> };
|
||||
@@ -107,8 +111,17 @@ export const POST = apiHandler(
|
||||
{ email: { equals: parsed.data.identifier, mode: "insensitive" } },
|
||||
],
|
||||
},
|
||||
select: { id: true },
|
||||
select: GRANT_PARTY_SELECT,
|
||||
});
|
||||
// No `errorCode`, deliberately, and the same for the 429 above. Rate-limit
|
||||
// refusals carry none anywhere in this application, and the sibling
|
||||
// invitation route (`POST /api/account/grants`) answers an unknown
|
||||
// identifier with a bare 404 too. A code here would make one route in the
|
||||
// family unlike both. The published contract says so, and says that a
|
||||
// client resolves on `meta.errorCode` FIRST: this route's other 404 —
|
||||
// "you are not a Guardian of that profile" — does carry one, and branching
|
||||
// on the status would tell somebody to check a spelling when the real
|
||||
// answer is that the profile is no longer theirs to administer.
|
||||
if (!invitee) return apiError("No account with that name or e-mail", 404);
|
||||
|
||||
try {
|
||||
@@ -124,18 +137,31 @@ export const POST = apiHandler(
|
||||
action: { name: "managed_profile.guardian.invited" },
|
||||
meta: { profile_id: id, grant_id: grant.id },
|
||||
});
|
||||
return apiSuccess(grant, 201);
|
||||
// The roster's entry, not the grant row. A fresh invitation is always
|
||||
// PENDING, so the mapper cannot answer null here — but it decides that
|
||||
// rather than this route asserting it.
|
||||
const view = toManagedProfileGuardianView(grant, invitee);
|
||||
if (!view) throw new Error("A new invitation resolved to an ended grant");
|
||||
return apiSuccess(view, 201);
|
||||
} catch (error) {
|
||||
if (error instanceof GrantError && error.code === "self_grant") {
|
||||
return apiError("An account cannot be shared with itself", 422, {
|
||||
errorCode: "managed_profile.guardian.self",
|
||||
});
|
||||
}
|
||||
if (error instanceof GrantError) {
|
||||
if (
|
||||
error instanceof GrantError &&
|
||||
error.code === "duplicate_live_grant"
|
||||
) {
|
||||
return apiError("That account already has access", 409, {
|
||||
errorCode: "managed_profile.guardian.duplicate",
|
||||
});
|
||||
}
|
||||
// No arm for `self_grant`, and its absence is a decision. It fires when
|
||||
// the grantor equals the grantee; in this family the grantor is the
|
||||
// PROFILE, never the caller, so inviting yourself names somebody who is
|
||||
// not the grantor and lands on the duplicate refusal above, while naming
|
||||
// the profile itself is refused one step earlier as `managed_grantee`.
|
||||
// The 422 that used to sit here published `managed_profile.guardian.self`
|
||||
// — a code no request could elicit, on a route this release freezes. A
|
||||
// grant refusal nothing can produce is not caught into a sentence that
|
||||
// would then be wrong; it reaches the operator as a 500 with its wide
|
||||
// event, which is what an impossible state deserves.
|
||||
if (error instanceof ManagedProfileLifecycleError) {
|
||||
if (error.code === "managed_grantee") {
|
||||
return apiError("A managed profile cannot be a Guardian", 422, {
|
||||
|
||||
@@ -46,6 +46,44 @@ export interface ManagedProfileGuardianView {
|
||||
acceptedAt: string | null;
|
||||
}
|
||||
|
||||
/** The lifecycle columns this view derives its state from. */
|
||||
type GuardianGrantRow = {
|
||||
id: string;
|
||||
invitedAt: Date;
|
||||
acceptedAt: Date | null;
|
||||
expiresAt: Date | null;
|
||||
revokedAt: Date | null;
|
||||
};
|
||||
|
||||
/**
|
||||
* One live Guardian grant, as every surface publishes it.
|
||||
*
|
||||
* Exported because the invitation route answers with it too. That route used to
|
||||
* return the `AccountGrant` row Prisma handed it — `grantorId`, `granteeId`,
|
||||
* `scopeJson`, `lastUsedAt` and the rest — which made a table's columns the
|
||||
* wire contract of a route nobody had described. One mapper means the roster
|
||||
* and the invitation cannot describe a Guardian differently.
|
||||
*
|
||||
* `null` for a row that is neither PENDING nor ACTIVE, so an ended grant cannot
|
||||
* reach a list the last-Guardian floor is counted from. The caller decides what
|
||||
* an ended row means for it; here it simply is not a Guardian.
|
||||
*/
|
||||
export function toManagedProfileGuardianView(
|
||||
grant: GuardianGrantRow,
|
||||
account: GrantParty,
|
||||
now: Date = new Date(),
|
||||
): ManagedProfileGuardianView | null {
|
||||
const state = grantState(grant, now);
|
||||
if (state !== "PENDING" && state !== "ACTIVE") return null;
|
||||
return {
|
||||
grantId: grant.id,
|
||||
account,
|
||||
state,
|
||||
invitedAt: grant.invitedAt.toISOString(),
|
||||
acceptedAt: grant.acceptedAt?.toISOString() ?? null,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* The roster, or `null` when the caller may not have it.
|
||||
*
|
||||
@@ -107,19 +145,12 @@ export async function listManagedProfileGuardians(input: {
|
||||
|
||||
const roster: ManagedProfileGuardianView[] = [];
|
||||
for (const grant of grants) {
|
||||
const state = grantState(grant, now);
|
||||
// The `where` above already excludes both, so this is the belt on the
|
||||
// derivation rather than the filter itself: `grantState` is the authority
|
||||
// on what a row IS, and a row it calls ended must not reach a roster the
|
||||
// last-Guardian floor is counted from.
|
||||
if (state !== "PENDING" && state !== "ACTIVE") continue;
|
||||
roster.push({
|
||||
grantId: grant.id,
|
||||
account: grant.grantee,
|
||||
state,
|
||||
invitedAt: grant.invitedAt.toISOString(),
|
||||
acceptedAt: grant.acceptedAt?.toISOString() ?? null,
|
||||
});
|
||||
// The `where` above already excludes an ended row, so the mapper's null is
|
||||
// the belt on the derivation rather than the filter itself: `grantState` is
|
||||
// the authority on what a row IS, and a row it calls ended must not reach a
|
||||
// roster the last-Guardian floor is counted from.
|
||||
const view = toManagedProfileGuardianView(grant, grant.grantee, now);
|
||||
if (view) roster.push(view);
|
||||
}
|
||||
return roster;
|
||||
}
|
||||
@@ -476,7 +476,17 @@ describe("every call the managed-profile card can make", () => {
|
||||
expect(response.status).toBe(201);
|
||||
const grant = (await response.json()).data;
|
||||
expect(grant.acceptedAt).toBeNull();
|
||||
expect(new Date(grant.expiresAt).toISOString()).toBe(expiresAt);
|
||||
|
||||
// Read the stored row, because the entry the route answers with does not
|
||||
// carry the expiry: a Guardian is published in one shape everywhere, and
|
||||
// that shape is the roster's five fields. Asserting the column is the
|
||||
// stronger claim anyway — it proves the offset instant was persisted
|
||||
// rather than that it was echoed.
|
||||
const stored = await getPrismaClient().accountGrant.findUniqueOrThrow({
|
||||
where: { id: grant.grantId },
|
||||
select: { expiresAt: true },
|
||||
});
|
||||
expect(stored.expiresAt?.toISOString()).toBe(expiresAt);
|
||||
|
||||
// PENDING, and therefore not counted by the floor — which is the sentence
|
||||
// the card states before anybody tries to leave.
|
||||
@@ -523,6 +533,70 @@ describe("every call the managed-profile card can make", () => {
|
||||
expect((await response.json()).data.acceptedAt).toBeNull();
|
||||
});
|
||||
|
||||
it("answers an invitation with the roster's shape, not the grant row", async () => {
|
||||
// `Object.keys`, for the same reason the roster read asserts it that way:
|
||||
// the handler used to answer with the `AccountGrant` row straight out of
|
||||
// Prisma — `grantorId`, `granteeId`, `scopeJson`, `lastUsedAt` and the
|
||||
// rest — and every presence assertion in this file passed while it did.
|
||||
// The invitation now answers with the entry the roster publishes, so one
|
||||
// shape describes a Guardian everywhere and the published contract does
|
||||
// not pin a table's columns to the wire.
|
||||
const creator = await person("creator");
|
||||
const invitee = await person("invitee");
|
||||
signIn(creator);
|
||||
const { profile } = await createManagedProfile({
|
||||
creatorId: creator.id,
|
||||
displayName: "Managed record",
|
||||
dateOfBirth: null,
|
||||
locale: "en",
|
||||
timezone: "UTC",
|
||||
});
|
||||
|
||||
const response = await inviteGuardian(profile.id, {
|
||||
identifier: invitee.username,
|
||||
expiresAt: null,
|
||||
});
|
||||
expect(response.status).toBe(201);
|
||||
|
||||
const entry = (await response.json()).data as Record<string, unknown>;
|
||||
expect(Object.keys(entry).sort()).toEqual([
|
||||
"acceptedAt",
|
||||
"account",
|
||||
"grantId",
|
||||
"invitedAt",
|
||||
"state",
|
||||
]);
|
||||
expect(Object.keys(entry.account as object).sort()).toEqual([
|
||||
"displayName",
|
||||
"id",
|
||||
"username",
|
||||
]);
|
||||
expect(entry.state).toBe("PENDING");
|
||||
expect((entry.account as { username: string }).username).toBe(
|
||||
invitee.username,
|
||||
);
|
||||
|
||||
// The pipe, not the ends: the entry the invitation answered with is the
|
||||
// entry the roster read answers with a moment later.
|
||||
const roster = (await (await listGuardians(profile.id)).json())
|
||||
.data as Array<Record<string, unknown>>;
|
||||
expect(roster).toContainEqual(entry);
|
||||
});
|
||||
|
||||
it("answers a removal with the access it ended and when", async () => {
|
||||
const { creator, profile, invitation } =
|
||||
await profileWithPendingSecondGuardian();
|
||||
signIn(creator);
|
||||
|
||||
const response = await removeGuardian(profile.id, invitation.id);
|
||||
expect(response.status).toBe(200);
|
||||
|
||||
const ended = (await response.json()).data as Record<string, unknown>;
|
||||
expect(Object.keys(ended).sort()).toEqual(["grantId", "revokedAt"]);
|
||||
expect(ended.grantId).toBe(invitation.id);
|
||||
expect(ended.revokedAt).toEqual(expect.any(String));
|
||||
});
|
||||
|
||||
it("refuses a bare calendar date as an expiry", async () => {
|
||||
const creator = await person("creator");
|
||||
const invitee = await person("invitee");
|
||||
|
||||
Reference in new issue
Block a user