|
| 1 | +--- |
| 2 | +"@objectstack/plugin-approvals": minor |
| 3 | +"@objectstack/rest": patch |
| 4 | +--- |
| 5 | + |
| 6 | +fix(approvals): an approval action is recorded against the authenticated caller, never a body field (#3800) |
| 7 | + |
| 8 | +Every mutating approvals entrypoint takes an `actorId`, and the REST routes |
| 9 | +filled it from `body.actorId ?? body.actor_id ?? context.userId` — so the body |
| 10 | +won. The service then authorized *that value*: `pending_approvers.includes( |
| 11 | +input.actorId)` for a decision, `submitter_id === actorId` for a recall. It never |
| 12 | +checked that the value named the caller. |
| 13 | + |
| 14 | +So any authenticated user could POST `{"actorId": "<someone else>"}` and have |
| 15 | +that person's approval recorded, the request finalized, and the owning flow run |
| 16 | +resumed down the `approve` edge — or name a request's submitter and recall it. |
| 17 | +With `api.requireAuth` unset the anonymous-deny never fires either, so an |
| 18 | +unauthenticated request could do the same. |
| 19 | + |
| 20 | +#3783 drew this line for the *data-write* identity and called the audit-row half |
| 21 | +"tolerable". It was not: the same unchecked string was the authorization key, so |
| 22 | +naming someone else was not a mislabelled audit row, it was how you got through |
| 23 | +the door. |
| 24 | + |
| 25 | +The actor is now resolved server-side (`ApprovalService.resolveActor`) on all |
| 26 | +nine entrypoints — `decide` / `decideNode`, `recall`, `sendBack`, `resubmit`, |
| 27 | +`reassign`, `remind`, `requestInfo`, `comment`. |
| 28 | + |
| 29 | +**The rule is not "`actorId` must equal `context.userId`."** A slot can |
| 30 | +legitimately be keyed by something else: the approver resolver stores the |
| 31 | +`type:value` literal when a graph lookup finds no holders, and the Console picks |
| 32 | +from the caller's own identity list — user id, email, or `role:<r>`. The rule is |
| 33 | +**"the actor must be an identity the server can prove belongs to the caller"**: |
| 34 | + |
| 35 | +- A **system** context keeps its explicit actor. The SLA sweep's reserved |
| 36 | + `system:sla` sentinel and the ADR-0043 action link — whose single-use hashed |
| 37 | + token binds exactly one approver — are unchanged. They are the only callers |
| 38 | + holding a trustworthy actor with no session behind them. |
| 39 | +- A caller with **no identity at all** is now refused. This is the anonymous case |
| 40 | + above. |
| 41 | +- **No `actorId`, or one naming the caller**, resolves to the caller. This is the |
| 42 | + common path and what the Console already sends. |
| 43 | +- **Any other value** is accepted only when the server can prove the caller holds |
| 44 | + it — `position:<p>` / `role:<p>` against the positions on the resolved authz |
| 45 | + context, or the caller's own email (one lazy `sys_user` read, taken only when |
| 46 | + nothing cheaper matched). Otherwise `FORBIDDEN`. |
| 47 | + |
| 48 | +REST still forwards the body value; it is now a *hint* the service validates, |
| 49 | +which is what keeps the email and `type:value` slot cases working. |
| 50 | + |
| 51 | +**Upgrade note.** A client that deliberately sent another user's `actorId` now |
| 52 | +gets `403 FORBIDDEN` instead of silently succeeding. Send the action as the |
| 53 | +acting user's own session — the field can be omitted entirely, and the caller is |
| 54 | +used. Server-to-server callers that legitimately act for someone else should |
| 55 | +present a system context, as the SLA sweep and the action link already do. |
| 56 | + |
| 57 | +This also makes two existing claims true that were previously aspirational: the |
| 58 | +approval object's declared actions say "`actorId` defaults to the caller |
| 59 | +server-side… the service remains the authority on who may act", and |
| 60 | +`attachViewers` documents `can_act` as mirroring "the exact authorization the |
| 61 | +decision methods enforce". |
0 commit comments