-
Notifications
You must be signed in to change notification settings - Fork 218
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
Lozensky/enable managed identity #2650
Conversation
…zureAD/microsoft-identity-web into lozensky/EnableManagedIdentity
tests/Microsoft.Identity.Web.Test/TokenAcquisitionAuthorityTests.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Identity.Web.TokenAcquisition/DefaultAuthorizationHeaderProvider.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisitionOptions.cs
Outdated
Show resolved
Hide resolved
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.
🕐
src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.cs
Outdated
Show resolved
Hide resolved
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.
Please remove WithHttpClient modifier.
See my comment about ManagedIdentityOptions
- this would be nice to have for future proofing, but not strictly required.
Otherwise looks good! Great tests.
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.
LGTM.
Nice job, @JoshLozensky !
- I've the same question as Jenny for the TokenAcquisitionOptions. Let's do simple!
- Please add .zip to the .gitignore file: we really don't want to add zips of Mb to the repo, which we'll never look at!
src/Microsoft.Identity.Web.TokenAcquisition/DefaultAuthorizationHeaderProvider.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.ManagedIdentity.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.ManagedIdentity.cs
Outdated
Show resolved
Hide resolved
...ationTests/PlaywrightTraces/B2CWebAppCallsWebApiLocally_TodoAppFunctionsCorrectly_net6.0.zip
Outdated
Show resolved
Hide resolved
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.
zips are still in the change set.
src/Microsoft.Identity.Web.TokenAcquisition/DefaultAuthorizationHeaderProvider.cs
Show resolved
Hide resolved
src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.ManagedIdentity.cs
Show resolved
Hide resolved
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.
Very nice work @JoshLozensky Looking forward to getting this out to customers very soon.
Add support for managed identities
Summary of the changes (Less than 80 chars)
Added logic to default to managed identity credentials if available.
Description
In TokenAquisition.GetAuthenticationResultForAppAsync() added a check for managed identity configuration in the tokenAquisitionOptions parameter. If present, default to this authentication method and use system-assigned or user-assigned managed identity per the user's configuration.
Fixes #1775 #2551