mirror of
https://github.com/prowler-cloud/prowler.git
synced 2026-07-23 20:42:02 +00:00
fix(sdk): ignore disabled users in Entra MFA check (#10429)
Co-authored-by: Hugo Pereira Brito <101209179+HugoPBrito@users.noreply.github.com>
This commit is contained in:
@@ -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)
|
||||
|
||||
---
|
||||
|
||||
|
||||
+1
-1
@@ -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)
|
||||
|
||||
@@ -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):
|
||||
|
||||
+80
@@ -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())
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user