Skip to content

Commit 4c2626e

Browse files
Implemented GitHub feedback
1 parent 599a7b9 commit 4c2626e

File tree

6 files changed

+88
-25
lines changed

6 files changed

+88
-25
lines changed

src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsManagedIdentitySource.cs

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -288,18 +288,21 @@ public static async Task<bool> ProbeImdsEndpointAsync(
288288

289289
try
290290
{
291-
response = await requestContext.ServiceBundle.HttpManager.SendRequestAsync(
292-
GetValidatedEndpoint(requestContext.Logger, imdsEndpoint, queryParams),
293-
headers,
294-
body: null,
295-
method: HttpMethod.Get,
296-
logger: requestContext.Logger,
297-
doNotThrow: false,
298-
mtlsCertificate: null,
299-
validateServerCertificate: null,
300-
cancellationToken: requestContext.UserCancellationToken,
301-
retryPolicy: retryPolicy)
302-
.ConfigureAwait(false);
291+
using (var timeoutCts = new CancellationTokenSource(TimeSpan.FromSeconds(1)))
292+
{
293+
response = await requestContext.ServiceBundle.HttpManager.SendRequestAsync(
294+
GetValidatedEndpoint(requestContext.Logger, imdsEndpoint, queryParams),
295+
headers,
296+
body: null,
297+
method: HttpMethod.Get,
298+
logger: requestContext.Logger,
299+
doNotThrow: false,
300+
mtlsCertificate: null,
301+
validateServerCertificate: null,
302+
cancellationToken: timeoutCts.Token,
303+
retryPolicy: retryPolicy)
304+
.ConfigureAwait(false);
305+
}
303306
}
304307
catch (Exception ex)
305308
{

src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityClient.cs

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,8 @@ internal async Task<ManagedIdentitySource> GetManagedIdentitySourceAsync(
106106
bool noImdsV2 = false)
107107
{
108108
// First check env vars to avoid the probe if possible
109-
ManagedIdentitySource source = GetManagedIdentitySourceNoImdsV2(requestContext.Logger);
110-
#pragma warning disable CS0618 // Type or member is obsolete
111-
if (source != ManagedIdentitySource.DefaultToImds)
112-
#pragma warning restore CS0618 // Type or member is obsolete
109+
ManagedIdentitySource source = GetManagedIdentitySourceNoImds(requestContext.Logger);
110+
if (source != ManagedIdentitySource.None)
113111
{
114112
s_sourceName = source;
115113
return source;
@@ -144,10 +142,18 @@ internal async Task<ManagedIdentitySource> GetManagedIdentitySourceAsync(
144142
return s_sourceName;
145143
}
146144

147-
// Detect managed identity source based on the availability of environment variables.
148-
// The result of this method is not cached because reading environment variables is cheap.
149-
// This method is perf sensitive any changes should be benchmarked.
150-
internal static ManagedIdentitySource GetManagedIdentitySourceNoImdsV2(ILoggerAdapter logger = null)
145+
/// <summary>
146+
/// Detects the managed identity source based on the availability of environment variables.
147+
/// It does not probe IMDS, but it checks for all other sources.
148+
/// This method does not cache its result, as reading environment variables is inexpensive.
149+
/// It is performance sensitive; any changes should be benchmarked.
150+
/// </summary>
151+
/// <param name="logger">Optional logger for diagnostic output.</param>
152+
/// <returns>
153+
/// The detected <see cref="ManagedIdentitySource"/> based on environment variables.
154+
/// Returns <c>ManagedIdentitySource.None</c> if no environment-based source is detected.
155+
/// </returns>
156+
internal static ManagedIdentitySource GetManagedIdentitySourceNoImds(ILoggerAdapter logger = null)
151157
{
152158
string identityEndpoint = EnvironmentVariables.IdentityEndpoint;
153159
string identityHeader = EnvironmentVariables.IdentityHeader;
@@ -189,9 +195,7 @@ internal static ManagedIdentitySource GetManagedIdentitySourceNoImdsV2(ILoggerAd
189195
}
190196
else
191197
{
192-
#pragma warning disable CS0618 // Type or member is obsolete
193-
return ManagedIdentitySource.DefaultToImds;
194-
#pragma warning restore CS0618 // Type or member is obsolete
198+
return ManagedIdentitySource.None;
195199
}
196200
}
197201

src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentitySource.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public enum ManagedIdentitySource
4848
/// Indicates that the source is defaulted to IMDS since no environment variables are set.
4949
/// This is used to detect the managed identity source.
5050
/// </summary>
51-
[Obsolete("Use Imds instead. We are no longer defaulting to IMDS. An error will be thrown instead if no managed identity sources are available.")]
51+
[Obsolete("In use only to support the now obsolete GetManagedIdentitySource API. Will be removed in a future version. Use GetManagedIdentitySourceAsync instead.")]
5252
DefaultToImds,
5353

5454
/// <summary>

src/client/Microsoft.Identity.Client/ManagedIdentityApplication.cs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,16 @@ public async Task<ManagedIdentitySource> GetManagedIdentitySourceAsync()
7777
[Obsolete("Use GetManagedIdentitySourceAsync() instead. \"ManagedIdentityApplication mi = miBuilder.Build() as ManagedIdentityApplication;\"")]
7878
public static ManagedIdentitySource GetManagedIdentitySource()
7979
{
80-
return ManagedIdentityClient.GetManagedIdentitySourceNoImdsV2();
80+
var source = ManagedIdentityClient.GetManagedIdentitySourceNoImds();
81+
82+
return source == ManagedIdentitySource.None
83+
#pragma warning disable CS0618
84+
// ManagedIdentitySource.DefaultToImds is marked obsolete, but is intentionally used here as a sentinel value to support legacy detection logic.
85+
// This value signals that none of the environment-based managed identity sources were detected.
86+
? ManagedIdentitySource.DefaultToImds
87+
#pragma warning restore CS0618
88+
: source;
89+
8190
}
8291
}
8392
}

tests/Microsoft.Identity.Test.Common/Core/Mocks/MockHttpMessageHandler.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,14 @@ internal class MockHttpMessageHandler : HttpClientHandler
4141

4242
public X509Certificate2 ExpectedMtlsBindingCertificate { get; set; }
4343

44+
public Func<HttpRequestMessage, CancellationToken, Task<HttpResponseMessage>> HandlerFunc { get; set; }
45+
4446
protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
4547
{
48+
if (HandlerFunc != null)
49+
{
50+
return await HandlerFunc(request, cancellationToken).ConfigureAwait(false);
51+
}
4652

4753
ActualRequestMessage = request;
4854

tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/ImdsTests.cs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33

44
using System;
55
using System.Net;
6+
using System.Net.Http;
67
using System.Threading.Tasks;
78
using Microsoft.Identity.Client;
89
using Microsoft.Identity.Client.AppConfig;
910
using Microsoft.Identity.Client.ManagedIdentity;
11+
using Microsoft.Identity.Client.ManagedIdentity.V2;
1012
using Microsoft.Identity.Test.Common.Core.Helpers;
1113
using Microsoft.Identity.Test.Common.Core.Mocks;
1214
using Microsoft.Identity.Test.Unit.Helpers;
@@ -431,5 +433,44 @@ await mi.AcquireTokenForManagedIdentity(ManagedIdentityTests.Resource)
431433
Assert.AreEqual(Num504Errors, requestsMade);
432434
}
433435
}
436+
437+
[TestMethod]
438+
public async Task ProbeImdsEndpointAsync_TimesOutAfterOneSecond()
439+
{
440+
using (new EnvVariableContext())
441+
using (var httpManager = new MockHttpManager())
442+
{
443+
var miBuilder = ManagedIdentityApplicationBuilder.Create(ManagedIdentityId.SystemAssigned);
444+
445+
miBuilder
446+
.WithHttpManager(httpManager)
447+
.WithRetryPolicyFactory(_testRetryPolicyFactory);
448+
449+
var managedIdentityApp = miBuilder.Build();
450+
451+
httpManager.AddMockHandler(MockHelpers.MockImdsProbeFailure(ImdsVersion.V2));
452+
453+
var imdsV1Handler = new MockHttpMessageHandler
454+
{
455+
HandlerFunc = async (request, cancellationToken) =>
456+
{
457+
// IMDS (V1 and V2) probe has a timeout after 1 second
458+
await Task.Delay(TimeSpan.FromSeconds(2), cancellationToken).ConfigureAwait(false);
459+
return new HttpResponseMessage(HttpStatusCode.OK);
460+
}
461+
};
462+
httpManager.AddMockHandler(imdsV1Handler);
463+
464+
var miSource = await (managedIdentityApp as ManagedIdentityApplication).GetManagedIdentitySourceAsync().ConfigureAwait(false);
465+
Assert.AreEqual(ManagedIdentitySource.None, miSource); // Probe timed out, no source available
466+
467+
var ex = await Assert.ThrowsExceptionAsync<MsalServiceException>(async () =>
468+
await managedIdentityApp.AcquireTokenForManagedIdentity(ManagedIdentityTests.Resource)
469+
.ExecuteAsync().ConfigureAwait(false)
470+
).ConfigureAwait(false);
471+
472+
Assert.AreEqual(MsalError.ManagedIdentityAllSourcesUnavailable, ex.ErrorCode);
473+
}
474+
}
434475
}
435476
}

0 commit comments

Comments
 (0)