-
Notifications
You must be signed in to change notification settings - Fork 340
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.
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
[Feature Request] Service Fabric MI Auth doesn't validate server cert. #4462
Comments
We have agreed with Azure SDK to not extend the HttpClient factory for this. It is acceptable for MSAL use it's own HttpClient that doesn't go through the extensibility pipeline. Retry policies etc. are still required. |
@neha-bhargava - it's acceptable for this call to MSI to not use IHttpClientFactory / an HttpClient created externally. But try to find a way so as to keep the code path testable. If you need to make changes to HttpManager, please see @gladjohn 's PR on SLC first, as it has a sweeping refactor of that. Maybe it's worth pulling that refactor out. |
Is this still blocked? |
MSAL client type
Managed identity
Problem Statement
Currently, MSAL .NET reads the Thumbprint from env but doesn't use it to validate the cert from the server in the transport layer.
This logic needs to be implemented.
Proposed solution
.NET Identity SDK does this, this logic can be ported from here:
https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/identity/Azure.Identity/src/ServiceFabricManagedIdentitySource.cs#L55
Other languages
java: https://github.com/Azure/azure-sdk-for-java/blob/main/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java#L1032
Py: (not implemented) https://github.com/Azure/azure-sdk-for-python/blob/main/sdk/identity/azure-identity/azure/identity/_credentials/service_fabric.py#L38
JS: (not implemented) https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/identity/identity/src/credentials/managedIdentityCredential/fabricMsi.ts#L128
The text was updated successfully, but these errors were encountered: