Skip to content

Commit 3d04daf

Browse files
akshay-vizCopilot
andcommitted
feat(model-apps): probe what a persona can actually DO, as that persona
`role-privileges` (this PR) proves a 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`. So a role can verify clean and still leave the persona unable to work. `probe-persona.js` closes that gap by executing real reads AS the persona. Dataverse impersonation makes it human-free and needs no new auth: 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 the `systemuserid` that the legacy `MSCRMCallerID` header takes -- so the common case needs no directory lookup and no application user. It upgrades to the documented-preferred `CallerObjectId` when `azureactivedirectoryobjectid` is readable. Verified empirically that the existing transport carries a per-call header with `Authorization` and the OData headers intact, so nothing in the client changed. Three decisions carry the design: 1. It probes the NEGATIVE direction. For each persona it reads an entity that another persona declares and this one does not. An over-broad role is invisible from the inside -- everything the user tries succeeds -- so it can only be found by trying something that should fail. `appmodule` is excluded because the build injects it for every persona, so it would fail every run. 2. An empty 200 on a negative probe is INCONCLUSIVE, never a pass. Dataverse answers "no privilege" with 403 but "narrower scope" with a filtered 200, which is indistinguishable from an authorized read of an empty table. Reporting that as a pass would manufacture confidence in the one direction that matters. 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 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, which is loud, but the header being accepted and IGNORED: every probe would then run as the signed-in admin, every allow-probe would pass, and the report would look authoritative while proving nothing. `WhoAmI` returns the EFFECTIVE user id, so comparing it against the impersonated id catches that silently. A 403 there reports the real cause -- the caller needs `prvActOnBehalfOfAnotherUser`, assigned DIRECTLY, since a team-inherited grant does not satisfy it. Read-only by default; `--allow-mutations` only PLANS write probes rather than executing them, because writing to someone's environment to verify it deserves its own explicit design. Scope limit, documented in the script header, AGENTS.md and the CHANGELOG so the output is never over-read: this exercises the Web API. 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. Orchestration lives in the pure lib behind injected IO, mirroring readerFor/verifySpec, so principal resolution, entity-set resolution and error containment are all testable without a network. 31 new tests; 1511 total. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 42626da2-b66f-4162-acaa-b1127ef23d89
1 parent e10bc7f commit 3d04daf

5 files changed

Lines changed: 836 additions & 0 deletions

File tree

plugins/model-apps/AGENTS.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,34 @@ the pipeline and delegates each script's **behavioral spec** to the entries belo
284284
nothing. This is the F5 "convergence" mitigation: the build is additive (edits to existing artifacts
285285
aren't re-applied in place — teardown + rebuild to converge), and verify makes any resulting
286286
divergence **loud**.
287+
- **`scripts/probe-persona.js``scripts/lib/persona-probe.js`** — read-only **authorization** probes run
288+
AS each persona via Dataverse impersonation. `role-privileges` (above, in verify) compares metadata: it
289+
proves the role HOLDS the declared privileges. Whether the persona can actually perform the operation
290+
additionally depends on record ownership, business-unit placement, team membership, sharing, and
291+
server-side plug-ins — none of which appear in `roleprivileges` — so this executes real reads and
292+
reports what happens. Impersonation makes it human-free: effective privileges are the INTERSECTION of
293+
caller and target, so a System Administrator driving it cannot mask a permission the persona lacks.
294+
The principal comes from `personas[].assignTo.users[]`, which already holds `systemuserid` GUIDs —
295+
exactly what the legacy `MSCRMCallerID` header takes, so the common case needs **no directory lookup and
296+
no application user**; it upgrades to the preferred `CallerObjectId` when `azureactivedirectoryobjectid`
297+
is readable. Three things carry the design:
298+
(1) It probes the **negative** direction — for each persona it reads an entity another persona declares
299+
and this one does not. An over-broad role is invisible from the inside because everything the user tries
300+
succeeds, so it is only detectable by trying something that should fail. `appmodule` is never probed
301+
negatively (the build injects it for every persona, so it would fail on every run).
302+
(2) An empty `200` on a negative probe is **inconclusive, never a pass**: Dataverse returns 403 for *no
303+
privilege* but a filtered 200 for *a narrower scope*, and an empty result is indistinguishable from
304+
"authorized but the table is empty". Calling that a pass would manufacture confidence in the one
305+
direction that matters. Inconclusive results do not fail the run but are always counted and listed, so
306+
an all-inconclusive run cannot masquerade as clean.
307+
(3) A **`WhoAmI` canary** runs first. The dangerous failure is not a 403 (loud) but the header being
308+
accepted and IGNORED — every probe would then run as the signed-in admin and report false passes.
309+
`WhoAmI` returns the effective user id, so comparing it to the impersonated id detects that silently.
310+
A 403 there reports the real cause: the caller needs `prvActOnBehalfOfAnotherUser`, assigned **directly**
311+
(a team-inherited grant does not satisfy it). Read-only by default; `--allow-mutations` only *plans*
312+
write probes. **Scope limit, stated so results are not over-read:** this exercises the Web API, so it
313+
says nothing about UCI navigation, form/control visibility, client script, the command bar, layout or
314+
accessibility. A green run means the data operations are authorized, never that the app works.
287315
- **`scripts/ai-preflight.js`** — standalone preflight report: prints each AI feature's on/off status
288316
and the exact admin action needed (Power Platform Admin Center → Environments → Settings → Product →
289317
Features) for anything off. Never fails. The `ai-features` build phase calls this logic internally and
@@ -405,6 +433,7 @@ scripts/
405433
download-model-app.js ← app-builder: pull a deployed app into an editable spec (edit flow)
406434
teardown-model-app.js ← app-builder: classifier-safe reverse-of-build teardown
407435
verify-model-app.js ← app-builder: reconcile the spec against the deployed app
436+
probe-persona.js ← app-builder: run authorization probes AS each persona via Dataverse impersonation (read-only)
408437
preview-form.js ← app-builder: ASCII form wireframe for authoring review
409438
preview-app.js ← app-builder: ASCII whole-app design preview (data model + sitemap + forms + page-intents + design)
410439
write-app-spec-doc.js ← app-builder: renders the readable model-app-plan.md design doc from app-spec.json
@@ -431,6 +460,7 @@ scripts/
431460
spec-shape.js ← shared structural normalization for both authoring gates
432461
surface-resolver.js ← pure: resolve personas[].jobs[].surfaces[] to the spec artifacts that satisfy them
433462
role-privileges.js ← pure: declared persona privileges + subset comparison against a deployed role
463+
persona-probe.js ← pure: plan/interpret impersonated authorization probes (allow + deny) per persona
434464
odata.js ← OData literal escaping helpers
435465
genpage-cli.js ← pac model genpage upload/list/download wrapper
436466
hydrate-spec.js ← reconstruct an App Spec from a deployed app (edit flow)
@@ -787,6 +817,7 @@ az account set --subscription <sub-id>
787817
node scripts/check-auth.js --env <envUrl> # az token + WhoAmI preflight (pac optional; --require-pac for genpage)
788818
node scripts/build-model-app.js --env <envUrl> --spec @<dir>/app-spec.json [--sample-data --publish] --apply --verify
789819
node scripts/verify-model-app.js --env <envUrl> --spec @<dir>/app-spec.json
820+
node scripts/probe-persona.js --env <envUrl> --spec @<dir>/app-spec.json # authorization AS each persona (read-only)
790821
node scripts/teardown-model-app.js --env <envUrl> --spec @<dir>/app-spec.json --apply
791822
```
792823

plugins/model-apps/CHANGELOG.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,27 @@ jobs-to-be-done surfaces checkable, fixes four crash paths, and corrects a
99
smoke-eval assertion that could never pass live.
1010

1111
### Added
12+
- **`probe-persona.js` — authorization probes run AS each persona.** The
13+
`role-privileges` check below compares metadata and stops there; whether a
14+
persona can actually perform an operation also depends on record ownership,
15+
business-unit placement, team membership, sharing and server-side plug-ins,
16+
none of which appear in `roleprivileges`. This runs real reads under Dataverse
17+
impersonation, so it needs no human and no application user: effective
18+
privileges are the intersection of caller and target, and
19+
`personas[].assignTo.users[]` already carries the `systemuserid` the legacy
20+
`MSCRMCallerID` header takes (upgrading to `CallerObjectId` when the Entra
21+
object id is readable). It also probes the **negative** direction — reading an
22+
entity another persona declares and this one does not — because an over-broad
23+
role is invisible from the inside, where everything the user tries succeeds.
24+
An empty `200` on a negative probe is reported **inconclusive, never a pass**
25+
(Dataverse answers "no privilege" with 403 but "narrower scope" with a
26+
filtered 200, which is indistinguishable from an empty table), and a `WhoAmI`
27+
canary runs first to catch the impersonation header being accepted and
28+
silently IGNORED — which would otherwise run every probe as the signed-in
29+
admin and report false passes. Read-only by default. It exercises the Web API,
30+
so it says nothing about UCI navigation, form visibility, client script or
31+
layout: a green run means the data operations are authorized, not that the app
32+
works.
1233
- **`verify` now proves what a persona security role GRANTS, not just that it
1334
exists.** The `role` check only asserted a role row carrying the SDK ownership
1435
marker, so a role built with the wrong access — or one whose privilege write
Lines changed: 260 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,260 @@
1+
// plugins/model-apps/scripts/lib/persona-probe.js
2+
// PURE: plan and interpret impersonated authorization probes for each persona in an App Spec.
3+
//
4+
// WHY this exists. `role-privileges` proves the deployed role HOLDS the declared privileges — a
5+
// metadata comparison. It cannot prove the persona can actually perform the operation: privilege
6+
// depth interacts with record ownership, business-unit placement, team membership, sharing, and
7+
// server-side plug-ins, none of which are visible in `roleprivileges`. This probe closes that gap by
8+
// executing real Web API calls AS the persona and checking the outcome.
9+
//
10+
// Impersonation makes this cheap and human-free. The caller sends the target user's id on each
11+
// request; effective privileges become the INTERSECTION of caller and target, so a System
12+
// Administrator driving the probe cannot mask a permission the persona lacks:
13+
// CallerObjectId -> the Entra object id (preferred)
14+
// MSCRMCallerID -> the Dataverse systemuserid (legacy)
15+
// The caller needs `prvActOnBehalfOfAnotherUser`, assigned DIRECTLY (a Team-inherited grant does not
16+
// satisfy it). https://learn.microsoft.com/en-us/power-apps/developer/data-platform/webapi/impersonate-another-user-web-api
17+
//
18+
// WHAT THIS CANNOT DO — stated here so the report is never over-read. This exercises the Web API.
19+
// It says nothing about UCI navigation, which form opens, control/field visibility, client-side
20+
// script, the command bar, layout, accessibility, or whether a human can find the screen at all. A
21+
// green probe run means "the data operations are authorized", never "the app works".
22+
'use strict';
23+
24+
const { declaredPrivileges } = require('./role-privileges.js');
25+
26+
// Read is the only access we can exercise WITHOUT changing the environment, so it is the default.
27+
// Everything else creates, mutates or destroys rows and is planned but not executed unless the
28+
// caller opts in — a verification tool that silently writes to someone's environment is a trap.
29+
const MUTATING_ACCESS = new Set(['create', 'write', 'delete', 'append', 'appendto', 'assign', 'share']);
30+
31+
const isMutating = (access) => MUTATING_ACCESS.has(String(access || '').trim().toLowerCase());
32+
33+
// Entities the platform grants broadly and which therefore prove nothing as a NEGATIVE probe: a
34+
// persona that never declares `appmodule` still reads it, because the build injects that privilege
35+
// (see declaredPrivileges) and the platform needs it to open any app at all.
36+
const NEVER_PROBE_DENIED = new Set(['appmodule']);
37+
38+
/**
39+
* Plan the probes for every persona in the spec.
40+
*
41+
* Two probe kinds, and the negative one is the point:
42+
* - `expect: 'allow'` — an entity+access the persona DECLARES. Proves the grant works end to end.
43+
* - `expect: 'deny'` — an entity another persona declares but THIS one does not. Proves isolation
44+
* between personas, which is the failure nobody notices: an over-broad role
45+
* looks perfect from the inside because everything the user tries succeeds.
46+
*
47+
* @param {object} spec App Spec (already migrated/validated by the caller)
48+
* @param {object} [opts]
49+
* @param {boolean} [opts.includeMutations=false] plan mutating probes too (still not executed here)
50+
* @returns {{ probes: Array, warnings: string[] }}
51+
*/
52+
function planProbes(spec, opts = {}) {
53+
const includeMutations = opts.includeMutations === true;
54+
const personas = (spec && spec.personas) || [];
55+
const warnings = [];
56+
const probes = [];
57+
58+
if (personas.length === 0) {
59+
warnings.push('spec declares no personas — nothing to probe');
60+
return { probes, warnings };
61+
}
62+
63+
// Declared privileges per persona, computed once: used for the positive probes AND to derive each
64+
// persona's negative set by difference.
65+
const declaredByPersona = new Map();
66+
for (const p of personas) {
67+
const name = String((p && p.persona) || '').trim();
68+
if (!name) {
69+
warnings.push('skipped a persona with no name');
70+
continue;
71+
}
72+
declaredByPersona.set(name, declaredPrivileges(p));
73+
}
74+
75+
// Every entity ANY persona declares. The negative probes are drawn from this set rather than from
76+
// the whole data model, because an entity nobody asked for tells us nothing about role design.
77+
const allEntities = new Set();
78+
for (const declared of declaredByPersona.values()) {
79+
for (const d of declared) allEntities.add(d.entity);
80+
}
81+
82+
for (const [persona, declared] of declaredByPersona) {
83+
const ownEntities = new Set(declared.map((d) => d.entity));
84+
85+
for (const d of declared) {
86+
if (isMutating(d.access) && !includeMutations) continue;
87+
probes.push({
88+
persona,
89+
entity: d.entity,
90+
access: d.access,
91+
scope: d.scope,
92+
expect: 'allow',
93+
mutating: isMutating(d.access),
94+
reason: `persona declares ${d.access} on ${d.entity} at ${d.scope} scope`,
95+
});
96+
}
97+
98+
for (const entity of allEntities) {
99+
if (ownEntities.has(entity) || NEVER_PROBE_DENIED.has(entity)) continue;
100+
probes.push({
101+
persona,
102+
entity,
103+
access: 'read',
104+
scope: null,
105+
expect: 'deny',
106+
mutating: false,
107+
reason: `another persona declares ${entity}; this one does not, so it should not be readable`,
108+
});
109+
}
110+
}
111+
112+
if (!includeMutations && probes.every((p) => !p.mutating)) {
113+
const skipped = [...declaredByPersona.values()].flat().filter((d) => isMutating(d.access)).length;
114+
if (skipped > 0) {
115+
warnings.push(`${skipped} mutating privilege(s) planned but NOT executed (read-only run; pass --allow-mutations to include them)`);
116+
}
117+
}
118+
119+
return { probes, warnings };
120+
}
121+
122+
/**
123+
* Turn one raw HTTP outcome into a finding.
124+
*
125+
* @param {object} probe from planProbes
126+
* @param {object} outcome { status:number|null, rowCount:number|null, error?:string }
127+
* @returns {{ probe, result: 'pass'|'fail'|'inconclusive', detail: string }}
128+
*
129+
* The `inconclusive` result is the load-bearing part. Dataverse expresses "you may not see this"
130+
* two different ways:
131+
* - NO privilege at all -> 403 Forbidden
132+
* - privilege at a NARROWER scope -> 200 OK with the rows filtered out
133+
* So an empty 200 on a negative probe is genuinely ambiguous: it looks identical to "authorized, but
134+
* this table happens to be empty". Reporting that as a pass would manufacture false confidence in
135+
* exactly the direction that matters, so it is reported as inconclusive and the operator is told
136+
* what would disambiguate it (seed a row owned by someone else).
137+
*/
138+
function interpretOutcome(probe, outcome) {
139+
const status = outcome && outcome.status;
140+
const rowCount = outcome && outcome.rowCount;
141+
const finding = (result, detail) => ({ probe, result, detail });
142+
143+
// A transport-level failure is never evidence about authorization.
144+
if (outcome && outcome.error && status == null) {
145+
return finding('inconclusive', `request failed before a status was returned: ${outcome.error}`);
146+
}
147+
148+
if (probe.expect === 'allow') {
149+
if (status === 403) return finding('fail', 'denied (403) but the persona declares this privilege');
150+
if (status === 401) return finding('inconclusive', 'unauthorized (401) — impersonation or auth problem, not a role finding');
151+
if (status === 404) return finding('fail', 'not found (404) — the entity set does not exist for this persona');
152+
if (typeof status === 'number' && status >= 200 && status < 300) {
153+
// A scoped read legitimately returns zero rows; that is not a failure of the grant.
154+
return finding('pass', rowCount === 0 ? 'authorized (no rows visible at this scope)' : 'authorized');
155+
}
156+
return finding('inconclusive', `unexpected status ${status}`);
157+
}
158+
159+
// expect === 'deny'
160+
if (status === 403) return finding('pass', 'correctly denied (403)');
161+
if (typeof status === 'number' && status >= 200 && status < 300) {
162+
if (rowCount > 0) {
163+
return finding('fail', `readable (${rowCount} row(s) visible) but no job declares this entity — the role is broader than the spec`);
164+
}
165+
return finding(
166+
'inconclusive',
167+
'returned 200 with no rows: cannot distinguish "denied by scope" from "authorized but empty". Seed a row owned by another user to disambiguate.',
168+
);
169+
}
170+
if (status === 401) return finding('inconclusive', 'unauthorized (401) — impersonation or auth problem, not a role finding');
171+
return finding('inconclusive', `unexpected status ${status}`);
172+
}
173+
174+
/**
175+
* Roll findings up into a report.
176+
* `ok` is false when anything FAILED. Inconclusive results do not fail the run — they are genuine
177+
* unknowns, and failing on them would train the operator to ignore the tool — but they are counted
178+
* and listed so an all-inconclusive run cannot masquerade as a clean one.
179+
*/
180+
function summarize(findings) {
181+
const counts = { pass: 0, fail: 0, inconclusive: 0 };
182+
for (const f of findings) counts[f.result] = (counts[f.result] || 0) + 1;
183+
return {
184+
ok: counts.fail === 0,
185+
counts,
186+
total: findings.length,
187+
failures: findings.filter((f) => f.result === 'fail'),
188+
inconclusive: findings.filter((f) => f.result === 'inconclusive'),
189+
};
190+
}
191+
192+
/**
193+
* Execute planned probes against injected IO. Kept here rather than in the CLI so the orchestration
194+
* — principal resolution, entity-set resolution, error containment — is testable without a network,
195+
* mirroring how `verifySpec` takes an injected reader.
196+
*
197+
* @param {Array} probes from planProbes
198+
* @param {object} io
199+
* principalFor(persona) -> { header:'CallerObjectId'|'MSCRMCallerID', value:string } | null
200+
* entitySetName(entity) -> Promise<string|null> (the OData collection name, e.g. co_workorders)
201+
* readOne(entitySet, hdr)-> Promise<{ status, rowCount, error? }>
202+
* @returns {Promise<Array>} findings
203+
*
204+
* Every failure mode here degrades to `inconclusive` rather than `fail`. A probe that could not be
205+
* RUN proves nothing about the role, and reporting it as a role failure would send the operator to
206+
* fix a security role when the real problem is a missing test user or an unresolvable entity.
207+
*/
208+
async function executeProbes(probes, io) {
209+
const findings = [];
210+
// One metadata read per entity, not per probe — the negative probes alone are O(personas × entities).
211+
const setNameCache = new Map();
212+
const principalCache = new Map();
213+
214+
for (const probe of probes) {
215+
if (!principalCache.has(probe.persona)) {
216+
principalCache.set(probe.persona, io.principalFor(probe.persona));
217+
}
218+
const principal = principalCache.get(probe.persona);
219+
if (!principal) {
220+
findings.push({
221+
probe,
222+
result: 'inconclusive',
223+
detail: `no test user for persona '${probe.persona}' — declare assignTo.users[] or assign the role to a user`,
224+
});
225+
continue;
226+
}
227+
228+
if (!setNameCache.has(probe.entity)) {
229+
let name = null;
230+
try {
231+
name = await io.entitySetName(probe.entity);
232+
} catch (err) {
233+
name = null;
234+
void err;
235+
}
236+
setNameCache.set(probe.entity, name);
237+
}
238+
const entitySet = setNameCache.get(probe.entity);
239+
if (!entitySet) {
240+
findings.push({
241+
probe,
242+
result: 'inconclusive',
243+
detail: `could not resolve the entity set name for '${probe.entity}'`,
244+
});
245+
continue;
246+
}
247+
248+
let outcome;
249+
try {
250+
outcome = await io.readOne(entitySet, { [principal.header]: principal.value });
251+
} catch (err) {
252+
outcome = { status: null, rowCount: null, error: (err && err.message) || String(err) };
253+
}
254+
findings.push(interpretOutcome(probe, outcome));
255+
}
256+
257+
return findings;
258+
}
259+
260+
module.exports = { planProbes, interpretOutcome, executeProbes, summarize, isMutating };

0 commit comments

Comments
 (0)