diff --git a/.changeset/olive-donkeys-shake.md b/.changeset/olive-donkeys-shake.md new file mode 100644 index 0000000000..ee6359fedf --- /dev/null +++ b/.changeset/olive-donkeys-shake.md @@ -0,0 +1,11 @@ +--- +'@objectstack/plugin-security': patch +--- + +Surface the `isDefault` audience-binding suggestion on stock instead of skipping auto-bound declarations + +`GET /api/v1/security/suggested-bindings` returned an empty list on a stock boot even though the `isDefault` permission set and its `everyone` binding both existed. The security plugin binds the app's baseline set to the `everyone` anchor at boot, before the first reconcile runs, so `syncAudienceBindingSuggestions` always found the declaration already satisfied and skipped it entirely — no row was written, and the declaration only ever appeared after an admin deleted the binding by hand. + +An already-satisfied declaration is now recorded rather than skipped, in the state it is actually in: `confirmed` with an empty `resolved_by`, which is how the backing object defines an observed binding ("bound at boot or by hand, not confirmed through the prompt"). It is deliberately not `pending`: that is the actionable-prompt state the console panel lists and the confirm/dismiss methods accept, so a pending row would ask an admin to accept a binding that already exists. + +The existing flow is unchanged — an unbound declaration still surfaces as `pending`, and the pending-to-confirmed transition still fires when the binding is observed later. diff --git a/packages/plugins/plugin-security/src/suggested-audience-bindings.test.ts b/packages/plugins/plugin-security/src/suggested-audience-bindings.test.ts index 37fcd104ba..55253de262 100644 --- a/packages/plugins/plugin-security/src/suggested-audience-bindings.test.ts +++ b/packages/plugins/plugin-security/src/suggested-audience-bindings.test.ts @@ -101,13 +101,98 @@ describe('syncAudienceBindingSuggestions (ADR-0090 D5/D9)', () => { expect(ql.tables.sys_audience_binding_suggestion).toHaveLength(1); }); - it('skips a set that is already bound to the anchor (e.g. the boot baseline)', async () => { + // [#7677] The boot baseline auto-bind (security-plugin) runs BEFORE the first + // sync, so on stock every app-declared `isDefault` set is already bound by the + // time this reconciler first sees it. Skipping those left the whole surface + // empty on stock — the declaration was only ever surfaced after an admin + // deleted the binding by hand. It is now recorded in the state it is actually + // in: confirmed (observed), never pending — a bound declaration is not + // awaiting a decision. + it('records an already-bound set as confirmed/observed (the boot baseline) rather than skipping it', async () => { const ql = makeQl([CRM_PACKAGE]); ql.tables.sys_permission_set.push({ id: 'ps_1', name: 'crm_readonly', package_id: 'com.example.crm' }); ql.tables.sys_position_permission_set.push({ id: 'pps_1', position_id: 'pos_everyone', permission_set_id: 'ps_1' }); + const out = await syncAudienceBindingSuggestions(ql); + + expect(out.created).toBe(1); + const rows = ql.tables.sys_audience_binding_suggestion; + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ + package_id: 'com.example.crm', + permission_set_name: 'crm_readonly', + anchor: 'everyone', + status: 'confirmed', + }); + // Observed, NOT resolved through the prompt — the object schema defines an + // empty `resolved_by` on a confirmed row as exactly that. + expect(rows[0].resolved_by).toBeUndefined(); + expect(rows[0].resolved_at).toBeTruthy(); + }); + + it('is idempotent for an already-bound set — a second sync creates nothing and does not duplicate', async () => { + const ql = makeQl([CRM_PACKAGE]); + ql.tables.sys_permission_set.push({ id: 'ps_1', name: 'crm_readonly', package_id: 'com.example.crm' }); + ql.tables.sys_position_permission_set.push({ id: 'pps_1', position_id: 'pos_everyone', permission_set_id: 'ps_1' }); + await syncAudienceBindingSuggestions(ql); + + const out2 = await syncAudienceBindingSuggestions(ql); + + expect(out2.created).toBe(0); + expect(out2.confirmedObserved).toBe(0); + expect(out2.pruned).toBe(0); + expect(ql.tables.sys_audience_binding_suggestion).toHaveLength(1); + expect(ql.tables.sys_audience_binding_suggestion[0].status).toBe('confirmed'); + }); + + it('surfaces the stock isDefault declaration through the list endpoint (#7677)', async () => { + // Stock: the app's isDefault set exists AND the boot auto-bind already + // bound it to `everyone`. The list must not be empty. + const ql = makeQl([CRM_PACKAGE]); + ql.tables.sys_permission_set.push({ id: 'ps_1', name: 'crm_readonly', package_id: 'com.example.crm' }); + ql.tables.sys_position_permission_set.push({ id: 'pps_1', position_id: 'pos_everyone', permission_set_id: 'ps_1' }); + const deps = makeDeps(ql); + + const { suggestions, synced } = await listAudienceBindingSuggestions(deps, ADMIN_CTX, {}); + + expect(synced.created).toBe(1); + expect(suggestions).toHaveLength(1); + expect(suggestions[0]).toMatchObject({ + permission_set_name: 'crm_readonly', + anchor: 'everyone', + status: 'confirmed', + }); + + // Second call: nothing created, no duplicate row. + const again = await listAudienceBindingSuggestions(deps, ADMIN_CTX, {}); + expect(again.synced.created).toBe(0); + expect(again.suggestions).toHaveLength(1); + + // The console panel lists `status=pending` as the actionable prompt set — + // a satisfied declaration must not appear there (never nag). + const prompts = await listAudienceBindingSuggestions(deps, ADMIN_CTX, { status: 'pending' }); + expect(prompts.suggestions).toHaveLength(0); + }); + + it('still creates a PENDING row when the declaration is unbound (the discriminating step)', async () => { + // The card's discriminator: with no binding row, the declaration surfaces + // as an actionable PENDING suggestion, and it is confirmable end to end. + const ql = makeQl([CRM_PACKAGE]); + ql.tables.sys_permission_set.push({ id: 'ps_1', name: 'crm_readonly', package_id: 'com.example.crm' }); + const deps = makeDeps(ql); + + const { suggestions, synced } = await listAudienceBindingSuggestions(deps, ADMIN_CTX, {}); + expect(synced.created).toBe(1); + expect(suggestions[0].status).toBe('pending'); + + // …and the pending→confirmed (observed) transition still fires when the + // binding is observed again. + ql.tables.sys_position_permission_set.push({ id: 'pps_1', position_id: 'pos_everyone', permission_set_id: 'ps_1' }); + const out = await syncAudienceBindingSuggestions(ql); + expect(out.confirmedObserved).toBe(1); expect(out.created).toBe(0); - expect(ql.tables.sys_audience_binding_suggestion).toHaveLength(0); + expect(ql.tables.sys_audience_binding_suggestion).toHaveLength(1); + expect(ql.tables.sys_audience_binding_suggestion[0].status).toBe('confirmed'); }); it('marks a pending suggestion confirmed when the binding appears out-of-band', async () => { diff --git a/packages/plugins/plugin-security/src/suggested-audience-bindings.ts b/packages/plugins/plugin-security/src/suggested-audience-bindings.ts index c3cd72366c..12d8679471 100644 --- a/packages/plugins/plugin-security/src/suggested-audience-bindings.ts +++ b/packages/plugins/plugin-security/src/suggested-audience-bindings.ts @@ -13,10 +13,14 @@ * every currently-declared `isDefault` set — boot-declared stack metadata * AND installed package manifests (which the registry updates at * `installPackage` time, so a runtime install is visible immediately, - * no reboot needed) — and reconciles the table: missing → pending row; - * binding already present → confirmed (observed); declaration gone - * (uninstall / flag dropped) → pending row pruned. It runs at boot, after - * a package-door `permission` publish, and on every list call. + * no reboot needed) — and reconciles the table: missing → pending row, or + * a `confirmed` (observed) row when the binding is already present; + * binding observed under an existing pending row → confirmed (observed); + * declaration gone (uninstall / flag dropped) → pending row pruned. It runs + * at boot, after a package-door `permission` publish, and on every list + * call. Every declaration is represented either way — an `isDefault` set + * the boot baseline auto-binds before the first sync is surfaced as + * confirmed/observed, never omitted (#7677). * - `confirmAudienceBindingSuggestion` creates the anchor binding **with the * caller's execution context**, so the ADR-0090 write gates do the real * enforcement: the D5/D9 audience-anchor gate (no high-privilege set on @@ -221,7 +225,25 @@ export async function syncAudienceBindingSuggestions( continue; } - if (bound) continue; // satisfied before ever being surfaced — nothing pending + // No row yet. A declaration that is ALREADY satisfied is recorded as + // `confirmed` (observed) rather than skipped — the same end state the + // pending→confirmed branch above reaches, just arrived at without ever + // passing through `pending`. [#7677] Skipping it entirely left the whole + // surface empty on stock: the security plugin binds the app's `isDefault` + // set to `everyone` at boot BEFORE the first sync runs, so "already bound" + // is the normal stock case, not the exception, and the declaration was + // only ever surfaced after someone deleted the binding by hand. + // + // `confirmed` and not `pending` because a bound declaration is not + // awaiting an admin decision: `pending` is the actionable-prompt state + // (the console panel lists exactly `status=pending` and offers + // confirm/dismiss, and both service methods 409 on anything else), so a + // pending row here would prompt an admin to "accept" a binding that + // already exists. `resolved_by` is left empty, which is precisely how the + // object schema defines an observed row: "Empty on a confirmed row means + // the binding was observed (e.g. bound at boot or by hand), not confirmed + // through the prompt." + const observed = bound; try { await ql.insert(SUGGESTION_OBJECT, { @@ -229,7 +251,8 @@ export async function syncAudienceBindingSuggestions( package_id: d.packageId, permission_set_name: d.set.name, anchor: d.anchor, - status: 'pending', + status: observed ? 'confirmed' : 'pending', + ...(observed ? { resolved_at: new Date().toISOString() } : {}), }, { context: SYSTEM_CTX }); out.created += 1; } catch { /* unique-index race with a concurrent sync — benign */ }