ImdsV2: Acquire Entra Token Over mTLS #5431
Conversation
src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsV2ManagedIdentitySource.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsV2ManagedIdentitySource.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/ImdsV2ManagedIdentitySource.cs
Outdated
Show resolved
Hide resolved
…quire_entra_token
Robbie-Microsoft
changed the title
src/client/Microsoft.Identity.Client/ManagedIdentity/V2/ImdsV2ManagedIdentitySource.cs
Outdated
Show resolved
Hide resolved
| /// <exception cref="ArgumentNullException">Thrown when certificatePem or privateKey is null</exception> | ||
| /// <exception cref="ArgumentException">Thrown when certificatePem is not a valid PEM certificate</exception> | ||
| /// <exception cref="FormatException">Thrown when the certificate cannot be parsed</exception> | ||
| internal X509Certificate2 AttachPrivateKeyToCert(string certificatePem, RSA privateKey) |
There was a problem hiding this comment.
You could move all this X509 related code to the existing CommonCryptographyManager
src/client/Microsoft.Identity.Client/ManagedIdentity/V2/ImdsV2ManagedIdentitySource.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/V2/ImdsV2ManagedIdentitySource.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/V2/ImdsV2ManagedIdentitySource.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ManagedIdentity/V2/ImdsV2ManagedIdentitySource.cs
Outdated
Show resolved
Hide resolved
…quire_entra_token
Robbie-Microsoft
merged commit ddc2117
into
rginsburg/msiv2_feature_branch
| .ExecuteAsync().ConfigureAwait(false); | ||
|
|
||
| Assert.IsNotNull(result); | ||
| Assert.IsNotNull(result.AccessToken); |
There was a problem hiding this comment.
overall looks good. can we also assert common headers we send to ESTS. which will contain correlation id MSAL version etc.
you can see the common headers being set here -
and here are some existing tests -
There was a problem hiding this comment.
The example you pointed me to in the tests is not clear to me. Can you please show me exactly how you would check that the headers from MsalIdHelper.GetMsalIdParameters exist in the request in the unit test?
There was a problem hiding this comment.
one way to check would be to run this test, (manual inspection)
Sni_Gets_Pop_Token_Successfully_TestAsync
And get the time stamp and correlation id from the logs
Then inspect MSAL related properties in LogsMiner.
And then you can debug that test and see where those headers are set.
There was a problem hiding this comment.
Can you please show me exactly how you would check that the headers
For unit test, you can check the headers in the outgoing request. for e.g take a look at CertificateOverrideAsync test. it uses MockHttpMessageHandler to check for ActualRequestHeaders
There was a problem hiding this comment.
This is still very unclear to me
There was a problem hiding this comment.
I've found another way to check the headers. My PR is here: #5459
| .ExecuteAsync().ConfigureAwait(false); | ||
|
|
||
| Assert.IsNotNull(result); | ||
| Assert.IsNotNull(result.AccessToken); |
There was a problem hiding this comment.
can we also ensure the mtls cert is being returned in the result
Finishing up the remainder of the flow for ImdsV2... this PR builds the request to acquire the entra token over mTLS.