mirror of
https://github.com/prowler-cloud/prowler.git
synced 2026-07-24 21:11:53 +00:00
fix(m365): exclude guest users from entra_users_mfa_capable (#10785)
This commit is contained in:
committed by
GitHub
parent
37e6c9761f
commit
e252058af4
@@ -19,6 +19,7 @@ All notable changes to the **Prowler SDK** are documented in this file.
|
||||
- AWS Organizations metadata retrieval for delegated administrator scans by using the assumed role session instead of the pre-assume credentials [(#10894)](https://github.com/prowler-cloud/prowler/pull/10894)
|
||||
- `admincenter_groups_not_public_visibility` check for M365 provider evaluating Security and Distribution groups, now restricted to Microsoft 365 (Unified) groups per CIS M365 Foundations 1.2.1 [(#10899)](https://github.com/prowler-cloud/prowler/pull/10899)
|
||||
- Google Workspace check reports now store the actual domain or account resource subject instead of `provider.identity` [(#10901)](https://github.com/prowler-cloud/prowler/pull/10901)
|
||||
- `entra_users_mfa_capable` evaluating disabled guest accounts; CIS 5.2.3.4 only targets enabled member users [(#10785)](https://github.com/prowler-cloud/prowler/pull/10785)
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -844,6 +844,7 @@ class Entra(M365Service):
|
||||
authentication_methods=reg_info.get(
|
||||
"authentication_methods", []
|
||||
),
|
||||
user_type=getattr(user, "user_type", None),
|
||||
)
|
||||
|
||||
next_link = getattr(users_response, "odata_next_link", None)
|
||||
@@ -1409,6 +1410,9 @@ class User(BaseModel):
|
||||
account_enabled: Whether the user account is enabled.
|
||||
authentication_methods: List of authentication method types registered by the user
|
||||
(e.g., 'fido2SecurityKey', 'microsoftAuthenticatorPush', 'mobilePhone').
|
||||
user_type: The user account type as reported by Microsoft Graph
|
||||
(typically 'Member' or 'Guest'). ``None`` when Microsoft Graph does not
|
||||
return the property; checks must not assume a default in that case.
|
||||
"""
|
||||
|
||||
id: str
|
||||
@@ -1418,6 +1422,7 @@ class User(BaseModel):
|
||||
is_mfa_capable: bool = False
|
||||
account_enabled: bool = True
|
||||
authentication_methods: List[str] = []
|
||||
user_type: Optional[str] = None
|
||||
|
||||
|
||||
class InvitationsFrom(Enum):
|
||||
|
||||
+28
-20
@@ -6,41 +6,49 @@ from prowler.providers.m365.services.entra.entra_client import entra_client
|
||||
|
||||
class entra_users_mfa_capable(Check):
|
||||
"""
|
||||
Ensure all users are MFA capable.
|
||||
Ensure all member users are MFA capable.
|
||||
|
||||
This check verifies if users are MFA capable.
|
||||
This check verifies if member users are MFA capable, aligning with CIS
|
||||
Microsoft 365 Foundations Benchmark recommendation 5.2.3.4
|
||||
("Ensure all member users are 'MFA capable'").
|
||||
|
||||
The check fails if any user is not MFA capable.
|
||||
Guest users and disabled accounts are excluded from the evaluation.
|
||||
"""
|
||||
|
||||
def execute(self) -> List[CheckReportM365]:
|
||||
"""
|
||||
Execute the admin MFA capable check for all users.
|
||||
Execute the MFA capable check for all enabled member users.
|
||||
|
||||
Iterates over the users retrieved from the Entra client and generates a report
|
||||
indicating if users are MFA capable.
|
||||
indicating if member users are MFA capable. Users explicitly typed as ``Guest``
|
||||
and disabled accounts are skipped, in line with the CIS recommendation that
|
||||
scopes the control to member users only. Users whose ``user_type`` could not
|
||||
be determined are still evaluated to avoid silently dropping accounts when
|
||||
Microsoft Graph does not return the property.
|
||||
|
||||
Returns:
|
||||
List[CheckReportM365]: A list containing a single report with the result of the check.
|
||||
List[CheckReportM365]: A list with one report per evaluated user.
|
||||
"""
|
||||
findings = []
|
||||
|
||||
for user in entra_client.users.values():
|
||||
if user.account_enabled:
|
||||
report = CheckReportM365(
|
||||
metadata=self.metadata(),
|
||||
resource=user,
|
||||
resource_name=user.name,
|
||||
resource_id=user.id,
|
||||
)
|
||||
if user.user_type == "Guest" or not user.account_enabled:
|
||||
continue
|
||||
|
||||
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."
|
||||
report = CheckReportM365(
|
||||
metadata=self.metadata(),
|
||||
resource=user,
|
||||
resource_name=user.name,
|
||||
resource_id=user.id,
|
||||
)
|
||||
|
||||
findings.append(report)
|
||||
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)
|
||||
|
||||
return findings
|
||||
|
||||
+183
@@ -237,3 +237,186 @@ class Test_entra_users_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
|
||||
|
||||
def test_disabled_guest_user_not_checked(self):
|
||||
"""Disabled guest user should not be checked: expected no results.
|
||||
|
||||
Regression test for https://github.com/prowler-cloud/prowler/issues/10637.
|
||||
CIS 5.2.3.4 evaluates only enabled member users; disabled guests must be skipped
|
||||
even when ``account_enabled`` cannot be derived from Exchange Online.
|
||||
"""
|
||||
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 Guest",
|
||||
on_premises_sync_enabled=False,
|
||||
directory_roles_ids=[],
|
||||
is_mfa_capable=False,
|
||||
account_enabled=False,
|
||||
user_type="Guest",
|
||||
)
|
||||
}
|
||||
|
||||
check = entra_users_mfa_capable()
|
||||
result = check.execute()
|
||||
|
||||
assert len(result) == 0
|
||||
|
||||
def test_enabled_guest_user_not_checked(self):
|
||||
"""Enabled guest user is out of scope for CIS 5.2.3.4: 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="Guest User",
|
||||
on_premises_sync_enabled=False,
|
||||
directory_roles_ids=[],
|
||||
is_mfa_capable=False,
|
||||
account_enabled=True,
|
||||
user_type="Guest",
|
||||
)
|
||||
}
|
||||
|
||||
check = entra_users_mfa_capable()
|
||||
result = check.execute()
|
||||
|
||||
assert len(result) == 0
|
||||
|
||||
def test_member_and_guest_users(self):
|
||||
"""Mix of member and guest users: only member 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,
|
||||
)
|
||||
|
||||
member_user_id = str(uuid4())
|
||||
guest_user_id = str(uuid4())
|
||||
entra_client.users = {
|
||||
member_user_id: User(
|
||||
id=member_user_id,
|
||||
name="Member User",
|
||||
on_premises_sync_enabled=False,
|
||||
directory_roles_ids=[],
|
||||
is_mfa_capable=False,
|
||||
account_enabled=True,
|
||||
user_type="Member",
|
||||
),
|
||||
guest_user_id: User(
|
||||
id=guest_user_id,
|
||||
name="Guest User",
|
||||
on_premises_sync_enabled=False,
|
||||
directory_roles_ids=[],
|
||||
is_mfa_capable=False,
|
||||
account_enabled=True,
|
||||
user_type="Guest",
|
||||
),
|
||||
}
|
||||
|
||||
check = entra_users_mfa_capable()
|
||||
result = check.execute()
|
||||
|
||||
assert len(result) == 1
|
||||
assert result[0].status == "FAIL"
|
||||
assert result[0].status_extended == "User Member User is not MFA capable."
|
||||
assert result[0].resource == entra_client.users[member_user_id]
|
||||
assert result[0].resource_name == "Member User"
|
||||
assert result[0].resource_id == member_user_id
|
||||
|
||||
def test_unknown_user_type_is_evaluated(self):
|
||||
"""Users without a ``user_type`` reported by Microsoft Graph must not be
|
||||
silently dropped.
|
||||
|
||||
We only skip users that Graph explicitly reports as ``Guest``; for everyone
|
||||
else (including ``user_type=None``) the check still evaluates MFA capability
|
||||
so that we never mask findings on accounts whose type cannot be determined.
|
||||
"""
|
||||
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="Test User",
|
||||
on_premises_sync_enabled=False,
|
||||
directory_roles_ids=[],
|
||||
is_mfa_capable=False,
|
||||
account_enabled=True,
|
||||
user_type=None,
|
||||
)
|
||||
}
|
||||
|
||||
check = entra_users_mfa_capable()
|
||||
result = check.execute()
|
||||
|
||||
assert len(result) == 1
|
||||
assert result[0].status == "FAIL"
|
||||
assert result[0].status_extended == "User Test User is not MFA capable."
|
||||
assert result[0].resource == entra_client.users[user_id]
|
||||
assert result[0].resource_name == "Test User"
|
||||
assert result[0].resource_id == user_id
|
||||
|
||||
Reference in New Issue
Block a user