Skip to content

Commit a002ec8

Browse files
fix(sdk): ignore disabled users in Entra MFA check (#10429)
Co-authored-by: Hugo Pereira Brito <101209179+HugoPBrito@users.noreply.github.com>
1 parent d7d0f19 commit a002ec8

File tree

5 files changed

+108
-4
lines changed

5 files changed

+108
-4
lines changed

prowler/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ All notable changes to the **Prowler SDK** are documented in this file.
88

99
- 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)
1010
- 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)
11+
- `entra_non_privileged_user_has_mfa` skips disabled users to avoid false positives [(#10426)](https://github.com/prowler-cloud/prowler/pull/10426)
1112

1213
---
1314

prowler/providers/azure/services/entra/entra_non_privileged_user_has_mfa/entra_non_privileged_user_has_mfa.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ def execute(self) -> Check_Report_Azure:
1111

1212
for tenant_domain, users in entra_client.users.items():
1313
for user in users.values():
14-
if not is_privileged_user(
14+
if user.account_enabled and not is_privileged_user(
1515
user, entra_client.directory_roles[tenant_domain]
1616
):
1717
report = Check_Report_Azure(metadata=self.metadata(), resource=user)

prowler/providers/azure/services/entra/entra_service.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
from typing import List, Optional
44
from uuid import UUID
55

6+
from kiota_abstractions.base_request_configuration import RequestConfiguration
67
from msgraph import GraphServiceClient
8+
from msgraph.generated.users.users_request_builder import UsersRequestBuilder
79
from pydantic.v1 import BaseModel
810

911
from prowler.lib.logger import logger
@@ -65,9 +67,16 @@ async def _get_users(self):
6567
logger.info("Entra - Getting users...")
6668
users = {}
6769
try:
70+
request_configuration = RequestConfiguration(
71+
query_parameters=UsersRequestBuilder.UsersRequestBuilderGetQueryParameters(
72+
select=["id", "displayName", "accountEnabled"]
73+
)
74+
)
6875
for tenant, client in self.clients.items():
6976
users.update({tenant: {}})
70-
users_response = await client.users.get()
77+
users_response = await client.users.get(
78+
request_configuration=request_configuration
79+
)
7180
registration_details = await self._get_user_registration_details(client)
7281

7382
try:
@@ -81,6 +90,9 @@ async def _get_users(self):
8190
is_mfa_capable=registration_details.get(
8291
user.id, False
8392
),
93+
account_enabled=getattr(
94+
user, "account_enabled", True
95+
),
8496
)
8597
}
8698
)
@@ -409,6 +421,7 @@ class User(BaseModel):
409421
id: str
410422
name: str
411423
is_mfa_capable: bool = False
424+
account_enabled: bool = True
412425

413426

414427
class DefaultUserRolePermissions(BaseModel):

tests/providers/azure/services/entra/entra_non_privileged_user_has_mfa/entra_non_privileged_user_has_mfa_test.py

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,86 @@ def test_entra_user_no_privileged_mfa(self):
142142
assert result[0].resource_id == user_id
143143
assert result[0].subscription == f"Tenant: {DOMAIN}"
144144

145+
def test_entra_disabled_user_no_privileged_no_mfa(self):
146+
entra_client = mock.MagicMock
147+
user_id = str(uuid4())
148+
149+
with (
150+
mock.patch(
151+
"prowler.providers.common.provider.Provider.get_global_provider",
152+
return_value=set_mocked_azure_provider(),
153+
),
154+
mock.patch(
155+
"prowler.providers.azure.services.entra.entra_non_privileged_user_has_mfa.entra_non_privileged_user_has_mfa.entra_client",
156+
new=entra_client,
157+
),
158+
):
159+
from prowler.providers.azure.services.entra.entra_non_privileged_user_has_mfa.entra_non_privileged_user_has_mfa import (
160+
entra_non_privileged_user_has_mfa,
161+
)
162+
from prowler.providers.azure.services.entra.entra_service import (
163+
DirectoryRole,
164+
User,
165+
)
166+
167+
user = User(
168+
id=user_id,
169+
name="foo",
170+
is_mfa_capable=False,
171+
account_enabled=False,
172+
)
173+
174+
entra_client.users = {DOMAIN: {f"foo@{DOMAIN}": user}}
175+
entra_client.directory_roles = {
176+
DOMAIN: {
177+
"Global Administrator": DirectoryRole(id=str(uuid4()), members=[])
178+
}
179+
}
180+
181+
check = entra_non_privileged_user_has_mfa()
182+
result = check.execute()
183+
assert len(result) == 0
184+
185+
def test_entra_disabled_user_no_privileged_mfa(self):
186+
entra_client = mock.MagicMock
187+
user_id = str(uuid4())
188+
189+
with (
190+
mock.patch(
191+
"prowler.providers.common.provider.Provider.get_global_provider",
192+
return_value=set_mocked_azure_provider(),
193+
),
194+
mock.patch(
195+
"prowler.providers.azure.services.entra.entra_non_privileged_user_has_mfa.entra_non_privileged_user_has_mfa.entra_client",
196+
new=entra_client,
197+
),
198+
):
199+
from prowler.providers.azure.services.entra.entra_non_privileged_user_has_mfa.entra_non_privileged_user_has_mfa import (
200+
entra_non_privileged_user_has_mfa,
201+
)
202+
from prowler.providers.azure.services.entra.entra_service import (
203+
DirectoryRole,
204+
User,
205+
)
206+
207+
user = User(
208+
id=user_id,
209+
name="foo",
210+
is_mfa_capable=True,
211+
account_enabled=False,
212+
)
213+
214+
entra_client.users = {DOMAIN: {f"foo@{DOMAIN}": user}}
215+
entra_client.directory_roles = {
216+
DOMAIN: {
217+
"Global Administrator": DirectoryRole(id=str(uuid4()), members=[])
218+
}
219+
}
220+
221+
check = entra_non_privileged_user_has_mfa()
222+
result = check.execute()
223+
assert len(result) == 0
224+
145225
def test_entra_user_privileged_no_mfa(self):
146226
entra_client = mock.MagicMock
147227
user_id = str(uuid4())

tests/providers/azure/services/entra/entra_service_test.py

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ def test_get_users(self):
147147
assert entra_client.users[DOMAIN]["user-1@tenant1.es"].id == "id-1"
148148
assert entra_client.users[DOMAIN]["user-1@tenant1.es"].name == "User 1"
149149
assert entra_client.users[DOMAIN]["user-1@tenant1.es"].is_mfa_capable is False
150+
assert entra_client.users[DOMAIN]["user-1@tenant1.es"].account_enabled is True
150151

151152
def test_get_authorization_policy(self):
152153
entra_client = Entra(set_mocked_azure_provider())
@@ -229,8 +230,8 @@ def test_azure_entra__get_users_handles_pagination():
229230
entra_service = Entra.__new__(Entra)
230231

231232
users_page_one = [
232-
SimpleNamespace(id="user-1", display_name="User 1"),
233-
SimpleNamespace(id="user-2", display_name="User 2"),
233+
SimpleNamespace(id="user-1", display_name="User 1", account_enabled=False),
234+
SimpleNamespace(id="user-2", display_name="User 2", account_enabled=True),
234235
]
235236
users_page_two = [
236237
SimpleNamespace(id="user-3", display_name="User 3"),
@@ -288,9 +289,18 @@ def test_azure_entra__get_users_handles_pagination():
288289

289290
assert len(users["tenant-1"]) == 3
290291
assert users_builder.get.await_count == 1
292+
request_configuration = users_builder.get.await_args.kwargs["request_configuration"]
293+
assert request_configuration.query_parameters.select == [
294+
"id",
295+
"displayName",
296+
"accountEnabled",
297+
]
291298
with_url_mock.assert_called_once_with("next-link")
292299
registration_details_builder.get.assert_awaited()
293300
registration_details_builder.with_url.assert_not_called()
294301
assert users["tenant-1"]["user-1"].is_mfa_capable is True
302+
assert users["tenant-1"]["user-1"].account_enabled is False
295303
assert users["tenant-1"]["user-2"].is_mfa_capable is True
304+
assert users["tenant-1"]["user-2"].account_enabled is True
296305
assert users["tenant-1"]["user-3"].is_mfa_capable is False
306+
assert users["tenant-1"]["user-3"].account_enabled is True

0 commit comments

Comments
 (0)