App Registration credential `$Expired` variable not reset between iterations

[StrongWind1]

Disclaimer: This issue was identified and written by Claude Code (model: claude-opus-4-6-1m) during an automated code review, and has had a cursory review by a human before submission.

Summary

In check_AppRegistrations.psm1, the $Expired variable is only assigned inside an if ($null -ne $creds.EndDateTime) block. When a credential has a null EndDateTime (i.e., no expiration), $Expired retains the value from the previous credential in the loop. This causes the credential to incorrectly inherit the expired/not-expired status of a completely different credential.

Affected file

modules/check_AppRegistrations.psm1

Evidence

Secrets loop (lines 297-312)

# Line 297-312
$AppCredentialsSecrets = foreach ($creds in $item.PasswordCredentials) {
    if ($null -ne $creds.EndDateTime) {
        try {
            $endDate = [datetime]$creds.EndDateTime
            $Expired = ($endDate - (Get-Date)).TotalDays -le 0
        } catch {
            $Expired = "?"
        }
    }
    # If EndDateTime is null, $Expired is NOT reset — it keeps the value
    # from the previous iteration
    [pscustomobject]@{
        Type = "Secret"
        DisplayName = ...
        EndDateTime = $creds.EndDateTime
        StartDateTime = $creds.StartDateTime
        Expired = $Expired       # <-- stale value when EndDateTime is null
        AppName = $item.DisplayName
    }
}

Certificates loop (lines 327-343) — same pattern

# Line 327-343
$AppCredentialsCertificates = foreach ($creds in $item.KeyCredentials) {
    if ($null -ne $creds.EndDateTime) {
        try {
            $endDate = [datetime]$creds.EndDateTime
            $Expired = ($endDate - (Get-Date)).TotalDays -le 0
        } catch {
            $Expired = "?"
        }
    }
    [pscustomobject]@{
        Type = "Certificate"
        DisplayName = ...
        EndDateTime = $creds.EndDateTime
        StartDateTime = $creds.StartDateTime
        Expired = $Expired       # <-- stale value when EndDateTime is null
    }
}

Same pattern in the Enterprise Apps credential processing (lines 95-125)

# Lines 97-110
if ($type -eq "Secret" ) {
    if ($null -ne $Object.EndDateTime) {
        if (($Object.EndDateTime - (Get-Date).Date).TotalDays -le 0) {
            $Expired = $True
        } else {
            $Expired = $False
        }
    }
    # $Expired not reset if EndDateTime is null
    [PSCustomObject]@{
        ...
        Expired = $Expired
    }
}

Reproduction scenario

1. An app registration has two secrets:
   • Secret A: EndDateTime = 2024-01-01 (expired)
   • Secret B: EndDateTime = $null (no expiration)
2. Processing order: Secret A first, then Secret B
3. Secret A sets $Expired = $true
4. Secret B has null EndDateTime, so the if block is skipped — $Expired remains $true
5. Secret B is reported as Expired = True despite having no expiration date

Impact

The App Registrations report may show credentials as expired when they are not (or vice versa), depending on the processing order of credentials within the same application.

Suggested fix

Reset $Expired to a default value at the start of each iteration:

$AppCredentialsSecrets = foreach ($creds in $item.PasswordCredentials) {
    $Expired = $null  # or "N/A" for credentials with no expiration
    if ($null -ne $creds.EndDateTime) {
        ...
    }
    ...
}

Version

V20260316

[zh54321]

Thanks for reporting this.
I do not think this has real-world impact, since secrets and certificates are expected to have an expiration date. However, I agree that initializing the variable explicitly is a code quality improvement. I updated the credential handling to initialize a fresh Expired value for each secret/certificate.