|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * #19249 — the `set_user_manager` row action on `sys_user`, pinned against the |
| 5 | + * three things about it that are decisions rather than code. |
| 6 | + * |
| 7 | + * `sys_user.manager_id` drives the approvals `manager` rung and the ADR-0057 |
| 8 | + * `own_and_reports` read scope, and `POST /api/v1/auth/admin/set-user-manager` |
| 9 | + * (#16678 Phase 3) is its only product write surface. This action is the |
| 10 | + * Console affordance that reaches it, and each assertion below exists because |
| 11 | + * the OBVIOUS edit at that spot is the wrong one: |
| 12 | + * |
| 13 | + * 1. **The write goes to the admin endpoint, never the generic data API.** |
| 14 | + * `sys_user` is `managedBy: 'better-auth'` and the ADR-0092 D2 |
| 15 | + * managed-update whitelist is `{name, image, locale}`, so a picker that |
| 16 | + * PATCHed `/api/v1/data/sys_user/:id` would be refused by the identity |
| 17 | + * write guard — correctly — and would read as a Console bug. |
| 18 | + * 2. **The `visible` predicate carries the directory-sync term and NOT the |
| 19 | + * self-service ownership term.** The three self-service identity actions |
| 20 | + * on this object spell `record.source != "idp_provisioned"` as one half of |
| 21 | + * `record.id == ctx.user.id && …`. Copying that predicate whole — the |
| 22 | + * natural thing to do, and what a reader of the card would do — would hide |
| 23 | + * an ADMIN action on someone else's row from every admin. The failure is |
| 24 | + * silent and fail-closed (#8990): the button simply is not offered. |
| 25 | + * 3. **No second copy of the server's refusals.** Self-assignment, cycle, |
| 26 | + * depth, cross-organization and directory-owned identity are all enforced |
| 27 | + * at the write, in one derivation (`applyUserManagerLink`), which the bulk |
| 28 | + * importer already routes onto rather than re-deriving. A client-side |
| 29 | + * duplicate is the drift, not the safety net — so this file pins that the |
| 30 | + * declaration reads exactly one record column and declares no predicate |
| 31 | + * for any of the other four refusals. |
| 32 | + * |
| 33 | + * The sweep in `action-predicate-sparse-face.test.ts` already covers this |
| 34 | + * action's predicate for the sparse-face guard; that is deliberately not |
| 35 | + * restated here. |
| 36 | + */ |
| 37 | + |
| 38 | +import { describe, expect, it } from 'vitest'; |
| 39 | +import { celEngine } from '@objectstack/formula'; |
| 40 | +import { SysUser } from './sys-user.object.js'; |
| 41 | + |
| 42 | +type AnyParam = Record<string, unknown>; |
| 43 | +type AnyAction = Record<string, unknown> & { name?: string; params?: AnyParam[] }; |
| 44 | + |
| 45 | +const actionsOf = (): AnyAction[] => ((SysUser.actions ?? []) as unknown as AnyAction[]); |
| 46 | + |
| 47 | +const ACTION = (() => { |
| 48 | + const found = actionsOf().find((a) => a.name === 'set_user_manager'); |
| 49 | + if (!found) throw new Error('sys_user declares no set_user_manager action'); |
| 50 | + return found; |
| 51 | +})(); |
| 52 | + |
| 53 | +/** |
| 54 | + * `defineObject` normalizes a CEL shorthand into a `{dialect, source}` |
| 55 | + * envelope at parse time, so the stored value is never the string the file |
| 56 | + * spells. Read through this or the assertions run against `undefined`. |
| 57 | + */ |
| 58 | +function sourceOf(raw: unknown): string | undefined { |
| 59 | + if (typeof raw === 'string') return raw; |
| 60 | + if (raw && typeof raw === 'object' && typeof (raw as { source?: unknown }).source === 'string') { |
| 61 | + return (raw as { source: string }).source; |
| 62 | + } |
| 63 | + return undefined; |
| 64 | +} |
| 65 | + |
| 66 | +/** Evaluate through the canonical engine; a fault is reported, never thrown. */ |
| 67 | +function evaluate(source: string, record: Record<string, unknown>): boolean | string { |
| 68 | + const r = celEngine.evaluate( |
| 69 | + { dialect: 'cel', source }, |
| 70 | + { record, user: { id: 'admin_1' }, extra: { features: {} } }, |
| 71 | + ); |
| 72 | + if (!r.ok) return `FAULT ${r.error.message.split('\n')[0].trim()}`; |
| 73 | + return typeof r.value === 'boolean' ? r.value : `NON-BOOLEAN ${JSON.stringify(r.value)}`; |
| 74 | +} |
| 75 | + |
| 76 | +describe('#19249 — sys_user.set_user_manager posts the admin endpoint', () => { |
| 77 | + it('targets the ruled route with the ruled record-id key', () => { |
| 78 | + expect(ACTION.type).toBe('api'); |
| 79 | + expect(ACTION.target).toBe('/api/v1/auth/admin/set-user-manager'); |
| 80 | + // The endpoint reads `userId` (and `user_id`) off the body; the row id is |
| 81 | + // injected under that key rather than collected from the user. |
| 82 | + expect(ACTION.recordIdParam).toBe('userId'); |
| 83 | + // POST is the default and the only method this endpoint is mounted for; |
| 84 | + // declaring PATCH/PUT here would 404 at the click. |
| 85 | + expect(ACTION.method === undefined || ACTION.method === 'POST').toBe(true); |
| 86 | + }); |
| 87 | + |
| 88 | + it('⛔ does NOT write manager_id through the generic data API', () => { |
| 89 | + // The whole prohibition on this card, expressed as a predicate over the |
| 90 | + // declaration: no data-plane target, and no declarative row write (which |
| 91 | + // runs on the data plane AS THE CALLER and would hit the same guard). |
| 92 | + expect(String(ACTION.target)).not.toContain('/data/'); |
| 93 | + expect(ACTION.operation).toBeUndefined(); |
| 94 | + expect(ACTION.patch).toBeUndefined(); |
| 95 | + }); |
| 96 | + |
| 97 | + it('surfaces on the row menu AND the record-detail header', () => { |
| 98 | + expect(ACTION.locations).toContain('list_item'); |
| 99 | + expect(ACTION.locations).toContain('record_header'); |
| 100 | + }); |
| 101 | + |
| 102 | + it('collects the manager as an inline sys_user lookup under the body key the endpoint reads', () => { |
| 103 | + const params = (ACTION.params ?? []) as AnyParam[]; |
| 104 | + expect(params).toHaveLength(1); |
| 105 | + const [p] = params; |
| 106 | + expect(p.name).toBe('managerId'); |
| 107 | + expect(p.type).toBe('lookup'); |
| 108 | + expect(p.reference).toBe('sys_user'); |
| 109 | + // INLINE, not field-backed: `{ field: 'manager_id' }` would inherit the |
| 110 | + // referenced field's metadata, and that field is `readonly: true` |
| 111 | + // (ADR-0092 D4 keeps it non-editable in the standard edit form). |
| 112 | + expect(p.field).toBeUndefined(); |
| 113 | + // Required, because the endpoint refuses an ABSENT or empty key by design |
| 114 | + // ("managerId is required — send null to clear the link, never omit the |
| 115 | + // key"). A dialog that could submit nothing would turn an ordinary click |
| 116 | + // into a 400 about a key the user never saw. |
| 117 | + expect(p.required).toBe(true); |
| 118 | + }); |
| 119 | + |
| 120 | + it('⛔ declares no second copy of the server-side refusals', () => { |
| 121 | + // Every refusal is enforced at the write and surfaces from there. What |
| 122 | + // would go wrong quietly is a client-side re-derivation drifting from the |
| 123 | + // endpoint's; so the predicate is pinned to the ONE column it reads, and |
| 124 | + // the declaration to having no other gate at all. |
| 125 | + const visible = sourceOf(ACTION.visible); |
| 126 | + expect(visible).toBeDefined(); |
| 127 | + const columns = new Set( |
| 128 | + [...visible!.matchAll(/record\.([a-z_][a-z0-9_]*)/gi)].map((m) => m[1]), |
| 129 | + ); |
| 130 | + expect([...columns]).toEqual(['source']); |
| 131 | + expect(ACTION.disabled).toBeUndefined(); |
| 132 | + // The manager chain, the organization screen and the self-assignment check |
| 133 | + // are the endpoint's; none of them is expressible — or declared — here. |
| 134 | + const declaration = JSON.stringify(ACTION); |
| 135 | + expect(declaration).not.toContain('ctx.user.id'); |
| 136 | + expect(declaration).not.toContain('manager_user_id'); |
| 137 | + expect(declaration).not.toContain('sys_business_unit'); |
| 138 | + }); |
| 139 | +}); |
| 140 | + |
| 141 | +describe('#19249 — the visible predicate: directory-sync term, NOT the self-service one', () => { |
| 142 | + const visible = (): string => { |
| 143 | + const source = sourceOf(ACTION.visible); |
| 144 | + if (!source) throw new Error('set_user_manager has no CEL source'); |
| 145 | + return source; |
| 146 | + }; |
| 147 | + |
| 148 | + it('is hidden for a directory-owned identity (the server answers 403 idp_provisioned)', () => { |
| 149 | + expect(evaluate(visible(), { source: 'idp_provisioned' })).toBe(false); |
| 150 | + }); |
| 151 | + |
| 152 | + it('⭐ is OFFERED to an admin on someone else\'s env-native row', () => { |
| 153 | + // The counter-direction, and the one that a verbatim copy of the |
| 154 | + // self-service predicate would break: this row is not the signed-in user. |
| 155 | + expect(evaluate(visible(), { id: 'someone_else', source: 'env_native' })).toBe(true); |
| 156 | + }); |
| 157 | + |
| 158 | + it('carries the same directory-sync term the self-service actions spell', () => { |
| 159 | + // Byte-identical term, so the two never drift into two spellings of one |
| 160 | + // rule — while the ownership half they also carry stays out (asserted |
| 161 | + // above by the offered-to-an-admin verdict). |
| 162 | + expect(visible()).toContain('record.source != "idp_provisioned"'); |
| 163 | + const selfService = actionsOf().find((a) => a.name === 'change_my_password'); |
| 164 | + expect(sourceOf(selfService?.visible)).toContain('record.source != "idp_provisioned"'); |
| 165 | + }); |
| 166 | + |
| 167 | + it('⛔ is NOT gated on features.admin, and that is deliberate', () => { |
| 168 | + // The `admin` flag hides buttons whose endpoint is "only wired when |
| 169 | + // `auth.plugins.admin` is enabled" — the 404-avoidance the block header on |
| 170 | + // the admin actions states. This route is not one of those: it is an |
| 171 | + // ObjectStack mount registered unconditionally beside `unlock-user` and |
| 172 | + // authorized by the ADR-0068 platform-admin gate, so gating it would hide |
| 173 | + // a WORKING affordance on every host that never opted into that plugin — |
| 174 | + // exactly the population #16678 measured as having no write surface for |
| 175 | + // this column at all. If this assertion is ever flipped, the registry |
| 176 | + // entry `PUBLIC_AUTH_FEATURES.admin.gatedInputs` must gain |
| 177 | + // `sys_user.actions.set_user_manager` in the same change, or |
| 178 | + // `feature-gate-guard.test.ts` goes red in its reverse direction. |
| 179 | + expect(ACTION.requiresFeature).toBeUndefined(); |
| 180 | + expect(visible()).not.toContain('features.'); |
| 181 | + }); |
| 182 | +}); |
0 commit comments