-
Notifications
You must be signed in to change notification settings - Fork 385
Bearer Requests should Fallback to IMDS in Preview #5562
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1462,5 +1462,42 @@ private static void AssertCertSubjectCnDc(X509Certificate2 cert, string expected | |
| } | ||
|
|
||
| #endregion | ||
|
|
||
| #region Fallback Behavior Tests | ||
| // Verifies non-mTLS request after IMDSv2 detection falls back per-request to IMDSv1 (Bearer), | ||
| [DataTestMethod] | ||
| [DataRow(UserAssignedIdentityId.None, null)] | ||
| [DataRow(UserAssignedIdentityId.ClientId, TestConstants.ClientId)] | ||
|
Comment on lines
+1468
to
+1470
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be consistent with all other test, can you also add cases for ResourceId and ObjectId? |
||
| public async Task NonMtlsRequest_FallbackToImdsV1( | ||
| UserAssignedIdentityId idKind, | ||
| string idValue) | ||
| { | ||
| using (new EnvVariableContext()) | ||
| using (var httpManager = new MockHttpManager()) | ||
| { | ||
| ManagedIdentityClient.ResetSourceForTest(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't we have something that runs before each test? Shouldn't this be put there? |
||
| SetEnvironmentVariables(ManagedIdentitySource.ImdsV2, TestConstants.ImdsEndpoint); | ||
|
|
||
| var mi = await CreateManagedIdentityAsync(httpManager, idKind, idValue, managedIdentityKeyType: ManagedIdentityKeyType.KeyGuard).ConfigureAwait(false); | ||
|
|
||
| // Fallback token (Bearer) mock | ||
| httpManager.AddManagedIdentityMockHandler( | ||
| ManagedIdentityTests.ImdsEndpoint, | ||
| ManagedIdentityTests.Resource, | ||
| MockHelpers.GetMsiSuccessfulResponse(), | ||
| ManagedIdentitySource.Imds, | ||
| userAssignedIdentityId: idKind, | ||
| userAssignedId: idValue); | ||
|
|
||
| var token = await mi.AcquireTokenForManagedIdentity(ManagedIdentityTests.Resource) | ||
| // No .WithMtlsProofOfPossession() => triggers fallback | ||
| .ExecuteAsync().ConfigureAwait(false); | ||
|
|
||
| Assert.AreEqual(Bearer, token.TokenType); | ||
| Assert.IsNull(token.BindingCertificate, "Bearer token should not have binding certificate."); | ||
| Assert.AreEqual(TokenSource.IdentityProvider, token.AuthenticationResultMetadata.TokenSource); | ||
| } | ||
| } | ||
| #endregion | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be elif? You want both if statements to run?