Update auth lib to latest#29505
Conversation
…tiple projects Assumption: transitive dependency, not referenced directly by code, OK to be provided by Az.Accounts at runtime
| Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status. |
| @@ -22,10 +22,12 @@ namespace Microsoft.Azure.PowerShell.Authenticators.Factories | |||
| { | |||
| public class AzureCredentialFactory | |||
| { | |||
| #pragma warning disable CS0618 // ManagedIdentityCredential(string) is obsolete; suppressed pending migration to ManagedIdentityId API | |||
| public virtual TokenCredential CreateManagedIdentityCredential(string clientId) | |||
| { | |||
| return new ManagedIdentityCredential(clientId); | |||
There was a problem hiding this comment.
To avoid more testing down the road, switch to the replacement overload for user-assigned managed identity with client ID. Then you can remove the warning suppression for this method.
| return new ManagedIdentityCredential(clientId); | |
| return new ManagedIdentityCredential(ManagedIdentityId.FromUserAssignedClientId(clientId)); |
| @@ -56,8 +56,10 @@ public async Task UserAssignedMSI() | |||
|
|
|||
| var mockAzureCredentialFactory = new Mock<AzureCredentialFactory>(); | |||
| //id must be equal to accountId | |||
| #pragma warning disable CS0618 // ManagedIdentityCredential(string) is obsolete; suppressed pending migration | |||
| mockAzureCredentialFactory.Setup(f => f.CreateManagedIdentityCredential(It.Is<string>(id => id == accountId))) | |||
| .Returns(new ManagedIdentityCredential(accountId)); | |||
There was a problem hiding this comment.
See my other comment about using the replacement overload
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 62 changed files in this pull request and generated 11 comments.
Comments suppressed due to low confidence (2)
src/Search/Search/Models/PSIdentity.cs:33
- The explicit conversion operator parameter type is
Azure.Management.Search.Models.Identity, but this module’s SDK types are inMicrosoft.Azure.Management.Search.Models(and the file already imports that namespace). This will not compile unless anAzure.Management.Search.Modelsnamespace exists. Update the operator signature to use the SDKIdentitytype.
public PSIdentityType Type { get; set; }
public static explicit operator PSIdentity(Azure.Management.Search.Models.Identity v)
{
PSIdentityType? identityType = v?.Type.ParsePSIdentityType();
if (identityType.HasValue)
src/OperationalInsights/OperationalInsights/Models/PSIdentity.cs:33
- The file imports
Microsoft.Azure.Management.OperationalInsights.Models, but the constructor parameter type isManagement.OperationalInsights.Models.Identity, which doesn’t resolve without aManagementnamespace alias. This will fail compilation; use the SDKIdentitytype from the imported namespace (or fully-qualifyMicrosoft.Azure.Management.OperationalInsights.Models.Identity).
TenantId = tenantId;
Type = string.IsNullOrEmpty(type) ? "SystemAssigned" : type;
}
public PSIdentity(Management.OperationalInsights.Models.Identity identity)
{
this.PrincipalId = identity.PrincipalId;
this.TenantId = identity.TenantId;
this.Type = identity.Type.ToString();
}
| NetworkRuleSet = networkRuleSet ?? service.NetworkRuleSet, | ||
| PublicNetworkAccess = publicNetworkAccess ?? service.PublicNetworkAccess, | ||
| Identity = (Identity)identity ?? service.Identity, | ||
| Identity = (Azure.Management.Search.Models.Identity)identity ?? service.Identity, |
| publicNetworkAccess: publicNetworkAccess, | ||
| identity: (Identity)identity, | ||
| identity: (Azure.Management.Search.Models.Identity)identity, | ||
| networkRuleSet: networkRuleSet, |
| } | ||
|
|
||
| public static explicit operator Identity(PSIdentity v) | ||
| public static explicit operator Azure.Management.Search.Models.Identity(PSIdentity v) | ||
| { | ||
| string identityType = v?.Type.ToString(); | ||
| if (identityType != null) | ||
| { | ||
| return new Identity( | ||
| return new Azure.Management.Search.Models.Identity( | ||
| type: identityType, | ||
| principalId: v?.PrincipalId, | ||
| tenantId: v?.TenantId); |
| public string PublicNetworkAccess { get; private set; } | ||
|
|
||
| public Identity Identity { get; private set; } | ||
| public Azure.Management.CognitiveServices.Models.Identity Identity { get; private set; } |
| } | ||
|
|
||
| updateParameters.Identity = new Identity(resourceIdentityType); | ||
| updateParameters.Identity = new Azure.Management.CognitiveServices.Models.Identity(resourceIdentityType); |
| } | ||
|
|
||
| public Identity GetIdentity() | ||
| public Management.OperationalInsights.Models.Identity GetIdentity() | ||
| { | ||
| return new Identity(this.getIdentityType(), this.PrincipalId, this.TenantId); | ||
| return new Management.OperationalInsights.Models.Identity(this.getIdentityType(), this.PrincipalId, this.TenantId); | ||
| } |
| @@ -18,7 +18,6 @@ | |||
| <PackageReference Include="Portable.BouncyCastle" Version="1.8.8" /> | |||
| <PackageReference Include="Microsoft.Azure.KeyVault" Version="3.0.1" /> | |||
| <PackageReference Include="Microsoft.Azure.KeyVault.WebKey" Version="3.0.1" /> | |||
| <ProjectReference Include="..\RecoveryServices.Backup.CrossRegionRestore.Management.Sdk\RecoveryServices.Backup.CrossRegionRestore.Management.Sdk.csproj" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> |
| if (clientId != null) | ||
| { | ||
| return new ManagedIdentityCredential(ManagedIdentityId.FromUserAssignedClientId(clientId)); | ||
| } | ||
| return new ManagedIdentityCredential(ManagedIdentityId.SystemAssigned); |
| - [x] create an agent | ||
| - [x] update azure.identity and msal | ||
| - [x] address namespace conflict of types named Identity because Azure.Core pulls in namespaces of Microsoft.Identity (because of Microsoft.Identity.Client) | ||
| - [x] update other dependencies | ||
| - [ ] System.Text.Json might not be necessary for PowerShell 7+? | ||
| - [ ] Errors of "Could not load file or assembly 'Microsoft.Azure.PowerShell.AssemblyLoading, Version=5.3.3.0" during smoke test. Might be regression of parallal loading, need to investigate. |
| <PackageReference Include="Microsoft.Identity.Client" Version="4.82.1" /> | ||
| <PackageReference Include="Microsoft.Identity.Client.Extensions.Msal" Version="4.82.1" /> | ||
| <PackageReference Include="Microsoft.Identity.Client.Broker" Version="4.82.1" /> | ||
| <PackageReference Include="Azure.Identity" Version="1.21.0" /> |
There was a problem hiding this comment.
You'll need to use this version of Azure.Core instead, so that extra querystring parameters can be passed via the new AdditionalQueryParameters property. All the Azure.Identity credentials you need live in Azure.Core now.
| <PackageReference Include="Azure.Identity" Version="1.21.0" /> | |
| <PackageReference Include="Azure.Core" Version="1.55.0" /> |
| </ItemGroup> | ||
| <ItemGroup> | ||
| <PackageReference Include="Azure.Core" Version="1.50.0"/> | ||
| <PackageReference Include="Azure.Core" Version="1.54.0"/> |
There was a problem hiding this comment.
Bump to latest stable release
| <PackageReference Include="Azure.Core" Version="1.54.0"/> | |
| <PackageReference Include="Azure.Core" Version="1.55.0"/> |
Description
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/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.## Upcoming Releaseheader in the past tense.ChangeLog.mdif no new release is required, such as fixing test case only.