Skip to content

Add Support for Azure TokenCredential Authentication #7

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

irwin08
Copy link

@irwin08 irwin08 commented May 30, 2022

My organization requires the use of identity-based authentication instead of a simple authorization key when working with cosmos. This is one possible implementation meeting these requirements.

One thing to note is that the signature of the AzureCosmosDb factory method has been altered to take a TokenCredential as well as an authorization key, with both having defaults of null. So, anyone using the default values for everything, and just the old required positional parameters should have no issues. However, if someone was using passing arguments positionally after authorizationKey, this would be a breaking change. However, the alternative is to pass in the TokenCredential argument at the very end of the method, which feels very unnatural.

@tghamm
Copy link

tghamm commented May 31, 2022

@irwin08 is this commit necessary if the changes referenced in #6 are merged? There they suggest providing your own client because of a Singleton policy, which seems like it would give you the control you want over construction of the client entirely. To be clear, I'm not the maintainer, I just did a lot of the work on 2.0. Your use case makes total sense, just wondering if their commit kills two birds with one stone?

@irwin08
Copy link
Author

irwin08 commented Jun 5, 2022

@tghamm Sorry for the delayed response. Yes, I think if the #6 is merged, that will be fine and handle my use case. So I am happy with that solution. I think the only real advantage of my solution is it's a tiny bit faster to use and get going for someone who doesn't want to get bogged down in the details of making your own client. (However this is marginal at best, it isn't that much more work.)

So yes I agree with you and support #6 over my PR unless there are objections to #6 for some reason.

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.

2 participants