From bce1f5ad5ed8c14e1688b39093df2d199162c352 Mon Sep 17 00:00:00 2001 From: James Xian Date: Thu, 29 Oct 2026 13:56:15 -0700 Subject: [PATCH] Fix outbound federated credential name collisions Name new credentials by outbound managed identity principal ID while reusing matching legacy trusts and preserving unrelated credentials. Add regression coverage and document the naming behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- setup/docs/README.md | 5 +++ setup/support/Epp.Setup.psm1 | 6 ++- setup/tests/Certificates.Tests.ps1 | 62 ++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 2 deletions(-) diff --git a/setup/docs/README.md b/setup/docs/README.md index 259668a..ed0d25a 100644 --- a/setup/docs/README.md +++ b/setup/docs/README.md @@ -165,6 +165,11 @@ Setup configures: - That Microsoft principal's tenant-wide Graph `Application.Read.All`, identifier URI, public JWE certificate, and Easy Auth caller restriction. - The disclosed outbound managed-identity federated credential for OAuth providers. +New outbound federated credentials are named using the managed identity's principal ID, so a +recreated identity does not collide with a credential for the previous identity. Exact +issuer/subject/audience matches are reused under any name, including legacy Function-based names. +Other trusts are preserved, not overwritten or automatically removed. + ## Prerequisites for Step 2 - **Azure Cloud Shell in PowerShell mode, or PowerShell 7+**, with an Azure CLI user login. diff --git a/setup/support/Epp.Setup.psm1 b/setup/support/Epp.Setup.psm1 index 770f894..41ab2ac 100644 --- a/setup/support/Epp.Setup.psm1 +++ b/setup/support/Epp.Setup.psm1 @@ -2283,7 +2283,7 @@ function Set-EppApplicationEndpoint { $issuer = "https://login.microsoftonline.com/$($Inputs.TenantId)/v2.0" $audience = 'api://AzureADTokenExchange' - $credentialName = "epp-$($Outputs.functionAppName.value)-outbound" + $credentialName = "epp-$($Outputs.outboundPrincipalId.value)-outbound" $credentials = Invoke-EppDirectoryCommand -Inputs $Inputs -Arguments @('ad', 'app', 'federated-credential', 'list', '--id', $application.Id) if ($credentials -isnot [Array]) { throw 'Azure CLI returned an invalid federated-credential list.' } $credentials = @($credentials | ForEach-Object { [pscustomobject]$_ }) @@ -2292,7 +2292,9 @@ function Set-EppApplicationEndpoint { @($_.Audiences).Count -eq 1 -and $_.Audiences[0] -ceq $audience }) if (-not $matching.Count) { - if (@($credentials | Where-Object Name -eq $credentialName).Count) { throw 'The outbound federated credential name is already used for a different trust. It will not be overwritten.' } + if (@($credentials | Where-Object Name -eq $credentialName).Count) { + throw "The outbound federated credential '$credentialName' is already used for a different trust. Expected issuer '$issuer', subject '$($Outputs.outboundPrincipalId.value)', and audience '$audience'. It will not be overwritten." + } New-EppFederatedCredential -Inputs $Inputs -ObjectId $application.Id -Credential @{ name = $credentialName; issuer = $issuer; subject = $Outputs.outboundPrincipalId.value; audiences = @($audience) } diff --git a/setup/tests/Certificates.Tests.ps1 b/setup/tests/Certificates.Tests.ps1 index 47f66a9..735459c 100644 --- a/setup/tests/Certificates.Tests.ps1 +++ b/setup/tests/Certificates.Tests.ps1 @@ -658,6 +658,68 @@ try { Assert ($saved.Count -eq 1 -and $saved[0].providerTenantId -ceq $result.providerTenantId -and $saved[0].accessTokenScopeTenant -ceq $result.accessTokenScopeTenant -and $saved[0].allowedProviderTenantIds -contains $result.accessTokenScopeTenant) 'Persist both tenant roles and the OAuth authority in the approved allowlist.' + + $script:FederatedCredentials = @() + $script:FederationCreates = 0 + function script:Invoke-EppDirectoryCommand { + param($Inputs, $Arguments) + Assert (($Arguments -join ' ') -ceq "ad app federated-credential list --id $($script:Application.Id)") 'List credentials only on the selected application.' + return ,$script:FederatedCredentials + } + function script:New-EppFederatedCredential { + param($Inputs, $ObjectId, $Credential) + Assert ($ObjectId -ceq $script:Application.Id) 'Create federation only on the selected application.' + Assert (@($script:FederatedCredentials | Where-Object name -eq $Credential.name).Count -eq 0) 'Never create a duplicate credential name.' + $script:FederationCreates++ + $script:FederatedCredentials += $Credential + } + $context.CertificatePlan = Get-EppCertificatePlan @planArgs @reuseArgs -NonInteractive + $registration.ConfigureFederation = $true + $registration.Outputs.functionAppName = @{ value = $names.functionApp } + $registration.Outputs.outboundPrincipalId = @{ value = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' } + $expectedName = 'epp-aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa-outbound' + $expectedIssuer = "https://login.microsoftonline.com/$($inputs.TenantId)/v2.0" + & $script:SetEndpointWithoutFederation @registration -KeyId $keyId + $trust = $script:FederatedCredentials[0] + Assert ($trust.name -ceq $expectedName -and $trust.issuer -ceq $expectedIssuer -and + $trust.subject -ceq $registration.Outputs.outboundPrincipalId.value -and + $trust.audiences.Count -eq 1 -and $trust.audiences[0] -ceq 'api://AzureADTokenExchange') 'Name new federation by principal ID and preserve the exact trust.' + & $script:SetEndpointWithoutFederation @registration -KeyId $keyId + Assert ($script:FederationCreates -eq 1) 'An unchanged rerun must reuse federation.' + + $legacyTrust = @{} + $trust + $legacyTrust.name = "epp-$($names.functionApp)-outbound" + $script:FederatedCredentials = @($legacyTrust) + & $script:SetEndpointWithoutFederation @registration -KeyId $keyId + Assert ($script:FederationCreates -eq 1) 'Reuse an exact legacy trust without creating another credential.' + foreach ($mismatch in @( + @{ subject = 'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb' }, + @{ issuer = 'https://login.microsoftonline.com/bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb/v2.0' }, + @{ audiences = @('api://different-audience') }, + @{ audiences = @('api://AzureADTokenExchange', 'api://extra-audience') } + )) { + $stale = @{} + $legacyTrust + foreach ($entry in $mismatch.GetEnumerator()) { $stale[$entry.Key] = $entry.Value } + $snapshot = $stale | ConvertTo-Json -Compress + $script:FederatedCredentials = @($stale) + & $script:SetEndpointWithoutFederation @registration -KeyId $keyId + Assert ($script:FederatedCredentials.Count -eq 2 -and + $script:FederatedCredentials[1].name -ceq $expectedName -and + ($script:FederatedCredentials[0] | ConvertTo-Json -Compress) -ceq $snapshot) 'A mismatched legacy trust must not block or be overwritten by the current identity trust.' + } + $registration.Outputs.outboundPrincipalId.value = 'cccccccc-cccc-cccc-cccc-cccccccccccc' + & $script:SetEndpointWithoutFederation @registration -KeyId $keyId + Assert ($script:FederatedCredentials.Count -eq 3 -and + $script:FederatedCredentials[2].name -ceq 'epp-cccccccc-cccc-cccc-cccc-cccccccccccc-outbound') 'A recreated identity must get a distinct name even when the Function name is unchanged.' + $conflict = @{} + $script:FederatedCredentials[2] + $conflict.subject = 'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb' + $script:FederatedCredentials = @($conflict) + $beforeCreates = $script:FederationCreates + Assert-Throws { + & $script:SetEndpointWithoutFederation @registration -KeyId $keyId + } "epp-cccccccc-cccc-cccc-cccc-cccccccccccc-outbound.*different trust.*Expected issuer.*subject.*audience.*not be overwritten" + Assert ($script:FederationCreates -eq $beforeCreates -and + $script:FederatedCredentials[0].subject -ceq $conflict.subject) 'A conflicting identity-specific name must still fail without replacing the trust.' } finally { if ($issued) { $issued.Certificate.Dispose() }