-
Notifications
You must be signed in to change notification settings - Fork 117
35001 - Add test for Conditional Access RMS exclusions #719
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a new assessment test (35001) to verify that Microsoft Rights Management Service (RMS) is properly excluded from Conditional Access policies, addressing the issue where RMS authentication blocking can prevent users from accessing encrypted content protected by sensitivity labels.
Key Changes
- Implements test logic to identify CA policies that target RMS without excluding it
- Adds comprehensive unit tests covering multiple scenarios (no policies, properly excluded RMS, blocking policies, error handling, and disabled policies)
- Provides detailed documentation explaining the RMS exclusion requirement and remediation steps
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/powershell/tests/Test-Assessment.35001.ps1 | Main test implementation that queries CA policies and identifies those that block RMS authentication |
| src/powershell/tests/Test-Assessment.35001.md | Documentation explaining why RMS should be excluded from CA policies and remediation steps |
| code-tests/test-assessments/Test-Assessment.35001.Tests.ps1 | Comprehensive unit tests covering various policy scenarios and edge cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #region Data Collection | ||
| Write-PSFMessage '🟦 Start' -Tag Test -Level VeryVerbose | ||
|
|
||
| $activity = 'Checking Conditional Access RMS Exclusions' | ||
| Write-ZtProgress -Activity $activity -Status 'Getting Conditional Access policies' |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test specifies 'MIP_P1' as the MinimumLicense but doesn't include a license check at the beginning of the function. Other tests in the codebase that require specific licenses use Get-ZtLicense to check for the license and skip execution if not present. However, Get-ZtLicense currently only supports 'EntraIDP1', 'EntraIDP2', 'EntraIDGovernance', 'EntraWorkloadID', and 'Intune' - it does not support 'MIP_P1'.
You should either:
- Add support for 'MIP_P1' in the Get-ZtLicense function and add a license check at the beginning of this test
- Use an alternative license (like EntraIDP1 or EntraIDP2) that includes MIP capabilities, or
- If no license check is needed, consider updating the MinimumLicense attribute to reflect this
|
|
||
| $testResultDetail = @{ | ||
| TestId = '35001' | ||
| Title = 'Conditional Access RMS Exclusions' |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'Title' parameter is included in the testResultDetail hashtable, but when looking at other tests in the codebase, the Title is typically derived from the ZtTest attribute and not passed explicitly to Add-ZtTestResultDetail. This duplication may be unnecessary. Consider removing the Title parameter from the hashtable to follow the pattern used by other tests, unless there's a specific reason for this deviation.
| Title = 'Conditional Access RMS Exclusions' |
| $testResultMarkdown += "| :--- | :--- | :--- | :--- | :--- | :--- |`n" | ||
|
|
||
| foreach ($policy in $blockingPolicies) { | ||
| $policyLink = "https://entra.microsoft.com/#view/Microsoft_AAD_ConditionalAccess/ConditionalAccessBlade/~/Policies/$($policy.id)" |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The URL format used for the policy link is inconsistent with the pattern used in other tests in the codebase. Most other tests use the format "https://entra.microsoft.com/#view/Microsoft_AAD_ConditionalAccess/PolicyBlade/policyId/{policy.id}" to link directly to a specific policy. This test uses "https://entra.microsoft.com/#view/Microsoft_AAD_ConditionalAccess/ConditionalAccessBlade/~/Policies/{policy.id}" instead. Consider updating to match the standard pattern for consistency across the codebase.
| $policyLink = "https://entra.microsoft.com/#view/Microsoft_AAD_ConditionalAccess/ConditionalAccessBlade/~/Policies/$($policy.id)" | |
| $policyLink = "https://entra.microsoft.com/#view/Microsoft_AAD_ConditionalAccess/PolicyBlade/policyId/$($policy.id)" |
Fix https://github.com/microsoft/ztspecs/issues/53