Add support for Microsoft Exo Hybrid domain in DKIM#1464
Add support for Microsoft Exo Hybrid domain in DKIM#1464brianreidc7 wants to merge 1 commit intomaester365:mainfrom
Conversation
This test errors when it encounters the tenantname.mail.onmicrosoft.com domain. Added a elseif to deal with this case.
There was a problem hiding this comment.
Pull request overview
This PR updates the CIS DKIM check to handle Exchange Online hybrid coexistence domains (*.mail.onmicrosoft.com) without erroring, aligning the DKIM DNS lookup logic with how these domains behave in hybrid deployments.
Changes:
- Add detection for
*.mail.onmicrosoft.comaccepted domains. - Adjust DKIM DNS name selection so hybrid domains don’t use the
.onmicrosoft.comCNAME-based path.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $isMicrosoftExoHybridDomain = $domain.DomainName.EndsWith(".mail.onmicrosoft.com") | ||
| $dkimDnsName = if ($isMicrosoftExoHybridDomain) { | ||
| "$($Selector)._domainkey.$($domain.DomainName)" | ||
| } elseif ($isMicrosoftDomain) { | ||
| $dkimSigningConfig."$($selector)CNAME" |
There was a problem hiding this comment.
The if ($isMicrosoftExoHybridDomain) branch produces the same DKIM DNS name as the final else branch, so this conditional can be simplified (e.g., treat only “.onmicrosoft.com” excluding “.mail.onmicrosoft.com” as the special CNAME case). This will make the intent clearer and reduce duplicated logic.
There was a problem hiding this comment.
The order is intentional. If the other way around then onmicrosoft.com is checked first and that matches mail onmicrosoft.com as well.
| $isMicrosoftExoHybridDomain = $domain.DomainName.EndsWith(".mail.onmicrosoft.com") | ||
| $dkimDnsName = if ($isMicrosoftExoHybridDomain) { | ||
| "$($Selector)._domainkey.$($domain.DomainName)" |
There was a problem hiding this comment.
PR title/description suggests adding support for ExO hybrid “*.mail.onmicrosoft.com” handling in DKIM checks; note that Test-MtCisaDkim has the same onmicrosoft CNAME logic but was not updated, so CISA DKIM checks may still hit the same hybrid-domain scenario. If the intent is repo-wide DKIM support, consider applying the same handling there too.
There was a problem hiding this comment.
Yes it does but the code I have on my machine hasn't been updated for years for this module and the code in github.com looks different than what I see deployed. The deployed code has no section for onmicrosoft.com handling - or maybe I'm looking in the wrong place (I'm no developer and all this git stuff is confusing) so I tried to find the other module and update it but it looks nothing like the copy on my PC. So I left that one well alone.
There was a problem hiding this comment.
Ah, ok. It sounds like you may be working on an outdated fork. I'll review and help get it synced with the upstream main branch.
|
@brianreidc7, can you please review the two comments above from the GitHub Copilot review. They look relevant to me. I'll perform a final review myself on Friday if this is ready! |
|
This cisa one also throws an error for the same reason I fixed the cis one, but the cisa code doesn't have any onmicrosoft.com handling in it or I was looking at the wrong file |
Description
This test errors when it encounters the tenantname.mail.onmicrosoft.com domain. Added a elseif to deal with this case. This domain name occurs whenever Exchange Online Hybrid Mode has been implemented.
Contribution Checklist
Before submitting this PR, please confirm you have completed the following:
/powershell/tests/pester.ps1on your local system.Join us at the Maester repository discussions 💬 or Entra Discord 🧑💻 for more help and conversations!