diff --git a/.changeset/approvals-actionable-links.md b/.changeset/approvals-actionable-links.md new file mode 100644 index 0000000000..c83488615b --- /dev/null +++ b/.changeset/approvals-actionable-links.md @@ -0,0 +1,6 @@ +--- +"@objectstack/plugin-approvals": minor +"@objectstack/spec": patch +--- + +ADR-0043 actionable approval links (#1743). `remind()` now fans out per approver: every concrete identity gets its own single-use approve/reject links in the notification payload. Tokens are 256-bit, stored as SHA-256 hashes only (`sys_approval_token`), scoped to one request + action + approver, 72h TTL, consumed-before-decide (replay burns), and re-validated at redemption against the live request (decided/recalled/reassigned ⇒ dead link). The plugin mounts a session-less bilingual confirm page at `GET /api/v1/approvals/act` (renders only — mail-gateway prefetch safe) and redeems exclusively on the `POST`, auditing the decision as the bound approver. diff --git a/docs/adr/0043-actionable-approval-links.md b/docs/adr/0043-actionable-approval-links.md new file mode 100644 index 0000000000..7410f6c828 --- /dev/null +++ b/docs/adr/0043-actionable-approval-links.md @@ -0,0 +1,77 @@ +# ADR-0043: Actionable approval links — single-use tokens with a session-less confirm page + +**Status**: Proposed (2026-06-12) +**Deciders**: ObjectStack Protocol Architects +**Builds on**: [ADR-0042](./0042-approval-sla-escalation.md) (reserved system actors, audit-first discipline), thread interactions (#1740), [ADR-0012/0030](./0030-notification-platform-convergence.md) (messaging + outbox) +**Closes**: [#1743](https://github.com/objectstack-ai/framework/issues/1743) +**Consumers**: `@objectstack/plugin-approvals` (token store + redemption + pages), messaging templates, future email channel wiring + +--- + +## TL;DR + +Approvers should act from an email/IM message without signing in — the +biggest lever on approval latency. The button behind that experience is a +URL whose bearer has **no session**, so the token in it must carry the +entire authorization, deliberately weakened on every axis: + +| axis | decision | failure it prevents | +|---|---|---| +| scope | one token = one request + one action + one approver | leaked token ≠ account takeover | +| storage | only the **SHA-256 hash** is stored (`sys_approval_token`) | a DB leak yields no usable links | +| single-use | `consumed_at` set transactionally before deciding | forwarded email replayed | +| TTL | 72 h default | months-old mail approving today's request | +| identity | token binds `approver_id`; the decision is audited as that approver | anonymous decisions | +| invalidation | redemption re-checks the request is still pending **and** the approver still holds the slot | stale links after reassign / recall / decision | +| scanner-proof | **GET never executes** — it renders a confirm page whose button POSTs | mail-gateway link prefetchers approving requests | + +The last row is the classic production incident: enterprise mail security +(Outlook SafeLinks et al.) pre-fetches every link in a message. Any +GET-executes design gets requests approved by robots. + +## Mechanics + +- **`sys_approval_token`** (new object — table creation only, no + migrations): `token_hash`, `request_id`, `action` + (`approve`/`reject`), `approver_id`, `expires_at`, `consumed_at`. +- **Issue** (`issueActionTokens`): 256-bit random raw tokens, returned + once, hashes stored. Wired into `remind()` — each pending approver with + a concrete identity (not `role:*` literals) gets their **own** + notification carrying their own approve/reject links. (Open-time + notification remains the flow author's `notify` node; templates there + can adopt the same links later.) +- **Confirm page** (`GET /api/v1/approvals/act?token=…`): session-less + minimal HTML rendered by the plugin on the host Hono app — request + summary (flow label, record title, action) + a POST form. Invalid / + expired / consumed tokens render an explanatory page with a Console + deep link; the GET **never** mutates. +- **Redeem** (`POST /api/v1/approvals/act`): hash lookup → not consumed → + not expired → request still `pending` → `approver_id` still in + `pending_approvers` → mark consumed → `decide()` **as that approver** + (system context carries the bound identity). Every check failure maps + to a distinct, non-enumerable result page. +- **URLs**: relative by default (works inside Console/IM webviews); + deployments set `publicBaseUrl` (plugin option) for absolute links in + outbound email. + +## Non-goals (v1) + +- Comment capture on the confirm page (a decision comment field is a + fast follow; the page ships decision-only). +- Email channel *delivery* configuration — the links ride the existing + messaging payloads; SMTP setup is deployment concern. +- Rate limiting beyond single-use + TTL (the token space is 2^256; + brute force is not the threat — leaked links are, and those die on + first use / decision / reassign / expiry). + +## Consequences + +- The remind nudge becomes genuinely actionable — one tap from the + notification to a decision, with the audit trail showing the human + approver (never a system actor). +- A deliberate, narrow bypass lane around session auth exists; its + entire surface is this ADR's table, and every property is enforced in + `redeemActionToken` with tests per row. +- Stale-link UX is explicit: recalled/decided/reassigned requests answer + with "this link is no longer valid" + a Console deep link, not an + error code. diff --git a/packages/plugins/plugin-approvals/src/action-link-pages.ts b/packages/plugins/plugin-approvals/src/action-link-pages.ts new file mode 100644 index 0000000000..57322ade07 --- /dev/null +++ b/packages/plugins/plugin-approvals/src/action-link-pages.ts @@ -0,0 +1,102 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Session-less HTML for the actionable-link confirm/result pages (ADR-0043). + * + * Deliberately tiny and dependency-free: these pages are reached from an + * email or IM message by a bearer with no session, so they must not assume + * the Console bundle, auth state, or client-side i18n. Static bilingual + * (EN / 中文) copy keeps them readable for the demo audience without a + * locale negotiation step. + * + * The GET page NEVER mutates — the decision happens only on the POST form + * submit (mail-gateway link prefetchers must not approve requests). + */ + +import type { ApprovalRequestRow, ApprovalActionKind } from '@objectstack/spec/contracts'; + +function esc(s: unknown): string { + return String(s ?? '') + .replaceAll('&', '&').replaceAll('<', '<').replaceAll('>', '>') + .replaceAll('"', '"').replaceAll("'", '''); +} + +function shell(title: string, body: string): string { + return ` +
+ + +${approving ? '确认通过该审批请求?' : '确认拒绝该审批请求?'} + Acting as · 操作身份:${esc(input.approverId)}
+ ${summaryRows(input.request)} + +This link is single-use and expires automatically. · 此链接一次有效,过期自动失效。
`); +} + +const RESULT_COPY: Record${esc(copy.body)}
+Open the Approvals Inbox · 打开审批中心
`); +} diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index 34f3b13e22..23a0c0959d 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -447,7 +447,9 @@ describe('ApprovalService (node era)', () => { const req = await svc.openNodeRequest(openInput(['u9', 'u2']), CTX); const out = await svc.remind(req.id, { actorId: 'u1' }, CTX); // u1 = submitter (CTX.userId) expect(out.notified).toBe(2); - expect(emitted[0]).toMatchObject({ topic: 'approval.reminder', audience: ['u9', 'u2'] }); + // ADR-0043: per-approver fan-out so each reminder carries personal links. + const reminders = emitted.filter(e => e.topic === 'approval.reminder'); + expect(reminders.map(r => r.audience)).toEqual([['u9'], ['u2']]); const actions = await svc.listActions(req.id, SYS); expect(actions.at(-1)?.action).toBe('remind'); // The fake clock steps 1s per call — well inside the 4h cool-down. @@ -482,6 +484,76 @@ describe('ApprovalService (node era)', () => { expect(actions.filter(a => a.action === 'comment')).toHaveLength(2); }); + // ── actionable links (ADR-0043) ───────────────────────────────── + + it('issueActionTokens: stores hashes only and binds approver + action', async () => { + const req = await svc.openNodeRequest(openInput(['u9']), CTX); + const tokens = await svc.issueActionTokens(req.id, 'u9'); + expect(tokens.approve).not.toBe(tokens.reject); + const rows = engine._tables['sys_approval_token']; + expect(rows).toHaveLength(2); + expect(rows.every(r => r.token_hash.length === 64)).toBe(true); // sha256 hex, never the raw token + expect(rows.every(r => !JSON.stringify(r).includes(tokens.approve))).toBe(true); + await expect(svc.issueActionTokens(req.id, 'stranger')).rejects.toThrow(/FORBIDDEN/); + }); + + it('redeem: approves as the bound approver and burns the token (single-use)', async () => { + const resumed: any[] = []; + svc.attachAutomation({ async resume(runId, signal) { resumed.push({ runId, signal }); } }); + const req = await svc.openNodeRequest(openInput(['u9']), CTX); + const { approve } = await svc.issueActionTokens(req.id, 'u9'); + const out = await svc.redeemActionToken(approve); + expect(out).toMatchObject({ ok: true, action: 'approve', approverId: 'u9' }); + expect((out as any).request.status).toBe('approved'); + expect(resumed[0]?.signal?.branchLabel).toBe('approve'); + const acts = await svc.listActions(req.id, SYS); + expect(acts.at(-1)).toMatchObject({ action: 'approve', actor_id: 'u9', comment: 'Via action link' }); + // replay + expect(await svc.redeemActionToken(approve)).toMatchObject({ ok: false, reason: 'consumed' }); + }); + + it('peek: validates without consuming (GET never mutates)', async () => { + const req = await svc.openNodeRequest(openInput(['u9']), CTX); + const { reject } = await svc.issueActionTokens(req.id, 'u9'); + expect(await svc.peekActionToken(reject)).toMatchObject({ ok: true, action: 'reject' }); + expect(await svc.peekActionToken(reject)).toMatchObject({ ok: true }); // still live + const fresh = await svc.getRequest(req.id, SYS); + expect(fresh?.status).toBe('pending'); + }); + + it('redeem: dead tokens — invalid, expired, decided request, reassigned slot', async () => { + const req = await svc.openNodeRequest(openInput(['u9']), CTX); + expect(await svc.redeemActionToken('garbage')).toMatchObject({ ok: false, reason: 'invalid' }); + + const short = await svc.issueActionTokens(req.id, 'u9', { ttlMs: 1 }); + // fake clock advances 1s per call — far beyond a 1ms TTL + expect(await svc.redeemActionToken(short.approve)).toMatchObject({ ok: false, reason: 'expired' }); + + const live = await svc.issueActionTokens(req.id, 'u9'); + await svc.reassign(req.id, { actorId: 'u9', to: 'u7' }, CTX); + expect(await svc.redeemActionToken(live.approve)).toMatchObject({ ok: false, reason: 'not_approver' }); + + const forU7 = await svc.issueActionTokens(req.id, 'u7'); + await svc.decideNode(req.id, { decision: 'approve', actorId: 'u7' }, SYS); + expect(await svc.redeemActionToken(forU7.reject)).toMatchObject({ ok: false, reason: 'not_pending' }); + }); + + it('remind: each concrete approver gets their own action links', async () => { + const emitted: any[] = []; + svc.attachMessaging({ async emit(input) { emitted.push(input); } }); + const req = await svc.openNodeRequest(openInput(['u9', 'ada@example.com']), CTX); + await svc.remind(req.id, { actorId: 'u1' }, CTX); + const reminders = emitted.filter(e => e.topic === 'approval.reminder'); + expect(reminders).toHaveLength(2); + for (const r of reminders) { + expect(r.audience).toHaveLength(1); + expect(r.payload.actions).toHaveLength(2); + expect(r.payload.actions[0].url).toContain('/api/v1/approvals/act?token='); + } + const urls = reminders.flatMap(r => r.payload.actions.map((a: any) => a.url)); + expect(new Set(urls).size).toBe(4); // every link is personal + per-action + }); + // ── pagination + search pushdown (#1745) ──────────────────────── async function openMany(n: number) { diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 6bd3a75aca..be76431888 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -1,5 +1,6 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. +import { createHash, randomBytes } from 'node:crypto'; import { APPROVAL_BRANCH_LABELS, type ApprovalNodeConfig, @@ -78,6 +79,14 @@ export const ESCALATION_SCAN_INTERVAL_MS = 5 * 60 * 1000; /** Reserved actor id for machine decisions made by the SLA scanner. */ export const SLA_ACTOR_ID = 'system:sla'; +/** Default lifetime of an actionable-link token (ADR-0043). */ +export const ACTION_TOKEN_TTL_MS = 72 * 60 * 60 * 1000; + +/** Outcome of redeeming (or peeking) an actionable-link token. */ +export type ActionTokenOutcome = + | { ok: true; action: 'approve' | 'reject'; request: ApprovalRequestRow; approverId: string } + | { ok: false; reason: 'invalid' | 'expired' | 'consumed' | 'not_pending' | 'not_approver'; request?: ApprovalRequestRow }; + const SYSTEM_CTX = { isSystem: true, roles: [], permissions: [] } as const; function uid(prefix: string): string { @@ -182,6 +191,12 @@ export interface ApprovalServiceOptions { automation?: ApprovalResumeSurface; /** Optional messaging service for thread notifications. */ messaging?: ApprovalMessagingSurface; + /** + * Absolute origin prefixed onto actionable links (ADR-0043), e.g. + * `https://app.example.com`. Defaults to relative URLs, which work inside + * the Console and IM webviews; outbound email needs the absolute form. + */ + publicBaseUrl?: string; } export class ApprovalService implements IApprovalService { @@ -190,6 +205,7 @@ export class ApprovalService implements IApprovalService { private readonly logger?: ApprovalServiceOptions['logger']; private automation?: ApprovalResumeSurface; private messaging?: ApprovalMessagingSurface; + private publicBaseUrl: string; constructor(opts: ApprovalServiceOptions) { this.engine = opts.engine; @@ -197,6 +213,7 @@ export class ApprovalService implements IApprovalService { this.logger = opts.logger; this.automation = opts.automation; this.messaging = opts.messaging; + this.publicBaseUrl = (opts.publicBaseUrl ?? '').replace(/\/$/, ''); } /** Attach (or replace) the automation surface used to resume flow runs. */ @@ -735,23 +752,153 @@ export class ApprovalService implements IApprovalService { actor_id: input.actorId, comment: input.comment ?? null, created_at: nowIso, }, { context: SYSTEM_CTX }); - const notified = await this.notify({ - topic: 'approval.reminder', - audience: pending, - actorId: input.actorId, - source: { object: 'sys_approval_request', id: requestId }, - dedupKey: `approval-remind-${requestId}-${nowIso}`, - payload: { - title: 'Approval reminder', - message: `A decision on ${raw.object_name}/${raw.record_id} is still waiting on you.`, - actionUrl: '/system/approvals', - }, - }); + // Per-approver fan-out: concrete identities (user ids / emails) each get + // their OWN one-tap approve/reject links (ADR-0043); `role:*`-style + // literals can't carry a personal token and fall back to a plain nudge. + let notified = 0; + const concrete = pending.filter(a => a && !a.includes(':')); + const literals = pending.filter(a => a && a.includes(':')); + for (const approver of concrete) { + try { + const tokens = await this.issueActionTokens(requestId, approver); + notified += await this.notify({ + topic: 'approval.reminder', + audience: [approver], + actorId: input.actorId, + source: { object: 'sys_approval_request', id: requestId }, + dedupKey: `approval-remind-${requestId}-${nowIso}-${approver}`, + payload: { + title: 'Approval reminder', + message: `A decision on ${raw.object_name}/${raw.record_id} is still waiting on you.`, + actionUrl: '/system/approvals', + actions: [ + { label: 'Approve', url: this.actionLinkUrl(tokens.approve) }, + { label: 'Reject', url: this.actionLinkUrl(tokens.reject) }, + ], + }, + }); + } catch (err: any) { + this.logger?.warn?.('[approvals] reminder with action links failed', { + request: requestId, approver, error: err?.message ?? String(err), + }); + } + } + if (literals.length) { + notified += await this.notify({ + topic: 'approval.reminder', + audience: literals, + actorId: input.actorId, + source: { object: 'sys_approval_request', id: requestId }, + dedupKey: `approval-remind-${requestId}-${nowIso}`, + payload: { + title: 'Approval reminder', + message: `A decision on ${raw.object_name}/${raw.record_id} is still waiting on you.`, + actionUrl: '/system/approvals', + }, + }); + } const fresh = await this.getRequest(requestId, context); return { request: fresh!, notified }; } + // ── Actionable links (ADR-0043) ────────────────────────────── + + /** Build the session-less confirm-page URL for a raw token. */ + actionLinkUrl(rawToken: string): string { + return `${this.publicBaseUrl}/api/v1/approvals/act?token=${encodeURIComponent(rawToken)}`; + } + + /** + * Issue one-tap approve/reject tokens for one approver on one pending + * request. Raw tokens are returned ONCE; only SHA-256 hashes are stored + * (`sys_approval_token`), so a DB leak yields no usable links. + */ + async issueActionTokens( + requestId: string, + approverId: string, + opts?: { ttlMs?: number }, + ): Promise<{ approve: string; reject: string }> { + if (!approverId?.trim()) throw new Error('VALIDATION_FAILED: approverId is required'); + const raw = await this.loadPendingRow(requestId); + const pending = csvSplit(raw.pending_approvers); + if (!pending.includes(approverId)) { + throw new Error(`FORBIDDEN: '${approverId}' is not a pending approver on this request`); + } + const now = this.clock.now(); + const expires = new Date(now.getTime() + (opts?.ttlMs ?? ACTION_TOKEN_TTL_MS)).toISOString(); + const out = { approve: '', reject: '' }; + for (const action of ['approve', 'reject'] as const) { + const rawToken = randomBytes(32).toString('base64url'); + await this.engine.insert('sys_approval_token', { + id: uid('atok'), + organization_id: raw.organization_id ?? null, + token_hash: createHash('sha256').update(rawToken).digest('hex'), + request_id: requestId, + action, + approver_id: approverId, + expires_at: expires, + consumed_at: null, + created_at: now.toISOString(), + }, { context: SYSTEM_CTX }); + out[action] = rawToken; + } + return out; + } + + /** Shared validation chain for peek/redeem. Returns the token row when live. */ + private async resolveActionToken(rawToken: string): Promise< + { ok: true; token: any; request: ApprovalRequestRow } | Extract