Optimize 21814 - Privileged accounts are cloud native identities

[Sandeep Jha (sandeepjha000)]

Spec-21814

[Copilot]

🟡 Changes recommended

User counts can be overstated, deprecated metadata remains, and the new database behavior lacks focused tests.

1 open finding

• Remove deprecated unused MinimumLicense metadata · New

What changed in this PR

Optimizes assessment 21814 by replacing per-user Graph calls with exported database queries.

Changes:

• Queries privileged role users from vwRole and User.
• Evaluates synchronization status from cached data.
• Simplifies result generation and metadata.

File	Description
src/​powershell/​tests/​Test-Assessment.21814.ps1	Migrates assessment logic and reporting to database-backed role data.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

The reported user count can overcount identities, deprecated metadata remains, and the migration lacks regression tests.

1 open finding

• Remove deprecated unused MinimumLicense metadata

Previously missed (2)

In code that hasn't changed since last review

Add Pester coverage for assessment 21814 migration cases

src/​powershell/​tests/​Test-Assessment.21814.ps1:52

This migration replaces live Graph requests with SQL over direct, eligible, and expanded group assignments, but assessment 21814 has no Pester coverage. Add database-backed cases for synced/cloud-only users, duplicate roles, group-expanded assignments, and an empty result, following the assessment-test pattern in code-tests/test-assessments/Test-Assessment.21868.Tests.ps1.

Count distinct privileged users instead of role assignments

src/​powershell/​tests/​Test-Assessment.21814.ps1:66

SELECT DISTINCT still returns one row per role because roleDisplayName is selected, so a synced account assigned to multiple privileged roles is counted multiple times here. The message therefore reports role assignments as “privileged users”; count distinct user IDs while retaining the per-role rows for the table.

🧠 Review effort: Balanced

[Copilot]

🟢 Approval recommended

The database-backed implementation preserves assessment behavior while eliminating repeated Graph calls.

0 open findings

1 resolved since last review

• Remove deprecated unused MinimumLicense metadata

🧠 Review effort: Balanced

[Aleksandar Nikolić (alexandair)]

Sandeep Jha (@sandeepjha000) Please, address my feeedback.

[Aleksandar Nikolić (alexandair)]

Count Distinct Users Separately From Role Rows

The filtered collection at line 58 still contains one row per role/user combination:

    $syncedPrivilegedRoleUsers = @($privilegedRoleUsers | Where-Object { $_.onPremisesSyncEnabled -eq $true })

The computed count at line 65 therefore counts role rows, not distinct people. One synchronized user assigned two privileged roles was reported as two privileged users. This is a data-count issue, not a wording review.

The base implementation also counted users separately within each role, so this is an inherited limitation, not a new blocker. Keep the role rows for the table and use unique user IDs for a future user count correction.

[Copilot]

🟡 Changes recommended

Export statistics are not shared with parallel test workers, making the new completeness checks unreliable.

1 open finding

• Export statistics are unavailable to parallel test workers · New

🧠 Review effort: Balanced

[Copilot]

🟢 Approval recommended

The database-backed implementation is consistent with existing export structures and has no unresolved correctness issues.

0 open findings

1 resolved since last review

• Export statistics are unavailable to parallel test workers

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Confirmed policy failures are incorrectly overridden with an Investigate status when unresolved records also exist.

1 open finding

• Prioritize confirmed failures over missing records in status · New

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Missing role-definition records can be silently excluded, allowing an incomplete export to pass.

1 open finding

• Handle missing role definitions as unresolved assignments · New

1 resolved since last review

• Prioritize confirmed failures over missing records in status

🧠 Review effort: Balanced

[Copilot]

🟢 Approval recommended

No blocking issues were identified in the assessment logic or its database and reporting integration.

0 open findings

1 resolved since last review

• Handle missing role definitions as unresolved assignments

🧠 Review effort: Balanced

[Aleksandar Nikolić (alexandair)]

LGTM