Fix Get-AzSubscription silently ignoring mismatched -TenantId under MSI auth - #29994
Conversation
…SI auth Under Managed Service Identity authentication, Get-AzSubscription only supports the tenant of the Default Context. Passing a different -TenantId was silently swallowed (no output, no error), leaving scripts to fail confusingly downstream. Throw a clear PSInvalidOperationException instead so the mismatch is obvious and actionable. Fixes Azure#25710
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Fixes Get-AzSubscription behavior under Managed Service Identity (MSI) authentication so that providing a -TenantId that does not match the current context’s tenant results in an explicit, actionable error instead of silently returning no output.
Changes:
- Added an MSI tenant mismatch check in
GetAzureRMSubscriptionCommandthat throws aPSInvalidOperationExceptionmarked asErrorKind.UserError. - Introduced a new localized resource string for the mismatch message and updated the generated resource designer.
- Added/updated release notes and a unit test covering the new exception behavior.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Accounts/Accounts/Subscription/GetAzureRMSubscription.cs | Adds MSI tenant mismatch validation and throws a user-facing error. |
| src/Accounts/Accounts/Properties/Resources.resx | Adds the localized MSI tenant mismatch message text. |
| src/Accounts/Accounts/Properties/Resources.Designer.cs | Adds the strongly-typed accessor for the new resource string. |
| src/Accounts/Accounts/ChangeLog.md | Documents the behavior change for the upcoming release. |
| src/Accounts/Accounts.Test/UnitTest/GetAzureRMSubscriptionTest.cs | Adds a unit test asserting the mismatch exception type/message/ErrorKind. |
Files not reviewed (1)
- src/Accounts/Accounts/Properties/Resources.Designer.cs: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| * Fixed `Get-AzSubscription` to throw a clear error instead of silently returning nothing when `-TenantId` does not match the current context under Managed Service Identity (MSI) authentication. | ||
|
|
| using Microsoft.Azure.Commands.Common; | ||
| using Microsoft.Azure.Commands.Common.Authentication; | ||
| using Microsoft.Azure.Commands.Common.Authentication.Models; | ||
| using System; | ||
| using System.Management.Automation; | ||
| using Xunit; |
| Assert.Equal( | ||
| string.Format( | ||
| "The current context is using Managed Service Identity (MSI) authentication which only supports the tenant of the Managed Identity ({0}). The requested TenantId '{1}' does not match and cannot be used. Remove the -TenantId parameter or use Connect-AzAccount with a different authentication method to target a different tenant.", | ||
| defaultTenantId, | ||
| requestedTenantId), | ||
| exception.Message); | ||
| Assert.Equal(ErrorKind.UserError, exception.Data[AzurePSErrorDataKeys.ErrorKindKey]); |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
|
@microsoft-github-policy-service agree |
1 similar comment
|
@microsoft-github-policy-service agree |
… fix - Reference Azure#25710 in the changelog entry so users can trace context. - Import the missing Microsoft.Azure.Commands.Profile namespace so the new unit test compiles. - Assert against the actual localized Resources.MSITenantMismatch string instead of a hardcoded literal, so the test can't silently drift from the real message.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- src/Accounts/Accounts/Properties/Resources.Designer.cs: Generated file
Suppressed comments (1)
src/Accounts/Accounts/Properties/Resources.resx:436
- The message mixes terminology (“Managed Service Identity (MSI)” vs “Managed Identity”) and is a bit ambiguous about what
{0}represents (it reads like an identity rather than a tenant ID). Recommend using consistent terminology throughout and clarifying/quoting both tenant ID values consistently (e.g., explicitly call out ‘current tenant id’ and ‘requested tenant id’).
<data name="MSITenantMismatch" xml:space="preserve">
<value>The current context is using Managed Service Identity (MSI) authentication which only supports the tenant of the Managed Identity ({0}). The requested TenantId '{1}' does not match and cannot be used. Remove the -TenantId parameter or use Connect-AzAccount with a different authentication method to target a different tenant.</value>
</data>
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
Live test results — TestFx
|
Per Copilot review feedback on PR Azure#29994: use consistent 'Managed Identity' terminology instead of mixing MSI/Managed Identity, and make explicit which tenant id is which ('current tenant' vs the requested -TenantId).
| } | ||
| else | ||
| { | ||
| ThrowMSITenantMismatchError(DefaultContext.Tenant.Id, TenantId); |
|
|
||
| private void ThrowMSITenantMismatchError(string defaultTenant, string requestedTenant) | ||
| { | ||
| PSInvalidOperationException exception = new PSInvalidOperationException(string.Format(Resources.MSITenantMismatch, defaultTenant, requestedTenant)); |
| } | ||
| else | ||
| { | ||
| ThrowMSITenantMismatchError(DefaultContext.Tenant.Id, TenantId); |
Live test results — TestFx
|
There was a problem hiding this comment.
Reviewer summary — PR #29994
CI: ✅ all checks passing (1/1).
Live test (TestFx Record): ❌ FAIL — run https://github.com/Azure/issue-sentinel/actions/runs/32268892413
The live-test workflow re-ran the changed test file src/Accounts/Accounts.Test/UnitTest/GetAzureRMSubscriptionTest.cs
(filter FullyQualifiedName~GetAzureRMSubscriptionTest) against this PR's head commit 3c4c55d4c86177b3e0a7ff718763eb42581019d6
and it failed (exit code non-zero). This is a repeat failure — the prior live test at commit f514ddb8834a2c9b166bdfba581300a9ad496ae6
also failed the same test.
Since this PR was flagged for a human/Azure Client Tools Agent review rather than an automated Copilot fix cycle, please have
a maintainer or the PR author investigate GetAzureRMSubscriptionTest locally (e.g. dotnet test src/Accounts/Accounts.Test --filter FullyQualifiedName~GetAzureRMSubscriptionTest) to see the concrete assertion failure, since the workflow did not
capture a detailed log this run. Please re-request review once the test is passing again.
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
🤖 PR Validation —⚠️ Review suggested
️✔️Az.Aks
️✔️Az.Compute
️✔️Az.EventHub
️✔️Az.Functions
️✔️Az.KubernetesConfiguration
️✔️Az.ManagedServiceIdentity
️✔️Az.Monitor
️✔️Az.Network
️✔️Az.OperationalInsights
️✔️Az.PrivateDns
️✔️Az.Resources
️✔️Az.Sql
Description
Fixes #25710.
Under Managed Service Identity (MSI) authentication,
Get-AzSubscriptiononly supports the tenant of the Default Context. When a caller passed-TenantIdthat did not matchDefaultContext.Tenant.Id, the cmdlet silently did nothing — no output, no warning, no error — leaving the caller with no indication their request was ignored.This change throws a clear
PSInvalidOperationException(ErrorKind.UserError) in that mismatch case, with an actionable message pointing to removing-TenantIdor usingConnect-AzAccountto target a different tenant. Behavior for the matching-tenant case and for non-MSI auth is unchanged.Mandatory Checklist
Please choose the target release of Azure PowerShell. (⚠️ Target release is a different concept from API readiness. Please click below links for details.)
Check this box to confirm: I have read the Submitting Changes section of
CONTRIBUTING.mdand reviewed the following information:ChangeLog.mdfile(s) appropriatelysrc/Accounts/Accounts/ChangeLog.mdunder## Upcoming Release.GetAzureRMSubscriptionTest.cscovering the MSI tenant-mismatch exception (type, message,ErrorKind).Note: this is my first PR to this repo — I understand the Microsoft CLA needs to be signed before merge.