Skip to content

Test-MtCisPasswordExpiry: Only Check domains with isVerified: true to avoid false positives#1757

Open
blindzero wants to merge 11 commits into
maester365:mainfrom
blindzero:main
Open

Test-MtCisPasswordExpiry: Only Check domains with isVerified: true to avoid false positives#1757
blindzero wants to merge 11 commits into
maester365:mainfrom
blindzero:main

Conversation

@blindzero
Copy link
Copy Markdown
Contributor

📑 Description

Test-MtCisPasswordExpiry was checking on PasswordValidityPeriodInDays and isManaged: true for domains.
With unverified domains this led to false positive errors, as unverified domains have PasswordValidityPeriodInDays: false, although the setting is globally applied in Microsoft 365 Admin Portal UI.

My assumption is that unverified domains can't be properly used in Microsoft 365 anyways and therefor should be excluded from this test. As soon as a domain is verified, the proper PasswordValidityPeriodInDays value from tenant configuration is applied (checked this with a new domain on our end).

Changes

$result filter was enriched with -and ($isVerified -eq $true)

Closes #1714

✅ Checks

  • My pull request adheres to the code style of this project.
  • My code requires changes to the documentation.
  • I have updated the documentation as required.
  • The build and unit tests pass after running /powershell/tests/pester.ps1 locally.

… avoid false positives with MD double newline fix
@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented May 9, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@blindzero blindzero marked this pull request as ready for review May 10, 2026 13:16
@blindzero blindzero requested a review from a team as a code owner May 10, 2026 13:16
Copy link
Copy Markdown
Contributor

@Mynster9361 Mynster9361 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although this is not listed in the CIS documentation for this test i believe this addition is needed to avoid false positives.

LGTM

@SamErde SamErde requested a review from Copilot May 11, 2026 09:17
@SamErde SamErde added bug Something isn't working maester-test Related to a Maester test labels May 11, 2026
@SamErde
Copy link
Copy Markdown
Contributor

SamErde commented May 11, 2026

Thanks, @blindzero! Great catch!
Thanks for adding a review, @Mynster9361!

SamErde
SamErde previously approved these changes May 11, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a 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 updates the CIS control implementation for password expiry to avoid false positives by excluding unverified domains from evaluation when querying Microsoft Graph domain settings.

Changes:

  • Filter Test-MtCisPasswordExpiry results to only include domains where isVerified is true.
  • Minor Markdown formatting cleanup in the CIS guidance page.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
powershell/public/cis/Test-MtCisPasswordExpiry.ps1 Adds isVerified filtering to avoid flagging unverified domains as failing.
powershell/public/cis/Test-MtCisPasswordExpiry.md Removes an extra blank/whitespace line in the guidance content.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread powershell/public/cis/Test-MtCisPasswordExpiry.ps1 Outdated
Comment thread powershell/public/cis/Test-MtCisPasswordExpiry.ps1 Outdated
@SamErde
Copy link
Copy Markdown
Contributor

SamErde commented May 11, 2026

@copilot apply changes based on the comments in this thread

@blindzero
Copy link
Copy Markdown
Contributor Author

@SamErde added safety net for string value of passwordExpiry value. Ready for review. Thx.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment on lines +37 to +38
if (($_.authenticationType -ne "Managed") -or ($_.isVerified -ne $true)) {
return $false
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, @blindzero.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SamErde good one...thanks. implemented skip result logic. All tests ✅

Comment thread powershell/public/cis/Test-MtCisPasswordExpiry.ps1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working maester-test Related to a Maester test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🪲 CIS.M365.1.3.1 Test-MtCisPasswordExpiry failing though setting activated

4 participants