fix(m365): exclude disabled guest users from entra_users_mfa_capable#11002
Open
HugoPBrito wants to merge 1 commit intomasterfrom
Open
fix(m365): exclude disabled guest users from entra_users_mfa_capable#11002HugoPBrito wants to merge 1 commit intomasterfrom
HugoPBrito wants to merge 1 commit intomasterfrom
Conversation
Microsoft Graph's /users endpoint omits accountEnabled, userType and onPremisesSyncEnabled from the default property set. Without an explicit $select these fields were never returned, so the User model fell back to its Pydantic defaults (account_enabled=True, user_type=None) and the EXO PowerShell Get-User map was used to derive account_enabled. EXO does not return guest users at all, which meant disabled guest accounts surfaced as enabled members and failed the entra_users_mfa_capable check (CIS 5.2.3.4). Request the relevant fields via $select and use Graph as the source of truth for account_enabled and user_type, falling back to the EXO PowerShell value only when Graph does not return one. Fixes #10921.
Contributor
|
✅ Conflict Markers Resolved All conflict markers have been successfully resolved in this pull request. |
Contributor
|
✅ All necessary |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #11002 +/- ##
===========================================
+ Coverage 59.14% 88.16% +29.01%
===========================================
Files 8 131 +123
Lines 399 5550 +5151
===========================================
+ Hits 236 4893 +4657
- Misses 163 657 +494
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
🔒 Container Security ScanImage: 📊 Vulnerability Summary
5 package(s) affected
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Fix #10921
CIS Microsoft 365 Foundations Benchmark recommendation 5.2.3.4 (
entra_users_mfa_capable) scopes the control to enabled member users only, but the check was still raising findings for disabled guest accounts. This is a regression of the original fix delivered in #10785, which was validated against synthetic data but did not exercise the EXOGet-Usersource path used in production tenants.Description
Microsoft Graph's
/usersendpoint omitsaccountEnabled,userTypeandonPremisesSyncEnabledfrom the default property set, so they are never populated unless requested explicitly via$select. The previous implementation:await self.client.users.get()without$select, souser.account_enabledanduser.user_typearrived asNone/missing.account_enabledsolely from the EXO PowerShellGet-Usermap, which does not return guest accounts at all.For a disabled guest user this produced
account_enabled=True(Pydantic default) anduser_type=None, slipping past theif user.user_type == "Guest" or not user.account_enabledfilter inentra_users_mfa_capable.py.This PR:
$select(id,displayName,userType,accountEnabled,onPremisesSyncEnabled) to the Graphusers.get()call inEntra._get_users().account_enabled(and continues to use it foruser_type).Get-Usermap only when Graph does not return a value, preserving the legacy behaviour for any tenant where the Graph response is missing the property.Steps to review
prowler/providers/m365/services/entra/entra_service.py:_get_usersnow passes aRequestConfigurationwithUsersRequestBuilderGetQueryParameters(select=[...]).account_enabledis derived fromgetattr(user, "account_enabled", None)first, falling back to the EXOuser_accounts_statusmap only when Graph returnsNone.test__get_users_uses_graph_account_enabled_for_disabled_guestsproves a disabled guest from Graph (with empty EXO map) is now modelled asaccount_enabled=False. The pagination test additionally verifies the new$selectis sent on every Graph users.get() call.prowler/CHANGELOG.mdlists the fix under[5.26.0]->🐞 Fixedlinking issue (Regression) Disabled Guest users should not be included in "5.2.3.4 (L1) Ensure all member users are 'MFA capable'" check #10921.Checklist
Community Checklist
SDK/CLI
entra_users_mfa_capable(no permission changes required).License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.