Skip to content

Commit 383f1e9

Browse files
authored
Don't parse invalid json from failed MI responses (Azure#37194)
1 parent 4268d60 commit 383f1e9

File tree

4 files changed

+40
-5
lines changed

4 files changed

+40
-5
lines changed

sdk/identity/Azure.Identity/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
### Bugs Fixed
1010
- Fixed an issue with `TokenCachePersistenceOptions` where credentials in the same process would share the same cache, even if they had different configured names.
1111
- ManagedIdentityCredential now ignores empty ClientId values. [#37100](https://github.com/Azure/azure-sdk-for-net/issues/37100)
12+
- ManagedIdentityCredential will no longer attempt to parse invalid json payloads on responses from the managed identity endpoint.
1213

1314
### Other Changes
1415
- All developer credentials in the `DefaultAzureCredential` credential chain will fall through to the next credential in the chain on any failure. Previously, some exceptions would throw `AuthenticationFailedException`, which stops further progress in the chain.

sdk/identity/Azure.Identity/src/ManagedIdentityRequestFailedDetailsParser.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4+
using System;
45
using System.Collections.Generic;
56
using System.Threading;
67
using Azure.Core;
@@ -13,8 +14,17 @@ internal class ManagedIdentityRequestFailedDetailsParser : RequestFailedDetailsP
1314
public override bool TryParse(Response response, out ResponseError error, out IDictionary<string, string> data)
1415
{
1516
data = new Dictionary<string, string>();
17+
error = null;
1618
try
1719
{
20+
// The response content is buffered at this point.
21+
string content = response.Content.ToString();
22+
23+
// Optimistic check for JSON object we expect
24+
if (content == null || !content.StartsWith("{", StringComparison.OrdinalIgnoreCase))
25+
{
26+
return false;
27+
}
1828
var message = ManagedIdentitySource.GetMessageFromResponse(response, false, CancellationToken.None).EnsureCompleted();
1929
error = new ResponseError(null, message);
2030
return true;

sdk/identity/Azure.Identity/src/ManagedIdentitySource.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,12 @@ protected virtual async ValueTask<AccessToken> HandleResponseAsync(
4949
Exception exception = null;
5050
try
5151
{
52-
using JsonDocument json = async
53-
? await JsonDocument.ParseAsync(response.ContentStream, default, cancellationToken).ConfigureAwait(false)
54-
: JsonDocument.Parse(response.ContentStream);
5552
if (response.Status == 200)
5653
{
54+
using JsonDocument json = async
55+
? await JsonDocument.ParseAsync(response.ContentStream, default, cancellationToken).ConfigureAwait(false)
56+
: JsonDocument.Parse(response.ContentStream);
57+
5758
return GetTokenFromResponse(json.RootElement);
5859
}
5960
}

sdk/identity/Azure.Identity/tests/ManagedIdentityCredentialTests.cs

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -767,7 +767,7 @@ public async Task VerifyClientAuthenticateThrows()
767767
}
768768

769769
[Test]
770-
public async Task VerifyClientAuthenticateReturnsInvalidJson([Values(200, 404, 403)] int status)
770+
public async Task VerifyClientAuthenticateReturnsInvalidJsonOnSuccess([Values(200)] int status)
771771
{
772772
using var environment = new TestEnvVar(
773773
new()
@@ -787,7 +787,30 @@ public async Task VerifyClientAuthenticateReturnsInvalidJson([Values(200, 404, 4
787787

788788
var ex = Assert.ThrowsAsync<CredentialUnavailableException>(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)));
789789
Assert.IsInstanceOf(typeof(System.Text.Json.JsonException), ex.InnerException);
790-
Assert.That(ex.Message, Does.Contain(ManagedIdentitySource.UnexpectedResponse));
790+
await Task.CompletedTask;
791+
}
792+
793+
[Test]
794+
public async Task VerifyClientAuthenticateReturnsInvalidJsonOnFailure([Values(404, 403, 429)] int status)
795+
{
796+
using var logger = AzureEventSourceListener.CreateConsoleLogger();
797+
using var environment = new TestEnvVar(
798+
new()
799+
{
800+
{ "MSI_ENDPOINT", null },
801+
{ "MSI_SECRET", null },
802+
{ "IDENTITY_ENDPOINT", null },
803+
{ "IDENTITY_HEADER", null },
804+
{ "AZURE_POD_IDENTITY_AUTHORITY_HOST", null }
805+
});
806+
var mockTransport = new MockTransport(request => CreateInvalidJsonResponse(status));
807+
var options = new TokenCredentialOptions() { Transport = mockTransport };
808+
options.Retry.MaxDelay = TimeSpan.Zero;
809+
var pipeline = CredentialPipeline.GetInstance(options);
810+
811+
ManagedIdentityCredential credential = InstrumentClient(new ManagedIdentityCredential("mock-client-id", pipeline));
812+
813+
var ex = Assert.ThrowsAsync<AuthenticationFailedException>(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)));
791814
await Task.CompletedTask;
792815
}
793816

0 commit comments

Comments
 (0)