diff --git a/apps/backstage/src/features/members/components/member-drawer.tsx b/apps/backstage/src/features/members/components/member-drawer.tsx index 5cd39678..4dbbf5bd 100644 --- a/apps/backstage/src/features/members/components/member-drawer.tsx +++ b/apps/backstage/src/features/members/components/member-drawer.tsx @@ -136,7 +136,7 @@ function EditBody({ onSubmit: (data: MemberInput) => Promise; }) { const { onUpload, onRemove } = useMemberPhoto(member.id); - const { canAssignPowerGrants } = useCan(); + const { canAssignBoardSeat } = useCan(); return (
diff --git a/apps/backstage/src/features/members/components/member-form.test.tsx b/apps/backstage/src/features/members/components/member-form.test.tsx index 0fa5bdde..c2252593 100644 --- a/apps/backstage/src/features/members/components/member-form.test.tsx +++ b/apps/backstage/src/features/members/components/member-form.test.tsx @@ -105,6 +105,52 @@ describe("MemberForm", () => { expect(screen.queryByText("Comisión de Eventos")).not.toBeInTheDocument(); }); + // The empty-state the delegation exists to explain. A chapter whose every cargo carries + // grants (which is the real production shape) leaves a non-delegate with zero options, and + // the bare Combobox "Sin resultados" cannot be told apart from an empty catalog. + it("explains an empty cargo list to a non-delegate, and stays silent for a delegate", async () => { + const gatedCargo = ( + id: string, + category: Position["category"], + grants: Position["grants"], + ): Position => ({ + id, + title: id, + titleFemale: id, + category, + grants, + term: null, + sigla: null, + description: "", + active: true, + deletedAt: null, + }); + const celOnly: Position[] = [ + gatedCargo("pos-cel", "CEL", []), + gatedCargo("pos-power", "JDL", ["Membership"]), + ]; + const { unmount } = render( + , + ); + expect(screen.getByRole("note")).toHaveTextContent(/Asientos de directiva/); + await userEvent.click(screen.getByLabelText("Cargo")); + expect(await screen.findByText("Sin resultados")).toBeInTheDocument(); + unmount(); + + // The delegate sees the very same catalog as assignable, and no note. + render( + , + ); + expect(screen.queryByRole("note")).not.toBeInTheDocument(); + await userEvent.click(screen.getByLabelText("Cargo")); + expect(screen.queryByText("Sin resultados")).not.toBeInTheDocument(); + }); + // The admin half of memberSchemaFor: a member enrolled before memberNameValid() existed // must stay editable. Without the per-member schema the form blocks on a name the admin // never touched, making the rules' touched('name') affordance unreachable. diff --git a/apps/backstage/src/features/members/components/member-form.tsx b/apps/backstage/src/features/members/components/member-form.tsx index e6ee820a..10ea19f4 100644 --- a/apps/backstage/src/features/members/components/member-form.tsx +++ b/apps/backstage/src/features/members/components/member-form.tsx @@ -26,8 +26,10 @@ import { avatarColor } from "../lib/member-display"; import { cargoOptionsForEditor, cargoTakedownOnly, + noAssignableCargos, positionsLockedForNonAdmin, } from "../lib/assignable-cargo"; +import { NoAssignableCargosNote } from "./no-assignable-cargos-note"; interface MemberFormProps { positions: Position[]; @@ -310,6 +312,9 @@ export function MemberForm({ igual.

)} + {noAssignableCargos({ cargoOptions, allowPowerGrants, locked: positionsLocked }) && ( + + )} ( - + {children} ), @@ -142,4 +147,88 @@ describe("MemberInviteDrawer", () => { ); expect(screen.getByRole("button", { name: "Copiar enlace de acceso" })).toBeInTheDocument(); }); + + // --- create:MemberLogin delegation --- + + const drawer = ( + onProvision = vi.fn().mockResolvedValue({ email: "a@b.co", actionLink: "l" }), + ) => ({ + node: ( + {}} + onCreate={async () => "idD"} + onProvision={onProvision} + /> + ), + onProvision, + }); + + it("shows 'Enviar acceso' to a create:MemberLogin delegate, defaulted ON, and provisions", async () => { + const { node, onProvision } = drawer(); + renderWithAbility(node, { roles: ["Member"], perms: ["create:Member", "create:MemberLogin"] }); + const checkbox = screen.getByLabelText("Enviar acceso a la app"); + expect(checkbox).toBeChecked(); + await fill(); + fireEvent.click(screen.getByRole("button", { name: "Enviar invitación" })); + await waitFor(() => expect(onProvision).toHaveBeenCalledWith("idD")); + }); + + it("hides it from a member creator without the code, and never calls onProvision", async () => { + const { node, onProvision } = drawer(); + renderWithAbility(node, { roles: ["Member"], perms: ["create:Member"] }); + expect(screen.queryByLabelText("Enviar acceso a la app")).not.toBeInTheDocument(); + await fill(); + fireEvent.click(screen.getByRole("button", { name: "Enviar invitación" })); + await waitFor(() => expect(screen.queryByRole("alert")).not.toBeInTheDocument()); + expect(onProvision).not.toHaveBeenCalled(); + }); + + it("BLOCKING: hides it from a manage:all perm holder without the Admin role", async () => { + // The render-then-403 this gate exists to stop: beacon's requireAdminOrPerm is an exact + // code test, so the wildcard would fail server-side after the member was already created. + const { node } = drawer(); + renderWithAbility(node, { roles: ["Member"], perms: ["manage:all"] }); + expect(screen.queryByLabelText("Enviar acceso a la app")).not.toBeInTheDocument(); + }); + + it("does not attempt the invite when the cargo confers permissions and the caller is a delegate", async () => { + // beacon's power-seat guard would refuse it, so attempting it would create the member, + // 403, and point the user at a row action that fails identically forever. + const onProvision = vi.fn(); + const powerCargo = [ + { + id: "pos-power", + title: "Secretario", + titleFemale: null, + category: "CEL" as const, + grants: ["Secretary"] as never, + term: null, + sigla: null, + description: "", + active: true, + deletedAt: null, + }, + ]; + renderWithAbility( + {}} + onCreate={async () => "idB"} + onProvision={onProvision} + />, + { roles: ["Member"], perms: ["create:Member", "create:MemberLogin", "update:BoardSeat"] }, + ); + await fill(); + await userEvent.click(screen.getByLabelText("Cargo")); + // positionTitle derives the female variant from the title when titleFemale is null, and + // fill() picks "Femenino" — so the rendered label is "Secretaria". + await userEvent.click(await screen.findByText(/Secretari[ao]/)); + fireEvent.click(screen.getByRole("button", { name: "Enviar invitación" })); + await waitFor(() => expect(screen.getByRole("alert")).toBeInTheDocument()); + expect(screen.getByRole("alert")).toHaveTextContent(/solo un Admin puede enviarle el acceso/); + expect(onProvision).not.toHaveBeenCalled(); + }); }); diff --git a/apps/backstage/src/features/members/components/member-invite-drawer.tsx b/apps/backstage/src/features/members/components/member-invite-drawer.tsx index e36ac1ba..28a79537 100644 --- a/apps/backstage/src/features/members/components/member-invite-drawer.tsx +++ b/apps/backstage/src/features/members/components/member-invite-drawer.tsx @@ -15,6 +15,9 @@ interface MemberInviteDrawerProps { } interface DoneState { + /** The invite was skipped because the member's cargo confers permissions and the caller is + * not an Admin — beacon would refuse it, so nothing was attempted. */ + blockedByCargo: boolean; name: string; email: string; provisioned: boolean; @@ -34,26 +37,28 @@ export function MemberInviteDrawer({ onCreate, onProvision, }: MemberInviteDrawerProps) { - // Provisioning login is Admin-role-only (provisionMemberLogin → requireAdmin). A - // non-Admin may still create the member; they just can't send access here, so hide - // the option and default it off — otherwise the provision step fails silently after - // the member is already created. - const { isAdmin, canAssignPowerGrants } = useCan(); + // Provisioning login is the Admin role OR the create:MemberLogin perm + // (provisionMemberLogin → requireAdminOrPerm). A member creator without either may still + // create the member; they just can't send access here, so hide the option and default it + // off — otherwise the provision step fails silently after the member is already created. + const { canProvisionLogin, canAssignBoardSeat, isAdmin } = useCan(); const [done, setDone] = useState(null); - const [sendAccess, setSendAccess] = useState(isAdmin); + const [sendAccess, setSendAccess] = useState(canProvisionLogin); const [copyState, setCopyState] = useState<"idle" | "copied" | "failed">("idle"); // The drawer mounts with the page, before the auth token's claims decode (the store // emits with empty claims first, then re-emits). Re-sync the default each time it - // OPENS — by then isAdmin is resolved — so an Admin's first invite doesn't silently + // OPENS — by then the flag is resolved — so a provisioner's first invite doesn't silently // default "Enviar acceso" off. Won't clobber a manual toggle (deps stable while open). + // This matters MORE now than it did for a role gate: `perms` is minted by claims-sync and + // lands in the same late token, so a perm-derived flag is false for exactly as long. useEffect(() => { - if (open) setSendAccess(isAdmin); - }, [open, isAdmin]); + if (open) setSendAccess(canProvisionLogin); + }, [open, canProvisionLogin]); const reset = () => { setDone(null); - setSendAccess(isAdmin); + setSendAccess(canProvisionLogin); setCopyState("idle"); }; @@ -68,7 +73,15 @@ export function MemberInviteDrawer({ let emailSent = false; let actionLink: string | null = null; let errorDetail: string | null = null; - if (sendAccess) { + // beacon refuses a non-Admin provisioning a member seated on a granting cargo (the + // power-seat guard). The rules DO let that member be created, so without this check the + // drawer would create them, 403 on the invite, and send the user to a row action that + // fails the same way on every retry. Decide before writing anything. + const seatedCargo = data.cargoId ? positions.find((p) => p.id === data.cargoId) : undefined; + const provisionBlocked = !isAdmin && (seatedCargo?.grants.length ?? 0) > 0; + if (sendAccess && provisionBlocked) { + errorDetail = null; + } else if (sendAccess) { // The member is already created; if provisioning fails, fall through to the // done screen with provisioned=false ("aún no tiene acceso, invítalo desde su // fila") instead of throwing — a thrown error reads as a create failure and @@ -77,7 +90,7 @@ export function MemberInviteDrawer({ try { const result = await onProvision(id); provisioned = true; - actionLink = result.actionLink; + actionLink = result.actionLink || null; try { await requestPasswordReset(data.email); emailSent = true; @@ -91,6 +104,7 @@ export function MemberInviteDrawer({ } } setDone({ + blockedByCargo: sendAccess && provisionBlocked, name: data.name, email: data.email, provisioned, @@ -118,32 +132,42 @@ export function MemberInviteDrawer({

{`Invitación enviada a ${done.email}. Recibirá un correo para crear su contraseña y acceder a la app.`}

+ ) : done.blockedByCargo ? ( +

+ {`${done.name} fue creado, pero su cargo otorga permisos: solo un Admin puede enviarle el acceso. Pídeselo para completar la invitación.`} +

) : done.provisioned && !done.emailSent ? ( <>

- El correo no se pudo enviar. Comparte el enlace de acceso manualmente. + {done.actionLink + ? "El correo no se pudo enviar. Comparte el enlace de acceso manualmente." + : "El correo no se pudo enviar. Pídele a un Admin que reenvíe la invitación."}

{done.errorDetail && (

Detalle: {done.errorDetail}

)} - - {copyState === "failed" && ( - - {done.actionLink} - + {done.actionLink && ( + <> + + {copyState === "failed" && ( + + {done.actionLink} + + )} + )} ) : ( @@ -177,11 +201,11 @@ export function MemberInviteDrawer({ submitLabel="Enviar invitación" pendingLabel="Enviando…" showPreview - allowPowerGrants={canAssignPowerGrants} + allowPowerGrants={canAssignBoardSeat} defaultValues={{ joinDate: today(), status: "Activo", cargoId: null, comisionIds: [] }} onSubmit={handleSubmit} > - {isAdmin && ( + {canProvisionLogin && ( { expect(onSubmit).toHaveBeenCalledWith({ cargoId: null, comisionIds: [] }); }); + it("explains an empty cargo list to a non-delegate, and never doubles up with the locked note", async () => { + // A catalog of only CEL / power-granting cargos — the real production shape, and the + // state that made the picker silently empty. + const gated: Position[] = [ + { ...pos("presi", "CEL"), grants: [] }, + { ...pos("power", "JDL"), grants: ["Membership"] }, + ]; + const { unmount } = render( + , + ); + expect(screen.getByRole("note")).toHaveTextContent(/Asientos de directiva/); + unmount(); + + // A delegate assigns the same catalog: no note. + const asDelegate = render( + , + ); + expect(screen.queryByRole("note")).not.toBeInTheDocument(); + asDelegate.unmount(); + + // Locked (seated on a power cargo) renders the LOCKED note and not this one. They are + // mutually exclusive today only because cargoOptionsForEditor appends the held cargo, + // making the list non-empty — pin it so a change there cannot produce two notes. + render( + , + ); + const notes = screen.getAllByRole("note"); + expect(notes).toHaveLength(1); + expect(notes[0]).toHaveTextContent(/Solo un Admin/); + }); + it("submits selected cargo and comisiones", async () => { const onSubmit = vi.fn().mockResolvedValue(undefined); render( diff --git a/apps/backstage/src/features/members/components/member-positions-form.tsx b/apps/backstage/src/features/members/components/member-positions-form.tsx index bc02f796..8ac9b25c 100644 --- a/apps/backstage/src/features/members/components/member-positions-form.tsx +++ b/apps/backstage/src/features/members/components/member-positions-form.tsx @@ -7,8 +7,10 @@ import { type MemberGender, type Position } from "@luminova/types"; import { cargoOptionsForEditor, cargoTakedownOnly, + noAssignableCargos, positionsLockedForNonAdmin, } from "../lib/assignable-cargo"; +import { NoAssignableCargosNote } from "./no-assignable-cargos-note"; const positionsSchema = z.object({ cargoId: z.string().min(1).nullable(), @@ -128,6 +130,7 @@ export function MemberPositionsForm({ con «Quitar cargo» y guardar, o elegir otro cargo.

)} + {noAssignableCargos({ cargoOptions, allowPowerGrants, locked }) && } {formError && (
{formError} diff --git a/apps/backstage/src/features/members/components/member-profile-page.tsx b/apps/backstage/src/features/members/components/member-profile-page.tsx index 61476ea8..fd4da41e 100644 --- a/apps/backstage/src/features/members/components/member-profile-page.tsx +++ b/apps/backstage/src/features/members/components/member-profile-page.tsx @@ -16,6 +16,7 @@ import { useActivitiesByTerm } from "../../activities/hooks/use-activities-by-te import { useInitiativesByTerm } from "../../initiatives/hooks/use-initiatives-by-term"; import { summarizeParticipations } from "../lib/participation-summary"; import { pointsRank } from "../../../lib/points-rank"; +import { requestPasswordReset } from "../../../lib/auth/request-password-reset"; import { useProvisionMemberLogin } from "../hooks/use-provision-member-login"; import { useUpdateMember } from "../hooks/use-update-member"; import { useSetMemberPositions } from "../hooks/use-set-member-positions"; @@ -128,8 +129,11 @@ export function MemberProfilePage() { actions={
{member.status && {member.status}} - {/* provisionMemberLogin is requireAdmin (role), not the manage:all perm. */} - + {/* provisionMemberLogin is requireAdminOrPerm(create:MemberLogin) — the Admin + role or that exact code, never the manage:all perm. */} + {/* `!member.uid` mirrors beacon's adoption guard: a delegate may only mint a NEW + login, so "Reenviar acceso" would 403 on every click for them. */} +
@@ -145,7 +149,7 @@ export function MemberProfilePage() { defaultValues={memberFormDefaults(member)} submitLabel="Guardar cambios" pendingLabel="Guardando…" - allowPowerGrants={gate.canAssignPowerGrants} + allowPowerGrants={gate.canAssignBoardSeat} onSubmit={handleEdit} avatarSeed={member.name} /> @@ -173,7 +177,7 @@ export function MemberProfilePage() { (null); + const [sent, setSent] = useState(false); const [error, setError] = useState(null); const label = member.uid ? "Reenviar acceso" : "Invitar acceso"; + // beacon withholds the action link from a non-Admin caller (it is a bearer credential for + // the account). The client then does what the invite drawer already does — send the reset + // mail itself through the unprivileged sendPasswordResetEmail — so a delegate's invite + // still lands. Without this the delegate got an empty code block and a copy button that + // copied nothing, with the account already created and no way to set a password. const invite = () => { setError(null); provision.mutate(member.id, { onSuccess: (result) => { - setLink(result.actionLink); - setOpen(true); + if (result.actionLink) { + setLink(result.actionLink); + setOpen(true); + return; + } + setSent(false); + void requestPasswordReset(result.email) + .then(() => setSent(true)) + .catch((err: unknown) => { + console.error("No se pudo enviar el correo de acceso", err); + setError( + "Se creó el acceso, pero no se pudo enviar el correo. Pídele a un Admin que lo reenvíe.", + ); + }); }, onError: (err) => setError(provisionErrorMessage(err, "No se pudo generar el acceso.")), }); @@ -255,6 +277,11 @@ function InviteAccess({ member }: { member: Member }) { {error}

)} + {sent && ( +

+ Invitación enviada por correo. +

+ )}

diff --git a/apps/backstage/src/features/members/components/member-row-menu.test.tsx b/apps/backstage/src/features/members/components/member-row-menu.test.tsx index 6aa23f07..792236f7 100644 --- a/apps/backstage/src/features/members/components/member-row-menu.test.tsx +++ b/apps/backstage/src/features/members/components/member-row-menu.test.tsx @@ -126,4 +126,23 @@ describe("MemberRowMenu", () => { expect(screen.queryByText("Desafiliar")).not.toBeInTheDocument(); expect(screen.queryByText("Invitar a la app")).not.toBeInTheDocument(); }); + + it("shows the invite item to a create:MemberLogin delegate with no privileged role", async () => { + // The affordance moved off the Admin ROLE onto canProvisionLogin, mirroring beacon's + // requireAdminOrPerm. update:Member is what keeps the menu itself reachable. + renderMenu(member({ status: "Activo" }), { + roles: ["Member"], + perms: ["update:Member", "create:MemberLogin"], + }); + await userEvent.click(screen.getByLabelText(/Acciones para Ana/)); + expect(screen.getByText("Invitar a la app")).toBeInTheDocument(); + }); + + it("BLOCKING: hides the invite item from a manage:all perm holder without the Admin role", async () => { + // Exact-code gate, matching the callable. A wildcard holder clicking this would get a + // permission-denied from beacon after the fact. + renderMenu(member({ status: "Activo" }), { roles: ["Member"], perms: ["manage:all"] }); + await userEvent.click(screen.getByLabelText(/Acciones para Ana/)); + expect(screen.queryByText("Invitar a la app")).not.toBeInTheDocument(); + }); }); diff --git a/apps/backstage/src/features/members/components/member-row-menu.tsx b/apps/backstage/src/features/members/components/member-row-menu.tsx index 89e0cece..03d9a6f9 100644 --- a/apps/backstage/src/features/members/components/member-row-menu.tsx +++ b/apps/backstage/src/features/members/components/member-row-menu.tsx @@ -2,6 +2,7 @@ import { Menu, MenuItem, MenuSeparator } from "@luminova/ui"; import type { Member, MemberStatus } from "@luminova/types"; import { Can } from "../../../lib/authz/ability-context"; import { ActionGate } from "../../../lib/authz/action-gate"; +import { useCan } from "../../../lib/authz/use-can"; interface MemberRowMenuProps { member: Member; @@ -20,6 +21,7 @@ export function MemberRowMenu({ onSetStatus, onUnpublish, }: MemberRowMenuProps) { + const { canProvisionLogin, isAdmin } = useCan(); return (

onEdit(member)}>Editar miembro - {/* provisionMemberLogin is requireAdmin (role), not the manage:all perm. */} - + {/* provisionMemberLogin is requireAdminOrPerm(create:MemberLogin) — the Admin role or + that exact code, never the manage:all perm. The `!member.uid` half is the adoption + guard mirrored: a delegate may only mint a NEW login, so offering "Reenviar" to one + would be an item that 403s on every click. */} + onProvision(member)}> {member.uid ? "Reenviar invitación" : "Invitar a la app"} diff --git a/apps/backstage/src/features/members/components/no-assignable-cargos-note.tsx b/apps/backstage/src/features/members/components/no-assignable-cargos-note.tsx new file mode 100644 index 00000000..fe0b830f --- /dev/null +++ b/apps/backstage/src/features/members/components/no-assignable-cargos-note.tsx @@ -0,0 +1,20 @@ +/** Why the Cargo picker is empty, for an editor who is not a board-seat delegate. + * + * Without it the Combobox renders its bare "Sin resultados" and the editor cannot tell a + * permission ceiling from an empty catalog — which is the state a chapter whose every cargo + * carries grants lands in permanently. + * + * Shared by both member forms rather than typed into each: the `locked` and `takedownOnly` + * notes legitimately differ in wording between them, this one does not. + * + * The quoted permission name must stay equal to `permissionLabel("update:BoardSeat")` — + * ACTION_LABELS.update + SUBJECT_LABELS.BoardSeat, in features/permissions. Nothing enforces + * the match across the two features, so it is stated here. */ +export function NoAssignableCargosNote() { + return ( +

+ Ningún cargo del catálogo es asignable con tus permisos. Los cargos del Comité Ejecutivo Local + y los que otorgan permisos requieren un Admin o el permiso «Editar Asientos de directiva». +

+ ); +} diff --git a/apps/backstage/src/features/members/lib/assignable-cargo.ts b/apps/backstage/src/features/members/lib/assignable-cargo.ts index 97c1b5b0..982c985c 100644 --- a/apps/backstage/src/features/members/lib/assignable-cargo.ts +++ b/apps/backstage/src/features/members/lib/assignable-cargo.ts @@ -71,6 +71,24 @@ export function cargoTakedownOnly( export type CargoOption = { value: string; label: string; disabled?: boolean }; +/** + * The third derived render-state, alongside `positionsLockedForNonAdmin` and + * `cargoTakedownOnly`: this editor may assign, but the ceiling filtered every option away. + * + * Lives here for the same reason as its two siblings — both member forms ask it, and typing + * the three-clause condition at each call site is how the two forms drift. `locked` is passed + * in rather than recomputed because the forms derive it differently (one from the stored + * cargo, one from `defaultValues`), and it must be excluded: a locked slot renders its own + * note, and `cargoOptionsForEditor` appends the held cargo, so the two states cannot co-fire. + */ +export function noAssignableCargos(input: { + cargoOptions: readonly CargoOption[]; + allowPowerGrants: boolean; + locked: boolean; +}): boolean { + return !input.locked && !input.allowPowerGrants && input.cargoOptions.length === 0; +} + /** * The cargo Combobox options for one editor, shared by both member forms so they cannot * disagree about the same rules predicate (they did: one dropped the held CEL seat, the other diff --git a/apps/backstage/src/features/permissions/lib/permission-matrix.ts b/apps/backstage/src/features/permissions/lib/permission-matrix.ts index 1a7356a2..7c8e7cff 100644 --- a/apps/backstage/src/features/permissions/lib/permission-matrix.ts +++ b/apps/backstage/src/features/permissions/lib/permission-matrix.ts @@ -38,6 +38,8 @@ export const SUBJECT_LABELS: Record, string> = Lead: "Prospectos", Notification: "Notificaciones", Showcase: "Destacados públicos", + BoardSeat: "Asientos de directiva", + MemberLogin: "Acceso de miembros", }; /** Human label for a single code, e.g. "Editar Miembros". */ diff --git a/apps/backstage/src/features/positions/components/positions-page.tsx b/apps/backstage/src/features/positions/components/positions-page.tsx index b28bb549..84a76aab 100644 --- a/apps/backstage/src/features/positions/components/positions-page.tsx +++ b/apps/backstage/src/features/positions/components/positions-page.tsx @@ -34,7 +34,7 @@ export function PositionsPage() { const seedPositions = useSeedPositions(); // Grants + seed are power-grant writes the rules gate on the Admin *role* // (hasAnyRole(['Admin'])), not the manage:all perm — use the role-based capability. - const { isAdmin, canAssignPowerGrants } = useCan(); + const { isAdmin, canEditCargoCatalog } = useCan(); const [editing, setEditing] = useState(null); const [deactivateTarget, setDeactivateTarget] = useState(null); @@ -175,7 +175,7 @@ export function PositionsPage() { key={editing === "new" ? "new" : editing.id} defaultValues={editing === "new" ? undefined : positionToInput(editing)} submitLabel={editing === "new" ? "Crear" : "Guardar"} - canEditGrants={canAssignPowerGrants} + canEditGrants={canEditCargoCatalog} onSubmit={handleSubmit} /> )} diff --git a/apps/backstage/src/lib/authz/use-can.test.ts b/apps/backstage/src/lib/authz/use-can.test.ts index 52557627..bf4069f2 100644 --- a/apps/backstage/src/lib/authz/use-can.test.ts +++ b/apps/backstage/src/lib/authz/use-can.test.ts @@ -73,6 +73,56 @@ describe("buildCan", () => { expect(can({ roles: ["Member"], perms: ["manage:all"] }).canFeatureInitiatives).toBe(false); }); + // Mirrors firestore.rules' boardSeatDelegate(): Admin by ROLE, everyone else by the exact + // update:BoardSeat PERM. + it("canAssignBoardSeat holds for Admin by role or an update:BoardSeat perm", () => { + expect(can({ roles: ["Admin"] }).canAssignBoardSeat).toBe(true); + expect(can({ roles: ["Admin"], perms: [] }).canAssignBoardSeat).toBe(true); + expect(can({ roles: ["Member"], perms: ["update:BoardSeat"] }).canAssignBoardSeat).toBe(true); + expect(can({ roles: ["Membership"], perms: ["manage:Member"] }).canAssignBoardSeat).toBe(false); + }); + + it("canAssignBoardSeat is false for a manage:all perm holder without the Admin role", () => { + expect(can({ roles: ["Member"], perms: ["manage:all"] }).canAssignBoardSeat).toBe(false); + }); + + // THE C1 PIN. These were one flag while both were Admin-role-only; the delegation splits + // them, and re-unifying would hand a seat delegate the cargo CATALOG — the door round the + // back, since minting a grant-free CEL 'Presidente' and then seating yourself on it lands + // you at public board rank 0. + it("canEditCargoCatalog stays Admin-role-only — update:BoardSeat does NOT reach it", () => { + expect(can({ roles: ["Admin"] }).canEditCargoCatalog).toBe(true); + expect(can({ roles: ["Member"], perms: ["update:BoardSeat"] }).canEditCargoCatalog).toBe(false); + expect( + can({ roles: ["Member"], perms: ["update:Position", "update:BoardSeat"] }) + .canEditCargoCatalog, + ).toBe(false); + expect(can({ roles: ["Member"], perms: ["manage:all"] }).canEditCargoCatalog).toBe(false); + }); + + // Mirrors beacon's requireAdminOrPerm(request, "create:MemberLogin"). + it("canProvisionLogin holds for Admin by role or a create:MemberLogin perm", () => { + expect(can({ roles: ["Admin"] }).canProvisionLogin).toBe(true); + expect(can({ roles: ["Admin"], perms: [] }).canProvisionLogin).toBe(true); + expect(can({ roles: ["Member"], perms: ["create:MemberLogin"] }).canProvisionLogin).toBe(true); + expect(can({ roles: ["Membership"], perms: ["manage:Member"] }).canProvisionLogin).toBe(false); + }); + + it("canProvisionLogin is false for a manage:all perm holder without the Admin role", () => { + expect(can({ roles: ["Member"], perms: ["manage:all"] }).canProvisionLogin).toBe(false); + }); + + // The two delegations are independent by construction — an Admin may grant emailing + // without board seating and vice versa. Pinned because they ship together. + it("the two delegations do not imply each other", () => { + const seat = can({ roles: ["Member"], perms: ["update:BoardSeat"] }); + expect(seat.canAssignBoardSeat).toBe(true); + expect(seat.canProvisionLogin).toBe(false); + const login = can({ roles: ["Member"], perms: ["create:MemberLogin"] }); + expect(login.canProvisionLogin).toBe(true); + expect(login.canAssignBoardSeat).toBe(false); + }); + // Same invariant the `` gate carries: a conditional own-doc grant answers only the // per-document question, never the collection one. it("a plain Member's own-doc grant does not answer the collection question", () => { diff --git a/apps/backstage/src/lib/authz/use-can.ts b/apps/backstage/src/lib/authz/use-can.ts index 94dfa074..b6505fa0 100644 --- a/apps/backstage/src/lib/authz/use-can.ts +++ b/apps/backstage/src/lib/authz/use-can.ts @@ -1,6 +1,7 @@ import { useMemo } from "react"; import { hasAnyRole, hasPerm, type AuthClaims, type Role } from "@luminova/auth/roles"; import type { Action, AppAbility, Subject } from "@luminova/auth/ability"; +import type { PermissionCode } from "@luminova/types"; import type { ParticipationRole } from "@luminova/types/engine"; import { isNavItemVisible, type NavItem } from "../../components/nav-config"; import { canRemoveEntry } from "../../features/check-in/lib/can-remove-entry"; @@ -30,13 +31,36 @@ export interface Can { /** May curate the public /programas page (rules' `canCurateFeatured`). Named here so the * policy lives in one place, not scattered role-array literals at each call site. */ readonly canFeatureInitiatives: boolean; - /** The Admin-only half of the positions authority — one flag because firestore.rules keys - * every part of it on the same `hasAnyRole(['Admin'])`: assigning a power-granting cargo - * (`cargoAssignableByNonAdmin` / `currentCargoGrantsEmpty` / `createPositionsSafe`) or a CEL cargo - * at all, creating a board-surfacing cargo (`boardSurfacingCategory()`), and editing a - * stored cargo's `grants`, `category` or — on a board cargo — `title`/`titleFemale`. - * Named so the policy isn't a bare `isAdmin` at each grant site. */ - readonly canAssignPowerGrants: boolean; + /** May SEAT a member on a cargo the plain non-Admin lane refuses — a power-granting one or + * a CEL one. Mirrors firestore.rules' `boardSeatDelegate()` disjunct for disjunct. + * Governs the member CREATE and UPDATE lanes only. */ + readonly canAssignBoardSeat: boolean; + /** May AUTHOR the positions catalog: create a board-surfacing cargo + * (`boardSurfacingCategory()`), and edit a stored cargo's `grants`, `category` or — on a + * board cargo — `title`/`titleFemale`. + * + * Split from `canAssignBoardSeat` and deliberately NOT widened by the delegation. These + * were one flag while both were `hasAnyRole(['Admin'])`; they are different authorities + * and firestore.rules now keys them on different predicates. Re-unifying them would hand + * a seat delegate the catalog, and the catalog is the door round the back: mint a + * grant-free CEL 'Presidente', then seat yourself on it at public board rank 0. */ + readonly canEditCargoCatalog: boolean; + /** May run `provisionMemberLogin` — create the member's Auth account, link their uid, get + * the password-reset link. Mirrors beacon's + * `requireAdminOrPerm(request, "create:MemberLogin")`. Cargo-agnostic: it applies to every + * new member, board seat or not. + * + * NOT the invite email itself — `requestPasswordReset` is a client-side + * `sendPasswordResetEmail` any signed-in user can already call. */ + readonly canProvisionLogin: boolean; +} + +/** The shape every delegable capability gate takes: the Admin ROLE, or one exact permission + * code. Extracted at the third occurrence — `hasPerm` is deliberately NOT `abilityAllows`, + * and re-deriving that decision per flag is how one of them ends up looser than its rule. + * See the canFeatureInitiatives comment below for the full reasoning it encodes. */ +function adminOrPerm(claims: AuthClaims, code: PermissionCode): boolean { + return hasAnyRole(claims, ["Admin"]) || hasPerm(claims, code); } /** Pure builder — no React — so the gate logic is unit-testable. */ @@ -65,8 +89,12 @@ export function buildCan(ability: AppAbility, claims: AuthClaims): Can { // Destacar checkbox to a manage:all perm holder whose write firestore.rules then // rejects — taking the whole save down with it. `probe.ts` does not help here: it // narrows CONDITIONAL grants, and the divergence is the unconditional wildcard. - canFeatureInitiatives: hasAnyRole(claims, ["Admin"]) || hasPerm(claims, "update:Showcase"), - canAssignPowerGrants: hasAnyRole(claims, ["Admin"]), + canFeatureInitiatives: adminOrPerm(claims, "update:Showcase"), + // Same exact-code discipline as canFeatureInitiatives above, for the same reason: a + // `manage:all` holder must not see an affordance firestore.rules then rejects. + canAssignBoardSeat: adminOrPerm(claims, "update:BoardSeat"), + canEditCargoCatalog: hasAnyRole(claims, ["Admin"]), + canProvisionLogin: adminOrPerm(claims, "create:MemberLogin"), }; } diff --git a/apps/beacon/src/callable-auth.test.ts b/apps/beacon/src/callable-auth.test.ts new file mode 100644 index 00000000..9a9357e0 --- /dev/null +++ b/apps/beacon/src/callable-auth.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, it } from "vitest"; +import { HttpsError, type CallableRequest } from "firebase-functions/v2/https"; +import { callerIsAdmin, requireAdmin, requireAdminOrPerm } from "./callable-auth.js"; + +/** A callable request carrying just the claim shape the gates read. The cast is test-only + * and justified: CallableRequest carries rawRequest/acceptsStreaming/etc. that no gate + * touches, and building them would assert nothing. */ +function req(token?: Record): CallableRequest { + return (token === undefined ? {} : { auth: { uid: "u", token } }) as unknown as CallableRequest; +} + +function codeOf(fn: () => void): string { + try { + fn(); + } catch (err) { + return err instanceof HttpsError ? err.code : "not-an-https-error"; + } + return "no-throw"; +} + +describe("requireAdmin", () => { + it("rejects an unauthenticated caller", () => { + expect(codeOf(() => requireAdmin(req()))).toBe("unauthenticated"); + }); + it("rejects a signed-in non-Admin", () => { + expect(codeOf(() => requireAdmin(req({ roles: ["Member"] })))).toBe("permission-denied"); + }); + it("accepts an Admin", () => { + expect(codeOf(() => requireAdmin(req({ roles: ["Admin", "Member"] })))).toBe("no-throw"); + }); + it("rejects a manage:all perm holder who is not Admin by role", () => { + // requireAdmin is a ROLE gate; the wildcard perm has never satisfied it and must not + // start now that a sibling gate reads perms from the same token. + expect(codeOf(() => requireAdmin(req({ roles: ["Member"], perms: ["manage:all"] })))).toBe( + "permission-denied", + ); + }); +}); + +describe("requireAdminOrPerm", () => { + it("rejects an unauthenticated caller", () => { + expect(codeOf(() => requireAdminOrPerm(req(), "create:MemberLogin"))).toBe("unauthenticated"); + }); + + it("accepts an Admin carrying no perms claim at all", () => { + // The role disjunct must stand alone — an Admin whose perms claim has not been minted + // yet (or was fail-closed to empty by the cap) still passes. + expect(codeOf(() => requireAdminOrPerm(req({ roles: ["Admin"] }), "create:MemberLogin"))).toBe( + "no-throw", + ); + }); + + it("accepts a non-Admin holding the exact code", () => { + expect( + codeOf(() => + requireAdminOrPerm( + req({ roles: ["Member"], perms: ["create:MemberLogin"] }), + "create:MemberLogin", + ), + ), + ).toBe("no-throw"); + }); + + it("BLOCKING: manage:all does NOT satisfy it", () => { + // Exact-code, mirroring firestore.rules' hasPerm(). A canDo-style expansion here would + // hand the delegation to every wildcard holder silently. + expect( + codeOf(() => + requireAdminOrPerm(req({ roles: ["Member"], perms: ["manage:all"] }), "create:MemberLogin"), + ), + ).toBe("permission-denied"); + }); + + it("BLOCKING: manage:MemberLogin does NOT satisfy create:MemberLogin", () => { + // The subject wildcard is equally inert — the gate is the literal code, not canDo(). + expect( + codeOf(() => + requireAdminOrPerm( + req({ roles: ["Member"], perms: ["manage:MemberLogin"] }), + "create:MemberLogin", + ), + ), + ).toBe("permission-denied"); + }); + + it("keeps the two delegations independent", () => { + // A board-seat delegate is not a login provisioner and vice versa. Pinned because both + // codes ship together and the obvious future mistake is to conflate them. + expect( + codeOf(() => + requireAdminOrPerm( + req({ roles: ["Member"], perms: ["update:BoardSeat"] }), + "create:MemberLogin", + ), + ), + ).toBe("permission-denied"); + }); + + it("fails closed on a malformed perms claim", () => { + // A string (or anything non-array) reads as empty rather than throwing — a malformed + // token must deny, not 500. + expect( + codeOf(() => + requireAdminOrPerm( + req({ roles: ["Member"], perms: "create:MemberLogin" }), + "create:MemberLogin", + ), + ), + ).toBe("permission-denied"); + }); +}); + +describe("callerIsAdmin", () => { + it("is false for an unauthenticated caller and for a wildcard perm holder", () => { + expect(callerIsAdmin(req())).toBe(false); + expect(callerIsAdmin(req({ roles: ["Member"], perms: ["manage:all"] }))).toBe(false); + }); + it("is true only for the Admin role", () => { + expect(callerIsAdmin(req({ roles: ["Admin"] }))).toBe(true); + }); +}); diff --git a/apps/beacon/src/callable-auth.ts b/apps/beacon/src/callable-auth.ts index d84932f4..1c87adf1 100644 --- a/apps/beacon/src/callable-auth.ts +++ b/apps/beacon/src/callable-auth.ts @@ -1,10 +1,23 @@ import { HttpsError, type CallableRequest } from "firebase-functions/v2/https"; +import type { PermissionCode } from "@luminova/types"; -function callerRoles(request: CallableRequest): string[] { - const token = request.auth?.token as { roles?: unknown } | undefined; - return Array.isArray(token?.roles) - ? (token.roles as unknown[]).filter((role): role is string => typeof role === "string") - : []; +/** One reader for both string-array claims. `roles` and `perms` are read identically and + * had drifted into two copies of the same three lines the moment a second gate needed one. + * + * The `as` narrows `DecodedIdToken`'s `[key: string]: any` index signature to `unknown`, + * which is a tightening — every value is still filtered before use. Deliberately NOT + * `permsFromClaims` from claims-sync: that returns `PermissionCode[] | undefined` because + * `getExistingClaims` needs absence and empty to differ for its claim diff, and importing + * it would pull the Firestore port's runtime graph (chunk, role-doc, resolve-member-perms) + * into the callable trust boundary for a membership test. */ +function stringArrayClaim(request: CallableRequest, key: "roles" | "perms"): string[] { + const token = request.auth?.token as Record | undefined; + const raw = token?.[key]; + return Array.isArray(raw) ? raw.filter((v): v is string => typeof v === "string") : []; +} + +export function callerIsAdmin(request: CallableRequest): boolean { + return stringArrayClaim(request, "roles").includes("Admin"); } /** Reject anyone who isn't a signed-in Admin. Shared by every admin-only callable. */ @@ -12,7 +25,23 @@ export function requireAdmin(request: CallableRequest): void { if (!request.auth) { throw new HttpsError("unauthenticated", "sign-in required"); } - if (!callerRoles(request).includes("Admin")) { + if (!callerIsAdmin(request)) { throw new HttpsError("permission-denied", "Admin role required"); } } + +/** Admin by ROLE, or the exact permission code — the callable-side mirror of + * firestore.rules' `hasAnyRole(['Admin']) || hasPerm(code)`. + * + * Exact-code, deliberately not a `canDo`-style expansion: `manage:all` must not satisfy a + * delegation gate, or every wildcard holder silently becomes a delegate. Same discipline as + * the rules' `hasPerm()` and backstage's `hasPerm`. A malformed `perms` claim (non-array, + * string, absent) reads as empty and therefore denies. */ +export function requireAdminOrPerm(request: CallableRequest, code: PermissionCode): void { + if (!request.auth) { + throw new HttpsError("unauthenticated", "sign-in required"); + } + if (callerIsAdmin(request)) return; + if (stringArrayClaim(request, "perms").includes(code)) return; + throw new HttpsError("permission-denied", `Admin role or ${code} required`); +} diff --git a/apps/beacon/src/claims-sync/firestore-deps.ts b/apps/beacon/src/claims-sync/firestore-deps.ts index 2fa90a1f..f81c4958 100644 --- a/apps/beacon/src/claims-sync/firestore-deps.ts +++ b/apps/beacon/src/claims-sync/firestore-deps.ts @@ -4,6 +4,7 @@ import { isValidRole, type Role } from "@luminova/auth/roles"; import { isValidPermissionCode, type PermissionCode } from "@luminova/types/permission"; import { chunk } from "../chunk.js"; import { isSafeDocId } from "../firestore-util.js"; +import { readPositionGrants } from "../read-position-grants.js"; import { isActiveRoleDoc, permsFromRoleDoc } from "./role-doc.js"; import type { LiveBuiltInRoleDoc } from "./resolve-member-perms.js"; import type { ClaimsSyncDeps } from "./sync.js"; @@ -276,17 +277,18 @@ export function firestoreClaimsDeps(db: Firestore, auth: Auth): FirestoreClaimsD }, getPosition: async (id) => { // Belt-and-braces: resolveTrustedGrants in sync.ts already screens cargoId (that is - // the port-independent gate the test fakes inherit), but this is the site where an - // unscreened id becomes a permanent db.doc() throw, so it does not rely on its caller. - if (!isSafeDocId(id)) return null; - const snap = await db.doc(`positions/${id}`).get(); - if (!snap.exists) return null; - const grants = (snap.data()?.grants ?? []) as unknown[]; - return { grants: grants.filter((g): g is Role => isValidRole(g)) }; + // the port-independent gate the test fakes inherit), but readPositionGrants screens + // again at the site where an unscreened id becomes a permanent db.doc() throw, so this + // does not rely on its caller. + const grants = await readPositionGrants(db, id); + return grants === null ? null : { grants }; }, - getUserRoles: async (uid) => { + getAssignerClaims: async (uid) => { + // Same per-instance loadUser memo getExistingClaims uses — reading perms alongside + // roles costs no extra Auth read. const user = await loadUser(uid); - return user ? rolesFromClaims(user.customClaims as Record | undefined) : []; + const claims = user?.customClaims as Record | undefined; + return { roles: rolesFromClaims(claims), perms: permsFromClaims(claims) ?? [] }; }, getExistingClaims: async (uid) => { const user = await loadUser(uid); diff --git a/apps/beacon/src/claims-sync/sync.test.ts b/apps/beacon/src/claims-sync/sync.test.ts index 48fd7220..b5bef8f0 100644 --- a/apps/beacon/src/claims-sync/sync.test.ts +++ b/apps/beacon/src/claims-sync/sync.test.ts @@ -34,6 +34,9 @@ const customRole = (id: string, permissions: PermissionCode[]): RoleDefinition = function fakeDeps(opts: { positions: Record; userRoles: Record; + /** The assigner's `perms` claim — the second trust source alongside the Admin role, and the + * one that does NOT extend to conferring Admin (see resolveTrustedGrants). */ + userPerms?: Record; existing: Record; builtInDocs?: RoleDefinition[]; customRoles?: Record; @@ -42,7 +45,10 @@ function fakeDeps(opts: { const writes: Record = {}; const deps: ClaimsSyncDeps = { getPosition: async (id) => opts.positions[id] ?? null, - getUserRoles: async (uid) => opts.userRoles[uid] ?? [], + getAssignerClaims: async (uid) => ({ + roles: opts.userRoles[uid] ?? [], + perms: opts.userPerms?.[uid] ?? [], + }), getExistingClaims: async (uid) => opts.existing[uid] ?? { roles: [] }, // COVERAGE is preserved (no liveness filter — a deactivated built-in must still reach // resolveMemberPerms so it COVERS its key), but `live` is COMPUTED with the production @@ -126,6 +132,186 @@ describe("syncMemberClaims", () => { }); }); + it("honors NON-Admin power grants when the assigner holds update:BoardSeat", async () => { + // The mirror of the Admin case above, and the reason the perm disjunct exists: a delegate + // stamps their own uid into assignedBy, so without it the seat would land and mint nothing + // — the member published on the world-readable Directiva, powerless and silent. + const { deps, writes } = fakeDeps({ + positions: { "pos-dir": { grants: ["Membership"] } }, + userRoles: { "delegate-uid": ["Member"] }, + userPerms: { "delegate-uid": ["update:BoardSeat"] }, + existing: { "target-uid": { roles: ["Member"] } }, + }); + await syncMemberClaims( + deps, + { + uid: "target-uid", + positions: { "2026": { cargoId: "pos-dir", comisionIds: [], assignedBy: "delegate-uid" } }, + }, + "2026", + ); + expect(writes["target-uid"]).toEqual({ + roles: ["Membership", "Member"], + perms: permsFor(["Membership", "Member"]), + }); + }); + + it("BLOCKING: manage:all does NOT satisfy the trust gate", async () => { + // The gate is an exact code test, matching firestore.rules' hasPerm(). The CASL wildcard + // must not answer it on the server any more than it does in the client gate — otherwise + // any manage:all holder silently becomes a seat delegate. + const { deps, writes } = fakeDeps({ + positions: { "pos-dir": { grants: ["Membership"] } }, + userRoles: { "wildcard-uid": ["Member"] }, + userPerms: { "wildcard-uid": ["manage:all"] }, + existing: { "target-uid": { roles: ["Member"], perms: permsFor(["Member"]) } }, + }); + await syncMemberClaims( + deps, + { + uid: "target-uid", + positions: { "2026": { cargoId: "pos-dir", comisionIds: [], assignedBy: "wildcard-uid" } }, + }, + "2026", + ); + expect(writes["target-uid"]).toBeUndefined(); + }); + + it("BLOCKING: a delegate may NOT confer power on THEMSELVES", async () => { + // One write, no puppet: the spec's own recommended pairing (update:Position + + // update:BoardSeat) can seat itself on a Secretario/ProjectManager cargo through the + // positions-only lane, and without this that mints those roles onto the author — + // update:BoardSeat becomes a self-service grant of every built-in role but Admin. + // Conferring power on others is the delegation; on yourself it is self-promotion. + const { deps, writes } = fakeDeps({ + positions: { "pos-sec": { grants: ["Secretary"] } }, + userRoles: { "delegate-uid": ["Member"] }, + userPerms: { "delegate-uid": ["update:BoardSeat"] }, + existing: { "delegate-uid": { roles: ["Member"], perms: permsFor(["Member"]) } }, + }); + await syncMemberClaims( + deps, + { + uid: "delegate-uid", + positions: { "2026": { cargoId: "pos-sec", comisionIds: [], assignedBy: "delegate-uid" } }, + }, + "2026", + ); + expect(writes["delegate-uid"]).toBeUndefined(); + }); + + it("still lets a delegate confer a non-Admin cargo on SOMEONE ELSE", async () => { + // The paired ALLOW — the restriction is on self-dealing, not on the delegation itself. + const { deps, writes } = fakeDeps({ + positions: { "pos-sec": { grants: ["Secretary"] } }, + userRoles: { "delegate-uid": ["Member"] }, + userPerms: { "delegate-uid": ["update:BoardSeat"] }, + existing: { "target-uid": { roles: ["Member"] } }, + }); + await syncMemberClaims( + deps, + { + uid: "target-uid", + positions: { "2026": { cargoId: "pos-sec", comisionIds: [], assignedBy: "delegate-uid" } }, + }, + "2026", + ); + expect(writes["target-uid"]).toEqual({ + roles: ["Secretary", "Member"], + perms: permsFor(["Secretary", "Member"]), + }); + }); + + it("still lets an ADMIN seat themselves on a non-Admin cargo", async () => { + // The self-assignment half defers to the Admin ROLE, so it costs an Admin nothing. + const { deps, writes } = fakeDeps({ + positions: { "pos-sec": { grants: ["Secretary"] } }, + userRoles: { "admin-uid": ["Admin", "Member"] }, + existing: { "admin-uid": { roles: ["Admin", "Member"] } }, + }); + await syncMemberClaims( + deps, + { + uid: "admin-uid", + positions: { "2026": { cargoId: "pos-sec", comisionIds: [], assignedBy: "admin-uid" } }, + }, + "2026", + ); + expect(writes["admin-uid"]).toEqual({ + roles: ["Secretary", "Member"], + perms: permsFor(["Secretary", "Member"]), + }); + }); + + it("BLOCKING: a delegate may NOT confer ADMIN — the guard the delegation rests on", async () => { + // Conferring Admin is reserved to the Admin ROLE. Without this a delegate mints an Admin, + // that Admin is itself a trust source, and the delegation can never be revoked — via the + // one-write self-loop OR the two-write puppet loop (seat a second member you control; it + // is not a self-assignment, so a reflexivity check would miss it entirely). + const { deps, writes } = fakeDeps({ + positions: { "pos-pres": { grants: ["Admin"] } }, + userRoles: { "delegate-uid": ["Member"] }, + userPerms: { "delegate-uid": ["update:BoardSeat"] }, + existing: { "target-uid": { roles: ["Member"], perms: permsFor(["Member"]) } }, + }); + await syncMemberClaims( + deps, + { + uid: "target-uid", + positions: { "2026": { cargoId: "pos-pres", comisionIds: [], assignedBy: "delegate-uid" } }, + }, + "2026", + ); + expect(writes["target-uid"]).toBeUndefined(); + }); + + it("REGRESSION: an Admin who SELF-assigned their Admin cargo keeps it", async () => { + // The seeded bootstrap president: seed-president.mjs stamps assignedBy = their own uid, + // and their perms are manage:all — never the exact update:BoardSeat code. An earlier form + // of this guard keyed on `assignedBy === member.uid` and would have stripped exactly this + // member on their next write, in production, with no in-app recovery. Verified against the + // live member doc before it shipped; this pins it. + const { deps, writes } = fakeDeps({ + positions: { "pos-pres": { grants: ["Admin"] } }, + userRoles: { "president-uid": ["Admin", "Member"] }, + userPerms: { "president-uid": ["manage:all"] }, + existing: { + "president-uid": { roles: ["Admin", "Member"], perms: permsFor(["Admin", "Member"]) }, + }, + }); + await syncMemberClaims( + deps, + { + uid: "president-uid", + positions: { + "2026": { cargoId: "pos-pres", comisionIds: [], assignedBy: "president-uid" }, + }, + }, + "2026", + ); + // Idempotent no-op: the claims are already correct, so no write — and crucially not a strip. + expect(writes["president-uid"]).toBeUndefined(); + }); + + it("de-elevates a delegate-conferred NON-Admin grant once the perm is revoked", async () => { + // Revocation is real for everything a delegate can actually confer. + const { deps, writes } = fakeDeps({ + positions: { "pos-dir": { grants: ["Membership"] } }, + userRoles: { "delegate-uid": ["Member"] }, + userPerms: { "delegate-uid": [] }, + existing: { "target-uid": { roles: ["Membership", "Member"] } }, + }); + await syncMemberClaims( + deps, + { + uid: "target-uid", + positions: { "2026": { cargoId: "pos-dir", comisionIds: [], assignedBy: "delegate-uid" } }, + }, + "2026", + ); + expect(writes["target-uid"]).toEqual({ roles: ["Member"], perms: permsFor(["Member"]) }); + }); + it("BLOCKING: positive-and-inert — a grant-free cargo from a NON-Admin assigner mints nothing", async () => { // The members-positions lane (firestore.rules' fourth members update arm, keyed on // update:Position) lets an org-chart editor who is NOT an Admin assign GRANT-FREE cargos. @@ -153,9 +339,9 @@ describe("syncMemberClaims", () => { }); const spied: ClaimsSyncDeps = { ...deps, - getUserRoles: async (uid) => { + getAssignerClaims: async (uid) => { assignerLookups.push(uid); - return deps.getUserRoles(uid); + return deps.getAssignerClaims(uid); }, }; await syncMemberClaims( @@ -536,7 +722,7 @@ describe("syncMemberClaims", () => { const writes: Record = {}; const deps: ClaimsSyncDeps = { getPosition: async () => ({ grants: ["Admin"] }), - getUserRoles: async () => { + getAssignerClaims: async () => { throw new Error("auth lookup failed"); }, getExistingClaims: async () => ({ roles: ["Member"] }), diff --git a/apps/beacon/src/claims-sync/sync.ts b/apps/beacon/src/claims-sync/sync.ts index c1177ac4..a0b861a7 100644 --- a/apps/beacon/src/claims-sync/sync.ts +++ b/apps/beacon/src/claims-sync/sync.ts @@ -13,8 +13,13 @@ export interface MemberClaims { export interface ClaimsSyncDeps extends RolePermsDeps { /** Catalog position by id, or null if missing/deleted. */ getPosition(id: string): Promise<{ grants: Role[] } | null>; - /** The assigner's current claim roles (for the power-grant trust gate). */ - getUserRoles(uid: string): Promise; + /** The assigner's current claim roles AND perms (for the power-grant trust gate). + * Both, from one call: the gate asks a single question and two accessors would be two + * chances for a later edit to consult one and not the other. Deliberately not folded into + * `getExistingClaims` — the spy in sync.test.ts proves the gate is never REACHED on a + * grant-free cargo by asserting no assigner lookup happened, and a shared accessor would + * degrade that assertion to a uid-filtering heuristic. */ + getAssignerClaims(uid: string): Promise<{ roles: Role[]; perms: PermissionCode[] }>; /** The target member's existing custom claims. */ getExistingClaims(uid: string): Promise<{ roles: Role[]; perms?: PermissionCode[] }>; setClaims(uid: string, claims: MemberClaims): Promise; @@ -34,7 +39,7 @@ type MemberLike = { * `comisionIds` is the one slot rules cannot grant-check (no array iteration), so * honoring it would let a console-written power comisión — or a power cargo's id * smuggled into comisionIds — mint claims. Ignoring it entirely also means a - * Ignoring it entirely also means a permitted non-Admin positions edit (which restamps + * permitted non-Admin positions edit (which restamps * the shared `assignedBy`) can no longer strip Admin-granted power. That last part was * NOT true of the rules until currentCargoGrantsEmpty() landed: the rules denied * ASSIGNING a power cargo, never OVERWRITING one, so a manage:Member holder could @@ -43,14 +48,57 @@ type MemberLike = { * claim true; do not re-loosen it without re-reading this comment. * * The assigner lookup runs only when the cargo actually confers power. - * `getUserRoles` reads the assigner's LIVE claims, so a later Firestore write + * `getAssignerClaims` reads the assigner's LIVE claims, so a later Firestore write * that re-invokes this function re-evaluates trust: if the assigner has since * lost Admin, their previously granted power cargo is revoked and claims - * reflect current org state (by design). */ + * reflect current org state (by design). + * + * TWO trust sources, mirroring firestore.rules' boardSeatDelegate(): the Admin ROLE, or + * the exact `update:BoardSeat` PERM. The perm is not optional politeness — a delegate + * stamps their OWN uid into `assignedBy` (the rules' assignedBySelf()), so without it the + * seat lands, the member is published on the world-readable Directiva, and no claim is + * minted. Visible, powerless, and silent: half-working rather than safe. + * + * TWO restrictions, and together they are the guard the whole delegation rests on: + * - a cargo whose grants include `Admin` is honored only for an assigner holding the Admin + * ROLE; and + * - a SELF-assignment is honored only for an Admin, whatever the cargo grants. + * Everything else — a delegate seating SOMEONE ELSE on a non-Admin power cargo — is honored + * for an `update:BoardSeat` holder, and that is the feature. + * + * The self-assignment half is not the discredited reflexivity check (see below); it is + * narrower and it closes a different hole. Without it `update:BoardSeat` is a self-service + * grant of every built-in role but Admin: the spec's own recommended pairing + * (`update:Position` + `update:BoardSeat`) can write `positions. = { cargoId: + * , assignedBy: }` onto its OWN member doc + * through the positions-only lane — one write, no puppet needed — and claims-sync would mint + * those roles onto the author. Conferring power on others is the delegation; conferring it on + * yourself is self-promotion. + * + * Why the ADMIN half keys on the grant rather than on reflexivity (which is what this first + * shipped as): for Admin the danger is not reflexivity, it is that a delegate can mint an + * Admin AT ALL, because a minted Admin is itself a trust source and the delegation then + * cannot be revoked. Blocking only `assignedBy === memberUid` stops the one-write self-loop + * and not the two-write puppet loop — a delegate creates a second member on a mailbox they control, seats IT on + * Presidente (not a self-assignment, so the perm is trusted), and that puppet is Admin + * forever; revoking the delegate's code de-elevates nobody. It also had a worse problem: the + * seeded bootstrap president self-stamps `assignedBy` (tools/scripts/lib/seed-president.mjs) + * and their perms are `manage:all`, never the exact `update:BoardSeat` code — so the + * self-assignment form stripped the sitting president's Admin on their next member write. + * + * Keying on the GRANT fixes both: no cargo-derived Admin can ever originate from a delegate, + * so there is no loop to close and no anchor to special-case, and an Admin seating anyone — + * including themselves — is untouched. The self-assignment half rides alongside it and is + * safe for the same reason: it too defers to the Admin ROLE, which the seeded president has. Revocation is then real for everything a delegate + * CAN confer: strip the perm and the next write to that member drops the grants. + * + * The cost, stated so nobody reads it as a bug: a delegate seating a member on an + * Admin-granting cargo publishes the seat but mints no claim. An Admin must re-stamp it. */ async function resolveTrustedGrants( deps: ClaimsSyncDeps, cargoId: string | null, assignedBy: string | undefined, + memberUid: string, ): Promise { // FULL screening, not just the empty-string half this used to check. `cargoId` comes // straight off the member doc and every implementation of `getPosition` interpolates it @@ -65,10 +113,17 @@ async function resolveTrustedGrants( if (!isSafeDocId(cargoId)) return []; const position = await deps.getPosition(cargoId); if (!position || position.grants.length === 0) return []; - const assignerIsAdmin = assignedBy - ? (await deps.getUserRoles(assignedBy)).includes("Admin") - : false; - return assignerIsAdmin ? [...new Set(position.grants)] : []; + if (!assignedBy) return []; + const assigner = await deps.getAssignerClaims(assignedBy); + const assignerIsAdmin = assigner.roles.includes("Admin"); + // A delegate may confer power on OTHERS, never on themselves, and never Admin at all. + // Both halves need the Admin role; only the third case honors the perm. + const selfAssigned = assignedBy === memberUid; + const trusted = + position.grants.includes("Admin") || selfAssigned + ? assignerIsAdmin + : assignerIsAdmin || assigner.perms.includes("update:BoardSeat"); + return trusted ? [...new Set(position.grants)] : []; } function sameList(a: readonly string[], b: readonly string[]): boolean { @@ -96,7 +151,12 @@ export async function syncMemberClaims( ): Promise { if (!member.uid) return; const term = member.positions?.[termKey]; - const trustedGrants = await resolveTrustedGrants(deps, term?.cargoId ?? null, term?.assignedBy); + const trustedGrants = await resolveTrustedGrants( + deps, + term?.cargoId ?? null, + term?.assignedBy, + member.uid, + ); const existing = await deps.getExistingClaims(member.uid); const hadScanner = existing.roles.includes("Scanner"); @@ -113,6 +173,9 @@ export async function syncMemberClaims( // keep a stale grant while dropping a revoke). We still write the recomputed // `roles` + empty `perms` so a concurrent role revocation always lands — // never leave the member on stale, possibly-elevated claims. + // Note for update:BoardSeat holders: this takes their delegation with it, silently. A + // delegate over the cap keeps seating (their cached token still passes the rules) while + // this function stops honoring the grants — the seat publishes, no claim is minted. deps.logError?.("effective perms exceed cap; writing empty perms (fail-closed)", { uid: member.uid, count: perms.length, diff --git a/apps/beacon/src/provision-deps.ts b/apps/beacon/src/provision-deps.ts index 15aa1175..5deef27d 100644 --- a/apps/beacon/src/provision-deps.ts +++ b/apps/beacon/src/provision-deps.ts @@ -1,5 +1,6 @@ import type { Auth } from "firebase-admin/auth"; import type { Firestore } from "firebase-admin/firestore"; +import { readPositionGrants } from "./read-position-grants.js"; import type { ProvisionDeps } from "./provision-member-login.js"; // Null only for the "account does not exist" outcome — a transient Auth error @@ -18,12 +19,19 @@ export function firestoreProvisionDeps(db: Firestore, auth: Auth): ProvisionDeps getUserByEmail: (email) => auth.getUserByEmail(email).catch(nullIfUserNotFound), getUserByUid: (uid) => auth.getUser(uid).catch(nullIfUserNotFound), // Tolerate a concurrent create (a parallel invite would otherwise throw - // auth/email-already-exists). - createUser: (email) => auth.createUser({ email }).catch(() => auth.getUserByEmail(email)), + // auth/email-already-exists) — and ONLY that. A blanket catch also swallowed quota, + // disabled-provider and invalid-email errors and re-surfaced them as an unrelated + // auth/user-not-found, destroying the diagnostic. + createUser: (email) => + auth.createUser({ email }).catch((err: unknown) => { + if ((err as { code?: unknown } | null)?.code !== "auth/email-already-exists") throw err; + return auth.getUserByEmail(email); + }), setClaims: (uid, claims) => auth.setCustomUserClaims(uid, claims), linkUid: async (id, uid) => { await db.doc(`members/${id}`).update({ uid }); }, passwordResetLink: (email) => auth.generatePasswordResetLink(email), + getPositionGrants: (cargoId) => readPositionGrants(db, cargoId), }; } diff --git a/apps/beacon/src/provision-member-login.test.ts b/apps/beacon/src/provision-member-login.test.ts index 05dc2ef2..14e9f3c8 100644 --- a/apps/beacon/src/provision-member-login.test.ts +++ b/apps/beacon/src/provision-member-login.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect } from "vitest"; +import type { Role } from "@luminova/auth/roles"; import { validateProvisionInput, nextClaims, @@ -35,6 +36,7 @@ describe("nextClaims", () => { function fakeDeps(opts: { member?: Record | null; usersByEmail?: Record; + positions?: Record; }) { const calls = { createUser: [] as string[], setClaims: [] as string[], linkUid: [] as string[] }; const users = opts.usersByEmail ?? {}; @@ -55,12 +57,14 @@ function fakeDeps(opts: { }, getUserByUid: async (uid) => Object.values(users).find((u) => u.uid === uid) ?? null, passwordResetLink: async (email) => `link:${email}`, + getPositionGrants: async (cargoId) => opts.positions?.[cargoId] ?? null, }; return { deps, calls }; } describe("provisionMember", () => { const active = { email: "a@b.co", active: true }; + const TERM = String(new Date().getUTCFullYear()); it("rejects when the member is already linked to a DIFFERENT live auth user (email changed)", async () => { const { deps, calls } = fakeDeps({ @@ -89,31 +93,40 @@ describe("provisionMember", () => { }); it("self-heals a stale link when the linked account was deleted — adopts by email, de-elevated", async () => { + // ADMIN caller. The self-heal is an adoption too — it binds an account this member was + // never linked to — so it sits behind the same guard, and deliberately: `email` is not + // pinned on the members update arm either, so an update:Member delegate could retarget an + // already-linked member at an Admin's mailbox and reach this branch whenever the stale + // link happens to be dead. Recovery from a deleted account stays an Admin op. const { deps, calls } = fakeDeps({ member: { ...active, uid: "dead-uid" }, usersByEmail: { "a@b.co": { uid: "u2", email: "a@b.co", customClaims: { roles: ["Admin"] } }, }, }); - const result = await provisionMember(deps, "m1"); + const result = await provisionMember(deps, "m1", true); expect(result.email).toBe("a@b.co"); expect(calls.createUser).toEqual([]); expect(calls.linkUid).toEqual(["u2"]); }); it("self-heals a stale link by minting a fresh account when the email resolves nothing", async () => { + // ADMIN caller: a stored uid means this member was provisioned once already, so recovery + // is an Admin op. A delegate hits the reprovision guard instead (test below). const { deps, calls } = fakeDeps({ member: { ...active, uid: "dead-uid" } }); - await provisionMember(deps, "m1"); + await provisionMember(deps, "m1", true); expect(calls.createUser).toEqual(["a@b.co"]); expect(calls.linkUid).toEqual(["new-a@b.co"]); }); it("re-provisions idempotently when the stored uid matches the resolved user (resend invite)", async () => { + // ADMIN caller. Resend returns a live password-reset link for the member's address, so it + // is Admin-only — see the non-Admin BLOCKING case below. const { deps, calls } = fakeDeps({ member: { ...active, uid: "u1" }, usersByEmail: { "a@b.co": { uid: "u1", email: "a@b.co" } }, }); - const result = await provisionMember(deps, "m1"); + const result = await provisionMember(deps, "m1", true); expect(result).toEqual({ email: "a@b.co", actionLink: "link:a@b.co" }); expect(calls.createUser).toEqual([]); expect(calls.setClaims).toEqual(["u1"]); @@ -121,8 +134,9 @@ describe("provisionMember", () => { }); it("provisions an unlinked member, creating the auth user when absent", async () => { + // ADMIN caller: only an Admin receives the action link (see the delegate pair below). const { deps, calls } = fakeDeps({ member: active }); - const result = await provisionMember(deps, "m1"); + const result = await provisionMember(deps, "m1", true); expect(result).toEqual({ email: "a@b.co", actionLink: "link:a@b.co" }); expect(calls.createUser).toEqual(["a@b.co"]); expect(calls.setClaims).toEqual(["new-a@b.co"]); @@ -130,17 +144,308 @@ describe("provisionMember", () => { }); it("reuses an existing auth user for an unlinked member (pre-created account)", async () => { + // ADMIN caller: adoption is the documented recovery op and stays open for the Admin role. + // The `true` is load-bearing — the same call with `false` is the takedown case below. const { deps, calls } = fakeDeps({ member: active, usersByEmail: { "a@b.co": { uid: "u9", email: "a@b.co", customClaims: { roles: ["Scanner"] } }, }, }); - await provisionMember(deps, "m1"); + await provisionMember(deps, "m1", true); expect(calls.createUser).toEqual([]); expect(calls.linkUid).toEqual(["u9"]); }); + it("BLOCKING: a non-Admin caller may NOT adopt a pre-existing unlinked account", async () => { + // The takeover this guard closes: firestore.rules never constrains members.email and + // there is no uniqueness check, so a create:Member + create:MemberLogin delegate could + // file a member doc carrying a sitting Admin's email. Reaching the writes below would + // strip that Admin's claims (adoptedClaims), bind their uid to the attacker's member doc + // through the admin SDK, and hand back a password-reset link for their mailbox. + // Same fixture as "reuses an existing auth user for an unlinked member" one test above — + // the ONLY difference is the caller's privilege. + const { deps, calls } = fakeDeps({ + member: active, + usersByEmail: { + "a@b.co": { uid: "u9", email: "a@b.co", customClaims: { roles: ["Admin"] } }, + }, + }); + await expect(provisionMember(deps, "m1", false)).rejects.toMatchObject({ + code: "permission-denied", + }); + // Nothing partial: no claim write, no uid link, no reset link generated. + expect(calls.setClaims).toEqual([]); + expect(calls.linkUid).toEqual([]); + expect(calls.createUser).toEqual([]); + }); + + it("still lets a non-Admin caller mint a BRAND-NEW account", async () => { + // The delegation costs nothing on the path it is actually for: a genuinely new member + // has neither an Auth account nor a stored uid. + const fresh = fakeDeps({ member: active }); + await expect(provisionMember(fresh.deps, "m1", false)).resolves.toEqual({ + email: "a@b.co", + actionLink: "", + }); + expect(fresh.calls.createUser).toEqual(["a@b.co"]); + }); + + it("BLOCKING: a non-Admin caller may NOT re-provision an ALREADY-LINKED member", async () => { + // The resend path is an account-takeover primitive, not a convenience. passwordResetLink + // is generatePasswordResetLink — it hands the oobCode URL to the CALLER, unlike the + // client-side sendPasswordResetEmail which delivers it to the mailbox owner. So without + // this, a create:MemberLogin holder could pass the president's memberId, receive a live + // reset link for their address, and sign in as them. No adoption, no forged email — + // every other guard satisfied. + const { deps, calls } = fakeDeps({ + member: { ...active, uid: "u1" }, + usersByEmail: { + "a@b.co": { uid: "u1", email: "a@b.co", customClaims: { roles: ["Admin"] } }, + }, + }); + await expect(provisionMember(deps, "m1", false)).rejects.toMatchObject({ + code: "permission-denied", + }); + expect(calls.setClaims).toEqual([]); + expect(calls.linkUid).toEqual([]); + }); + + it("BLOCKING: a non-Admin caller may NOT provision a POWER-SEATED member", async () => { + // The escalation this closes, and the delegate forges nothing to get it: any uid-less + // member is reachable — including one an Admin already seated on an Admin-granting cargo, + // which is the normal state between being seated and being invited. linkUid() fires + // onMemberWritten, resolveTrustedGrants reads the STORED assignedBy (a genuine Admin), + // honors the grants, and mints Admin onto the uid this call just created. The attacker + // then reaches that uid through the invite mail — which lands in THEIR inbox if they also + // hold manage:Member and rewrote members.email first, since the rules never pin it. + const { deps, calls } = fakeDeps({ + member: { + ...active, + positions: { [TERM]: { cargoId: "pos-pres", comisionIds: [], assignedBy: "admin-uid" } }, + }, + positions: { "pos-pres": ["Admin"] }, + }); + await expect(provisionMember(deps, "m1", false)).rejects.toMatchObject({ + code: "permission-denied", + details: { reason: "power-seat-requires-admin" }, + }); + expect(calls.createUser).toEqual([]); + expect(calls.linkUid).toEqual([]); + }); + + it("BLOCKING: a non-Admin caller may NOT provision a member carrying DIRECT grants", async () => { + // The other half of the claims-mint surface. syncMemberClaims mints `roles` from trusted + // cargo grants AND `perms` from roleIds + permissionOverrides — the second path needs no + // cargo at all, and "granted but not yet invited" is exactly what the Admin-only roles + // panel produces. Without this, a manage:Member + create:MemberLogin holder rewrites such + // a member's email (the rules never pin it), provisions them, and the account they now + // control is minted that member's whole granted perm set — which may itself include + // update:BoardSeat, chaining into the seating lane. + const granted: Record[] = [ + { roleIds: ["custom-role"] }, + { permissionOverrides: { grant: ["update:BoardSeat"], revoke: [] } }, + ]; + for (const fields of granted) { + const { deps, calls } = fakeDeps({ member: { ...active, ...fields } }); + await expect(provisionMember(deps, "m1", false)).rejects.toMatchObject({ + code: "permission-denied", + details: { reason: "granted-member-requires-admin" }, + }); + expect(calls.createUser).toEqual([]); + expect(calls.linkUid).toEqual([]); + } + }); + + it("fails closed on a PRESENT but malformed grants shape", async () => { + const malformed: Record[] = [ + { roleIds: "custom-role" }, + { roleIds: {} }, + { permissionOverrides: "nope" }, + { permissionOverrides: { grant: "update:BoardSeat" } }, + { permissionOverrides: ["manage:all"] }, + ]; + for (const fields of malformed) { + const { deps } = fakeDeps({ member: { ...active, ...fields } }); + await expect(provisionMember(deps, "m1", false)).rejects.toMatchObject({ + code: "permission-denied", + details: { reason: "granted-member-requires-admin" }, + }); + } + }); + + it("treats empty / absent / explicitly-null grants as ungranted", async () => { + // The paired ALLOW. The rules' unchanged()/touched() gap admits an explicit null, and + // parseMember resolves that to [] — so null must read as "no grants", not as malformed, + // or ordinary members become un-invitable by a delegate. + const ungranted: Record[] = [ + {}, + { roleIds: [] }, + { roleIds: null }, + { permissionOverrides: null }, + { permissionOverrides: { grant: [], revoke: [] } }, + { permissionOverrides: { revoke: ["read:Member"] } }, + { roleIds: [], permissionOverrides: { grant: [], revoke: [] } }, + ]; + for (const fields of ungranted) { + const { deps } = fakeDeps({ member: { ...active, ...fields } }); + await expect(provisionMember(deps, "m1", false)).resolves.toMatchObject({ + email: "a@b.co", + }); + } + }); + + it("fails closed on a PRESENT but malformed positions shape", async () => { + // The guard's own bypass if these read as "no cargo". None is produced by a client write + // path — assignedBySelf() errors on a non-object term and the rules deny — but a console + // edit or a partial migration reaches them, and the whole point of the guard is that an + // unreadable cargo is not an absent one. + const shapes: Record[] = [ + { positions: "not-an-object" }, + { positions: { [TERM]: "not-an-object" } }, + { positions: { [TERM]: { cargoId: 42, comisionIds: [] } } }, + { positions: { [TERM]: { cargoId: "", comisionIds: [] } } }, + ]; + for (const positions of shapes) { + const { deps, calls } = fakeDeps({ member: { ...active, ...positions } }); + await expect(provisionMember(deps, "m1", false)).rejects.toMatchObject({ + code: "permission-denied", + details: { reason: "power-seat-requires-admin" }, + }); + expect(calls.createUser).toEqual([]); + } + }); + + it("treats a genuinely ABSENT cargo as unseated, not as malformed", async () => { + // The paired ALLOW. Without it the fail-closed test above would pass for a guard that + // simply refused every non-Admin provision, which is the whole delegation. + const shapes: Record[] = [ + {}, + { positions: {} }, + { positions: { [TERM]: { comisionIds: [] } } }, + { positions: { [TERM]: { cargoId: null, comisionIds: [] } } }, + { positions: { "1999": { cargoId: "pos-unknown-but-grantfree", comisionIds: [] } } }, + ]; + for (const positions of shapes) { + const { deps } = fakeDeps({ + member: { ...active, ...positions }, + positions: { "pos-unknown-but-grantfree": [] }, + }); + await expect(provisionMember(deps, "m1", false)).resolves.toMatchObject({ + email: "a@b.co", + }); + } + }); + + it("BLOCKING: a FUTURE-term power cargo is refused too, not just the current term", async () => { + // syncMemberClaims reads positions[currentTermKey()] at TRIGGER time, so a next-term entry + // is invisible today and mints on the UTC-year rollover — a genuine Admin in assignedBy, + // the cargo's grants honored, onto an account a delegate caused to exist. Every client + // lane is term-pinned so the shape needs a console edit or a migration, which is the same + // reachability this file already fail-closes on for a malformed cargoId. + const { deps, calls } = fakeDeps({ + member: { + ...active, + positions: { "2099": { cargoId: "pos-pres", comisionIds: [], assignedBy: "admin-uid" } }, + }, + positions: { "pos-pres": ["Admin"] }, + }); + await expect(provisionMember(deps, "m1", false)).rejects.toMatchObject({ + code: "permission-denied", + details: { reason: "power-seat-requires-admin" }, + }); + expect(calls.createUser).toEqual([]); + }); + + it("still allows a delegate when EVERY term's cargo is grant-free", async () => { + const { deps, calls } = fakeDeps({ + member: { + ...active, + positions: { + [TERM]: { cargoId: "pos-dir", comisionIds: [], assignedBy: "delegate-uid" }, + "2099": { cargoId: "pos-dir2", comisionIds: [], assignedBy: "delegate-uid" }, + }, + }, + positions: { "pos-dir": [], "pos-dir2": [] }, + }); + await expect(provisionMember(deps, "m1", false)).resolves.toMatchObject({ email: "a@b.co" }); + expect(calls.createUser).toEqual(["a@b.co"]); + }); + + it("fails closed when the seated cargo cannot be read", async () => { + // A missing or malformed cargo must not read as "no cargo" — that would be the guard's + // own bypass. + const missing = fakeDeps({ + member: { + ...active, + positions: { [TERM]: { cargoId: "pos-ghost", comisionIds: [], assignedBy: "admin-uid" } }, + }, + }); + await expect(provisionMember(missing.deps, "m1", false)).rejects.toMatchObject({ + code: "permission-denied", + }); + const malformed = fakeDeps({ + member: { + ...active, + positions: { [TERM]: { cargoId: "a/b", comisionIds: [], assignedBy: "admin-uid" } }, + }, + }); + await expect(provisionMember(malformed.deps, "m1", false)).rejects.toMatchObject({ + code: "permission-denied", + }); + }); + + it("still lets a delegate provision a member seated on a GRANT-FREE cargo", async () => { + // Seating plus inviting on a grant-free cargo mints nothing, and is exactly the enrolment + // flow the delegation exists for. Without this pair the guard above would pass for a rule + // that simply refused every seated member. + const { deps, calls } = fakeDeps({ + member: { + ...active, + positions: { [TERM]: { cargoId: "pos-dir", comisionIds: [], assignedBy: "delegate-uid" } }, + }, + positions: { "pos-dir": [] }, + }); + await expect(provisionMember(deps, "m1", false)).resolves.toMatchObject({ email: "a@b.co" }); + expect(calls.createUser).toEqual(["a@b.co"]); + }); + + it("BLOCKING: a delegate never receives the password-reset link", async () => { + // generatePasswordResetLink returns a bearer credential for the account. The client sends + // the reset mail itself through the unprivileged sendPasswordResetEmail, so a delegate has + // no need to hold it. Defence in depth behind the power-seat guard, not a substitute. + const delegate = fakeDeps({ member: active }); + await expect(provisionMember(delegate.deps, "m1", false)).resolves.toEqual({ + email: "a@b.co", + actionLink: "", + }); + const admin = fakeDeps({ member: active }); + await expect(provisionMember(admin.deps, "m1", true)).resolves.toEqual({ + email: "a@b.co", + actionLink: "link:a@b.co", + }); + }); + + it("BLOCKING: a non-Admin caller may NOT provision a member whose uid is set but account is gone", async () => { + // The self-heal branch: linkedUid points at a deleted account, so getUserByEmail may + // return null and the adoption half alone would let this through. A stored uid means an + // Admin already provisioned this member once — recovery is theirs. + const { deps, calls } = fakeDeps({ member: { ...active, uid: "dead-uid" } }); + await expect(provisionMember(deps, "m1", false)).rejects.toMatchObject({ + code: "permission-denied", + }); + expect(calls.createUser).toEqual([]); + }); + + it("defaults callerIsAdmin to false — a new call site must opt into adoption", async () => { + // The parameter defaults closed so an added caller that forgets it gets the SAFE path. + const { deps } = fakeDeps({ + member: active, + usersByEmail: { "a@b.co": { uid: "u9", email: "a@b.co" } }, + }); + await expect(provisionMember(deps, "m1")).rejects.toMatchObject({ code: "permission-denied" }); + }); + it("rejects a missing / inactive / email-less member", async () => { await expect(provisionMember(fakeDeps({ member: null }).deps, "m1")).rejects.toMatchObject({ code: "not-found", @@ -154,6 +459,9 @@ describe("provisionMember", () => { }); }); +// Every case here exercises the ADOPTION path, which is Admin-only — hence the explicit +// `true` third argument throughout. A non-Admin caller is refused before any of this runs +// (see "a non-Admin caller may NOT adopt a pre-existing unlinked account"). describe("provisionMember — stale-claims bootstrap (fresh adopt)", () => { it("strips stale org roles when adopting a pre-existing auth account, keeping Scanner", async () => { const claimsWrites: Record[] = []; @@ -173,7 +481,7 @@ describe("provisionMember — stale-claims bootstrap (fresh adopt)", () => { claimsWrites.push(claims); }, }; - await provisionMember(spied, "m1"); + await provisionMember(spied, "m1", true); expect(claimsWrites).toEqual([{ roles: ["Scanner", "Member"] }]); }); @@ -191,7 +499,7 @@ describe("provisionMember — stale-claims bootstrap (fresh adopt)", () => { claimsWrites.push(claims); }, }; - await provisionMember(spied, "m1"); + await provisionMember(spied, "m1", true); expect(claimsWrites).toEqual([{ roles: ["Member"] }]); }); @@ -209,7 +517,7 @@ describe("provisionMember — stale-claims bootstrap (fresh adopt)", () => { claimsWrites.push(claims); }, }; - await provisionMember(spied, "m1"); + await provisionMember(spied, "m1", true); expect(claimsWrites).toEqual([{ roles: ["Admin", "Member"] }]); }); }); diff --git a/apps/beacon/src/provision-member-login.ts b/apps/beacon/src/provision-member-login.ts index 7a602884..01c07b86 100644 --- a/apps/beacon/src/provision-member-login.ts +++ b/apps/beacon/src/provision-member-login.ts @@ -2,7 +2,8 @@ import { getAuth } from "firebase-admin/auth"; import { getFirestore } from "firebase-admin/firestore"; import { HttpsError, onCall } from "firebase-functions/v2/https"; import { isValidRole, type Role } from "@luminova/auth/roles"; -import { requireAdmin } from "./callable-auth.js"; +import { isSafeDocId } from "./firestore-util.js"; +import { callerIsAdmin, requireAdminOrPerm } from "./callable-auth.js"; import { firestoreProvisionDeps } from "./provision-deps.js"; import { ensureApp } from "./runtime.js"; @@ -16,7 +17,10 @@ interface RawClaims { export function validateProvisionInput(data: unknown): ProvisionInput { const raw = (data ?? {}) as { memberId?: unknown }; - if (typeof raw.memberId !== "string" || raw.memberId.length === 0 || raw.memberId.includes("/")) { + // isSafeDocId, not a hand-rolled subset: `..` and `__x__` build a valid ref and then fail at + // get() with a permanent INVALID_ARGUMENT, which surfaces as `internal` (a 500) instead of + // the invalid-argument this is meant to return. + if (!isSafeDocId(raw.memberId)) { throw new HttpsError("invalid-argument", "memberId is required"); } return { memberId: raw.memberId }; @@ -47,6 +51,9 @@ export interface ProvisionDeps { setClaims(uid: string, claims: ReturnType): Promise; linkUid(memberId: string, uid: string): Promise; passwordResetLink(email: string): Promise; + /** The cargo's grants, or null if missing. Only consulted to refuse a non-Admin + * provisioning of a POWER-SEATED member — see the power-seat guard. */ + getPositionGrants(cargoId: string): Promise; } /** Claims carried over when adopting an Auth account not currently linked to @@ -61,16 +68,92 @@ function adoptedClaims(existing: RawClaims | undefined): RawClaims { return { roles }; } +/** Whether this member carries DIRECT grants — a custom role or a per-member override. + * + * The second half of the privileged-member question. syncMemberClaims mints from two + * independent sources: trusted cargo grants become `roles`, and `roleIds` + + * `permissionOverrides` become `perms` (resolveMemberPerms). A guard that reads only the + * cargo half leaves the other wide open — and `roleIds`/`permissionOverrides` are exactly + * what the Admin-only panel writes, so "granted but not yet invited" is as ordinary a state + * as "seated but not yet invited". + * + * Fail-closed on any shape that is not a clean empty: a present-but-unparseable `roleIds` + * must refuse, not read as "no grants". Absent and null are the genuine empties — the rules' + * unchanged()/touched() gap admits an explicit null, and parseMember resolves that to []. */ +function hasDirectGrants(member: Record): boolean { + const roleIds = member.roleIds; + if (roleIds !== undefined && roleIds !== null) { + if (!Array.isArray(roleIds)) return true; + if (roleIds.length > 0) return true; + } + const overrides = member.permissionOverrides; + if (overrides === undefined || overrides === null) return false; + // Array before the typeof: `typeof [] === "object"`, so a legacy/console + // `permissionOverrides: ["manage:all"]` would reach `.grant === undefined` and read as + // ungranted — failing OPEN, which is what the roleIds branch above refuses to do. + if (Array.isArray(overrides) || typeof overrides !== "object") return true; + const grant = (overrides as { grant?: unknown }).grant; + if (grant === undefined || grant === null) return false; + if (!Array.isArray(grant)) return true; + return grant.length > 0; +} + +/** Every cargo id in the member's positions map, for the power-seat guard. + * + * EVERY term, not just the current one — and that is the point. `syncMemberClaims` reads + * `positions[currentTermKey()]` at TRIGGER time, so a future-term entry is invisible today + * and mints on the UTC-year rollover. All client write lanes are term-pinned + * (`positionsDelta().hasOnly()` on update, `keys().hasOnly()` on create, both binding Admins + * too), so such a map takes a console edit, an admin-SDK write or a legacy migration — the + * same reachability this file already fail-closes on for a malformed cargoId, and a + * console-authored next-term board slate is the more plausible of the two. + * + * Yields: + * a usable id — read its grants. + * "" — present but unreadable (a non-object entry, a non-string or empty + * cargoId, or an id `isSafeDocId` rejects). Deliberately NOT skipped: "" + * fails `isSafeDocId` at the port too, so the guard refuses. A malformed + * shape must never read as "no cargo" — that is the guard's own bypass. + * A genuinely absent cargo (no map, no entry, or `cargoId` absent/null) yields nothing, so an + * unseated member produces an empty list and the delegate may enrol them. */ +function readCargoIds(member: Record): string[] { + const positions = member.positions; + if (positions === undefined || positions === null) return []; + if (typeof positions !== "object") return [""]; + const ids: string[] = []; + for (const term of Object.values(positions as Record)) { + if (term === undefined || term === null) continue; + if (typeof term !== "object") { + ids.push(""); + continue; + } + const cargoId = (term as { cargoId?: unknown }).cargoId; + if (cargoId === undefined || cargoId === null) continue; + if (typeof cargoId !== "string" || cargoId.length === 0) { + ids.push(""); + continue; + } + ids.push(isSafeDocId(cargoId) ? cargoId : ""); + } + return [...new Set(ids)]; +} + /** Provision (or re-provision) a member's login. Refuses to relink a member whose * stored uid does not match the Auth user its email resolves to — silently * overwriting would orphan the old Auth account with its claims (possibly Admin) * still live and no member doc backing them. Relinking after an email change is a * deliberate console op. Same-uid re-provision stays allowed (resend invite). - * A failure after createUser leaves an unlinked Auth user — no compensation - * needed: the next run resolves it by email and adopts it (linkedUid null). */ + * A failure after createUser leaves an unlinked Auth user — no compensation needed FOR AN + * ADMIN: the next run resolves it by email and adopts it (linkedUid null). A delegate's retry + * is refused by the adoption guard below (`user !== null`), so a partial failure escalates + * that member to an Admin-only fix. Stated in the spec's operator notes too. */ export async function provisionMember( deps: ProvisionDeps, memberId: string, + /** Whether the CALLER holds the Admin role. A `create:MemberLogin` delegate does not, and + * is confined to the new-account path below — see the adoption guard. Defaults to false: + * a new call site must opt INTO the privileged path, never inherit it by omission. */ + callerHoldsAdminRole = false, ): Promise<{ email: string; actionLink: string }> { const member = await deps.getMember(memberId); if (member === null) throw new HttpsError("not-found", "member not found"); @@ -94,6 +177,78 @@ export async function provisionMember( ); } } + // ADOPTION GUARD — the boundary that makes create:MemberLogin delegable at all. + // + // Adoption is the branch where an Auth account already exists for this email and is not + // the one this member is linked to. For an Admin it is the documented recovery op. For a + // delegate it would be an account-takeover primitive, because NOTHING upstream ties + // members.email to the person: firestore.rules constrains totalPoints, uid, publicProfile, + // name, roleIds and positions on the create arm, never `email`, and no uniqueness check + // exists anywhere. So a create:Member + create:MemberLogin holder could file a member doc + // carrying a sitting Admin's email and reach the three writes below — adoptedClaims() + // stripping that Admin's claims, linkUid() binding the Admin's uid to the attacker's + // member doc through the admin SDK (the only path that can write members.uid at all), and + // passwordResetLink() handing back a reset link for the Admin's mailbox. + // + // A non-Admin therefore gets exactly ONE shape: mint a brand-new Auth account for a member + // that has none. Not "anything but adoption" — the RESEND path is just as dangerous and was + // the first draft's hole. `passwordResetLink` below is `generatePasswordResetLink`, which + // returns the oobCode URL TO THE CALLER; that is categorically different from + // `sendPasswordResetEmail`, which delivers the secret to the mailbox owner and is already + // unprivileged. So a delegate allowed to "resend" an invite for an ALREADY-LINKED member + // could name the president's memberId, take the returned link, set a password and sign in + // as them — no adoption involved, every existing guard satisfied. + // + // Hence both halves: no pre-existing account for this email (`user === null`) AND no + // pre-existing link (`linkedUid === null`). Costs the delegation nothing — a genuinely new + // member has neither — and leaves resend/adoption/self-heal to an Admin, plus the + // client-side sendPasswordResetEmail any member can already use on their own address. + if (!callerHoldsAdminRole && (user !== null || linkedUid !== null)) { + throw new HttpsError( + "permission-denied", + "this member already has a login; only an Admin can re-provision or link one", + { reason: "reprovision-requires-admin" }, + ); + } + // POWER-SEAT GUARD. The check above asks whether this is a NEW login; it does not ask whose + // member doc it is, and "unprovisioned" does not mean "enrolled by this delegate". Any + // uid-less member is reachable, including one an Admin already seated on an Admin-granting + // cargo — the normal state between being seated and being invited. + // + // Without this guard that is a clean escalation, and the delegate forges nothing: linkUid() + // below fires onMemberWritten, resolveTrustedGrants reads the STORED assignedBy (a genuine + // Admin), honors the grants, and mints Admin onto the uid this call just created. The + // attacker then reaches that uid through the invite — either the returned actionLink, or, + // if they also hold manage:Member, by rewriting members.email first (the rules do not pin + // it) so the ordinary reset mail lands in their own inbox. Suppressing the link alone + // therefore does NOT close it; the mint has to be refused at the source. + // + // Both halves of the claims-mint surface are checked, mirroring how syncMemberClaims splits + // it: hasDirectGrants() for the roleIds/permissionOverrides -> perms path, and the cargo + // read below for the grants -> roles path. Closing only one leaves the other reachable. + // + // Grant-free, un-granted members stay open: they mint nothing, so enrolling and inviting + // them is exactly the flow this delegation exists for. + if (!callerHoldsAdminRole) { + // Direct grants first — no read required, and it is the half a cargo check cannot see. + if (hasDirectGrants(member)) { + throw new HttpsError( + "permission-denied", + "this member has been granted roles or permissions; only an Admin can provision their login", + { reason: "granted-member-requires-admin" }, + ); + } + for (const cargoId of readCargoIds(member)) { + const grants = await deps.getPositionGrants(cargoId); + if (grants === null || grants.length > 0) { + throw new HttpsError( + "permission-denied", + "this member holds a cargo that confers permissions; only an Admin can provision their login", + { reason: "power-seat-requires-admin" }, + ); + } + } + } if (!user) user = await deps.createUser(email); const targetEmail = user.email ?? email; @@ -109,14 +264,27 @@ export async function provisionMember( nextClaims(user.uid === linkedUid ? existingClaims : adoptedClaims(existingClaims), "Member"), ); await deps.linkUid(memberId, user.uid); - const actionLink = await deps.passwordResetLink(targetEmail); + // The link is generatePasswordResetLink's oobCode URL — a bearer credential for this + // account. An Admin gets it as the manual fallback the invite drawer offers when the mail + // fails; a delegate does not need it (the client sends the reset mail itself through the + // unprivileged sendPasswordResetEmail) and must not hold it. Defence in depth behind the + // power-seat guard, not a substitute for it. + const actionLink = callerHoldsAdminRole ? await deps.passwordResetLink(targetEmail) : ""; return { email: targetEmail, actionLink } as const; } +// Delegable per docs/specs/board-seat-delegation.md: an Admin may hand `create:MemberLogin` +// to whoever is enrolling members, then revoke it. What the code gates is Auth account +// creation, uid linking and the initial claim write — NOT the invite email, which is a plain +// client-side sendPasswordResetEmail any signed-in user can already call. export const provisionMemberLogin = onCall(async (request) => { - requireAdmin(request); + requireAdminOrPerm(request, "create:MemberLogin"); const { memberId } = validateProvisionInput(request.data); ensureApp(); - return provisionMember(firestoreProvisionDeps(getFirestore(), getAuth()), memberId); + return provisionMember( + firestoreProvisionDeps(getFirestore(), getAuth()), + memberId, + callerIsAdmin(request), + ); }); diff --git a/apps/beacon/src/read-position-grants.test.ts b/apps/beacon/src/read-position-grants.test.ts new file mode 100644 index 00000000..3f0a5e64 --- /dev/null +++ b/apps/beacon/src/read-position-grants.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it } from "vitest"; +import type { Firestore } from "firebase-admin/firestore"; +import { readPositionGrants } from "./read-position-grants.js"; + +/** A Firestore stub narrow enough to drive `db.doc(path).get()`. The cast is test-only and + * justified: `readPositionGrants` touches exactly that one call chain. */ +function fakeDb(docs: Record | undefined>): Firestore { + return { + doc: (path: string) => ({ + get: async () => { + const data = docs[path]; + return { exists: data !== undefined, data: () => data }; + }, + }), + } as unknown as Firestore; +} + +describe("readPositionGrants", () => { + it("returns the valid roles from a well-formed cargo", async () => { + const db = fakeDb({ "positions/p1": { grants: ["Admin", "Membership"] } }); + await expect(readPositionGrants(db, "p1")).resolves.toEqual(["Admin", "Membership"]); + }); + + it("drops grant entries that are not valid roles", async () => { + const db = fakeDb({ "positions/p1": { grants: ["Admin", "NotARole", 42, null] } }); + await expect(readPositionGrants(db, "p1")).resolves.toEqual(["Admin"]); + }); + + it("returns [] for an absent or empty grants field", async () => { + const db = fakeDb({ "positions/p1": {}, "positions/p2": { grants: [] } }); + await expect(readPositionGrants(db, "p1")).resolves.toEqual([]); + await expect(readPositionGrants(db, "p2")).resolves.toEqual([]); + }); + + it("returns null for a missing doc and for an unsafe id", async () => { + const db = fakeDb({}); + await expect(readPositionGrants(db, "ghost")).resolves.toBeNull(); + await expect(readPositionGrants(db, "a/b")).resolves.toBeNull(); + await expect(readPositionGrants(db, "")).resolves.toBeNull(); + await expect(readPositionGrants(db, "__name__")).resolves.toBeNull(); + await expect(readPositionGrants(db, 42)).resolves.toBeNull(); + }); + + it("BLOCKING: returns null instead of THROWING on a non-array grants field", async () => { + // firestore.rules short-circuits every grants check on hasAnyRole(['Admin']) and never + // type-checks the field, so a console edit or a migration can store one of these. A + // TypeError here would be permanent per member: onMemberWritten is retry:false and the bad + // value persists in positions/, so every later write to any member seated on that cargo + // would re-throw and their claims would never sync again — silently. + const db = fakeDb({ + "positions/str": { grants: "Admin" }, + "positions/map": { grants: { 0: "Admin" } }, + "positions/num": { grants: 7 }, + "positions/bool": { grants: true }, + }); + for (const id of ["str", "map", "num", "bool"]) { + await expect(readPositionGrants(db, id)).resolves.toBeNull(); + } + }); +}); diff --git a/apps/beacon/src/read-position-grants.ts b/apps/beacon/src/read-position-grants.ts new file mode 100644 index 00000000..689d04bc --- /dev/null +++ b/apps/beacon/src/read-position-grants.ts @@ -0,0 +1,31 @@ +import type { Firestore } from "firebase-admin/firestore"; +import { isValidRole, type Role } from "@luminova/auth/roles"; +import { isSafeDocId } from "./firestore-util.js"; + +/** A cargo's trusted grants, or null when the id is unusable or the doc is missing. + * + * Two ports need exactly this — the claims-sync trust gate (`getPosition`) and the callable + * power-seat guard (`getPositionGrants`) — and they must not drift: both decide whether a + * cargo confers power, one before minting claims and one before creating a login. They + * differ only in their wrapper shape, so the read lives here and each adapter wraps it. + * + * `null` is the answer for an unusable id, a missing doc, or a malformed `grants` field, and + * it is fail-closed for BOTH callers: an unreadable cargo is treated as + * power-conferring by the guard and as grant-free by the trust gate, which is the safe + * direction in each. */ +export async function readPositionGrants(db: Firestore, id: unknown): Promise { + if (!isSafeDocId(id)) return null; + const snap = await db.doc(`positions/${id}`).get(); + if (!snap.exists) return null; + // A non-array `grants` returns null rather than throwing. firestore.rules short-circuits + // every grants check on hasAnyRole(['Admin']) and never type-checks the field, so a console + // edit or a migration can store a string or a map — and `.filter` on that is a TypeError. + // onMemberWritten is retry:false and the bad value PERSISTS, so the throw would kill claims + // sync for every member seated on that cargo, permanently and silently. null is fail-closed + // for both callers: the provisioning guard reads it as power-conferring, the trust gate as + // grant-free. + const raw = snap.data()?.grants; + if (raw !== undefined && raw !== null && !Array.isArray(raw)) return null; + const grants = (raw ?? []) as unknown[]; + return grants.filter((g): g is Role => isValidRole(g)); +} diff --git a/docs/plans/board-seat-delegation.md b/docs/plans/board-seat-delegation.md new file mode 100644 index 00000000..61207936 --- /dev/null +++ b/docs/plans/board-seat-delegation.md @@ -0,0 +1,527 @@ +# Implementation Plan — Board-seat + member-login delegation + +Branch: `feat/board-seat-delegation` + +## 0. Accepted decision + +The chapter owner has accepted that `update:BoardSeat` is a **claims-minting delegation**: a +delegate may seat a member on a cargo whose `grants` include `Admin`, and beacon's trust gate +will mint that claim. Deliberate; no guard is added against *that*. The delegation is meant to +be granted temporarily and revoked. + +The acceptance is explicitly premised on **revocability**. Three guards below exist to make that +premise true; without them it is false. They are not a narrowing of the accepted decision — a +delegate can still seat any vacant cargo, including `Presidente`. + +## 0b. Security guards (added after adversarial review — G1/G2/G3) + +Two independent reviews found three defects, all confirmed by reading code. Each guard below +removes a capability the owner did **not** ask for, and none removes one they did. + +### G1 — `provisionMemberLogin`: a non-Admin caller may only mint a NEW Auth account + +`provision-member-login.ts:82-114`. The relink refusal at `:85` is guarded by +`linkedUid !== null`, so it does not run for a member doc with no `uid`. Rules never constrain +`members.email` (`firestore.rules:428-435` pins `totalPoints`/`uid`/`publicProfile`/`name`/ +`roleIds`/`positions`, not `email`), and no uniqueness check exists. So a +`create:Member` + `create:MemberLogin` holder could create `members/evil` carrying a sitting +Admin's email, call the callable, and have it (a) adopt the Admin's Auth account, (b) strip its +claims via `adoptedClaims` at `:109`, (c) `linkUid` the Admin's uid onto their own doc through +the admin SDK at `:111`, and (d) return a **password-reset link for the Admin's email** at +`:112-114`. Full account takeover, unrelated to board seating. + +**Guard:** thread the caller's privilege into `provisionMember`. For a non-Admin caller require +`user === null || user.uid === linkedUid` **before** `:97` — a delegate may create a brand-new +Auth account or re-provision an already-linked one, never adopt a pre-existing account. Costs +the delegate nothing: a genuinely new member has no existing account. + +Note also: the invite **email** is not the privileged part. `requestPasswordReset` +(`apps/backstage/src/lib/auth/request-password-reset.ts`) is a plain client-side +`sendPasswordResetEmail` any signed-in user can already call. The callable's privilege is +account creation/adoption + uid linking + claim writing. + +### G2 — the trust gate must be non-reflexive on self-assignment + +`sync.ts:69` + `compute-roles.ts:8-12`. A delegate self-seats `Presidente` in one write on the +positions-only lane; beacon mints `roles: ["Admin","Member"]`. Revoking `update:BoardSeat` then +re-fires `onMemberWritten`, the gate reads their **live** claims, finds the `Admin` role the +cargo just minted, and re-honors the grants. The claim satisfies the gate that minted it, so the +delegation is permanent. `recomputeAllClaims` runs the same code and does not break the loop. + +**Guard:** when `assignedBy === member.uid`, trust **only** `assigner.perms.includes("update:BoardSeat")`, +never `assigner.roles.includes("Admin")`. Cargo-derived Admin can no longer bootstrap its own +trust. An Admin seating *someone else* is unaffected; an Admin self-seating still works via the +perm if they hold it, and via a second Admin otherwise. + +Test: assigner === target, `roles:["Admin"]`, `perms:[]` → grants dropped. + +### G3 — `currentCargoGrantsEmpty()` stays Admin-only + +`computeMemberRoles` derives the `roles` claim **exclusively** from cargo grants; directly +assigned `roleIds` feed `perms`, never `roles`. So bypassing `currentCargoGrantsEmpty()` lets a +delegate clear every Admin's cargo and strip them all. After the last one there is no Admin: +`setUserRoles` is `requireAdmin`, `roles/*` and `permissionOverrides` writes are Admin-only. +Unrecoverable outside the Firebase console. + +**Guard:** substitute `boardSeatDelegate()` only on the NEW-side conjunct. `positionsAssignmentSafe()` +becomes: + +``` +(boardSeatDelegate() || cargoAssignableByNonAdmin()) + && (hasAnyRole(['Admin']) || currentCargoGrantsEmpty()) +``` + +A delegate seats any **vacant** cargo but cannot displace a sitting power-cargo holder. Hand-over +stays an Admin action. `createPositionsSafe()` has no old side, so it takes the plain +substitution. + +If the owner later wants de-elevation delegated too, that is a separate code with its own +anti-lockout guard — not this one. + +## The two codes + +| Subject | Spanish label | Live code | Grants | +|---|---|---|---| +| `BoardSeat` | "Asientos de directiva" | `update:BoardSeat` | assign/clear ANY cargo — CEL category and power-granting alike — on the member CREATE and UPDATE lanes | +| `MemberLogin` | "Acceso de miembros" | `create:MemberLogin` | run `provisionMemberLogin`: link a Firebase Auth user and return the password-reset action link (the "Enviar acceso" invite email). Cargo-agnostic — applies to every new member, board seat or not. | + +Independent by construction: an Admin can grant emailing without board seating and vice versa. + +## Corrections to the original brief + +### C1 (critical). `canAssignPowerGrants` also governs the `/positions` CATALOG + +Five consumers, one of which is the catalog: + +- `member-invite-drawer.tsx:180` -> `MemberForm allowPowerGrants` (create lane) — in scope +- `member-drawer.tsx:153` -> `MemberForm allowPowerGrants` (update lane) — in scope +- `member-profile-page.tsx:148` -> `MemberForm allowPowerGrants` — in scope +- `member-profile-page.tsx:176` -> `MemberPositionsForm allowPowerGrants` — in scope +- `positions-page.tsx:178` -> `PositionForm canEditGrants` — **OUT of scope** + +`PositionForm.canEditGrants` unlocks the `grants` editor, the `category` select and the +board-cargo `title`/`titleFemale` fields (`position-form.tsx:88,134,157,179`) — exactly the +`/positions` arms that stay Admin-only. Widening the shared flag would render that editor for a +delegate whose write `firestore.rules` then rejects: the render-then-403 shape `use-can.ts`'s own +`canFeatureInitiatives` comment exists to prevent. + +**Resolution:** split the flag. + +- `canAssignBoardSeat` = `hasAnyRole(["Admin"]) || hasPerm("update:BoardSeat")` — the four member-lane call sites. +- `canEditCargoCatalog` = `hasAnyRole(["Admin"])` — `positions-page.tsx` only. +- `canAssignPowerGrants` is deleted. Both replacements are `Can` members, so every call site is compiler-guided. + +### C2. `PERMISSION_CAP` and `ALL_PERMISSION_CODES` — no test breaks + +- `SUBJECTS` 14 -> 16, `ALL_PERMISSION_CODES` 84 -> 96. `permission.test.ts:34` asserts + `ACTIONS.length * SUBJECTS.length`, self-adjusting. +- The 1000-byte claim test (`permission.test.ts:38`) keys on the longest code, still + `checkIn:MemberPoints` / `checkIn:Notification` (20 chars). `checkIn:MemberLogin` is 19, + `checkIn:BoardSeat` is 17. Worst case stays ~855 B. `PERMISSION_CAP` does not move. +- Product consequence worth one spec line: two more codes compete for the same 30-slot effective-perm + budget, and a member breaching the cap gets `perms: []` fail-closed (`sync.ts:106-118`) — silently + removing their `update:BoardSeat`. +- `sync.test.ts:580` `distinctCodes(n)` walks `ACTIONS x SUBJECTS` in order; adding subjects changes + which codes it picks, not their validity. No change. + +### C3. Two permission surfaces; neither needs a component change + +- `MATRIX_SUBJECTS` is a runtime `SUBJECTS.filter(...)` (`permission-matrix.ts:6`). Both new subjects + appear as checkbox rows in the `/permisos` role editor grid automatically. The only compiler-guided + edit is `SUBJECT_LABELS`. +- The per-member surface — `member-roles-panel.tsx`, Admin-only, on `/members/$memberId`, a MultiSelect + of chips — derives options from `ALL_PERMISSION_CODES` + `permissionLabel()` (`:17-21`), so it picks + the codes up for free. Without the `SUBJECT_LABELS` entries the chip would read "Crear MemberLogin" + (raw-subject fallback at `permission-matrix.ts:46`). + +### C4. `firestore.rules`: exactly TWO substitution sites + +`cargoAssignableByNonAdmin()` and `currentCargoGrantsEmpty()` contain no role check of their own — +they are pure cargo predicates. + +| Function | Current | After | +|---|---|---| +| `positionsAssignmentSafe()` | `hasAnyRole(['Admin']) \|\| (cargoAssignableByNonAdmin() && currentCargoGrantsEmpty())` | `boardSeatDelegate() \|\| (...unchanged...)` | +| `createPositionsSafe()` | `assignedBySelf() && (hasAnyRole(['Admin']) \|\| cargoAssignableByNonAdmin())` | `assignedBySelf() && (boardSeatDelegate() \|\| ...)` | + +Two further facts: + +- **`update:BoardSeat` on its own opens nothing.** Both functions are conjuncts inside an arm whose + entry condition is `canDo('update','Member')`, `canDo('update','Position') && hasOnly(['positions'])`, + or `canDo('create','Member')`. Rules tests must use a principal holding one of those, or they pass + for the wrong reason. +- **The delegate also gains de-elevation.** Bypassing `currentCargoGrantsEmpty()` means a delegate can + clear or replace an existing power cargo, not just assign one. Say it in the rules comment. + +Explicitly UNCHANGED (Admin-role-only): the `/positions` create arm's +`hasAnyRole(['Admin']) || (grants == [] && !boardSurfacingCategory())`; the `/positions` update arm's +`hasAnyRole(['Admin']) || (unchanged('grants') && unchanged('category') && ...)`; +`createPermissionAssignmentSafe()` / `updatePermissionAssignmentSafe()`; the members Admin takedown arm. + +### C5. `permsFromClaims` exists but is not reusable from `callable-auth.ts` + +`firestore-deps.ts:16` defines it module-local, not exported, and its contract differs: it validates +through `isValidPermissionCode` and returns `PermissionCode[] | undefined`, where `undefined` means "no +`perms` key" — which `getExistingClaims` uses for its claim diff. Importing it would pull +`firebase-admin/firestore`, `chunk`, `role-doc` and `resolve-member-perms` into the callable trust +boundary for a boolean membership test. + +Instead: extract the genuinely shared three lines — `stringArrayClaim(request, key)` — so `callerRoles` +and the new perms reader share one implementation. Satisfies guardrail #1 for the logic actually +duplicated, without coupling the trust boundary to the claims-sync Firestore port. + +### C6. `member-drawer.tsx` has NO provision affordance + +Complete list of `isAdmin`-gated provision affordances: + +| File | Line | Gate today | Move to | +|---|---|---|---| +| `member-invite-drawer.tsx` | 41, 43, 51, 56, 184 | `isAdmin` | `canProvisionLogin` | +| `member-row-menu.tsx` | 50 | `` | `` | +| `member-profile-page.tsx` | 132 | `` | `` | + +`members-page.tsx` owns the mutation and passes `onProvision` down unconditionally — no change. +`ActionGate` needs no change: `role` and `when` are ANDed and `role` is optional (`action-gate.tsx:21`). + +### C7. Only ONE test file implements `ClaimsSyncDeps` + +`apps/beacon/src/claims-sync/sync.test.ts` — the `fakeDeps` factory (`:43-85`), the `spied` +spread-override (`:154-160`), and the standalone rejecting deps (`:536-547`). + +### C8. `assignable-cargo.ts` — no change needed + +Fully parameterized on `allowPowerGrants` (`:41-115`). Widening the source of that boolean is the +entire change. Its doc comment needs one sentence ("Admin, or an `update:BoardSeat` delegate") so the +file stops claiming the branch is Admin-only. + +### C9. `BUILT_IN_ROLE_PERMS` — no change needed + +Neither code is seeded onto any built-in role. `role-seed.mjs` mirrors only `BUILT_IN_ROLE_PERMS` / +`ROLE_LABELS` / `ROLE_DESCRIPTIONS`, cross-checked by `role-definition.mirror.test.ts`. That table is +untouched. `seed-contract.test.ts` never enumerates `SUBJECTS`. + +--- + +## Slice 1 — Vocabulary, labels, spec + +1. `docs/specs/board-seat-delegation.md` (new) — the two codes; the accepted escalation decision; the + "`update:BoardSeat` alone opens nothing" dependency; the "revoking de-elevates on next write" note; + the out-of-scope list from C4; the cap note from C2. +2. `packages/types/src/permission.ts` — add `"BoardSeat"` and `"MemberLogin"` to `SUBJECTS`, before + `"all"`. Each gets a comment in the shape of the existing `Showcase` block: name the ONE live code, + state the gate is an exact `hasPerm`, state the sibling codes are inert *because* the gate is exact. +3. `packages/types/src/permission.test.ts` — subject in `SUBJECTS`; live code validates; inert siblings + validate too (the matrix renders the full grid and the role editor's write validation would reject an + assignable-but-unvalidatable code). +4. `apps/backstage/src/features/permissions/lib/permission-matrix.ts` — `SUBJECT_LABELS` gains + `BoardSeat: "Asientos de directiva"` and `MemberLogin: "Acceso de miembros"`. No `MATRIX_SUBJECTS` + edit (C3). + +Verify: `pnpm --filter @luminova/types run build && pnpm --filter @luminova/auth run build`, +`pnpm --filter @luminova/types run ci`, `pnpm --filter backstage run typecheck`. + +Commit: `feat(types): add BoardSeat and MemberLogin permission subjects` + +## Slice 2 — `firestore.rules` + rules tests + +1. `firestore.rules` — add above `cargoAssignableByNonAdmin()` (define-before-use is this file's + convention at that point): + + ``` + function boardSeatDelegate() { + return hasAnyRole(['Admin']) || hasPerm('update:BoardSeat'); + } + ``` + + Comment in the shape of `canCurateFeatured()`'s: Admin by ROLE (locked, undeactivatable); everyone + else by exact PERM so revoking the code revokes the authority, which a surviving role NAME would not; + `hasPerm` not `canDo` so `manage:all` cannot satisfy it and the sibling codes stay inert; and the + accepted decision that a delegate may seat a power-granting cargo, with beacon's trust gate widened + to match. + + Substitute at the two C4 sites. Extend the existing comments rather than replacing: the non-Admin + branch is unchanged, so a non-delegate is still held to + `cargoAssignableByNonAdmin() && currentCargoGrantsEmpty()` and the asymmetric grant-free-CEL takedown + survives; a delegate bypasses both conjuncts and therefore also gains replace/clear of a power cargo. + + Leave the `/positions` arms, the permission-assignment arms and the takedown arm alone. + +2. `tests/firestore-rules/rules.test.ts` — module-scoped principals beside `ORG_CHART`/`orgChart`: + + ```ts + const seatDelegate = () => as(SEAT_DELEGATE, [], ["update:Position", "update:BoardSeat"]); + const plainDelegate = () => as("seatonly-uid", [], ["update:BoardSeat"]); + ``` + + `update:Position` is load-bearing — without it the delegate never reaches an arm and every ALLOW + below would pass for the wrong reason. + + New fixtures in the once-only `beforeAll` seed: `members/m_delegate`, `members/m_delegate_power` + (seeded holding `pos1` in the current term), `members/m_delegate_cel`. + + Cases in `describe("firestore.rules — member positions assignment")`, before the terminal + "allows Admin to assign a power-conferring cargo" test: + - delegate assigns a grant-free CEL cargo (`pos_cel_free`) to `m_delegate_cel`, self-stamped — the + case `orgChart()` is denied twelve lines above. Pair them in the comment. + - delegate assigns a power-granting cargo (`pos1`) to `m_delegate`, self-stamped. + - delegate replaces the power cargo on `m_delegate_power` with `pos_soft` — the + `currentCargoGrantsEmpty()` bypass, i.e. de-elevation authority. + - `plainDelegate()` (has `update:BoardSeat`, lacks `update:Position`/`update:Member`) is denied any + positions write. Non-vacuity pin for the whole feature. + - delegate denied a forged `assignedBy` — `assignedBySelf()` is outside the substituted disjunction. + - delegate denied a non-current-term write — `positionsDelta().hasOnly([currentTermKey()])`. + - delegate denied any ride-along non-positions field on the `hasOnly(['positions'])` lane. + - regression pin: `orgChart()` still denied `pos1` and `pos_cel_free`, still allowed the grant-free + JDL `pos_soft`. Extend the existing comments to name the new disjunct. + - takedown pin: a non-delegate `update:Position` holder may still CLEAR a member off a grant-free CEL + seat. **No test today** — `pos_cel_free` appears only in assign-side assertions. Most important + regression guard in the slice, because the delegate bypass touches the same expression. + + Create lane in `describe("firestore.rules — members")`: a delegate holding + `create:Member` + `update:BoardSeat` may create a member born on `pos_cel_free` and on `pos1`, + self-stamped; the same principal without `update:BoardSeat` is still denied both. Mirrors the + existing `new_cel_free` / `new_cel_admin` pair. + + Catalog-UNCHANGED in `describe("firestore.rules — positions")`: a catalog delegate holding + `["create:Position","update:Position","update:BoardSeat"]` is still denied minting a CEL cargo and a + JDL dirección, still denied setting `grants`, still denied changing `category`, still denied + retitling a board cargo. Without the catalog codes these pass vacuously. + +Verify: `pnpm --filter @luminova/firestore-rules-tests run ci` + +Commit: `feat(rules): delegate board-seat assignment via update:BoardSeat` + +## Slice 3 — beacon claims-sync trust gate + +**Port shape.** Rename `getUserRoles(uid): Promise` to +`getAssignerClaims(uid): Promise<{ roles: Role[]; perms: PermissionCode[] }>`. + +Rejected alternatives: + +- *A second method `getUserPerms(uid)`* — two calls to answer one question is two chances for a future + edit to consult one and not the other, precisely where that must not happen (guardrail #1). Free in + production (`loadUser` memo) but not in the fakes, which would need two hand-consistent maps. +- *Reuse `getExistingClaims(uid)` for the assigner* — structurally smallest, but it would blunt the + sharpest test in `sync.test.ts`: the `spied` case at `:154` asserts `assignerLookups).toEqual([])` to + prove `resolveTrustedGrants` short-circuits on `grants.length === 0` **before** consulting the + assigner. Merged, the same spy would also record the target's own claim read, degrading the assertion + to a uid-filtering heuristic. Also `getExistingClaims` returns `perms?:` optional by design (absence + vs. empty is meaningful for its diff) — the wrong contract for a membership test. + +1. `apps/beacon/src/claims-sync/sync.ts` — port change; `resolveTrustedGrants` keeps the `isSafeDocId` + screen and the `grants.length === 0` early return exactly as they are, then: + + ```ts + const assigner = assignedBy ? await deps.getAssignerClaims(assignedBy) : null; + const trusted = + assigner !== null && + (assigner.roles.includes("Admin") || assigner.perms.includes("update:BoardSeat")); + return trusted ? [...new Set(position.grants)] : []; + ``` + + Extend the doc comment: name `update:BoardSeat` as the second trust source; state why (a delegate + stamps their own uid into `assignedBy` via `assignedBySelf()`, so without this the seat publishes on + the public Directiva and mints nothing — half-working, not safe); state the live-claims + re-evaluation now also covers perm revocation; cross-reference `boardSeatDelegate()` in + `firestore.rules`. One line on the cap interaction (C2). + +2. `apps/beacon/src/claims-sync/firestore-deps.ts` — rename the impl at `:287`, returning both, reusing + the two module-local readers already there. No new read: `loadUser` is the same per-instance memo. + +3. `apps/beacon/src/claims-sync/sync.test.ts` — update the three port literals (C7). `fakeDeps` gains a + `userPerms` option map. New cases: + - honors power grants when the assigner holds `update:BoardSeat` and NOT the Admin role — mirror of + the existing "honors power grants when assignedBy is Admin" at `:109`. + - a `manage:all` perm holder who is neither Admin-by-role nor an `update:BoardSeat` holder does NOT + satisfy the gate — the exact-code property, asserted server-side too. + - revocation — same fixture with the assigner's perms emptied; the target recomputes down to a plain + `Member` claim. + - the existing `:129` positive-and-inert spy case: update the override, keep both assertions verbatim. + +Verify: `pnpm --filter beacon exec vitest run src/claims-sync/sync.test.ts`, then +`pnpm --filter beacon run ci`. + +Commit: `feat(beacon): honor cargo grants from an update:BoardSeat assigner` + +## Slice 4 — beacon callable auth + +1. `apps/beacon/src/callable-auth.ts` — extract the shared reader (C5), then: + + ```ts + export function requireAdminOrPerm(request: CallableRequest, code: PermissionCode): void { + if (!request.auth) throw new HttpsError("unauthenticated", "sign-in required"); + if (callerRoles(request).includes("Admin")) return; + if (stringArrayClaim(request, "perms").includes(code)) return; + throw new HttpsError("permission-denied", `Admin role or ${code} required`); + } + ``` + + Comment: exact-code match mirroring the rules' `hasPerm()` and `use-can`'s `hasPerm` — deliberately + NOT a `canDo`-style expansion, so `manage:all` does not satisfy it. `requireAdmin` keeps its exact + current behaviour and message. + +2. `apps/beacon/src/callable-auth.test.ts` (new) — this trust boundary is unasserted today. + `requireAdmin`: unauthenticated / non-Admin / Admin. `requireAdminOrPerm`: unauthenticated; Admin with + no `perms` claim at all; `{roles:["Member"], perms:["create:MemberLogin"]}` passes; + `{roles:["Member"], perms:["manage:all"]}` **throws** (wildcard-must-not-satisfy); + `{roles:["Member"], perms:["update:BoardSeat"]}` asked for `create:MemberLogin` throws (independence); + a non-array `perms` claim throws (fail-closed on a malformed token). + +3. `apps/beacon/src/provision-member-login.ts` — `requireAdmin(request)` -> + `requireAdminOrPerm(request, "create:MemberLogin")` at `:118`. Comment: the callable links an Auth + account and returns a password-reset action link, delegable per the owner decision; the `adoptedClaims` + de-elevation and the different-uid refusal are unchanged and still bind on every caller. + + Do not touch `seed-roles.ts`, `recompute-claims.ts:22,65,267`, `set-user-roles.ts:86`. + +Verify: `pnpm --filter beacon run ci` + +Commit: `feat(beacon): allow create:MemberLogin to call provisionMemberLogin` + +## Slice 5 — backstage gate flags (the C1 split) + +1. `apps/backstage/src/lib/authz/use-can.ts` — delete `canAssignPowerGrants`; add + `canAssignBoardSeat`, `canEditCargoCatalog`, `canProvisionLogin` with doc comments. Both perm-based + flags use `hasPerm` from `@luminova/auth/roles`, never `abilityAllows` — one line referencing the + existing `canFeatureInitiatives` comment. `canEditCargoCatalog`'s comment must state that this is what + the seat delegation deliberately does NOT widen, or the next person will "unify" the two flags and + reopen C1. +2. `apps/backstage/src/lib/authz/use-can.test.ts` — mirror the six existing `canFeatureInitiatives` cases + per flag, plus `canEditCargoCatalog === false` for an `update:BoardSeat` holder (the C1 pin) and + `manage:all -> false` for all three. +3. `positions-page.tsx` — `canAssignPowerGrants` -> `canEditCargoCatalog` (`:37`, `:178`). +4. `member-invite-drawer.tsx` — -> `canAssignBoardSeat` (`:41`, `:180`). +5. `member-drawer.tsx` — same (`:139`, `:153`). +6. `member-profile-page.tsx` — `gate.canAssignPowerGrants` -> `gate.canAssignBoardSeat` (`:148`, `:176`). +7. `assignable-cargo.ts` — no logic change (C8); one sentence added to the `allowPowerGrants` doc + comments and the two form prop JSDocs: the branch is "Admin, or an `update:BoardSeat` delegate". + +Verify: `pnpm --filter backstage run ci` + +Commit: `feat(backstage): split the seat, catalog and login-provision gates` + +## Slice 6 — provision affordances move to `canProvisionLogin` + +1. `member-invite-drawer.tsx` — `canProvisionLogin` drives `useState` (`:43`), the open-resync + `useEffect` (`:50-52`), `reset()` (`:56`) and the checkbox render guard (`:184`). **Keep the + `useEffect`** — its reason (the drawer mounts before the token's claims decode; the store emits empty + claims first and re-emits) applies harder to a perms-derived flag, since `perms` is minted by + claims-sync and arrives in the same late token. Rewrite the comment to say "Admin-role or + `create:MemberLogin`". +2. `member-invite-drawer.test.tsx` — parameterize the claims wrapper (`:10-18`); add: a delegate + `{roles:["Member"], perms:["create:Member","create:MemberLogin"]}` sees the checkbox, defaults it ON, + reaches `onProvision`; a `create:Member`-only creator does not see it and `onProvision` is never + called; a `manage:all` holder does not see it. +3. `member-row-menu.tsx` — `` at `:50` -> `when={canProvisionLogin}`; hoist + the `useCan()` call to the component body (hook rules). +4. `member-row-menu.test.tsx` — add a delegate-sees-it case and a `manage:all`-does-not case. +5. `member-profile-page.tsx` — `` at `:131-133` -> `when={gate.canProvisionLogin}`. + **Do not touch** the `` at `:203` around `` — `roleIds` / + `permissionOverrides` writes stay Admin-only, and that is the panel where the delegation is granted. + +Verify: `pnpm --filter backstage run ci` + +Commit: `feat(backstage): gate the invite affordances on create:MemberLogin` + +## Slice 7 — honest empty state for the Cargo combobox + +One shared place: both forms render the same `` + `Combobox options={cargoOptions}` +shape (`member-form.tsx:230-264`, `member-positions-form.tsx:76-104`). Their `locked` / `takedownOnly` +notes legitimately differ in wording, so extract only the new note. + +1. `apps/backstage/src/features/members/components/no-assignable-cargos-note.tsx` (new) — props-free, + in the established `role="note" className="text-ui-xs text-ink-3"` pattern. The quoted permission + name must equal `permissionLabel("update:BoardSeat")` = "Editar Asientos de directiva"; one comment + line says so, since the strings live in different features and nothing enforces the match. +2. `member-form.tsx` — render when `!positionsLocked && !allowPowerGrants && cargoOptions.length === 0`, + in the note stack after `cargoTakedown` (`:306`). +3. `member-positions-form.tsx` — same condition, after the `takedownOnly` note (`:130`). +4. `member-form.test.tsx` — with `allowPowerGrants={false}` and a CEL/power-only positions list, the note + renders and the combobox has no selectable option; with `allowPowerGrants` the note is absent and the + CEL option is present. +5. `member-positions-form.test.tsx` — the same pair, plus: when `locked` is true the locked note renders + and the empty note does not (mutually exclusive today because `cargoOptionsForEditor` appends the held + cargo disabled — pin it so a future change cannot produce two notes). +6. `assignable-cargo.ts` — no code change; one sentence noting an empty return for a non-delegate is a + real, expected state the forms explain. + +Residual, stated in the PR body and deliberately not fixed: an Admin/delegate facing a genuinely empty +catalog still sees the bare "Sin resultados" from `packages/ui/src/components/combobox.tsx:28`. That is +an empty-catalog problem, not a permissions problem; giving it the delegation copy would be a lie. + +Verify: `pnpm --filter backstage run ci` + +Commit: `feat(backstage): explain an empty cargo list to a non-delegate` + +## Slice 8 — docs, route, review, PR + +`docs/specs/position-assignment-lane.md` gains a cross-reference (its "who may assign" narrative is now +one disjunct out of date). `packages/auth/CLAUDE.md`'s Gotchas gains one line naming `BoardSeat` / +`MemberLogin` as hand-granted codes deliberately absent from `BUILT_IN_ROLE_PERMS`. + +Then, per the binding review-routing contract: + +```bash +.claude/hooks/route.sh +pnpm pr-tests +``` + +Run whatever the router prints, stamp with the exact command it emits (trailer in the final paragraph), +mirror the token list under `## Reviews` in the PR body. + +## Fact-check corrections (applied — cite these, not the originals) + +| Plan said | Actually | +|---|---| +| `sync.ts:106-118` cap block | `sync.ts:111-122` | +| "six existing `canFeatureInitiatives` cases" | five, at `use-can.test.ts:44,54,58,64,72` | +| `member-profile-page.tsx:203` roles-panel gate | `:202-206` | +| "define-before-use is this file's convention" | false — `cargoAssignableByNonAdmin()` calls `nonAdminAssignable()` defined *after* it (`firestore.rules:169-175`). Place `boardSeatDelegate()` next to `canCurateFeatured()` (`:323`) instead | +| "worst case ~855 B" | 830 B; longest code unchanged at 20 chars, so the byte count does not move at all | +| insert note after `member-form.tsx:306` | after `:312` (`:306-312` is the whole `cargoTakedown` block). `member-positions-form.tsx:130` is correct as written | +| "`callable-auth` is unasserted today" | no *direct* unit test; indirect coverage at `reseed-role-perms.emulator.test.ts:54` | +| C5: importing `permsFromClaims` pulls in `firebase-admin/firestore` | those imports are **type-only**; the real runtime pull-in is `../chunk.js`, `../firestore-util.js`, `./role-doc.js` (`firestore-deps.ts:3-8`). Argument stands, evidence did not | +| "`members-page.tsx` owns the mutation" | two owners — also a local `InviteAccess` in `member-profile-page.tsx:224-225` | +| "hoist the `useCan()` call" in `member-row-menu.tsx` | nothing to hoist; the component calls no hook. Add the import and the call | +| C7 rejecting-deps literal `:536-547` | `:537-548`. Also unstated: `fakeDeps`' **opts parameter type at `:34-41`** needs the `userPerms` map | +| C8 "fully parameterized (`:41-115`)" | file is 116 lines; `positionsLockedForNonAdmin` (`:41-45`) takes only the cargo — the flag is applied externally at `member-form.tsx:121` / `member-positions-form.tsx:52`. "No change needed" still holds | +| C4 quotes `createPositionsSafe()` | omits the leading `!('positions' in request.resource.data) ||` disjunct (`firestore.rules:224-227`) | +| `member-form.tsx:230-264` Cargo Field | `:230-265` | +| "denied twelve lines above" | ~70 — denial at `rules.test.ts:2941`, insertion point `:3012`, describe runs `:2741-3019` | + +**Resolved open question (was Risk 5): token refresh does NOT happen.** `claims.ts` is a pure +14-line decoder; the token comes from `auth-store.ts:54` calling `getIdTokenResult()` with no +`forceRefresh`. A newly granted `update:BoardSeat` / `create:MemberLogin` is invisible to +`firestore.rules`, `requireAdminOrPerm` and `useCan` until the hourly refresh or a re-login. +**Operator note for the spec: after granting or revoking either code, the delegate must sign out +and back in.** Not fixed here — a force-refresh on every load costs a network round trip on the +critical path. + +## Risks and open questions + +1. **C1 is the review's centre of gravity.** A single widened `canAssignPowerGrants` silently delegates + the `/positions` catalog editor. Verify `positions-page.tsx` reads `canEditCargoCatalog` and that + `use-can.test.ts` pins `canEditCargoCatalog === false` for an `update:BoardSeat` holder. +2. **`update:BoardSeat` grants nothing alone, and the UI does not say so.** An Admin ticking only that + box produces a delegate who can still do nothing — they also need `update:Position` or + `update:Member`. Neither surface hints at the dependency. Documented in the spec; not otherwise fixed. +3. **Revocation is retroactive and silent.** Removing the perm de-elevates the people they seated on the + *next write* to each member doc — possibly never. Inherited from the existing Admin behaviour, but + "revoke the delegation" is not "undo what they did". No operator sweep short of `recomputeAllClaims`. +4. **The cap can silently revoke the delegation.** A delegate exceeding `PERMISSION_CAP = 30` is written + `perms: []` fail-closed, taking `update:BoardSeat` with it. +5. **Token freshness on grant.** Rules, `requireAdminOrPerm` and `useCan` all read `perms` off the ID + token. A newly granted code does not take effect until the token refreshes. Verify whether + `apps/backstage/src/lib/authz/claims.ts` force-refreshes; if not, the delegate sees "no access" for up + to an hour with no explanation. Not addressed here. +6. **`stringArrayClaim` vs. reusing `permsFromClaims` (C5).** A reviewer applying guardrail #1 + mechanically will call the new reader a copy. Counter-argument is in C5; cheap to switch to a shared + `apps/beacon/src/claims-read.ts` if a reviewer insists. +7. **Rules-test non-vacuity.** Three new rules tests pass for the wrong reason if their principal is + under-permissioned. Check each `as(uid, [], [...])` literal by hand. +8. **The grant-free-CEL takedown has no test today.** Slice 2 adds it at the same time as the change to + the expression it depends on, so it proves post-change behaviour, not preservation. Stronger evidence + would land that test on `main` first. +9. **No end-to-end cross-product test.** `use-can.test.ts` and `callable-auth.test.ts` each pin + independence at their own layer; nothing tests a `create:MemberLogin`-only holder seeing the invite + button and an empty cargo list simultaneously. Deliberate omission. diff --git a/docs/specs/board-seat-delegation.md b/docs/specs/board-seat-delegation.md new file mode 100644 index 00000000..9597dace --- /dev/null +++ b/docs/specs/board-seat-delegation.md @@ -0,0 +1,175 @@ +# Board-seat and member-login delegation + +Two explicitly grantable permission codes that let an Admin **temporarily** delegate work that +was previously hardcoded to the `Admin` role, then revoke it. Both are granted per member in the +`/members/$memberId` overrides panel (Admin-only) or per role in the `/permisos` matrix. + +## The two codes + +| Subject | Label in the matrix | Live code | Confers | +|---|---|---|---| +| `BoardSeat` | Asientos de directiva | `update:BoardSeat` | seat a member on **any vacant cargo** — CEL category and power-granting alike | +| `MemberLogin` | Acceso de miembros | `create:MemberLogin` | call `provisionMemberLogin` for a member who has **no login yet**: create their Auth account, link their uid, return the password-reset link | + +They are independent by construction. An Admin can grant emailing without board seating and vice +versa. The other five codes each subject generates (`manage:BoardSeat`, `read:MemberLogin`, …) +are inert: every gate is an exact `hasPerm` code test, never a `canDo` expansion, so `manage:all` +does not satisfy either one. + +## What `update:BoardSeat` does NOT do + +- **It confers nothing on its own.** It only widens the cargo set for an editor who *already* + holds `update:Member`, `create:Member`, or `update:Position`. A member holding only + `update:BoardSeat` still cannot write anything. Granting it alone is a no-op; the UI does not + say so. +- **It does not confer Admin, and it does not confer anything on the delegate themselves.** + See the section below: an Admin-granting cargo mints nothing unless the assigner holds the + Admin role, and a self-assignment mints nothing unless the assigner is an Admin. +- **It does not unseat anyone.** `currentCargoGrantsEmpty()` stays Admin-only, so a delegate + cannot displace a member sitting on a power-granting cargo. Hand-over is an Admin action. + One caveat, pre-existing and pinned by a rules test: that predicate reads only the CURRENT + term, so in the window after a UTC-year rollover an Admin's next-term slot is empty and any + `update:Position` holder can write into it. + Rationale: the `roles` claim is derived *exclusively* from cargo grants + (`compute-roles.ts`), so clearing every Admin's cargo would strip every Admin claim in the + chapter — and `setUserRoles`, `roles/*` writes and `permissionOverrides` writes are all + Admin-only, making that state unrecoverable outside the Firebase console. +- **It DOES raise the publication ceiling to CEL.** A plain `update:Position` holder can seat + only grant-free non-CEL cargos; adding this code lets them seat a CEL cargo, so "Presidente" + at public board rank 0 is no longer an Admin-only decision for a delegate. Publication and + authority are separate ceilings — the claims half still refuses to mint Admin. +- **It does not reach the `/positions` catalog.** Creating a CEL or JDL cargo, and editing any + cargo's `grants`, `category` or board `title`/`titleFemale`, stay Admin-only + (`boardSurfacingCategory()` and the update-arm pins). The delegate seats members on cargos + that already exist. +- **It does not touch `roleIds` or `permissionOverrides`.** `createPermissionAssignmentSafe()` / + `updatePermissionAssignmentSafe()` are unchanged, so a delegate cannot re-grant the delegation + to themselves or anyone else. + +## Conferring Admin stays Admin-only + +The chapter owner accepted that a delegate may seat power-granting cargos, on the explicit +premise that the delegation is **revocable**. Delivering that premise turned out to require one +rule, and it is the load-bearing guard of the whole feature: + +**A cargo whose `grants` include `Admin` is honored only when the assigner holds the Admin +ROLE**, and **a SELF-assignment is honored only for an Admin, whatever the cargo grants.** +Everything else — a delegate seating *someone else* on a non-Admin power cargo — is honored for +an `update:BoardSeat` holder, and that is the feature. + +The self-assignment half closes a separate, one-write hole: without it `update:BoardSeat` is a +self-service grant of every built-in role but Admin. The recommended pairing below +(`update:Position` + `update:BoardSeat`) can write a Secretario or ProjectManager cargo onto its +own member doc through the positions-only lane and be minted those roles. Conferring power on +others is the delegation; conferring it on yourself is self-promotion. A delegate seating +themselves still *publishes* the seat — it just confers nothing. + +Why, precisely: a minted Admin is itself a trust source, so a delegate who can mint one has +made the delegation permanent. Revoking their code de-elevates nobody, and neither +`recomputeAllClaims` nor any in-app path recovers it. + +Two earlier forms of this guard were wrong and are recorded so they are not re-proposed: + +- **Blocking only self-assignment** (`assignedBy === member.uid`) stops the one-write self-loop + and misses the two-write puppet loop: the delegate creates a second member on a mailbox they + control, seats *it* on Presidente — not a self-assignment, so the perm is trusted — and that + puppet is Admin forever. +- Worse, that same form **strips the sitting president**. `seed-president.mjs` stamps + `assignedBy` with the president's own uid, and an Admin's perms are `manage:all`, never the + exact `update:BoardSeat` code — so the president's own Admin claim would be dropped on the + next write to their member doc. Confirmed against the live production member doc before it + shipped, and pinned by a regression test. + +**The cost, so it is not read as a bug:** a delegate seating someone on an Admin-granting cargo +publishes the seat but mints no claim. An Admin must re-stamp it. Everything a delegate *can* +confer is genuinely revocable — strip the perm and the next write to that member drops the +grants. + +## `create:MemberLogin` — what is actually privileged + +The invite **email** is not. `apps/backstage/src/lib/auth/request-password-reset.ts` is a plain +client-side `sendPasswordResetEmail` that any signed-in user can already call. What the callable +owns is Auth account creation, uid linking (the only path that can write `members.uid` at all, +since the rules forbid it on every client lane) and the initial claim write. + +**A non-Admin caller may provision a member only when that member has NO login yet** — no +existing Auth account for their email, and no stored `uid`. Adoption, re-provision/resend, and +the deleted-account self-heal are all Admin-only. + +**And only when that member carries no grants of either kind.** `syncMemberClaims` mints from +two independent sources, so the guard has to ask about both: + +- *Direct grants* — `roleIds` and `permissionOverrides` become `perms`, with no cargo involved. + These are exactly what the Admin-only roles panel writes, so "granted but not yet invited" is + as ordinary a state as "seated but not yet invited". +- *Cargo grants* — a cargo's `grants` become `roles`. The guard checks EVERY term in the map, + not just the current one: `syncMemberClaims` reads the current term at trigger time, so a + future-term entry is invisible today and mints on the UTC-year rollover. + +**And only when that member is not POWER-SEATED.** "Unprovisioned" does not mean "enrolled by +this delegate": every uid-less member is reachable by `memberId`, including one an Admin already +seated on an Admin-granting cargo — the normal state between being seated and being invited. +Linking a uid fires `onMemberWritten`, and `resolveTrustedGrants` reads the *stored* `assignedBy` +— a genuine Admin — so the grants are honored and Admin is minted onto the account this call +just created. The delegate forges nothing. So a non-Admin provision is refused whenever the +member's current-term cargo carries any grants, and fails closed when that cargo cannot be read. + +**A non-Admin caller never receives the `actionLink`.** `generatePasswordResetLink` returns a +bearer credential for the account; the client sends the invite itself through the unprivileged +`sendPasswordResetEmail`, so a delegate has no need to hold it. This is defence in depth behind +the power-seat guard, not a substitute for it — with `manage:Member` an attacker can rewrite +`members.email` first (the rules do not pin it) and receive the ordinary reset mail in their own +inbox, which suppressing the link alone would not stop. + +The resend path is restricted for the same reason as adoption, and it is the less obvious one: +`passwordResetLink` is `generatePasswordResetLink`, which returns the oobCode URL **to the +caller** — categorically different from `sendPasswordResetEmail`, which delivers the secret to +the mailbox owner. So a delegate permitted to "resend" could pass the president's `memberId`, +receive a live reset link for their address, set a password and sign in as them, with every +other guard satisfied. Adoption is worse still: `members.email` is unconstrained by the rules +and has no uniqueness check, so a delegate could file a member doc carrying an Admin's email and +have the callable adopt that account, strip its claims and link its uid onto their own doc. + +This costs the delegation nothing: a genuinely new member has neither an account nor a uid. + +## Operator notes + +1. **Grant `update:BoardSeat` together with a member-editing capability.** On its own it does + nothing. The usual pairing is the `Membresía` role (which carries `manage:Member`) plus the + `update:BoardSeat` override; an org-chart-only delegate wants `update:Position` instead. +2. **The delegate must sign out and back in after being granted or revoked.** All three gates — + `firestore.rules`, `requireAdminOrPerm` and `useCan` — read the `perms` claim off the ID + token, and `auth-store.ts` calls `getIdTokenResult()` without `forceRefresh`. A freshly + granted code is invisible for up to an hour otherwise, and a freshly revoked one keeps + working for up to an hour. +3. **A delegate's failed provision escalates to an Admin.** If `provisionMemberLogin` fails + after creating the Auth account (a transient `setClaims`/`linkUid` error), the retry is + refused by the adoption guard — the account now exists, so `user !== null`. Finish it from + an Admin account. +4. **Revocation is not an undo.** Removing the code stops future seating. Members already seated + keep their cargo, and their cargo-derived claims are recomputed on the *next write* to their + member doc — which may be much later. To force it, re-write the member docs or run + `recomputeAllClaims`. +5. **The `PERMISSION_CAP` interaction.** A member whose resolved perms exceed 30 is written + `perms: []` fail-closed, which silently takes `update:BoardSeat` with it. Two more subjects + in the vocabulary make the 30-slot budget marginally tighter. +6. **Verify who you seat.** One residual no guard can close, and it is not specific to this + delegation — it is why enrolment and seating should not both be delegated blindly. A member + creator controls the `email` on the doc they file, and the invite goes to that address. So if + an Admin later seats a FABRICATED member on an Admin-granting cargo, the claim is minted onto + an account the fabricator controls. The chain needs an Admin to seat someone they did not + verify; it existed before this feature (an Admin provisioning the same fabricated doc sends + the invite to the same attacker address), and the power-seat guard means a delegate cannot + complete it alone. Treat the members list as the thing you verify before seating. +7. **There is a consistency window.** `firestore.rules` reads the token while beacon reads stored + claims. If a delegate's own perms are dropped (cap breach, or revocation) their cached token + still passes the rules for up to an hour, so a seat write can succeed while + `resolveTrustedGrants` declines to mint the grants. The member is then published on the public + Directiva with no claim — visible, powerless. Re-running the write after the token refreshes + resolves it. + +## Out of scope + +Creating or editing cargo docs; `roleIds` / `permissionOverrides` assignment; unseating a sitting +power-cargo holder; delegating `setUserRoles`, `seedRoles`, `recomputeAllClaims` or +`reseedBuiltInRolePerms`. All stay Admin-role-only. diff --git a/firestore.rules b/firestore.rules index af78c354..c6f13eae 100644 --- a/firestore.rules +++ b/firestore.rules @@ -203,11 +203,29 @@ service cloud.firestore { // also closes a ride-along: a non-Admin can no longer sneak a power cargo + forged // assignedBy under a different term key in the same write. Comisión power grants are // not loop-checkable here — the beacon claims-sync trust gate is their backstop. + // The two cargo conjuncts are gated SEPARATELY, and the asymmetry is the whole point. + // + // NEW side (cargoAssignableByNonAdmin, the cargo being written IN) — delegable. + // boardSeatDelegate() lifts it, so an update:BoardSeat holder may seat a CEL or + // power-granting cargo. That is the feature. + // + // OLD side (currentCargoGrantsEmpty, the cargo being REPLACED) — Admin role ONLY. + // NOT delegated, deliberately. computeMemberRoles derives the `roles` claim + // EXCLUSIVELY from cargo grants — directly-assigned roleIds feed `perms`, never + // `roles` — so a principal who can overwrite a sitting Admin's cargo can strip + // every Admin claim in the chapter one write at a time. After the last one there is + // no Admin: setUserRoles is requireAdmin, and roles/* plus permissionOverrides + // writes are all hasAnyRole(['Admin']). The chapter would be unrecoverable outside + // the Firebase console. A delegate therefore seats VACANT and grant-free cargos and + // cannot displace a power-cargo holder; hand-over stays an Admin action. + // + // A non-delegate is unchanged: both conjuncts still bind, so the deliberately + // asymmetric grant-free-CEL takedown below still works for an update:Position holder. function positionsAssignmentSafe() { return positionsDelta().hasOnly([currentTermKey()]) && assignedBySelf() - && (hasAnyRole(['Admin']) - || (cargoAssignableByNonAdmin() && currentCargoGrantsEmpty())); + && (boardSeatDelegate() || cargoAssignableByNonAdmin()) + && (hasAnyRole(['Admin']) || currentCargoGrantsEmpty()); } // Create has no prior resource to diff, so it can't use positionsDelta(); it applies the // same self-stamp + Admin-only-cargo gate to any positions it writes. Without this a @@ -221,9 +239,28 @@ service cloud.firestore { // Admin provisionMemberLogin then writes the uid, after which projectBoard publishes an // attacker-composed account at board rank 0 as Presidente. currentCargoGrantsEmpty() is // the only half a create cannot ask (no prior resource), and it has no old side to guard. + // Plain substitution on the cargo conjunct, unlike positionsAssignmentSafe(): a create has + // no prior resource, so there is no old side to keep Admin-only — currentCargoGrantsEmpty() + // has never applied here, and a member born on a cargo displaces nobody. + // + // The current-term restriction, though, is NOT optional and was missing: assignedBySelf() + // and cargoAssignableByNonAdmin() both read ONLY positions[currentTermKey()] (via + // assignedTerm()), so every OTHER term key in a created map was entirely unvalidated. The + // update arm closes this — see positionsDelta().hasOnly() and its comment — and the create + // arm has to say the same thing or the ride-along just moves to birth: + // positions: { "": , + // "": { cargoId: , assignedBy: } } + // Nothing inspects the second entry. On the UTC-year rollover claims-sync reads it, sees a + // genuine Admin as assignedBy, and mints Admin onto a member whose email — and therefore + // whose login — the creator controls. Forged attribution, one term deferred; the trust gate + // cannot help, because the assigner really is an Admin. + // Symmetric with the update arm: Admin is already current-term-only there, so this adds no + // Admin restriction that did not already exist on the other lane. function createPositionsSafe() { return !('positions' in request.resource.data) - || (assignedBySelf() && (hasAnyRole(['Admin']) || cargoAssignableByNonAdmin())); + || (request.resource.data.get('positions', {}).keys().hasOnly([currentTermKey()]) + && assignedBySelf() + && (boardSeatDelegate() || cargoAssignableByNonAdmin())); } // roleIds + permissionOverrides feed Auth custom claims via the beacon trigger. @@ -323,6 +360,27 @@ service cloud.firestore { function canCurateFeatured() { return hasAnyRole(['Admin']) || hasPerm('update:Showcase'); } + // Who may seat a member on a cargo the non-Admin lane otherwise refuses — a + // power-granting one, or a CEL one. Split the same way as canCurateFeatured(), for the + // same two reasons: Admin by ROLE (locked and undeactivatable, so its name carries none + // of the staleness this fixes), everyone else by the exact PERM, so revoking the code + // revokes the authority where a surviving role NAME in the claim would not. + // + // hasPerm, deliberately NOT canDo: manage:all must not answer this, and the other five + // *:BoardSeat codes the cross-product generates stay inert BECAUSE the gate is exact. + // + // What this rule does and does NOT buy, per docs/specs/board-seat-delegation.md: it lifts + // the cargo conjunct, so a delegate may SEAT anyone on a CEL or power-granting cargo and + // the seat publishes on the world-readable Directiva. It does not follow that the cargo's + // grants are MINTED — beacon's resolveTrustedGrants refuses an Admin-conferring cargo + // from a delegate outright, and refuses any granting cargo on a SELF-assignment. So a + // delegate seating themselves publishes and confers nothing, and seating someone else + // confers everything but Admin. Do not restate the mint rule here; read it there. + // + // Deliberately NOT widened to currentCargoGrantsEmpty(): see positionsAssignmentSafe(). + function boardSeatDelegate() { + return hasAnyRole(['Admin']) || hasPerm('update:BoardSeat'); + } function initiativeCreateAllowed(subject) { return canDo('create', subject) && request.resource.data.get('directionUids', []) == [] @@ -468,17 +526,23 @@ service cloud.firestore { && softDeleteSafe(); // Positions-only lane: an org-chart editor who is NOT a member editor. Keyed on // update:Position — the same capability that governs the positions CATALOG — and - // confined to the positions map. The power-cargo restriction is NOT relaxed: - // positionsAssignmentSafe()'s non-Admin branch still demands - // cargoAssignableByNonAdmin() && currentCargoGrantsEmpty(), so this principal assigns - // and clears grant-free cargos only, on BOTH sides of a swap. + // confined to the positions map. A PLAIN update:Position holder is still held to + // cargoAssignableByNonAdmin() && currentCargoGrantsEmpty(), so they assign and clear + // grant-free non-CEL cargos only, on BOTH sides of a swap. Adding update:BoardSeat lifts + // the NEW side only: that principal may seat a CEL or power-granting cargo, but still + // cannot displace a sitting power-cargo holder (the old side stays Admin-role-only). // - // This lane IS a public publication authority, deliberately, and its ceiling is JDL: - // grant-free JDL direcciones are board cargos, so its holder can put a member — - // including themselves — on the world-readable Directiva under a dirección. No claim - // is minted (claims-sync returns early on grants.length == 0). Accepted, and pinned by - // a rules test. CEL is NOT in that grant — cargoAssignableByNonAdmin() refuses it - // whatever its grants, so 'Presidente' at public rank 0 stays an Admin decision. + // This lane IS a public publication authority, deliberately. For a PLAIN update:Position + // holder its ceiling is JDL: grant-free JDL direcciones are board cargos, so they can put + // a member — including themselves — on the world-readable Directiva under a dirección, + // minting nothing (claims-sync returns early on grants.length == 0). Accepted, pinned by + // a rules test, and CEL stays out of reach — cargoAssignableByNonAdmin() refuses it + // whatever its grants. + // Adding update:BoardSeat RAISES that ceiling to CEL, so 'Presidente' at public rank 0 is + // no longer an Admin-only decision for a delegate. The claims half is what still holds: + // an Admin-conferring cargo mints nothing from a delegate, and neither does a + // self-assignment. Publication and authority are separate ceilings here — do not read + // one as the other. // // memberWriteInvariants() is implied by hasOnly(['positions']) TODAY and is stated // anyway: if hasOnly is ever widened — the obvious future edit is adding a second diff --git a/packages/types/src/permission.test.ts b/packages/types/src/permission.test.ts index e6ac2346..b6183c6a 100644 --- a/packages/types/src/permission.test.ts +++ b/packages/types/src/permission.test.ts @@ -62,6 +62,32 @@ describe("Showcase subject", () => { }); }); +describe("BoardSeat subject", () => { + it("is a known subject", () => { + expect(SUBJECTS).toContain("BoardSeat"); + }); + // Same shape as Showcase: only update:BoardSeat is read (firestore.rules' boardSeatDelegate, + // beacon's resolveTrustedGrants, backstage's canAssignBoardSeat). The siblings gate nothing + // but must stay VALID — the /permisos matrix renders the full grid and the per-member + // override panel offers every code, so an unvalidatable one would fail the write. + it("accepts update:BoardSeat and the inert siblings the matrix will render", () => { + expect(isValidPermissionCode("update:BoardSeat")).toBe(true); + expect(isValidPermissionCode("manage:BoardSeat")).toBe(true); + expect(isValidPermissionCode("read:BoardSeat")).toBe(true); + }); +}); + +describe("MemberLogin subject", () => { + it("is a known subject", () => { + expect(SUBJECTS).toContain("MemberLogin"); + }); + it("accepts create:MemberLogin and the inert siblings the matrix will render", () => { + expect(isValidPermissionCode("create:MemberLogin")).toBe(true); + expect(isValidPermissionCode("manage:MemberLogin")).toBe(true); + expect(isValidPermissionCode("read:MemberLogin")).toBe(true); + }); +}); + describe("Notification subject", () => { it("is a known subject", () => { expect(SUBJECTS).toContain("Notification"); diff --git a/packages/types/src/permission.ts b/packages/types/src/permission.ts index b691741d..71f2ba40 100644 --- a/packages/types/src/permission.ts +++ b/packages/types/src/permission.ts @@ -27,6 +27,21 @@ export const SUBJECTS = [ // the /permisos matrix renders the full actions × subjects grid, so `checkIn:Member` and // dozens like it are already assignable and equally inert. "Showcase", + // Delegable board seating. Only `update:BoardSeat` is live: firestore.rules' + // boardSeatDelegate() is `hasAnyRole(['Admin']) || hasPerm('update:BoardSeat')` — an EXACT + // code match, not canDo(), so `manage:all` cannot satisfy it and the other five codes are + // inert. It confers no authority ALONE: it only widens which cargo an editor who already + // holds update:Member / create:Member / update:Position may assign, from "grant-free and + // non-CEL" to "any vacant cargo". Displacing a sitting power-cargo holder stays Admin-only + // (currentCargoGrantsEmpty), and the /positions CATALOG stays Admin-only. + "BoardSeat", + // Delegable login provisioning. Only `create:MemberLogin` is live, read by beacon's + // requireAdminOrPerm on provisionMemberLogin. NOT the invite email itself — that is a + // client-side sendPasswordResetEmail any signed-in user can already call. What this gates is + // Auth account creation + uid linking + the initial claim write. A non-Admin holder may only + // mint a NEW account, never adopt a pre-existing one (see provisionMember's callerIsAdmin + // branch), so it cannot be turned on an existing privileged account. + "MemberLogin", "all", ] as const; export type Subject = (typeof SUBJECTS)[number]; diff --git a/tests/firestore-rules/rules.test.ts b/tests/firestore-rules/rules.test.ts index 035f981d..031564f0 100644 --- a/tests/firestore-rules/rules.test.ts +++ b/tests/firestore-rules/rules.test.ts @@ -47,6 +47,21 @@ function anon() { const ORG_CHART = "orgchart-uid"; const orgChart = () => as(ORG_CHART, [], ["update:Position"]); +/** The board-seat delegate: an org-chart editor who ALSO holds update:BoardSeat, so + * boardSeatDelegate() lifts the new-side cargo conjunct for them. `update:Position` is + * load-bearing, not decoration — without an entry capability the delegate never reaches the + * arm at all and every ALLOW below would pass for the wrong reason. */ +const SEAT_DELEGATE = "seatdelegate-uid"; +const seatDelegate = () => as(SEAT_DELEGATE, [], ["update:Position", "update:BoardSeat"]); + +/** update:BoardSeat and NOTHING else. The non-vacuity pin for the whole feature: the code + * widens which cargo an editor may assign, it does not make anyone an editor. */ +const plainDelegate = () => as("seatonly-uid", [], ["update:BoardSeat"]); + +/** A delegate on the CREATE lane. create:Member is the entry capability there. */ +const createDelegate = () => as("createdelegate-uid", [], ["create:Member", "update:BoardSeat"]); +const createOnly = () => as("createonly-uid", [], ["create:Member"]); + const MEMBER_DOC = { name: "Ana", totalPoints: 0, uid: "owner-uid", active: true, deletedAt: null }; /** The birth state every members/positions/allies create arm now requires (B2): born @@ -587,6 +602,52 @@ beforeAll(async () => { active: true, deletedAt: null, }); + // Board-seat delegation targets. Each assertion that SUCCEEDS needs its own doc: the + // suite seeds once and never resets, so a shared target would carry the previous test's + // cargo into the next one's currentCargoGrantsEmpty() check. + await setDoc(doc(db, "members/m_delegate"), { + name: "Delegado", + totalPoints: 0, + uid: "delegate-target-uid", + active: true, + deletedAt: null, + }); + // Target for the conjunct pins: never seated, so currentCargoGrantsEmpty() short-circuits + // true and each assertion is denied by the conjunct it actually names. + await setDoc(doc(db, "members/m_delegate_pins"), { + name: "Pins", + totalPoints: 0, + uid: "delegate-pins-uid", + active: true, + deletedAt: null, + }); + await setDoc(doc(db, "members/m_delegate_cel"), { + name: "Delegado CEL", + totalPoints: 0, + uid: "delegate-cel-uid", + active: true, + deletedAt: null, + }); + // Seated on a POWER cargo, for the G3 pin: a delegate must NOT be able to displace this. + await setDoc(doc(db, "members/m_delegate_power"), { + name: "Delegado Poder", + totalPoints: 0, + uid: "delegate-power-uid", + active: true, + deletedAt: null, + positions: { [TERM]: { cargoId: "pos1", comisionIds: [], assignedBy: "admin-uid" } }, + }); + // Seated on a GRANT-FREE CEL cargo, for the takedown pin. Seeded directly rather than + // produced by an earlier test, so currentCargoGrantsEmpty() actually evaluates its get() + // branch instead of short-circuiting on `prior == null` and passing vacuously. + await setDoc(doc(db, "members/m_cel_takedown"), { + name: "Takedown", + totalPoints: 0, + uid: "cel-takedown-uid", + active: true, + deletedAt: null, + positions: { [TERM]: { cargoId: "pos_cel_free", comisionIds: [], assignedBy: "admin-uid" } }, + }); // A member whose POWER cargo sits under a PRIOR term key, leaving the CURRENT term slot // empty. Pins the term-rollover residual (docs/specs/position-assignment-lane.md, // "Residual: the term-rollover window"): currentCargoGrantsEmpty() reads only @@ -1039,6 +1100,125 @@ describe("firestore.rules — members", () => { }), ); }); + it("allows an update:BoardSeat delegate to CREATE a member on a CEL or power cargo", async () => { + // createPositionsSafe() takes the plain substitution — a create has no prior resource, so + // there is no old side to keep Admin-only and a member born on a cargo displaces nobody. + await assertSucceeds( + setDoc(doc(createDelegate(), "members/new_delegate_cel"), { + name: "Ximena Paz", + totalPoints: 0, + ...BORN_LIVE, + positions: { + [TERM]: { cargoId: "pos_cel_free", comisionIds: [], assignedBy: "createdelegate-uid" }, + }, + }), + ); + await assertSucceeds( + setDoc(doc(createDelegate(), "members/new_delegate_pow"), { + name: "Ximena Paz", + totalPoints: 0, + ...BORN_LIVE, + positions: { + [TERM]: { cargoId: "pos1", comisionIds: [], assignedBy: "createdelegate-uid" }, + }, + }), + ); + }); + + it("BLOCKING: a delegate may NOT create with a forged assignedBy", async () => { + // pos_soft is grant-free and non-CEL, so the cargo conjunct passes for ANY principal and + // assignedBySelf() is the sole denier. Without this the create lane's self-stamp had zero + // mutation coverage — every existing forgery case paired the forged uid with a power cargo, + // so the cargo conjunct denied it and the self-stamp could have been deleted silently. + // What it stops: a member born on a power cargo with a real Admin's uid as assignedBy, + // which the beacon trust gate would honor permanently — surviving revocation of the + // delegate's own code, because the assigner really is an Admin. + await assertFails( + setDoc(doc(createDelegate(), "members/new_delegate_forged"), { + name: "Ximena Paz", + totalPoints: 0, + ...BORN_LIVE, + positions: { [TERM]: { cargoId: "pos_soft", comisionIds: [], assignedBy: "admin-uid" } }, + }), + ); + }); + + it("BLOCKING: the create-lane term restriction binds an ADMIN too", async () => { + // Symmetric with the update arm, which has denied non-current-term writes to everyone + // including Admin since before this branch. Stated as its own test because the tempting + // future edit is `hasAnyRole(['Admin']) ||` in front of the conjunct the first time a + // migration looks blocked — and that silently reopens the ride-along for anyone who can + // get an Admin to run a create. + await assertFails( + setDoc(doc(as("admin-uid", ["Admin"]), "members/new_admin_ridealong"), { + name: "Ximena Paz", + totalPoints: 0, + ...BORN_LIVE, + positions: { + [TERM]: { cargoId: "pos_soft", comisionIds: [], assignedBy: "admin-uid" }, + "2099": { cargoId: "pos1", comisionIds: [], assignedBy: "admin-uid" }, + }, + }), + ); + }); + + it("BLOCKING: a delegate may NOT create with a ride-along NON-current term key", async () => { + // The create-lane twin of positionsDelta().hasOnly([currentTermKey()]). assignedBySelf() + // and cargoAssignableByNonAdmin() both read only positions[currentTermKey()], so a second + // term key rode along completely unvalidated: a clean current-term entry to pass the arm, + // plus a next-term power cargo attributed to a real Admin. On the UTC-year rollover + // claims-sync reads THAT entry and mints Admin onto a member whose login the creator + // controls. Forged attribution, one term deferred. + await assertFails( + setDoc(doc(createDelegate(), "members/new_delegate_ridealong"), { + name: "Ximena Paz", + totalPoints: 0, + ...BORN_LIVE, + positions: { + [TERM]: { cargoId: "pos_soft", comisionIds: [], assignedBy: "createdelegate-uid" }, + "2099": { cargoId: "pos1", comisionIds: [], assignedBy: "admin-uid" }, + }, + }), + ); + }); + + it("BLOCKING: create:Member alone, and update:BoardSeat alone, each reach nothing", async () => { + // The create-lane twin of the update-lane non-vacuity pin. update:BoardSeat widens WHICH + // cargo a creator may use; it does not make anyone a creator. + await assertFails( + setDoc(doc(plainDelegate(), "members/new_plaindelegate"), { + name: "Ximena Paz", + totalPoints: 0, + ...BORN_LIVE, + }), + ); + }); + + it("BLOCKING: the same create principal WITHOUT update:BoardSeat is still denied both", async () => { + // The paired denial — otherwise the two ALLOWs above would pass for any create:Member + // holder and prove nothing about the new disjunct. + await assertFails( + setDoc(doc(createOnly(), "members/new_createonly_cel"), { + name: "Ximena Paz", + totalPoints: 0, + ...BORN_LIVE, + positions: { + [TERM]: { cargoId: "pos_cel_free", comisionIds: [], assignedBy: "createonly-uid" }, + }, + }), + ); + await assertFails( + setDoc(doc(createOnly(), "members/new_createonly_pow"), { + name: "Ximena Paz", + totalPoints: 0, + ...BORN_LIVE, + positions: { + [TERM]: { cargoId: "pos1", comisionIds: [], assignedBy: "createonly-uid" }, + }, + }), + ); + }); + it("allows Admin creating a member on a grant-free CEL cargo (the authority, not the delegate)", async () => { // The paired ALLOW: the CEL conjunct lives inside the non-Admin branch of the create arm // too, so seating the CEL at create stays possible — for an Admin only. Without this, @@ -2372,6 +2552,30 @@ describe("firestore.rules — positions", () => { setDoc(doc(as("admin-uid", ["Admin"]), "positions/mint_cel_admin"), boardCargo("CEL")), ); }); + it("BLOCKING: update:BoardSeat does NOT reach the positions CATALOG", async () => { + // The delegation is a SEATING authority, never an authoring one. It lifts the cargo + // conjunct on members/{id}.positions and nothing else — boardSurfacingCategory() on + // create, and the grants/category/title pins on update, all stay hasAnyRole(['Admin']). + // Without that boundary the delegation would be self-serving: mint a grant-free CEL + // 'Presidente', then seat yourself on it, landing at public board rank 0. + // The principal deliberately holds the CATALOG capabilities too, so each denial is the + // Admin-only pin firing and not a missing create:Position / update:Position. + const catalogDelegate = () => + as("catalogdelegate-uid", [], ["create:Position", "update:Position", "update:BoardSeat"]); + await assertFails( + setDoc(doc(catalogDelegate(), "positions/mint_cel_delegate"), boardCargo("CEL")), + ); + await assertFails( + setDoc(doc(catalogDelegate(), "positions/mint_jdl_delegate"), boardCargo("JDL")), + ); + await assertFails( + updateDoc(doc(catalogDelegate(), "positions/pos_payload"), { grants: ["Admin"] }), + ); + await assertFails(updateDoc(doc(catalogDelegate(), "positions/pos_cat"), { category: "CEL" })); + await assertFails( + updateDoc(doc(catalogDelegate(), "positions/pos_payload"), { title: "Presidente" }), + ); + }); // Deliberate fail-closed: a legacy power comisión (possible before the // invariant) is client-unwritable — even soft-delete — until an admin-SDK/ // console repair empties its grants. Documented in the design spec. @@ -3006,6 +3210,140 @@ describe("firestore.rules — member positions assignment", () => { ); }); + // --- update:BoardSeat delegation (docs/specs/board-seat-delegation.md) --- + + it("allows an update:BoardSeat delegate to assign a GRANT-FREE CEL cargo", async () => { + // The exact write orgChart() is denied above, same cargo, same lane, differing only by + // the delegate's update:BoardSeat. That pairing is what proves the new disjunct is what + // opened it, rather than the CEL conjunct having quietly disappeared. + await assertSucceeds( + updateDoc(doc(seatDelegate(), "members/m_delegate_cel"), { + [`positions.${TERM}`]: { + cargoId: "pos_cel_free", + comisionIds: [], + assignedBy: SEAT_DELEGATE, + }, + }), + ); + }); + + it("allows an update:BoardSeat delegate to assign a POWER-conferring cargo", async () => { + // The accepted claims-minting delegation: pos1 confers grants, and beacon's claims-sync + // will mint them because the delegate holds update:BoardSeat. Owner-accepted; the + // revocability that acceptance rests on is enforced in beacon (non-reflexive trust gate), + // not here. + await assertSucceeds( + updateDoc(doc(seatDelegate(), "members/m_delegate"), { + [`positions.${TERM}`]: { cargoId: "pos1", comisionIds: [], assignedBy: SEAT_DELEGATE }, + }), + ); + }); + + it("G3 BLOCKING: denies a delegate DISPLACING a member who already holds a power cargo", async () => { + // currentCargoGrantsEmpty() is deliberately NOT delegated. computeMemberRoles derives the + // `roles` claim exclusively from cargo grants, so a principal who can overwrite a sitting + // Admin's cargo can strip every Admin claim in the chapter one write at a time — and + // setUserRoles, roles/* and permissionOverrides are all Admin-role-only, so the result is + // unrecoverable outside the Firebase console. If this goes green, the chapter is one + // delegate away from having no Admin. + await assertFails( + updateDoc(doc(seatDelegate(), "members/m_delegate_power"), { + [`positions.${TERM}`]: { cargoId: "pos_soft", comisionIds: [], assignedBy: SEAT_DELEGATE }, + }), + ); + // Clearing it is the same authority and equally denied — the guard is about the cargo + // being REPLACED, not about what replaces it. + await assertFails( + updateDoc(doc(seatDelegate(), "members/m_delegate_power"), { + [`positions.${TERM}`]: { cargoId: null, comisionIds: [], assignedBy: SEAT_DELEGATE }, + }), + ); + }); + + it("BLOCKING: denies update:BoardSeat ALONE any positions write", async () => { + // Non-vacuity pin for the whole feature. The code widens WHICH cargo an editor may + // assign; it does not make anyone an editor. Without update:Position / update:Member the + // principal never reaches an arm — which is also why every ALLOW above pairs it with one. + await assertFails( + updateDoc(doc(plainDelegate(), "members/m_positions"), { + [`positions.${TERM}`]: { cargoId: "pos_soft", comisionIds: [], assignedBy: "seatonly-uid" }, + }), + ); + }); + + it("denies a delegate a forged assignedBy, a past term, and a ride-along field", async () => { + // assignedBySelf(), the current-term restriction and hasOnly(['positions']) all sit + // OUTSIDE the substituted disjunction. Pin that the delegation did not loosen them — + // a forged assignedBy is what the beacon trust gate reads to decide whether to mint. + // + // m_delegate_pins, NOT m_delegate: the suite seeds once and never resets, and the + // power-cargo ALLOW above leaves m_delegate holding pos1. Every assertion here would then + // be denied by currentCargoGrantsEmpty() — the G3 guard — instead of by the conjunct it + // names, i.e. all three would pass with their own conjunct deleted. + await assertFails( + updateDoc(doc(seatDelegate(), "members/m_delegate_pins"), { + [`positions.${TERM}`]: { cargoId: "pos_soft", comisionIds: [], assignedBy: "admin-uid" }, + }), + ); + // Paired with a VALID current-term entry on purpose. A lone 2099 write on an unseated doc + // is already denied by assignedBySelf() — the post-merge current term is {} — so the term + // conjunct would not be the denier and the assertion would pin nothing. With both keys + // present, assignedBySelf(), cargoAssignableByNonAdmin() and currentCargoGrantsEmpty() all + // pass and only positionsDelta().hasOnly([currentTermKey()]) denies. + await assertFails( + updateDoc(doc(seatDelegate(), "members/m_delegate_pins"), { + [`positions.${TERM}`]: { cargoId: "pos_soft", comisionIds: [], assignedBy: SEAT_DELEGATE }, + "positions.2099": { cargoId: "pos1", comisionIds: [], assignedBy: "admin-uid" }, + }), + ); + await assertFails( + updateDoc(doc(seatDelegate(), "members/m_delegate_pins"), { + [`positions.${TERM}`]: { cargoId: "pos_soft", comisionIds: [], assignedBy: SEAT_DELEGATE }, + name: "Renombrado", + }), + ); + }); + + it("BLOCKING: hasPerm is EXACT — manage:all and manage:BoardSeat are not board-seat delegates", async () => { + // boardSeatDelegate() uses hasPerm, not canDo, and until now nothing in the RULES pinned + // that — the property was tested at the client and callable layers only. Mutating it to + // canDo('update','BoardSeat') would hand board seating to every manage:all holder, which + // is reachable without the Admin role through a custom role doc or permissionOverrides. + const mgrAll = () => as("mgrall-uid", [], ["update:Position", "manage:all"]); + const mgrSeat = () => as("mgrseat-uid", [], ["update:Position", "manage:BoardSeat"]); + await assertFails( + updateDoc(doc(mgrAll(), "members/m_delegate_pins"), { + [`positions.${TERM}`]: { + cargoId: "pos_cel_free", + comisionIds: [], + assignedBy: "mgrall-uid", + }, + }), + ); + await assertFails( + updateDoc(doc(mgrSeat(), "members/m_delegate_pins"), { + [`positions.${TERM}`]: { + cargoId: "pos_cel_free", + comisionIds: [], + assignedBy: "mgrseat-uid", + }, + }), + ); + }); + + it("REGRESSION: a non-delegate may still CLEAR a member off a grant-free CEL seat", async () => { + // The deliberate asymmetry (firestore.rules' currentCargoGrantsEmpty comment): keeping a + // grant-free CEL seat is denied to a non-Admin, but clearing one is allowed, or a takedown + // would be stranded behind an Admin. This is the guard on the expression the delegation + // touches, and m_cel_takedown is SEEDED holding pos_cel_free so currentCargoGrantsEmpty() + // reaches its get() instead of short-circuiting on a null prior. + await assertSucceeds( + updateDoc(doc(orgChart(), "members/m_cel_takedown"), { + [`positions.${TERM}`]: { cargoId: null, comisionIds: [], assignedBy: ORG_CHART }, + }), + ); + }); + // LAST in this block on purpose: the suite seeds once and never resets, and this write // leaves members/m1 holding a power cargo. A Membership success case running after it // would be denied by currentCargoGrantsEmpty() — the C1 guard — not by its own subject.