Commit Graph

16 Commits

Author SHA1 Message Date
Tyler James Leonhardt 95ab795ff0 Detach authority/tenant from the PublicClientApp (#242719)
everything
2025-03-05 19:55:10 +01:00
Tyler James Leonhardt 5571308162 Force an update after acquiring a token interactively (#239539)
* Force an update after acquiring a token interactively

This will make sure the account cache is up-to-date before the acquireTokenInteractive ends.

A greater fix is maybe turning the accounts cache to be a promise... bit this is the candidate fix for now.

Fixes #235327

* also delete event
2025-02-03 23:56:45 +01:00
Tyler James Leonhardt e6cba379a4 Try asking for different claims in another request (#237531)
I'm hoping that this solution will be good enough until we stop depending on the id token for certain things.

Fixes https://github.com/microsoft/vscode/issues/237370
2025-01-08 23:55:18 +01:00
Tyler James Leonhardt 691eaea3bd Remove old code, simplify properties (#237512) 2025-01-08 19:11:01 +01:00
Tyler James Leonhardt 1410d77f6f reverse cancellation and sequencer (#237029)
When we cancel, then the promise should be cancelled. If we don't do this, we hang on the first interaction request until we timeout.

Fixes the 2nd point in https://github.com/microsoft/vscode/issues/236825#issuecomment-2563882150
2024-12-28 09:36:05 +01:00
Tyler James Leonhardt d55cb9a7a0 Use claims to force an idToken in Broker flow (#236623)
Looks like the Broker doesn't support `forceRefresh`... This is an alternative way of forcing a refresh.

Fixes https://github.com/microsoft/vscode/issues/229456
2024-12-19 19:25:37 +01:00
Tyler James Leonhardt b425f4802f Check idtoken expiration (#236011)
and force expiration in a similar way to the way MSAL does it for access tokens.

Fixes https://github.com/microsoft/vscode/issues/229456
2024-12-13 02:45:44 +01:00
Tyler James Leonhardt c0c6b07185 Disable MSAL for now (#235048)
Because of late breaking issues:

* https://github.com/microsoft/vscode/issues/234932
* https://github.com/microsoft/vscode/issues/234954

This also comments out the `_setupRefresh` logic since that is causing the high CPU load. I'd like to see what happens when we don't do this behavior.
2024-12-03 01:39:32 +00:00
Tyler James Leonhardt 893926f953 Always update refresh & sequencialize updates (#234716) 2024-11-27 05:38:53 -08:00
Tyler James Leonhardt 7ddb65bac8 Update logging & delete dead code (#234458)
* Update logging values so the logs aren't so noisy
* Delete a bunch of dead async code
2024-11-23 01:17:07 +01:00
Tyler James Leonhardt f6dd987698 No need for memento hack (#234450)
MSAL node made `clearCache` synchronous 🎉 so we can safely depend on it for clearing the cache.

> Context: The default behavior of MSAL's internal cache is that it is a union with what's in the persistant cache (secret storage) but what _we_ want is that secret storage is the source of truth, so every time we receive an update to secret storage, we clear the in-memory cache to get the data from the persistant cache.

Also bumps msal-node-extensions while we're at it.
2024-11-22 22:35:49 +01:00
Tyler James Leonhardt e5079d8a05 Migrate old accounts to MSAL (#234147)
Bascally, we reach into the old location in secret storage and if we find sessions (with a refresh token) we seed that in the MSAL world.

We do this one time... unless they switch back to the old world and then switch to the new world.

This has two different behaviors depending on if the Broker is used:
* If the broker is not used, this does what you might expect. It makes it seem totally transparent to the user that something has changed. All sessions get migrated over and the user is still logged in to what they were previously.
* If the broker is used... you don't get automatically logged in _unless_ you have already logged in to that account at the OS level. So this helps skip the "VS Code access layer" outlined in `accountAccess.ts`. Not as good as the previous bullet, but this is the best we can do in the broker world.

In time, we can remove this migration along with the old way of doing things.
2024-11-19 00:56:53 +01:00
Tyler James Leonhardt 305134296c Adopt the MSAL broker to talk to the OS for Microsoft auth (#233739)
This adopts the `NativeBrokerPlugin` provided by `@azure/msal-node-extensions` to provide the ability to use auth state from the OS, and show native auth dialogs instead of going to the browser.

This has several pieces:
* The adoption of the broker in the microsoft-authentication extension:
  * Adding `NativeBrokerPlugin` to our PCAs
  * Using the proposed handle API to pass the native window handle down to MSAL calls (btw, this API will change in a follow up PR)
  * Adopting an AccountAccess layer to handle:
    * giving the user control of which accounts VS Code uses
    * an eventing layer so that auth state can be updated across multiple windows
* Getting the extension to build properly and only build what it really needs. This required several package.json/webpack hacks:
  * Use a fake keytar since we don't use the feature in `@azure/msal-node-extensions` that uses keytar
  * Use a fake dpapi layer since we don't use the feature in  `@azure/msal-node-extensions` that uses it
  * Ensure the msal runtime `.node` and `.dll` files are included in the bundle
* Get the VS Code build to allow a native node module in an extension: by having a list of native extensions that will be built in the "ci" part of the build - in other words when VS Code is building on the target platform

There are a couple of followups:
* Refactor the `handle` API to handle (heh) Auxiliary Windows https://github.com/microsoft/vscode/issues/233106
* Separate the call to `acquireTokenSilent` and `acquireTokenInteractive` and all the usage of this native node module into a separate process or maybe in Core... we'll see. Something to experiment with after we have something working. NEEDS FOLLOW UP ISSUE

Fixes https://github.com/microsoft/vscode/issues/229431
2024-11-15 20:53:28 +09:00
Tyler James Leonhardt 3b1cb3b155 Bring back refresher (#229246)
Now we will refresh tokens over time so extensions can cache auth sessions in memory and just need to listen to the event to decide when to refresh their caches.
2024-09-21 01:26:53 +02:00
Tyler James Leonhardt b8be82f239 Workaround MSAL behavior (#228289)
* Workaround MSAL behavior

The main change this makes is around what scopes are being requested.

Due to an MSAL or Identity issue, if you request a resource like `FOO/user_impersonation` and then `email`... the 2nd call does not use Graph and instead uses FOO and FOO may not have an `email` scope available. To work around this, if we detect that all scopes being requested are [OIDC scopes](https://learn.microsoft.com/en-us/entra/identity-platform/scopes-oidc#openid-connect-scopes) then we tack on `User.Read` to make sure that what gets returned is in fact from Graph. This prevents an infinite loop that was happening before. MSAL/Identity should fix this behavior, but this works for now.

Additionally, MSAL does already tack on OIDC scopes to all requests so I removed the logic that adds those.

Couple small things:
* Make sure MSAL logs get logged (trace)
* Use a Sequencer to make sure acquireToken calls are done sequentially just in case.

* more comment
2024-09-12 02:29:29 +02:00
Tyler James Leonhardt 533d8ec6a5 Rework eventing for PCAs and fix a few bugs along the way (#227854)
A big change, but a good one... This addresses some core issues around how we manage multiple PublicClientApplications (which are an object that should be created for each set of clientId,authority). Previously, we were doing some pretty nasty things to detect when a new PCA was created/deleted and as a result it would cause infinite loops and the likes...

Now we've focused on managing that in SecretStorage by looking for a `publicClientApplications` key. This is all encapsulated in the new `PublicClientApplicationsSecretStorage`.

Since we no longer relied on that hack, we still needed some way to have a PCA inform that:
* accounts have changed
* the last account was removed (signaling that this PCA could be disposed of in `PublicClientApplicationsSecretStorage`)

Both of these events have been added to `CachedPublicClientApplication` (now in its own file) and are being used. (replacing the old `_accountChangeHandler` which was hacky... true events are cleaner).

Last thing in the eventing space is that I try to minimize calls to `_storePublicClientApplications` so to not spam events across SecretStorage. You can see this in my usage of `_doCreatePublicClientApplication` over `getOrCreate`.

Couple random other things:
* `changed` accounts are properly bubbled up in `_onDidChangeSessionsEmitter` which is needed when a token is refreshed
* `getSessions` when no scopes are passed in no longer causes new tokens to be minted
* we use to only remove the first account we found but in some cases there may be the same account across different PCAs, so there's a `return` that's removed in `authProvider.ts` that fixes this bug
* Logging is clearer and more verbose (in a good way)
2024-09-06 21:03:25 -07:00