Skip to content

Upgrade Microsoft.Identity.Client package reference #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
philon-msft merged 3 commits into from
Jun 1, 2023
Merged

Upgrade Microsoft.Identity.Client package reference #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
923190e
Upgrade package references
philon-msft May 25, 2023
4a1f9ca
Remove .WithExperimentalFeatures()
philon-msft May 25, 2023
a5c3bf9
Support resource ID for client-assigned MI
philon-msft May 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions sample/Sample.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,13 @@
case "3": // User-Assigned managed identity
Write("Redis cache host name: ");
cacheHostName = ReadLine()?.Trim();
Write("Managed identity Client ID: ");
var managedIdentityClientId = ReadLine()?.Trim();
Write("Managed identity Client ID or resource ID: ");
var managedIdentityId = ReadLine()?.Trim();
Write("Managed identity Principal (object) ID ('Username' from the 'Data Access Configuration' blade on the Azure Cache for Redis resource): ");
principalId = ReadLine()?.Trim();
WriteLine("Connecting with a user-assigned managed identity...");

configurationOptions = await ConfigurationOptions.Parse($"{cacheHostName}:6380").ConfigureForAzureWithUserAssignedManagedIdentityAsync(managedIdentityClientId!, principalId!);
configurationOptions = await ConfigurationOptions.Parse($"{cacheHostName}:6380").ConfigureForAzureWithUserAssignedManagedIdentityAsync(managedIdentityId!, principalId!);
configurationOptions.AbortOnConnectFail = true; // Fail fast for the purposes of this sample. In production code, this should remain false to retry connections on startup
LogTokenEvents(configurationOptions);

Expand Down
2 changes: 1 addition & 1 deletion src/AzureCacheForRedis.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public static async Task<ConfigurationOptions> ConfigureForAzureWithSystemAssign
/// Throws on failure by default (configurable in the <see cref="ConfigureForAzureAsync"/> method).
/// </summary>
/// <param name="configurationOptions">The configuration to update.</param>
/// <param name="clientId">Client ID of the user-assigned managed identity.</param>
/// <param name="clientId">Client ID or resource ID of the user-assigned managed identity.</param>
/// <param name="principalId">Principal (object) ID of the user-assigned managed identity.</param>
Copy link

@gladjohn gladjohn May 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MSAL does not support principalId for Managed Identity. Is this something you want us to support?

Copy link
Contributor Author

@philon-msft philon-msft May 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@samsaha-ms can you comment on this? We pass the MI's object/principal ID as the username for the Redis connection, so I believe we use it in some capacity while validating tokens on the server side

Copy link

@samsaha-ms samsaha-ms May 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is something related to what our extension does to interact with Redis, not related to MSAL.
It does two things 1. Acquire token from MSAL 2. Send AUTH request to Redis with principal-id and token to comply with redis command format "auth username password" and we validate this pair in server side.
We could actually make this field optional here, we could parse the acquired token and get the oid field in token and use that value while sending to redis. We can choose it do later after giving some thought as it might add client side overhead of parsing token.

/// <exception cref="MsalServiceException">When the token source is not supported or identified incorrectly.</exception>
/// <exception cref="HttpRequestException">Unable to contact the identity service to acquire a token.</exception>
Expand Down
9 changes: 4 additions & 5 deletions src/CacheIdentityClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

using Microsoft.Identity.Client;
using Microsoft.Identity.Client.AppConfig;
using System;
using System.Threading.Tasks;

Expand All @@ -27,13 +28,11 @@ internal class CacheIdentityClient : ICacheIdentityClient
private readonly Func<bool, Task<AuthenticationResult>> _getToken;

internal static ICacheIdentityClient CreateForSystemAssignedManagedIdentity()
=> new CacheIdentityClient(ManagedIdentityApplicationBuilder.Create()
.WithExperimentalFeatures()
=> new CacheIdentityClient(ManagedIdentityApplicationBuilder.Create(ManagedIdentityId.SystemAssigned)
.Build());

internal static ICacheIdentityClient CreateForUserAssignedManagedIdentity(string clientId)
=> new CacheIdentityClient(ManagedIdentityApplicationBuilder.Create(clientId)
.WithExperimentalFeatures()
internal static ICacheIdentityClient CreateForUserAssignedManagedIdentity(string id)
=> new CacheIdentityClient(ManagedIdentityApplicationBuilder.Create(Guid.TryParse(id, out _) ? ManagedIdentityId.WithUserAssignedClientId(id) : ManagedIdentityId.WithUserAssignedResourceId(id))
.Build());

internal static ICacheIdentityClient CreateForServicePrincipal(string clientId, string tenantId, string secret)
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.Azure.StackExchangeRedis.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@
</PropertyGroup>

<ItemGroup>
<None Include="..\README.md" Pack="true" PackagePath="\"/>
<None Include="..\README.md" Pack="true" PackagePath="\" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Identity.Client" Version="4.53.0" />
<PackageReference Include="Microsoft.Identity.Client" Version="4.54.1" />
<PackageReference Include="StackExchange.Redis" Version="2.6.111" />
</ItemGroup>

Expand Down
8 changes: 4 additions & 4 deletions tests/Microsoft.Azure.StackExchangeRedis.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@

<ItemGroup>
<PackageReference Include="FakeItEasy" Version="7.4.0" />
<PackageReference Include="Microsoft.Identity.Client" Version="4.53.0" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.5.0" />
<PackageReference Include="MSTest.TestAdapter" Version="3.0.2" />
<PackageReference Include="MSTest.TestFramework" Version="3.0.2" />
<PackageReference Include="Microsoft.Identity.Client" Version="4.54.1" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.6.0" />
<PackageReference Include="MSTest.TestAdapter" Version="3.0.3" />
<PackageReference Include="MSTest.TestFramework" Version="3.0.3" />
</ItemGroup>

<ItemGroup>
Expand Down