diff --git a/plugins/model-apps/AGENTS.md b/plugins/model-apps/AGENTS.md index 3a4801fd..109cc837 100644 --- a/plugins/model-apps/AGENTS.md +++ b/plugins/model-apps/AGENTS.md @@ -327,6 +327,34 @@ the pipeline and delegates each script's **behavioral spec** to the entries belo nothing. This is the F5 "convergence" mitigation: the build is additive (edits to existing artifacts aren't re-applied in place — teardown + rebuild to converge), and verify makes any resulting divergence **loud**. +- **`scripts/probe-persona.js` → `scripts/lib/persona-probe.js`** — read-only **authorization** probes run + AS each persona via Dataverse impersonation. `role-privileges` (above, in verify) compares metadata: it + proves the role HOLDS the declared privileges. Whether the persona can actually perform the operation + additionally depends on record ownership, business-unit placement, team membership, sharing, and + server-side plug-ins — none of which appear in `roleprivileges` — so this executes real reads and + reports what happens. Impersonation makes it human-free: effective privileges are the INTERSECTION of + caller and target, so a System Administrator driving it cannot mask a permission the persona lacks. + The principal comes from `personas[].assignTo.users[]`, which already holds `systemuserid` GUIDs — + exactly what the legacy `MSCRMCallerID` header takes, so the common case needs **no directory lookup and + no application user**; it upgrades to the preferred `CallerObjectId` when `azureactivedirectoryobjectid` + is readable. Three things carry the design: + (1) It probes the **negative** direction — for each persona it reads an entity another persona declares + and this one does not. An over-broad role is invisible from the inside because everything the user tries + succeeds, so it is only detectable by trying something that should fail. `appmodule` is never probed + negatively (the build injects it for every persona, so it would fail on every run). + (2) An empty `200` on a negative probe is **inconclusive, never a pass**: Dataverse returns 403 for *no + privilege* but a filtered 200 for *a narrower scope*, and an empty result is indistinguishable from + "authorized but the table is empty". Calling that a pass would manufacture confidence in the one + direction that matters. Inconclusive results do not fail the run but are always counted and listed, so + an all-inconclusive run cannot masquerade as clean. + (3) A **`WhoAmI` canary** runs first. The dangerous failure is not a 403 (loud) but the header being + accepted and IGNORED — every probe would then run as the signed-in admin and report false passes. + `WhoAmI` returns the effective user id, so comparing it to the impersonated id detects that silently. + A 403 there reports the real cause: the caller needs `prvActOnBehalfOfAnotherUser`, assigned **directly** + (a team-inherited grant does not satisfy it). Read-only by default; `--allow-mutations` only *plans* + write probes. **Scope limit, stated so results are not over-read:** this exercises the Web API, so it + says nothing about UCI navigation, form/control visibility, client script, the command bar, layout or + accessibility. A green run means the data operations are authorized, never that the app works. - **`scripts/ai-preflight.js`** — standalone preflight report: prints each AI feature's on/off status and the exact admin action needed (Power Platform Admin Center → Environments → Settings → Product → Features) for anything off. Never fails. The `ai-features` build phase calls this logic internally and @@ -428,6 +456,7 @@ references/ ← Shared reference docs localization.md ← Multi-language + RTL pattern (loaded conditionally) supported-dependencies.md ← Versioned package list for generated pages troubleshooting.md ← Deployment/runtime/env issues + persona-validation.md ← app-builder: probe-persona prerequisites, reading `inconclusive`, scope limit verified-icons.txt ← ~5000 Fluent UI icon names; Grep-validated by page-builder samples/ ← Example .tsx files (13 samples) plus app-builder spec samples scripts/ @@ -448,6 +477,7 @@ scripts/ download-model-app.js ← app-builder: pull a deployed app into an editable spec (edit flow) teardown-model-app.js ← app-builder: classifier-safe reverse-of-build teardown verify-model-app.js ← app-builder: reconcile the spec against the deployed app + probe-persona.js ← app-builder: run authorization probes AS each persona via Dataverse impersonation (read-only) preview-form.js ← app-builder: ASCII form wireframe for authoring review preview-app.js ← app-builder: ASCII whole-app design preview (data model + sitemap + forms + page-intents + design) write-app-spec-doc.js ← app-builder: renders the readable model-app-plan.md design doc from app-spec.json @@ -474,6 +504,7 @@ scripts/ spec-shape.js ← shared structural normalization for both authoring gates surface-resolver.js ← pure: resolve personas[].jobs[].surfaces[] to the spec artifacts that satisfy them role-privileges.js ← pure: declared persona privileges + subset comparison against a deployed role + persona-probe.js ← pure: plan/interpret impersonated authorization probes (allow + deny) per persona odata.js ← OData literal escaping helpers genpage-cli.js ← pac model genpage upload/list/download wrapper hydrate-spec.js ← reconstruct an App Spec from a deployed app (edit flow) @@ -865,6 +896,7 @@ az account set --subscription node scripts/check-auth.js --env # az token + WhoAmI preflight (pac optional; --require-pac for genpage) node scripts/build-model-app.js --env --spec @/app-spec.json [--sample-data --publish] --apply --verify node scripts/verify-model-app.js --env --spec @/app-spec.json +node scripts/probe-persona.js --env --spec @/app-spec.json # authorization AS each persona (read-only) node scripts/teardown-model-app.js --env --spec @/app-spec.json --apply ``` diff --git a/plugins/model-apps/CHANGELOG.md b/plugins/model-apps/CHANGELOG.md index f3ed73fe..fcd55125 100644 --- a/plugins/model-apps/CHANGELOG.md +++ b/plugins/model-apps/CHANGELOG.md @@ -30,6 +30,27 @@ smoke-eval assertion that could never pass live. serializers that hardcode 1033 with no caller override ([#455](https://github.com/microsoft/power-platform-skills/issues/455)). +- **`probe-persona.js` — authorization probes run AS each persona.** The + `role-privileges` check below compares metadata and stops there; whether a + persona can actually perform an operation also depends on record ownership, + business-unit placement, team membership, sharing and server-side plug-ins, + none of which appear in `roleprivileges`. This runs real reads under Dataverse + impersonation, so it needs no human and no application user: effective + privileges are the intersection of caller and target, and + `personas[].assignTo.users[]` already carries the `systemuserid` the legacy + `MSCRMCallerID` header takes (upgrading to `CallerObjectId` when the Entra + object id is readable). It also probes the **negative** direction — reading an + entity another persona declares and this one does not — because an over-broad + role is invisible from the inside, where everything the user tries succeeds. + An empty `200` on a negative probe is reported **inconclusive, never a pass** + (Dataverse answers "no privilege" with 403 but "narrower scope" with a + filtered 200, which is indistinguishable from an empty table), and a `WhoAmI` + canary runs first to catch the impersonation header being accepted and + silently IGNORED — which would otherwise run every probe as the signed-in + admin and report false passes. Read-only by default. It exercises the Web API, + so it says nothing about UCI navigation, form visibility, client script or + layout: a green run means the data operations are authorized, not that the app + works. - **`verify` now proves what a persona security role GRANTS, not just that it exists.** The `role` check only asserted a role row carrying the SDK ownership marker, so a role built with the wrong access — or one whose privilege write diff --git a/plugins/model-apps/references/persona-validation.md b/plugins/model-apps/references/persona-validation.md new file mode 100644 index 00000000..590570f4 --- /dev/null +++ b/plugins/model-apps/references/persona-validation.md @@ -0,0 +1,73 @@ +# Persona validation — what each persona can actually DO + +Loaded on demand by `/app-builder` Phase 3. Verifying a build proves the app matches its spec; this +page is about the separate question of whether each **persona** can actually work in it. + +## Two layers, deliberately separate + +| | `verify-model-app.js` → `role-privileges` | `probe-persona.js` | +|---|---|---| +| Proves | the role **holds** the declared privileges | the persona can **actually perform** the operation | +| Also depends on | nothing | record ownership, business unit, team membership, sharing, plug-ins | +| Cost | free — metadata reads during a verify that already runs | N × M round trips | +| Prerequisites | none | a test user + `prvActOnBehalfOfAnotherUser` | +| Answer shape | binary | can legitimately be **inconclusive** | +| So it is | part of the **build gate**, always on | **opt-in**, run when you want it | + +They are not redundant. A role can hold every declared privilege and still leave the persona unable +to work — depth interacts with who owns the records, which business unit they sit in, what teams the +user belongs to, what has been shared, and what server-side plug-ins reject. `roleprivileges` shows +none of that, so only executing a real operation answers it. + +Keeping the metadata check in the build gate is what makes it free and unconditional; moving it out +would restore the hole it was added to close — a role row exists, verify reports clean, and nothing +checks what it grants. + +## Running it + +Read-only. It changes nothing. + +```bash +node "${PLUGIN_ROOT}/scripts/probe-persona.js" --env --spec @/app-spec.json +``` + +`--allow-mutations` additionally *plans* create/write/delete probes. It does **not** execute them — +exercising a write to verify it needs fixture creation and cleanup, which is a separate design. + +## Prerequisites + +It reports clearly and exits rather than guessing when these are unmet: + +- the persona declares `assignTo.users[]` (already a `systemuserid`, which is what the impersonation + header takes — no directory lookup and no application user needed); +- the signed-in user holds **`prvActOnBehalfOfAnotherUser`**, assigned **directly** — a + team-inherited grant does not satisfy it. + +## Reading the output + +- **`pass`** — the operation behaved as declared. +- **`fail`** — a declared privilege did not work, or an entity the persona never declared *was* + readable (an over-broad role). +- **`inconclusive`** — the probe **proved nothing either way**. It is *not* a pass. Inconclusive + results do not fail the run, because they are genuine unknowns and failing on them would train you + to ignore the tool — but they are always counted, so an all-inconclusive run cannot look clean. + +The most common inconclusive is an empty `200` on a negative probe: Dataverse answers *"no privilege"* +with `403` but *"narrower scope"* with a filtered `200`, which is indistinguishable from an authorized +read of an empty table. Seed a row owned by another user to disambiguate. + +## Why it probes the negative direction + +For each persona it also reads an entity that **another** persona declares and this one does not. + +An over-broad role is invisible from the inside: every operation the user tries simply succeeds. It +can only be detected by trying something that *should* fail. `appmodule` is never probed negatively — +the build injects it for every persona, so it would report a failure on every run. + +## What a green run does NOT mean + +This exercises the **Web API**. It says nothing about UCI navigation, which form opens, field or +control visibility, client-side script, the command bar, layout, or accessibility. + +**A green run means the data operations are authorized — not that the app works.** Those still need a +browser pass or a human. diff --git a/plugins/model-apps/scripts/lib/persona-probe.js b/plugins/model-apps/scripts/lib/persona-probe.js new file mode 100644 index 00000000..ceddd604 --- /dev/null +++ b/plugins/model-apps/scripts/lib/persona-probe.js @@ -0,0 +1,278 @@ +// plugins/model-apps/scripts/lib/persona-probe.js +// PURE: plan and interpret impersonated authorization probes for each persona in an App Spec. +// +// WHY this exists. `role-privileges` proves the deployed role HOLDS the declared privileges — a +// metadata comparison. It cannot prove the persona can actually perform the operation: privilege +// depth interacts with record ownership, business-unit placement, team membership, sharing, and +// server-side plug-ins, none of which are visible in `roleprivileges`. This probe closes that gap by +// executing real Web API calls AS the persona and checking the outcome. +// +// Impersonation makes this cheap and human-free. The caller sends the target user's id on each +// request; effective privileges become the INTERSECTION of caller and target, so a System +// Administrator driving the probe cannot mask a permission the persona lacks: +// CallerObjectId -> the Entra object id (preferred) +// MSCRMCallerID -> the Dataverse systemuserid (legacy) +// The caller needs `prvActOnBehalfOfAnotherUser`, assigned DIRECTLY (a Team-inherited grant does not +// satisfy it). https://learn.microsoft.com/en-us/power-apps/developer/data-platform/webapi/impersonate-another-user-web-api +// +// WHAT THIS CANNOT DO — stated here so the report is never over-read. This exercises the Web API. +// It says nothing about UCI navigation, which form opens, control/field visibility, client-side +// script, the command bar, layout, accessibility, or whether a human can find the screen at all. A +// green probe run means "the data operations are authorized", never "the app works". +'use strict'; + +const { declaredPrivileges } = require('./role-privileges.js'); + +// Read is the only access we can exercise WITHOUT changing the environment, so it is the default. +// Everything else creates, mutates or destroys rows and is planned but not executed unless the +// caller opts in — a verification tool that silently writes to someone's environment is a trap. +const MUTATING_ACCESS = new Set(['create', 'write', 'delete', 'append', 'appendto', 'assign', 'share']); + +const isMutating = (access) => MUTATING_ACCESS.has(String(access || '').trim().toLowerCase()); + +// Entities the platform grants broadly and which therefore prove nothing as a NEGATIVE probe: a +// persona that never declares `appmodule` still reads it, because the build injects that privilege +// (see declaredPrivileges) and the platform needs it to open any app at all. +const NEVER_PROBE_DENIED = new Set(['appmodule']); + +/** + * Plan the probes for every persona in the spec. + * + * Two probe kinds, and the negative one is the point: + * - `expect: 'allow'` — an entity+access the persona DECLARES. Proves the grant works end to end. + * - `expect: 'deny'` — an entity another persona declares but THIS one does not. Proves isolation + * between personas, which is the failure nobody notices: an over-broad role + * looks perfect from the inside because everything the user tries succeeds. + * + * @param {object} spec App Spec (already migrated/validated by the caller) + * @param {object} [opts] + * @param {boolean} [opts.includeMutations=false] plan mutating probes too (still not executed here) + * @returns {{ probes: Array, warnings: string[] }} + */ +function planProbes(spec, opts = {}) { + const includeMutations = opts.includeMutations === true; + const personas = (spec && spec.personas) || []; + const warnings = []; + const probes = []; + + if (personas.length === 0) { + warnings.push('spec declares no personas — nothing to probe'); + return { probes, warnings }; + } + + // Declared privileges per persona, computed once: used for the positive probes AND to derive each + // persona's negative set by difference. + const declaredByPersona = new Map(); + for (const p of personas) { + const name = String((p && p.persona) || '').trim(); + if (!name) { + warnings.push('skipped a persona with no name'); + continue; + } + declaredByPersona.set(name, declaredPrivileges(p)); + } + + // Every entity ANY persona declares. The negative probes are drawn from this set rather than from + // the whole data model, because an entity nobody asked for tells us nothing about role design. + const allEntities = new Set(); + for (const declared of declaredByPersona.values()) { + for (const d of declared) allEntities.add(d.entity); + } + + for (const [persona, declared] of declaredByPersona) { + const ownEntities = new Set(declared.map((d) => d.entity)); + + for (const d of declared) { + if (isMutating(d.access) && !includeMutations) continue; + probes.push({ + persona, + entity: d.entity, + access: d.access, + scope: d.scope, + expect: 'allow', + mutating: isMutating(d.access), + reason: `persona declares ${d.access} on ${d.entity} at ${d.scope} scope`, + }); + } + + for (const entity of allEntities) { + if (ownEntities.has(entity) || NEVER_PROBE_DENIED.has(entity)) continue; + probes.push({ + persona, + entity, + access: 'read', + scope: null, + expect: 'deny', + mutating: false, + reason: `another persona declares ${entity}; this one does not, so it should not be readable`, + }); + } + } + + if (!includeMutations && probes.every((p) => !p.mutating)) { + const skipped = [...declaredByPersona.values()].flat().filter((d) => isMutating(d.access)).length; + if (skipped > 0) { + // Say what actually happened. These privileges were never turned into probes at all, so + // "planned but NOT executed" misdescribes the run for the operator reading it — it implies a + // plan exists and something stopped it, when the read-only default simply excludes them. + warnings.push(`${skipped} mutating privilege(s) were NOT probed — this is a read-only run, so nothing was verified about create/write/delete/append (pass --allow-mutations to probe them)`); + } + } + + return { probes, warnings }; +} + +/** + * Turn one raw HTTP outcome into a finding. + * + * @param {object} probe from planProbes + * @param {object} outcome { status:number|null, rowCount:number|null, error?:string } + * @returns {{ probe, result: 'pass'|'fail'|'inconclusive', detail: string }} + * + * The `inconclusive` result is the load-bearing part. Dataverse expresses "you may not see this" + * two different ways: + * - NO privilege at all -> 403 Forbidden + * - privilege at a NARROWER scope -> 200 OK with the rows filtered out + * So an empty 200 on a negative probe is genuinely ambiguous: it looks identical to "authorized, but + * this table happens to be empty". Reporting that as a pass would manufacture false confidence in + * exactly the direction that matters, so it is reported as inconclusive and the operator is told + * what would disambiguate it (seed a row owned by someone else). + */ +function interpretOutcome(probe, outcome) { + const status = outcome && outcome.status; + const rowCount = outcome && outcome.rowCount; + const finding = (result, detail) => ({ probe, result, detail }); + + // A transport-level failure is never evidence about authorization. + if (outcome && outcome.error && status == null) { + return finding('inconclusive', `request failed before a status was returned: ${outcome.error}`); + } + + if (probe.expect === 'allow') { + if (status === 403) return finding('fail', 'denied (403) but the persona declares this privilege'); + if (status === 401) return finding('inconclusive', 'unauthorized (401) — impersonation or auth problem, not a role finding'); + if (status === 404) return finding('fail', 'not found (404) — the entity set does not exist for this persona'); + if (typeof status === 'number' && status >= 200 && status < 300) { + // A scoped read legitimately returns zero rows; that is not a failure of the grant. + return finding('pass', rowCount === 0 ? 'authorized (no rows visible at this scope)' : 'authorized'); + } + return finding('inconclusive', `unexpected status ${status}`); + } + + // expect === 'deny' + if (status === 403) return finding('pass', 'correctly denied (403)'); + if (typeof status === 'number' && status >= 200 && status < 300) { + if (rowCount > 0) { + return finding('fail', `readable (${rowCount} row(s) visible) but no job declares this entity — the role is broader than the spec`); + } + return finding( + 'inconclusive', + 'returned 200 with no rows: cannot distinguish "denied by scope" from "authorized but empty". Seed a row owned by another user to disambiguate.', + ); + } + if (status === 401) return finding('inconclusive', 'unauthorized (401) — impersonation or auth problem, not a role finding'); + return finding('inconclusive', `unexpected status ${status}`); +} + +/** + * Roll findings up into a report. + * `ok` is false when anything FAILED. Inconclusive results do not fail the run — they are genuine + * unknowns, and failing on them would train the operator to ignore the tool — but they are counted + * and listed so an all-inconclusive run cannot masquerade as a clean one. + */ +function summarize(findings) { + const counts = { pass: 0, fail: 0, inconclusive: 0 }; + for (const f of findings) counts[f.result] = (counts[f.result] || 0) + 1; + return { + ok: counts.fail === 0, + counts, + total: findings.length, + failures: findings.filter((f) => f.result === 'fail'), + inconclusive: findings.filter((f) => f.result === 'inconclusive'), + }; +} + +/** + * Execute planned probes against injected IO. Kept here rather than in the CLI so the orchestration + * — principal resolution, entity-set resolution, error containment — is testable without a network, + * mirroring how `verifySpec` takes an injected reader. + * + * @param {Array} probes from planProbes + * @param {object} io + * principalFor(persona) -> { header:'CallerObjectId'|'MSCRMCallerID', value:string } | null + * entitySetName(entity) -> Promise (the OData collection name, e.g. co_workorders) + * readOne(entitySet, hdr)-> Promise<{ status, rowCount, error? }> + * @returns {Promise} findings + * + * Every failure mode here degrades to `inconclusive` rather than `fail`. A probe that could not be + * RUN proves nothing about the role, and reporting it as a role failure would send the operator to + * fix a security role when the real problem is a missing test user or an unresolvable entity. + */ +async function executeProbes(probes, io) { + const findings = []; + // One metadata read per entity, not per probe — the negative probes alone are O(personas × entities). + const setNameCache = new Map(); + const principalCache = new Map(); + + for (const probe of probes) { + // A mutating privilege cannot be proven by a read, and `readOne` is the only operation this + // executor performs. Running one anyway would report `write`/`create`/`delete` as PASS on the + // strength of a successful GET — a false pass on the privilege that matters most, and on the + // exact path a maker opts into with --allow-mutations. `planProbes` includes them so the report + // shows what WOULD be exercised; executing them needs fixture creation and cleanup, which is a + // separate design. Until then they are reported as proved-nothing, never as passes. + if (probe.mutating) { + findings.push({ + probe, + result: 'inconclusive', + detail: `planned only — a ${probe.access} privilege cannot be proven by a read, and mutating probes are not executed`, + }); + continue; + } + + if (!principalCache.has(probe.persona)) { + principalCache.set(probe.persona, io.principalFor(probe.persona)); + } + const principal = principalCache.get(probe.persona); + if (!principal) { + findings.push({ + probe, + result: 'inconclusive', + detail: `no test user for persona '${probe.persona}' — declare assignTo.users[] or assign the role to a user`, + }); + continue; + } + + if (!setNameCache.has(probe.entity)) { + let name = null; + try { + name = await io.entitySetName(probe.entity); + } catch (err) { + name = null; + void err; + } + setNameCache.set(probe.entity, name); + } + const entitySet = setNameCache.get(probe.entity); + if (!entitySet) { + findings.push({ + probe, + result: 'inconclusive', + detail: `could not resolve the entity set name for '${probe.entity}'`, + }); + continue; + } + + let outcome; + try { + outcome = await io.readOne(entitySet, { [principal.header]: principal.value }); + } catch (err) { + outcome = { status: null, rowCount: null, error: (err && err.message) || String(err) }; + } + findings.push(interpretOutcome(probe, outcome)); + } + + return findings; +} + +module.exports = { planProbes, interpretOutcome, executeProbes, summarize, isMutating }; diff --git a/plugins/model-apps/scripts/probe-persona.js b/plugins/model-apps/scripts/probe-persona.js new file mode 100644 index 00000000..a1473f5c --- /dev/null +++ b/plugins/model-apps/scripts/probe-persona.js @@ -0,0 +1,223 @@ +#!/usr/bin/env node +'use strict'; +// probe-persona: run authorization probes AS each persona, using Dataverse impersonation. +// +// `role-privileges` (in verify) proves the deployed role HOLDS the declared privileges. That is a +// metadata comparison and it stops there. Whether the persona can actually perform the operation +// also depends on record ownership, business-unit placement, team membership, sharing, and +// server-side plug-ins — none of which appear in `roleprivileges`. This probe executes real reads as +// the persona and reports what actually happens. +// +// It also probes the NEGATIVE direction, which nothing else does: for each persona it reads an +// entity that some OTHER persona declares and this one does not. An over-broad role is invisible +// from the inside — every operation the user tries succeeds — so it is only detectable by trying +// something that should fail. +// +// Usage: +// node probe-persona.js --env --spec @/app-spec.json +// [--workspace ] [--allow-mutations] +// +// Read-only by default: only `read` privileges are executed. `--allow-mutations` additionally PLANS +// create/write/delete probes; they are reported as planned but still not executed, because writing +// to someone's environment to verify it is a trap that belongs behind its own explicit design. +// +// Output: { ok, personas, counts, failures, inconclusive, warnings } + +const fs = require('node:fs'); +const path = require('node:path'); +const { parseArgs, readJsonArg, emitResult } = require('./lib/dataverse-auth.js'); +const { createAzHttpClient } = require('./lib/sdk-http-client.js'); +const { migrateAppSpec, validateAppSpec } = require('./lib/app-spec.js'); +const { planProbes, executeProbes, summarize } = require('./lib/persona-probe.js'); + +const API = 'api/data/v9.2'; + +function makeProvision(env, workspaceDir) { + const { createMakerSdk } = require('./vendor/cds-maker-sdk.cjs'); + const httpClient = createAzHttpClient(env); + fs.mkdirSync(workspaceDir, { recursive: true }); + const sdk = createMakerSdk({ workspacePath: workspaceDir, instanceUrl: env, httpClient }); + sdk.initWorkspace(); + return { sdk, httpClient }; +} + +// `WhoAmI` under impersonation is the canary for this whole tool. +// +// The dangerous failure is NOT a 403 — that is loud. It is the header being accepted and IGNORED: +// every probe would then run as the (System Administrator) caller, every allow-probe would pass, +// every deny-probe would report the role as over-broad or come back readable, and the run would look +// authoritative while proving nothing. `WhoAmI` returns the EFFECTIVE user id, so comparing it to +// the impersonated id detects that silently. +// https://learn.microsoft.com/en-us/power-apps/developer/data-platform/webapi/reference/whoami +// +// `expectedSystemUserId` is the target's `systemuserid`, which is directly comparable to +// `WhoAmI().UserId`. It is checked FIRST and is the only positive proof, because "effective == +// caller" is NOT by itself evidence of a dropped header: a persona's test user is allowed to BE the +// signed-in user (a single-account dev environment, or an admin validating their own persona), and +// treating that as a failure would block a legitimate configuration. Note the target is always +// known — `assignTo.users[]` carries a systemuserid even when the request upgrades to +// `CallerObjectId`, which sends the Entra object id instead. +async function checkImpersonation(httpClient, apiRoot, header, value, expectedSystemUserId) { + const bare = await httpClient.get(`${apiRoot}/WhoAmI()`); + if (!bare || bare.status < 200 || bare.status >= 300) { + return { ok: false, detail: `WhoAmI failed as the signed-in user (status ${bare && bare.status}) — check --env and az login` }; + } + const callerId = String((bare.body && bare.body.UserId) || '').toLowerCase(); + + const asUser = await httpClient.get(`${apiRoot}/WhoAmI()`, { headers: { [header]: value } }); + if (asUser && asUser.status === 403) { + return { ok: false, detail: 'impersonation refused (403) — the signed-in user needs prvActOnBehalfOfAnotherUser ("Act on Behalf of Another User"), assigned DIRECTLY and not inherited from a team' }; + } + if (!asUser || asUser.status < 200 || asUser.status >= 300) { + return { ok: false, detail: `impersonated WhoAmI failed (status ${asUser && asUser.status})` }; + } + const effectiveId = String((asUser.body && asUser.body.UserId) || '').toLowerCase(); + const expected = String(expectedSystemUserId || '').toLowerCase(); + + // Positive proof: the request really ran as the target. Correct even when the target IS the caller. + if (expected && effectiveId && effectiveId === expected) return { ok: true, effectiveId }; + + if (effectiveId && callerId && effectiveId === callerId) { + return expected + ? { ok: false, detail: `the ${header} header was accepted but IGNORED — the request ran as the signed-in user, not as the target persona user (${expected}); every probe would report false passes` } + // No target to compare against, so this is the older, weaker heuristic: it cannot tell a + // dropped header apart from a persona whose test user is the signed-in user. + : { ok: false, detail: `the ${header} header appears to have been IGNORED — the request ran as the signed-in user, and no target systemuserid was available to confirm otherwise` }; + } + if (expected && effectiveId && effectiveId !== expected) { + return { ok: false, detail: `the ${header} header resolved to an unexpected principal (${effectiveId}, expected ${expected}) — probes would describe the wrong user` }; + } + return { ok: true, effectiveId }; +} + +// Resolve the principal to impersonate for a persona. +// +// `personas[].assignTo.users[]` already carries Dataverse `systemuserid` GUIDs, which is exactly what +// the legacy `MSCRMCallerID` header takes — so in the common case no directory lookup is needed at +// all. `CallerObjectId` (the Entra object id) is the documented preference, so try to upgrade to it +// with one read of `azureactivedirectoryobjectid` and fall back when that is unavailable. +function makePrincipalResolver(spec, sdk, cache) { + return (personaName) => { + const persona = ((spec && spec.personas) || []).find((p) => p && p.persona === personaName); + const users = (persona && persona.assignTo && persona.assignTo.users) || []; + const systemUserId = users.find((u) => typeof u === 'string' && u.trim()); + if (!systemUserId) return null; + const oid = cache.get(String(systemUserId).toLowerCase()); + // `systemUserId` is carried alongside the header regardless of which header is used: the + // impersonation canary compares it to `WhoAmI().UserId`, and under `CallerObjectId` the header + // value itself is an Entra object id that cannot be compared to a systemuserid. + return oid + ? { header: 'CallerObjectId', value: oid, systemUserId: String(systemUserId).trim() } + : { header: 'MSCRMCallerID', value: String(systemUserId).trim(), systemUserId: String(systemUserId).trim() }; + }; +} + +async function loadObjectIds(sdk, spec) { + // One query for every declared test user; a miss simply leaves the persona on MSCRMCallerID. + const ids = new Set(); + for (const p of (spec && spec.personas) || []) { + for (const u of (p && p.assignTo && p.assignTo.users) || []) { + if (typeof u === 'string' && u.trim()) ids.add(u.trim().toLowerCase()); + } + } + const cache = new Map(); + for (const id of ids) { + try { + const rows = await sdk.queryRecords('systemuser', { + select: ['systemuserid', 'azureactivedirectoryobjectid'], + filter: `systemuserid eq ${id}`, // Edm.Guid — UNQUOTED + top: 1, + }); + const oid = rows && rows[0] && rows[0].azureactivedirectoryobjectid; + if (oid) cache.set(id, String(oid)); + } catch { + // A missing/unreadable user is not fatal: the probe falls back to MSCRMCallerID, and a truly + // absent user surfaces later as an inconclusive finding naming the persona. + } + } + return cache; +} + +async function main() { + const { positional, flags } = parseArgs(process.argv.slice(2)); + const env = typeof flags.env === 'string' ? flags.env : undefined; + const specArg = typeof flags.spec === 'string' ? flags.spec : positional[0]; + if (!env || !specArg) { + process.stderr.write('Usage: node probe-persona.js --env --spec @/app-spec.json [--allow-mutations]\n'); + process.exit(1); + } + + const specPath = path.resolve(specArg.startsWith('@') ? specArg.slice(1) : specArg); + const spec = migrateAppSpec(readJsonArg('@' + specPath)); + const v = validateAppSpec(spec, { profile: 'deploy' }); + if (!v.ok) { emitResult(false, { ok: false, errors: v.errors }); return; } + + const workspaceDir = typeof flags.workspace === 'string' + ? flags.workspace + : path.join(path.dirname(specPath), '.maker-workspace'); + const apiRoot = `${String(env).replace(/\/+$/, '')}/${API}`; + const { sdk, httpClient } = makeProvision(env, workspaceDir); + + const { probes, warnings } = planProbes(spec, { includeMutations: flags['allow-mutations'] === true }); + if (probes.length === 0) { emitResult(true, { ok: true, probes: 0, warnings }); return; } + + const oidCache = await loadObjectIds(sdk, spec); + const principalFor = makePrincipalResolver(spec, sdk, oidCache); + + // Preflight once, on the first persona that actually has a principal. Impersonation is an + // environment-wide capability, so a second check would cost a round trip and tell us nothing new. + const firstPrincipal = [...new Set(probes.map((p) => p.persona))].map(principalFor).find(Boolean); + if (!firstPrincipal) { + emitResult(false, { + ok: false, + probes: 0, + warnings: [...warnings, 'no persona declares assignTo.users[], so there is no principal to impersonate — nothing could be probed'], + }); + return; + } + const pre = await checkImpersonation(httpClient, apiRoot, firstPrincipal.header, firstPrincipal.value, firstPrincipal.systemUserId); + if (!pre.ok) { emitResult(false, { ok: false, preflight: pre.detail, warnings }); return; } + + const findings = await executeProbes(probes, { + principalFor, + entitySetName: async (entity) => { + const meta = await sdk.fetchEntityMetadata(String(entity).toLowerCase()); + return (meta && (meta.entitySetName || meta.EntitySetName)) || null; + }, + // `$top=1` is enough: we are testing authorization, not paging. `rowCount` only needs to + // distinguish "some rows visible" from "none", which is what makes a negative probe conclusive. + readOne: async (entitySet, headers) => { + const res = await httpClient.get(`${apiRoot}/${entitySet}?$top=1`, { headers }); + const value = res && res.body && res.body.value; + return { + status: res && res.status, + rowCount: Array.isArray(value) ? value.length : null, + }; + }, + }); + + const s = summarize(findings); + for (const f of findings) { + const mark = f.result === 'pass' ? '✓' : f.result === 'fail' ? '✗' : '?'; + process.stderr.write(` ${mark} [${f.probe.persona}] ${f.probe.expect} ${f.probe.access} ${f.probe.entity} — ${f.detail}\n`); + } + process.stderr.write(`\n${s.ok ? '✓ probe PASS' : `✗ probe FAIL — ${s.counts.fail} failing`} (${s.counts.pass} pass, ${s.counts.fail} fail, ${s.counts.inconclusive} inconclusive)\n`); + if (s.counts.inconclusive > 0) { + process.stderr.write(' note: inconclusive probes proved nothing either way — they are not passes.\n'); + } + + emitResult(s.ok, { + ok: s.ok, + counts: s.counts, + total: s.total, + failures: s.failures.map((f) => `${f.probe.persona}:${f.probe.entity}:${f.probe.access} (${f.detail})`), + inconclusive: s.inconclusive.map((f) => `${f.probe.persona}:${f.probe.entity}:${f.probe.access} (${f.detail})`), + warnings, + }); +} + +if (require.main === module) { + main().catch((err) => emitResult(false, err)); +} + +module.exports = { checkImpersonation, makePrincipalResolver, main }; diff --git a/plugins/model-apps/scripts/tests/persona-probe.test.js b/plugins/model-apps/scripts/tests/persona-probe.test.js new file mode 100644 index 00000000..14988270 --- /dev/null +++ b/plugins/model-apps/scripts/tests/persona-probe.test.js @@ -0,0 +1,404 @@ +'use strict'; +const { test } = require('node:test'); +const assert = require('node:assert'); + +const { planProbes, interpretOutcome, summarize, isMutating } = require('../lib/persona-probe.js'); + +// A two-persona spec: Dispatcher touches workorder + technician, Technician only workorder. +// That asymmetry is what makes the negative probes meaningful. +function spec() { + return { + personas: [ + { + persona: 'Dispatcher', + jobs: [ + { name: 'Assign work', privileges: [ + { entity: 'co_workorder', access: ['read', 'write'], scope: 'businessUnit' }, + { entity: 'co_technician', access: ['read'], scope: 'organization' }, + ] }, + ], + }, + { + persona: 'Technician', + jobs: [ + { name: 'Do work', privileges: [ + { entity: 'co_workorder', access: ['read'], scope: 'user' }, + ] }, + ], + }, + ], + }; +} + +const find = (probes, persona, entity, expect) => + probes.find((p) => p.persona === persona && p.entity === entity && p.expect === expect); + +test('plans an allow probe for each declared read privilege', () => { + const { probes } = planProbes(spec()); + assert.ok(find(probes, 'Dispatcher', 'co_workorder', 'allow'), 'dispatcher workorder read'); + assert.ok(find(probes, 'Dispatcher', 'co_technician', 'allow'), 'dispatcher technician read'); + assert.ok(find(probes, 'Technician', 'co_workorder', 'allow'), 'technician workorder read'); +}); + +test('plans a DENY probe for an entity another persona declares but this one does not', () => { + // The whole point: Technician never declares co_technician, so it must not be readable. + const { probes } = planProbes(spec()); + const deny = find(probes, 'Technician', 'co_technician', 'deny'); + assert.ok(deny, 'expected a deny probe for Technician -> co_technician'); + assert.strictEqual(deny.access, 'read'); + assert.match(deny.reason, /another persona declares/); +}); + +test('never plans a deny probe for an entity the persona DOES declare', () => { + const { probes } = planProbes(spec()); + assert.strictEqual(find(probes, 'Dispatcher', 'co_workorder', 'deny'), undefined); + assert.strictEqual(find(probes, 'Technician', 'co_workorder', 'deny'), undefined); +}); + +test('never plans a deny probe for appmodule', () => { + // The build injects appmodule read for every persona, so a negative result would be a false + // finding on every single run. + const { probes } = planProbes(spec()); + assert.strictEqual(probes.some((p) => p.entity === 'appmodule' && p.expect === 'deny'), false); +}); + +test('mutating privileges are planned only when opted in', () => { + const readOnly = planProbes(spec()); + assert.strictEqual(readOnly.probes.some((p) => p.mutating), false, 'read-only run must plan no mutations'); + assert.match(readOnly.warnings.join(' '), /were NOT probed/); + // The warning must not claim a plan existed — in a read-only run these are never turned into + // probes at all, and "planned but not executed" misdescribes that to an operator. + assert.doesNotMatch(readOnly.warnings.join(' '), /planned/i); + + const withMutations = planProbes(spec(), { includeMutations: true }); + const write = withMutations.probes.find((p) => p.entity === 'co_workorder' && p.access === 'write'); + assert.ok(write, 'expected the declared write privilege to be planned'); + assert.strictEqual(write.mutating, true); +}); + +test('a spec with no personas warns instead of throwing', () => { + const r = planProbes({ personas: [] }); + assert.deepStrictEqual(r.probes, []); + assert.match(r.warnings.join(' '), /no personas/); +}); + +test('isMutating classifies the access tokens', () => { + assert.strictEqual(isMutating('read'), false); + for (const a of ['create', 'write', 'delete', 'append', 'appendTo', 'assign', 'share']) { + assert.strictEqual(isMutating(a), true, `${a} should be mutating`); + } +}); + +// ── interpretOutcome ───────────────────────────────────────────────────────────────────────────── + +const allow = { persona: 'P', entity: 'e', access: 'read', expect: 'allow' }; +const deny = { persona: 'P', entity: 'e', access: 'read', expect: 'deny' }; + +test('allow: 2xx passes, 403 fails', () => { + assert.strictEqual(interpretOutcome(allow, { status: 200, rowCount: 3 }).result, 'pass'); + assert.strictEqual(interpretOutcome(allow, { status: 403 }).result, 'fail'); +}); + +test('allow: an empty 200 still passes — a scoped read legitimately sees nothing', () => { + const f = interpretOutcome(allow, { status: 200, rowCount: 0 }); + assert.strictEqual(f.result, 'pass'); + assert.match(f.detail, /no rows visible/); +}); + +test('deny: 403 passes, visible rows fail', () => { + assert.strictEqual(interpretOutcome(deny, { status: 403 }).result, 'pass'); + const f = interpretOutcome(deny, { status: 200, rowCount: 2 }); + assert.strictEqual(f.result, 'fail'); + assert.match(f.detail, /broader than the spec/); +}); + +test('deny: an EMPTY 200 is inconclusive, never a pass', () => { + // The trap this guards: "denied by scope" and "authorized but the table is empty" are the same + // response. Calling it a pass would manufacture confidence in the one direction that matters. + const f = interpretOutcome(deny, { status: 200, rowCount: 0 }); + assert.strictEqual(f.result, 'inconclusive'); + assert.match(f.detail, /cannot distinguish/); +}); + +test('401 is inconclusive on both directions — it is an auth problem, not a role finding', () => { + assert.strictEqual(interpretOutcome(allow, { status: 401 }).result, 'inconclusive'); + assert.strictEqual(interpretOutcome(deny, { status: 401 }).result, 'inconclusive'); +}); + +test('a transport failure with no status is inconclusive, not a fail', () => { + const f = interpretOutcome(allow, { status: null, error: 'ETIMEDOUT' }); + assert.strictEqual(f.result, 'inconclusive'); + assert.match(f.detail, /ETIMEDOUT/); +}); + +test('allow: 404 fails — the entity set is not reachable for this persona', () => { + assert.strictEqual(interpretOutcome(allow, { status: 404 }).result, 'fail'); +}); + +// ── summarize ──────────────────────────────────────────────────────────────────────────────────── + +test('summarize fails only on failures, but still surfaces inconclusives', () => { + const s = summarize([ + { result: 'pass' }, { result: 'pass' }, { result: 'inconclusive' }, + ]); + assert.strictEqual(s.ok, true, 'inconclusive alone must not fail the run'); + assert.strictEqual(s.counts.inconclusive, 1); + assert.strictEqual(s.inconclusive.length, 1); + + const bad = summarize([{ result: 'pass' }, { result: 'fail' }]); + assert.strictEqual(bad.ok, false); + assert.strictEqual(bad.failures.length, 1); +}); + +test('summarize counts every result so an all-inconclusive run cannot look clean', () => { + const s = summarize([{ result: 'inconclusive' }, { result: 'inconclusive' }]); + assert.strictEqual(s.ok, true); + assert.strictEqual(s.counts.pass, 0); + assert.strictEqual(s.total, 2); + assert.strictEqual(s.inconclusive.length, 2, 'caller can see the run proved nothing'); +}); + +// ── executeProbes ──────────────────────────────────────────────────────────────────────────────── + +const { executeProbes } = require('../lib/persona-probe.js'); + +function io(overrides = {}) { + return { + principalFor: () => ({ header: 'MSCRMCallerID', value: 'user-1' }), + entitySetName: async (e) => `${e}s`, + readOne: async () => ({ status: 200, rowCount: 1 }), + ...overrides, + }; +} + +const oneAllow = [{ persona: 'P', entity: 'co_workorder', access: 'read', expect: 'allow' }]; + +test('executeProbes sends the impersonation header for the persona', async () => { + const calls = []; + await executeProbes(oneAllow, io({ readOne: async (set, headers) => { calls.push({ set, headers }); return { status: 200, rowCount: 1 }; } })); + assert.strictEqual(calls[0].set, 'co_workorders'); + assert.deepStrictEqual(calls[0].headers, { MSCRMCallerID: 'user-1' }); +}); + +test('executeProbes uses CallerObjectId when the principal supplies it', async () => { + const calls = []; + await executeProbes(oneAllow, io({ + principalFor: () => ({ header: 'CallerObjectId', value: 'entra-oid' }), + readOne: async (set, headers) => { calls.push(headers); return { status: 200, rowCount: 1 }; }, + })); + assert.deepStrictEqual(calls[0], { CallerObjectId: 'entra-oid' }); +}); + +test('a persona with no test user is inconclusive, never a failure', async () => { + // Reporting "no test user" as a role failure would send the operator to edit a security role that + // is probably fine. + const f = await executeProbes(oneAllow, io({ principalFor: () => null })); + assert.strictEqual(f[0].result, 'inconclusive'); + assert.match(f[0].detail, /no test user/); +}); + +test('an unresolvable entity set is inconclusive', async () => { + const f = await executeProbes(oneAllow, io({ entitySetName: async () => null })); + assert.strictEqual(f[0].result, 'inconclusive'); + assert.match(f[0].detail, /entity set name/); +}); + +test('a throwing entitySetName is contained, not propagated', async () => { + const f = await executeProbes(oneAllow, io({ entitySetName: async () => { throw new Error('metadata boom'); } })); + assert.strictEqual(f[0].result, 'inconclusive'); +}); + +test('a throwing readOne becomes an inconclusive finding carrying the message', async () => { + const f = await executeProbes(oneAllow, io({ readOne: async () => { throw new Error('ECONNRESET'); } })); + assert.strictEqual(f[0].result, 'inconclusive'); + assert.match(f[0].detail, /ECONNRESET/); +}); + +test('entity-set and principal lookups are cached across probes', async () => { + // The negative probes are O(personas x entities), so an uncached metadata read per probe would + // multiply the run time for no benefit. + let setLookups = 0; + let principalLookups = 0; + const probes = [ + { persona: 'P', entity: 'co_workorder', access: 'read', expect: 'allow' }, + { persona: 'P', entity: 'co_workorder', access: 'read', expect: 'deny' }, + { persona: 'P', entity: 'co_workorder', access: 'read', expect: 'allow' }, + ]; + await executeProbes(probes, io({ + entitySetName: async (e) => { setLookups++; return `${e}s`; }, + principalFor: () => { principalLookups++; return { header: 'MSCRMCallerID', value: 'u' }; }, + })); + assert.strictEqual(setLookups, 1, 'entity set resolved once'); + assert.strictEqual(principalLookups, 1, 'principal resolved once per persona'); +}); + +test('executeProbes returns one finding per probe, in order', async () => { + const probes = [ + { persona: 'P', entity: 'a', access: 'read', expect: 'allow' }, + { persona: 'P', entity: 'b', access: 'read', expect: 'deny' }, + ]; + const f = await executeProbes(probes, io({ readOne: async (set) => (set === 'as' ? { status: 200, rowCount: 1 } : { status: 403 }) })); + assert.strictEqual(f.length, 2); + assert.strictEqual(f[0].result, 'pass'); + assert.strictEqual(f[1].result, 'pass'); + assert.strictEqual(f[1].probe.entity, 'b'); +}); + +// ── probe-persona CLI seams ────────────────────────────────────────────────────────────────────── + +const { checkImpersonation, makePrincipalResolver } = require('../probe-persona.js'); + +const ROOT = 'https://contoso.crm.dynamics.com/api/data/v9.2'; + +function httpStub(responses) { + const calls = []; + return { + calls, + get: async (url, options) => { + calls.push({ url, headers: (options && options.headers) || null }); + const r = responses.shift(); + if (typeof r === 'function') return r(); + return r; + }, + }; +} + +test('checkImpersonation passes when the effective user differs from the caller', async () => { + const http = httpStub([ + { status: 200, body: { UserId: 'CALLER-1' } }, + { status: 200, body: { UserId: 'TARGET-9' } }, + ]); + const r = await checkImpersonation(http, ROOT, 'MSCRMCallerID', 'target-9'); + assert.strictEqual(r.ok, true); + assert.deepStrictEqual(http.calls[1].headers, { MSCRMCallerID: 'target-9' }); +}); + +test('checkImpersonation FAILS when the header is accepted but ignored', async () => { + // The most dangerous failure mode for this tool: the run would execute entirely as the signed-in + // System Administrator, every allow-probe would pass, and the report would look authoritative + // while proving nothing at all. + const http = httpStub([ + { status: 200, body: { UserId: 'CALLER-1' } }, + { status: 200, body: { UserId: 'caller-1' } }, // same user, different casing + ]); + const r = await checkImpersonation(http, ROOT, 'CallerObjectId', 'oid-9', 'target-9'); + assert.strictEqual(r.ok, false); + assert.match(r.detail, /accepted but IGNORED/); +}); + +test('checkImpersonation PASSES when the persona test user IS the signed-in user', async () => { + // A persona whose test user is the caller is a legitimate configuration — a single-account dev + // environment, or an admin validating their own persona. Judging the header purely by + // "effective == caller" false-fails that setup and blocks the tool for no reason. The target + // systemuserid is the real oracle, and it is always known because assignTo.users[] carries it. + const http = httpStub([ + { status: 200, body: { UserId: 'CALLER-1' } }, + { status: 200, body: { UserId: 'CALLER-1' } }, + ]); + const r = await checkImpersonation(http, ROOT, 'MSCRMCallerID', 'caller-1', 'caller-1'); + assert.strictEqual(r.ok, true, 'impersonating yourself is valid, not a dropped header'); + assert.strictEqual(r.effectiveId, 'caller-1'); +}); + +test('checkImpersonation FAILS when the effective user is neither the caller nor the target', async () => { + const http = httpStub([ + { status: 200, body: { UserId: 'CALLER-1' } }, + { status: 200, body: { UserId: 'SOMEONE-ELSE' } }, + ]); + const r = await checkImpersonation(http, ROOT, 'MSCRMCallerID', 'target-9', 'target-9'); + assert.strictEqual(r.ok, false, 'probes would describe the wrong user'); + assert.match(r.detail, /unexpected principal/); +}); + +test('checkImpersonation reports the missing privilege on a 403', async () => { + const http = httpStub([ + { status: 200, body: { UserId: 'CALLER-1' } }, + { status: 403 }, + ]); + const r = await checkImpersonation(http, ROOT, 'MSCRMCallerID', 'target-9'); + assert.strictEqual(r.ok, false); + assert.match(r.detail, /prvActOnBehalfOfAnotherUser/); + assert.match(r.detail, /DIRECTLY/); +}); + +test('checkImpersonation fails clearly when the plain WhoAmI fails', async () => { + const http = httpStub([{ status: 401 }]); + const r = await checkImpersonation(http, ROOT, 'MSCRMCallerID', 'target-9'); + assert.strictEqual(r.ok, false); + assert.match(r.detail, /az login/); +}); + +test('principal resolver prefers CallerObjectId when an object id is known', async () => { + const s = { personas: [{ persona: 'Dispatcher', assignTo: { users: ['USER-1'] } }] }; + const cache = new Map([['user-1', 'entra-oid-1']]); + const resolve = makePrincipalResolver(s, null, cache); + assert.deepStrictEqual(resolve('Dispatcher'), { header: 'CallerObjectId', value: 'entra-oid-1', systemUserId: 'USER-1' }); +}); + +test('principal resolver falls back to MSCRMCallerID with the systemuserid', async () => { + // assignTo.users[] already holds systemuserid GUIDs, so this path needs no directory lookup. + const s = { personas: [{ persona: 'Dispatcher', assignTo: { users: ['user-1'] } }] }; + const resolve = makePrincipalResolver(s, null, new Map()); + assert.deepStrictEqual(resolve('Dispatcher'), { header: 'MSCRMCallerID', value: 'user-1', systemUserId: 'user-1' }); +}); + +test('the resolver always carries a systemUserId, even under CallerObjectId', async () => { + // The canary compares WhoAmI().UserId (a systemuserid) to the target. Under CallerObjectId the + // header value is an Entra object id, which is NOT comparable — so the systemuserid must travel + // alongside it or the canary loses its only reliable oracle. + const s = { personas: [{ persona: 'D', assignTo: { users: ['USER-1'] } }] }; + for (const cache of [new Map([['user-1', 'entra-oid-1']]), new Map()]) { + const p = makePrincipalResolver(s, null, cache)('D'); + assert.ok(p.systemUserId, `${p.header} must still carry the systemuserid`); + assert.strictEqual(p.systemUserId.toLowerCase(), 'user-1'); + } +}); + +test('principal resolver returns null when the persona declares no test user', async () => { + const s = { personas: [{ persona: 'Dispatcher' }, { persona: 'Tech', assignTo: { users: [] } }] }; + const resolve = makePrincipalResolver(s, null, new Map()); + assert.strictEqual(resolve('Dispatcher'), null); + assert.strictEqual(resolve('Tech'), null); + assert.strictEqual(resolve('Nobody'), null); +}); + +test('a mutating probe is NEVER executed as a read', async () => { + // Regression guard for a real review finding: executeProbes called readOne for every planned + // probe, so with --allow-mutations a `write` probe was exercised as a GET and a 200 was reported + // as PASS — a false pass on the privilege the maker specifically opted in to test. + const calls = []; + const probes = [ + { persona: 'P', entity: 'co_workorder', access: 'write', expect: 'allow', mutating: true }, + { persona: 'P', entity: 'co_workorder', access: 'read', expect: 'allow', mutating: false }, + ]; + const f = await executeProbes(probes, io({ readOne: async (set) => { calls.push(set); return { status: 200, rowCount: 1 }; } })); + + assert.strictEqual(calls.length, 1, 'only the read probe may reach the wire'); + assert.strictEqual(f[0].result, 'inconclusive', 'the write probe must not be a pass'); + assert.match(f[0].detail, /cannot be proven by a read/); + assert.strictEqual(f[1].result, 'pass'); +}); + +test('a mutating probe does not even resolve a principal or entity set', async () => { + // It short-circuits before any IO, so --allow-mutations cannot add round trips either. + let touched = 0; + const probes = [{ persona: 'P', entity: 'e', access: 'delete', expect: 'allow', mutating: true }]; + const f = await executeProbes(probes, io({ + principalFor: () => { touched++; return { header: 'MSCRMCallerID', value: 'u' }; }, + entitySetName: async () => { touched++; return 'es'; }, + readOne: async () => { touched++; return { status: 200, rowCount: 1 }; }, + })); + assert.strictEqual(touched, 0, 'no IO for a probe that will not be executed'); + assert.strictEqual(f[0].result, 'inconclusive'); +}); + +test('summarize: an all-mutating run reports zero passes, not success', async () => { + const probes = [ + { persona: 'P', entity: 'a', access: 'create', expect: 'allow', mutating: true }, + { persona: 'P', entity: 'b', access: 'delete', expect: 'allow', mutating: true }, + ]; + const s = summarize(await executeProbes(probes, io())); + assert.strictEqual(s.counts.pass, 0); + assert.strictEqual(s.counts.inconclusive, 2); + assert.strictEqual(s.ok, true, 'still not a failure — but the caller can see nothing was proven'); +}); diff --git a/plugins/model-apps/scripts/tests/verify-model-app.test.js b/plugins/model-apps/scripts/tests/verify-model-app.test.js index 7b60f5d0..2d5bbd05 100644 --- a/plugins/model-apps/scripts/tests/verify-model-app.test.js +++ b/plugins/model-apps/scripts/tests/verify-model-app.test.js @@ -484,11 +484,11 @@ test('entityPrivileges is ABSENT (not broken) when the client or org url is miss }); test('rolePrivileges paginates and never caps with top', async () => { - // Found by a LIVE run: with the previous op: 5000 a System Administrator role returned + // Found by a LIVE run: with the previous `top: 5000` a System Administrator role returned // EXACTLY 5000 rows -- silently truncated. Paginated it returns 7119, so 2119 privileges were // being dropped. A truncated page is the worst shape for this check: a declared privilege that // fell off the end reads as NOT HELD, so verify reports a correctly configured role as missing. - // Dataverse honors as a hard cap and omits @odata.nextLink, and the SDK rejects + // Dataverse honors `$top` as a hard cap and omits @odata.nextLink, and the SDK rejects // paginate+top, so asserting the ABSENCE of top matters as much as the presence of paginate. const calls = []; const sdk = stubSdk(); diff --git a/plugins/model-apps/skills/app-builder/SKILL.md b/plugins/model-apps/skills/app-builder/SKILL.md index c8a5972b..b0f83ae5 100644 --- a/plugins/model-apps/skills/app-builder/SKILL.md +++ b/plugins/model-apps/skills/app-builder/SKILL.md @@ -319,6 +319,19 @@ forms and sitemap subareas + icons; exits non-zero and lists anything missing): node "${PLUGIN_ROOT}/scripts/verify-model-app.js" --env --spec @/app-spec.json ``` +**Optional — probe what each persona can actually DO.** Verify's `role-privileges` check is a +*metadata* comparison; it cannot prove the persona can perform the operation, which also depends on +record ownership, business unit, team membership, sharing and plug-ins. To check that, run read-only +authorization probes **as each persona** via Dataverse impersonation: + +```bash +node "${PLUGIN_ROOT}/scripts/probe-persona.js" --env --spec @/app-spec.json +``` + +Prerequisites, how to read `inconclusive` (it is **not** a pass), and the scope limit — a green run +means the data operations are authorized, **not** that the app works — are in +[persona-validation.md](../../references/persona-validation.md). + Then open the app in the browser. Refine `app-spec.json` and re-run Phase 2 to iterate. **Teardown (cleanup).** To remove everything an App Spec built — e.g. a live-verification probe or a