Add Rego policies for CIS 5.1.4.2, 5.1.4.3, and 5.1.4.4 (Entra device join controls)#195
Add Rego policies for CIS 5.1.4.2, 5.1.4.3, and 5.1.4.4 (Entra device join controls)#195williamywccc wants to merge 4 commits into
Conversation
All three controls use the existing device_registration_policy collector (/policies/deviceRegistrationPolicy beta endpoint). The metadata for 5.1.4.2-5.1.4.4 previously referenced entra.devices.device_management_settings which does not expose the relevant fields; corrected to entra.devices.device_registration_policy. 5.1.4.2 - checks userDeviceQuota > 0 (0 treated as unlimited) 5.1.4.3 - checks azureADJoin.localAdministratorsConfiguration.enableGlobalAdmins == false 5.1.4.4 - checks azureADJoin.localAdministratorsConfiguration.registeringUsers == notAllowed The localAdministratorsConfiguration fields are beta-only and may be absent on older tenants; the policy returns a clear non-compliant message in that case. Made-with: Cursor
williamywccc
requested review from
6igby,
du-dhartley and
liyunze-coding
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1fb337a408
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@williamywccc Good catch on the collector mapping correction, thanks for fixing that. |
|
For 5.1.4.2: |
The API returns azureADJoin.localAdmins (not localAdministratorsConfiguration). Also maps registeringUsers @odata.type to a readable string so the 5.1.4.4 Rego policy can evaluate it correctly. Co-authored-by: Cursor <cursoragent@cursor.com>
Update 5.1.4.3 null message to reference correct localAdmins API key. Fix 5.1.4.4 build_message conflict by adding mutual exclusion guards to prevent both rules firing when val equals notallowed. Co-authored-by: Cursor <cursoragent@cursor.com>
5.1.4.2: tighten quota check to require userDeviceQuota <= 20 per CIS v6.0.0; update build_message to use else-chain and distinguish compliant vs over-limit values; update metadata notes to reflect 20-or-less rule. 5.1.4.4: refactor build_message to a single else-chain rule to guarantee mutual exclusivity and eliminate any eval_conflict_error. Co-authored-by: Cursor <cursoragent@cursor.com>
Hi David I have updated the Rego logic and the metadata file as we discussed. I refactored the build message rules into single else chains to ensure they are mutually exclusive and to avoid any evaluation conflicts. I have also completed the three manual scans against the live test tenant to verify the quota logic for 5.1.4.2. Please see the attached screenshots for the results: Scenario 1 (Fail): Quota set to 25 results in non-compliant status with the correct error message Scenario 2 (Pass): Quota set to 10 results in compliant status Scenario 3 (Edge Case): Quota set to exactly 20 also results in compliant status as required Everything should be ready for your final review now. Thanks! |
du-dhartley
left a comment
du-dhartley
left a comment
There was a problem hiding this comment.
The three screenshots don't show a manual scan, they show a manual policy execution against JSON that we can't verify. If you've used real tenant data then it's worth showing that here.
With that said, I tested this against a live tenant. Couple of things to note:
- Field name is localAdmins, not localAdministratorsConfiguration. Confirmed from the raw Graph beta response:
azureADJoin keys: ['isAdminConfigurable', 'allowedToJoin', 'localAdmins']
The collector code is correct. Please update to match in three places:
- PR description (the Controls table)
- docs/engine/policies/cis/microsoft-365-foundations/v6.0.0/controls.md (5.1.4.3 and 5.1.4.4 rows)
- engine/policies/cis/microsoft-365-foundations/v6.0.0/metadata.json (5.1.4.3 and 5.1.4.4 notes)
The 5.1.4.3 fallback message ("… azureADJoin.localAdmins not returned by API") already uses the right name, so the user-facing string is fine.
- 5.1.4.2 threshold is <= 20, not just > 0. The Rego enforces quota > 0 AND quota <= 20, but the PR body table and controls.md only mention > 0. Please update those to match what the policy actually enforces.
When the doc/metadata fix is complete, this will be good to merge.
2 participants
Summary
entra.devices.device_management_settings; the actual data lives inentra.devices.device_registration_policy(/policies/deviceRegistrationPolicybeta endpoint)localAdministratorsConfigurationfields needed for 5.1.4.3 and 5.1.4.4Controls
userDeviceQuota > 0— a value of 0 is treated as unlimitedazureADJoin.localAdministratorsConfiguration.enableGlobalAdmins == falseazureADJoin.localAdministratorsConfiguration.registeringUsers == notAllowedlocalAdministratorsConfigurationis a beta-only field and may be absent on older tenants. Both policies return a descriptive non-compliant message rather than a silent failure in that case.Test plan
Policy.Read.DeviceConfigurationpermissionlocalAdministratorsConfigurationfield presence on the target tenant's beta endpointMade with Cursor