From 02a83adfd40e9271d5edd7a83f5143e50a5f9d01 Mon Sep 17 00:00:00 2001 From: Prowler Bot Date: Tue, 12 May 2026 09:50:44 +0200 Subject: [PATCH] fix(m365): exclude disabled guest users from entra_users_mfa_capable (#11119) Co-authored-by: Hugo Pereira Brito <101209179+HugoPBrito@users.noreply.github.com> --- prowler/CHANGELOG.md | 8 ++ .../m365/services/entra/entra_service.py | 43 ++++++++- .../entra/microsoft365_entra_service_test.py | 93 ++++++++++++++++++- 3 files changed, 139 insertions(+), 5 deletions(-) diff --git a/prowler/CHANGELOG.md b/prowler/CHANGELOG.md index c633b2ba26..7df9373965 100644 --- a/prowler/CHANGELOG.md +++ b/prowler/CHANGELOG.md @@ -2,6 +2,14 @@ All notable changes to the **Prowler SDK** are documented in this file. +## [5.26.1] (Prowler UNRELEASED) + +### 🐞 Fixed + +- `entra_users_mfa_capable` no longer flags disabled guest users by requesting `accountEnabled` and `userType` from Microsoft Graph via `$select` and using Graph as the source of truth for `account_enabled` (EXO `Get-User` does not return guest users) [(#11002)](https://github.com/prowler-cloud/prowler/pull/11002) + +--- + ## [5.26.0] (Prowler v5.26.0) ### 🚀 Added diff --git a/prowler/providers/m365/services/entra/entra_service.py b/prowler/providers/m365/services/entra/entra_service.py index a6b8451387..56faef0157 100644 --- a/prowler/providers/m365/services/entra/entra_service.py +++ b/prowler/providers/m365/services/entra/entra_service.py @@ -5,10 +5,12 @@ from enum import Enum from typing import Dict, List, Optional from uuid import UUID +from kiota_abstractions.base_request_configuration import RequestConfiguration from msgraph.generated.models.o_data_errors.o_data_error import ODataError from msgraph.generated.security.microsoft_graph_security_run_hunting_query.run_hunting_query_post_request_body import ( RunHuntingQueryPostRequestBody, ) +from msgraph.generated.users.users_request_builder import UsersRequestBuilder from pydantic.v1 import BaseModel, validator from prowler.lib.logger import logger @@ -806,7 +808,29 @@ class Entra(M365Service): logger.info("Entra - Getting users...") users = {} try: - users_response = await self.client.users.get() + # Microsoft Graph's /users endpoint omits accountEnabled, userType and + # onPremisesSyncEnabled from the default property set, so we must request + # them explicitly via $select. Without this, disabled guest users surface + # as account_enabled=True (Pydantic default) and user_type=None, which + # bypasses the guest/disabled filters in checks like + # entra_users_mfa_capable (CIS 5.2.3.4). See issue #10921. + query_parameters = ( + UsersRequestBuilder.UsersRequestBuilderGetQueryParameters( + select=[ + "id", + "displayName", + "userType", + "accountEnabled", + "onPremisesSyncEnabled", + ], + ) + ) + request_configuration = RequestConfiguration( + query_parameters=query_parameters, + ) + users_response = await self.client.users.get( + request_configuration=request_configuration, + ) directory_roles = await self.client.directory_roles.get() async def fetch_role_members(directory_role): @@ -830,6 +854,19 @@ class Entra(M365Service): while users_response: for user in getattr(users_response, "value", []) or []: reg_info = registration_details.get(user.id, {}) + # Prefer Microsoft Graph as the source of truth for + # accountEnabled: it covers every directory user including + # guests, whereas EXO's Get-User only returns mail-enabled + # accounts and silently drops disabled guests. Fall back to + # the EXO PowerShell value only when Graph does not return a + # value (e.g. older tenants or permission-restricted reads). + graph_account_enabled = getattr(user, "account_enabled", None) + if graph_account_enabled is None: + account_enabled = not self.user_accounts_status.get( + user.id, {} + ).get("AccountDisabled", False) + else: + account_enabled = bool(graph_account_enabled) users[user.id] = User( id=user.id, name=user.display_name, @@ -838,9 +875,7 @@ class Entra(M365Service): ), directory_roles_ids=user_roles_map.get(user.id, []), is_mfa_capable=reg_info.get("is_mfa_capable", False), - account_enabled=not self.user_accounts_status.get( - user.id, {} - ).get("AccountDisabled", False), + account_enabled=account_enabled, authentication_methods=reg_info.get( "authentication_methods", [] ), 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..3ebc4d2ffc 100644 --- a/tests/providers/m365/services/entra/microsoft365_entra_service_test.py +++ b/tests/providers/m365/services/entra/microsoft365_entra_service_test.py @@ -521,10 +521,26 @@ class Test_Entra_Service: assert len(users) == 6 assert users_builder.get.await_count == 1 - assert users_builder.get.await_args.kwargs == {} + # The Graph users.get() call must request accountEnabled, userType and + # onPremisesSyncEnabled via $select. They are not part of the default + # property set, and omitting them causes disabled guest users to leak + # into checks like entra_users_mfa_capable (issue #10921). + request_configuration = users_builder.get.await_args.kwargs[ + "request_configuration" + ] + assert set(request_configuration.query_parameters.select) == { + "id", + "displayName", + "userType", + "accountEnabled", + "onPremisesSyncEnabled", + } with_url_mock.assert_called_once_with("next-link") assert users["user-1"].directory_roles_ids == ["role-template-1"] assert users["user-6"].directory_roles_ids == ["role-template-1"] + # When Graph does not return accountEnabled (legacy SimpleNamespace + # fixtures) we still honour the EXO PowerShell fallback for backwards + # compatibility. assert users["user-6"].account_enabled is False assert users["user-1"].is_mfa_capable is True assert users["user-2"].is_mfa_capable is False @@ -532,6 +548,81 @@ class Test_Entra_Service: assert users["user-6"].authentication_methods == ["mobilePhone"] assert users["user-2"].authentication_methods == [] + def test__get_users_uses_graph_account_enabled_for_disabled_guests(self): + """Regression test for https://github.com/prowler-cloud/prowler/issues/10921. + + Disabled guest users do not appear in EXO's ``Get-User`` output, so the + previous code resolved their ``account_enabled`` from the EXO map, + defaulted it to ``True`` and surfaced them as failing findings in + ``entra_users_mfa_capable``. The Graph ``accountEnabled`` value must be + used as the source of truth so disabled guests are excluded. + """ + entra_service = Entra.__new__(Entra) + # Empty EXO map mirrors the production scenario where the disabled guest + # is absent from Get-User results. + entra_service.user_accounts_status = {} + + graph_users = [ + SimpleNamespace( + id="member-1", + display_name="Member User", + on_premises_sync_enabled=False, + account_enabled=True, + user_type="Member", + ), + SimpleNamespace( + id="guest-1", + display_name="Disabled Guest", + on_premises_sync_enabled=False, + account_enabled=False, + user_type="Guest", + ), + SimpleNamespace( + id="guest-2", + display_name="Enabled Guest", + on_premises_sync_enabled=False, + account_enabled=True, + user_type="Guest", + ), + ] + users_response = SimpleNamespace( + value=graph_users, + odata_next_link=None, + ) + users_builder = SimpleNamespace( + get=AsyncMock(return_value=users_response), + with_url=MagicMock(), + ) + directory_roles_builder = SimpleNamespace( + get=AsyncMock(return_value=SimpleNamespace(value=[])), + by_directory_role_id=MagicMock(), + ) + registration_details_builder = SimpleNamespace( + get=AsyncMock(return_value=SimpleNamespace(value=[], odata_next_link=None)), + with_url=MagicMock(), + ) + reports_builder = SimpleNamespace( + authentication_methods=SimpleNamespace( + user_registration_details=registration_details_builder + ) + ) + + entra_service.client = SimpleNamespace( + users=users_builder, + directory_roles=directory_roles_builder, + reports=reports_builder, + ) + + users = asyncio.run(entra_service._get_users()) + + assert len(users) == 3 + assert users["member-1"].account_enabled is True + assert users["member-1"].user_type == "Member" + assert users["guest-1"].account_enabled is False + assert users["guest-1"].user_type == "Guest" + assert users["guest-2"].account_enabled is True + assert users["guest-2"].user_type == "Guest" + def test__get_user_registration_details_handles_pagination(self): entra_service = Entra.__new__(Entra)