diff --git a/src/__tests__/managed-profile-contract.test.ts b/src/__tests__/managed-profile-contract.test.ts index aff12b1ea..2cce87c68 100644 --- a/src/__tests__/managed-profile-contract.test.ts +++ b/src/__tests__/managed-profile-contract.test.ts @@ -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"), diff --git a/src/app/api/managed-profiles/[id]/guardians/[grantId]/route.ts b/src/app/api/managed-profiles/[id]/guardians/[grantId]/route.ts index 6b33c31d0..d1261cb5d 100644 --- a/src/app/api/managed-profiles/[id]/guardians/[grantId]/route.ts +++ b/src/app/api/managed-profiles/[id]/guardians/[grantId]/route.ts @@ -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, { diff --git a/src/app/api/managed-profiles/[id]/guardians/route.ts b/src/app/api/managed-profiles/[id]/guardians/route.ts index ff797843c..014c02fa9 100644 --- a/src/app/api/managed-profiles/[id]/guardians/route.ts +++ b/src/app/api/managed-profiles/[id]/guardians/route.ts @@ -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, { diff --git a/src/lib/managed-profiles/guardian-list.ts b/src/lib/managed-profiles/guardian-list.ts index 21340ae0c..db27a63d4 100644 --- a/src/lib/managed-profiles/guardian-list.ts +++ b/src/lib/managed-profiles/guardian-list.ts @@ -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; } diff --git a/tests/integration/managed-profile-surface.test.ts b/tests/integration/managed-profile-surface.test.ts index 6425a661d..54df50a30 100644 --- a/tests/integration/managed-profile-surface.test.ts +++ b/tests/integration/managed-profile-surface.test.ts @@ -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; + 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>; + 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; + 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");