From ad3d4536fb40d613f247a849c6a989c3e94ab3ed Mon Sep 17 00:00:00 2001 From: Hugo Pereira Brito <101209179+HugoPBrito@users.noreply.github.com> Date: Wed, 20 Aug 2025 16:45:00 +0200 Subject: [PATCH] fix(m365): only evaluate enabled users in `entra_users_mfa_capable` (#8544) --- prowler/CHANGELOG.md | 1 + .../m365/lib/powershell/m365_powershell.py | 14 +++ .../m365/services/entra/entra_service.py | 8 ++ .../entra_users_mfa_capable.py | 27 +++--- .../entra_users_mfa_capable_test.py | 94 +++++++++++++++++++ 5 files changed, 131 insertions(+), 13 deletions(-) diff --git a/prowler/CHANGELOG.md b/prowler/CHANGELOG.md index 6981aed941..7892cab44f 100644 --- a/prowler/CHANGELOG.md +++ b/prowler/CHANGELOG.md @@ -28,6 +28,7 @@ All notable changes to the **Prowler SDK** are documented in this file. - AWS resource-arn filtering [(#8533)](https://github.com/prowler-cloud/prowler/pull/8533) - GitHub App authentication for GitHub provider [(#8529)](https://github.com/prowler-cloud/prowler/pull/8529) - List all accessible organizations in GitHub provider [(#8535)](https://github.com/prowler-cloud/prowler/pull/8535) +- Only evaluate enabled accounts in `entra_users_mfa_capable` check [(#8544)](https://github.com/prowler-cloud/prowler/pull/8544) --- diff --git a/prowler/providers/m365/lib/powershell/m365_powershell.py b/prowler/providers/m365/lib/powershell/m365_powershell.py index 3cd2dda4df..26380ae2a2 100644 --- a/prowler/providers/m365/lib/powershell/m365_powershell.py +++ b/prowler/providers/m365/lib/powershell/m365_powershell.py @@ -982,6 +982,20 @@ class M365PowerShell(PowerShellSession): """ return self.execute("Get-SharingPolicy | ConvertTo-Json", json_parse=True) + def get_user_account_status(self) -> dict: + """ + Get User Account Status. + + Retrieves the current user account status settings for Exchange Online. + + Returns: + dict: User account status settings in JSON format. + """ + return self.execute( + "$dict=@{}; Get-User -ResultSize Unlimited | ForEach-Object { $dict[$_.Id] = @{ AccountDisabled = $_.AccountDisabled } }; $dict | ConvertTo-Json", + json_parse=True, + ) + # This function is used to install the required M365 PowerShell modules in Docker containers def initialize_m365_powershell_modules(): diff --git a/prowler/providers/m365/services/entra/entra_service.py b/prowler/providers/m365/services/entra/entra_service.py index e7159e1787..2cf160a940 100644 --- a/prowler/providers/m365/services/entra/entra_service.py +++ b/prowler/providers/m365/services/entra/entra_service.py @@ -14,7 +14,10 @@ from prowler.providers.m365.m365_provider import M365Provider class Entra(M365Service): def __init__(self, provider: M365Provider): super().__init__(provider) + if self.powershell: + self.powershell.connect_exchange_online() + self.user_accounts_status = self.powershell.get_user_account_status() self.powershell.close() loop = get_event_loop() @@ -36,6 +39,7 @@ class Entra(M365Service): self.groups = attributes[3] self.organizations = attributes[4] self.users = attributes[5] + self.user_accounts_status = {} async def _get_authorization_policy(self): logger.info("Entra - Getting authorization policy...") @@ -405,6 +409,9 @@ class Entra(M365Service): if registration_details.get(user.id, None) is not None else False ), + account_enabled=not self.user_accounts_status.get(user.id, {}).get( + "AccountDisabled", False + ), ) except Exception as error: logger.error( @@ -585,6 +592,7 @@ class User(BaseModel): on_premises_sync_enabled: bool directory_roles_ids: List[str] = [] is_mfa_capable: bool = False + account_enabled: bool = True 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 8345c6963e..4b4075aa11 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 @@ -26,20 +26,21 @@ class entra_users_mfa_capable(Check): findings = [] for user in entra_client.users.values(): - report = CheckReportM365( - metadata=self.metadata(), - resource=user, - resource_name=user.name, - resource_id=user.id, - ) + if user.account_enabled: + report = CheckReportM365( + metadata=self.metadata(), + resource=user, + resource_name=user.name, + resource_id=user.id, + ) - 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." + 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) + 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 4415628b2c..b84b8976ae 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 @@ -34,6 +34,7 @@ class Test_entra_users_mfa_capable: on_premises_sync_enabled=False, directory_roles_ids=[], is_mfa_capable=False, + account_enabled=True, ) } @@ -75,6 +76,7 @@ class Test_entra_users_mfa_capable: on_premises_sync_enabled=False, directory_roles_ids=[], is_mfa_capable=True, + account_enabled=True, ) } @@ -117,6 +119,7 @@ class Test_entra_users_mfa_capable: on_premises_sync_enabled=False, directory_roles_ids=[], is_mfa_capable=True, + account_enabled=True, ), user2_id: User( id=user2_id, @@ -124,6 +127,7 @@ class Test_entra_users_mfa_capable: on_premises_sync_enabled=False, directory_roles_ids=[], is_mfa_capable=False, + account_enabled=True, ), } @@ -143,3 +147,93 @@ class Test_entra_users_mfa_capable: assert result[1].resource == entra_client.users[user2_id] assert result[1].resource_name == "Test User 2" assert result[1].resource_id == user2_id + + def test_disabled_user_not_checked(self): + """Disabled user should not be checked: 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="Disabled User", + on_premises_sync_enabled=False, + directory_roles_ids=[], + is_mfa_capable=False, + account_enabled=False, # Disabled user + ) + } + + check = entra_users_mfa_capable() + result = check.execute() + + # No results should be returned for disabled users + assert len(result) == 0 + + def test_mixed_enabled_disabled_users(self): + """Mix of enabled and disabled users: only enabled 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, + ) + + enabled_user_id = str(uuid4()) + disabled_user_id = str(uuid4()) + entra_client.users = { + enabled_user_id: User( + id=enabled_user_id, + name="Enabled User", + on_premises_sync_enabled=False, + directory_roles_ids=[], + is_mfa_capable=True, + account_enabled=True, # Enabled user + ), + disabled_user_id: User( + id=disabled_user_id, + name="Disabled User", + on_premises_sync_enabled=False, + directory_roles_ids=[], + is_mfa_capable=False, + account_enabled=False, # Disabled user + ), + } + + check = entra_users_mfa_capable() + result = check.execute() + + # Only the enabled user should be checked + assert len(result) == 1 + assert result[0].status == "PASS" + assert result[0].status_extended == "User Enabled User is 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