From 58ab87f011f16a5ad91c08e1e4037d27a974c716 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 20 Jul 2026 02:54:42 +0000 Subject: [PATCH] feat(approvals): gate declared decision actions on a server-computed viewer capability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `getRequest` / `listRequests` now attach a per-viewer block to each row from the caller's context — `viewer: { can_act, is_submitter }` (ApprovalRequestRow): - can_act — the caller is a current pending approver (their user id is in the resolved pending_approvers while status is pending). This is the exact check the decision methods authorize with, so it already reflects position/team/manager resolution. - is_submitter — the caller submitted the request. The declared decision actions on sys_approval_request now gate their `visible` CEL on it: approver actions (approve/reject/reassign/send-back/request-info) use `record.viewer.can_act`; submitter levers (remind/recall/resubmit) use `record.viewer.is_submitter`. Before this, approver actions only trimmed the non-pending case, so a submitter viewing their own pending request saw buttons they couldn't use (the backend 403'd), and a position-addressed approver could be wrongly hidden by a client-side identity heuristic. Where `viewer` is absent (a row surfaced outside a service read with a user context), the predicate fails closed. Tests: getRequest returns can_act true for a pending approver, false for the submitter and strangers, and false once finalized; listRequests attaches viewer to every row; the object contract test pins the new predicates. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016ypkQikZ55oWUHUnMebwXA --- .changeset/approvals-viewer-can-act.md | 26 ++++++++++++++ .../src/approval-service.test.ts | 27 ++++++++++++++ .../plugin-approvals/src/approval-service.ts | 22 ++++++++++++ .../src/sys-approval-request.object.test.ts | 11 +++--- .../src/sys-approval-request.object.ts | 36 ++++++++++--------- .../spec/src/contracts/approval-service.ts | 22 ++++++++++++ 6 files changed, 123 insertions(+), 21 deletions(-) create mode 100644 .changeset/approvals-viewer-can-act.md diff --git a/.changeset/approvals-viewer-can-act.md b/.changeset/approvals-viewer-can-act.md new file mode 100644 index 0000000000..b496d67485 --- /dev/null +++ b/.changeset/approvals-viewer-can-act.md @@ -0,0 +1,26 @@ +--- +"@objectstack/spec": minor +"@objectstack/plugin-approvals": minor +--- + +feat(approvals): server-computed `viewer` capability for precise decision-action gating + +`getRequest` / `listRequests` now attach a per-viewer block — +`viewer: { can_act, is_submitter }` — computed from the caller's context +(`ApprovalRequestRow.viewer`): + +- `can_act` — the caller is a *current pending approver* (their user id is in the + request's resolved `pending_approvers` while it is still `pending`). This is + the same check the decision methods authorize with, so it already reflects + position/team/manager resolution — strictly more accurate than a client-side + identity guess. +- `is_submitter` — the caller submitted the request. + +The declared decision actions on `sys_approval_request` now gate on it: approver +actions (approve/reject/reassign/send-back/request-info) use +`record.viewer.can_act`; submitter levers (remind/recall/resubmit) use +`record.viewer.is_submitter`. Previously approver actions only trimmed the +non-pending case, so a submitter viewing their own pending request saw buttons +they couldn't use (the backend 403'd); a position-addressed approver could be +wrongly hidden by the old client heuristic. Where `viewer` is absent (a row +surfaced outside a service read with a user context), the predicate fails closed. diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index 3e1e1f333a..c789108c15 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -414,6 +414,33 @@ describe('ApprovalService (node era)', () => { expect(await svc.getRequest('nope', SYS)).toBeNull(); }); + // ── viewer capability (#3310) ─────────────────────────────────── + it('getRequest: viewer.can_act is true for a pending approver, false for the submitter', async () => { + const req = await svc.openNodeRequest(openInput(['u9']), CTX); // submitter u1, approver u9 + const asApprover = await svc.getRequest(req.id, { userId: 'u9', tenantId: 't1' } as any); + expect(asApprover!.viewer).toEqual({ can_act: true, is_submitter: false }); + const asSubmitter = await svc.getRequest(req.id, { userId: 'u1', tenantId: 't1' } as any); + expect(asSubmitter!.viewer).toEqual({ can_act: false, is_submitter: true }); + const asOther = await svc.getRequest(req.id, { userId: 'u_stranger', tenantId: 't1' } as any); + expect(asOther!.viewer).toEqual({ can_act: false, is_submitter: false }); + }); + + it('getRequest: viewer.can_act drops to false once the request is finalized', async () => { + const req = await svc.openNodeRequest(openInput(['u9']), CTX); + await svc.decideNode(req.id, { decision: 'approve', actorId: 'u9' }, SYS); // → approved + const after = await svc.getRequest(req.id, { userId: 'u9', tenantId: 't1' } as any); + expect(after!.status).toBe('approved'); + expect(after!.viewer!.can_act).toBe(false); + }); + + it('listRequests: attaches viewer to every row from the caller context', async () => { + await svc.openNodeRequest(openInput(['u9']), CTX); + const rows = await svc.listRequests({ status: 'pending' }, { userId: 'u9', tenantId: 't1' } as any); + expect(rows.length).toBeGreaterThan(0); + expect(rows.every(r => r.viewer != null)).toBe(true); + expect(rows[0].viewer!.can_act).toBe(true); + }); + // ── recall ────────────────────────────────────────────────────── it('recall: submitter withdraws a pending request', async () => { diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index eb107ca6ff..d52a449fdb 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -2174,6 +2174,7 @@ export class ApprovalService implements IApprovalService { const rows = await this.engine.find('sys_approval_request', findOpts); const list = Array.isArray(rows) ? rows.map(rowFromRequest) : []; await this.enrichRows(list); + this.attachViewers(list, context); return list; } @@ -2221,6 +2222,7 @@ export class ApprovalService implements IApprovalService { await this.enrichRows([row]); await this.attachFlowSteps(row); await this.attachDecisionProgress(row, rows[0]); + this.attachViewers([row], context); return row; } @@ -2270,6 +2272,26 @@ export class ApprovalService implements IApprovalService { } catch { /* display-only enrichment */ } } + /** + * Attach the per-viewer capability block (#3310) from the caller's context. + * `can_act` mirrors the exact authorization the decision methods enforce — the + * caller's user id is in the resolved `pending_approvers` while the request is + * still `pending` (position/team/manager approvers are already resolved to + * concrete user ids at open time, so a plain membership test is faithful). + * `is_submitter` is a straight owner check. System/tokenless contexts get a + * both-false block. Cheap + synchronous — safe on list reads. + */ + private attachViewers(rows: ApprovalRequestRow[], context: SharingExecutionContext): void { + const uid = (context as any)?.userId != null ? String((context as any).userId) : null; + for (const row of rows) { + const pending = row.pending_approvers ?? []; + (row as any).viewer = { + can_act: row.status === 'pending' && !!uid && pending.includes(uid), + is_submitter: !!uid && row.submitter_id != null && String(row.submitter_id) === uid, + }; + } + } + /** * Derive approval-step progress from the owning flow's graph (single-read * enrichment only — list reads skip it). Walks from the start node diff --git a/packages/plugins/plugin-approvals/src/sys-approval-request.object.test.ts b/packages/plugins/plugin-approvals/src/sys-approval-request.object.test.ts index e293ea90d9..5c7699e389 100644 --- a/packages/plugins/plugin-approvals/src/sys-approval-request.object.test.ts +++ b/packages/plugins/plugin-approvals/src/sys-approval-request.object.test.ts @@ -61,15 +61,16 @@ describe('sys_approval_request declared actions', () => { } }); - it('gates submitter-only levers on the current user; approver actions only on pending', () => { + it('gates on the server-computed viewer block (#3310): approver actions on can_act, submitter levers on is_submitter', () => { for (const name of ['approval_remind', 'approval_recall', 'approval_resubmit']) { - expect(vis(name)).toContain('record.submitter_id == ctx.user.id'); + expect(vis(name)).toContain('record.viewer.is_submitter'); + expect(vis(name)).not.toContain('can_act'); } - // Approver-side actions defer who-can-act to the service; they only trim the - // non-pending case in the UI. for (const name of ['approval_approve', 'approval_reject', 'approval_send_back', 'approval_request_info', 'approval_reassign']) { - expect(vis(name)).toContain('record.status == "pending"'); + expect(vis(name)).toContain('record.viewer.can_act'); + // who-can-act is server-derived, never a client identity guess expect(vis(name)).not.toContain('ctx.user.id'); + expect(vis(name)).not.toContain('is_submitter'); } }); diff --git a/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts b/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts index 2b0d55e03e..f3e04c2572 100644 --- a/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts +++ b/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts @@ -236,8 +236,12 @@ export const SysApprovalRequest = ObjectSchema.create({ // (and their params) ship as metadata, not as hand-written buttons. Each // targets the existing approvals REST route; `{id}` resolves from the row // and `actorId` defaults to the caller server-side. The service remains the - // authority on who may act (pending-approver check) — `visible` only trims - // the obvious non-pending case. + // authority on who may act; `visible` gates on the server-computed + // per-viewer block (#3310): approver actions on `record.viewer.can_act` + // (the caller is a current pending approver — same check the service + // authorizes a decision with, so position/team approvers resolve correctly), + // submitter actions on `record.viewer.is_submitter`. `viewer` is attached by + // getRequest/listRequests; where it is absent the predicate fails closed. actions: [ { name: 'approval_approve', @@ -253,7 +257,7 @@ export const SysApprovalRequest = ObjectSchema.create({ // string[]`; the decision route persists them on `sys_approval_action`. { name: 'attachments', label: 'Attachments', type: 'file', multiple: true, required: false }, ], - visible: 'record.status == "pending"', + visible: 'record.viewer.can_act', locations: ['record_section', 'list_item'], successMessage: 'Approved.', refreshAfter: true, @@ -269,7 +273,7 @@ export const SysApprovalRequest = ObjectSchema.create({ { name: 'comment', label: 'Comment', type: 'textarea', required: false }, { name: 'attachments', label: 'Attachments', type: 'file', multiple: true, required: false }, ], - visible: 'record.status == "pending"', + visible: 'record.viewer.can_act', confirmText: 'Reject this request? A rejection is final for every approver.', locations: ['record_section', 'list_item'], successMessage: 'Rejected.', @@ -291,7 +295,7 @@ export const SysApprovalRequest = ObjectSchema.create({ { field: 'submitter_id', name: 'to', label: 'New approver', required: true, helpText: 'User to hand this step to' }, { name: 'comment', label: 'Comment', type: 'textarea', required: false }, ], - visible: 'record.status == "pending"', + visible: 'record.viewer.can_act', locations: ['record_section'], successMessage: 'Reassigned.', refreshAfter: true, @@ -299,8 +303,8 @@ export const SysApprovalRequest = ObjectSchema.create({ // ── Approver secondary decisions ──────────────────────────────── // Send back for revision / request more info (ADR-0044). Both are approver - // actions on a pending request; the service is the authority on who may act, - // so `visible` only trims the non-pending case (matching approve/reject). + // actions, so `visible` gates on `record.viewer.can_act` (a current pending + // approver) — same as approve/reject. The service stays the authority. { name: 'approval_send_back', label: 'Send back', @@ -311,7 +315,7 @@ export const SysApprovalRequest = ObjectSchema.create({ params: [ { name: 'comment', label: 'Reason', type: 'textarea', required: false }, ], - visible: 'record.status == "pending"', + visible: 'record.viewer.can_act', locations: ['record_section'], successMessage: 'Sent back for revision.', refreshAfter: true, @@ -326,7 +330,7 @@ export const SysApprovalRequest = ObjectSchema.create({ params: [ { name: 'comment', label: 'What do you need?', type: 'textarea', required: true }, ], - visible: 'record.status == "pending"', + visible: 'record.viewer.can_act', locations: ['record_section'], successMessage: 'Information requested.', refreshAfter: true, @@ -334,10 +338,10 @@ export const SysApprovalRequest = ObjectSchema.create({ // ── Submitter continuity actions ──────────────────────────────── // Remind / recall (pending) and resubmit / recall (returned). These are the - // submitter's own levers, so `visible` gates on `submitter_id == ctx.user.id` - // — the current user is exposed via the console's predicate scope. The - // service re-checks ownership; the predicate keeps a non-submitter from ever - // seeing a button they cannot use. + // submitter's own levers, so `visible` gates on `record.viewer.is_submitter` + // (server-computed on the current viewer). The service re-checks ownership; + // the predicate keeps a non-submitter from ever seeing a button they cannot + // use. { name: 'approval_remind', label: 'Send reminder', @@ -348,7 +352,7 @@ export const SysApprovalRequest = ObjectSchema.create({ params: [ { name: 'comment', label: 'Note', type: 'textarea', required: false }, ], - visible: 'record.status == "pending" && record.submitter_id == ctx.user.id', + visible: 'record.status == "pending" && record.viewer.is_submitter', locations: ['record_section'], successMessage: 'Reminder sent.', refreshAfter: true, @@ -365,7 +369,7 @@ export const SysApprovalRequest = ObjectSchema.create({ ], // Recall applies while the request is live for the submitter — pending // (withdraw) or returned (abandon the revision instead of resubmitting). - visible: '(record.status == "pending" || record.status == "returned") && record.submitter_id == ctx.user.id', + visible: '(record.status == "pending" || record.status == "returned") && record.viewer.is_submitter', confirmText: 'Recall this request? Approvers can no longer act on it and the record is unlocked.', locations: ['record_section'], successMessage: 'Recalled.', @@ -381,7 +385,7 @@ export const SysApprovalRequest = ObjectSchema.create({ params: [ { name: 'comment', label: 'What changed?', type: 'textarea', required: false }, ], - visible: 'record.status == "returned" && record.submitter_id == ctx.user.id', + visible: 'record.status == "returned" && record.viewer.is_submitter', locations: ['record_section'], successMessage: 'Resubmitted.', refreshAfter: true, diff --git a/packages/spec/src/contracts/approval-service.ts b/packages/spec/src/contracts/approval-service.ts index dc23dcbf7b..f5e57047b2 100644 --- a/packages/spec/src/contracts/approval-service.ts +++ b/packages/spec/src/contracts/approval-service.ts @@ -112,6 +112,28 @@ export interface ApprovalRequestRow { need: number; groups?: Array<{ group: string; got: number; need: number; satisfied: boolean }>; }; + + /** + * Server-computed capability for THE CURRENT VIEWER (#3310), attached by + * `getRequest` / `listRequests` from the caller's context. Lets a client gate + * decision actions precisely without re-deriving identity resolution: + * declared approver actions use `record.viewer.can_act`, submitter actions use + * `record.viewer.is_submitter`. + * + * - `can_act` — the caller is a *current pending approver* (their user id is in + * the request's resolved `pending_approvers` while it is still `pending`). + * This mirrors the exact check the service uses to authorize a decision, so + * it is strictly more accurate than a client-side identity guess (it already + * reflects position/team/manager resolution baked into `pending_approvers`). + * - `is_submitter` — the caller submitted the request. + * + * Absent when the row is surfaced outside a service read with a user context + * (e.g. a raw data-API grid); a `record.viewer.*` predicate then fails closed. + */ + viewer?: { + can_act: boolean; + is_submitter: boolean; + }; } /** Kinds of entries on a request's audit trail. */