Migrate to MSAL

[TylerLeonhardt]

AAD Admins have the power to enforce logins of a certain kind that make our current OAuth flow insufficient for users to log in (it blocks it) and instead they have to log in using something like MSAL.js or msal-node.

Because of this, we should move to MSAL... but this is a challenge because:

• We have code that runs in node and on the web (so we need some combo of MSAL.js and msal-node without bloating VS Code too much)
• We have a model of the Microsoft auth extension running in one place (be it, in a local extension host or an extension host on the remote attached) and the fact that the token is minted here vs there should not affect the user
• On the web, we proxy the code exchanging and delegate that to vscode.dev's server... due to CORS issues with identity with how the VS Code app is configured. It's unclear what app configuration would need to be done to allow MSAL to do what it needs to do

[TylerLeonhardt]

I attempted to implement this but ran into a major problem: MSAL-node doesn't support a shared cache across PCAs well.

This would be required if you have multiple VS Code windows open... You sign out of an account in window A, and you expect window B to reflect that signed out account.

I have opened 3 issues internally that are as followed:

Deserializing in an ICachePlugin keeps around old state

In the beforeCacheAccess we are told to basically:

• read the cache from the persenstent store
• deserialize it

In other words:

  async beforeCacheAccess(tokenCacheContext: TokenCacheContext): Promise<void> {
    const data = await this._secretStorage.get();
    if (data) {
      tokenCacheContext.tokenCache.deserialize(data);
    }
  }

My initial assumption was that the call to deserialize replaces the cached in-memory state. In other words, whatever you pass in to deserialize will be the new state.

But I was wrong.

Here's what runs:

https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-node/src/cache/NodeStorage.ts#L108-L123

You'll notice that cache is created by combining the previous values of the cache with the new values of the cache.

This is not the behavior I'm looking for. I want data to be the source of truth... in other words I want deserialize(data) to completely replace the in-memory cache...

The current behavior makes it very hard to have 2 processes use the same store because if I remove an account from one process, I can't just read the data in and replace the in-memory cache.

Which leads me to issue 2:

Why is clearCache floating a promise? This makes it unreliable to clear the cache

While attempting to implement Msal-node in VS Code, I noticed this code:

https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-node/src/client/ClientApplication.ts#L615

is floating a promise when the code:

https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-node/src/cache/NodeStorage.ts#L493-L504

isn't async.

This makes it quite challenging to clear the cache in a reliable way... which means I can't even solve issue 1 with a workaround.

Lastly, getting to this point took a while... because a lot of stuff happens when you simply call getAllAccount...

All calls to getAllAccounts are resulting in a cache write

A recent change to support multi-tenants resulted in getAllAccounts potentially writing back to the cache after reading. msal-node is now setting a flag statically that indicates cache has changed on getAllAccounts which results in a cache write every time it's called. This potentially results in unnecessary operations and/or loops if an application is reacting to cache changes.

At this point, I've been asked to wait and see what the response from the MSAL-node team is. Here's hoping we can get through this.

[lippertmarkus]

@TylerLeonhardt is the MSAL authentication for vscode.dev planned for September as well?

[TylerLeonhardt]

@lippertmarkus later today insiders.vscode.dev should be using MSAL. Assuming there are no major blockers, vscode.dev will get this as well. Please give insiders.vscode.dev a go after today.

[lippertmarkus]

@TylerLeonhardt great, thanks! What do I need to setup in my app registration? Only have https://insiders.vscode.dev/redirect as an SPA redirectUri?

[TylerLeonhardt]

No sadly you need new ones...

These in the SPA configuration:

• https://insiders.vscode.dev/msal
• https://vscode.dev/msal

[lippertmarkus]

just tried it out and can confirm that it works fine, thanks!

[TylerLeonhardt]

@lippertmarkus the build of insiders.vscode.dev that uses MSAL shouldn't be released yet. You'll see this if you go through the auth flow... the redirectUri will still say /redirect but it should say /msal to know you're using MSAL.

[lippertmarkus]

I enabled "Use MSAL" in the settings and then the CORS error I got before was gone but I'll just try again later

[lippertmarkus]

Today it also works without enabling "Use MSAL"

[TylerLeonhardt]

Summary of Status

vscode.dev

Current status

As of 9/9/24, we have enabled using MSAL.js for all Microsoft auth calls in insiders.vscode.dev.

Next steps

• [x] https://github.com/microsoft/vscode/issues/219871
• [x] Monitor for issues that arise
• [x] Early Oct: Roll out to Stable vscode.dev
• [x] Likely Oct: Restore local vscode.dev development using MSAL for auth
• [ ] Likely Nov: Delete old non-MSAL auth code

Open issues

None, but to vent a bit we've had to workaround several MSAL.js bugs:

• https://github.com/AzureAD/microsoft-authentication-library-for-js/issues/6269
• https://github.com/AzureAD/microsoft-authentication-library-for-js/issues/6829

VS Code Desktop

Current status

As of 9/11/24, we have a fairly stable implementation of Microsoft Authentication using MSAL-node (vanilla, no broker). In order to use it, you must apply this VS Code setting: microsoft.useMsal.

We did this so that we had a solution that used MSAL-node on all platforms. In other words, this will be the solution when there is no broker available.

Next steps

• [x] https://github.com/microsoft/vscode/issues/229415
• [x] Early next week: we will roll this out to 50% in Insiders (again, now that some bugs are squashed)
• [ ] Likely Nov: https://github.com/microsoft/vscode/issues/229431

Open issues

None, but to vent a bit we've had to workaround several MSAL-node bugs:

• (internally tracked) All calls to getAllAccounts are resulting in a cache write
• (internally tracked) Why is clearCache floating a promise? This makes it unreliable to clear the cache
• (internally tracked) Deserializing in an ICachePlugin keeps around old state
• (internally tracked) Requesting only OIDC scopes after requesting a non-Graph resource returns a token for that non-Graph resource instead which causes cache miss for the OIDC call every time (infinite loop caused)

VS Code CLI

code serve-web

Current status

This uses the same code as VS Code Desktop. See that for status.

code tunnel

Current status

Blocked until a solution for Rust code is available.

[TylerLeonhardt]

Summary of Status

vscode.dev ✅

Current status

As of 9/9/24, we have enabled using MSAL.js for all Microsoft auth calls in vscode.dev.

Next steps

• [x] Nov: Delete old non-MSAL auth code
• [x] Nov: Remove workaround that was put in place for https://github.com/AzureAD/microsoft-authentication-library-for-js/issues/6269

Open issues

We worked around https://github.com/AzureAD/microsoft-authentication-library-for-js/issues/6829 but my suspicion is that we will need browsers to handle this better.

VS Code Desktop 🏃

Current status

As of 11/27/24, we have a fairly stable implementation of Microsoft Authentication using MSAL-node that also uses the broker if it's available.

This means that we currently support all security related measures that we can:

• broker flow on Windows
• using a supported auth library on all platforms

Next steps

• [x] Nov: Make MSAL the default in Insiders
• [ ] Early Dec: Make MSAL the default in Stable (this is simply pending a VS Code release, the code is in place)
• [ ] TBD: Broker flow on macOS & Linux (requires the Identity team to support this in MSAL node)

Open issues

None, but to vent a bit we've had to workaround several MSAL-node bugs:

• (internally tracked) Deserializing in an ICachePlugin keeps around old state
• (internally tracked) Requesting only OIDC scopes after requesting a non-Graph resource returns a token for that non-Graph resource instead which causes cache miss for the OIDC call every time (infinite loop caused)

VS Code CLI

code serve-web

Current status

This uses the same code as VS Code Desktop. See that for status.

code tunnel ✋

Current status

Blocked until a solution for Rust code is available.

[TylerLeonhardt]

moving to backlog as we are blocked on all remaining work

[TylerLeonhardt]

Summary of Status

VS Code Desktop 🏃

Current status

Adoption of the broker flow on Windows is done so from now on, you can assume that that is included in the Windows implementation.

We had to rollback MSAL as we hit this issue: https://github.com/microsoft/vscode/issues/229456 This has pushed back making MSAL the default... the timeline below is up-to-date.

You can enable MSAL manually with:

"microsoft-authentication.implementation": "msal"

Next steps

• [x] Mid Jan: Make MSAL the default in Insiders
• [ ] Early Feb: Stable ships with MSAL default
• [ ] Early March: Remove all 'classic' flow code
• [ ] TBD: Broker flow on macOS & Linux (requires the Identity team to support this in MSAL node)

Open issues

• https://github.com/microsoft/vscode/issues/229456

Not a blocker, but we have one issue that we still have a workaround for:

• (internally tracked) Requesting only OIDC scopes after requesting a non-Graph resource returns a token for that non-Graph resource instead which causes cache miss for the OIDC call every time (infinite loop caused)

VS Code CLI

code serve-web

Current status

This uses the same code as VS Code Desktop. See that for status.

code tunnel ✋

Current status

Blocked until a solution for Rust code is available.

vscode.dev ✅

See last update for status.

[TylerLeonhardt]

Summary of Status

VS Code Desktop 🏃

Current status

Adoption of the broker flow on Windows is done so from now on, you can assume that that is included in the Windows implementation.

The previous blocker, https://github.com/microsoft/vscode/issues/229456, has been fixed. We can now proceed with making it the default.

You no longer need the setting:

"microsoft-authentication.implementation": "msal"

msal is the default. However, it was kept in, for now, so users can rollback to classic should they need to.

Next steps

• [x] Mid Jan: Make MSAL the default in Insiders
• [x] Early Feb: Stable ships with MSAL default
• [ ] Early March: Remove all 'classic' flow code
• [ ] TBD: Broker flow on macOS & Linux (requires the Identity team to support this in MSAL node)

Open issues

Not a blocker, but we have one issue that we still have a workaround for:

• (internally tracked) Requesting only OIDC scopes after requesting a non-Graph resource returns a token for that non-Graph resource instead which causes cache miss for the OIDC call every time (infinite loop caused)

VS Code CLI

code serve-web

Current status

This uses the same code as VS Code Desktop. See that for status.

code tunnel ✋

Current status

Blocked until a solution for Rust code is available.

vscode.dev ✅

See last update for status.

[TylerLeonhardt]

Unfortunately, due to a whole class of issues caused by the native broker https://github.com/microsoft/vscode/issues?q=is%3Aopen+is%3Aissue+label%3Amicrosoft-authentication+label%3Abug+label%3Aupstream

I just don't feel comfortable removing the classic implementation yet... so it's postponed for now.

[TylerLeonhardt]

Summary of Status

VS Code Desktop 🏃

Current status

Microsoft Authentication using MSAL is currently on by-default, using MSAL Node. This will go through the broker in the following situations:

• On Windows x64
• On macOS ARM, if the machine is intuned and optted in to brokering

The platforms that do not go through the broker:

• Windows ARM (coming next quarter)
• macOS x64 (coming next quarter)
• Linux (all archs) (coming soon ™️)

We are awaiting MSAL to enable these platforms.

Next steps

• [ ] ~Remove classic implementation~ See open issues
• [ ] TBD: Broker flows on remaining platforms (requires the Identity team to support this in MSAL node)

Open issues

There are numerous bugs that have come up due to broker adoption. I have opened internal issues on these but they have not been addressed yet. Because of this, I cannot in good faith remove the classic implementation. However, I have added a msal-no-broker option to the microsoft-authentication.implementation setting so that if we need to get rid of classic we can at least use an MSAL Node implementation without the broker.

Not a blocker, but we have one issue that we still have a workaround for:

• (internally tracked) Requesting only OIDC scopes after requesting a non-Graph resource returns a token for that non-Graph resource instead which causes cache miss for the OIDC call every time (infinite loop caused)

VS Code CLI

code serve-web

Current status

This uses the same code as VS Code Desktop. See that for status.

code tunnel ✋

Current status

Blocked until a solution for Rust code is available.

vscode.dev :running:

https://vscode.dev, the web version of VS Code, is completely onboarded to MSAL.js. However, we may need to do some work to enable brokering in the web.

Next steps

• [ ] TBD: Investigate what's required for brokering in the web