From 65a1436d8e0c6e5a144444033335649ca38cb7f3 Mon Sep 17 00:00:00 2001 From: "Hugo P.Brito" Date: Thu, 9 Apr 2026 15:17:46 +0100 Subject: [PATCH] fix(m365): tighten device filter matching --- ...ccess_policy_sign_in_frequency_enforced.py | 42 ++++-- ..._policy_sign_in_frequency_enforced_test.py | 8 +- .../entra/microsoft365_entra_service_test.py | 127 ++++++++++++++++++ 3 files changed, 160 insertions(+), 17 deletions(-) diff --git a/prowler/providers/m365/services/entra/entra_conditional_access_policy_sign_in_frequency_enforced/entra_conditional_access_policy_sign_in_frequency_enforced.py b/prowler/providers/m365/services/entra/entra_conditional_access_policy_sign_in_frequency_enforced/entra_conditional_access_policy_sign_in_frequency_enforced.py index 62ac4d4993..a9586aafb2 100644 --- a/prowler/providers/m365/services/entra/entra_conditional_access_policy_sign_in_frequency_enforced/entra_conditional_access_policy_sign_in_frequency_enforced.py +++ b/prowler/providers/m365/services/entra/entra_conditional_access_policy_sign_in_frequency_enforced/entra_conditional_access_policy_sign_in_frequency_enforced.py @@ -22,16 +22,22 @@ class entra_conditional_access_policy_sign_in_frequency_enforced(Check): non-corporate devices. """ - NON_CORPORATE_INCLUDE_PATTERNS = ( - r"device\.iscompliant\s*-ne\s*true", - r"device\.iscompliant\s*-eq\s*false", - r'device\.trusttype\s*-ne\s*"serverad"', - r"device\.trusttype\s*-ne\s*'serverad'", + NON_CORPORATE_INCLUDE_PATTERN_GROUPS = ( + ( + r"device\.iscompliant\s*-ne\s*true", + r"device\.iscompliant\s*-eq\s*false", + ), + ( + r'device\.trusttype\s*-ne\s*"serverad"', + r"device\.trusttype\s*-ne\s*'serverad'", + ), ) - CORPORATE_EXCLUDE_PATTERNS = ( - r"device\.iscompliant\s*-eq\s*true", - r'device\.trusttype\s*-eq\s*"serverad"', - r"device\.trusttype\s*-eq\s*'serverad'", + CORPORATE_EXCLUDE_PATTERN_GROUPS = ( + (r"device\.iscompliant\s*-eq\s*true",), + ( + r'device\.trusttype\s*-eq\s*"serverad"', + r"device\.trusttype\s*-eq\s*'serverad'", + ), ) def _targets_all_users(self, included_users: list[str]) -> bool: @@ -77,13 +83,23 @@ class entra_conditional_access_policy_sign_in_frequency_enforced(Check): return False rule = device_conditions.device_filter_rule.lower() if device_conditions.device_filter_mode == DeviceFilterMode.INCLUDE: - return self._rule_matches_any(rule, self.NON_CORPORATE_INCLUDE_PATTERNS) + return self._rule_matches_required_groups( + rule, self.NON_CORPORATE_INCLUDE_PATTERN_GROUPS + ) elif device_conditions.device_filter_mode == DeviceFilterMode.EXCLUDE: - return self._rule_matches_any(rule, self.CORPORATE_EXCLUDE_PATTERNS) + return self._rule_matches_required_groups( + rule, self.CORPORATE_EXCLUDE_PATTERN_GROUPS + ) return False - def _rule_matches_any(self, rule: str, patterns: tuple[str, ...]) -> bool: - return any(re.search(pattern, rule) for pattern in patterns) + def _rule_matches_required_groups( + self, rule: str, pattern_groups: tuple[tuple[str, ...], ...] + ) -> bool: + """Return True when the rule matches at least one pattern from each group.""" + return all( + any(re.search(pattern, rule) for pattern in group) + for group in pattern_groups + ) def execute(self) -> list[CheckReportM365]: """Execute the check for sign-in frequency enforcement in Conditional Access policies. diff --git a/tests/providers/m365/services/entra/entra_conditional_access_policy_sign_in_frequency_enforced/entra_conditional_access_policy_sign_in_frequency_enforced_test.py b/tests/providers/m365/services/entra/entra_conditional_access_policy_sign_in_frequency_enforced/entra_conditional_access_policy_sign_in_frequency_enforced_test.py index cdd0f53587..2d3588b671 100644 --- a/tests/providers/m365/services/entra/entra_conditional_access_policy_sign_in_frequency_enforced/entra_conditional_access_policy_sign_in_frequency_enforced_test.py +++ b/tests/providers/m365/services/entra/entra_conditional_access_policy_sign_in_frequency_enforced/entra_conditional_access_policy_sign_in_frequency_enforced_test.py @@ -616,7 +616,7 @@ class Test_entra_conditional_access_policy_sign_in_frequency_enforced: policy_id=policy_id_2, display_name=display_name_2, device_filter_mode=DeviceFilterMode.INCLUDE, - device_filter_rule='device.trustType -ne "ServerAD"', + device_filter_rule='device.isCompliant -ne True -or device.trustType -ne "ServerAD"', ), } @@ -629,7 +629,7 @@ class Test_entra_conditional_access_policy_sign_in_frequency_enforced: assert result[0].resource_id == policy_id_2 def test_entra_policy_with_trust_type_only(self): - """Test PASS with device filter referencing only trustType.""" + """Test FAIL when device filter references only trustType.""" policy_id = str(uuid4()) display_name = "TrustType Filter Policy" entra_client = mock.MagicMock @@ -663,5 +663,5 @@ class Test_entra_conditional_access_policy_sign_in_frequency_enforced: result = check.execute() assert len(result) == 1 - assert result[0].status == "PASS" - assert result[0].resource_name == display_name + assert result[0].status == "FAIL" + assert result[0].resource_name == "Conditional Access Policies" 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 d83219793e..d7dc507e84 100644 --- a/tests/providers/m365/services/entra/microsoft365_entra_service_test.py +++ b/tests/providers/m365/services/entra/microsoft365_entra_service_test.py @@ -18,12 +18,15 @@ from prowler.providers.m365.services.entra.entra_service import ( CredentialRestriction, DefaultAppManagementPolicy, DefaultUserRolePermissions, + DeviceConditions, + DeviceFilterMode, Entra, GrantControlOperator, GrantControls, InvitationsFrom, Organization, PersistentBrowser, + PlatformConditions, SessionControls, SignInFrequency, SignInFrequencyInterval, @@ -286,6 +289,130 @@ class Test_Entra_Service: ) } + def test__get_conditional_access_policies_parses_device_filter(self): + from msgraph.generated.models.filter_mode import FilterMode + + entra_service = Entra.__new__(Entra) + entra_service._get_raw_authentication_flows = AsyncMock(return_value={}) + + policy = SimpleNamespace( + id="id-1", + display_name="Policy with Device Filter", + conditions=SimpleNamespace( + applications=SimpleNamespace( + include_applications=["All"], + exclude_applications=[], + include_user_actions=[], + ), + users=SimpleNamespace( + include_groups=[], + exclude_groups=[], + include_users=["All"], + exclude_users=[], + include_roles=[], + exclude_roles=[], + ), + client_app_types=[], + user_risk_levels=[], + sign_in_risk_levels=[], + insider_risk_levels=None, + platforms=None, + devices=SimpleNamespace( + device_filter=SimpleNamespace( + mode=FilterMode.Include, + rule='device.isCompliant -ne True -or device.trustType -ne "ServerAD"', + ) + ), + ), + grant_controls=SimpleNamespace( + built_in_controls=[], + operator="AND", + authentication_strength=None, + ), + session_controls=SimpleNamespace( + persistent_browser=SimpleNamespace( + is_enabled=False, + mode="always", + ), + sign_in_frequency=SimpleNamespace( + is_enabled=True, + value=1, + type="hours", + frequency_interval="timeBased", + ), + application_enforced_restrictions=SimpleNamespace(is_enabled=False), + ), + state="enabled", + ) + + entra_service.client = SimpleNamespace( + identity=SimpleNamespace( + conditional_access=SimpleNamespace( + policies=SimpleNamespace( + get=AsyncMock(return_value=SimpleNamespace(value=[policy])) + ) + ) + ) + ) + + conditional_access_policies = asyncio.run( + entra_service._get_conditional_access_policies() + ) + + assert conditional_access_policies["id-1"] == ConditionalAccessPolicy( + id="id-1", + display_name="Policy with Device Filter", + conditions=Conditions( + application_conditions=ApplicationsConditions( + included_applications=["All"], + excluded_applications=[], + included_user_actions=[], + ), + user_conditions=UsersConditions( + included_groups=[], + excluded_groups=[], + included_users=["All"], + excluded_users=[], + included_roles=[], + excluded_roles=[], + ), + client_app_types=[], + user_risk_levels=[], + sign_in_risk_levels=[], + insider_risk_levels=None, + platform_conditions=PlatformConditions( + include_platforms=[], + exclude_platforms=[], + ), + authentication_flows=None, + device_conditions=DeviceConditions( + device_filter_mode=DeviceFilterMode.INCLUDE, + device_filter_rule='device.isCompliant -ne True -or device.trustType -ne "ServerAD"', + ), + ), + grant_controls=GrantControls( + built_in_controls=[], + operator=GrantControlOperator.AND, + authentication_strength=None, + ), + session_controls=SessionControls( + persistent_browser=PersistentBrowser( + is_enabled=False, + mode="always", + ), + sign_in_frequency=SignInFrequency( + is_enabled=True, + frequency=1, + type=SignInFrequencyType.HOURS, + interval=SignInFrequencyInterval.TIME_BASED, + ), + application_enforced_restrictions=ApplicationEnforcedRestrictions( + is_enabled=False + ), + ), + state=ConditionalAccessPolicyState.ENABLED, + ) + @patch( "prowler.providers.m365.services.entra.entra_service.Entra._get_groups", new=mock_entra_get_groups,