botbuilder-js
botbuilder-js copied to clipboard
microsoft
Migrate to MSAL from adal-node
On inspection of https://github.com/AzureAD/microsoft-authentication-library-for-js/pull/2023, there is now support for using client certificates in MSAL, which means we can continue moving away from adal-node.
@Zerryth, please reach out to either @carlosscastro or myself for any questions.
Tentatively placing in R11 milestone.
Marking as P2 since we're not blocking anyone by not performing this migration in R11.
@Zerryth, let's chat offline about the plan for this work.
Hello mate, I want to find out if there is any update on this open issue? BR
No updates yet, this work is scheduled for the next release of the SDK which means it should be picked up relatively soon, perhaps in a few weeks.
No updates yet, this work is scheduled for the next release of the SDK which means it should be picked up relatively soon, perhaps in a few weeks.
Thank you so much for the response. 🤞
From @joshgummersall 's comment in PR#3247
@Zerryth are you still planning on resolving #2782 as well?
Short answer is yes. 😅
I'll go ahead and start working on it, since in the MSAL migration FAQs it says that ADAL is deprecating June 30th, 2022, and that the recommended authentication library to use with Microsoft identity platform is MSAL. However I was hesitant at first (which is why I made a separate PR in 3247), because:
- Looking at our SDK, it looks like we use:
-
client credentialsgrant type (regular channel <--> bot communication) - and
authorization codegrant type (for scenarios when we have a user sign in usingOAuthPrompt)
-
- Speaking to the MSAL.js team, it looks like they released the
client credentialsflow, however it's available in theirmsal-nodepackage, which is still in preview (other packages in MSAL.js have GA'd, just not this one)- In the past Chris Mullins had rejected merging in other preview features, like QnAMaker multi-turn prompt, when it was still in preview, even though QnAMaker was something that we already had other parts of integrated in our SDK
- I spoke to Steven G. and says I should try implementing this migration regardless, given the recommendation on migration in their docs
If anyone else feels strongly otherwise about folding in the preview MSAL package, LMK and we can hold off--seems like we do have runway time until mid-2022 anyways
I didn't realize it wasn't a GA package yet - seems fine to hold off for now then.
Is there any active work on this issue? adal-node has a dependency on xmldom which has security vulnerability CVE-2021-32796 in versions <= 0.6.0. Due to inactivity by the original maintainer, the remaining xmldom team cannot publish a patched version and have changed the dependency name to @xmldom/xmldom. See https://github.com/xmldom/xmldom/issues/271 Therefore, with azal-node in archive state I am checking if the move to msal has started.
Unfortunately, MSAL only supports a limiter number of Typescript versions. We have aimed to support a wide matrix of Typescript versions. Moving to MSAL would effectively cut that support matrix in half at the moment.
Luckily, I have put together a sample bot with MSAL authentication enabled: https://github.com/joshgummersall/msal-bot
Take a look specifically at msalAppCredentials.ts and msalServiceClientCredentialFactory.ts.
This sample does use the new CloudAdapter, so it may not be a trivial change. However, it should be possible to do.
One other note: this bot also patches JWT validation code to support HTTP proxy. It's probably best to look at the initial commit and ignore the network client/ProxyHttpClient pieces.
@Zerryth, @joshgummersall - In the meantime it looks like adal-node has a newer version out that resolves the xmldom vulnerability: https://github.com/AzureAD/microsoft-authentication-library-for-js/issues/4011
New version is 0.2.3
@aflinchb https://github.com/microsoft/botbuilder-js/pull/3929 handled that! It'll be published through official channels in the coming weeks.
Hi @mrivera-ms, there is a draft PR with this migration, but it was put on hold because msal-node package is not compatible with old versions of Typescript. It supports versions over 3.8 while the SDK checks against versions from 3.3 to 4.1.
Should we update the list of supported TypeScript versions so this change can take place?
Hello @ceciliaavila @mrivera-ms I am wondering if there have been any updates to this as adal has been deprecated for a while and this might start posing security vulnerabilities going forward.
