feat: implement enterprise privileged access and identity security rules (AZ-IDN-016-025) - #279
feat: implement enterprise privileged access and identity security rules (AZ-IDN-016-025)#279TFT444 wants to merge 7 commits into
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
|
@TFT444 - Resolve the conflicts, post which an initial review would be appreciated from @ritiksah141 @parthrohit22. |
11e460e to
deab51b
Compare
conflict solved. @ritiksah141 @parthrohit22 have a look please i believe its mergable now. verify it please |
parthrohit22
left a comment
There was a problem hiding this comment.
The overall direction is useful, but this PR is not merge-ready. Several collectors and rules do not match the Microsoft Graph or Azure SDK contracts, which can cause incorrect findings and silent false negatives.
Blocking issues:
-
PIM detection uses
roleAssignmentSchedulesbut expectsEligible/Activeassignment types. Microsoft Graph returnsAssigned/Activated; eligible assignments must be collected fromroleEligibilitySchedules. AZ-IDN-017 and AZ-IDN-018 will therefore misclassify valid PIM assignments. -
AZ-IDN-023 calls unsupported Identity Protection policy endpoints. The resulting exception is converted to
None, so the rule silently returns no findings. -
AZ-IDN-024 checks for
"All"inexcludeServicePrincipals, but Graph represents exclusions as service-principal IDs. The workload identity coverage logic needs to useincludeServicePrincipalsand actual policy scope. -
Privileged role assignments can target users, groups, or service principals. The collector treats every principal as a user, causing incorrect MFA, stale-account, and break-glass findings.
-
Cosmos DB and cache collection failures return empty lists. The rules interpret those empty lists as compliant inventories, creating silent false negatives.
-
AZ-STOR-009 reads
period_since_creation_in_days, but the Azure SDK property isimmutability_period_since_creation_in_days. -
Several new rules compare Azure SDK enum objects using
str(value)rather than normalizing.value. This affects TLS, encryption-source, audit-policy, and immutability-state checks. -
AZ-IDN-021 and AZ-IDN-022 do not validate included users, exclusions, report-only state, or application scope. A policy covering only a subset of users can incorrectly suppress a tenant-wide finding.
Please correct these issues and add regression tests using representative Microsoft Graph payloads and actual Azure SDK model objects before merging.
8f0c830 to
fc98ddb
Compare
|
@parthrohit22 all 8 blockers from your review have been addressed in the latest commit. Quick summary of what changed: PIM collector now queries both |
|
@TFT444, I have noted the commit addressing the eight SDK/API blockers. Since this branch is now significantly behind |
Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
…les (AZ-IDN-016-025) Add 10 new rules covering privileged identity security for Microsoft Entra ID: - AZ-IDN-016: Privileged user missing phishing-resistant MFA (CRITICAL) - AZ-IDN-017: Global Administrator permanently assigned outside PIM (HIGH) - AZ-IDN-018: Privileged role assigned outside PIM (HIGH) - AZ-IDN-019: Stale privileged account retains active access (HIGH) - AZ-IDN-020: No emergency access accounts detected (HIGH) - AZ-IDN-021: Legacy authentication protocols not blocked (HIGH) - AZ-IDN-022: No MFA requirement for Azure Management (HIGH) - AZ-IDN-023: Identity Protection risk policies disabled (MEDIUM) - AZ-IDN-024: Service principals excluded from MFA enforcement (MEDIUM) - AZ-IDN-025: Privileged role-assignable group has no owner (MEDIUM) Add 5 new Graph API collectors to azure_client.py: - get_privileged_role_members, get_privileged_users_mfa_methods, get_pim_role_assignments, get_identity_protection_policies, get_privileged_groups Add MockAzureClient support for all new collectors (fully offline tests). Add 58 tests in test_rules_identity_priv.py covering compliant, violating, empty inventory, API failure (None), and edge cases. Add 10 remediation playbooks (fix_az_idn_016.sh through fix_az_idn_025.sh). Add compliance mappings for CIS Azure 2.0.0, NIST CSF, ISO 27001, SOC 2. Add docs/rules-reference.md entries for all 10 rules. Closes #258 Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
…ty rules PIM detection (IDN-017, IDN-018): - Collector now queries both roleAssignmentSchedules (Active) and roleEligibilitySchedules (Eligible) so eligible assignments are actually detected; previously only Active schedules were fetched - principalId/principalType tracked for all principal types (users, groups, service principals), not only users Identity Protection (IDN-023): - Replace unreliable direct policy endpoints with CA-policy inspection: checks conditions.userRiskLevels / signInRiskLevels on enabled policies CA policy user scope (IDN-021, IDN-022): - _covers_all_users() helper added; policies that target a subset of users no longer suppress a tenant-wide finding Workload identity exclusion (IDN-024): - Fix excludeServicePrincipals check: the field holds SP IDs, not All; rewrite to use includeServicePrincipals presence and All-exclude logic Collection failures (Cosmos DB, Redis cache): - get_cosmos_accounts / get_managed_caches return None on failure instead of []; callers log a warning and skip rather than treating missing inventory as compliant Enum normalization (AZ-STOR-009, AZ-DB-007): - Replace raw str() calls with enum_str() for SDK enum fields - Fix immutability retention property: period_since_creation_in_days -> immutability_period_since_creation_in_days Test fixtures updated to match new API contracts (principalId, deep-merge of CA policy conditions, correct immutability property name). CI: add chromadb CVE-2026-45830 and CVE-2026-45833 to pip-audit ignore list (no patched version; removal tracked in PR #317). Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
The skip condition checked for the ai/vectorstore/ directory, which can exist from a previous build without the BM25 index file. After PR #317 replaced chromadb with a JSON BM25 index, the test ran and raised VectorStoreNotBuilt instead of skipping. Changed skipif to check for the actual index file so the class is skipped correctly when the index has not been built. Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
fc98ddb to
2bb2694
Compare
|
@m-khan-97 @parthrohit22 the branch has been rebased onto current dev and all 870 tracked tests pass with 0 failures. The 8 SDK/API blockers from Parth's review were addressed in a prior commit (PIM collector queries both |
Summary
Implements GitHub issue #258 — 10 new Azure Identity security rules covering privileged access and identity protection for Microsoft Entra ID.
New Rules:
Infrastructure Added:
Test Plan
Closes #258