Skip to content

Commit 885ee62

Browse files
fix(m365): admincenter service unnecessary msgraph calls and repeated resource_id (#9019)
Co-authored-by: César Arroba <cesar@prowler.com>
1 parent bbeccaf commit 885ee62

File tree

15 files changed

+99
-81
lines changed

15 files changed

+99
-81
lines changed

prowler/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ All notable changes to the **Prowler SDK** are documented in this file.
2424
- Add `resource_name` for checks under `logging` for the GCP provider [(#9023)](https://github.com/prowler-cloud/prowler/pull/9023)
2525
- Fix `ec2_instance_with_outdated_ami` check to handle None AMIs [(#9046)](https://github.com/prowler-cloud/prowler/pull/9046)
2626
- Handle timestamp when transforming compliance findings in CCC [(#9042)](https://github.com/prowler-cloud/prowler/pull/9042)
27+
- Update `resource_id` for admincenter service and avoid unnecessary msgraph requests [(#9019)](https://github.com/prowler-cloud/prowler/pull/9019)
2728

2829
---
2930

prowler/providers/m365/services/admincenter/admincenter_service.py

Lines changed: 20 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,13 @@ def __init__(self, provider: M365Provider):
4545
asyncio.gather(
4646
self._get_directory_roles(),
4747
self._get_groups(),
48-
self._get_domains(),
48+
self._get_password_policy(),
4949
)
5050
)
5151

5252
self.directory_roles = attributes[0]
5353
self.groups = attributes[1]
54-
self.domains = attributes[2]
54+
self.password_policy = attributes[2]
5555

5656
if created_loop:
5757
asyncio.set_event_loop(None)
@@ -192,34 +192,31 @@ async def _get_groups(self):
192192
)
193193
return groups
194194

195-
async def _get_domains(self):
196-
logger.info("M365 - Getting domains...")
197-
domains = {}
195+
async def _get_password_policy(self):
196+
logger.info("M365 - Getting password policy...")
197+
password_policy = None
198198
try:
199+
logger.info("M365 - Getting domains...")
199200
domains_list = await self.client.domains.get()
200-
domains.update({})
201-
for domain in domains_list.value:
202-
if domain:
203-
password_validity_period = getattr(
204-
domain, "password_validity_period_in_days", None
205-
)
206-
if password_validity_period is None:
207-
password_validity_period = 0
201+
for domain in getattr(domains_list, "value", []) or []:
202+
if not domain:
203+
continue
204+
password_validity_period = getattr(
205+
domain, "password_validity_period_in_days", None
206+
)
207+
if password_validity_period is None:
208+
password_validity_period = 0
208209

209-
domains.update(
210-
{
211-
domain.id: Domain(
212-
id=domain.id,
213-
password_validity_period=password_validity_period,
214-
)
215-
}
216-
)
210+
password_policy = PasswordPolicy(
211+
password_validity_period=password_validity_period,
212+
)
213+
break
217214

218215
except Exception as error:
219216
logger.error(
220217
f"{error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
221218
)
222-
return domains
219+
return password_policy
223220

224221

225222
class User(BaseModel):
@@ -242,8 +239,7 @@ class Group(BaseModel):
242239
visibility: Optional[str]
243240

244241

245-
class Domain(BaseModel):
246-
id: str
242+
class PasswordPolicy(BaseModel):
247243
password_validity_period: int
248244

249245

prowler/providers/m365/services/admincenter/admincenter_settings_password_never_expire/admincenter_settings_password_never_expire.py

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@
77

88

99
class admincenter_settings_password_never_expire(Check):
10-
"""Check if domains have a 'Password never expires' policy.
10+
"""Check if the tenant enforces a 'Password never expires' policy.
1111
12-
This check verifies whether the password policy for each domain is set to never expire.
13-
If the domain password validity period is set to `2147483647`, the policy is considered to
14-
have 'password never expires'.
12+
This check verifies whether the tenant-wide password policy (surfaced through the first
13+
domain returned by Microsoft 365) is set to never expire. If the password validity period
14+
is set to `2147483647`, the policy is considered to have 'password never expires'.
1515
1616
Attributes:
1717
metadata: Metadata associated with the check (inherited from Check).
@@ -20,30 +20,32 @@ class admincenter_settings_password_never_expire(Check):
2020
def execute(self) -> List[CheckReportM365]:
2121
"""Execute the check for password never expires policy.
2222
23-
This method iterates over all domains and checks if the password validity period is set
24-
to `2147483647`, indicating that passwords for users in the domain never expire.
23+
This method inspects the tenant-level password validity configuration (exposed through
24+
the first available domain) and checks if the password validity period is set to
25+
`2147483647`, indicating that passwords for users in the domain never expire.
2526
2627
Returns:
2728
List[CheckReportM365]: A list of reports indicating whether the domain's password
2829
policy is set to never expire.
2930
"""
3031
findings = []
31-
for domain in admincenter_client.domains.values():
32+
password_policy = getattr(admincenter_client, "password_policy", None)
33+
if password_policy:
3234
report = CheckReportM365(
3335
self.metadata(),
34-
resource=domain,
35-
resource_name=domain.id,
36-
resource_id=domain.id,
36+
resource=password_policy,
37+
resource_name="Password Policy",
38+
resource_id="passwordPolicy",
3739
)
3840
report.status = "FAIL"
3941
report.status_extended = (
40-
f"Domain {domain.id} does not have a Password never expires policy."
42+
"Tenant Password policy does not have a Password never expires policy."
4143
)
4244

43-
if domain.password_validity_period == 2147483647:
45+
if password_policy.password_validity_period == 2147483647:
4446
report.status = "PASS"
4547
report.status_extended = (
46-
f"Domain {domain.id} Password policy is set to never expire."
48+
"Tenant Password policy is set to never expire."
4749
)
4850

4951
findings.append(report)

prowler/providers/m365/services/sharepoint/sharepoint_external_sharing_managed/sharepoint_external_sharing_managed.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ def execute(self) -> List[CheckReportM365]:
3636
self.metadata(),
3737
resource=settings if settings else {},
3838
resource_name="SharePoint Settings",
39-
resource_id=sharepoint_client.tenant_domain,
39+
resource_id="sharepointSettings",
4040
)
4141
report.status = "FAIL"
4242
report.status_extended = "SharePoint external sharing is not managed through domain restrictions."

prowler/providers/m365/services/sharepoint/sharepoint_external_sharing_restricted/sharepoint_external_sharing_restricted.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ def execute(self) -> List[CheckReportM365]:
3232
self.metadata(),
3333
resource=settings if settings else {},
3434
resource_name="SharePoint Settings",
35-
resource_id=sharepoint_client.tenant_domain,
35+
resource_id="sharepointSettings",
3636
)
3737
report.status = "FAIL"
3838
report.status_extended = (

prowler/providers/m365/services/sharepoint/sharepoint_guest_sharing_restricted/sharepoint_guest_sharing_restricted.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ def execute(self) -> List[CheckReportM365]:
3333
self.metadata(),
3434
resource=settings if settings else {},
3535
resource_name="SharePoint Settings",
36-
resource_id=sharepoint_client.tenant_domain,
36+
resource_id="sharepointSettings",
3737
)
3838
report.status = "FAIL"
3939
report.status_extended = "Guest sharing is not restricted; guest users can share items they do not own."

prowler/providers/m365/services/sharepoint/sharepoint_modern_authentication_required/sharepoint_modern_authentication_required.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ def execute(self) -> List[CheckReportM365]:
3535
self.metadata(),
3636
resource=settings if settings else {},
3737
resource_name="SharePoint Settings",
38-
resource_id=sharepoint_client.tenant_domain,
38+
resource_id="sharepointSettings",
3939
)
4040
report.status = "PASS"
4141
report.status_extended = "Microsoft 365 SharePoint does not allow access to apps that don't use modern authentication."

prowler/providers/m365/services/sharepoint/sharepoint_onedrive_sync_restricted_unmanaged_devices/sharepoint_onedrive_sync_restricted_unmanaged_devices.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ def execute(self) -> List[CheckReportM365]:
3434
self.metadata(),
3535
resource=settings if settings else {},
3636
resource_name="SharePoint Settings",
37-
resource_id=sharepoint_client.tenant_domain,
37+
resource_id="sharepointSettings",
3838
)
3939
report.status = "PASS"
4040
report.status_extended = "Microsoft 365 SharePoint does not allow OneDrive sync to unmanaged devices."

tests/providers/m365/services/admincenter/admincenter_settings_password_never_expire/admincenter_settings_password_never_expire_test.py

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
from unittest import mock
2-
from uuid import uuid4
32

43
from tests.providers.m365.m365_fixtures import DOMAIN, set_mocked_m365_provider
54

@@ -15,6 +14,7 @@ def test_admincenter_no_domains(self):
1514
"prowler.providers.common.provider.Provider.get_global_provider",
1615
return_value=set_mocked_m365_provider(),
1716
),
17+
mock.patch("prowler.providers.m365.lib.service.service.M365PowerShell"),
1818
mock.patch(
1919
"prowler.providers.m365.lib.powershell.m365_powershell.M365PowerShell.connect_exchange_online"
2020
),
@@ -27,7 +27,7 @@ def test_admincenter_no_domains(self):
2727
admincenter_settings_password_never_expire,
2828
)
2929

30-
admincenter_client.domains = {}
30+
admincenter_client.password_policy = None
3131

3232
check = admincenter_settings_password_never_expire()
3333
result = check.execute()
@@ -43,6 +43,7 @@ def test_admincenter_domain_password_expire(self):
4343
"prowler.providers.common.provider.Provider.get_global_provider",
4444
return_value=set_mocked_m365_provider(),
4545
),
46+
mock.patch("prowler.providers.m365.lib.service.service.M365PowerShell"),
4647
mock.patch(
4748
"prowler.providers.m365.lib.powershell.m365_powershell.M365PowerShell.connect_exchange_online"
4849
),
@@ -52,29 +53,27 @@ def test_admincenter_domain_password_expire(self):
5253
),
5354
):
5455
from prowler.providers.m365.services.admincenter.admincenter_service import (
55-
Domain,
56+
PasswordPolicy,
5657
)
5758
from prowler.providers.m365.services.admincenter.admincenter_settings_password_never_expire.admincenter_settings_password_never_expire import (
5859
admincenter_settings_password_never_expire,
5960
)
6061

61-
id_domain = str(uuid4())
62-
63-
admincenter_client.domains = {
64-
id_domain: Domain(id=id_domain, password_validity_period=5),
65-
}
62+
admincenter_client.password_policy = PasswordPolicy(
63+
password_validity_period=5
64+
)
6665

6766
check = admincenter_settings_password_never_expire()
6867
result = check.execute()
6968
assert len(result) == 1
7069
assert result[0].status == "FAIL"
7170
assert (
7271
result[0].status_extended
73-
== f"Domain {id_domain} does not have a Password never expires policy."
72+
== "Tenant Password policy does not have a Password never expires policy."
7473
)
75-
assert result[0].resource == admincenter_client.domains[id_domain].dict()
76-
assert result[0].resource_name == id_domain
77-
assert result[0].resource_id == id_domain
74+
assert result[0].resource == admincenter_client.password_policy.dict()
75+
assert result[0].resource_name == "Password Policy"
76+
assert result[0].resource_id == "passwordPolicy"
7877
assert result[0].location == "global"
7978

8079
def test_admincenter_password_not_expire(self):
@@ -87,6 +86,7 @@ def test_admincenter_password_not_expire(self):
8786
"prowler.providers.common.provider.Provider.get_global_provider",
8887
return_value=set_mocked_m365_provider(),
8988
),
89+
mock.patch("prowler.providers.m365.lib.service.service.M365PowerShell"),
9090
mock.patch(
9191
"prowler.providers.m365.lib.powershell.m365_powershell.M365PowerShell.connect_exchange_online"
9292
),
@@ -96,27 +96,25 @@ def test_admincenter_password_not_expire(self):
9696
),
9797
):
9898
from prowler.providers.m365.services.admincenter.admincenter_service import (
99-
Domain,
99+
PasswordPolicy,
100100
)
101101
from prowler.providers.m365.services.admincenter.admincenter_settings_password_never_expire.admincenter_settings_password_never_expire import (
102102
admincenter_settings_password_never_expire,
103103
)
104104

105-
id_domain = str(uuid4())
106-
107-
admincenter_client.domains = {
108-
id_domain: Domain(id=id_domain, password_validity_period=2147483647),
109-
}
105+
admincenter_client.password_policy = PasswordPolicy(
106+
password_validity_period=2147483647
107+
)
110108

111109
check = admincenter_settings_password_never_expire()
112110
result = check.execute()
113111
assert len(result) == 1
114112
assert result[0].status == "PASS"
115113
assert (
116114
result[0].status_extended
117-
== f"Domain {id_domain} Password policy is set to never expire."
115+
== "Tenant Password policy is set to never expire."
118116
)
119-
assert result[0].resource == admincenter_client.domains[id_domain].dict()
120-
assert result[0].resource_name == id_domain
121-
assert result[0].resource_id == id_domain
117+
assert result[0].resource == admincenter_client.password_policy.dict()
118+
assert result[0].resource_name == "Password Policy"
119+
assert result[0].resource_id == "passwordPolicy"
122120
assert result[0].location == "global"

tests/providers/m365/services/sharepoint/sharepoint_external_sharing_managed/sharepoint_external_sharing_managed_test.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ def test_external_sharing_invalid_mode(self):
2020
"prowler.providers.common.provider.Provider.get_global_provider",
2121
return_value=set_mocked_m365_provider(),
2222
),
23+
mock.patch("prowler.providers.m365.lib.service.service.M365PowerShell"),
2324
mock.patch(
2425
"prowler.providers.m365.services.sharepoint.sharepoint_external_sharing_managed.sharepoint_external_sharing_managed.sharepoint_client",
2526
new=sharepoint_client,
@@ -49,7 +50,7 @@ def test_external_sharing_invalid_mode(self):
4950
result[0].status_extended
5051
== "SharePoint external sharing is not managed through domain restrictions."
5152
)
52-
assert result[0].resource_id == DOMAIN
53+
assert result[0].resource_id == "sharepointSettings"
5354
assert result[0].location == "global"
5455
assert result[0].resource_name == "SharePoint Settings"
5556
assert result[0].resource == sharepoint_client.settings.dict()
@@ -66,6 +67,7 @@ def test_allow_list_empty(self):
6667
"prowler.providers.common.provider.Provider.get_global_provider",
6768
return_value=set_mocked_m365_provider(),
6869
),
70+
mock.patch("prowler.providers.m365.lib.service.service.M365PowerShell"),
6971
mock.patch(
7072
"prowler.providers.m365.services.sharepoint.sharepoint_external_sharing_managed.sharepoint_external_sharing_managed.sharepoint_client",
7173
new=sharepoint_client,
@@ -95,7 +97,7 @@ def test_allow_list_empty(self):
9597
result[0].status_extended
9698
== "SharePoint external sharing is managed through domain restrictions with mode 'allowList' but the list is empty."
9799
)
98-
assert result[0].resource_id == DOMAIN
100+
assert result[0].resource_id == "sharepointSettings"
99101
assert result[0].location == "global"
100102
assert result[0].resource_name == "SharePoint Settings"
101103
assert result[0].resource == sharepoint_client.settings.dict()
@@ -112,6 +114,7 @@ def test_block_list_empty(self):
112114
"prowler.providers.common.provider.Provider.get_global_provider",
113115
return_value=set_mocked_m365_provider(),
114116
),
117+
mock.patch("prowler.providers.m365.lib.service.service.M365PowerShell"),
115118
mock.patch(
116119
"prowler.providers.m365.services.sharepoint.sharepoint_external_sharing_managed.sharepoint_external_sharing_managed.sharepoint_client",
117120
new=sharepoint_client,
@@ -141,7 +144,7 @@ def test_block_list_empty(self):
141144
result[0].status_extended
142145
== "SharePoint external sharing is managed through domain restrictions with mode 'blockList' but the list is empty."
143146
)
144-
assert result[0].resource_id == DOMAIN
147+
assert result[0].resource_id == "sharepointSettings"
145148
assert result[0].location == "global"
146149
assert result[0].resource_name == "SharePoint Settings"
147150
assert result[0].resource == sharepoint_client.settings.dict()
@@ -158,6 +161,7 @@ def test_allow_list_non_empty(self):
158161
"prowler.providers.common.provider.Provider.get_global_provider",
159162
return_value=set_mocked_m365_provider(),
160163
),
164+
mock.patch("prowler.providers.m365.lib.service.service.M365PowerShell"),
161165
mock.patch(
162166
"prowler.providers.m365.services.sharepoint.sharepoint_external_sharing_managed.sharepoint_external_sharing_managed.sharepoint_client",
163167
new=sharepoint_client,
@@ -187,7 +191,7 @@ def test_allow_list_non_empty(self):
187191
result[0].status_extended
188192
== "SharePoint external sharing is managed through domain restrictions with mode 'allowList'."
189193
)
190-
assert result[0].resource_id == DOMAIN
194+
assert result[0].resource_id == "sharepointSettings"
191195
assert result[0].location == "global"
192196
assert result[0].resource_name == "SharePoint Settings"
193197
assert result[0].resource == sharepoint_client.settings.dict()
@@ -233,7 +237,7 @@ def test_block_list_non_empty(self):
233237
result[0].status_extended
234238
== "SharePoint external sharing is managed through domain restrictions with mode 'blockList'."
235239
)
236-
assert result[0].resource_id == DOMAIN
240+
assert result[0].resource_id == "sharepointSettings"
237241
assert result[0].location == "global"
238242
assert result[0].resource_name == "SharePoint Settings"
239243
assert result[0].resource == sharepoint_client.settings.dict()
@@ -252,6 +256,7 @@ def test_empty_settings(self):
252256
"prowler.providers.common.provider.Provider.get_global_provider",
253257
return_value=set_mocked_m365_provider(),
254258
),
259+
mock.patch("prowler.providers.m365.lib.service.service.M365PowerShell"),
255260
mock.patch(
256261
"prowler.providers.m365.services.sharepoint.sharepoint_external_sharing_managed.sharepoint_external_sharing_managed.sharepoint_client",
257262
new=sharepoint_client,

0 commit comments

Comments
 (0)