From 8683ef9fe7258d956fa92c1320cd701ed3054e3b Mon Sep 17 00:00:00 2001 From: Bianca Date: Fri, 30 Oct 2026 16:27:51 +0100 Subject: [PATCH] feat(people): describe each bound role's permissions and inherited bindings GET /api/internal/people-roles, and the bound role list in POST answers, now give each bound role every permission it grants (`permissions`, through parent roles too) and its own binding followed by the ones it inherits (`bindings`). The backend keeps assignments for People records that aren't linked yet, and this IdP only sees them when the record is linked. With these fields the backend applies peopleRoleChangeProblem's checks itself (bounded delegation and the rules of inherited bindings) before the link. Without them it refuses those assignments (PoliNetworkOrg/backend#105). Refs #78 --- README.md | 4 ++- src/auth/people-binding.test.ts | 19 ++++++++++- src/auth/people-role-command.ts | 39 ++++++++++++++++++++--- src/auth/people-roles.integration.test.ts | 4 +++ src/auth/people-roles.ts | 16 ++++------ 5 files changed, 66 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 85e8456..eca30f8 100644 --- a/README.md +++ b/README.md @@ -343,7 +343,7 @@ Future consumers should use authorization code with PKCE, validate token signatu The admin dashboard's People register gives people predetermined roles: those bound, on the role's page (People register card), to what holding them means in People. A role's holders are admins of the Telegram groups (`admin`), Capo Admins (`capo`), or members or the Lead of a team (`team::member`, `team::lead`, where the key is the dashboard's team key: `it`, `hr`, `design`, `intl` or `events`). Binding a role needs `idp:people:bind`, and, as when editing a role, only Master Admin can bind or unbind a role granting permissions they don't hold. Managed roles, and roles that carry any `idp:*` permission directly, through a parent or through an implication, can't be bound, and saving a role or permission that would give a bound role one is refused. Bind only one role to each meaning: `trusted-admin` inherits `admin`, so binding it as `admin` too makes giving `admin` ambiguous (409). -`GET /api/internal/people-roles` (backend client, `idp:access:read`) lists the bound roles and everyone holding them by hand or through a parent role, with their name, directory email, Telegram ID and username, which of those roles they were given directly, and since when they hold each one (`since`, `YYYY-MM-DD`: the earliest of their current assignments conferring it). Unlike the access snapshot it includes roles that grant no permission. People without a login account are the backend's own: they never appear here. +`GET /api/internal/people-roles` (backend client, `idp:access:read`) lists the bound roles, each with every permission it grants (`permissions`, through parent roles too) and its own binding followed by those it inherits (`bindings`), and everyone holding them by hand or through a parent role, with their name, directory email, Telegram ID and username, which of those roles they were given directly, and since when they hold each one (`since`, `YYYY-MM-DD`: the earliest of their current assignments conferring it). Unlike the access snapshot it includes roles that grant no permission. People without a login account are the backend's own: they never appear here. `POST /api/internal/people-roles` (backend client, `idp:people:roles`) applies one People command `{ id, actor, sub, reason, changes }` on behalf of the dashboard user the backend authenticated. `sub` is always a login account. Each change names a desired state: `{ kind: "admin" | "capo", active }` or `{ kind: "team", team, role: "member" | "lead" | null }`. This IdP plans the concrete assignments against its own role graph and applies all of them or none: a concept with no bound role stays People's own and is skipped; it refuses with 409 when the mapping is ambiguous (not exactly one bound role to give), or when the requested state would still be held through another role (ending `admin` for a Capo Admin, say), rather than reporting a change that did not happen. @@ -361,6 +361,8 @@ The backend only names the actor; this IdP resolves the actor's own access and r - unless Master Admin, the actor holds every permission the role grants. +The backend applies the same rules to assignments it keeps for records that aren't linked yet, which this IdP only sees when the record is linked: that is what each bound role's `permissions` and `bindings` are for. + The response is the person's bound roles after the command (`held`, `assigned`) and the bound role list. Each assignment is audited as `role.assign`/`role.unassign` with `via: "people"`, the reason and the command ID, and the command itself as `people.roles` under `people-command:`: repeating an ID with the same command, for example after a lost response, applies nothing again and returns the current roles, and repeating it with a different command is refused with 409. #### Deploying People roles diff --git a/src/auth/people-binding.test.ts b/src/auth/people-binding.test.ts index 12c1a00..42bebcd 100644 --- a/src/auth/people-binding.test.ts +++ b/src/auth/people-binding.test.ts @@ -14,7 +14,7 @@ import { MASTER_ADMIN_ROLE_KEY, resolveAccess, } from "./rbac"; -import { planPeopleCommand } from "./people-role-command"; +import { describePeopleBoundRoles, planPeopleCommand } from "./people-role-command"; function permission(key: string, managed = false): PermissionSummary { return { @@ -313,3 +313,20 @@ describe("planPeopleCommand with Capo Admins", () => { ).toThrow(/exactly one/); }); }); + +it("describes each bound role with its effective permissions and inherited bindings", () => { + const described = describePeopleBoundRoles(catalog); + expect(described.find((entry) => entry.key === "capo-admin")).toEqual({ + key: "capo-admin", + name: "capo-admin", + binding: "capo", + permissions: ["people:self", "tg:trusted"], + bindings: ["capo", "admin"], + }); + expect(described.find((entry) => entry.key === "hr-lead")).toMatchObject({ + permissions: ["admin:access", "people:read", "people:self", "people:write"], + bindings: ["team:hr:lead", "team:hr:member"], + }); + // Roles People can't manage are left out, as everywhere else. + expect(described.map((entry) => entry.key)).not.toContain("sneaky"); +}); diff --git a/src/auth/people-role-command.ts b/src/auth/people-role-command.ts index 4cf806f..a71b05a 100644 --- a/src/auth/people-role-command.ts +++ b/src/auth/people-role-command.ts @@ -1,6 +1,6 @@ import { z } from "zod"; import { parsePeopleBinding, peopleBindingProblem } from "./people-binding"; -import { expandRoleKeys, type RbacCatalog } from "./rbac"; +import { effectiveRolePermissions, expandRoleKeys, type RbacCatalog } from "./rbac"; /** * One People command from the backend. Each change names a desired state for one concept. @@ -35,8 +35,22 @@ export const peopleCommandSchema = z ctx.addIssue({ code: "custom", message: "Each concept may appear only once" }); }); export type PeopleCommand = z.infer; +/** + * A role bound to People, as the backend sees it. `permissions` and `bindings` let it apply + * `peopleRoleChangeProblem`'s rules itself to assignments it keeps before a record is linked, + * which this identity provider only sees at link time. + */ +export type PeopleBoundRole = { + key: string; + name: string; + binding: string; + /** Every permission the role grants, including through parent roles, sorted. */ + permissions: string[]; + /** Its own binding, then those of the roles it inherits. */ + bindings: string[]; +}; export type PeopleCommandResult = { - roles: { key: string; name: string; binding: string }[]; + roles: PeopleBoundRole[]; held: string[]; assigned: string[]; }; @@ -49,11 +63,28 @@ export function peopleBoundRoles(catalog: RbacCatalog) { ); } +/** The bound roles, with what `peopleRoleChangeProblem` checks for each of them. */ +export function describePeopleBoundRoles(catalog: RbacCatalog): PeopleBoundRole[] { + return peopleBoundRoles(catalog).map((r) => { + const inherited = expandRoleKeys(catalog, [r.key]).flatMap((key) => { + const binding = catalog.roles.find((entry) => entry.key === key)?.peopleBinding; + return key !== r.key && parsePeopleBinding(binding) ? [binding!] : []; + }); + return { + key: r.key, + name: r.name, + binding: r.peopleBinding!, + permissions: effectiveRolePermissions(catalog, r.key), + bindings: [r.peopleBinding!, ...inherited], + }; + }); +} + export function peopleCommandResult(catalog: RbacCatalog, assigned: string[]): PeopleCommandResult { - const roles = peopleBoundRoles(catalog); + const roles = describePeopleBoundRoles(catalog); const keys = new Set(roles.map((r) => r.key)); return { - roles: roles.map((r) => ({ key: r.key, name: r.name, binding: r.peopleBinding! })), + roles, held: expandRoleKeys(catalog, assigned).filter((key) => keys.has(key)), assigned: assigned.filter((key) => keys.has(key)).sort(), }; diff --git a/src/auth/people-roles.integration.test.ts b/src/auth/people-roles.integration.test.ts index 19731bf..2aec31c 100644 --- a/src/auth/people-roles.integration.test.ts +++ b/src/auth/people-roles.integration.test.ts @@ -194,6 +194,10 @@ describe.skipIf(!process.env.PEOPLE_ROLES_TEST_DATABASE_URL)( expect(directory.roles.map((entry) => entry.binding)).toEqual( expect.arrayContaining(["admin", "capo", `team:${team}:member`, `team:${team}:lead`]), ); + expect(directory.roles.find((entry) => entry.key === `${team}-capo`)).toMatchObject({ + permissions: [], + bindings: ["capo", "admin"], + }); const today = new Date().toISOString().slice(0, 10); const person = directory.users.find((entry) => entry.sub === target)!; expect(person).not.toHaveProperty("azureId"); diff --git a/src/auth/people-roles.ts b/src/auth/people-roles.ts index f959e52..4c52535 100644 --- a/src/auth/people-roles.ts +++ b/src/auth/people-roles.ts @@ -8,10 +8,11 @@ import { logAuthorizationDenial } from "./denial-log"; import { readIdentitySubject, refreshIdentityMembership } from "./identity-subject"; import { peopleRoleChangeProblem } from "./people-binding"; import { + type PeopleBoundRole, type PeopleCommand, type PeopleCommandResult, PeopleCommandConflict, - peopleBoundRoles, + describePeopleBoundRoles, peopleCommandResult, planPeopleCommand, } from "./people-role-command"; @@ -20,7 +21,7 @@ import { RbacError, assignedRoleKeys, auditSnapshot, readCatalog } from "./rbac- export type PeopleDirectory = { /** Every role People manages, with what holding it means. */ - roles: { key: string; name: string; binding: string }[]; + roles: PeopleBoundRole[]; /** Everyone holding at least one of those roles, by hand or through a parent role. */ users: { sub: string; @@ -50,14 +51,9 @@ export async function readPeopleDirectory(): Promise { return db.transaction( async (transaction) => { const catalog = await readCatalog(transaction); - const bound = peopleBoundRoles(catalog); - const boundKeys = new Set(bound.map((entry) => entry.key)); - const roles = bound.map((entry) => ({ - key: entry.key, - name: entry.name, - binding: entry.peopleBinding!, - })); - if (!bound.length) return { roles, users: [] }; + const roles = describePeopleBoundRoles(catalog); + const boundKeys = new Set(roles.map((entry) => entry.key)); + if (!roles.length) return { roles, users: [] }; const assignments = await transaction .select({ userId: userRole.userId, roleKey: role.key, assignedAt: userRole.assignedAt })