From a002ec8190e94d0ef262f2c23c859498f5ff3d1f Mon Sep 17 00:00:00 2001 From: Prowler Bot Date: Mon, 23 Mar 2026 16:30:01 +0100 Subject: [PATCH] fix(sdk): ignore disabled users in Entra MFA check (#10429) Co-authored-by: Hugo Pereira Brito <101209179+HugoPBrito@users.noreply.github.com> --- prowler/CHANGELOG.md | 1 + .../entra_non_privileged_user_has_mfa.py | 2 +- .../azure/services/entra/entra_service.py | 15 +++- .../entra_non_privileged_user_has_mfa_test.py | 80 +++++++++++++++++++ .../services/entra/entra_service_test.py | 14 +++- 5 files changed, 108 insertions(+), 4 deletions(-) diff --git a/prowler/CHANGELOG.md b/prowler/CHANGELOG.md index 2c8b47daf4..9582f7123a 100644 --- a/prowler/CHANGELOG.md +++ b/prowler/CHANGELOG.md @@ -8,6 +8,7 @@ All notable changes to the **Prowler SDK** are documented in this file. - Azure MySQL flexible server checks now compare configuration values case-insensitively to avoid false negatives when Azure returns lowercase values [(#10396)](https://github.com/prowler-cloud/prowler/pull/10396) - Azure `vm_backup_enabled` and `vm_sufficient_daily_backup_retention_period` checks now compare VM names case-insensitively to avoid false negatives when Azure stores backup item names in a different case [(#10395)](https://github.com/prowler-cloud/prowler/pull/10395) +- `entra_non_privileged_user_has_mfa` skips disabled users to avoid false positives [(#10426)](https://github.com/prowler-cloud/prowler/pull/10426) --- diff --git a/prowler/providers/azure/services/entra/entra_non_privileged_user_has_mfa/entra_non_privileged_user_has_mfa.py b/prowler/providers/azure/services/entra/entra_non_privileged_user_has_mfa/entra_non_privileged_user_has_mfa.py index c86fc02da7..d231a7a6b1 100644 --- a/prowler/providers/azure/services/entra/entra_non_privileged_user_has_mfa/entra_non_privileged_user_has_mfa.py +++ b/prowler/providers/azure/services/entra/entra_non_privileged_user_has_mfa/entra_non_privileged_user_has_mfa.py @@ -11,7 +11,7 @@ class entra_non_privileged_user_has_mfa(Check): for tenant_domain, users in entra_client.users.items(): for user in users.values(): - if not is_privileged_user( + if user.account_enabled and not is_privileged_user( user, entra_client.directory_roles[tenant_domain] ): report = Check_Report_Azure(metadata=self.metadata(), resource=user) diff --git a/prowler/providers/azure/services/entra/entra_service.py b/prowler/providers/azure/services/entra/entra_service.py index 011ea675b0..eb1d62ac11 100644 --- a/prowler/providers/azure/services/entra/entra_service.py +++ b/prowler/providers/azure/services/entra/entra_service.py @@ -3,7 +3,9 @@ from asyncio import gather from typing import List, Optional from uuid import UUID +from kiota_abstractions.base_request_configuration import RequestConfiguration from msgraph import GraphServiceClient +from msgraph.generated.users.users_request_builder import UsersRequestBuilder from pydantic.v1 import BaseModel from prowler.lib.logger import logger @@ -65,9 +67,16 @@ class Entra(AzureService): logger.info("Entra - Getting users...") users = {} try: + request_configuration = RequestConfiguration( + query_parameters=UsersRequestBuilder.UsersRequestBuilderGetQueryParameters( + select=["id", "displayName", "accountEnabled"] + ) + ) for tenant, client in self.clients.items(): users.update({tenant: {}}) - users_response = await client.users.get() + users_response = await client.users.get( + request_configuration=request_configuration + ) registration_details = await self._get_user_registration_details(client) try: @@ -81,6 +90,9 @@ class Entra(AzureService): is_mfa_capable=registration_details.get( user.id, False ), + account_enabled=getattr( + user, "account_enabled", True + ), ) } ) @@ -409,6 +421,7 @@ class User(BaseModel): id: str name: str is_mfa_capable: bool = False + account_enabled: bool = True class DefaultUserRolePermissions(BaseModel): diff --git a/tests/providers/azure/services/entra/entra_non_privileged_user_has_mfa/entra_non_privileged_user_has_mfa_test.py b/tests/providers/azure/services/entra/entra_non_privileged_user_has_mfa/entra_non_privileged_user_has_mfa_test.py index 4667b665ed..4d2f289a90 100644 --- a/tests/providers/azure/services/entra/entra_non_privileged_user_has_mfa/entra_non_privileged_user_has_mfa_test.py +++ b/tests/providers/azure/services/entra/entra_non_privileged_user_has_mfa/entra_non_privileged_user_has_mfa_test.py @@ -142,6 +142,86 @@ class Test_entra_non_privileged_user_has_mfa: assert result[0].resource_id == user_id assert result[0].subscription == f"Tenant: {DOMAIN}" + def test_entra_disabled_user_no_privileged_no_mfa(self): + entra_client = mock.MagicMock + user_id = str(uuid4()) + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_azure_provider(), + ), + mock.patch( + "prowler.providers.azure.services.entra.entra_non_privileged_user_has_mfa.entra_non_privileged_user_has_mfa.entra_client", + new=entra_client, + ), + ): + from prowler.providers.azure.services.entra.entra_non_privileged_user_has_mfa.entra_non_privileged_user_has_mfa import ( + entra_non_privileged_user_has_mfa, + ) + from prowler.providers.azure.services.entra.entra_service import ( + DirectoryRole, + User, + ) + + user = User( + id=user_id, + name="foo", + is_mfa_capable=False, + account_enabled=False, + ) + + entra_client.users = {DOMAIN: {f"foo@{DOMAIN}": user}} + entra_client.directory_roles = { + DOMAIN: { + "Global Administrator": DirectoryRole(id=str(uuid4()), members=[]) + } + } + + check = entra_non_privileged_user_has_mfa() + result = check.execute() + assert len(result) == 0 + + def test_entra_disabled_user_no_privileged_mfa(self): + entra_client = mock.MagicMock + user_id = str(uuid4()) + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_azure_provider(), + ), + mock.patch( + "prowler.providers.azure.services.entra.entra_non_privileged_user_has_mfa.entra_non_privileged_user_has_mfa.entra_client", + new=entra_client, + ), + ): + from prowler.providers.azure.services.entra.entra_non_privileged_user_has_mfa.entra_non_privileged_user_has_mfa import ( + entra_non_privileged_user_has_mfa, + ) + from prowler.providers.azure.services.entra.entra_service import ( + DirectoryRole, + User, + ) + + user = User( + id=user_id, + name="foo", + is_mfa_capable=True, + account_enabled=False, + ) + + entra_client.users = {DOMAIN: {f"foo@{DOMAIN}": user}} + entra_client.directory_roles = { + DOMAIN: { + "Global Administrator": DirectoryRole(id=str(uuid4()), members=[]) + } + } + + check = entra_non_privileged_user_has_mfa() + result = check.execute() + assert len(result) == 0 + def test_entra_user_privileged_no_mfa(self): entra_client = mock.MagicMock user_id = str(uuid4()) diff --git a/tests/providers/azure/services/entra/entra_service_test.py b/tests/providers/azure/services/entra/entra_service_test.py index 8e3a25e59f..75ef4f98c4 100644 --- a/tests/providers/azure/services/entra/entra_service_test.py +++ b/tests/providers/azure/services/entra/entra_service_test.py @@ -147,6 +147,7 @@ class Test_Entra_Service: assert entra_client.users[DOMAIN]["user-1@tenant1.es"].id == "id-1" assert entra_client.users[DOMAIN]["user-1@tenant1.es"].name == "User 1" assert entra_client.users[DOMAIN]["user-1@tenant1.es"].is_mfa_capable is False + assert entra_client.users[DOMAIN]["user-1@tenant1.es"].account_enabled is True def test_get_authorization_policy(self): entra_client = Entra(set_mocked_azure_provider()) @@ -229,8 +230,8 @@ def test_azure_entra__get_users_handles_pagination(): entra_service = Entra.__new__(Entra) users_page_one = [ - SimpleNamespace(id="user-1", display_name="User 1"), - SimpleNamespace(id="user-2", display_name="User 2"), + SimpleNamespace(id="user-1", display_name="User 1", account_enabled=False), + SimpleNamespace(id="user-2", display_name="User 2", account_enabled=True), ] users_page_two = [ SimpleNamespace(id="user-3", display_name="User 3"), @@ -288,9 +289,18 @@ def test_azure_entra__get_users_handles_pagination(): assert len(users["tenant-1"]) == 3 assert users_builder.get.await_count == 1 + request_configuration = users_builder.get.await_args.kwargs["request_configuration"] + assert request_configuration.query_parameters.select == [ + "id", + "displayName", + "accountEnabled", + ] with_url_mock.assert_called_once_with("next-link") registration_details_builder.get.assert_awaited() registration_details_builder.with_url.assert_not_called() assert users["tenant-1"]["user-1"].is_mfa_capable is True + assert users["tenant-1"]["user-1"].account_enabled is False assert users["tenant-1"]["user-2"].is_mfa_capable is True + assert users["tenant-1"]["user-2"].account_enabled is True assert users["tenant-1"]["user-3"].is_mfa_capable is False + assert users["tenant-1"]["user-3"].account_enabled is True