search

Azure / azure-sdk-for-js

This repository is for active development of the Azure SDK for JavaScript (NodeJS & Browser). For consumers of the SDK we recommend visiting our public developer docs at https://docs.microsoft.com/javascript/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-js.
MIT License
2.07k stars 1.19k forks source link

Replace keytar with @napi-rs/keyring in @azure/identity-cache-persistence #29288

Open altinokdarici opened 6 months ago

altinokdarici commented 6 months ago

@azure/identity-cache-persistence uses keytar. In Linux, keytar requires libsecret which is not part of WSL that comes with Windows and GitHub codespaces.

I realized that we can NOT take full advantage of @azure/identity-cache-persistence in those Linux env where libsecret is not installed. It's not easy/fair to people to install libsecret just for the sake of @azure/identity-cache-persistence

There is another npm package called @napi-rs/keyring - npm (npmjs.com) that does what keytar does, but no libsecret requirement. Although keytar is more popular, I think keyring is promising alternative that also solves the libsecret issue.

github-actions[bot] commented 6 months ago

@KarishmaGhiya @maorleger

github-actions[bot] commented 6 months ago

Thank you for your feedback. Tagging and routing to the team member best able to assist.

maorleger commented 5 months ago

Hey @altinokdarici - appreciate you reaching out to us about this and laying out the scenario. Yes, keytar requires libsecret and they document it that in their README

With that said, customers of the Azure SDK should not need to dig into our internal dependencies to understand what they need to install. We will update the README with more information about our dependencies and the system requirements to be clearer here.

Of course, your suggestion was not to improve the README but find an alternative to keytar that does not depend on libsecret

At the moment it's a matter of prioritization - we will need to investigate, vet, and implement the alternative. This is something we should probably do; however, just to set expectations it will take us some time to look into this closely. I don't have an ETA, but wanted to be transparent about where we're at.

For the next steps, I'll bring this back to the team for prioritization. We'll look at the package you suggested as well as other alternatives and will update this when we have progress to share.

Again, thanks for reaching out - let me know if there's anything else we can help with!

altinokdarici commented 5 months ago

@maorleger, Thanks for your response. Although I understand that you need to investigate and vet the suggested package, I'm happy to help you with the implementation. In fact, I already have a local branch that introduces this change. Please let me know if this helps you in anyway.

maorleger commented 5 months ago

Hey @altinokdarici - yes would love to see what the work to swap this out looks like.

Although now that I look closely (having not worked on identity-cache-persistence at the time) it looks like we use @azure/msal-node-extensions which is hosted here and owned by a different team.

  1. We use LibSecretPersistence in linux
  2. LibSecretPersistence uses keytar

So, all that to say, I think if @azure/msal-node-extensions was to use a different provider for their LibSecretPersistence implementation (or provide another implementation for us to use) we'd be able to swap things out on our end.

Does that line up with your understanding? If so, you could open an issue at https://github.com/AzureAD/microsoft-authentication-library-for-js and see if they have plans to move away from keytar?