diff --git a/prowler/CHANGELOG.md b/prowler/CHANGELOG.md index 9912bdf333..d5dbeeaba3 100644 --- a/prowler/CHANGELOG.md +++ b/prowler/CHANGELOG.md @@ -19,6 +19,7 @@ All notable changes to the **Prowler SDK** are documented in this file. - AWS Organizations metadata retrieval for delegated administrator scans by using the assumed role session instead of the pre-assume credentials [(#10894)](https://github.com/prowler-cloud/prowler/pull/10894) - `admincenter_groups_not_public_visibility` check for M365 provider evaluating Security and Distribution groups, now restricted to Microsoft 365 (Unified) groups per CIS M365 Foundations 1.2.1 [(#10899)](https://github.com/prowler-cloud/prowler/pull/10899) - Google Workspace check reports now store the actual domain or account resource subject instead of `provider.identity` [(#10901)](https://github.com/prowler-cloud/prowler/pull/10901) +- `entra_users_mfa_capable` evaluating disabled guest accounts; CIS 5.2.3.4 only targets enabled member users [(#10785)](https://github.com/prowler-cloud/prowler/pull/10785) --- diff --git a/prowler/providers/m365/services/entra/entra_service.py b/prowler/providers/m365/services/entra/entra_service.py index 2511e4056e..a6b8451387 100644 --- a/prowler/providers/m365/services/entra/entra_service.py +++ b/prowler/providers/m365/services/entra/entra_service.py @@ -844,6 +844,7 @@ class Entra(M365Service): authentication_methods=reg_info.get( "authentication_methods", [] ), + user_type=getattr(user, "user_type", None), ) next_link = getattr(users_response, "odata_next_link", None) @@ -1409,6 +1410,9 @@ class User(BaseModel): account_enabled: Whether the user account is enabled. authentication_methods: List of authentication method types registered by the user (e.g., 'fido2SecurityKey', 'microsoftAuthenticatorPush', 'mobilePhone'). + user_type: The user account type as reported by Microsoft Graph + (typically 'Member' or 'Guest'). ``None`` when Microsoft Graph does not + return the property; checks must not assume a default in that case. """ id: str @@ -1418,6 +1422,7 @@ class User(BaseModel): is_mfa_capable: bool = False account_enabled: bool = True authentication_methods: List[str] = [] + user_type: Optional[str] = None class InvitationsFrom(Enum): diff --git a/prowler/providers/m365/services/entra/entra_users_mfa_capable/entra_users_mfa_capable.py b/prowler/providers/m365/services/entra/entra_users_mfa_capable/entra_users_mfa_capable.py index 4b4075aa11..58a38d14d5 100644 --- a/prowler/providers/m365/services/entra/entra_users_mfa_capable/entra_users_mfa_capable.py +++ b/prowler/providers/m365/services/entra/entra_users_mfa_capable/entra_users_mfa_capable.py @@ -6,41 +6,49 @@ from prowler.providers.m365.services.entra.entra_client import entra_client class entra_users_mfa_capable(Check): """ - Ensure all users are MFA capable. + Ensure all member users are MFA capable. - This check verifies if users are MFA capable. + This check verifies if member users are MFA capable, aligning with CIS + Microsoft 365 Foundations Benchmark recommendation 5.2.3.4 + ("Ensure all member users are 'MFA capable'"). - The check fails if any user is not MFA capable. + Guest users and disabled accounts are excluded from the evaluation. """ def execute(self) -> List[CheckReportM365]: """ - Execute the admin MFA capable check for all users. + Execute the MFA capable check for all enabled member users. Iterates over the users retrieved from the Entra client and generates a report - indicating if users are MFA capable. + indicating if member users are MFA capable. Users explicitly typed as ``Guest`` + and disabled accounts are skipped, in line with the CIS recommendation that + scopes the control to member users only. Users whose ``user_type`` could not + be determined are still evaluated to avoid silently dropping accounts when + Microsoft Graph does not return the property. Returns: - List[CheckReportM365]: A list containing a single report with the result of the check. + List[CheckReportM365]: A list with one report per evaluated user. """ findings = [] for user in entra_client.users.values(): - if user.account_enabled: - report = CheckReportM365( - metadata=self.metadata(), - resource=user, - resource_name=user.name, - resource_id=user.id, - ) + if user.user_type == "Guest" or not user.account_enabled: + continue - if not user.is_mfa_capable: - report.status = "FAIL" - report.status_extended = f"User {user.name} is not MFA capable." - else: - report.status = "PASS" - report.status_extended = f"User {user.name} is MFA capable." + report = CheckReportM365( + metadata=self.metadata(), + resource=user, + resource_name=user.name, + resource_id=user.id, + ) - findings.append(report) + if not user.is_mfa_capable: + report.status = "FAIL" + report.status_extended = f"User {user.name} is not MFA capable." + else: + report.status = "PASS" + report.status_extended = f"User {user.name} is MFA capable." + + findings.append(report) return findings diff --git a/tests/providers/m365/services/entra/entra_users_mfa_capable/entra_users_mfa_capable_test.py b/tests/providers/m365/services/entra/entra_users_mfa_capable/entra_users_mfa_capable_test.py index b84b8976ae..28cc266347 100644 --- a/tests/providers/m365/services/entra/entra_users_mfa_capable/entra_users_mfa_capable_test.py +++ b/tests/providers/m365/services/entra/entra_users_mfa_capable/entra_users_mfa_capable_test.py @@ -237,3 +237,186 @@ class Test_entra_users_mfa_capable: assert result[0].resource == entra_client.users[enabled_user_id] assert result[0].resource_name == "Enabled User" assert result[0].resource_id == enabled_user_id + + def test_disabled_guest_user_not_checked(self): + """Disabled guest user should not be checked: expected no results. + + Regression test for https://github.com/prowler-cloud/prowler/issues/10637. + CIS 5.2.3.4 evaluates only enabled member users; disabled guests must be skipped + even when ``account_enabled`` cannot be derived from Exchange Online. + """ + entra_client = mock.MagicMock + entra_client.audited_tenant = "audited_tenant" + entra_client.audited_domain = DOMAIN + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_m365_provider(), + ), + mock.patch( + "prowler.providers.m365.services.entra.entra_users_mfa_capable.entra_users_mfa_capable.entra_client", + new=entra_client, + ), + ): + from prowler.providers.m365.services.entra.entra_users_mfa_capable.entra_users_mfa_capable import ( + entra_users_mfa_capable, + ) + + user_id = str(uuid4()) + entra_client.users = { + user_id: User( + id=user_id, + name="Disabled Guest", + on_premises_sync_enabled=False, + directory_roles_ids=[], + is_mfa_capable=False, + account_enabled=False, + user_type="Guest", + ) + } + + check = entra_users_mfa_capable() + result = check.execute() + + assert len(result) == 0 + + def test_enabled_guest_user_not_checked(self): + """Enabled guest user is out of scope for CIS 5.2.3.4: expected no results.""" + entra_client = mock.MagicMock + entra_client.audited_tenant = "audited_tenant" + entra_client.audited_domain = DOMAIN + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_m365_provider(), + ), + mock.patch( + "prowler.providers.m365.services.entra.entra_users_mfa_capable.entra_users_mfa_capable.entra_client", + new=entra_client, + ), + ): + from prowler.providers.m365.services.entra.entra_users_mfa_capable.entra_users_mfa_capable import ( + entra_users_mfa_capable, + ) + + user_id = str(uuid4()) + entra_client.users = { + user_id: User( + id=user_id, + name="Guest User", + on_premises_sync_enabled=False, + directory_roles_ids=[], + is_mfa_capable=False, + account_enabled=True, + user_type="Guest", + ) + } + + check = entra_users_mfa_capable() + result = check.execute() + + assert len(result) == 0 + + def test_member_and_guest_users(self): + """Mix of member and guest users: only member users should be checked.""" + entra_client = mock.MagicMock + entra_client.audited_tenant = "audited_tenant" + entra_client.audited_domain = DOMAIN + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_m365_provider(), + ), + mock.patch( + "prowler.providers.m365.services.entra.entra_users_mfa_capable.entra_users_mfa_capable.entra_client", + new=entra_client, + ), + ): + from prowler.providers.m365.services.entra.entra_users_mfa_capable.entra_users_mfa_capable import ( + entra_users_mfa_capable, + ) + + member_user_id = str(uuid4()) + guest_user_id = str(uuid4()) + entra_client.users = { + member_user_id: User( + id=member_user_id, + name="Member User", + on_premises_sync_enabled=False, + directory_roles_ids=[], + is_mfa_capable=False, + account_enabled=True, + user_type="Member", + ), + guest_user_id: User( + id=guest_user_id, + name="Guest User", + on_premises_sync_enabled=False, + directory_roles_ids=[], + is_mfa_capable=False, + account_enabled=True, + user_type="Guest", + ), + } + + check = entra_users_mfa_capable() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "FAIL" + assert result[0].status_extended == "User Member User is not MFA capable." + assert result[0].resource == entra_client.users[member_user_id] + assert result[0].resource_name == "Member User" + assert result[0].resource_id == member_user_id + + def test_unknown_user_type_is_evaluated(self): + """Users without a ``user_type`` reported by Microsoft Graph must not be + silently dropped. + + We only skip users that Graph explicitly reports as ``Guest``; for everyone + else (including ``user_type=None``) the check still evaluates MFA capability + so that we never mask findings on accounts whose type cannot be determined. + """ + entra_client = mock.MagicMock + entra_client.audited_tenant = "audited_tenant" + entra_client.audited_domain = DOMAIN + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_m365_provider(), + ), + mock.patch( + "prowler.providers.m365.services.entra.entra_users_mfa_capable.entra_users_mfa_capable.entra_client", + new=entra_client, + ), + ): + from prowler.providers.m365.services.entra.entra_users_mfa_capable.entra_users_mfa_capable import ( + entra_users_mfa_capable, + ) + + user_id = str(uuid4()) + entra_client.users = { + user_id: User( + id=user_id, + name="Test User", + on_premises_sync_enabled=False, + directory_roles_ids=[], + is_mfa_capable=False, + account_enabled=True, + user_type=None, + ) + } + + check = entra_users_mfa_capable() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "FAIL" + assert result[0].status_extended == "User Test User is not MFA capable." + assert result[0].resource == entra_client.users[user_id] + assert result[0].resource_name == "Test User" + assert result[0].resource_id == user_id