Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions prowler/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment thread
HugoPBrito marked this conversation as resolved.

---

## [5.26.0] (Prowler v5.26.0)

### 🚀 Added
Expand Down
43 changes: 39 additions & 4 deletions prowler/providers/m365/services/entra/entra_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
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
Expand Down Expand Up @@ -806,7 +808,29 @@ async def _get_users(self):
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):
Expand All @@ -830,6 +854,19 @@ async def fetch_role_members(directory_role):
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,
Expand All @@ -838,9 +875,7 @@ async def fetch_role_members(directory_role):
),
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", []
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -521,17 +521,108 @@ def test__get_users_paginates_through_next_links(self):

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
assert users["user-1"].authentication_methods == ["fido2SecurityKey"]
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)

Expand Down
Loading