From e8ffe59ce23f3ea4a3a441d11f50549d5904a9fe Mon Sep 17 00:00:00 2001 From: Zeus Almightee <53497290+ernestprovo23@users.noreply.github.com> Date: Wed, 17 Jun 2026 10:16:49 -0400 Subject: [PATCH] feat(m365/entra): add entra_conditional_access_policy_no_deleted_object_references check (#11236) Co-authored-by: Daniel Barranquero --- prowler/CHANGELOG.md | 1 + .../__init__.py | 0 ...no_deleted_object_references.metadata.json | 44 ++ ...ess_policy_no_deleted_object_references.py | 157 ++++++ .../m365/services/entra/entra_service.py | 149 +++++- ...olicy_no_deleted_object_references_test.py | 466 ++++++++++++++++++ .../entra/microsoft365_entra_service_test.py | 186 +++++++ 7 files changed, 1002 insertions(+), 1 deletion(-) create mode 100644 prowler/providers/m365/services/entra/entra_conditional_access_policy_no_deleted_object_references/__init__.py create mode 100644 prowler/providers/m365/services/entra/entra_conditional_access_policy_no_deleted_object_references/entra_conditional_access_policy_no_deleted_object_references.metadata.json create mode 100644 prowler/providers/m365/services/entra/entra_conditional_access_policy_no_deleted_object_references/entra_conditional_access_policy_no_deleted_object_references.py create mode 100644 tests/providers/m365/services/entra/entra_conditional_access_policy_no_deleted_object_references/entra_conditional_access_policy_no_deleted_object_references_test.py diff --git a/prowler/CHANGELOG.md b/prowler/CHANGELOG.md index d187019a5b..14fa976490 100644 --- a/prowler/CHANGELOG.md +++ b/prowler/CHANGELOG.md @@ -22,6 +22,7 @@ All notable changes to the **Prowler SDK** are documented in this file. - Jira timeout preventing the calls from hanging indefinitely when the Jira endpoint is unreachable or slow [(#11602)](https://github.com/prowler-cloud/prowler/pull/11602) - TLS certificate verification in the `codepipeline_project_repo_private` check, which previously used an unverified SSL context, leaving the repository-visibility probe open to MITM tampering [(#11603)](https://github.com/prowler-cloud/prowler/pull/11603) - `entra_directory_sync_object_takeover_blocked` check for the M365 provider, verifying that hybrid Entra tenants block cloud object takeover through both soft-match and hard-match directory synchronization [(#11098)](https://github.com/prowler-cloud/prowler/pull/11098) +- `entra_conditional_access_policy_no_deleted_object_references` check for M365 provider [(#11236)](https://github.com/prowler-cloud/prowler/pull/11236) ### 🔄 Changed diff --git a/prowler/providers/m365/services/entra/entra_conditional_access_policy_no_deleted_object_references/__init__.py b/prowler/providers/m365/services/entra/entra_conditional_access_policy_no_deleted_object_references/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/prowler/providers/m365/services/entra/entra_conditional_access_policy_no_deleted_object_references/entra_conditional_access_policy_no_deleted_object_references.metadata.json b/prowler/providers/m365/services/entra/entra_conditional_access_policy_no_deleted_object_references/entra_conditional_access_policy_no_deleted_object_references.metadata.json new file mode 100644 index 0000000000..42b3acaeb6 --- /dev/null +++ b/prowler/providers/m365/services/entra/entra_conditional_access_policy_no_deleted_object_references/entra_conditional_access_policy_no_deleted_object_references.metadata.json @@ -0,0 +1,44 @@ +{ + "Provider": "m365", + "CheckID": "entra_conditional_access_policy_no_deleted_object_references", + "CheckTitle": "Conditional Access policies must not reference deleted users, groups, or roles", + "CheckType": [], + "ServiceName": "entra", + "SubServiceName": "", + "ResourceIdTemplate": "", + "Severity": "medium", + "ResourceType": "NotDefined", + "ResourceGroup": "IAM", + "Description": "Every object identifier referenced by any Conditional Access policy under conditions.users (includeUsers, excludeUsers, includeGroups, excludeGroups, includeRoles, excludeRoles) must resolve to an existing Microsoft Entra object. This check audits all Conditional Access policies regardless of state and reports any whose user, group, or role references no longer resolve in the directory.", + "Risk": "When a user, group, or directory role referenced by a Conditional Access policy stops resolving (account or group deleted, role template removed), the reference becomes orphaned. include* references silently shrink the policy's enforcement scope; exclude* references can cause the policy to evaluate unexpectedly. This is a common root cause of MFA-not-applied incidents.", + "RelatedUrl": "", + "AdditionalURLs": [ + "https://learn.microsoft.com/en-us/graph/api/resources/conditionalaccesspolicy?view=graph-rest-1.0", + "https://learn.microsoft.com/en-us/graph/api/resources/conditionalaccessusers?view=graph-rest-1.0", + "https://learn.microsoft.com/en-us/graph/api/user-get?view=graph-rest-1.0", + "https://learn.microsoft.com/en-us/graph/api/group-get?view=graph-rest-1.0", + "https://learn.microsoft.com/en-us/graph/api/unifiedroledefinition-get?view=graph-rest-1.0", + "https://learn.microsoft.com/en-us/entra/identity/conditional-access/concept-conditional-access-users-groups" + ], + "Remediation": { + "Code": { + "CLI": "", + "NativeIaC": "", + "Other": "1. Sign in to the Microsoft Entra admin center (https://entra.microsoft.com)\n2. Navigate to Protection > Conditional Access > Policies\n3. Open each policy reported by this check\n4. Under Assignments > Users, remove every user, group, or role identifier reported as deleted\n5. Save the policy and re-run the audit\n6. For ongoing hygiene, audit Conditional Access policies after any bulk user/group/role cleanup", + "Terraform": "" + }, + "Recommendation": { + "Text": "Audit each Conditional Access policy quarterly and remove references to deleted users, groups, or directory roles. Stale references in include collections silently shrink enforcement scope; stale references in exclude collections can cause policies to behave unexpectedly. Treat both as misconfigurations regardless of policy state.", + "Url": "https://hub.prowler.com/check/entra_conditional_access_policy_no_deleted_object_references" + } + }, + "Categories": [ + "identity-access", + "e3" + ], + "DependsOn": [], + "RelatedTo": [ + "entra_conditional_access_policy_directory_sync_account_excluded" + ], + "Notes": "The check runs against all Conditional Access policies regardless of state (enabled, disabled, enabledForReportingButNotEnforced) — stale references in disabled policies are a misconfiguration that becomes live the moment the policy is re-enabled. Only HTTP 404 responses flag an identifier as deleted (FAIL). Transient Graph errors (5xx, throttling, insufficient permissions) are not treated as deletions; a policy whose references could not be resolved for those reasons is reported as MANUAL so it is not silently considered clean." +} diff --git a/prowler/providers/m365/services/entra/entra_conditional_access_policy_no_deleted_object_references/entra_conditional_access_policy_no_deleted_object_references.py b/prowler/providers/m365/services/entra/entra_conditional_access_policy_no_deleted_object_references/entra_conditional_access_policy_no_deleted_object_references.py new file mode 100644 index 0000000000..e5c5eff389 --- /dev/null +++ b/prowler/providers/m365/services/entra/entra_conditional_access_policy_no_deleted_object_references/entra_conditional_access_policy_no_deleted_object_references.py @@ -0,0 +1,157 @@ +from prowler.lib.check.models import Check, CheckReportM365 +from prowler.providers.m365.services.entra.entra_client import entra_client +from prowler.providers.m365.services.entra.entra_service import ( + CONDITIONAL_ACCESS_SENTINEL_IDS, + ConditionalAccessPolicyState, +) + + +class entra_conditional_access_policy_no_deleted_object_references(Check): + """ + Ensure Conditional Access policies do not reference deleted directory objects. + + Stale references to deleted users, groups, or directory roles silently change + the runtime behavior of a Conditional Access policy: include* references + shrink enforcement scope, exclude* references can change exemption logic. + Either way, the policy stops behaving the way the operator believes it does. + + The directory-object existence check runs once at service init time and is + cached on the entra client. This check reads from that cache and reports any + policy whose users/groups/roles inclusion or exclusion collections name an + identifier that no longer resolves in Microsoft Entra ID. + + Identifiers whose Graph lookup failed with a non-404 error (5xx, throttling, + insufficient permissions) are cached separately: they could not be verified + as present or deleted, so the policy is reported as MANUAL rather than being + silently treated as clean. + + - PASS: The policy references no deleted users, groups, or roles. + - FAIL: The policy references at least one deleted user, group, or role. + - MANUAL: At least one referenced identifier could not be resolved due to a + transient Graph error, so the policy could not be fully evaluated. + """ + + def execute(self) -> list[CheckReportM365]: + findings = [] + unresolved = entra_client.unresolved_directory_object_references + errored = entra_client.errored_directory_object_references + + for policy_id, policy in entra_client.conditional_access_policies.items(): + report = CheckReportM365( + metadata=self.metadata(), + resource=policy, + resource_name=policy.display_name, + resource_id=policy_id, + ) + + orphans = self._collect_references_in(policy, unresolved) + unverified = self._collect_references_in(policy, errored) + + if orphans: + # A confirmed deletion takes precedence over unverified ones. + report.status = "FAIL" + report.status_extended = self._format_failure( + policy.display_name, orphans, unverified, policy.state + ) + elif unverified: + # Nothing confirmed deleted, but we could not verify every + # reference — do not claim the policy is clean. + report.status = "MANUAL" + report.status_extended = self._format_manual( + policy.display_name, unverified + ) + else: + report.status = "PASS" + report.status_extended = ( + f"Conditional Access policy {policy.display_name} references no " + f"deleted directory objects." + ) + + findings.append(report) + + return findings + + @staticmethod + def _collect_references_in(policy, id_set): + """Walk the six identifier collections and return references in ``id_set``. + + Args: + policy: The Conditional Access policy to inspect. + id_set: Set of ``(type, id)`` pairs to match references against. + + Returns: + list[tuple[str, str, str]]: ``(type, id, side)`` tuples where + ``type`` is one of ``user|group|role``, ``id`` is the Graph + identifier, and ``side`` is one of ``include|exclude``. + """ + if not policy.conditions or not policy.conditions.user_conditions: + return [] + + uc = policy.conditions.user_conditions + collections = ( + ("user", "include", uc.included_users), + ("user", "exclude", uc.excluded_users), + ("group", "include", uc.included_groups), + ("group", "exclude", uc.excluded_groups), + ("role", "include", uc.included_roles), + ("role", "exclude", uc.excluded_roles), + ) + + matches = [] + for type_, side, identifiers in collections: + for identifier in identifiers: + if identifier in CONDITIONAL_ACCESS_SENTINEL_IDS: + continue + if (type_, identifier) in id_set: + matches.append((type_, identifier, side)) + return matches + + @staticmethod + def _group_by_type(references): + """Group ``(type, id, side)`` references into a deterministic message part.""" + by_type = {"user": [], "group": [], "role": []} + for type_, identifier, side in references: + by_type[type_].append(f"{identifier} ({side})") + + parts = [] + for type_ in ("user", "group", "role"): + if by_type[type_]: + joined = ", ".join(sorted(by_type[type_])) + parts.append(f"{type_}s: {joined}") + return "; ".join(parts) + + @classmethod + def _format_failure(cls, display_name, orphans, unverified, state=None): + # Surface report-only mode explicitly: the stale references are not yet + # enforced, but become live the moment the policy is turned on. + report_only = ( + " The policy is in report-only mode, so these references are not " + "enforced yet but will take effect once it is enabled." + if state == ConditionalAccessPolicyState.ENABLED_FOR_REPORTING + else "" + ) + + # If some references also could not be resolved, say so rather than + # implying the rest of the policy is fully clean. + unverified_note = ( + f" Additionally, {len(unverified)} reference(s) could not be verified " + f"due to transient Microsoft Graph errors." + if unverified + else "" + ) + + return ( + f"Conditional Access policy {display_name} references " + f"{len(orphans)} deleted directory object(s) — " + f"{cls._group_by_type(orphans)}.{report_only}{unverified_note}" + ) + + @classmethod + def _format_manual(cls, display_name, unverified): + return ( + f"Conditional Access policy {display_name} could not be fully evaluated: " + f"{len(unverified)} reference(s) could not be resolved due to transient " + f"Microsoft Graph errors (5xx, throttling, or insufficient permissions) — " + f"{cls._group_by_type(unverified)}. Re-run the scan or review the policy " + f"manually." + ) diff --git a/prowler/providers/m365/services/entra/entra_service.py b/prowler/providers/m365/services/entra/entra_service.py index 39d958b44b..bbaa80edcc 100644 --- a/prowler/providers/m365/services/entra/entra_service.py +++ b/prowler/providers/m365/services/entra/entra_service.py @@ -3,7 +3,7 @@ import json from asyncio import gather from datetime import datetime, timezone from enum import Enum -from typing import Any, Dict, List, Optional, Tuple +from typing import Any, Dict, List, Optional, Set, Tuple from uuid import UUID from kiota_abstractions.base_request_configuration import RequestConfiguration @@ -18,6 +18,12 @@ from prowler.lib.logger import logger from prowler.providers.m365.lib.service.service import M365Service from prowler.providers.m365.m365_provider import M365Provider +# Sentinel identifiers used in Conditional Access ``conditions.users`` +# collections that do not correspond to real directory objects and must not be +# resolved against Graph. Shared by the resolver below and the check that reads +# its result. +CONDITIONAL_ACCESS_SENTINEL_IDS = {"All", "None", "GuestsOrExternalUsers"} + class Entra(M365Service): """ @@ -110,6 +116,23 @@ class Entra(M365Service): self.app_registrations: Dict[str, "AppRegistration"] = attributes[11] self.user_accounts_status = {} + # Resolve directory-object identifiers referenced by Conditional Access + # policies. This runs as a separate phase because it depends on the + # main gather having populated ``conditional_access_policies`` first. + # The result is cached on the instance so sync checks can read it + # without issuing Graph calls of their own. ``unresolved`` holds ids + # confirmed deleted (HTTP 404); ``errored`` holds ids whose lookup + # failed for any other reason (5xx, throttling, permission) and could + # therefore be neither confirmed present nor confirmed deleted. + self.unresolved_directory_object_references: Set[Tuple[str, str]] + self.errored_directory_object_references: Set[Tuple[str, str]] + ( + self.unresolved_directory_object_references, + self.errored_directory_object_references, + ) = loop.run_until_complete( + self._resolve_directory_object_references(self.conditional_access_policies) + ) + if created_loop: asyncio.set_event_loop(None) loop.close() @@ -1379,6 +1402,130 @@ OAuthAppInfo ) return app_registrations + async def _resolve_directory_object_references( + self, + policies: Dict[str, "ConditionalAccessPolicy"], + ) -> Tuple[Set[Tuple[str, str]], Set[Tuple[str, str]]]: + """Resolve every user/group/role identifier referenced by CA policies. + + Walks the inclusion/exclusion collections of every loaded Conditional + Access policy, deduplicates the resulting identifiers per type, and + queries Microsoft Graph for each one. Identifiers that return HTTP 404 + are reported as deleted. Non-404 errors (5xx, throttling, permission, + transient network failures) are reported as unresolvable: they must not + be flagged as deletions, but they must also not be silently treated as + clean resolutions, so the downstream check surfaces them as MANUAL. + + The sentinel values ``All``, ``None``, and ``GuestsOrExternalUsers`` + are not directory identifiers and are excluded before any Graph call. + + Args: + policies: Conditional Access policies keyed by policy ID. + + Returns: + Tuple[Set[Tuple[str, str]], Set[Tuple[str, str]]]: A pair of + ``(type, identifier)`` sets. The first holds identifiers that + failed to resolve via Graph with HTTP 404 (deleted); the second + holds identifiers whose lookup failed for any other reason and + could be neither confirmed present nor confirmed deleted. + """ + logger.info( + "Entra - Resolving directory-object references in Conditional " + "Access policies..." + ) + + ids_by_type: Dict[str, Set[str]] = { + "user": set(), + "group": set(), + "role": set(), + } + + for policy in policies.values(): + if not getattr(policy, "conditions", None): + continue + user_conditions = getattr(policy.conditions, "user_conditions", None) + if user_conditions is None: + continue + for ident in (user_conditions.included_users or []) + ( + user_conditions.excluded_users or [] + ): + if ident and ident not in CONDITIONAL_ACCESS_SENTINEL_IDS: + ids_by_type["user"].add(ident) + for ident in (user_conditions.included_groups or []) + ( + user_conditions.excluded_groups or [] + ): + if ident and ident not in CONDITIONAL_ACCESS_SENTINEL_IDS: + ids_by_type["group"].add(ident) + for ident in (user_conditions.included_roles or []) + ( + user_conditions.excluded_roles or [] + ): + if ident and ident not in CONDITIONAL_ACCESS_SENTINEL_IDS: + ids_by_type["role"].add(ident) + + unresolved: Set[Tuple[str, str]] = set() + errored: Set[Tuple[str, str]] = set() + + # Resolve types in parallel; within a type, walk identifiers serially + # to keep concurrent Graph calls bounded and avoid throttling. + await gather( + self._resolve_identifiers_for_type( + "user", ids_by_type["user"], unresolved, errored + ), + self._resolve_identifiers_for_type( + "group", ids_by_type["group"], unresolved, errored + ), + self._resolve_identifiers_for_type( + "role", ids_by_type["role"], unresolved, errored + ), + ) + return unresolved, errored + + async def _resolve_identifiers_for_type( + self, + type_: str, + identifiers: Set[str], + unresolved: Set[Tuple[str, str]], + errored: Set[Tuple[str, str]], + ) -> None: + """Resolve a set of identifiers of a given type, mutating the result sets. + + Only HTTP 404 (or ``Request_ResourceNotFound``) responses add to + ``unresolved``. Every other error (5xx, throttling, permission, or an + unexpected exception) is logged and added to ``errored`` so the check + can report it as unverified instead of silently treating it as clean. + """ + for identifier in identifiers: + try: + if type_ == "user": + await self.client.users.by_user_id(identifier).get() + elif type_ == "group": + await self.client.groups.by_group_id(identifier).get() + elif type_ == "role": + await self.client.role_management.directory.role_definitions.by_unified_role_definition_id( + identifier + ).get() + else: + continue + except ODataError as error: + status_code = getattr(error, "response_status_code", None) + error_code = getattr(error.error, "code", None) if error.error else None + if status_code == 404 or error_code == "Request_ResourceNotFound": + unresolved.add((type_, identifier)) + else: + errored.add((type_, identifier)) + logger.warning( + f"Entra - Could not resolve {type_} '{identifier}' for " + f"Conditional Access reference check: " + f"{error.__class__.__name__}: {error}" + ) + except Exception as error: + errored.add((type_, identifier)) + logger.warning( + f"Entra - Unexpected error resolving {type_} '{identifier}' " + f"for Conditional Access reference check: " + f"{error.__class__.__name__}: {error}" + ) + class ConditionalAccessPolicyState(Enum): ENABLED = "enabled" diff --git a/tests/providers/m365/services/entra/entra_conditional_access_policy_no_deleted_object_references/entra_conditional_access_policy_no_deleted_object_references_test.py b/tests/providers/m365/services/entra/entra_conditional_access_policy_no_deleted_object_references/entra_conditional_access_policy_no_deleted_object_references_test.py new file mode 100644 index 0000000000..6d3e7786cf --- /dev/null +++ b/tests/providers/m365/services/entra/entra_conditional_access_policy_no_deleted_object_references/entra_conditional_access_policy_no_deleted_object_references_test.py @@ -0,0 +1,466 @@ +from unittest import mock +from uuid import uuid4 + +from prowler.providers.m365.services.entra.entra_service import ( + ApplicationsConditions, + ConditionalAccessPolicy, + ConditionalAccessPolicyState, + Conditions, + GrantControlOperator, + GrantControls, + PersistentBrowser, + SessionControls, + SignInFrequency, + UsersConditions, +) +from tests.providers.m365.m365_fixtures import DOMAIN, set_mocked_m365_provider + + +def _make_policy( + *, + display_name="Test Policy", + state=ConditionalAccessPolicyState.ENABLED, + included_users=None, + excluded_users=None, + included_groups=None, + excluded_groups=None, + included_roles=None, + excluded_roles=None, +): + """Build a ConditionalAccessPolicy with the minimum fields required by the model.""" + policy_id = str(uuid4()) + policy = ConditionalAccessPolicy( + id=policy_id, + display_name=display_name, + conditions=Conditions( + application_conditions=ApplicationsConditions( + included_applications=[], + excluded_applications=[], + included_user_actions=[], + ), + user_conditions=UsersConditions( + included_users=included_users or [], + excluded_users=excluded_users or [], + included_groups=included_groups or [], + excluded_groups=excluded_groups or [], + included_roles=included_roles or [], + excluded_roles=excluded_roles or [], + ), + client_app_types=[], + ), + grant_controls=GrantControls( + built_in_controls=[], + operator=GrantControlOperator.OR, + authentication_strength=None, + ), + session_controls=SessionControls( + persistent_browser=PersistentBrowser(is_enabled=False, mode=""), + sign_in_frequency=SignInFrequency( + is_enabled=False, frequency=None, type=None, interval=None + ), + ), + state=state, + ) + return policy_id, policy + + +def _entra_client_mock(): + client = mock.MagicMock() + client.audited_tenant = "audited_tenant" + client.audited_domain = DOMAIN + # Default to clean resolution; individual tests override as needed. + client.unresolved_directory_object_references = set() + client.errored_directory_object_references = set() + return client + + +CHECK_MODULE = ( + "prowler.providers.m365.services.entra." + "entra_conditional_access_policy_no_deleted_object_references." + "entra_conditional_access_policy_no_deleted_object_references.entra_client" +) + + +class Test_entra_conditional_access_policy_no_deleted_object_references: + def test_no_policies(self): + """No Conditional Access policies in tenant: no findings.""" + entra_client = _entra_client_mock() + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_m365_provider(), + ), + mock.patch(CHECK_MODULE, new=entra_client), + ): + from prowler.providers.m365.services.entra.entra_conditional_access_policy_no_deleted_object_references.entra_conditional_access_policy_no_deleted_object_references import ( + entra_conditional_access_policy_no_deleted_object_references, + ) + + entra_client.conditional_access_policies = {} + entra_client.unresolved_directory_object_references = set() + + check = entra_conditional_access_policy_no_deleted_object_references() + result = check.execute() + + assert len(result) == 0 + + def test_sentinel_only_references_pass(self): + """Policy with only sentinel values ('All', 'GuestsOrExternalUsers') passes.""" + entra_client = _entra_client_mock() + policy_id, policy = _make_policy( + display_name="MFA For All", + included_users=["All"], + excluded_users=["GuestsOrExternalUsers"], + ) + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_m365_provider(), + ), + mock.patch(CHECK_MODULE, new=entra_client), + ): + from prowler.providers.m365.services.entra.entra_conditional_access_policy_no_deleted_object_references.entra_conditional_access_policy_no_deleted_object_references import ( + entra_conditional_access_policy_no_deleted_object_references, + ) + + entra_client.conditional_access_policies = {policy_id: policy} + entra_client.unresolved_directory_object_references = set() + + check = entra_conditional_access_policy_no_deleted_object_references() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "PASS" + assert ( + "references no deleted directory objects" in result[0].status_extended + ) + assert result[0].resource_id == policy_id + assert result[0].resource_name == "MFA For All" + + def test_all_references_resolve_pass(self): + """Policy with real identifiers, none in the unresolved set: PASS.""" + entra_client = _entra_client_mock() + live_user = str(uuid4()) + live_group = str(uuid4()) + policy_id, policy = _make_policy( + display_name="Targeted Policy", + included_users=[live_user], + included_groups=[live_group], + ) + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_m365_provider(), + ), + mock.patch(CHECK_MODULE, new=entra_client), + ): + from prowler.providers.m365.services.entra.entra_conditional_access_policy_no_deleted_object_references.entra_conditional_access_policy_no_deleted_object_references import ( + entra_conditional_access_policy_no_deleted_object_references, + ) + + entra_client.conditional_access_policies = {policy_id: policy} + entra_client.unresolved_directory_object_references = set() + + check = entra_conditional_access_policy_no_deleted_object_references() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "PASS" + + def test_deleted_user_in_include_fails(self): + """Policy referencing a deleted user in includeUsers fails with type+side reported.""" + entra_client = _entra_client_mock() + deleted_user = str(uuid4()) + policy_id, policy = _make_policy( + display_name="Require MFA", + included_users=[deleted_user], + ) + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_m365_provider(), + ), + mock.patch(CHECK_MODULE, new=entra_client), + ): + from prowler.providers.m365.services.entra.entra_conditional_access_policy_no_deleted_object_references.entra_conditional_access_policy_no_deleted_object_references import ( + entra_conditional_access_policy_no_deleted_object_references, + ) + + entra_client.conditional_access_policies = {policy_id: policy} + entra_client.unresolved_directory_object_references = { + ("user", deleted_user) + } + + check = entra_conditional_access_policy_no_deleted_object_references() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "FAIL" + assert "1 deleted directory object(s)" in result[0].status_extended + assert "users:" in result[0].status_extended + assert deleted_user in result[0].status_extended + assert "(include)" in result[0].status_extended + + def test_deleted_group_in_exclude_fails(self): + """Policy referencing a deleted group in excludeGroups fails with exclude side reported.""" + entra_client = _entra_client_mock() + deleted_group = str(uuid4()) + policy_id, policy = _make_policy( + display_name="Block Legacy Auth", + included_users=["All"], + excluded_groups=[deleted_group], + ) + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_m365_provider(), + ), + mock.patch(CHECK_MODULE, new=entra_client), + ): + from prowler.providers.m365.services.entra.entra_conditional_access_policy_no_deleted_object_references.entra_conditional_access_policy_no_deleted_object_references import ( + entra_conditional_access_policy_no_deleted_object_references, + ) + + entra_client.conditional_access_policies = {policy_id: policy} + entra_client.unresolved_directory_object_references = { + ("group", deleted_group) + } + + check = entra_conditional_access_policy_no_deleted_object_references() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "FAIL" + assert "groups:" in result[0].status_extended + assert "(exclude)" in result[0].status_extended + + def test_deleted_role_in_disabled_policy_still_fails(self): + """Disabled policy with a stale role reference still FAILs (per spec).""" + entra_client = _entra_client_mock() + deleted_role = str(uuid4()) + policy_id, policy = _make_policy( + display_name="Legacy Admin Policy", + state=ConditionalAccessPolicyState.DISABLED, + included_roles=[deleted_role], + ) + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_m365_provider(), + ), + mock.patch(CHECK_MODULE, new=entra_client), + ): + from prowler.providers.m365.services.entra.entra_conditional_access_policy_no_deleted_object_references.entra_conditional_access_policy_no_deleted_object_references import ( + entra_conditional_access_policy_no_deleted_object_references, + ) + + entra_client.conditional_access_policies = {policy_id: policy} + entra_client.unresolved_directory_object_references = { + ("role", deleted_role) + } + + check = entra_conditional_access_policy_no_deleted_object_references() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "FAIL" + assert "roles:" in result[0].status_extended + assert deleted_role in result[0].status_extended + + def test_orphans_grouped_by_type_across_collections(self): + """A single policy with orphans of every type aggregates them grouped by type.""" + entra_client = _entra_client_mock() + deleted_user = str(uuid4()) + deleted_group = str(uuid4()) + deleted_role = str(uuid4()) + policy_id, policy = _make_policy( + display_name="Composite Policy", + included_users=[deleted_user], + excluded_groups=[deleted_group], + included_roles=[deleted_role], + ) + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_m365_provider(), + ), + mock.patch(CHECK_MODULE, new=entra_client), + ): + from prowler.providers.m365.services.entra.entra_conditional_access_policy_no_deleted_object_references.entra_conditional_access_policy_no_deleted_object_references import ( + entra_conditional_access_policy_no_deleted_object_references, + ) + + entra_client.conditional_access_policies = {policy_id: policy} + entra_client.unresolved_directory_object_references = { + ("user", deleted_user), + ("group", deleted_group), + ("role", deleted_role), + } + + check = entra_conditional_access_policy_no_deleted_object_references() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "FAIL" + assert "3 deleted directory object(s)" in result[0].status_extended + assert "users:" in result[0].status_extended + assert "groups:" in result[0].status_extended + assert "roles:" in result[0].status_extended + + def test_report_only_policy_failure_notes_mode(self): + """A report-only policy with an orphan FAILs and flags the not-yet-enforced state.""" + entra_client = _entra_client_mock() + deleted_user = str(uuid4()) + policy_id, policy = _make_policy( + display_name="Report Only MFA", + state=ConditionalAccessPolicyState.ENABLED_FOR_REPORTING, + included_users=[deleted_user], + ) + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_m365_provider(), + ), + mock.patch(CHECK_MODULE, new=entra_client), + ): + from prowler.providers.m365.services.entra.entra_conditional_access_policy_no_deleted_object_references.entra_conditional_access_policy_no_deleted_object_references import ( + entra_conditional_access_policy_no_deleted_object_references, + ) + + entra_client.conditional_access_policies = {policy_id: policy} + entra_client.unresolved_directory_object_references = { + ("user", deleted_user) + } + + check = entra_conditional_access_policy_no_deleted_object_references() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "FAIL" + assert "report-only mode" in result[0].status_extended + + def test_unverified_reference_is_manual(self): + """A reference that errored (non-404) yields MANUAL, not PASS.""" + entra_client = _entra_client_mock() + errored_group = str(uuid4()) + policy_id, policy = _make_policy( + display_name="Throttled Lookup Policy", + included_users=["All"], + excluded_groups=[errored_group], + ) + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_m365_provider(), + ), + mock.patch(CHECK_MODULE, new=entra_client), + ): + from prowler.providers.m365.services.entra.entra_conditional_access_policy_no_deleted_object_references.entra_conditional_access_policy_no_deleted_object_references import ( + entra_conditional_access_policy_no_deleted_object_references, + ) + + entra_client.conditional_access_policies = {policy_id: policy} + entra_client.unresolved_directory_object_references = set() + entra_client.errored_directory_object_references = { + ("group", errored_group) + } + + check = entra_conditional_access_policy_no_deleted_object_references() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "MANUAL" + assert "could not be fully evaluated" in result[0].status_extended + assert errored_group in result[0].status_extended + + def test_orphan_takes_precedence_over_unverified(self): + """A confirmed deletion FAILs even when another reference is unverified.""" + entra_client = _entra_client_mock() + deleted_user = str(uuid4()) + errored_group = str(uuid4()) + policy_id, policy = _make_policy( + display_name="Mixed Policy", + included_users=[deleted_user], + excluded_groups=[errored_group], + ) + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_m365_provider(), + ), + mock.patch(CHECK_MODULE, new=entra_client), + ): + from prowler.providers.m365.services.entra.entra_conditional_access_policy_no_deleted_object_references.entra_conditional_access_policy_no_deleted_object_references import ( + entra_conditional_access_policy_no_deleted_object_references, + ) + + entra_client.conditional_access_policies = {policy_id: policy} + entra_client.unresolved_directory_object_references = { + ("user", deleted_user) + } + entra_client.errored_directory_object_references = { + ("group", errored_group) + } + + check = entra_conditional_access_policy_no_deleted_object_references() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "FAIL" + assert "1 deleted directory object(s)" in result[0].status_extended + assert "could not be verified" in result[0].status_extended + + def test_multiple_policies_mixed(self): + """Two policies: one clean, one with an orphan. Distinct PASS/FAIL findings.""" + entra_client = _entra_client_mock() + deleted_user = str(uuid4()) + + clean_id, clean_policy = _make_policy( + display_name="Clean Policy", + included_users=["All"], + ) + dirty_id, dirty_policy = _make_policy( + display_name="Stale Reference Policy", + excluded_users=[deleted_user], + ) + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_m365_provider(), + ), + mock.patch(CHECK_MODULE, new=entra_client), + ): + from prowler.providers.m365.services.entra.entra_conditional_access_policy_no_deleted_object_references.entra_conditional_access_policy_no_deleted_object_references import ( + entra_conditional_access_policy_no_deleted_object_references, + ) + + entra_client.conditional_access_policies = { + clean_id: clean_policy, + dirty_id: dirty_policy, + } + entra_client.unresolved_directory_object_references = { + ("user", deleted_user) + } + + check = entra_conditional_access_policy_no_deleted_object_references() + result = check.execute() + + assert len(result) == 2 + + clean_result = next(r for r in result if r.resource_id == clean_id) + dirty_result = next(r for r in result if r.resource_id == dirty_id) + + assert clean_result.status == "PASS" + assert dirty_result.status == "FAIL" + assert "(exclude)" in dirty_result.status_extended diff --git a/tests/providers/m365/services/entra/microsoft365_entra_service_test.py b/tests/providers/m365/services/entra/microsoft365_entra_service_test.py index 4643ea93cc..ba6247c7bd 100644 --- a/tests/providers/m365/services/entra/microsoft365_entra_service_test.py +++ b/tests/providers/m365/services/entra/microsoft365_entra_service_test.py @@ -878,3 +878,189 @@ class Test_Entra_Service: assert merged.password_credentials[0].key_id == "cred-app" assert merged.password_credentials[0].display_name == "app-level-secret" assert merged.password_credentials[0].is_active() + + def test__resolve_identifiers_for_type_flags_only_404(self): + """Only HTTP 404 / Request_ResourceNotFound mark an id as deleted. + + Transient errors (5xx, throttling) and successful resolutions must + never be added to the unresolved set — that is the contract the check + relies on to avoid false positives during Graph outages. + """ + from msgraph.generated.models.o_data_errors.main_error import MainError + from msgraph.generated.models.o_data_errors.o_data_error import ODataError + + deleted_by_status = "deleted-status-404" + deleted_by_code = "deleted-code-rnf" + transient = "transient-503" + live = "live-user" + + error_404 = ODataError() + error_404.response_status_code = 404 + error_404.error = None # status code alone is enough + + error_rnf = ODataError() + error_rnf.response_status_code = None + error_rnf.error = MainError() + error_rnf.error.code = "Request_ResourceNotFound" + + error_503 = ODataError() + error_503.response_status_code = 503 + error_503.error = MainError() + error_503.error.code = "ServiceUnavailable" + + user_builders = { + deleted_by_status: SimpleNamespace(get=AsyncMock(side_effect=error_404)), + deleted_by_code: SimpleNamespace(get=AsyncMock(side_effect=error_rnf)), + transient: SimpleNamespace(get=AsyncMock(side_effect=error_503)), + live: SimpleNamespace(get=AsyncMock(return_value=SimpleNamespace(id=live))), + } + + entra_service = Entra.__new__(Entra) + entra_service.client = SimpleNamespace( + users=SimpleNamespace( + by_user_id=MagicMock(side_effect=lambda uid: user_builders[uid]) + ) + ) + + unresolved = set() + errored = set() + asyncio.run( + entra_service._resolve_identifiers_for_type( + "user", set(user_builders), unresolved, errored + ) + ) + + assert unresolved == { + ("user", deleted_by_status), + ("user", deleted_by_code), + } + # The transient 503 must be recorded as errored (unverified), never as + # deleted and never silently dropped. + assert errored == {("user", transient)} + + def test__resolve_identifiers_for_type_role_uses_role_definitions_endpoint(self): + """A deleted role is resolved against the roleDefinitions endpoint.""" + from msgraph.generated.models.o_data_errors.o_data_error import ODataError + + deleted_role = "deleted-role-id" + + error_404 = ODataError() + error_404.response_status_code = 404 + error_404.error = None + + by_role_id = MagicMock( + return_value=SimpleNamespace(get=AsyncMock(side_effect=error_404)) + ) + + entra_service = Entra.__new__(Entra) + entra_service.client = SimpleNamespace( + role_management=SimpleNamespace( + directory=SimpleNamespace( + role_definitions=SimpleNamespace( + by_unified_role_definition_id=by_role_id + ) + ) + ) + ) + + unresolved = set() + errored = set() + asyncio.run( + entra_service._resolve_identifiers_for_type( + "role", {deleted_role}, unresolved, errored + ) + ) + + assert unresolved == {("role", deleted_role)} + assert errored == set() + by_role_id.assert_called_once_with(deleted_role) + + def test__resolve_directory_object_references_skips_sentinels_and_dedups(self): + """End-to-end resolver: sentinels are never queried, ids are deduped + across policies, and only deleted ids land in the unresolved set.""" + from msgraph.generated.models.o_data_errors.o_data_error import ODataError + + deleted_user = "deleted-user-id" + live_user = "live-user-id" + deleted_group = "deleted-group-id" + errored_group = "errored-group-id" + + def _user_conditions(**kwargs): + base = { + "included_users": [], + "excluded_users": [], + "included_groups": [], + "excluded_groups": [], + "included_roles": [], + "excluded_roles": [], + } + base.update(kwargs) + return SimpleNamespace(**base) + + def _policy(user_conditions): + return SimpleNamespace( + conditions=SimpleNamespace(user_conditions=user_conditions) + ) + + policies = { + "policy-a": _policy( + _user_conditions( + included_users=["All", deleted_user, live_user], + excluded_groups=[deleted_group, errored_group], + ) + ), + # Same deleted_user referenced again — must be resolved only once. + "policy-b": _policy( + _user_conditions( + included_users=[deleted_user], + excluded_users=["GuestsOrExternalUsers"], + ) + ), + # Policy without user conditions must be skipped without error. + "policy-c": SimpleNamespace( + conditions=SimpleNamespace(user_conditions=None) + ), + } + + error_404 = ODataError() + error_404.response_status_code = 404 + error_404.error = None + + error_503 = ODataError() + error_503.response_status_code = 503 + error_503.error = None + + user_builders = { + deleted_user: SimpleNamespace(get=AsyncMock(side_effect=error_404)), + live_user: SimpleNamespace( + get=AsyncMock(return_value=SimpleNamespace(id=live_user)) + ), + } + group_builders = { + deleted_group: SimpleNamespace(get=AsyncMock(side_effect=error_404)), + errored_group: SimpleNamespace(get=AsyncMock(side_effect=error_503)), + } + by_user_id = MagicMock(side_effect=lambda uid: user_builders[uid]) + by_group_id = MagicMock(side_effect=lambda gid: group_builders[gid]) + + entra_service = Entra.__new__(Entra) + entra_service.client = SimpleNamespace( + users=SimpleNamespace(by_user_id=by_user_id), + groups=SimpleNamespace(by_group_id=by_group_id), + ) + + unresolved, errored = asyncio.run( + entra_service._resolve_directory_object_references(policies) + ) + + assert unresolved == { + ("user", deleted_user), + ("group", deleted_group), + } + # The 503 group is unverified, not deleted — it lands in errored. + assert errored == {("group", errored_group)} + # Sentinels are filtered before any Graph call; only the two real user + # ids are queried, and the deduped deleted_user is queried exactly once. + queried_users = {call.args[0] for call in by_user_id.call_args_list} + assert queried_users == {deleted_user, live_user} + assert user_builders[deleted_user].get.await_count == 1