mirror of
https://github.com/prowler-cloud/prowler.git
synced 2026-05-15 00:33:27 +00:00
fix(m365): exclude disabled guest users from entra_users_mfa_capable (#11002)
This commit is contained in:
committed by
GitHub
parent
759f7b84d6
commit
1b0e12ec51
@@ -11,6 +11,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) [(#10921)](https://github.com/prowler-cloud/prowler/issues/10921)
|
||||
|
||||
---
|
||||
|
||||
## [5.26.0] (Prowler v5.26.0)
|
||||
|
||||
### 🚀 Added
|
||||
|
||||
@@ -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", []
|
||||
),
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user