Skip to content
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

Merged
merged 40 commits into from Jan 28, 2024
Merged

Conversation

JoshLozensky
Copy link
Contributor

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

Copy link
Collaborator

@jennyf19 jennyf19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🕐

Copy link
Member

@bgavrilMS bgavrilMS left a 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.

Copy link
Collaborator

@jmprieur jmprieur left a 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!

Copy link
Contributor

@keegan-caruso keegan-caruso left a 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.

.gitignore Show resolved Hide resolved
Copy link
Collaborator

@jennyf19 jennyf19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit: Very nice work @JoshLozensky Looking forward to getting this out to customers very soon.

@jennyf19 jennyf19 added this to the 2.17.0 milestone Jan 27, 2024
@jmprieur jmprieur merged commit 8909553 into master Jan 28, 2024
4 checks passed
@jmprieur jmprieur deleted the lozensky/EnableManagedIdentity branch January 28, 2024 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Managed Identity support in IDownstreamWebApi / IDownstreamApi
6 participants