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

Migrate the msal-node-extensions to use @napi-rs/keyring instead of keytar #7170

Open
kenotron opened this issue Jun 17, 2024 · 4 comments
Open
Labels
feature-unconfirmed msal-node Related to msal-node package msal-node-extensions Related to msal-node-extensions package Needs: Attention 👋 Awaiting response from the MSAL.js team public-client Issues regarding PublicClientApplications question Customer is asking for a clarification, use case or information.

Comments

@kenotron
Copy link

Core Library

MSAL Node (@azure/msal-node)

Wrapper Library

MSAL Node Extensions (@azure/msal-node-extensions)

Public or Confidential Client?

Public

Description

Currently, the msal-node-extensions imposes a dependency on keytar. This is a cross-platform library that abstracts keystore. On Windows, this extension already handles credential persistence via the in-built Data Protection. On Linux, the keytar library depends on libsecret, which is then dependent on a dbus... this is not available in headless workloads like WSL2. A better approach is to utilize @napi-rs/keyring which is a thin napi-rs wrapper on top of the keyring-rs crate written in Rust. There, the secret store is implemented natively in Rust as well. https://www.npmjs.com/package/@napi-rs/keyring uses https://github.com/hwchen/keyring-rs uses https://github.com/hwchen/secret-service-rs ... this means that headless workloads can also use keystores with this implementation.

In addition, the author of the @napi-rs/keyring was recently awarded OSS fund from MSFT which means that the author is a credible source of quality code. I highly recommend us to move away from keytar.

Source

Internal (Microsoft)

@kenotron kenotron added feature-unconfirmed question Customer is asking for a clarification, use case or information. labels Jun 17, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Attention 👋 Awaiting response from the MSAL.js team label Jun 17, 2024
@github-actions github-actions bot added msal-node Related to msal-node package msal-node-extensions Related to msal-node-extensions package public-client Issues regarding PublicClientApplications labels Jun 17, 2024
@sameerag
Copy link
Member

cc @bgavrilMS to consider this for msal-node

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs: Author Feedback Awaiting response from issue author and removed Needs: Attention 👋 Awaiting response from the MSAL.js team labels Jun 18, 2024
@TylerLeonhardt
Copy link
Contributor

TylerLeonhardt commented Jun 20, 2024

I think the other big motivating factor is that keytar is deprecated and unmaintained since 2022:
http://github.com/atom/node-keytar

MSAL should use a maintained package to store secrets. Especially when those secrets are auth tokens.

@kenotron
Copy link
Author

Agreed, @bgavrilMS - Please make sure that this is prioritized, as there is a good story from Windows to use wsl2 for node.js & frontend devs

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs: Attention 👋 Awaiting response from the MSAL.js team and removed Needs: Author Feedback Awaiting response from issue author labels Jun 20, 2024
@rzhao271
Copy link

Because @azure/msal-node-extensions uses keytar, which has been unmaintained since 2022, we are now getting a Component Governance alert for the GitHub Copilot extension for VS Code due to keytar using string_decoder. Because keytar handles secrets and is not a devDependency, the following alert seems to be relevant: "[The string_decoder package] does not zero out memory before returning it, so it risks information disclosure".

I notice that the latest @azure/msal-node does not contain string_decoder, but the latest @azure/msal-node-extensions still does.
I would like @azure/msal-node-extensions to also move onto @napi-rs/keyring so that it along with all downstream projects can become more current and secure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-unconfirmed msal-node Related to msal-node package msal-node-extensions Related to msal-node-extensions package Needs: Attention 👋 Awaiting response from the MSAL.js team public-client Issues regarding PublicClientApplications question Customer is asking for a clarification, use case or information.
Projects
None yet
Development

No branches or pull requests

4 participants