Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .ci/migration-hashes.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
"057_principal_relationships.sql": "124f72d04f718f615ac665f47b8500479c84db7d39969159d55afd95e0525717",
"058_rename_entra_app_role.sql": "b86da2c0c2b0fc2934affcab1298886b4e5d23644742b2c433abc08fc3d1c57f",
"059_context_acyclic_trigger.sql": "b5cd241b6a5df8deb5834f56e020ba8e0be6ca02f324d8054f0042b6fd2eccd7",
"060_auth_role_change_log.sql": "a03e9ab49545a9e9d18b9fe9160f298205e640875cfeeb5cde2982570d4e4c8b"
"060_auth_role_change_log.sql": "a03e9ab49545a9e9d18b9fe9160f298205e640875cfeeb5cde2982570d4e4c8b",
"061_business_role_covers_itself.sql": "0dc53ad1e6691691d43a80d929b5f55a207b267831465ba8abb67569eb10e6f6"
}
}
43 changes: 43 additions & 0 deletions app/api/contract-tests/governedIntentGap.contract.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,46 @@ describe('matrix matview — managedByAccessPackage from governance coverage', (
expect(rows).toHaveLength(0);
});
});

// Migration 061: holding a business role is governed access, so the role's own
// row must be covered by the role itself — otherwise the matrix paints the role
// row ungoverned and the scope statistics count it as an ungoverned assignment.
describe('business-role coverage — a role covers its own membership row (061)', () => {
async function coverage(principalId) {
await pool.query(`REFRESH MATERIALIZED VIEW "vw_UserPermissionAssignmentViaBusinessRole"`);
const r = await pool.query(
`SELECT "resourceId", "businessRoleId"
FROM "vw_UserPermissionAssignmentViaBusinessRole"
WHERE "userId" = $1
ORDER BY "resourceId"`,
[principalId],
);
return r.rows;
}

it('reports both the role itself and the resources it Contains', async () => {
await assign({ resourceId: BR, principalId: U_OK, governed: true });
const rows = await coverage(U_OK);
expect(rows).toEqual(expect.arrayContaining([
{ resourceId: BR, businessRoleId: BR },
{ resourceId: G, businessRoleId: BR },
]));
expect(rows).toHaveLength(2);
});

it('leaves a subject who does not hold the role uncovered', async () => {
await assign({ resourceId: G, principalId: U_UN, governed: false });
expect(await coverage(U_UN)).toEqual([]);
});

it('drops the self row again when the role assignment is soft-deleted', async () => {
await assign({ resourceId: BR, principalId: U_GAP, governed: true });
expect(await coverage(U_GAP)).toHaveLength(2);
await pool.query(
`UPDATE "ResourceAssignments" SET "deletedAt" = now()
WHERE "resourceId" = $1 AND "principalId" = $2`,
[BR, U_GAP],
);
expect(await coverage(U_GAP)).toEqual([]);
});
});
59 changes: 59 additions & 0 deletions app/api/src/db/migrations/061_business_role_covers_itself.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
-- 061: a business role also covers its own membership row
--
-- "vw_UserPermissionAssignmentViaBusinessRole" answers one question: which
-- business role(s) account for this (subject, resource) cell? Migration 049
-- built it purely from the `Contains` relationships, so it listed the resources
-- a role grants but never the role itself — even though holding a business role
-- IS governed access (a Direct assignment carrying governed=true, see 049).
--
-- The business role is a resource row in the matrix like any other, so that
-- omission surfaced everywhere the view is the governed signal:
-- * the role's own row rendered ungoverned — no business-role colour on its
-- cells, and it disappeared from the Governed view entirely;
-- * the scope statistics counted every business-role membership as an
-- ungoverned assignment, understating the governed percentage;
-- * a role that grants no resources at all never appeared in the roll-ups.
--
-- Fixed at the source: the view gains a self arm, so every consumer (matrix
-- colouring, the Governed filter, scope statistics, the roll-up builders)
-- agrees without any of them special-casing the role row.

DROP MATERIALIZED VIEW IF EXISTS "vw_UserPermissionAssignmentViaBusinessRole" CASCADE;
CREATE MATERIALIZED VIEW "vw_UserPermissionAssignmentViaBusinessRole" AS
-- Arm 1 (unchanged): the resources a governance resource Contains.
SELECT
bru."principalId" AS "userId",
rr."childResourceId" AS "groupId",
rr."childResourceId" AS "resourceId",
rr."parentResourceId" AS "businessRoleId"
FROM "ResourceRelationships" rr
JOIN "Resources" gov
ON gov.id = rr."parentResourceId" AND gov."governanceResource"
JOIN "ResourceAssignments" bru
ON bru."resourceId" = rr."parentResourceId"
AND bru."principalId" IS NOT NULL
AND bru."deletedAt" IS NULL
WHERE rr."relationshipType" = 'Contains'
UNION
-- Arm 2 (new): the governance resource covers its own membership cell.
SELECT
bru."principalId" AS "userId",
gov.id AS "groupId",
gov.id AS "resourceId",
gov.id AS "businessRoleId"
FROM "Resources" gov
JOIN "ResourceAssignments" bru
ON bru."resourceId" = gov.id
AND bru."principalId" IS NOT NULL
AND bru."deletedAt" IS NULL
WHERE gov."governanceResource"
WITH NO DATA;

CREATE UNIQUE INDEX "ix_vw_UPABR_pk"
ON "vw_UserPermissionAssignmentViaBusinessRole" ("userId", "groupId", "businessRoleId");
CREATE INDEX "ix_vw_UPABR_userId"
ON "vw_UserPermissionAssignmentViaBusinessRole" ("userId");
CREATE INDEX "ix_vw_UPABR_groupId"
ON "vw_UserPermissionAssignmentViaBusinessRole" ("groupId");

REFRESH MATERIALIZED VIEW "vw_UserPermissionAssignmentViaBusinessRole";
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { describe, it, expect } from 'vitest';
import { readFileSync } from 'fs';
import { join, dirname } from 'path';
import { fileURLToPath } from 'url';

const __dirname = dirname(fileURLToPath(import.meta.url));
const sql = readFileSync(join(__dirname, '061_business_role_covers_itself.sql'), 'utf8');

// Text-level guard, mirroring 054's/058's. The behavioural proof (a real
// Postgres returning the self row) lives in
// app/api/contract-tests/governedIntentGap.contract.test.js.
describe('migration 061 — a business role covers its own membership row', () => {
it('rebuilds the business-role coverage matview', () => {
expect(sql).toMatch(
/DROP MATERIALIZED VIEW IF EXISTS "vw_UserPermissionAssignmentViaBusinessRole" CASCADE/,
);
expect(sql).toMatch(
/CREATE MATERIALIZED VIEW "vw_UserPermissionAssignmentViaBusinessRole"/,
);
});

it('keeps the Contains arm and adds a self arm', () => {
expect(sql).toContain(`WHERE rr."relationshipType" = 'Contains'`);
// The self arm reports the governance resource as its own role AND resource.
expect(sql).toMatch(/gov\.id\s+AS "resourceId",\s*\n\s*gov\.id\s+AS "businessRoleId"/);
expect(sql.match(/^UNION$/m)).not.toBeNull();
});

it('counts only effective assignments — soft-deleted ones stay out of both arms', () => {
const guards = sql.match(/bru\."deletedAt" IS NULL/g);
expect(guards).toHaveLength(2);
});

it('restricts the self arm to governance resources', () => {
expect(sql).toContain(`WHERE gov."governanceResource"`);
});

it('recreates the indexes the matview is queried through and populates it', () => {
for (const ix of ['ix_vw_UPABR_pk', 'ix_vw_UPABR_userId', 'ix_vw_UPABR_groupId']) {
expect(sql).toContain(`"${ix}"`);
}
expect(sql).toMatch(
/REFRESH MATERIALIZED VIEW "vw_UserPermissionAssignmentViaBusinessRole"/,
);
});
});
17 changes: 17 additions & 0 deletions app/api/src/matrix/scopeHistory.js
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,14 @@ export function buildScopeAsofSql({ filter, principalColSet, resourceColSet, con
-- assignment type, from 049 on as a normal Direct membership on a resource
-- flagged governanceResource. The derived governed rows are not history
-- tracked, so coverage is reconstructed from membership and Contains facts.
--
-- Two arms, mirroring "vw_UserPermissionAssignmentViaBusinessRole" (049 +
-- 061) so the as-of numbers use the same definition of governed as the live
-- scope statistics. Without arm 2 the history path reported every
-- business-role membership row as ungoverned while the live path counted it,
-- so the latest timeseries point disagreed with live scope-stats.
coverage AS (
-- Arm 1: the resources a governance resource Contains.
SELECT DISTINCT ga.pid AS "userId", rr.child AS "groupId"
FROM asof_assign ga
JOIN asof_contains rr ON rr.parent = ga.rid
Expand All @@ -268,6 +275,16 @@ export function buildScopeAsofSql({ filter, principalColSet, resourceColSet, con
WHERE (ar.state->>'id') = ga.rid
AND COALESCE((ar.state->>'governanceResource')::boolean, false)
)
UNION
-- Arm 2: the governance resource covers its own membership cell —
-- holding a business role IS governed access.
SELECT DISTINCT ga.pid AS "userId", ga.rid AS "groupId"
FROM asof_assign ga
WHERE EXISTS (
SELECT 1 FROM asof_resources ar
WHERE (ar.state->>'id') = ga.rid
AND COALESCE((ar.state->>'governanceResource')::boolean, false)
)
),
sp AS (
SELECT (sp.state->>'id')::uuid AS id
Expand Down
20 changes: 20 additions & 0 deletions app/api/src/matrix/scopeHistory.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,26 @@ describe('buildScopeAsofSql', () => {
expect(scopeMode).toBe('attribute');
});

// The live scope statistics read vw_UserPermissionAssignmentViaBusinessRole,
// which since migration 061 has a second arm: holding a business role is
// itself governed access. The as-of path rebuilds that definition in SQL
// rather than reading the view, so it needs the same arm — without it the
// latest timeseries point reported a lower governed count than live
// scope-stats for the very same instant.
it('counts a governance resource membership as governed in its own right (061 parity)', () => {
const { sql } = build(EMPTY);
const coverage = sql.slice(sql.indexOf('coverage AS ('), sql.indexOf('sp AS ('));

// Arm 1 stays: coverage via the Contains relationship.
expect(coverage).toContain('asof_contains');
// Arm 2: the role's own cell, keyed on the assignment's own resource
// (ga.rid) rather than a Contains child.
expect(coverage).toMatch(/UNION/);
expect(coverage).toMatch(/ga\.rid AS "groupId"/);
// Both arms gate on the resource being a governance resource.
expect(coverage.match(/governanceResource/g)).toHaveLength(2);
});

it('excludes group-shaped principals from the subject count', () => {
const { sql } = build(EMPTY);
expect(sql).toContain('#microsoft.graph.group');
Expand Down
10 changes: 7 additions & 3 deletions app/ui/e2e/export-validation.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,13 @@ test.describe('Matrix Excel export — access-package columns match the grid', (
for (const tr of table.querySelectorAll('tbody tr')) {
const tds = [...tr.querySelectorAll('td')];
if (tds.length !== headers.length) continue; // spacer / message row
// The name cell carries an expander glyph for expandable rows; the
// export writes the bare display name.
const resource = (tds[1]?.innerText || '').replace(/^[▶▼]\s*/, '').trim();
// The export writes the bare display name, but the name cell also holds
// the fold/expand toggles, the nesting elbow and the "BR" overlap chips
// — so read the name element itself and only fall back to scraping the
// cell (older rows, and the rotated views) when it isn't there.
const nameEl = tds[1]?.querySelector('[data-row-name]');
const resource = (nameEl?.textContent ?? tds[1]?.innerText ?? '')
.replace(/^[▶▼└\s]+/, '').trim();
const type = (tds[headers.length - 2]?.innerText || '').trim();
const cells = [];
for (const col of apCols) {
Expand Down
Loading
Loading