Update logic for AS04 check in EIDSCA and format of cmdlet header#1584
Update logic for AS04 check in EIDSCA and format of cmdlet header#1584Cloud-Architekt wants to merge 3 commits intomainfrom
Conversation
- Updated function definitions in multiple scripts to remove hidden characters. - Improved documentation formatting in the @templateps1.txt and various test scripts. - Adjusted logic in Test-MtEidscaAS04.ps1 to use select-object for unique values.
Up to standards ✅🟢 Issues
|
Deploying maester with
|
| Latest commit: |
f017d1b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fc740eea.maester.pages.dev |
| Branch Preview URL: | https://tnh-eidscaas04.maester.pages.dev |
|
@SamErde: There have been some updates to
Several changes which has been made directly in Maester has been updated in the original source of EIDSCA now. Could you please review the PR and the described changes? |
…fined in EIDSCA schema now
There was a problem hiding this comment.
Pull request overview
This PR updates the EIDSCA test suite and its generator/templates by removing hidden characters from function declarations, adjusting formatting in generated headers/docs, and updating the AS04 check to account for multiple includeTargets values.
Changes:
- Remove hidden BOM/zero-width characters from many EIDSCA internal test function headers and reformat cmdlet help headers.
- Update EIDSCA.AS04 logic/docs to treat
.includeTargets.isUsableForSignInas potentially multi-valued (viaSelect-Object -Unique). - Adjust the EIDSCA test-generation script formatting/regexes and update how it emits the
BeforeDiscoveryblock.
Reviewed changes
Copilot reviewed 49 out of 50 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| website/docs/tests/eidsca/EIDSCA.AS04.md | Updates AS04 docs “Setting” field to reflect uniqueness handling. |
| tests/EIDSCA/Test-EIDSCA.Generated.Tests.ps1 | Updates generated EIDSCA Pester tests (discovery + AS04 comment snippet). |
| powershell/public/eidsca/Test-MtEidscaControl.ps1 | Re-formats comment-based help header placement/format. |
| powershell/internal/eidsca/Test-MtEidscaST09.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaST08.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaPR06.ps1 | Removes hidden character + tweaks “not set” detection logic. |
| powershell/internal/eidsca/Test-MtEidscaPR05.ps1 | Removes hidden character + tweaks “not set” detection logic. |
| powershell/internal/eidsca/Test-MtEidscaPR03.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaPR02.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaPR01.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaCR04.ps1 | Removes hidden character + tweaks “not set” detection logic. |
| powershell/internal/eidsca/Test-MtEidscaCR03.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaCR02.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaCR01.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaCP04.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaCP03.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaCP01.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAV01.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAT02.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAT01.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAS04.ps1 | Updates AS04 logic/docs to use Select-Object -Unique for multi-target values. |
| powershell/internal/eidsca/Test-MtEidscaAS04.md | Updates AS04 markdown “Test script” snippet to reflect uniqueness handling. |
| powershell/internal/eidsca/Test-MtEidscaAP14.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAP10.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAP09.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAP08.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAP07.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAP06.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAP05.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAP04.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAP01.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAM10.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAM09.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAM07.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAM06.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAM04.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAM03.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAM02.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAM01.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAG03.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAG02.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAG01.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAF06.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAF05.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAF04.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAF03.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAF02.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/Test-MtEidscaAF01.ps1 | Removes hidden character before function keyword. |
| powershell/internal/eidsca/@templateps1.txt | Re-formats template help text indentation (but still contains BOM). |
| build/eidsca/Update-EidscaTests.ps1 | Updates generator formatting, MITRE/title regex, and emitted BeforeDiscovery header. |
Comments suppressed due to low confidence (1)
build/eidsca/Update-EidscaTests.ps1:75
- In
GetRecommendedValueMarkdown, the numeric-comparison branch (theStartsWith('>')/StartsWith('<')block) references$RecommendedValuewhen composing the markdown, but that variable isn’t defined in this function. This causes missing/incorrect markdown for numeric recommended values. Please use the parameter ($RecommendedValueMarkdown) or extract the numeric portion before composing the string.
if ($RecommendedValueMarkdown -like "@('*,*')") {
$RecommendedValueMarkdown = $RecommendedValueMarkdown -replace "@\(", "" -replace "\)", ""
return "$RecommendedValueMarkdown"
} elseif ($RecommendedValueMarkdown.StartsWith(">") -or $RecommendedValueMarkdown.StartsWith("<")) {
$RecommendedValueText = (GetCompareOperator($RecommendedValueMarkdown)).Text
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| BeforeDiscovery { | ||
| try { | ||
| $AuthorizationPolicyAvailable = (Invoke-MtGraphRequest -RelativeUri 'policies/authorizationpolicy' -ApiVersion beta) | ||
| $SettingsApiAvailable = (Invoke-MtGraphRequest -RelativeUri 'settings' -ApiVersion beta).values.name | ||
| $EntraIDPlan = Get-MtLicenseInformation -Product 'EntraID' | ||
| $EnabledAuthMethods = (Get-MtAuthenticationMethodPolicyConfig -State Enabled).Id | ||
| $EnabledAdminConsentWorkflow = (Invoke-MtGraphRequest -RelativeUri 'policies/adminConsentRequestPolicy' -ApiVersion beta).isenabled | ||
| } catch { | ||
| $EntraIDPlan = "NotConnected" | ||
| } | ||
| $AuthorizationPolicyAvailable = (Invoke-MtGraphRequest -RelativeUri 'policies/authorizationpolicy' -ApiVersion beta) | ||
| $SettingsApiAvailable = (Invoke-MtGraphRequest -RelativeUri 'settings' -ApiVersion beta).values.name | ||
| $EntraIDPlan = Get-MtLicenseInformation -Product 'EntraID' | ||
| $EnabledAuthMethods = (Get-MtAuthenticationMethodPolicyConfig -State Enabled).Id | ||
| $EnabledAdminConsentWorkflow = (Invoke-MtGraphRequest -RelativeUri 'policies/adminConsentRequestPolicy' -ApiVersion beta).isenabled |
There was a problem hiding this comment.
BeforeDiscovery now runs several Graph calls without any error handling. If Connect-MgGraph hasn’t been run or required permissions aren’t present, an exception here will abort test discovery and prevent the entire EIDSCA suite from loading. Please reintroduce a try/catch (and set sensible defaults / add a skipped reason) similar to other test suites’ BeforeDiscovery blocks.
| <# | ||
| Check if "https://graph.microsoft.com/beta/policies/authenticationMethodsPolicy/authenticationMethodConfigurations('Sms')" | ||
| .includeTargets.isUsableForSignIn -eq 'false' | ||
| .includeTargets.isUsableForSignIn | select-object -unique -eq 'false' | ||
| #> | ||
| Test-MtEidscaControl -CheckId AS04 | Should -Be 'false' |
There was a problem hiding this comment.
The example expression in the comment is not valid PowerShell as written: piping to select-object -unique and then appending -eq 'false' without parentheses will be parsed as an (invalid) parameter to Select-Object rather than a comparison. Please update the example to parenthesize the pipeline (or split into two statements) so it reflects executable syntax.
There was a problem hiding this comment.
Was this a find/replace all issue?
| Queries policies/authenticationMethodsPolicy/authenticationMethodConfigurations('Sms') | ||
| and returns the result of | ||
| graph/policies/authenticationMethodsPolicy/authenticationMethodConfigurations('Sms').includeTargets.isUsableForSignIn -eq 'false' | ||
| graph/policies/authenticationMethodsPolicy/authenticationMethodConfigurations('Sms').includeTargets.isUsableForSignIn | select-object -unique -eq 'false' | ||
|
|
There was a problem hiding this comment.
The docstring’s “returns the result of … | select-object -unique -eq 'false'” example is not valid PowerShell without parentheses; -eq 'false' will be parsed as a parameter to Select-Object rather than a comparison. Please parenthesize the pipeline or split it into two steps in the documentation so the example is syntactically correct.
There was a problem hiding this comment.
Was this a find/replace all issue?
| [string]$tenantValue = $result.includeTargets.isUsableForSignIn | select-object -unique | ||
| $testResult = $tenantValue -eq 'false' | ||
| $tenantValueNotSet = ($null -eq $tenantValue -or $tenantValue -eq "") -and 'false' -notlike '*$null*' |
There was a problem hiding this comment.
$result.includeTargets.isUsableForSignIn | Select-Object -Unique can still output multiple values (e.g., a mix of True/False across targets). Casting that pipeline to [string] will yield System.Object[] when multiple unique values exist, which breaks the comparison and the rendered output. Consider keeping it as an array (e.g., @(... ) | Select-Object -Unique) and defining the pass/fail rule explicitly (e.g., pass only if all values are false, otherwise fail), and format the output by joining the unique values.
| [string]$tenantValue = $result.includeTargets.isUsableForSignIn | select-object -unique | |
| $testResult = $tenantValue -eq 'false' | |
| $tenantValueNotSet = ($null -eq $tenantValue -or $tenantValue -eq "") -and 'false' -notlike '*$null*' | |
| $tenantValues = @($result.includeTargets.isUsableForSignIn | Select-Object -Unique) | |
| [string]$tenantValue = ($tenantValues | ForEach-Object { [string]$_ }) -join ', ' | |
| $testResult = $tenantValues.Count -gt 0 -and (@($tenantValues | Where-Object { $_ -ne $false })).Count -eq 0 | |
| $tenantValueNotSet = $tenantValues.Count -eq 0 -and 'false' -notlike '*$null*' |
There was a problem hiding this comment.
Was this a find/replace all issue?
| ``` | ||
| https://graph.microsoft.com/beta/policies/authenticationMethodsPolicy/authenticationMethodConfigurations('Sms') | ||
| .includeTargets.isUsableForSignIn -eq 'false' | ||
| .includeTargets.isUsableForSignIn | select-object -unique -eq 'false' |
There was a problem hiding this comment.
The “Test script” snippet is not valid PowerShell as written: .includeTargets.isUsableForSignIn | select-object -unique -eq 'false' needs parentheses (or two statements) for the -eq comparison to apply to the pipeline result. Please correct the snippet so readers can copy/paste it successfully.
| .includeTargets.isUsableForSignIn | select-object -unique -eq 'false' | |
| (.includeTargets.isUsableForSignIn | Select-Object -Unique) -eq 'false' |
There was a problem hiding this comment.
Was this a find/replace all issue?
| @@ -294,7 +294,7 @@ function GetPortalDeepLinkMarkdown($portalDeepLink) { | |||
| $result = $portalDeepLink | |||
| if (![string]::IsNullOrEmpty($portalDeepLink)) { | |||
| $domain = ($uri -as [System.URI]).Host | |||
There was a problem hiding this comment.
GetPortalDeepLinkMarkdown uses $domain = ($uri -as [System.URI]).Host, but $uri is not defined in this function. This will always set $domain to $null (or throw under strict mode) and generates incorrect markdown (e.g., "View in "). Use $portalDeepLink for URI parsing (or assign $uri = $portalDeepLink first).
| $domain = ($uri -as [System.URI]).Host | |
| $domain = ($portalDeepLink -as [System.URI]).Host |
| @@ -1,20 +1,20 @@ | |||
| function %PSFunctionName% { | |||
There was a problem hiding this comment.
This template file still begins with a hidden BOM/zero-width character before function (visible as function). Since the PR is removing hidden characters from generated functions, leaving the BOM here will cause regenerated scripts to reintroduce it. Please remove the BOM from the template’s first line.
| function %PSFunctionName% { | |
| function %PSFunctionName% { |
| [int]$tenantValue = $result.values | where-object name -eq 'LockoutDurationInSeconds' | select-object -expand value | ||
| $testResult = $tenantValue -ge 60 | ||
| $tenantValueNotSet = $null -eq $tenantValue -and 60 -notlike '*$null*' | ||
| $tenantValueNotSet = ($null -eq $tenantValue -or $tenantValue -eq "") -and 60 -notlike '*$null*' |
There was a problem hiding this comment.
$tenantValue is explicitly cast to [int], so it will never be $null or "" (missing values typically coerce to 0). As a result, $tenantValueNotSet will never be true and the “not configured explicitly” branch can’t be reached. Please check for presence before casting (e.g., evaluate the raw expanded value or whether the setting name exists) or use a nullable type.
| [int]$tenantValue = $result.values | where-object name -eq 'LockoutThreshold' | select-object -expand value | ||
| $testResult = $tenantValue -le 10 | ||
| $tenantValueNotSet = $null -eq $tenantValue -and 10 -notlike '*$null*' | ||
| $tenantValueNotSet = ($null -eq $tenantValue -or $tenantValue -eq "") -and 10 -notlike '*$null*' |
There was a problem hiding this comment.
$tenantValue is cast to [int], so it won’t be $null/empty; missing values typically coerce to 0. This makes $tenantValueNotSet effectively dead code. Please detect “not set” before casting (or use a nullable type / presence check) so the explicit-not-configured messaging works as intended.
| [int]$tenantValue = $result.requestDurationInDays | ||
| $testResult = $tenantValue -le 30 | ||
| $tenantValueNotSet = $null -eq $tenantValue -and 30 -notlike '*$null*' | ||
| $tenantValueNotSet = ($null -eq $tenantValue -or $tenantValue -eq "") -and 30 -notlike '*$null*' |
There was a problem hiding this comment.
$tenantValue is cast to [int], so it won’t be $null/empty; missing values typically coerce to 0. This makes $tenantValueNotSet effectively unreachable and the “not configured explicitly” message won’t trigger. Please detect unset values before casting or use a nullable/presence check.
SamErde
left a comment
There was a problem hiding this comment.
It looks like a copy/paste issue added the text select-object -unique in several places unintentionally. See examples at:
- https://github.com/maester365/maester/pull/1584/changes#diff-97e08097528d09052791d4bbd4480671aa09ceeb4c40d57f6215f194a241ff37R8
- https://github.com/maester365/maester/pull/1584/changes#diff-5a0f63b198f6cafb7165585953ad4038d75cc90cbe314399c16c168fcd92a994R12
- https://github.com/maester365/maester/pull/1584/changes#diff-5a0f63b198f6cafb7165585953ad4038d75cc90cbe314399c16c168fcd92a994R17
- https://github.com/maester365/maester/pull/1584/changes#diff-5a0f63b198f6cafb7165585953ad4038d75cc90cbe314399c16c168fcd92a994R29
- https://github.com/maester365/maester/pull/1584/changes#diff-ac2284571e3aae689fd018ff91ec6b49cacd63e5e32d9ac84e512cf65f047612R372
- https://github.com/maester365/maester/pull/1584/changes#diff-46e8663646817d19a3a47ab43dfeee21289c39612d91a18941af0e81293e1702R27
| } catch { | ||
| $EntraIDPlan = "NotConnected" | ||
| } | ||
| $AuthorizationPolicyAvailable = (Invoke-MtGraphRequest -RelativeUri 'policies/authorizationpolicy' -ApiVersion beta) |
There was a problem hiding this comment.
It looks like something happened here. The try/catch is removed and the proper indentation was lost.
Description
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!