Skip to content

Commit

Permalink
Let DAC chain continue for 400 responses with 'Identity not found' (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
christothes authored Oct 21, 2024
1 parent 3fadc42 commit ec5fafd
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 5 deletions.
7 changes: 6 additions & 1 deletion sdk/identity/Azure.Identity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,17 @@
### Breaking Changes

### Bugs Fixed
- Fixed an issue that prevented ManagedIdentityCredential from attempting to detect if Workload Identity is enabled in the current environment. [#46653](https://github.com/Azure/azure-sdk-for-net/issues/46653)
- Fixed an issue that prevented `ManagedIdentityCredential` from attempting to detect if Workload Identity is enabled in the current environment. [#46653](https://github.com/Azure/azure-sdk-for-net/issues/46653)
- Fixed an issue that prevented `DefaultAzureCredential` from progressing past `ManagedIdentityCredential` in some scenarios where the identity was not available. [#46709](https://github.com/Azure/azure-sdk-for-net/issues/46709)

### Other Changes

## 1.13.0 (2024-10-14)

### Breaking Changes
- Previously, if a clientID or ResourceID was specified for Cloud Shell managed identity, which is not supported, the clientID or resourceID would be silently ignored. Now, an exception will be thrown if a clientID or resourceID is specified for Cloud Shell managed identity.
- Previously, if a clientID or ResourceID was specified for Service Fabric managed identity, which is not supported, the clientID or resourceID would be silently ignored. Now, an exception will be thrown if a clientID or resourceID is specified for Service Fabric managed identity.

### Features Added
- `ManagedIdentityCredential` now supports specifying a user-assigned managed identity by object ID.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ protected override async ValueTask<AccessToken> HandleResponseAsync(bool async,
// handle error status codes indicating managed identity is not available
string baseMessage = response.Status switch
{
400 when IsProbeRequest(message) => throw new ProbeRequestResponseException(),
400 when IsRetriableProbeRequest(message) => throw new ProbeRequestResponseException(),
400 => IdentityUnavailableError,
502 => GatewayError,
504 => GatewayError,
Expand All @@ -169,10 +169,11 @@ protected override async ValueTask<AccessToken> HandleResponseAsync(bool async,
return token;
}

public static bool IsProbeRequest(HttpMessage message)
public static bool IsRetriableProbeRequest(HttpMessage message)
=> message.Request.Uri.Host == s_imdsEndpoint.Host &&
message.Request.Uri.Path == s_imdsEndpoint.AbsolutePath &&
!message.Request.Headers.TryGetValue(metadataHeaderName, out _);
!message.Request.Headers.TryGetValue(metadataHeaderName, out _) &&
(message.Response.Content?.ToString().IndexOf("Identity not found", StringComparison.InvariantCulture) < 0);

private class ImdsRequestFailedDetailsParser : RequestFailedDetailsParser
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ public void VerifyImdsRequestFailureForDockerDesktopThrowsCUE(string error)
{
using var environment = new TestEnvVar(new() { { "MSI_ENDPOINT", null }, { "MSI_SECRET", null }, { "IDENTITY_ENDPOINT", null }, { "IDENTITY_HEADER", null }, { "AZURE_POD_IDENTITY_AUTHORITY_HOST", null } });

var expectedMessage = $"connecting to 169.254.169.254:80: connecting to 169.254.169.254:80: dial tcp 169.254.169.254:80: connectex: A socket operation was attempted to an unreachable {error}.";
var expectedMessage = error;
var response = CreateInvalidJsonResponse(403, expectedMessage);
var mockTransport = new MockTransport(response);
var options = new TokenCredentialOptions() { Transport = mockTransport, IsChainedCredential = true };
Expand Down Expand Up @@ -422,6 +422,31 @@ public void VerifyImdsRequestFailureWithInvalidJsonPopulatesExceptionMessage()
Assert.That(ex.Message, Does.Contain(expectedMessage));
}

[NonParallelizable]
[Test]
[TestCase("""{"error":"invalid_request","error_description":"Identity not found"}""")]
[TestCase(null)]
public void VerifyImdsRequestFailureWithValidJsonIdentityNotFoundErrorThrowsCUE(string content)
{
using var environment = new TestEnvVar(new() { { "MSI_ENDPOINT", null }, { "MSI_SECRET", null }, { "IDENTITY_ENDPOINT", null }, { "IDENTITY_HEADER", null }, { "AZURE_POD_IDENTITY_AUTHORITY_HOST", null } });

var response = CreateResponse(400, content);
var mockTransport = new MockTransport(req => response);
var options = new TokenCredentialOptions() { Transport = mockTransport, IsChainedCredential = true };
var pipeline = CredentialPipeline.GetInstance(options);

ManagedIdentityCredential credential = InstrumentClient(new ManagedIdentityCredential("mock-client-id", pipeline, options));
if (content != null)
{
var ex = Assert.ThrowsAsync<CredentialUnavailableException>(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)));
Assert.That(ex.Message, Does.Contain(ImdsManagedIdentityProbeSource.IdentityUnavailableError));
}
else
{
var ex = Assert.ThrowsAsync<AuthenticationFailedException>(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)));
}
}

[NonParallelizable]
[Test]
[TestCase(502, ImdsManagedIdentitySource.GatewayError)]
Expand Down Expand Up @@ -1128,5 +1153,15 @@ private static MockResponse CreateInvalidJsonResponse(int status, string message
response.SetContent(message);
return response;
}

private static MockResponse CreateResponse(int status, string message)
{
var response = new MockResponse(status);
if (message != null)
{
response.SetContent(message);
}
return response;
}
}
}

0 comments on commit ec5fafd

Please sign in to comment.